Skip to content

fix(ssa): Consider shl and shr to have side effects #9580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 21, 2025

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Aug 20, 2025

Description

Problem*

Resolves #9544

Summary*

A duplicate shl or shr was hoisted into the common dominator block by constant_folding, causing the circuit to fail. This can be prevented if has_side_effects returns true for them, which seems to be the case.

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.

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: 6e50331 Previous: 854bea9 Ratio
test_report_zkpassport_noir-ecdsa_ 3 s 2 s 1.50

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

CC: @TomAFrench

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2025
@aakoshh aakoshh added this pull request to the merge queue Aug 21, 2025
@aakoshh aakoshh removed this pull request from the merge queue due to a manual request Aug 21, 2025
@aakoshh aakoshh enabled auto-merge August 21, 2025 10:19
@aakoshh aakoshh disabled auto-merge August 21, 2025 10:20
@aakoshh aakoshh added this pull request to the merge queue Aug 21, 2025
Merged via the queue into master with commit 1f16498 Aug 21, 2025
122 checks passed
@aakoshh aakoshh deleted the af/9544-fix-inevitable-shift branch August 21, 2025 11:30
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
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.

Min != Full (Brillig): Attempted to shift by 962916098 bits on a type of bit size 8
2 participants