Skip to content

fix(ssa): Do not remove unused checked binary ops#8303

Merged
TomAFrench merged 3 commits intomasterfrom
mv/do-not-remove-unused-checked-bin-ops
May 1, 2025
Merged

fix(ssa): Do not remove unused checked binary ops#8303
TomAFrench merged 3 commits intomasterfrom
mv/do-not-remove-unused-checked-bin-ops

Conversation

@vezenovm
Copy link
Contributor

@vezenovm vezenovm commented May 1, 2025

Description

Problem*

Resolves #8302
Resolves #8230
Resolves #8231
Resolves #8233

Summary*

We are removing all binary ops. As binary ops are side effectual based upon a predicate we should not be removing them.

Additional Context

In follow-ups we can check whether the predicate will always be false. But for now this is sufficient to get ACIR and Brillig aligned.

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
Copy link
Contributor

github-actions bot commented May 1, 2025

Changes to circuit sizes

Generated at commit: 9fcd84521f8a8039558f3a6c05ea345cc4a7b638, compared to commit: 9e69d4cd6235b9183cefa84c7f1bca6ea7057c4e

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_4449 +548 ❌ +685.00% +616 ❌ +27.91%
conditional_regression_underflow +15 ❌ +1500.00% +13 ❌ +20.31%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_4449 628 (+548) +685.00% 2,823 (+616) +27.91%
conditional_regression_underflow 16 (+15) +1500.00% 77 (+13) +20.31%
6_array 431 (+72) +20.06% 5,760 (+178) +3.19%
fold_after_inlined_calls 4 (+2) +100.00% 2,763 (+3) +0.11%

@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2025

Changes to Brillig bytecode sizes

Generated at commit: 9fcd84521f8a8039558f3a6c05ea345cc4a7b638, compared to commit: 9e69d4cd6235b9183cefa84c7f1bca6ea7057c4e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
brillig_fns_as_values_inliner_max +15 ❌ +68.18%
brillig_fns_as_values_inliner_min +15 ❌ +68.18%
brillig_fns_as_values_inliner_zero +15 ❌ +68.18%

Full diff report 👇
Program Brillig opcodes (+/-) %
brillig_fns_as_values_inliner_max 37 (+15) +68.18%
brillig_fns_as_values_inliner_min 37 (+15) +68.18%
brillig_fns_as_values_inliner_zero 37 (+15) +68.18%
conditional_regression_underflow_inliner_max 67 (+25) +59.52%
conditional_regression_underflow_inliner_min 67 (+25) +59.52%
conditional_regression_underflow_inliner_zero 67 (+25) +59.52%
brillig_calls_inliner_max 53 (+14) +35.90%
brillig_calls_inliner_zero 53 (+14) +35.90%
fold_after_inlined_calls_inliner_max 34 (+7) +25.93%
fold_after_inlined_calls_inliner_zero 34 (+7) +25.93%
fold_after_inlined_calls_inliner_min 42 (+7) +20.00%
regression_6734_inliner_min 68 (+7) +11.48%
6_array_inliner_max 341 (+4) +1.19%
6_array_inliner_zero 384 (+4) +1.05%
6_array_inliner_min 417 (+4) +0.97%

@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2025

Changes to number of Brillig opcodes executed

Generated at commit: 9fcd84521f8a8039558f3a6c05ea345cc4a7b638, compared to commit: 9e69d4cd6235b9183cefa84c7f1bca6ea7057c4e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
brillig_calls_inliner_max +11 ❌ +36.67%
brillig_calls_inliner_zero +11 ❌ +36.67%
brillig_fns_as_values_inliner_max +7 ❌ +35.00%
brillig_fns_as_values_inliner_min +7 ❌ +35.00%
brillig_fns_as_values_inliner_zero +7 ❌ +35.00%

Full diff report 👇
Program Brillig opcodes (+/-) %
brillig_calls_inliner_max 41 (+11) +36.67%
brillig_calls_inliner_zero 41 (+11) +36.67%
brillig_fns_as_values_inliner_max 27 (+7) +35.00%
brillig_fns_as_values_inliner_min 27 (+7) +35.00%
brillig_fns_as_values_inliner_zero 27 (+7) +35.00%
fold_after_inlined_calls_inliner_max 26 (+3) +13.04%
fold_after_inlined_calls_inliner_zero 26 (+3) +13.04%
fold_after_inlined_calls_inliner_min 38 (+3) +8.57%
regression_6734_inliner_min 68 (+3) +4.62%
6_array_inliner_max 1,427 (+3) +0.21%
6_array_inliner_zero 1,895 (+3) +0.16%
6_array_inliner_min 2,480 (+3) +0.12%

@vezenovm vezenovm requested a review from a team May 1, 2025 15:15
@TomAFrench TomAFrench enabled auto-merge May 1, 2025 15:28
@TomAFrench TomAFrench added this pull request to the merge queue May 1, 2025
Merged via the queue into master with commit ce0135a May 1, 2025
117 checks passed
@TomAFrench TomAFrench deleted the mv/do-not-remove-unused-checked-bin-ops branch May 1, 2025 15:48
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 2, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: disallow emitting multiple `MemoryInit` opcodes for the same block
(noir-lang/noir#8291)
fix(ssa): Remove unused calls to pure functions
(noir-lang/noir#8298)
fix(ssa): Do not remove unused checked binary ops
(noir-lang/noir#8303)
fix: Return zero and insert an assertion if RHS bit size is over the
limit in euclidian division
(noir-lang/noir#8294)
feat: remove unnecessary dynamic arrays when pushing onto slices
(noir-lang/noir#8287)
feat(testing): Add SSA interpreter for testing SSA
(noir-lang/noir#8115)
END_COMMIT_OVERRIDE

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants