Skip to content

Commit

Permalink
fix(ssa): Handle right shift with constants (#2481)
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm authored Aug 29, 2023
1 parent b55f399 commit 13a8c87
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,14 @@ fn main(x: u64) {
// shifts on runtime values
assert(x << 1 == 128);
assert(x >> 2 == 16);

regression_2250();
}

fn regression_2250() {
let a: u1 = 1 >> 1;
assert(a == 0);

let b: u32 = 1 >> 32;
assert(b == 0);
}
14 changes: 12 additions & 2 deletions crates/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,8 @@ impl Binary {

if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
// Thus, we do not evaluate this divison as we want to avoid triggering a panic and a division by zero
// should fail to satisfy constraints laid down during ACIR generation.
// Thus, we do not evaluate this divison as we want to avoid triggering a panic,
// and division by zero should be handled by laying down constraints during ACIR generation.
if matches!(self.operator, BinaryOp::Div) && rhs == FieldElement::zero() {
return SimplifyResult::None;
}
Expand Down Expand Up @@ -756,6 +756,16 @@ impl Binary {

let lhs = truncate(lhs.try_into_u128()?, *bit_size);
let rhs = truncate(rhs.try_into_u128()?, *bit_size);

// The divisor is being truncated into the type of the operand, which can potentially
// lead to the rhs being zero.
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
// Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
// and the operation should be handled by ACIR generation.
if matches!(self.operator, BinaryOp::Div) && rhs == 0 {
return None;
}

let result = function(lhs, rhs);
truncate(result, *bit_size).into()
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<'a> FunctionContext<'a> {
self.builder.insert_binary(lhs, BinaryOp::Mul, pow)
}

/// Insert ssa instructions which computes lhs << rhs by doing lhs/2^rhs
/// Insert ssa instructions which computes lhs >> rhs by doing lhs/2^rhs
fn insert_shift_right(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId {
let base = self.builder.field_constant(FieldElement::from(2_u128));
let pow = self.pow(base, rhs);
Expand Down

0 comments on commit 13a8c87

Please sign in to comment.