Skip to content

fix: incorrect max bit size in remove_bit_shifts#9585

Merged
asterite merged 6 commits intomasterfrom
ab/remove_bit_shifts_incorrect_exponent_decomposition
Aug 20, 2025
Merged

fix: incorrect max bit size in remove_bit_shifts#9585
asterite merged 6 commits intomasterfrom
ab/remove_bit_shifts_incorrect_exponent_decomposition

Conversation

@asterite
Copy link
Collaborator

Description

Problem

Resolves #9541

Summary

When decomposing an exponent into bits we always need to take into account the maximum number of bits the exponent type can hold. Previously if the exponent was casted from a smaller type, for example from u8 to u32, we'd use the maximum number of bits of values in u8, which is 8, then take ilog2 of that, which is 3, then add 1 to get 4. But values that fit in u8 that are less than 32, like 26, need more than 4 bits to be represented. Given that the maximum value to shift an u32 is 32, it makes sense that the maximum bits is the bits to represent 32.

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

github-actions bot commented Aug 20, 2025

Changes to circuit sizes

Generated at commit: 87e0b6668b229f9ccc76d5bccbb9641541c7e651, compared to commit: 52a8579901ba03c53cbb022d968435bb4f66e2c8

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
u16_support -4 ✅ -10.53% -5 ✅ -0.18%
bit_shifts_runtime -24 ✅ -4.41% -35 ✅ -0.72%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
u128_type 134 (-4) -2.90% 4,617 (-5) -0.11%
binary_operator_overloading 88 (-4) -4.35% 3,300 (-5) -0.15%
inactive_signed_bitshift 76 (-4) -5.00% 2,844 (-5) -0.18%
u16_support 34 (-4) -10.53% 2,792 (-5) -0.18%
bit_shifts_runtime 520 (-24) -4.41% 4,860 (-35) -0.72%

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: 3ea2e92 Previous: 910ed69 Ratio
test_report_zkpassport_noir-ecdsa_ 2 s 1 s 2

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

CC: @TomAFrench

@asterite asterite requested a review from a team August 20, 2025 17:05
@asterite asterite added this pull request to the merge queue Aug 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2025
@asterite asterite added this pull request to the merge queue Aug 20, 2025
@asterite asterite removed this pull request from the merge queue due to a manual request Aug 20, 2025
@asterite asterite enabled auto-merge August 20, 2025 19:43
@asterite asterite added this pull request to the merge queue Aug 20, 2025
Merged via the queue into master with commit 0b8c415 Aug 20, 2025
119 of 122 checks passed
@asterite asterite deleted the ab/remove_bit_shifts_incorrect_exponent_decomposition branch August 20, 2025 20:27
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 22, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: some mem2reg refactors regarding expressions and aliases (noir-lang/noir#9610)
feat: keep last loads from predecessors in mem2reg (noir-lang/noir#9492)
chore: Update flattening docs (noir-lang/noir#9588)
chore: remove redundant globals creation (noir-lang/noir#9606)
chore: simplify Expression in mem2reg (noir-lang/noir#9599)
chore: remove duplicate `contains_reference` in mem2reg (noir-lang/noir#9602)
chore!: remove `verify_signature_slice` methods from stdlib (noir-lang/noir#9597)
fix(expand): correctly handle nested dereferences (noir-lang/noir#9598)
fix(ssa): Do not simplify on lhs being zero for shifts (noir-lang/noir#9596)
chore: store last loads in `HashSet` instead of `HashMap` in mem2reg (noir-lang/noir#9498)
chore: `--no-ssa-locations` for `nargo interpret` and show negative values when printing SSA (noir-lang/noir#9586)
fix: `assert_constant` refactors and fixes from audit (noir-lang/noir#9547)
fix(ssa): Consider `shl` and `shr` to have side effects (noir-lang/noir#9580)
fix: avoid invalid cast in `remove_bit_shifts` (noir-lang/noir#9570)
fix(mem2reg): Consider aliases of a loaded address to be loaded from as well (noir-lang/noir#9567)
fix: Consume correct number of fields when printing references (noir-lang/noir#9579)
chore: Add a section for numeric type aliases (noir-lang/noir#9589)
chore(remove_paired_rc): Add various unit tests (noir-lang/noir#9425)
fix: incorrect max bit size in `remove_bit_shifts` (noir-lang/noir#9585)
chore(ssa): Simplify shl/shr identity operations (noir-lang/noir#9587)
chore: greenlight `brillig_array_get_and_set` for audits (noir-lang/noir#9540)
chore(ssa): Update comments on `loop_invariant` for audit and some missing unit tests (noir-lang/noir#9574)
chore: Switch to node v22.15.0 (noir-lang/noir#9582)
chore: Update unrolling docs for audit (noir-lang/noir#9572)
chore: greenlight `array_set_optimization` (noir-lang/noir#9560)
fix(acir_gen): Keep range checks before side effects (noir-lang/noir#9340)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 22, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: some mem2reg refactors regarding expressions and aliases
(noir-lang/noir#9610)
feat: keep last loads from predecessors in mem2reg
(noir-lang/noir#9492)
chore: Update flattening docs
(noir-lang/noir#9588)
chore: remove redundant globals creation
(noir-lang/noir#9606)
chore: simplify Expression in mem2reg
(noir-lang/noir#9599)
chore: remove duplicate `contains_reference` in mem2reg
(noir-lang/noir#9602)
chore!: remove `verify_signature_slice` methods from stdlib
(noir-lang/noir#9597)
fix(expand): correctly handle nested dereferences
(noir-lang/noir#9598)
fix(ssa): Do not simplify on lhs being zero for shifts
(noir-lang/noir#9596)
chore: store last loads in `HashSet` instead of `HashMap` in mem2reg
(noir-lang/noir#9498)
chore: `--no-ssa-locations` for `nargo interpret` and show negative
values when printing SSA (noir-lang/noir#9586)
fix: `assert_constant` refactors and fixes from audit
(noir-lang/noir#9547)
fix(ssa): Consider `shl` and `shr` to have side effects
(noir-lang/noir#9580)
fix: avoid invalid cast in `remove_bit_shifts`
(noir-lang/noir#9570)
fix(mem2reg): Consider aliases of a loaded address to be loaded from as
well (noir-lang/noir#9567)
fix: Consume correct number of fields when printing references
(noir-lang/noir#9579)
chore: Add a section for numeric type aliases
(noir-lang/noir#9589)
chore(remove_paired_rc): Add various unit tests
(noir-lang/noir#9425)
fix: incorrect max bit size in `remove_bit_shifts`
(noir-lang/noir#9585)
chore(ssa): Simplify shl/shr identity operations
(noir-lang/noir#9587)
chore: greenlight `brillig_array_get_and_set` for audits
(noir-lang/noir#9540)
chore(ssa): Update comments on `loop_invariant` for audit and some
missing unit tests (noir-lang/noir#9574)
chore: Switch to node v22.15.0
(noir-lang/noir#9582)
chore: Update unrolling docs for audit
(noir-lang/noir#9572)
chore: greenlight `array_set_optimization`
(noir-lang/noir#9560)
fix(acir_gen): Keep range checks before side effects
(noir-lang/noir#9340)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 23, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: some mem2reg refactors regarding expressions and aliases
(noir-lang/noir#9610)
feat: keep last loads from predecessors in mem2reg
(noir-lang/noir#9492)
chore: Update flattening docs
(noir-lang/noir#9588)
chore: remove redundant globals creation
(noir-lang/noir#9606)
chore: simplify Expression in mem2reg
(noir-lang/noir#9599)
chore: remove duplicate `contains_reference` in mem2reg
(noir-lang/noir#9602)
chore!: remove `verify_signature_slice` methods from stdlib
(noir-lang/noir#9597)
fix(expand): correctly handle nested dereferences
(noir-lang/noir#9598)
fix(ssa): Do not simplify on lhs being zero for shifts
(noir-lang/noir#9596)
chore: store last loads in `HashSet` instead of `HashMap` in mem2reg
(noir-lang/noir#9498)
chore: `--no-ssa-locations` for `nargo interpret` and show negative
values when printing SSA (noir-lang/noir#9586)
fix: `assert_constant` refactors and fixes from audit
(noir-lang/noir#9547)
fix(ssa): Consider `shl` and `shr` to have side effects
(noir-lang/noir#9580)
fix: avoid invalid cast in `remove_bit_shifts`
(noir-lang/noir#9570)
fix(mem2reg): Consider aliases of a loaded address to be loaded from as
well (noir-lang/noir#9567)
fix: Consume correct number of fields when printing references
(noir-lang/noir#9579)
chore: Add a section for numeric type aliases
(noir-lang/noir#9589)
chore(remove_paired_rc): Add various unit tests
(noir-lang/noir#9425)
fix: incorrect max bit size in `remove_bit_shifts`
(noir-lang/noir#9585)
chore(ssa): Simplify shl/shr identity operations
(noir-lang/noir#9587)
chore: greenlight `brillig_array_get_and_set` for audits
(noir-lang/noir#9540)
chore(ssa): Update comments on `loop_invariant` for audit and some
missing unit tests (noir-lang/noir#9574)
chore: Switch to node v22.15.0
(noir-lang/noir#9582)
chore: Update unrolling docs for audit
(noir-lang/noir#9572)
chore: greenlight `array_set_optimization`
(noir-lang/noir#9560)
fix(acir_gen): Keep range checks before side effects
(noir-lang/noir#9340)
END_COMMIT_OVERRIDE
mralj pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: some mem2reg refactors regarding expressions and aliases (noir-lang/noir#9610)
feat: keep last loads from predecessors in mem2reg (noir-lang/noir#9492)
chore: Update flattening docs (noir-lang/noir#9588)
chore: remove redundant globals creation (noir-lang/noir#9606)
chore: simplify Expression in mem2reg (noir-lang/noir#9599)
chore: remove duplicate `contains_reference` in mem2reg (noir-lang/noir#9602)
chore!: remove `verify_signature_slice` methods from stdlib (noir-lang/noir#9597)
fix(expand): correctly handle nested dereferences (noir-lang/noir#9598)
fix(ssa): Do not simplify on lhs being zero for shifts (noir-lang/noir#9596)
chore: store last loads in `HashSet` instead of `HashMap` in mem2reg (noir-lang/noir#9498)
chore: `--no-ssa-locations` for `nargo interpret` and show negative values when printing SSA (noir-lang/noir#9586)
fix: `assert_constant` refactors and fixes from audit (noir-lang/noir#9547)
fix(ssa): Consider `shl` and `shr` to have side effects (noir-lang/noir#9580)
fix: avoid invalid cast in `remove_bit_shifts` (noir-lang/noir#9570)
fix(mem2reg): Consider aliases of a loaded address to be loaded from as well (noir-lang/noir#9567)
fix: Consume correct number of fields when printing references (noir-lang/noir#9579)
chore: Add a section for numeric type aliases (noir-lang/noir#9589)
chore(remove_paired_rc): Add various unit tests (noir-lang/noir#9425)
fix: incorrect max bit size in `remove_bit_shifts` (noir-lang/noir#9585)
chore(ssa): Simplify shl/shr identity operations (noir-lang/noir#9587)
chore: greenlight `brillig_array_get_and_set` for audits (noir-lang/noir#9540)
chore(ssa): Update comments on `loop_invariant` for audit and some missing unit tests (noir-lang/noir#9574)
chore: Switch to node v22.15.0 (noir-lang/noir#9582)
chore: Update unrolling docs for audit (noir-lang/noir#9572)
chore: greenlight `array_set_optimization` (noir-lang/noir#9560)
fix(acir_gen): Keep range checks before side effects (noir-lang/noir#9340)
END_COMMIT_OVERRIDE
ludamad pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Dec 16, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: some mem2reg refactors regarding expressions and aliases (noir-lang/noir#9610)
feat: keep last loads from predecessors in mem2reg (noir-lang/noir#9492)
chore: Update flattening docs (noir-lang/noir#9588)
chore: remove redundant globals creation (noir-lang/noir#9606)
chore: simplify Expression in mem2reg (noir-lang/noir#9599)
chore: remove duplicate `contains_reference` in mem2reg (noir-lang/noir#9602)
chore!: remove `verify_signature_slice` methods from stdlib (noir-lang/noir#9597)
fix(expand): correctly handle nested dereferences (noir-lang/noir#9598)
fix(ssa): Do not simplify on lhs being zero for shifts (noir-lang/noir#9596)
chore: store last loads in `HashSet` instead of `HashMap` in mem2reg (noir-lang/noir#9498)
chore: `--no-ssa-locations` for `nargo interpret` and show negative values when printing SSA (noir-lang/noir#9586)
fix: `assert_constant` refactors and fixes from audit (noir-lang/noir#9547)
fix(ssa): Consider `shl` and `shr` to have side effects (noir-lang/noir#9580)
fix: avoid invalid cast in `remove_bit_shifts` (noir-lang/noir#9570)
fix(mem2reg): Consider aliases of a loaded address to be loaded from as well (noir-lang/noir#9567)
fix: Consume correct number of fields when printing references (noir-lang/noir#9579)
chore: Add a section for numeric type aliases (noir-lang/noir#9589)
chore(remove_paired_rc): Add various unit tests (noir-lang/noir#9425)
fix: incorrect max bit size in `remove_bit_shifts` (noir-lang/noir#9585)
chore(ssa): Simplify shl/shr identity operations (noir-lang/noir#9587)
chore: greenlight `brillig_array_get_and_set` for audits (noir-lang/noir#9540)
chore(ssa): Update comments on `loop_invariant` for audit and some missing unit tests (noir-lang/noir#9574)
chore: Switch to node v22.15.0 (noir-lang/noir#9582)
chore: Update unrolling docs for audit (noir-lang/noir#9572)
chore: greenlight `array_set_optimization` (noir-lang/noir#9560)
fix(acir_gen): Keep range checks before side effects (noir-lang/noir#9340)
END_COMMIT_OVERRIDE
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.

ACIR != Brillig: Field failed to decompose into specified 4 limbs

2 participants