feat: unchecked math operations in SSA (#7011)

Co-authored-by: Tom French <[email protected]> Co-authored-by: Tom French <[email protected]>
noir-lang · Jan 10, 2025 · f6ed6aa · f6ed6aa
1 parent c41a0ed
commit f6ed6aa
Show file tree

Hide file tree

Showing 26 changed files with 391 additions and 201 deletions.
diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs
@@ -1949,9 +1949,9 @@ impl<'a> Context<'a> {
         let bit_count = binary_type.bit_size::<FieldElement>();
         let num_type = binary_type.to_numeric_type();
         let result = match binary.operator {
-            BinaryOp::Add => self.acir_context.add_var(lhs, rhs),
-            BinaryOp::Sub => self.acir_context.sub_var(lhs, rhs),
-            BinaryOp::Mul => self.acir_context.mul_var(lhs, rhs),
+            BinaryOp::Add { .. } => self.acir_context.add_var(lhs, rhs),
+            BinaryOp::Sub { .. } => self.acir_context.sub_var(lhs, rhs),
+            BinaryOp::Mul { .. } => self.acir_context.mul_var(lhs, rhs),
             BinaryOp::Div => self.acir_context.div_var(
                 lhs,
                 rhs,
@@ -2070,7 +2070,7 @@ impl<'a> Context<'a> {
             Value::Instruction { instruction, .. } => {
                 if matches!(
                     &dfg[*instruction],
-                    Instruction::Binary(Binary { operator: BinaryOp::Sub, .. })
+                    Instruction::Binary(Binary { operator: BinaryOp::Sub { .. }, .. })
                 ) {
                     // Subtractions must first have the integer modulus added before truncation can be
                     // applied. This is done in order to prevent underflow.
@@ -3159,7 +3159,11 @@ mod test {
         let func_with_nested_call_v1 = builder.add_parameter(Type::field());
 
         let two = builder.field_constant(2u128);
-        let v0_plus_two = builder.insert_binary(func_with_nested_call_v0, BinaryOp::Add, two);
+        let v0_plus_two = builder.insert_binary(
+            func_with_nested_call_v0,
+            BinaryOp::Add { unchecked: false },
+            two,
+        );
 
         let foo_id = Id::test_new(2);
         let foo_call = builder.import_function(foo_id);

diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
@@ -1318,9 +1318,9 @@ impl<'block> BrilligBlock<'block> {
                     BrilligBinaryOp::Modulo
                 }
             }
-            BinaryOp::Add => BrilligBinaryOp::Add,
-            BinaryOp::Sub => BrilligBinaryOp::Sub,
-            BinaryOp::Mul => BrilligBinaryOp::Mul,
+            BinaryOp::Add { .. } => BrilligBinaryOp::Add,
+            BinaryOp::Sub { .. } => BrilligBinaryOp::Sub,
+            BinaryOp::Mul { .. } => BrilligBinaryOp::Mul,
             BinaryOp::Eq => BrilligBinaryOp::Equals,
             BinaryOp::Lt => {
                 if is_signed {

diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs
@@ -382,14 +382,14 @@ mod test {
         builder.switch_to_block(b2);
 
         let twenty_seven = builder.field_constant(27u128);
-        let v7 = builder.insert_binary(v0, BinaryOp::Add, twenty_seven);
+        let v7 = builder.insert_binary(v0, BinaryOp::Add { unchecked: false }, twenty_seven);
         builder.insert_store(v3, v7);
 
         builder.terminate_with_jmp(b3, vec![]);
 
         builder.switch_to_block(b1);
 
-        let v6 = builder.insert_binary(v1, BinaryOp::Add, twenty_seven);
+        let v6 = builder.insert_binary(v1, BinaryOp::Add { unchecked: false }, twenty_seven);
         builder.insert_store(v3, v6);
 
         builder.terminate_with_jmp(b3, vec![]);
@@ -501,7 +501,7 @@ mod test {
 
         builder.switch_to_block(b2);
 
-        let v6 = builder.insert_binary(v4, BinaryOp::Mul, v4);
+        let v6 = builder.insert_binary(v4, BinaryOp::Mul { unchecked: false }, v4);
 
         builder.terminate_with_jmp(b4, vec![v0]);
 
@@ -526,7 +526,7 @@ mod test {
 
         let v12 = builder.insert_load(v3, Type::field());
 
-        let v13 = builder.insert_binary(v12, BinaryOp::Add, v6);
+        let v13 = builder.insert_binary(v12, BinaryOp::Add { unchecked: false }, v6);
 
         builder.insert_store(v3, v13);
 
@@ -535,13 +535,13 @@ mod test {
         builder.switch_to_block(b8);
 
         let one = builder.field_constant(1u128);
-        let v15 = builder.insert_binary(v7, BinaryOp::Add, one);
+        let v15 = builder.insert_binary(v7, BinaryOp::Add { unchecked: false }, one);
 
         builder.terminate_with_jmp(b4, vec![v15]);
 
         builder.switch_to_block(b6);
 
-        let v16 = builder.insert_binary(v4, BinaryOp::Add, one);
+        let v16 = builder.insert_binary(v4, BinaryOp::Add { unchecked: false }, one);
 
         builder.terminate_with_jmp(b1, vec![v16]);
 

diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
@@ -251,14 +251,6 @@ impl FunctionBuilder {
         operator: BinaryOp,
         rhs: ValueId,
     ) -> ValueId {
-        let lhs_type = self.type_of_value(lhs);
-        let rhs_type = self.type_of_value(rhs);
-        if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
-            assert_eq!(
-                lhs_type, rhs_type,
-                "ICE - Binary instruction operands must have the same type"
-            );
-        }
         let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
         self.insert_instruction(instruction, None).first()
     }

diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
@@ -230,6 +230,7 @@ impl DataFlowGraph {
         if !self.is_handled_by_runtime(&instruction) {
             return InsertInstructionResult::InstructionRemoved;
         }
+
         match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {
             SimplifyResult::SimplifiedTo(simplification) => {
                 InsertInstructionResult::SimplifiedTo(simplification)

diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
@@ -412,10 +412,15 @@ impl Instruction {
 
             // Some binary math can overflow or underflow
             Binary(binary) => match binary.operator {
-                BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Div | BinaryOp::Mod => {
-                    true
-                }
-                BinaryOp::Eq
+                BinaryOp::Add { unchecked: false }
+                | BinaryOp::Sub { unchecked: false }
+                | BinaryOp::Mul { unchecked: false }
+                | BinaryOp::Div
+                | BinaryOp::Mod => true,
+                BinaryOp::Add { unchecked: true }
+                | BinaryOp::Sub { unchecked: true }
+                | BinaryOp::Mul { unchecked: true }
+                | BinaryOp::Eq
                 | BinaryOp::Lt
                 | BinaryOp::And
                 | BinaryOp::Or
@@ -566,16 +571,19 @@ impl Instruction {
         match self {
             Instruction::Binary(binary) => {
                 match binary.operator {
-                    BinaryOp::Add
-                    | BinaryOp::Sub
-                    | BinaryOp::Mul
+                    BinaryOp::Add { unchecked: false }
+                    | BinaryOp::Sub { unchecked: false }
+                    | BinaryOp::Mul { unchecked: false }
                     | 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::Add { unchecked: true }
+                    | BinaryOp::Sub { unchecked: true }
+                    | BinaryOp::Mul { unchecked: true }
+                    | BinaryOp::Eq
                     | BinaryOp::Lt
                     | BinaryOp::And
                     | BinaryOp::Or