feat: re-enable early mem2reg pass #9744

TomAFrench · 2025-09-05T14:48:22Z

Description

Problem*

Resolves

Summary*

Wanted to have a look at whether we can reenable this after the mem2reg internal review.

I've got a PR in aztec-packages to test whether this is going to break over there: AztecProtocol/aztec-packages#16814

Additional Context

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 · 2025-09-05T14:53:11Z

Changes to number of Brillig opcodes executed

Generated at commit: ed75482f870c9fcf89dc0d0a089c8cb247104642, compared to commit: 5e276642b138346845d42323992a9445eb2255f9

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
regression_6674_3_inliner_max	-60 ✅	-4.98%
regression_6674_3_inliner_zero	-60 ✅	-4.98%
a_7_function_inliner_max	-258 ✅	-12.58%
references_inliner_max	-40 ✅	-21.74%
regression_unused_nested_array_get_inliner_max	-59 ✅	-76.62%
regression_unused_nested_array_get_inliner_min	-59 ✅	-76.62%
regression_unused_nested_array_get_inliner_zero	-59 ✅	-76.62%

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_numeric_generic_poseidon_inliner_zero	4,492 (+114)	+2.60%
no_predicates_numeric_generic_poseidon_inliner_zero	4,492 (+114)	+2.60%
bench_2_to_17_inliner_zero	460,746 (+3,272)	+0.72%
hashmap_inliner_zero	71,978 (+504)	+0.71%
hashmap_inliner_min	80,498 (+552)	+0.69%
fold_2_to_17_inliner_zero	1,053,461 (+6,224)	+0.59%
fold_2_to_17_inliner_min	1,053,533 (+6,224)	+0.59%
bench_2_to_17_inliner_min	553,901 (+3,272)	+0.59%
fold_numeric_generic_poseidon_inliner_min	4,642 (+27)	+0.59%
no_predicates_numeric_generic_poseidon_inliner_min	4,642 (+27)	+0.59%
uhashmap_inliner_min	175,041 (+701)	+0.40%
uhashmap_inliner_zero	164,841 (+555)	+0.34%
array_sort_inliner_min	990 (+1)	+0.10%
regression_5252_inliner_min	940,367 (+30)	+0.00%
brillig_cow_regression_inliner_max	194,344 (-1)	-0.00%
brillig_cow_regression_inliner_zero	194,344 (-1)	-0.00%
fold_2_to_17_inliner_max	773,779 (-4)	-0.00%
regression_5252_inliner_zero	927,840 (-570)	-0.06%
poseidonsponge_x5_254_inliner_zero	186,372 (-120)	-0.06%
poseidonsponge_x5_254_inliner_max	152,735 (-122)	-0.08%
regression_5252_inliner_max	759,158 (-610)	-0.08%
poseidon_bn254_hash_width_3_inliner_max	137,192 (-119)	-0.09%
uhashmap_inliner_max	143,416 (-155)	-0.11%
array_to_slice_inliner_zero	1,703 (-4)	-0.23%
array_to_slice_inliner_max	949 (-4)	-0.42%
array_sort_inliner_max	868 (-4)	-0.46%
array_sort_inliner_zero	868 (-4)	-0.46%
numeric_type_alias_inliner_max	1,312 (-7)	-0.53%
numeric_type_alias_inliner_zero	1,312 (-7)	-0.53%
hashmap_inliner_max	56,630 (-326)	-0.57%
regression_6674_1_inliner_max	663 (-13)	-1.92%
regression_6674_2_inliner_max	663 (-13)	-1.92%
nested_array_dynamic_inliner_max	2,572 (-70)	-2.65%
references_inliner_zero	313 (-13)	-3.99%
nested_array_in_slice_inliner_max	1,097 (-47)	-4.11%
regression_6674_3_inliner_max	1,146 (-60)	-4.98%
regression_6674_3_inliner_zero	1,146 (-60)	-4.98%
a_7_function_inliner_max	1,793 (-258)	-12.58%
references_inliner_max	144 (-40)	-21.74%
regression_unused_nested_array_get_inliner_max	18 (-59)	-76.62%
regression_unused_nested_array_get_inliner_min	18 (-59)	-76.62%
regression_unused_nested_array_get_inliner_zero	18 (-59)	-76.62%

github-actions · 2025-09-05T14:53:14Z

Changes to Brillig bytecode sizes

Generated at commit: ed75482f870c9fcf89dc0d0a089c8cb247104642, compared to commit: 5e276642b138346845d42323992a9445eb2255f9

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
a_7_function_inliner_max	-125 ✅	-24.18%
regression_unused_nested_array_get_inliner_max	-14 ✅	-41.18%
regression_unused_nested_array_get_inliner_min	-14 ✅	-41.18%
regression_unused_nested_array_get_inliner_zero	-14 ✅	-41.18%
references_inliner_max	-62 ✅	-45.59%

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_numeric_generic_poseidon_inliner_zero	503 (+6)	+1.21%
no_predicates_numeric_generic_poseidon_inliner_zero	503 (+6)	+1.21%
regression_5252_inliner_zero	3,220 (-1)	-0.03%
poseidonsponge_x5_254_inliner_zero	2,888 (-1)	-0.03%
uhashmap_inliner_min	7,421 (-4)	-0.05%
hashmap_inliner_zero	7,967 (-6)	-0.08%
poseidon_bn254_hash_width_3_inliner_max	4,836 (-4)	-0.08%
brillig_cow_regression_inliner_max	1,201 (-1)	-0.08%
brillig_cow_regression_inliner_zero	1,201 (-1)	-0.08%
regression_5252_inliner_max	3,952 (-4)	-0.10%
poseidonsponge_x5_254_inliner_max	3,668 (-4)	-0.11%
uhashmap_inliner_zero	6,923 (-16)	-0.23%
array_sort_inliner_max	409 (-2)	-0.49%
array_sort_inliner_zero	409 (-2)	-0.49%
array_to_slice_inliner_zero	700 (-4)	-0.57%
hashmap_inliner_max	18,559 (-120)	-0.64%
array_to_slice_inliner_max	491 (-4)	-0.81%
fold_2_to_17_inliner_max	466 (-4)	-0.85%
uhashmap_inliner_max	11,098 (-96)	-0.86%
numeric_type_alias_inliner_max	326 (-7)	-2.10%
numeric_type_alias_inliner_zero	326 (-7)	-2.10%
nested_array_dynamic_inliner_max	1,638 (-78)	-4.55%
regression_6674_1_inliner_max	205 (-13)	-5.96%
regression_6674_2_inliner_max	205 (-13)	-5.96%
nested_array_in_slice_inliner_max	870 (-73)	-7.74%
references_inliner_zero	198 (-17)	-7.91%
regression_6674_3_inliner_max	410 (-64)	-13.50%
regression_6674_3_inliner_zero	410 (-64)	-13.50%
a_7_function_inliner_max	392 (-125)	-24.18%
regression_unused_nested_array_get_inliner_max	20 (-14)	-41.18%
regression_unused_nested_array_get_inliner_min	20 (-14)	-41.18%
regression_unused_nested_array_get_inliner_zero	20 (-14)	-41.18%
references_inliner_max	74 (-62)	-45.59%

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: `304e6da`	Previous: `14659f3`	Ratio
`private-kernel-inner`	`0.017` s	`0.014` s	`1.21`

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

CC: @TomAFrench

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: `304e6da`	Previous: `14659f3`	Ratio
`test_report_zkpassport_noir-ecdsa_`	`3` s	`2` s	`1.50`
`test_report_zkpassport_noir_rsa_`	`1` s	`0` s	`+∞`

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

CC: @TomAFrench

vezenovm

Good with me once CI passes on AztecProtocol/aztec-packages#16814

TomAFrench · 2025-09-05T15:41:02Z

~~I think the regressions here come from performing the LICM during the preprocessing step which results in us pulling the creation of some arrays up and out of a control-dependent loop.~~ We're eliminating some loads in preprocessing previously which we're not doing with the new mem2reg pass.

TomAFrench · 2025-09-05T16:49:20Z

Taking the SSA for bench_2_to_17 just after the preprocessing pass, an example of the problematic output:

brillig(inline) fn perform_duplex f5 {
  b0(v2: &mut [Field; 3], v3: &mut [Field; 4], v4: &mut u32, v5: &mut u1):
    jmp b1(u32 0)
  b1(v6: u32):
    v8 = lt v6, u32 3
    jmpif v8 then: b2, else: b3
  b2():
    v16 = load v2 -> [Field; 3]  <---- Here we load multiple references which are only used in b4. We could load these in b4 directly as there's no scope for the reference being mutated.
    v17 = load v3 -> [Field; 4]
    v18 = load v4 -> u32
    v19 = load v5 -> u1
    v20 = lt v6, v18
    jmpif v20 then: b4, else: b5
  b3():
    v9 = load v2 -> [Field; 3]
    v10 = load v3 -> [Field; 4]
    v11 = load v4 -> u32
    v12 = load v5 -> u1
    inc_rc v10
    v15 = call poseidon2_permutation(v10, u32 4) -> [Field; 4]
    store v9 at v2
    store v15 at v3
    store v11 at v4
    store v12 at v5
    return
  b4():
    v21 = lt v6, u32 4
    constrain v21 == u1 1, "Index out of bounds"
    v23 = array_get v17, index v6 -> Field
    v24 = lt v6, u32 3
    constrain v24 == u1 1, "Index out of bounds"
    v25 = array_get v16, index v6 -> Field
    v26 = add v23, v25
    v27 = lt v6, u32 4
    constrain v27 == u1 1, "Index out of bounds"
    v28 = array_set v17, index v6, value v26
    v30 = unchecked_add v6, u32 1
    store v16 at v2
    store v28 at v3
    store v18 at v4
    store v19 at v5
    jmp b5()
  b5():
    v31 = unchecked_add v6, u32 1
    jmp b1(v31)
}

This can be compared with

brillig(inline) fn perform_duplex f5 {
  b0(v2: &mut [Field; 3], v3: &mut [Field; 4], v4: &mut u32, v5: &mut u1):
    jmp b1(u32 0)
  b1(v6: u32):
    v8 = lt v6, u32 3
    jmpif v8 then: b2, else: b3
  b2():
    v16 = load v4 -> u32
    v17 = lt v6, v16
    jmpif v17 then: b4, else: b5
  b3():
    v9 = load v3 -> [Field; 4]
    inc_rc v9
    v12 = call poseidon2_permutation(v9, u32 4) -> [Field; 4]
    v13 = load v2 -> [Field; 3]
    v14 = load v4 -> u32
    v15 = load v5 -> u1
    store v13 at v2
    store v12 at v3
    store v14 at v4
    store v15 at v5
    return
  b4():
    v18 = load v3 -> [Field; 4]
    v19 = lt v6, u32 4
    constrain v19 == u1 1, "Index out of bounds"
    v21 = array_get v18, index v6 -> Field
    v22 = load v2 -> [Field; 3]
    v23 = lt v6, u32 3
    constrain v23 == u1 1, "Index out of bounds"
    v24 = array_get v22, index v6 -> Field
    v25 = add v21, v24
    v26 = load v2 -> [Field; 3]
    v27 = load v3 -> [Field; 4]
    v28 = load v4 -> u32
    v29 = load v5 -> u1
    v30 = lt v6, u32 4
    constrain v30 == u1 1, "Index out of bounds"
    v31 = array_set v27, index v6, value v25
    store v26 at v2
    store v31 at v3
    store v28 at v4
    store v29 at v5
    jmp b5()
  b5():
    v33 = unchecked_add v6, u32 1
    jmp b1(v33)
}

TomAFrench · 2025-09-05T17:10:43Z

I've created an issue to track the source of the regression (at least for bench_2_to_17_inliner_zero) in #9745

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: `dc956e8`	Previous: `5e27664`	Ratio
`rollup-base-private`	`18.94` s	`15.2` s	`1.25`
`rollup-base-public`	`16.84` s	`13.68` s	`1.23`

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

CC: @TomAFrench

TomAFrench added 2 commits September 5, 2025 14:47

feat: re-enable early mem2reg pass

2969ae8

snapshots

304e6da

github-actions bot reviewed Sep 5, 2025

View reviewed changes

TomAFrench requested review from asterite and vezenovm September 5, 2025 15:07

vezenovm approved these changes Sep 5, 2025

View reviewed changes

TomAFrench marked this pull request as ready for review September 5, 2025 16:24

TomAFrench added this pull request to the merge queue Sep 5, 2025

TomAFrench removed this pull request from the merge queue due to a manual request Sep 5, 2025

chore: add test showing poor mem2reg behaviour

93720af

TomAFrench mentioned this pull request Sep 5, 2025

mem2reg pass eliminates duplicate loads suboptimally #9745

Open

Merge branch 'master' into tf/enable-mem2reg

dc956e8

github-actions bot reviewed Sep 5, 2025

View reviewed changes

TomAFrench added this pull request to the merge queue Sep 5, 2025

Merged via the queue into master with commit b245272 Sep 5, 2025
122 checks passed

TomAFrench deleted the tf/enable-mem2reg branch September 5, 2025 18:07

noirwhal mentioned this pull request Sep 5, 2025

chore: Release Noir(1.0.0-beta.13) #9737

Open

AztecBot mentioned this pull request Sep 6, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#16723

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: re-enable early mem2reg pass #9744

feat: re-enable early mem2reg pass #9744

Uh oh!

TomAFrench commented Sep 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 5, 2025 •

edited

Loading

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

vezenovm left a comment

Uh oh!

TomAFrench commented Sep 5, 2025 •

edited

Loading

Uh oh!

TomAFrench commented Sep 5, 2025

Uh oh!

TomAFrench commented Sep 5, 2025 •

edited

Loading

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Uh oh!

feat: re-enable early mem2reg pass #9744

feat: re-enable early mem2reg pass #9744

Uh oh!

Conversation

TomAFrench commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem*

Summary*

Additional Context

Documentation*

PR Checklist*

Uh oh!

github-actions bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes to number of Brillig opcodes executed

🧾 Summary (10% most significant diffs)

Uh oh!

github-actions bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes to Brillig bytecode sizes

🧾 Summary (10% most significant diffs)

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

vezenovm left a comment

Choose a reason for hiding this comment

Uh oh!

TomAFrench commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomAFrench commented Sep 5, 2025

Uh oh!

TomAFrench commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Uh oh!

Uh oh!

TomAFrench commented Sep 5, 2025 •

edited

Loading

github-actions bot commented Sep 5, 2025 •

edited

Loading

github-actions bot commented Sep 5, 2025 •

edited

Loading

TomAFrench commented Sep 5, 2025 •

edited

Loading

TomAFrench commented Sep 5, 2025 •

edited

Loading