fix(interpreter): Return -1 for negative shr signed overflow or 0 for positive shr signed overflow by vezenovm · Pull Request #8828 · noir-lang/noir

vezenovm · 2025-06-06T19:57:12Z

Description

Problem*

Follow-up to #8805. We updated the comptime interpreter

Summary*

Get the SSA interpreter and the frontend interpreter inline with our ACIR/Brillig runtimes.

SSA Interpreter:
Fallback to -1 when we have an overflowing shift right with a negative lhs and fallback to 0 when we have an overflowing shift right with a positive lhs.

Comptime:

Same as above but the fallbacks look a little different due to the usage of macros for shifting.

I would like to switch to https://doc.rust-lang.org/std/primitive.i32.html#method.unbounded_shr. As per the description this is the method we are looking for instead of wrapping_shr:

If rhs is larger or equal to the number of bits in self, the entire value is shifted out, which yields 0 for a positive number, and -1 for a negative number.

However, it is currently unstable for our Rust version. And would require a #![feature(unbounded_shifts)] feature gate. However, this feature has been stabilized in more recent versions (1.87.0).

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.

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

github-actions

⚠️ 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: `9e15762`	Previous: `c4a8746`	Ratio
`test_report_zkpassport_noir_rsa_`	`1` s	`0` s	`+∞`

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

CC: @TomAFrench

…flow-neg-one' into mv/ssa-interpret-shr-signed-overflow-neg-one

github-actions

⚠️ 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: `4d9cf9d`	Previous: `c4a8746`	Ratio
`rollup-merge`	`0.004` s	`0.003` s	`1.33`

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

CC: @TomAFrench

vezenovm · 2025-06-06T21:23:30Z

I have some changes that fix the comptime vs brillig shr flake https://github.com/noir-lang/noir/actions/runs/15499196774/job/43643781238

...pshots/compile_success_no_bug/comptime_right_shift_no_overflow/execute__tests__expanded.snap

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore(docs): Update noirjs app page to use to beta.6 (noir-lang/noir#8853) fix: support recursive call to main function in SSA parser (noir-lang/noir#8760) chore(SSA): validate that constrain values have the same type (noir-lang/noir#8850) fix: Comptime field division should error when the rhs is zero (noir-lang/noir#8845) chore: redo typo PR by osrm (noir-lang/noir#8840) fix: (SSA interpreter) to_le_bits returns [u1; _], not [u8; _] (noir-lang/noir#8837) chore(ssa): Initial validation module (noir-lang/noir#8765) chore: bump external pinned commits (noir-lang/noir#8834) feat(fuzz): Generate arbitrary constraints (noir-lang/noir#8820) fix: bind self generic type in trait calls via a concrete type in more cases (noir-lang/noir#8827) fix(comptime): Overflow on shl (noir-lang/noir#8829) fix(interpreter): Return -1 for negative shr signed overflow or 0 for positive shr signed overflow (noir-lang/noir#8828) feat(ssa_fuzzer): branching + constrains (noir-lang/noir#8599) chore(docs): Add experimental warning in Debugger docs (noir-lang/noir#8824) fix: Thread errors through remove_if_else instead of panicing when the value merger finds reference values (noir-lang/noir#8783) fix(interpreter): Do not overflow on signed checked ops (noir-lang/noir#8806) feat: short circuit creation of `Type::InfixExpr` containing errors (noir-lang/noir#8826) fix(mem2reg): Keep last stores used in array returned from a function (noir-lang/noir#8801) chore(ci): `cargo clippy` CI script to save time (noir-lang/noir#8787) chore: only follow bindings on interface to `arithmetic` module (noir-lang/noir#8822) fix: bind self generic type in trait calls via a concrete type (noir-lang/noir#8825) chore(docs): Reorder tooling docs (noir-lang/noir#8742) chore: small fix for outdated docs (noir-lang/noir#8821) fix(mem2reg): Keep last stores used in MakeArray (noir-lang/noir#8743) fix!: Error when re-assigning a mutable reference (noir-lang/noir#8790) fix!: indexing arrays with non-u32 is now an error (noir-lang/noir#8804) fix: signed right shift overflows to 0 or -1 (noir-lang/noir#8805) chore(fuzz): Tool to minimize Noir programs with `cvise` (noir-lang/noir#8789) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

have the shr overflow for signed integer fallback to negative one

afca483

vezenovm requested a review from a team June 6, 2025 19:57

clippy

576f9f2

vezenovm commented Jun 6, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs Outdated Show resolved Hide resolved

Update compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

ae4637f

jfecher reviewed Jun 6, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs Show resolved Hide resolved

github-actions bot reviewed Jun 6, 2025

View reviewed changes

vezenovm added 4 commits June 6, 2025 20:35

some new tests

768c4c4

Merge remote-tracking branch 'origin/mv/ssa-interpret-shr-signed-over…

c75cad6

…flow-neg-one' into mv/ssa-interpret-shr-signed-overflow-neg-one

handle pos vs neg lhs for shr signed overflow

4d9cf9d

Merge branch 'master' into mv/ssa-interpret-shr-signed-overflow-neg-one

9e15762

vezenovm requested a review from jfecher June 6, 2025 20:41

github-actions bot reviewed Jun 6, 2025

View reviewed changes

vezenovm changed the title ~~fix(interpreter): Return -1 for shr signed overflow~~ fix(interpreter): Return -1 for negative shr signed overflow or 0 for positive shr signed overflow Jun 6, 2025

jfecher approved these changes Jun 6, 2025

View reviewed changes

jfecher enabled auto-merge June 6, 2025 21:16

jfecher added this pull request to the merge queue Jun 6, 2025

vezenovm removed this pull request from the merge queue due to a manual request Jun 6, 2025

vezenovm added 2 commits June 6, 2025 21:24

fix how we handle >> in comptime interpreter

0b06eef

expand tests

1346439

vezenovm enabled auto-merge June 6, 2025 21:27

vezenovm added 2 commits June 6, 2025 21:39

bump aztec-nr timeout

e94826f

snap

9077306

vezenovm commented Jun 7, 2025

View reviewed changes

...pshots/compile_success_no_bug/comptime_right_shift_no_overflow/execute__tests__expanded.snap Show resolved Hide resolved

vezenovm added this pull request to the merge queue Jun 7, 2025

Merged via the queue into master with commit 522d4c1 Jun 7, 2025
117 checks passed

vezenovm deleted the mv/ssa-interpret-shr-signed-overflow-neg-one branch June 7, 2025 21:00

noirwhal mentioned this pull request Jun 7, 2025

chore: Release Noir(1.0.0-beta.7) #8520

Merged

AztecBot mentioned this pull request Jun 8, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#14878

Merged

AztecBot mentioned this pull request Jun 10, 2025

chore(next): release 0.88.0 AztecProtocol/aztec-packages#14495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(interpreter): Return -1 for negative shr signed overflow or 0 for positive shr signed overflow#8828

fix(interpreter): Return -1 for negative shr signed overflow or 0 for positive shr signed overflow#8828
vezenovm merged 11 commits intomasterfrom
mv/ssa-interpret-shr-signed-overflow-neg-one

vezenovm commented Jun 6, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

Uh oh!

vezenovm commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vezenovm commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem*

Summary*

Additional Context

Documentation*

PR Checklist*

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Uh oh!

vezenovm commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vezenovm commented Jun 6, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading