Skip to content

fix(ssa): Change constraint message to "multiply"#9230

Merged
aakoshh merged 3 commits intomasterfrom
af/9225-fix-expand-signed-check-msg
Jul 17, 2025
Merged

fix(ssa): Change constraint message to "multiply"#9230
aakoshh merged 3 commits intomasterfrom
af/9225-fix-expand-signed-check-msg

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jul 17, 2025

Description

Problem*

Resolves #9225

Summary*

Changes the error message of the constraints inserted by the expand_signed_checks pass to match the operation.

Additional Context

check_signed_overflow handles Add, Sub and Mul, but the message only depended on whether the operation was Sub in which case it was "subtract", otherwise it was "add". The Mul strategy had its own error message with "multiply", and used the sub/add message after an addition operation; but that addition was part of the overall multiplication overflow check, so I think it's an implementation detail, and potentially causes confusion. It's not clear whether this distinction was deliberate, or the result of merging strategies.

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.

@aakoshh aakoshh changed the title Change error message to multiply fix(ssa): Change constraint message to "multiply" Jul 17, 2025
@aakoshh aakoshh requested a review from a team July 17, 2025 12:00
@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2025

Changes to Brillig bytecode sizes

Generated at commit: dad5924ca7c431b37de57bf0344c9771d2870dc6, compared to commit: 39a504c14ba855ab4eed7adf9b016ef07c48bc66

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
signed_overflow_in_else_regression_8617_inliner_max -3 ✅ -3.33%
signed_overflow_in_else_regression_8617_inliner_min -3 ✅ -3.33%
signed_overflow_in_else_regression_8617_inliner_zero -3 ✅ -3.33%

Full diff report 👇
Program Brillig opcodes (+/-) %
loop_invariant_regression_inliner_min 280 (-3) -1.06%
loop_invariant_regression_inliner_max 216 (-3) -1.37%
loop_invariant_regression_inliner_zero 216 (-3) -1.37%
signed_cmp_inliner_max 100 (-3) -2.91%
signed_cmp_inliner_min 100 (-3) -2.91%
signed_cmp_inliner_zero 100 (-3) -2.91%
signed_overflow_in_else_regression_8617_inliner_max 87 (-3) -3.33%
signed_overflow_in_else_regression_8617_inliner_min 87 (-3) -3.33%
signed_overflow_in_else_regression_8617_inliner_zero 87 (-3) -3.33%

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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: 5db8b9e Previous: 39a504c Ratio
rollup-block-root-empty 26.56 s 20.88 s 1.27

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

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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: b1ea586 Previous: 39a504c Ratio
test_report_zkpassport_noir_rsa_ 1 s 0 s +∞

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

CC: @TomAFrench

@aakoshh aakoshh enabled auto-merge July 17, 2025 12:18
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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: b1ea586 Previous: 39a504c Ratio
rollup-root 0.005 s 0.004 s 1.25

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

CC: @TomAFrench

@aakoshh aakoshh added this pull request to the merge queue Jul 17, 2025
Merged via the queue into master with commit 8797651 Jul 17, 2025
124 of 128 checks passed
@aakoshh aakoshh deleted the af/9225-fix-expand-signed-check-msg branch July 17, 2025 14:31
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 22, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(SSA): validate MakeArray instruction
(noir-lang/noir#9183)
chore(docs): Add links to ACIR and source reference docs
(noir-lang/noir#9260)
fix: comptime code not mutating shared ref to struct field
(noir-lang/noir#9250)
fix(acir_gen): Bail out of `handle_constant_index` when it encounters
`DynamicArray` (noir-lang/noir#9259)
feat: allow paths in l-values
(noir-lang/noir#9254)
fix: parse AsTraitPath in type expressions
(noir-lang/noir#9258)
chore: add acir-gen unit tests per ssa instruction (2)
(noir-lang/noir#9185)
fix(licm): Ensure that all nested loops the current block is part of are
guaranteed to execute (noir-lang/noir#9249)
chore: bump external pinned commits
(noir-lang/noir#9256)
chore: enforce clippy in `ssa_fuzzer`
(noir-lang/noir#9247)
chore: clippy (noir-lang/noir#9246)
chore: skip `ram_blowup_regression` on PRs
(noir-lang/noir#9231)
chore: mark bignum as expected to pass
(noir-lang/noir#9244)
fix: suggest traits via visible reexports if they are not directly
visible (noir-lang/noir#9242)
fix: bind self when type-checking AsTraitPath
(noir-lang/noir#9236)
chore(docs): Include list to hashing libraries at the top of the
relevant docs page (noir-lang/noir#9239)
fix(fuzz): Use scoping for variable dynamism
(noir-lang/noir#9233)
fix(ssa): Change constraint message to "multiply"
(noir-lang/noir#9230)
feat: Add `compiler_unstable_features` to `Nargo.toml`
(noir-lang/noir#9219)
chore(fuzz): Increase loop frequency in Brillig
(noir-lang/noir#9228)
chore: bump noir-edwards dep
(noir-lang/noir#9229)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 22, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(SSA): validate MakeArray instruction
(noir-lang/noir#9183)
chore(docs): Add links to ACIR and source reference docs
(noir-lang/noir#9260)
fix: comptime code not mutating shared ref to struct field
(noir-lang/noir#9250)
fix(acir_gen): Bail out of `handle_constant_index` when it encounters
`DynamicArray` (noir-lang/noir#9259)
feat: allow paths in l-values
(noir-lang/noir#9254)
fix: parse AsTraitPath in type expressions
(noir-lang/noir#9258)
chore: add acir-gen unit tests per ssa instruction (2)
(noir-lang/noir#9185)
fix(licm): Ensure that all nested loops the current block is part of are
guaranteed to execute (noir-lang/noir#9249)
chore: bump external pinned commits
(noir-lang/noir#9256)
chore: enforce clippy in `ssa_fuzzer`
(noir-lang/noir#9247)
chore: clippy (noir-lang/noir#9246)
chore: skip `ram_blowup_regression` on PRs
(noir-lang/noir#9231)
chore: mark bignum as expected to pass
(noir-lang/noir#9244)
fix: suggest traits via visible reexports if they are not directly
visible (noir-lang/noir#9242)
fix: bind self when type-checking AsTraitPath
(noir-lang/noir#9236)
chore(docs): Include list to hashing libraries at the top of the
relevant docs page (noir-lang/noir#9239)
fix(fuzz): Use scoping for variable dynamism
(noir-lang/noir#9233)
fix(ssa): Change constraint message to "multiply"
(noir-lang/noir#9230)
feat: Add `compiler_unstable_features` to `Nargo.toml`
(noir-lang/noir#9219)
chore(fuzz): Increase loop frequency in Brillig
(noir-lang/noir#9228)
chore: bump noir-edwards dep
(noir-lang/noir#9229)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Jan Beneš <janbenes1234@gmail.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

Development

Successfully merging this pull request may close these issues.

Unexpected error message in "expand signed checks"

2 participants