Skip to content

Commit

Permalink
fix: don't deduplicate binary math of unsigned types (#6848)
Browse files Browse the repository at this point in the history
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
  • Loading branch information
asterite and vezenovm authored Dec 19, 2024
1 parent a97402a commit ee0754b
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 51 deletions.
24 changes: 19 additions & 5 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,25 @@ impl Instruction {
/// If true the instruction will depend on `enable_side_effects` context during acir-gen.
pub(crate) fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
match self {
Instruction::Binary(binary)
if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) =>
{
true
Instruction::Binary(binary) => {
match binary.operator {
BinaryOp::Add
| BinaryOp::Sub
| BinaryOp::Mul
| BinaryOp::Div
| BinaryOp::Mod => {
// 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::Eq
| BinaryOp::Lt
| BinaryOp::And
| BinaryOp::Or
| BinaryOp::Xor
| BinaryOp::Shl
| BinaryOp::Shr => false,
}
}

Instruction::ArrayGet { array, index } => {
Expand All @@ -569,7 +584,6 @@ impl Instruction {
_ => false,
},
Instruction::Cast(_, _)
| Instruction::Binary(_)
| Instruction::Not(_)
| Instruction::Truncate { .. }
| Instruction::Constrain(_, _, _)
Expand Down
92 changes: 46 additions & 46 deletions compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,17 +263,17 @@ mod test {
fn simple_loop_invariant_code_motion() {
let src = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
jmp b1(u32 0)
b1(v2: u32):
v5 = lt v2, u32 4
b0(v0: i32, v1: i32):
jmp b1(i32 0)
b1(v2: i32):
v5 = lt v2, i32 4
jmpif v5 then: b3, else: b2
b2():
return
b3():
v6 = mul v0, v1
constrain v6 == u32 6
v8 = add v2, u32 1
constrain v6 == i32 6
v8 = add v2, i32 1
jmp b1(v8)
}
";
Expand All @@ -287,17 +287,17 @@ mod test {
// `v6 = mul v0, v1` in b3 should now be `v3 = mul v0, v1` in b0
let expected = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
b0(v0: i32, v1: i32):
v3 = mul v0, v1
jmp b1(u32 0)
b1(v2: u32):
v6 = lt v2, u32 4
jmp b1(i32 0)
b1(v2: i32):
v6 = lt v2, i32 4
jmpif v6 then: b3, else: b2
b2():
return
b3():
constrain v3 == u32 6
v9 = add v2, u32 1
constrain v3 == i32 6
v9 = add v2, i32 1
jmp b1(v9)
}
";
Expand All @@ -312,25 +312,25 @@ mod test {
// is hoisted to the parent loop's pre-header block.
let src = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
jmp b1(u32 0)
b1(v2: u32):
v6 = lt v2, u32 4
b0(v0: i32, v1: i32):
jmp b1(i32 0)
b1(v2: i32):
v6 = lt v2, i32 4
jmpif v6 then: b3, else: b2
b2():
return
b3():
jmp b4(u32 0)
b4(v3: u32):
v7 = lt v3, u32 4
jmp b4(i32 0)
b4(v3: i32):
v7 = lt v3, i32 4
jmpif v7 then: b6, else: b5
b5():
v9 = add v2, u32 1
v9 = add v2, i32 1
jmp b1(v9)
b6():
v10 = mul v0, v1
constrain v10 == u32 6
v12 = add v3, u32 1
constrain v10 == i32 6
v12 = add v3, i32 1
jmp b4(v12)
}
";
Expand All @@ -344,25 +344,25 @@ mod test {
// `v10 = mul v0, v1` in b6 should now be `v4 = mul v0, v1` in b0
let expected = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
b0(v0: i32, v1: i32):
v4 = mul v0, v1
jmp b1(u32 0)
b1(v2: u32):
v7 = lt v2, u32 4
jmp b1(i32 0)
b1(v2: i32):
v7 = lt v2, i32 4
jmpif v7 then: b3, else: b2
b2():
return
b3():
jmp b4(u32 0)
b4(v3: u32):
v8 = lt v3, u32 4
jmp b4(i32 0)
b4(v3: i32):
v8 = lt v3, i32 4
jmpif v8 then: b6, else: b5
b5():
v10 = add v2, u32 1
v10 = add v2, i32 1
jmp b1(v10)
b6():
constrain v4 == u32 6
v12 = add v3, u32 1
constrain v4 == i32 6
v12 = add v3, i32 1
jmp b4(v12)
}
";
Expand All @@ -386,19 +386,19 @@ mod test {
// hoist `v7 = mul v6, v0`.
let src = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
jmp b1(u32 0)
b1(v2: u32):
v5 = lt v2, u32 4
b0(v0: i32, v1: i32):
jmp b1(i32 0)
b1(v2: i32):
v5 = lt v2, i32 4
jmpif v5 then: b3, else: b2
b2():
return
b3():
v6 = mul v0, v1
v7 = mul v6, v0
v8 = eq v7, u32 12
constrain v7 == u32 12
v9 = add v2, u32 1
v8 = eq v7, i32 12
constrain v7 == i32 12
v9 = add v2, i32 1
jmp b1(v9)
}
";
Expand All @@ -411,19 +411,19 @@ mod test {

let expected = "
brillig(inline) fn main f0 {
b0(v0: u32, v1: u32):
b0(v0: i32, v1: i32):
v3 = mul v0, v1
v4 = mul v3, v0
v6 = eq v4, u32 12
jmp b1(u32 0)
b1(v2: u32):
v9 = lt v2, u32 4
v6 = eq v4, i32 12
jmp b1(i32 0)
b1(v2: i32):
v9 = lt v2, i32 4
jmpif v9 then: b3, else: b2
b2():
return
b3():
constrain v4 == u32 12
v11 = add v2, u32 1
constrain v4 == i32 12
v11 = add v2, i32 1
jmp b1(v11)
}
";
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_6834/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_6834"
version = "0.1.0"
type = "bin"
authors = [""]

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_success/regression_6834/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
year = 1
min_age = 1
9 changes: 9 additions & 0 deletions test_programs/execution_success/regression_6834/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main(year: u32, min_age: u8) -> pub u32 {
if min_age == 0 {
(year - 2) / 4
} else if year > 1 {
(year - 2) / 4
} else {
0
}
}

0 comments on commit ee0754b

Please sign in to comment.