fix(inlining): Use centralized CallGraph structure for inline info computation

[vezenovm]

Description

Problem*

Resolves #8448

Summary*

Use the new ir::call_graph::CallGraph structure for computing the SSA InlineInfos.

This was done as follows:

1. Compute the CallGraph
2. Compute cycles in the graph (recursive functions)
3. Compute recursive function weights and costs
4. Build an acyclic graph by removing cycles from our initial call graph
5. Run a topological sort on our acyclic graph, using petgraph::algo::toposort. toposort requires a graph without cycles. We could also use DfsPostOrder to do this but as we are already determining the graph cycles I chose to simply build a new acyclic graph.
6. Reverse the result of the topological sort. This is done as we want to process the weight of the leaves first. Compute the remaining inline information for the topologically sorted acyclic graph.

Additional Context

If curious about the bytecode size changes in uhashmap look at #8489 (comment) for more information. tldr; the bug this PR fixes causes the byte code size difference.

Next steps: I am going to add weighted edges to the call graph as to be able to move compute_callers and compute_callees into the CallGraph impl.

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: d21121d626940ba856484e8ca1f4ffb2847f308f, compared to commit: f4078e1e175b6003ed25215fb870639ee9900cb7

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_zero	+17 ❌	+0.25%
uhashmap_inliner_max	+14 ❌	+0.12%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_zero	6,898 (+17)	+0.25%
uhashmap_inliner_max	11,586 (+14)	+0.12%

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: d21121d626940ba856484e8ca1f4ffb2847f308f, compared to commit: f4078e1e175b6003ed25215fb870639ee9900cb7

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_max	+1,193 ❌	+0.83%
uhashmap_inliner_zero	+1,304 ❌	+0.74%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_max	144,422 (+1,193)	+0.83%
uhashmap_inliner_zero	176,879 (+1,304)	+0.74%

[vezenovm]

Program Brillig opcodes (+/-) %
uhashmap_inliner_zero +17 ❌ +0.25%
uhashmap_inliner_max +14 ❌ +0.12%

Investigating these bytecode size increases. As this PR fixes a bug where we were not marking mutually recursive functions accurately I could see that we were inlining a mutually recursive function somewhere we should not have been.

The large snapshots diffs look to largely be a renumbering of indices and debug symbol changes. Looking into these changes as well.

[vezenovm]

The large snapshots diffs look to largely be a renumbering of indices and debug symbol changes. Looking into these changes as well.

The 50k lines snapshot diff was due to a bug with mislabelling inline targets. Fixed in 6d4a7f1

[vezenovm]

uhashmap_inliner_max +14 ❌ +0.12%

This is due to the bug mislabelling mutually recursive functions that this PR fixes. We have the following insertion method in umap.nr of the stdlib:

Details

    // Insert key-value entry. In case key was already present, value is overridden.
    // docs:start:insert
    pub unconstrained fn insert<H>(&mut self, key: K, value: V)
    where
        K: Eq + Hash,
        B: BuildHasher<H>,
        H: Hasher,
    {
        // docs:end:insert
        self.try_resize();

        let hash = self.hash(key);

        for attempt in 0..self._table.len() {
            let index = self.quadratic_probe(hash, attempt as u32);
            let mut slot = self._table[index];
            let mut insert = false;

            // Either marked as deleted or has unset key-value.
            if slot.is_available() {
                insert = true;
                self._len += 1;
            } else {
                let (current_key, _) = slot.key_value_unchecked();
                if current_key == key {
                    insert = true;
                }
            }

            if insert {
                slot.set(key, value);
                self._table[index] = slot;
                break;
            }
        }
    }

    unconstrained fn try_resize<H>(&mut self)
    where
        B: BuildHasher<H>,
        K: Eq + Hash,
        H: Hasher,
    {
        if self.len() + 1 >= self.capacity() / 2 {
            let capacity = self.capacity() * 2;
            let mut new_map = UHashMap::with_hasher_and_capacity(self._build_hasher, capacity);

            for entry in self.entries() {
                new_map.insert(entry.0, entry.1);
            }
            *self = new_map;
        }
    }

It is obvious that try_resize is mutually recursive. During preprocess_fns we can debug print our inline infos:

[compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs:23:13] name = "try_resize"
[compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs:24:13] &info = InlineInfo {
    is_brillig_entry_point: false,
    is_acir_entry_point: false,
    is_recursive: false,
    should_inline: true,
    weight: 119,
    cost: -3,
}
[compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs:23:13] name = "with_hasher_and_capacity"
[compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs:24:13] &info = InlineInfo {
    is_brillig_entry_point: false,
    is_acir_entry_point: false,
    is_recursive: false,
    should_inline: true,
    weight: 19,
    cost: -4,
}

is_recursive for try_resize should be true. On master we will determine that try_resize is not an inline target. We have this logic in preprocess_fns:

            // Skip preprocessing heavy functions that gained most of their weight from transitive accumulation, which tend to be near the entry.
            // These can be processed later by the regular SSA passes.
            let is_heavy = transitive_weight > own_weight * 10;

            // Functions which are inline targets will be processed in later passes.
            // Here we want to treat the functions which will be inlined into them.
            let is_target =
                inline_infos.get(&id).map(|info| info.is_inline_target()).unwrap_or_default();

            if is_heavy || is_target {
                continue;
            }

Thus, on master we preprocess try_resize. After this PR we appropriately mark try_resize as recursive and thus it is determined to be a target and we do not preprocess the function. This is the cause of the byte code discrepancy.

[github-actions]

ACVM Benchmarks

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
purely_sequential_opcodes	273006 ns/iter (± 227)	267707 ns/iter (± 2096)	1.02
perfectly_parallel_opcodes	243354 ns/iter (± 5653)	239218 ns/iter (± 4787)	1.02
perfectly_parallel_batch_inversion_opcodes	3241633 ns/iter (± 45270)	3589293 ns/iter (± 3094)	0.90

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Compilation Time

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
private-kernel-inner	2.38 s	2.29 s	1.04
private-kernel-reset	6.992 s	6.44 s	1.09
private-kernel-tail	1.058 s	1.124 s	0.94
rollup-base-private	16.04 s	16.3 s	0.98
rollup-base-public	12.74 s	12.84 s	0.99
rollup-block-root-empty	1.344 s	1.31 s	1.03
rollup-block-root-single-tx	134 s	128 s	1.05
rollup-block-root	130 s	124 s	1.05
rollup-merge	1.248 s	1.104 s	1.13
rollup-root	1.632 s	1.644 s	0.99
semaphore-depth-10	0.875 s	0.821 s	1.07

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Artifact Size

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
private-kernel-inner	1127.1 KB	1127.1 KB	1
private-kernel-reset	2051 KB	2051 KB	1
private-kernel-tail	583.1 KB	583.1 KB	1
rollup-base-private	5123.2 KB	5123.2 KB	1
rollup-base-public	3941.9 KB	3941.9 KB	1
rollup-block-root-empty	256.8 KB	256.8 KB	1
rollup-block-root-single-tx	25679 KB	25679 KB	1
rollup-block-root	25706.2 KB	25706.2 KB	1
rollup-merge	181.7 KB	181.7 KB	1
rollup-root	473.4 KB	473.4 KB	1
semaphore-depth-10	636.4 KB	636.4 KB	1

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Execution Time

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
private-kernel-inner	0.028 s	0.028 s	1
private-kernel-reset	0.16 s	0.159 s	1.01
private-kernel-tail	0.011 s	0.011 s	1
rollup-base-private	0.306 s	0.306 s	1
rollup-base-public	0.196 s	0.192 s	1.02
rollup-block-root	11.2 s	11 s	1.02
rollup-merge	0.004 s	0.004 s	1
rollup-root	0.014 s	0.013 s	1.08
semaphore-depth-10	0.021 s	0.019 s	1.11

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Test Suite Duration

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr	66 s	66 s	1
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts	98 s	97 s	1.01
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	42 s	43 s	0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib	189 s	196 s	0.96
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib	193 s	192 s	1.01
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types	61 s	60 s	1.02
test_report_noir-lang_noir-bignum_	416 s	405 s	1.03
test_report_noir-lang_noir_bigcurve_	238 s	229 s	1.04
test_report_noir-lang_sha512_	29 s	28 s	1.04
test_report_zkpassport_noir_rsa_	22 s	24 s	0.92

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Compilation Memory

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
private-kernel-inner	313.29 MB	313.29 MB	1
private-kernel-reset	569.09 MB	569.09 MB	1
private-kernel-tail	228.21 MB	228.21 MB	1
rollup-base-private	1440 MB	1440 MB	1
rollup-base-public	1460 MB	1460 MB	1
rollup-block-root-empty	398.67 MB	398.67 MB	1
rollup-block-root-single-tx	7890 MB	7890 MB	1
rollup-block-root	7900 MB	7900 MB	1
rollup-merge	380.73 MB	380.74 MB	1.00
rollup-root	441.65 MB	441.66 MB	1.00
semaphore_depth_10	126.94 MB	126.94 MB	1

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Execution Memory

Details

Benchmark suite	Current: 980fd23	Previous: f4078e1	Ratio
private-kernel-inner	238.07 MB	238.07 MB	1
private-kernel-reset	260.32 MB	260.32 MB	1
private-kernel-tail	213.56 MB	213.56 MB	1
rollup-base-private	544.98 MB	544.98 MB	1
rollup-base-public	537.43 MB	537.43 MB	1
rollup-block-root	1440 MB	1440 MB	1
rollup-merge	366.5 MB	366.5 MB	1
rollup-root	372.43 MB	372.43 MB	1
semaphore_depth_10	91.5 MB	91.5 MB	1

This comment was automatically generated by workflow using github-action-benchmark.

[TomAFrench]

LGTM

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: cc33b16	Previous: 8cf48fc	Ratio
test_report_noir-lang_noir_bigcurve_	287 s	223 s	1.29

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 46e472c	Previous: f4078e1	Ratio
private-kernel-inner	3.022 s	2.29 s	1.32
rollup-block-root-empty	1.83 s	1.31 s	1.40

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench