Skip to content

Commit

Permalink
Merge branch 'master' into gd/issue_7103
Browse files Browse the repository at this point in the history
  • Loading branch information
guipublic authored Feb 5, 2025
2 parents 6678cbc + e476f95 commit 5a0b097
Show file tree
Hide file tree
Showing 10 changed files with 479 additions and 376 deletions.
13 changes: 8 additions & 5 deletions compiler/noirc_evaluator/src/acir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,12 +549,12 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
&mut self,
lhs: AcirVar,
rhs: AcirVar,
predicate: AcirVar,
assert_message: Option<AssertionPayload<F>>,
) -> Result<(), RuntimeError> {
let diff_var = self.sub_var(lhs, rhs)?;

let one = self.add_constant(F::one());
let _ = self.inv_var(diff_var, one)?;
let _ = self.inv_var(diff_var, predicate)?;
if let Some(payload) = assert_message {
self.acir_ir
.assertion_payloads
Expand Down Expand Up @@ -613,7 +613,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
}
NumericType::Signed { bit_size } => {
let (quotient_var, _remainder_var) =
self.signed_division_var(lhs, rhs, bit_size)?;
self.signed_division_var(lhs, rhs, bit_size, predicate)?;
Ok(quotient_var)
}
}
Expand Down Expand Up @@ -1050,6 +1050,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
lhs: AcirVar,
rhs: AcirVar,
bit_size: u32,
predicate: AcirVar,
) -> Result<(AcirVar, AcirVar), RuntimeError> {
// We derive the signed division from the unsigned euclidean division.
// note that this is not euclidean division!
Expand Down Expand Up @@ -1079,7 +1080,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {

// Performs the division using the unsigned values of lhs and rhs
let (q1, r1) =
self.euclidean_division_var(unsigned_lhs, unsigned_rhs, bit_size - 1, one)?;
self.euclidean_division_var(unsigned_lhs, unsigned_rhs, bit_size - 1, predicate)?;

// Unsigned to signed: derive q and r from q1,r1 and the signs of lhs and rhs
// Quotient sign is lhs sign * rhs sign, whose resulting sign bit is the XOR of the sign bits
Expand Down Expand Up @@ -1117,7 +1118,9 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
};

let (_, remainder_var) = match numeric_type {
NumericType::Signed { bit_size } => self.signed_division_var(lhs, rhs, bit_size)?,
NumericType::Signed { bit_size } => {
self.signed_division_var(lhs, rhs, bit_size, predicate)?
}
_ => self.euclidean_division_var(lhs, rhs, bit_size, predicate)?,
};
Ok(remainder_var)
Expand Down
7 changes: 6 additions & 1 deletion compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,12 @@ impl<'a> Context<'a> {
None
};

self.acir_context.assert_neq_var(lhs, rhs, assert_payload)?;
self.acir_context.assert_neq_var(
lhs,
rhs,
self.current_side_effects_enabled_var,
assert_payload,
)?;
}
Instruction::Cast(value_id, _) => {
let acir_var = self.convert_numeric_value(*value_id, dfg)?;
Expand Down
10 changes: 7 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,13 +595,17 @@ impl Instruction {
match binary.operator {
BinaryOp::Add { unchecked: false }
| BinaryOp::Sub { unchecked: false }
| BinaryOp::Mul { unchecked: false }
| BinaryOp::Div
| BinaryOp::Mod => {
| BinaryOp::Mul { unchecked: false } => {
// Some binary math can overflow or underflow, but this is only the case
// for unsigned types (here we assume the type of binary.lhs is the same)
dfg.type_of_value(binary.rhs).is_unsigned()
}
BinaryOp::Div | BinaryOp::Mod => {
// Div and Mod require a predicate if the RHS may be zero.
dfg.get_numeric_constant(binary.rhs)
.map(|rhs| !rhs.is_zero())
.unwrap_or(true)
}
BinaryOp::Add { unchecked: true }
| BinaryOp::Sub { unchecked: true }
| BinaryOp::Mul { unchecked: true }
Expand Down
61 changes: 61 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1724,4 +1724,65 @@ mod test {
let ssa = ssa.purity_analysis().fold_constants_using_constraints();
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn does_not_deduplicate_field_divisions_under_different_predicates() {
// Regression test for https://github.com/noir-lang/noir/issues/7283
let src = "
acir(inline) fn main f0 {
b0(v0: Field, v1: Field, v2: u1):
enable_side_effects v2
v3 = div v1, v0
v4 = mul v3, v0
v5 = not v2
enable_side_effects v5
v6 = div v1, v0
return
}
";

let ssa = Ssa::from_str(src).unwrap();
let ssa = ssa.fold_constants();
assert_normalized_ssa_equals(ssa, src);
}

#[test]
fn does_not_deduplicate_unsigned_divisions_under_different_predicates() {
// Regression test for https://github.com/noir-lang/noir/issues/7283
let src = "
acir(inline) fn main f0 {
b0(v0: u32, v1: u32, v2: u1):
enable_side_effects v2
v3 = div v1, v0
v4 = not v2
enable_side_effects v4
v5 = div v1, v0
return
}
";

let ssa = Ssa::from_str(src).unwrap();
let ssa = ssa.fold_constants();
assert_normalized_ssa_equals(ssa, src);
}

#[test]
fn does_not_deduplicate_signed_divisions_under_different_predicates() {
// Regression test for https://github.com/noir-lang/noir/issues/7283
let src = "
acir(inline) fn main f0 {
b0(v0: i32, v1: i32, v2: u1):
enable_side_effects v2
v3 = div v1, v0
v4 = not v2
enable_side_effects v4
v5 = div v1, v0
return
}
";

let ssa = Ssa::from_str(src).unwrap();
let ssa = ssa.fold_constants();
assert_normalized_ssa_equals(ssa, src);
}
}
Loading

0 comments on commit 5a0b097

Please sign in to comment.