Skip to content

Commit 1f16498

Browse files
aakoshhTomAFrench
andauthored
fix(ssa): Consider shl and shr to have side effects (#9580)
Co-authored-by: Tom French <[email protected]>
1 parent 854bea9 commit 1f16498

14 files changed

+739
-5
lines changed

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,16 +460,21 @@ impl Instruction {
460460
BinaryOp::Div | BinaryOp::Mod => {
461461
dfg.get_numeric_constant(binary.rhs).is_none_or(|c| c.is_zero())
462462
}
463+
BinaryOp::Shl | BinaryOp::Shr => {
464+
// Bit-shifts which are known to be by a number of bits less than the bit size of the type have no side effects.
465+
dfg.get_numeric_constant(binary.rhs).is_none_or(|c| {
466+
let typ = dfg.type_of_value(binary.lhs);
467+
c >= typ.bit_size().into()
468+
})
469+
}
463470
BinaryOp::Add { unchecked: true }
464471
| BinaryOp::Sub { unchecked: true }
465472
| BinaryOp::Mul { unchecked: true }
466473
| BinaryOp::Eq
467474
| BinaryOp::Lt
468475
| BinaryOp::And
469476
| BinaryOp::Or
470-
| BinaryOp::Xor
471-
| BinaryOp::Shl
472-
| BinaryOp::Shr => false,
477+
| BinaryOp::Xor => false,
473478
},
474479

475480
// These don't have side effects
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[package]
2+
name = "regression_9544"
3+
type = "bin"
4+
authors = [""]
5+
6+
[dependencies]
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
c = 8483479640938442498
2+
return = 5
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
fn main(c: u64) -> pub u64 {
2+
if c == 1 {
3+
if c == 2 {
4+
c >> c
5+
} else {
6+
3
7+
}
8+
} else {
9+
if c == 4 {
10+
c >> c
11+
} else {
12+
5
13+
}
14+
}
15+
}

tooling/ast_fuzzer/src/compare/interpreted.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ impl Comparable for ssa::interpreter::errors::InterpreterError {
223223
(DivisionByZero { .. }, ConstrainEqFailed { msg, .. }) => {
224224
msg.as_ref().is_some_and(|msg| msg == "attempt to divide by zero")
225225
}
226+
(PoppedFromEmptySlice { .. }, ConstrainEqFailed { msg, .. }) => {
227+
// The removal of unreachable instructions can replace popping from an empty slice with an always-fail constraint.
228+
msg.as_ref().is_some_and(|msg| msg == "Index out of bounds")
229+
}
226230
(e1, e2) => {
227231
// The format strings contain SSA instructions,
228232
// where the only difference might be the value ID.

tooling/ast_fuzzer/src/program/func.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,8 +1724,7 @@ impl<'a> FunctionContext<'a> {
17241724

17251725
/// Generate a `match` expression, returning a given type.
17261726
///
1727-
/// Match needs a variable; if we don't have one to produce the target type from,
1728-
/// it returns `None`.
1727+
/// Match needs a variable; if we don't have one to produce the target type from, it returns `None`.
17291728
fn gen_match(
17301729
&mut self,
17311730
u: &mut Unstructured,
@@ -1827,6 +1826,7 @@ impl<'a> FunctionContext<'a> {
18271826
let item_id = self.next_local_id();
18281827
let item_name = format!("item_{}", local_name(item_id));
18291828
self.locals.add(item_id, false, item_name.clone(), item_type.clone());
1829+
self.set_dynamic(item_id, src_dyn);
18301830
arguments.push((item_id, item_name));
18311831
}
18321832
// Generate the original expression we wanted with the new arguments in scope.

tooling/nargo_cli/tests/snapshots/execution_success/regression_9544/execute__tests__expanded.snap

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tooling/nargo_cli/tests/snapshots/execution_success/regression_9544/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap

Lines changed: 168 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)