Skip to content

chore(ssa): Simplify shl/shr identity operations#9587

Merged
vezenovm merged 1 commit intomasterfrom
mv/simplify-shl-shr
Aug 20, 2025
Merged

chore(ssa): Simplify shl/shr identity operations#9587
vezenovm merged 1 commit intomasterfrom
mv/simplify-shl-shr

Conversation

@vezenovm
Copy link
Contributor

Description

Problem*

Resolves something I noticed while reviewing #9585

Summary*

Shifting by an lhs by zero will always result in the provided lhs.
The same is true for when the lhs is zero. Shifting zeroes will simply produce another number of zeroes.

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.

@vezenovm vezenovm requested a review from a team August 20, 2025 19:32
@vezenovm vezenovm added this pull request to the merge queue Aug 20, 2025
Merged via the queue into master with commit fb01ee5 Aug 20, 2025
122 checks passed
@vezenovm vezenovm deleted the mv/simplify-shl-shr branch August 20, 2025 20:17
@TomAFrench
Copy link
Member

@vezenovm Doesn't this allow you to shift a zero value by greater than the size of the type without an error?

@TomAFrench
Copy link
Member

We should maintain this as an error rather than optimizing it out.

@vezenovm
Copy link
Contributor Author

@vezenovm Doesn't this allow you to shift a zero value by greater than the size of the type without an error?

I thought it may be safe as zero shifted by any amount should still be zero. But if we would like to maintain this as an error I can remove the lhs_is_zero simplification.

@asterite
Copy link
Collaborator

We should maintain this as an error rather than optimizing it out.

I thought the same thing. But I checked with Rust and in debug mode it overflows and in release mode it doesn't so I wasn't sure, but shifting zero is unlikely to happen to maybe it's fine to error in that case.

@aakoshh
Copy link
Contributor

aakoshh commented Aug 21, 2025

but shifting zero is unlikely to happen

Fuzzer says: "Hold my🍺"

@TomAFrench
Copy link
Member

TomAFrench commented Aug 21, 2025

I thought it may be safe as zero shifted by any amount should still be zero. But if we would like to maintain this as an error I can remove the lhs_is_zero simplification.

I don't see it as a safety thing but I think that if we say that performing a bitshift larger than the type's size results in an error then we shouldn't allow this on certain values. 1u32 >> 32 and 0u32 >> 32 are both "guaranteed to be zero" but now we error on one and not the other.

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.

4 participants