fix(ssa): run simplify_cfg to a fixed point so no constant-condition jmpif survives - #13515
fix(ssa): run simplify_cfg to a fixed point so no constant-condition jmpif survives#13515AztecBot wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
ACVM Benchmarks
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
purely_sequential_opcodes |
124329 ns/iter (± 2282) |
174213 ns/iter (± 880) |
0.71 |
perfectly_parallel_opcodes |
104961 ns/iter (± 1590) |
142319 ns/iter (± 503) |
0.74 |
perfectly_parallel_batch_inversion_opcodes |
1727795 ns/iter (± 32226) |
2651776 ns/iter (± 2412) |
0.65 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Brillig Compilation Time
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
2.204 s |
1.518 s |
1.45 |
private-kernel-reset-tail |
2.26 s |
2.2 s |
1.03 |
private-kernel-reset |
1.798 s |
1.814 s |
0.99 |
rollup-block-root-first-empty-tx |
2.334 s |
2.414 s |
0.97 |
rollup-block-root-single-tx |
1.94 s |
2.48 s |
0.78 |
rollup-block-root |
2.62 s |
2.52 s |
1.04 |
rollup-checkpoint-merge |
2.478 s |
2.738 s |
0.91 |
rollup-checkpoint-root-single-block |
3.9 s |
3.04 s |
1.28 |
rollup-checkpoint-root |
3.87 s |
2.88 s |
1.34 |
rollup-root |
2.59 s |
2.588 s |
1.00 |
rollup-tx-base-private |
2.63 s |
2.804 s |
0.94 |
rollup-tx-base-public |
2.822 s |
2.708 s |
1.04 |
rollup-tx-merge |
2.07 s |
2.418 s |
0.86 |
semaphore-depth-10 |
0.412 s |
0.423 s |
0.97 |
sha512-100-bytes |
0.373 s |
0.374 s |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Compilation Time
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
6.92 s |
5.184 s |
1.33 |
private-kernel-reset-tail |
213 s |
225 s |
0.95 |
private-kernel-reset |
13.3 s |
13.56 s |
0.98 |
rollup-block-root-first-empty-tx |
2.424 s |
2.484 s |
0.98 |
rollup-block-root-single-tx |
1.99 s |
2.52 s |
0.79 |
rollup-block-root |
2.76 s |
2.58 s |
1.07 |
rollup-checkpoint-merge |
2.562 s |
2.902 s |
0.88 |
rollup-checkpoint-root-single-block |
266 s |
217 s |
1.23 |
rollup-checkpoint-root |
246 s |
230 s |
1.07 |
rollup-root |
2.758 s |
2.662 s |
1.04 |
rollup-tx-base-private |
15.88 s |
18.62 s |
0.85 |
rollup-tx-base-public |
51.18 s |
50.18 s |
1.02 |
rollup-tx-merge |
2.046 s |
2.484 s |
0.82 |
semaphore-depth-10 |
0.916 s |
0.97 s |
0.94 |
sha512-100-bytes |
1.75 s |
1.653 s |
1.06 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Brillig Execution Time
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
0.031 s |
0.023 s |
1.35 |
private-kernel-reset-tail |
0.048 s |
0.047 s |
1.02 |
private-kernel-reset |
0.008 s |
0.008 s |
1 |
rollup-block-root-first-empty-tx |
0.003 s |
0.003 s |
1 |
rollup-block-root-single-tx |
0.002 s |
0.002 s |
1 |
rollup-block-root |
0.003 s |
0.003 s |
1 |
rollup-checkpoint-merge |
0.001 s |
0.002 s |
0.50 |
rollup-root |
0.002 s |
0.002 s |
1 |
rollup-tx-base-private |
0.044 s |
0.048 s |
0.92 |
rollup-tx-base-public |
0.033 s |
0.033 s |
1 |
rollup-tx-merge |
0.001 s |
0.001 s |
1 |
semaphore-depth-10 |
0.007 s |
0.008 s |
0.88 |
sha512-100-bytes |
0.013 s |
0.013 s |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Elaboration Time
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
1.412 s |
1.014 s |
1.39 |
private-kernel-reset-tail |
1.26 s |
1.3 s |
0.97 |
private-kernel-reset |
1.234 s |
1.256 s |
0.98 |
rollup-block-root-first-empty-tx |
2.078 s |
2.118 s |
0.98 |
rollup-block-root-single-tx |
1.69 s |
2.16 s |
0.78 |
rollup-block-root |
2.27 s |
2.13 s |
1.07 |
rollup-checkpoint-merge |
2.164 s |
2.448 s |
0.88 |
rollup-checkpoint-root-single-block |
2.13 s |
1.64 s |
1.30 |
rollup-checkpoint-root |
2.09 s |
1.57 s |
1.33 |
rollup-root |
2.224 s |
2.152 s |
1.03 |
rollup-tx-base-private |
2.112 s |
2.238 s |
0.94 |
rollup-tx-base-public |
2.192 s |
2.076 s |
1.06 |
rollup-tx-merge |
1.764 s |
2.104 s |
0.84 |
semaphore-depth-10 |
0.279 s |
0.288 s |
0.97 |
sha512-100-bytes |
0.258 s |
0.257 s |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Execution Time
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
0.04 s |
0.03 s |
1.33 |
private-kernel-reset-tail |
0.176 s |
0.174 s |
1.01 |
private-kernel-reset |
0.116 s |
0.116 s |
1 |
rollup-block-root-first-empty-tx |
0.003 s |
0.003 s |
1 |
rollup-block-root-single-tx |
0.002 s |
0.003 s |
0.67 |
rollup-block-root |
0.004 s |
0.004 s |
1 |
rollup-checkpoint-merge |
0.002 s |
0.002 s |
1 |
rollup-checkpoint-root-single-block |
7.682 s |
7.196 s |
1.07 |
rollup-checkpoint-root |
8.966 s |
6.92 s |
1.30 |
rollup-root |
0.003 s |
0.003 s |
1 |
rollup-tx-base-private |
0.278 s |
0.307 s |
0.91 |
rollup-tx-base-public |
0.238 s |
0.238 s |
1 |
rollup-tx-merge |
0.002 s |
0.002 s |
1 |
semaphore-depth-10 |
0.008 s |
0.009 s |
0.89 |
sha512-100-bytes |
0.044 s |
0.043 s |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Opcode count
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
18980 opcodes |
18980 opcodes |
1 |
private-kernel-reset-tail |
96712 opcodes |
96712 opcodes |
1 |
private-kernel-reset |
80908 opcodes |
80908 opcodes |
1 |
rollup-block-root-first-empty-tx |
1038 opcodes |
1038 opcodes |
1 |
rollup-block-root-single-tx |
922 opcodes |
922 opcodes |
1 |
rollup-block-root |
2105 opcodes |
2105 opcodes |
1 |
rollup-checkpoint-merge |
1435 opcodes |
1435 opcodes |
1 |
rollup-checkpoint-root-single-block |
1388565 opcodes |
1388565 opcodes |
1 |
rollup-checkpoint-root |
1389745 opcodes |
1389745 opcodes |
1 |
rollup-root |
1797 opcodes |
1797 opcodes |
1 |
rollup-tx-base-private |
245177 opcodes |
245177 opcodes |
1 |
rollup-tx-base-public |
280542 opcodes |
280542 opcodes |
1 |
rollup-tx-merge |
1303 opcodes |
1303 opcodes |
1 |
semaphore-depth-10 |
5699 opcodes |
5699 opcodes |
1 |
sha512-100-bytes |
13173 opcodes |
13173 opcodes |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Artifact Size
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
1105.9 KB |
1105.9 KB |
1 |
private-kernel-reset-tail |
2681.5 KB |
2681.5 KB |
1 |
private-kernel-reset |
2113.5 KB |
2113.5 KB |
1 |
rollup-block-root-first-empty-tx |
276.9 KB |
276.9 KB |
1 |
rollup-block-root-single-tx |
280.4 KB |
280.4 KB |
1 |
rollup-block-root |
337.7 KB |
337.7 KB |
1 |
rollup-checkpoint-merge |
436.2 KB |
436.2 KB |
1 |
rollup-checkpoint-root-single-block |
30746.1 KB |
30746.1 KB |
1 |
rollup-checkpoint-root |
30777.3 KB |
30777.3 KB |
1 |
rollup-root |
470.8 KB |
470.8 KB |
1 |
rollup-tx-base-private |
4598.9 KB |
4598.9 KB |
1 |
rollup-tx-base-public |
5167.3 KB |
5167.3 KB |
1 |
rollup-tx-merge |
209.1 KB |
209.1 KB |
1 |
semaphore-depth-10 |
502.9 KB |
502.9 KB |
1 |
sha512-100-bytes |
449.9 KB |
449.9 KB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Brillig Artifact Size
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
823.4 KB |
823.4 KB |
1 |
private-kernel-reset-tail |
904.1 KB |
904.1 KB |
1 |
private-kernel-reset |
666.7 KB |
666.7 KB |
1 |
rollup-block-root-first-empty-tx |
324.7 KB |
324.7 KB |
1 |
rollup-block-root-single-tx |
328.6 KB |
328.6 KB |
1 |
rollup-block-root |
390.5 KB |
390.5 KB |
1 |
rollup-checkpoint-merge |
312.6 KB |
312.6 KB |
1 |
rollup-checkpoint-root-single-block |
690.6 KB |
690.6 KB |
1 |
rollup-checkpoint-root |
731.2 KB |
731.2 KB |
1 |
rollup-root |
489.9 KB |
489.9 KB |
1 |
rollup-tx-base-private |
691.3 KB |
691.3 KB |
1 |
rollup-tx-base-public |
854.6 KB |
854.6 KB |
1 |
rollup-tx-merge |
244.6 KB |
244.6 KB |
1 |
semaphore-depth-10 |
2073.7 KB |
2073.7 KB |
1 |
sha512-100-bytes |
151.4 KB |
151.4 KB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Compilation Memory
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
313.85 MB |
313.85 MB |
1 |
private-kernel-reset-tail |
548.48 MB |
548.48 MB |
1 |
private-kernel-reset |
524.69 MB |
524.69 MB |
1 |
rollup-block-root-first-empty-tx |
361.47 MB |
361.47 MB |
1 |
rollup-block-root-single-tx |
361.47 MB |
361.47 MB |
1 |
rollup-block-root |
362.34 MB |
362.34 MB |
1 |
rollup-checkpoint-merge |
361.47 MB |
361.47 MB |
1 |
rollup-checkpoint-root-single-block |
5800 MB |
5800 MB |
1 |
rollup-checkpoint-root |
5800 MB |
5800 MB |
1 |
rollup-root |
363.06 MB |
363.06 MB |
1 |
rollup-tx-base-private |
870.83 MB |
870.83 MB |
1 |
rollup-tx-base-public |
1340 MB |
1340 MB |
1 |
rollup-tx-merge |
361.47 MB |
361.47 MB |
1 |
semaphore_depth_10 |
103.76 MB |
103.76 MB |
1 |
sha512_100_bytes |
149.12 MB |
149.12 MB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Execution Memory
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
private-kernel-inner |
294.22 MB |
294.22 MB |
1 |
private-kernel-reset-tail |
335.23 MB |
335.23 MB |
1 |
private-kernel-reset |
321.54 MB |
321.54 MB |
1 |
rollup-block-root |
362.55 MB |
362.55 MB |
1 |
rollup-checkpoint-merge |
362.71 MB |
362.71 MB |
1 |
rollup-checkpoint-root-single-block |
1280 MB |
1280 MB |
1 |
rollup-checkpoint-root |
1280 MB |
1280 MB |
1 |
rollup-root |
362.95 MB |
362.95 MB |
1 |
rollup-tx-base-private |
468.33 MB |
468.33 MB |
1 |
rollup-tx-base-public |
480.69 MB |
480.69 MB |
1 |
rollup-tx-merge |
362.19 MB |
362.19 MB |
1 |
semaphore_depth_10 |
84.1 MB |
84.1 MB |
1 |
sha512_100_bytes |
77.92 MB |
77.92 MB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Test Suite Duration
Details
| Benchmark suite | Current: 0134f0a | Previous: c6b4e22 | Ratio |
|---|---|---|---|
test_report_AztecProtocol_aztec-packages_noir-projects_fnd_noir-protocol-circuits_crates_blob |
296 s |
220 s |
1.35 |
test_report_AztecProtocol_aztec-packages_noir-projects_fnd_noir-protocol-circuits_crates_private-kernel-lib |
1040 s |
847 s |
1.23 |
test_report_AztecProtocol_aztec-packages_noir-projects_fnd_noir-protocol-circuits_crates_types |
125 s |
159 s |
0.79 |
test_report_AztecProtocol_aztec-packages_noir-projects_labs_aztec-nr |
808 s |
817 s |
0.99 |
test_report_AztecProtocol_aztec-packages_noir-projects_labs_noir-contracts |
463 s |
472 s |
0.98 |
test_report_noir-lang_noir-bignum_ |
167 s |
172 s |
0.97 |
test_report_noir-lang_noir_bigcurve_ |
337 s |
334 s |
1.01 |
test_report_noir-lang_noir_json_parser_ |
14 s |
17 s |
0.82 |
test_report_noir-lang_sha256_ |
33 s |
34 s |
0.97 |
test_report_noir-lang_sha512_ |
13 s |
12 s |
1.08 |
test_report_zkpassport_noir-ecdsa_ |
5 s |
4 s |
1.25 |
test_report_zkpassport_noir_rsa_ |
2 s |
2 s |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
Problem
The nightly AST fuzzer run found seed
0x78416a6800100000failingvalid_after_pass:The backtrace shows
simplify_cfgfailing its own post-check:Function::simplify_functionaccumulates rewrites in aValueMappingwhile a worklist drains, but that mapping only reaches the terminators of already-visited blocks in the sweep that runs once the worklist is empty:A block is only re-queued while it is a predecessor of a block that just simplified, so an entry recorded after a block was last visited lands on that block's terminator for the first time in the final sweep — past the fold that would have turned a now-constant
jmpifinto ajmp.Instrumenting the final sweep on the seed shows exactly that:
In the seed the chain is:
b17foldsjmpif u1 0tojmp b26, strandingb25and everything reachable only through it (b27,b28,b31).b31carried the only back-edge into loop headerb15, sob15drops to a single predecessor and rule 3 ("remove block arguments for blocks with only a single predecessor") recordsv76 -> u1 1. The block branching onv76had already been visited.Note this is not a missing terminator refresh —
simplify_current_blockalready re-applies pending replacements to the terminator before inspecting it. The gap is that the block is never looked at again.Solution
Iterate to a fixed point, using the invariant the post-check already asserts as the termination signal.
simplify_function's body moves intosimplify_function_once, which returns whether it left a constant-conditionjmpifbehind:Termination: an extra round only runs because a constant-condition branch exists, and folding one strictly reduces the number of branches.
Confirmation that a second sweep is all that was missing: calling
simplify_function()twice makes the seed pass.Tests
folds_jmpif_whose_condition_becomes_constant_after_the_block_was_visitedinsimplify_cfg— 20 lines of SSA reproducing the chain above in miniature:b3folds its constantjmpif, stranding the only back-edge into headerb1, whose parameters are then replaced, recordingv2 -> u1 1for ab2that was already visited.Block ordering is load-bearing and the doc comment says so:
b2has to be popped beforeb3, which is why the header'sjmpiflistsb3in thethenslot.Verified on
origin/master(4bcac14457bb2560510d81f53314ce88fe3d4984, the commit CI ran on):Block b2 in function main has a JmpIf with a constant condition— the same failure mode as CI.cargo test -p noirc_evaluator— 1950 passed, 0 failed.NOIR_AST_FUZZER_SEED=0x78416a6800100000 cargo test -p noir_ast_fuzzer_fuzz valid_after_pass.cargo clippy -p noirc_evaluator --all-targetsandcargo fmt --all -- --checkclean.On the two-commit red/green split for snapshot tests in
CLAUDE.md: it cannot apply here. Without the fix the test panics inside the pass at the post-check, so there is no buggy snapshot to record in a red commit — the red state is a panic, not a diff.Unrelated harness nit spotted while debugging (not fixed here)
valid_after_passprints the failing AST only whenvalidate_each_passreturnsErr. This failure is anassert!inside the pass, so the panic escapes that path and CI logged no AST at all — the seed had to be re-run locally to see the program. Wrapping the per-pass call incatch_unwind, or printing the AST from a panic hook, would make these reports self-contained. Happy to do that separately if useful.Full RCA
https://gist.github.com/AztecBot/90b9f72e65d1334abb2d0848488e78d2 — part of a run-wide write-up covering all 9 seeds (5 distinct bugs).
Created by claudebox · group:
slackbot· requested by Tom (@TomAFrench) · Slack thread