fix(ssa): Remove the array cache from the function inserter

[vezenovm]

Description

Problem*

Resolves #8333

Take this reduction of the original program:

fn main() -> pub [[u8; 32]; 2] {
    let mut result = [[0; 32]; 2];
    let actions_data: [Field; 30] = [0; 30];
    for i in 0..1 {
        result = [actions_data[i].to_be_bytes::<32>(), actions_data[i + 1].to_be_bytes::<32>()];
    }
    result
}

As part of #6071 an array cache was added to the function inserter which is used during unrolling to prevent OOM panics. This array cache also hoists make array instructions out of the loop

noir/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs

Line 146 in 72ce94b

block = pre_loop;

In the above program we have a call whose result is used in a make array. That make array containing the call result is then hoisted out of the loop, but the call itself is not hoisted. This leads to a mismatch in the ordering of instructions, thus triggering a panic during inlining. The same panic can be triggerd by normalizing IDs after the unrolling pass (e.g. --show-ssa).

Summary*

#6071 was built before LICM and Brillig globals existed at all (we used to inline arrays wherever the global was used). The cache was a workaround to avoid compilation time/memory blow-ups for large constant arrays used in a loop. I chose to lean into LICM and remove the cache entirely. This allows us to simplify the function inserter and have better separation of concerns (hoisting vs. unrolling).

Additional Context

After this PR it may be worth considering to require LICM to precede unrolling. We currently always run LICM before unrolling, however, we may want to make this requirement more explicit.

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]

ACVM Benchmarks

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
purely_sequential_opcodes	253390 ns/iter (± 447)	261238 ns/iter (± 2167)	0.97
perfectly_parallel_opcodes	223239 ns/iter (± 7007)	232794 ns/iter (± 3185)	0.96
perfectly_parallel_batch_inversion_opcodes	3564917 ns/iter (± 7004)	3574022 ns/iter (± 18752)	1.00

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

[github-actions]

Artifact Size

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
private-kernel-inner	1132.6 KB	1132.6 KB	1
private-kernel-reset	2051 KB	2051 KB	1
private-kernel-tail	586.4 KB	586.4 KB	1
rollup-base-private	5129.1 KB	5129.1 KB	1
rollup-base-public	4070.5 KB	4070.5 KB	1
rollup-block-root-empty	256.7 KB	256.7 KB	1
rollup-block-root-single-tx	25697.8 KB	25697.8 KB	1
rollup-block-root	25721.5 KB	25721.5 KB	1
rollup-merge	181.6 KB	181.6 KB	1
rollup-root	416.5 KB	416.5 KB	1
semaphore-depth-10	636.3 KB	636.3 KB	1

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

[github-actions]

Compilation Time

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
private-kernel-inner	2.324 s	2.308 s	1.01
private-kernel-reset	6.952 s	6.428 s	1.08
private-kernel-tail	1.062 s	1.074 s	0.99
rollup-base-private	17.54 s	16.44 s	1.07
rollup-base-public	15.28 s	13.68 s	1.12
rollup-block-root-empty	1.416 s	1.524 s	0.93
rollup-block-root-single-tx	130 s	129 s	1.01
rollup-block-root	128 s	122 s	1.05
rollup-merge	1.194 s	1.192 s	1.00
rollup-root	1.694 s	1.668 s	1.02
semaphore-depth-10	0.834 s	0.91 s	0.92

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

[github-actions]

Execution Time

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
private-kernel-inner	0.028 s	0.028 s	1
private-kernel-reset	0.163 s	0.161 s	1.01
private-kernel-tail	0.011 s	0.011 s	1
rollup-base-private	0.316 s	0.312 s	1.01
rollup-base-public	0.202 s	0.194 s	1.04
rollup-block-root	11.7 s	11.5 s	1.02
rollup-merge	0.004 s	0.004 s	1
rollup-root	0.009 s	0.009 s	1
semaphore-depth-10	0.021 s	0.02 s	1.05

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

[github-actions]

Execution Memory

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
private-kernel-inner	242.04 MB	242.04 MB	1
private-kernel-reset	264.09 MB	264.09 MB	1
private-kernel-tail	217.23 MB	217.23 MB	1
rollup-base-private	577.89 MB	577.89 MB	1
rollup-base-public	574.21 MB	574.21 MB	1
rollup-block-root	1460 MB	1460 MB	1
rollup-merge	399.73 MB	399.73 MB	1
rollup-root	405.29 MB	405.29 MB	1
semaphore_depth_10	92.93 MB	92.93 MB	1

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

[github-actions]

Compilation Memory

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
private-kernel-inner	333.06 MB	333.11 MB	1.00
private-kernel-reset	572.37 MB	572.37 MB	1
private-kernel-tail	232.88 MB	232.89 MB	1.00
rollup-base-private	1470 MB	1470 MB	1
rollup-base-public	1630 MB	1630 MB	1
rollup-block-root-empty	429.54 MB	429.52 MB	1.00
rollup-block-root-single-tx	7240 MB	7240 MB	1
rollup-block-root	7250 MB	7250 MB	1
rollup-merge	413.83 MB	413.81 MB	1.00
rollup-root	468.04 MB	468.07 MB	1.00
semaphore_depth_10	128.37 MB	128.37 MB	1

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

[github-actions]

Test Suite Duration

Details

Benchmark suite	Current: 9f6c366	Previous: a16e848	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr	66 s	65 s	1.02
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts	96 s	98 s	0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	40 s	43 s	0.93
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib	197 s	185 s	1.06
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib	188 s	181 s	1.04
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types	62 s	60 s	1.03
test_report_noir-lang_noir-bignum_	382 s	379 s	1.01
test_report_noir-lang_noir_bigcurve_	234 s	264 s	0.89
test_report_noir-lang_sha512_	32 s	31 s	1.03
test_report_zkpassport_noir_rsa_	4 s	2 s	2

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

[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: 9f6c366	Previous: a16e848	Ratio
test_report_zkpassport_noir_rsa_	4 s	2 s	2

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

CC: @TomAFrench

[vezenovm]

Huh looks like all the reports aside semaphore are gone.

[TomAFrench]

Yeah, they got broken by a previous PR. 🤷

[vezenovm]

Yeah, they got broken by a previous PR. 🤷

Assumed as such, I just didn't have the chance to investigate yet.

[TomAFrench]

It needs a sync to be fixed.