fix(ssa): Recursive shared Brillig entry points #8099
Merged
Conversation
…it test for updating cloned function call sites, and rewriting of entry point analysis
Contributor
There was a problem hiding this comment.
⚠️ 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: c1c2c63 | Previous: 38070d0 | Ratio |
|---|---|---|---|
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
53 s |
40 s |
1.32 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
5 tasks
michaeljklein
suggested changes
Apr 21, 2025
michaeljklein
approved these changes
Apr 22, 2025
vezenovm
commented
Apr 22, 2025
Contributor
There was a problem hiding this comment.
⚠️ 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: 4745eca | Previous: 20fdf4c | Ratio |
|---|---|---|---|
private-kernel-tail |
0.021 s |
0.017 s |
1.24 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
This was referenced Apr 23, 2025
github-merge-queue bot
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Apr 28, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: let `nargo test` use the new greybox fuzzer (noir-lang/noir#8234) chore(docs): add fixed bugs to list (noir-lang/noir#8201) fix(parser): do not use `Ident::default()` (noir-lang/noir#8224) feat(fuzz): Add experimental comptime code generator (noir-lang/noir#8207) fix: let SSA parser parse calls to print (noir-lang/noir#8232) feat: added 1, -1 case for inversions in brillig (noir-lang/noir#8225) chore: clippy (noir-lang/noir#8220) fix: don't take implicitly added named generics when checking where clauses (noir-lang/noir#8184) chore(docs): Defunctionalization and some minor cleanup (noir-lang/noir#8217) feat: add `--no-fuzz` and `--only-fuzz` to `nargo test` (noir-lang/noir#8213) chore: add regression test for panic encountered in bigcurve library. (noir-lang/noir#8211) fix: validate numeric types when passing them to a FunctionBuilder (noir-lang/noir#8215) chore: refactor constrainedness checks on trait impls (noir-lang/noir#8170) chore(test): Add smoke test for AST generation (noir-lang/noir#8048) chore: bump more dependencies (noir-lang/noir#8208) chore: Don't use `i1` in the AST fuzzer (noir-lang/noir#8204) fix: Do not panic if RHS constant in division has more bits than the operand (noir-lang/noir#8197) chore: bump cargo deps (noir-lang/noir#8205) fix: Handle truncating constants to 128 bits (noir-lang/noir#8180) fix!: disallow the `i1` type (noir-lang/noir#8172) chore(fuzz): Make sure `main` makes at least one call (noir-lang/noir#8202) fix: Use `get_local_or_global_instruction` in `try_optimize_array_set_from_previous_get` (noir-lang/noir#8200) fix: returns 0 for right shift overflow (noir-lang/noir#8189) feat: avoid overflow check when subtracting from a value that is the maximum for its bitsize (noir-lang/noir#8190) feat: optimize `checked_to_unchecked` to take into account multiplications (noir-lang/noir#8188) chore: document ssa `inline_simple_functions` (noir-lang/noir#8169) chore(docs): remove_unreachable SSA pass (noir-lang/noir#8196) chore: Change AST fuzzer recursion limit (noir-lang/noir#8173) chore(acir): Unify how we display witness indices (noir-lang/noir#8192) fix: Detect ABI change of fuzzing harnesses (noir-lang/noir#8193) chore(acir): Test that `BLACKBOX::RANGE` display the number of bits (noir-lang/noir#8191) fix(debugger): send idle at loop end + fix test (noir-lang/noir#8187) fix: Use `get_local_or_global_instruction` when simplifying `IfElse` (noir-lang/noir#8185) fix(parser): avoid using `Location::dummy()` (noir-lang/noir#8178) chore: Push a bug list (noir-lang/noir#8186) feat(greybox_fuzzer): Should_fail and should_fail_with (noir-lang/noir#8118) chore: move unsigned overflow check from acir/brillig to ssa (noir-lang/noir#8163) feat: location tree for debug_info (noir-lang/noir#7034) fix: Use `IntegerConstant` for loop boundaries in `unrolling` (noir-lang/noir#8094) fix(ssa): Recursive shared Brillig entry points (noir-lang/noir#8099) chore: enable '--pedantic-solving' on more tests (noir-lang/noir#7701) chore(readme): Update `acvm-repo` READMEs (noir-lang/noir#8150) chore(test): add `test_programs` dir for expected-panic tests (noir-lang/noir#8147) chore(docs): minor updates in solidity doc (noir-lang/noir#8160) feat(debugger): debug test functions (noir-lang/noir#7958) chore: migrate recursive proof test to ultrahonk (noir-lang/noir#8038) chore: bump glob package (noir-lang/noir#8159) chore: bump external pinned commits (noir-lang/noir#8139) chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs (noir-lang/noir#8158) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Ary Borenszweig <asterite@gmail.com> Co-authored-by: TomAFrench <tom@tomfren.ch>
github-merge-queue bot
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Apr 28, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: let `nargo test` use the new greybox fuzzer (noir-lang/noir#8234) chore(docs): add fixed bugs to list (noir-lang/noir#8201) fix(parser): do not use `Ident::default()` (noir-lang/noir#8224) feat(fuzz): Add experimental comptime code generator (noir-lang/noir#8207) fix: let SSA parser parse calls to print (noir-lang/noir#8232) feat: added 1, -1 case for inversions in brillig (noir-lang/noir#8225) chore: clippy (noir-lang/noir#8220) fix: don't take implicitly added named generics when checking where clauses (noir-lang/noir#8184) chore(docs): Defunctionalization and some minor cleanup (noir-lang/noir#8217) feat: add `--no-fuzz` and `--only-fuzz` to `nargo test` (noir-lang/noir#8213) chore: add regression test for panic encountered in bigcurve library. (noir-lang/noir#8211) fix: validate numeric types when passing them to a FunctionBuilder (noir-lang/noir#8215) chore: refactor constrainedness checks on trait impls (noir-lang/noir#8170) chore(test): Add smoke test for AST generation (noir-lang/noir#8048) chore: bump more dependencies (noir-lang/noir#8208) chore: Don't use `i1` in the AST fuzzer (noir-lang/noir#8204) fix: Do not panic if RHS constant in division has more bits than the operand (noir-lang/noir#8197) chore: bump cargo deps (noir-lang/noir#8205) fix: Handle truncating constants to 128 bits (noir-lang/noir#8180) fix!: disallow the `i1` type (noir-lang/noir#8172) chore(fuzz): Make sure `main` makes at least one call (noir-lang/noir#8202) fix: Use `get_local_or_global_instruction` in `try_optimize_array_set_from_previous_get` (noir-lang/noir#8200) fix: returns 0 for right shift overflow (noir-lang/noir#8189) feat: avoid overflow check when subtracting from a value that is the maximum for its bitsize (noir-lang/noir#8190) feat: optimize `checked_to_unchecked` to take into account multiplications (noir-lang/noir#8188) chore: document ssa `inline_simple_functions` (noir-lang/noir#8169) chore(docs): remove_unreachable SSA pass (noir-lang/noir#8196) chore: Change AST fuzzer recursion limit (noir-lang/noir#8173) chore(acir): Unify how we display witness indices (noir-lang/noir#8192) fix: Detect ABI change of fuzzing harnesses (noir-lang/noir#8193) chore(acir): Test that `BLACKBOX::RANGE` display the number of bits (noir-lang/noir#8191) fix(debugger): send idle at loop end + fix test (noir-lang/noir#8187) fix: Use `get_local_or_global_instruction` when simplifying `IfElse` (noir-lang/noir#8185) fix(parser): avoid using `Location::dummy()` (noir-lang/noir#8178) chore: Push a bug list (noir-lang/noir#8186) feat(greybox_fuzzer): Should_fail and should_fail_with (noir-lang/noir#8118) chore: move unsigned overflow check from acir/brillig to ssa (noir-lang/noir#8163) feat: location tree for debug_info (noir-lang/noir#7034) fix: Use `IntegerConstant` for loop boundaries in `unrolling` (noir-lang/noir#8094) fix(ssa): Recursive shared Brillig entry points (noir-lang/noir#8099) chore: enable '--pedantic-solving' on more tests (noir-lang/noir#7701) chore(readme): Update `acvm-repo` READMEs (noir-lang/noir#8150) chore(test): add `test_programs` dir for expected-panic tests (noir-lang/noir#8147) chore(docs): minor updates in solidity doc (noir-lang/noir#8160) feat(debugger): debug test functions (noir-lang/noir#7958) chore: migrate recursive proof test to ultrahonk (noir-lang/noir#8038) chore: bump glob package (noir-lang/noir#8159) chore: bump external pinned commits (noir-lang/noir#8139) chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs (noir-lang/noir#8158) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Ary Borenszweig <asterite@gmail.com> Co-authored-by: TomAFrench <tom@tomfren.ch>
github-merge-queue bot
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Apr 28, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: let `nargo test` use the new greybox fuzzer (noir-lang/noir#8234) chore(docs): add fixed bugs to list (noir-lang/noir#8201) fix(parser): do not use `Ident::default()` (noir-lang/noir#8224) feat(fuzz): Add experimental comptime code generator (noir-lang/noir#8207) fix: let SSA parser parse calls to print (noir-lang/noir#8232) feat: added 1, -1 case for inversions in brillig (noir-lang/noir#8225) chore: clippy (noir-lang/noir#8220) fix: don't take implicitly added named generics when checking where clauses (noir-lang/noir#8184) chore(docs): Defunctionalization and some minor cleanup (noir-lang/noir#8217) feat: add `--no-fuzz` and `--only-fuzz` to `nargo test` (noir-lang/noir#8213) chore: add regression test for panic encountered in bigcurve library. (noir-lang/noir#8211) fix: validate numeric types when passing them to a FunctionBuilder (noir-lang/noir#8215) chore: refactor constrainedness checks on trait impls (noir-lang/noir#8170) chore(test): Add smoke test for AST generation (noir-lang/noir#8048) chore: bump more dependencies (noir-lang/noir#8208) chore: Don't use `i1` in the AST fuzzer (noir-lang/noir#8204) fix: Do not panic if RHS constant in division has more bits than the operand (noir-lang/noir#8197) chore: bump cargo deps (noir-lang/noir#8205) fix: Handle truncating constants to 128 bits (noir-lang/noir#8180) fix!: disallow the `i1` type (noir-lang/noir#8172) chore(fuzz): Make sure `main` makes at least one call (noir-lang/noir#8202) fix: Use `get_local_or_global_instruction` in `try_optimize_array_set_from_previous_get` (noir-lang/noir#8200) fix: returns 0 for right shift overflow (noir-lang/noir#8189) feat: avoid overflow check when subtracting from a value that is the maximum for its bitsize (noir-lang/noir#8190) feat: optimize `checked_to_unchecked` to take into account multiplications (noir-lang/noir#8188) chore: document ssa `inline_simple_functions` (noir-lang/noir#8169) chore(docs): remove_unreachable SSA pass (noir-lang/noir#8196) chore: Change AST fuzzer recursion limit (noir-lang/noir#8173) chore(acir): Unify how we display witness indices (noir-lang/noir#8192) fix: Detect ABI change of fuzzing harnesses (noir-lang/noir#8193) chore(acir): Test that `BLACKBOX::RANGE` display the number of bits (noir-lang/noir#8191) fix(debugger): send idle at loop end + fix test (noir-lang/noir#8187) fix: Use `get_local_or_global_instruction` when simplifying `IfElse` (noir-lang/noir#8185) fix(parser): avoid using `Location::dummy()` (noir-lang/noir#8178) chore: Push a bug list (noir-lang/noir#8186) feat(greybox_fuzzer): Should_fail and should_fail_with (noir-lang/noir#8118) chore: move unsigned overflow check from acir/brillig to ssa (noir-lang/noir#8163) feat: location tree for debug_info (noir-lang/noir#7034) fix: Use `IntegerConstant` for loop boundaries in `unrolling` (noir-lang/noir#8094) fix(ssa): Recursive shared Brillig entry points (noir-lang/noir#8099) chore: enable '--pedantic-solving' on more tests (noir-lang/noir#7701) chore(readme): Update `acvm-repo` READMEs (noir-lang/noir#8150) chore(test): add `test_programs` dir for expected-panic tests (noir-lang/noir#8147) chore(docs): minor updates in solidity doc (noir-lang/noir#8160) feat(debugger): debug test functions (noir-lang/noir#7958) chore: migrate recursive proof test to ultrahonk (noir-lang/noir#8038) chore: bump glob package (noir-lang/noir#8159) chore: bump external pinned commits (noir-lang/noir#8139) chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs (noir-lang/noir#8158) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Ary Borenszweig <asterite@gmail.com> Co-authored-by: TomAFrench <tom@tomfren.ch>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem*
Resolves #8069
For more context surrounding what the Brillig entry point analysis pass is doing and why you can reference #7277 and the doc comments on the pass itself.
The current SSA for the new
brillig_entry_points_regression_8069test:Details
We can see how the calls to
f1andf2inf2are never replaced with duplicated entry points.We currently have a single mapping of old function ids to new function ids. This leads to IDs being muddled across entry points. We clone the functions and set-up the calls that needs updating in each function before actually performing the update. Thus, let's say we updated the function calls for f1, where the calls f1 and f2 map to f3 and f4 respectively. As the calls to update are already set-up, when we go to update the function calls for f2 (which should map to f5 and f6), we will resolve f2 to f3. The calls to update are set-up off the original Brillig entry points call graph, thus there will be no new functions to update for f3.
Summary*
This fix required a couple changes:
execution_successand a unit test tobrillig_entry_points.rscalls_to_update.New SSA after this PR:
Details
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.