feat(ssa): constant_folding with loop

[aakoshh]

Description

Problem*

Resolves #9963

Summary*

Alternative to #10013 that works more in line with the "restart from" idea in #9963 (comment)

During constant folding when we encounter a block b2 that repeats an instruction from b1, and hoists it into the common dominator b0, then we remember that after the current folding iteration is over, we should revisit b1 to deduplicate whatever that instruction was with the one now in b0, but before that visit b0 as well, because we need to rebuild the cache in order for the deduplication to happen in b1.

Once the current iteration is finished, we re-queue the blocks which got instructions hoisted into them. Here we had to options:

1. Reduce them to a single common dominator. This does potentially more work than necessary, but will visit blocks and build the cache in a straight forward order.
2. Start from multiple blocks, as long as they don't dominate each other. This reduces the number of blocks to revisit (don't have to do a prefix of unrelated blocks), but it can have tricky caching behaviour with the way we follow successors, when we reach blocks in different order than in the first iteration.

To make option 2 work, I switched to a new VisitOncePriorityQueue based on Reverse Post Order, which should ensure that blocks should be visited in the same order regardless where we start from.

Added a --constant-folding-max-iter option with a default value of 5 to set a limit.

Includes some fixes to simplifying and improvements to caching, described in #10013

Additional Context

The revisit mechanism in #10013 was difficult to reason about, and although tests were green, some noir-contracts in aztec-packages still hit ICE with unmapped instructions. I reckon the reason was that the cache was visible in revisits, so in a CFG like b0 -> b1 -> [b2, b3] -> b4 if we revisited b1 we saw instructions cached by b4, and started hoisting them into b0, or earlier versions of themselves.

In this version the cache is cleared and rebuilt from scratch in each iteration, so blocks should only see the ones that preceded them, but not themselves, for example.

It is also easier to control how many times we will go over the data.

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]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: c606da3	Previous: 0c5df89	Ratio
perfectly_parallel_batch_inversion_opcodes	2781784 ns/iter (± 22557)	2253936 ns/iter (± 1758)	1.23

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

CC: @TomAFrench

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: 94d0647b409bec36a1273ce403e2d127bb191a7f, compared to commit: 5e75f23559479aa4e2e95168b9c128bbce2fb622

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
lambda_from_array_inliner_max	-240 ✅	-5.81%
lambda_from_array_inliner_min	-240 ✅	-5.81%
lambda_from_array_inliner_zero	-240 ✅	-5.81%
binary_operator_overloading_inliner_max	-39 ✅	-11.27%
regression_9455_inliner_max	-25 ✅	-53.19%
regression_9455_inliner_min	-25 ✅	-53.19%
regression_9455_inliner_zero	-25 ✅	-53.19%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
tuple_inputs_inliner_min	288 (+1)	+0.35%
tuple_inputs_inliner_zero	288 (+1)	+0.35%
tuple_inputs_inliner_max	306 (+1)	+0.33%
slice_dynamic_index_inliner_max	1,694 (+4)	+0.24%
slice_dynamic_index_inliner_zero	1,694 (+4)	+0.24%
hashmap_inliner_zero	7,977 (-1)	-0.01%
uhashmap_inliner_min	7,186 (-2)	-0.03%
uhashmap_inliner_zero	6,922 (-2)	-0.03%
hashmap_inliner_min	8,895 (-3)	-0.03%
poseidon_bn254_hash_width_3_inliner_zero	4,423 (-2)	-0.05%
slice_dynamic_index_inliner_min	2,153 (-1)	-0.05%
regression_5252_inliner_max	4,019 (-2)	-0.05%
uhashmap_inliner_max	11,260 (-20)	-0.18%
hashmap_inliner_max	18,574 (-37)	-0.20%
slices_inliner_min	2,177 (-5)	-0.23%
slices_inliner_zero	1,642 (-5)	-0.30%
poseidon_bn254_hash_width_3_inliner_max	4,832 (-22)	-0.45%
slices_inliner_max	1,679 (-9)	-0.53%
nested_array_dynamic_inliner_max	1,601 (-10)	-0.62%
regression_11294_inliner_min	290 (-2)	-0.68%
poseidonsponge_x5_254_inliner_max	3,675 (-26)	-0.70%
nested_array_dynamic_inliner_min	1,327 (-10)	-0.75%
nested_array_dynamic_inliner_zero	1,327 (-10)	-0.75%
regression_9538_inliner_max	93 (-1)	-1.06%
regression_9538_inliner_min	93 (-1)	-1.06%
regression_9538_inliner_zero	93 (-1)	-1.06%
no_predicates_numeric_generic_poseidon_inliner_max	619 (-8)	-1.28%
fold_numeric_generic_poseidon_inliner_max	619 (-8)	-1.28%
array_oob_regression_7965_inliner_max	269 (-5)	-1.82%
array_oob_regression_7965_inliner_min	269 (-5)	-1.82%
array_oob_regression_7965_inliner_zero	269 (-5)	-1.82%
higher_order_functions_inliner_min	1,592 (-30)	-1.85%
regression_9496_inliner_max	534 (-12)	-2.20%
regression_9496_inliner_min	534 (-12)	-2.20%
regression_9496_inliner_zero	534 (-12)	-2.20%
higher_order_functions_inliner_zero	721 (-29)	-3.87%
lambda_from_array_inliner_max	3,888 (-240)	-5.81%
lambda_from_array_inliner_min	3,888 (-240)	-5.81%
lambda_from_array_inliner_zero	3,888 (-240)	-5.81%
binary_operator_overloading_inliner_max	307 (-39)	-11.27%
regression_9455_inliner_max	22 (-25)	-53.19%
regression_9455_inliner_min	22 (-25)	-53.19%
regression_9455_inliner_zero	22 (-25)	-53.19%

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: d4844ed	Previous: 5e75f23	Ratio
rollup-root	0.005 s	0.004 s	1.25

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

CC: @TomAFrench

[aakoshh]

It looks like https://github.com/noir-lang/noir/actions/runs/18049115787/job/51367229271?pr=10019 is still problematic and probably ran out of memory.

On the positive side https://github.com/noir-lang/noir/actions/runs/18049115779/job/51367215495?pr=10019 didn't fail.

[github-actions]

ACVM Benchmarks

Details

Benchmark suite	Current: 1ada822	Previous: 5e75f23	Ratio
purely_sequential_opcodes	251763 ns/iter (± 667)	252758 ns/iter (± 354)	1.00
perfectly_parallel_opcodes	223116 ns/iter (± 6045)	223559 ns/iter (± 1020)	1.00
perfectly_parallel_batch_inversion_opcodes	2780436 ns/iter (± 2782)	2780381 ns/iter (± 1825)	1.00

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

[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: 9078152	Previous: 052462c	Ratio
rollup-block-root-first-empty-tx	1.712 s	1.418 s	1.21

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

CC: @TomAFrench

[aakoshh]

The failure in https://github.com/noir-lang/noir/actions/runs/18196209420/job/51803638263 can be reproduced like this:

cargo run -q -p nargo_cli -- --program-dir ../aztec-packages/noir-projects/noir-protocol-circuits   test --force --silence-warnings --package private_kernel_lib --show-ssa --exact tests::previous_kernel_validator_builder::validate_common::private_log_length_exceeds_max__private_tail

The issue was starting from multiple blocks, which left some block in invalid state. I tried to reproduce it, but couldn't get the exact chain of caches and hoists right, so I just pushed a fix. For the record, this is the CFG that I tried with:

        // This test recreates a situation from `tests::previous_kernel_validator_builder::validate_common::private_log_length_exceeds_max__private_tail`
        // in private_kernel_lib in aztec-packages/noir-projects/noir-protocol-circuits where starting a second iteration from multiple blocks
        // lead to instructions going unmapped.
        //
        // We have a CFG like this:
        //       b2
        //      /  \
        //    b1    b4--\       b7
        //   /  \  /     \     /  \
        // b0    b3      b5--b6    b9
        //   \          /      \  /
        //    \--------/        b8
        //
        // Say that we find duplicates in b2/b3 and b7/b8, which result in instructions hoisted into b1 and b6.
        // We need to revisit b1 and b6. None of them dominate the other, but if we start from both, we will
        // b6 before we visit b5, which dominates it. If something then changes in b5, it can make the rest invalid.

        // visit order will be b0, b1, b5, b2, b3, b6, b4, b7, b8, b9
        let src = r#"
          brillig(inline) predicate_pure fn main f0 {
            b0(v0: u1, v1: u1, v11: u8):
              jmpif v0 then: b1, else: b5
            b1():
              jmpif v1 then: b2, else: b3
            b2():
              v2 = make_array [u8 0]: [u8; 1]      
              v3 = array_get v2, index u32 0        
              jmp b4()
            b3():
              v4 = make_array [u8 0]: [u8; 1]     
              v5 = array_get v4, index u32 0 
              jmp b4()
            b4():
              jmp b5()
            b5():
              v6 = make_array [u8 0]: [u8; 1]      
              v12 = array_set v6, index u32 0, value u8 2  
              v7 = array_get v6, index u32 0-> u8         
              jmp b6()
            b6():
              v8 = add v7, u8 100                        
              jmpif v0 then: b7, else: b8
            b7():
              v9 = make_array [u8 1] : [u8; 1]
              jmp b9()
            b8():
              v10 = make_array [u8 1] : [u8; 1]          
              jmp b9()
            b9():
              return
          }
        "#;

My aim was for b5 to change in a way after b6 was revisited in the 2nd iteration so that the referring to v7 becomes invalid. I can certainly force the order to be b6 and then b5, but so far couldn't find a combination of instructions that cause something that b6 uses from b5 to change its value ID. It would require something that b5 can reuse from cache in the 2nd iteration, something new from a block that dominates it that wasn't there in the 1st iteration.

[aakoshh]

I think I have another idea how to trigger this in a unit test, it's a bit more involved ♟️

Managed to do it in d3021ed

And I want to try using a priority queue keyed by the rank in the reverse-post-order, which should ensure identical ordering regardless of where we start from.