fix(mem2reg): Mark block parameters with unknown alias sets in presence of nested references by vezenovm · Pull Request #9629 · noir-lang/noir

vezenovm · 2025-08-25T20:05:39Z

Description

Problem*

Resolves #9501

Summary*

When handling the terminator we marked parameters as aliasing one another if we passed the same reference to multiple parameter slots. However, this fails to account for a reference being passed through a nested reference. If there are parameters with nested references we give up and assume all parameter references have unknown alias sets. We also were not totally handling unknown alias sets correctly.

In the last_store optimization we now check whether the alias set is unknown. If it is unknown we cannot remove the last store.
When setting last loads we do not do it if the reference has an unknown alias set.

Additional Context

The performance alerts are outdated. Check the benchmarks themselves to see that the numbers are mostly unchanged.

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.

…eferences

github-actions

ACVM Benchmarks

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`purely_sequential_opcodes`	`248716` ns/iter (`± 424`)	`250826` ns/iter (`± 807`)	`0.99`
`perfectly_parallel_opcodes`	`217909` ns/iter (`± 2655`)	`218933` ns/iter (`± 2677`)	`1.00`
`perfectly_parallel_batch_inversion_opcodes`	`2780017` ns/iter (`± 35883`)	`2782883` ns/iter (`± 5817`)	`1.00`

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

github-actions

Artifact Size

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`708.9` KB	`708.9` KB	`1`
`private-kernel-reset`	`2032.5` KB	`2032.5` KB	`1`
`private-kernel-tail`	`535.2` KB	`535.2` KB	`1`
`rollup-base-private`	`4319.6` KB	`4319.6` KB	`1`
`rollup-base-public`	`3332` KB	`3332` KB	`1`
`rollup-block-root-empty`	`3847.1` KB	`3847.1` KB	`1`
`rollup-block-root-single-tx`	`30747.8` KB	`30747.8` KB	`1`
`rollup-block-root`	`30780.5` KB	`30780.5` KB	`1`
`rollup-merge`	`187` KB	`187` KB	`1`
`rollup-root`	`388.9` KB	`388.9` KB	`1`
`semaphore-depth-10`	`631.5` KB	`631.5` KB	`1`
`sha512-100-bytes`	`525.5` KB	`525.5` KB	`1`

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

github-actions

Compilation Time

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`1.868` s	`1.67` s	`1.12`
`private-kernel-reset`	`7.676` s	`8.152` s	`0.94`
`private-kernel-tail`	`1.364` s	`1.312` s	`1.04`
`rollup-base-private`	`16.14` s	`16.2` s	`1.00`
`rollup-base-public`	`13.56` s	`13.1` s	`1.04`
`rollup-block-root-empty`	`20.14` s	`20.6` s	`0.98`
`rollup-block-root-single-tx`	`187` s	`195` s	`0.96`
`rollup-block-root`	`190` s	`198` s	`0.96`
`rollup-merge`	`1.398` s	`1.342` s	`1.04`
`rollup-root`	`1.468` s	`1.494` s	`0.98`
`semaphore-depth-10`	`0.777` s	`0.796` s	`0.98`
`sha512-100-bytes`	`1.6` s	`1.745` s	`0.92`

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

github-actions

Execution Time

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`0.014` s	`0.014` s	`1`
`private-kernel-reset`	`0.155` s	`0.154` s	`1.01`
`private-kernel-tail`	`0.01` s	`0.01` s	`1`
`rollup-base-private`	`0.269` s	`0.265` s	`1.02`
`rollup-base-public`	`0.161` s	`0.16` s	`1.01`
`rollup-block-root`	`13.1` s	`13` s	`1.01`
`rollup-merge`	`0.002` s	`0.002` s	`1`
`rollup-root`	`0.004` s	`0.004` s	`1`
`semaphore-depth-10`	`0.019` s	`0.019` s	`1`
`sha512-100-bytes`	`0.105` s	`0.104` s	`1.01`

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: `17ef4a9`	Previous: `d171555`	Ratio
`private-kernel-inner`	`2.484` s	`1.688` s	`1.47`
`private-kernel-reset`	`13.44` s	`7.77` s	`1.73`
`rollup-base-private`	`23.72` s	`15.38` s	`1.54`
`rollup-base-public`	`22.8` s	`13.22` s	`1.72`
`rollup-block-root-single-tx`	`264` s	`191` s	`1.38`
`rollup-block-root`	`247` s	`194` s	`1.27`
`semaphore-depth-10`	`0.934` s	`0.77` s	`1.21`
`sha512-100-bytes`	`2.072` s	`1.524` s	`1.36`

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

CC: @TomAFrench

github-actions

Opcode count

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`14792` opcodes	`14792` opcodes	`1`
`private-kernel-reset`	`68868` opcodes	`68868` opcodes	`1`
`private-kernel-tail`	`11177` opcodes	`11177` opcodes	`1`
`rollup-base-private`	`221335` opcodes	`221335` opcodes	`1`
`rollup-base-public`	`159954` opcodes	`159954` opcodes	`1`
`rollup-block-root-empty`	`68106` opcodes	`68106` opcodes	`1`
`rollup-block-root-single-tx`	`964513` opcodes	`964513` opcodes	`1`
`rollup-block-root`	`965799` opcodes	`965799` opcodes	`1`
`rollup-merge`	`1409` opcodes	`1409` opcodes	`1`
`rollup-root`	`2631` opcodes	`2631` opcodes	`1`
`semaphore-depth-10`	`5700` opcodes	`5700` opcodes	`1`
`sha512-100-bytes`	`13173` opcodes	`13173` opcodes	`1`

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

vezenovm · 2025-08-25T20:20:46Z

These are some pretty serious byte code regressions. I will have to investigate further to see if we can avoid them.

github-actions

Test Suite Duration

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr`	`98` s	`99` s	`0.99`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts`	`114` s	`107` s	`1.07`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob`	`172` s	`181` s	`0.95`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib`	`210` s	`215` s	`0.98`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib`	`33` s	`33` s	`1`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib`	`641` s	`704` s	`0.91`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types`	`109` s	`108` s	`1.01`
`test_report_noir-lang_noir-bignum_`	`505` s	`504` s	`1.00`
`test_report_noir-lang_noir_bigcurve_`	`284` s	`284` s	`1`
`test_report_noir-lang_sha256_`	`16` s	`16` s	`1`
`test_report_noir-lang_sha512_`	`15` s	`15` s	`1`
`test_report_zkpassport_noir-ecdsa_`	`3` s	`2` s	`1.50`
`test_report_zkpassport_noir_rsa_`	`0` s	`1` s	`0`

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: `e01a312`	Previous: `b7509f4`	Ratio
`test_report_zkpassport_noir-ecdsa_`	`3` s	`2` s	`1.50`

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

CC: @TomAFrench

github-actions

Execution Memory

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`206.72` MB	`206.72` MB	`1`
`private-kernel-reset`	`243.53` MB	`243.53` MB	`1`
`private-kernel-tail`	`195.94` MB	`195.94` MB	`1`
`rollup-base-private`	`500.25` MB	`500.25` MB	`1`
`rollup-base-public`	`432.9` MB	`432.9` MB	`1`
`rollup-block-root`	`1500` MB	`1500` MB	`1`
`rollup-merge`	`328.19` MB	`328.19` MB	`1`
`rollup-root`	`330.55` MB	`330.55` MB	`1`
`semaphore_depth_10`	`69.5` MB	`69.5` MB	`1`
`sha512_100_bytes`	`54.99` MB	`54.99` MB	`1`

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

github-actions

Compilation Memory

Details

Benchmark suite	Current: `e01a312`	Previous: `b7509f4`	Ratio
`private-kernel-inner`	`239.69` MB	`239.63` MB	`1.00`
`private-kernel-reset`	`549.1` MB	`549.1` MB	`1`
`private-kernel-tail`	`213.85` MB	`213.85` MB	`1`
`rollup-base-private`	`1350` MB	`1350` MB	`1`
`rollup-base-public`	`1400` MB	`1400` MB	`1`
`rollup-block-root-empty`	`1090` MB	`1090` MB	`1`
`rollup-block-root-single-tx`	`9580` MB	`9580` MB	`1`
`rollup-block-root`	`9590` MB	`9590` MB	`1`
`rollup-merge`	`330.56` MB	`330.56` MB	`1`
`rollup-root`	`341.34` MB	`341.34` MB	`1`
`semaphore_depth_10`	`104.76` MB	`104.76` MB	`1`
`sha512_100_bytes`	`233.14` MB	`233.17` MB	`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 Memory'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `17ef4a9`	Previous: `d171555`	Ratio
`private-kernel-reset`	`933.37` MB	`549.1` MB	`1.70`
`rollup-base-private`	`1830` MB	`1350` MB	`1.36`
`rollup-base-public`	`2190` MB	`1400` MB	`1.56`
`rollup-block-root-single-tx`	`13460` MB	`9580` MB	`1.41`
`rollup-block-root`	`13460` MB	`9590` MB	`1.40`

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

CC: @TomAFrench

vezenovm · 2025-08-26T18:08:12Z

The performance alerts are outdated. Check the benchmarks themselves to see that the numbers are mostly unchanged.

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: don't thread-bomb unnecessarily (noir-lang/noir#9643) chore(mem2reg): Only add to per function last_loads if load is not removed (noir-lang/noir#9647) chore(mem2reg): add a few regression tests (noir-lang/noir#9615) fix: Monomorphize function values as pairs of `(constrained, unconstrained)` (noir-lang/noir#9484) fix(mem2reg): Mark block parameters with unknown alias sets in presence of nested references (noir-lang/noir#9629) fix(ssa): Put some default in `Value::uninitialized` for references in the SSA interpreter (noir-lang/noir#9603) feat(ssa_fuzzer): ecdsa blackbox functions (noir-lang/noir#9584) fix(mem2reg): missing alias from block parameter to its argument (noir-lang/noir#9640) fix(acir_gen): A slice might be a nested Array, not a flattened DynamicArray (noir-lang/noir#9600) chore: add another mem2reg regression for #9613 (noir-lang/noir#9635) chore: document remove_if_else (in preparation for audit) (noir-lang/noir#9621) fix(mem2reg): Assume all function reference parameters have an unknown alias set with nested references (noir-lang/noir#9632) chore: add a regression test for #9613 (noir-lang/noir#9630) fix: Revert "feat(mem2reg): address last known value is independent of its… (noir-lang/noir#9628) fix(inlining): Do not inline globals and lower them during ACIR gen (noir-lang/noir#9626) chore(brillig): Include function name with `--count-array-copies` debug information (noir-lang/noir#9623) chore: use `assert_ssa_does_not_change` throughout all SSA tests (noir-lang/noir#9625) chore: only run remove_paired_rc in brillig functions (noir-lang/noir#9624) chore: some inlining refactors (noir-lang/noir#9622) chore(mem2reg): avoid redundant PostOrder computation (noir-lang/noir#9620) chore: bump external pinned commits (noir-lang/noir#9618) chore: document intrinsics (noir-lang/noir#9382) fix: Make inc/dec_rc impure (noir-lang/noir#9617) chore: greenlight `checked_to_unchecked` for audits (noir-lang/noir#9537) feat(mem2reg): address last known value is independent of its aliases (noir-lang/noir#9613) fix: Fix if-else alias in mem2reg (noir-lang/noir#9611) END_COMMIT_OVERRIDE

appropriately marked all reference parameters as unknown for nested r…

9077be8

…eferences

vezenovm added the bench-show Display benchmark results on PR label Aug 25, 2025

vezenovm added 2 commits August 25, 2025 20:06

handle arrays with nested refs as well

bb66b27

merge conflicts w/ master

17ef4a9

github-actions bot reviewed Aug 25, 2025

View reviewed changes

vezenovm marked this pull request as draft August 25, 2025 20:20

github-actions bot reviewed Aug 25, 2025

View reviewed changes

vezenovm added 2 commits August 26, 2025 16:52

fix regresions by accurately checking aliases when analyzing Store

c531158

merge conflicts w/ master

e01a312

vezenovm marked this pull request as ready for review August 26, 2025 18:07

vezenovm requested a review from a team August 26, 2025 18:08

asterite approved these changes Aug 26, 2025

View reviewed changes

vezenovm added this pull request to the merge queue Aug 26, 2025

Merged via the queue into master with commit 6870579 Aug 26, 2025
124 checks passed

vezenovm deleted the mv/block-params-aliasing-nested-refs branch August 26, 2025 19:05

noirwhal mentioned this pull request Aug 26, 2025

chore: Release Noir(1.0.0-beta.12) #9565

Merged

AztecBot mentioned this pull request Aug 27, 2025

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

Merged

Conversation

vezenovm commented Aug 25, 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 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

ACVM Benchmarks

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Artifact Size

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Compilation Time

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Execution Time

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Opcode count

Uh oh!

vezenovm commented Aug 25, 2025

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Test Suite Duration

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Execution Memory

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Compilation Memory

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

vezenovm commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vezenovm commented Aug 25, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading