merge conflicts w/ master

.github/workflows/reports.yml

@@ -303,11 +303,14 @@ jobs:
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, take_average: true }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, take_average: true  }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, take_average: true }
-          #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, take_average: true }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private, take_average: true }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public, take_average: true }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, take_average: true }
+          #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-empty }
+          #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-single-tx }
+          #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-merge, take_average: true }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, take_average: true }
 
     name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }}
     steps:

compiler/noirc_evaluator/src/acir/generated_acir.rs

@@ -479,7 +479,7 @@ impl<F: AcirField> GeneratedAcir<F> {
     pub(crate) fn is_equal(&mut self, lhs: &Expression<F>, rhs: &Expression<F>) -> Witness {
         let t = lhs - rhs;
 
-        self.is_zero(&t)
+        self.is_zero(t)
     }
 
     /// Returns a `Witness` that is constrained to be:
@@ -534,36 +534,32 @@ impl<F: AcirField> GeneratedAcir<F> {
     /// By setting `z` to be `0`, we can make `y` equal to `1`.
     /// This is easily observed: `y = 1 - t * 0`
     /// Now since `y` is one, this means that `t` needs to be zero, or else `y * t == 0` will fail.
-    fn is_zero(&mut self, t_expr: &Expression<F>) -> Witness {
-        // We're checking for equality with zero so we can negate the expression without changing the result.
-        // This is useful as it will sometimes allow us to simplify an expression down to a witness.
-        let t_witness = if let Some(witness) = t_expr.to_witness() {
-            witness
+    fn is_zero(&mut self, t_expr: Expression<F>) -> Witness {
+        // We're going to be multiplying this expression by two different witnesses in a second so we want to
+        // ensure that this expression only contains a single witness. We can tolerate coefficients and constant terms however.
+        let linear_t = if t_expr.is_degree_one_univariate() {
+            t_expr
         } else {
-            let negated_expr = t_expr * -F::one();
-            self.get_or_create_witness(&negated_expr)
+            Expression::<F>::from(self.get_or_create_witness(&t_expr))
         };
 
         // Call the inversion directive, since we do not apply a constraint
         // the prover can choose anything here.
-        let z = self.brillig_inverse(t_witness.into());
+        let z = self.brillig_inverse(linear_t.clone());
+        let z_expr = Expression::<F>::from(z);
 
         let y = self.next_witness_index();
+        let y_expr = Expression::<F>::from(y);
 
         // Add constraint y == 1 - tz => y + tz - 1 == 0
-        let y_is_boolean_constraint = Expression {
-            mul_terms: vec![(F::one(), t_witness, z)],
-            linear_combinations: vec![(F::one(), y)],
-            q_c: -F::one(),
-        };
+        let mut y_is_boolean_constraint =
+            (&z_expr * &linear_t).expect("multiplying two linear expressions");
+        y_is_boolean_constraint.push_addition_term(F::one(), y);
+        let y_is_boolean_constraint = y_is_boolean_constraint - F::one();
         self.assert_is_zero(y_is_boolean_constraint);
 
         // Add constraint that y * t == 0;
-        let ty_zero_constraint = Expression {
-            mul_terms: vec![(F::one(), t_witness, y)],
-            linear_combinations: vec![],
-            q_c: F::zero(),
-        };
+        let ty_zero_constraint = (&y_expr * &linear_t).expect("multiplying two linear expressions");
         self.assert_is_zero(ty_zero_constraint);
 
         y

compiler/noirc_evaluator/src/acir/mod.rs

@@ -1952,9 +1952,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,
@@ -2073,7 +2073,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.
@@ -3162,7 +3162,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);

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -836,7 +836,7 @@ impl<'block, 'global, Registers: RegisterAllocator> BrilligBlock<'block, 'global
                 self.brillig_context.deallocate_register(rc_register);
             }
             Instruction::EnableSideEffectsIf { .. } => {
-                todo!("enable_side_effects not supported by brillig")
+                unreachable!("enable_side_effects not supported by brillig")
             }
             Instruction::IfElse { .. } => {
                 unreachable!("IfElse instructions should not be possible in brillig")
@@ -1368,9 +1368,9 @@ impl<'block, 'global, Registers: RegisterAllocator> BrilligBlock<'block, 'global
                     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 {

compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs

@@ -384,14 +384,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![]);
@@ -503,7 +503,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]);
 
@@ -528,7 +528,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);
 
@@ -537,13 +537,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]);
 

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()
     }

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

@@ -181,7 +181,16 @@ impl DataFlowGraph {
 
     /// Check if the function runtime would simply ignore this instruction.
     pub(crate) fn is_handled_by_runtime(&self, instruction: &Instruction) -> bool {
-        !(self.runtime().is_acir() && instruction.is_brillig_only())
+        match self.runtime() {
+            RuntimeType::Acir(_) => !matches!(
+                instruction,
+                Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }
+            ),
+            RuntimeType::Brillig(_) => !matches!(
+                instruction,
+                Instruction::EnableSideEffectsIf { .. } | Instruction::IfElse { .. }
+            ),
+        }
     }
 
     fn insert_instruction_without_simplification(
@@ -205,7 +214,7 @@ impl DataFlowGraph {
         call_stack: CallStackId,
     ) -> InsertInstructionResult {
         if !self.is_handled_by_runtime(&instruction_data) {
-            return InsertInstructionResult::InstructionRemoved;
+            panic!("Attempted to insert instruction not handled by runtime: {instruction_data:?}");
         }
 
         let id = self.insert_instruction_without_simplification(
@@ -228,8 +237,9 @@ impl DataFlowGraph {
         call_stack: CallStackId,
     ) -> InsertInstructionResult {
         if !self.is_handled_by_runtime(&instruction) {
-            return InsertInstructionResult::InstructionRemoved;
+            panic!("Attempted to insert instruction not handled by runtime: {instruction:?}");
         }
+
         match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {
             SimplifyResult::SimplifiedTo(simplification) => {
                 InsertInstructionResult::SimplifiedTo(simplification)

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
@@ -1058,12 +1066,6 @@ impl Instruction {
             Instruction::Noop => Remove,
         }
     }
-
-    /// Some instructions are only to be used in Brillig and should be eliminated
-    /// after runtime separation, never to be be reintroduced in an ACIR runtime.
-    pub(crate) fn is_brillig_only(&self) -> bool {
-        matches!(self, Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. })
-    }
 }
 
 /// Given a chain of operations like: