diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index d08d5339237..3d7e0b06d5b 100644 --- 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 index 8425e4d5e56..ae8854a95f0 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -7,7 +7,8 @@ use super::{ call_stack::{CallStack, CallStackHelper, CallStackId}, function::{FunctionId, RuntimeType}, instruction::{ - Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, + Binary, BinaryOp, Instruction, InstructionId, InstructionResultType, Intrinsic, + TerminatorInstruction, }, map::DenseMap, types::{NumericType, Type}, @@ -174,6 +175,44 @@ impl DataFlowGraph { instruction_data: Instruction, ctrl_typevars: Option>, ) -> InstructionId { + let instruction_data = if let Instruction::Binary(binary) = instruction_data { + let lhs = binary.lhs; + let rhs = binary.rhs; + let operator = binary.operator; + 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 operator = if lhs_type.is_field() { + // Unchecked operations between fields or bools don't make sense, so we convert those to non-unchecked + // to reduce noise and confusion in the generated SSA. + match operator { + BinaryOp::Add { unchecked: true } => BinaryOp::Add { unchecked: false }, + BinaryOp::Sub { unchecked: true } => BinaryOp::Sub { unchecked: false }, + BinaryOp::Mul { unchecked: true } => BinaryOp::Mul { unchecked: false }, + _ => operator, + } + } else if lhs_type.is_bool() { + // Unchecked mul between bools doesn't make sense, so we convert that to non-unchecked + if let BinaryOp::Mul { unchecked: true } = operator { + BinaryOp::Mul { unchecked: false } + } else { + operator + } + } else { + operator + }; + + Instruction::Binary(Binary { operator, ..binary }) + } else { + instruction_data + }; + let id = self.instructions.insert(instruction_data); self.make_instruction_results(id, ctrl_typevars); id diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 0dd7fd92ee5..2247671a79c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -119,6 +119,16 @@ impl Type { matches!(self, Type::Numeric(NumericType::Unsigned { .. })) } + /// Returns whether the `Type` represents the Field type. + pub(crate) fn is_field(&self) -> bool { + matches!(self, Type::Numeric(NumericType::NativeField)) + } + + /// Returns whether the `Type` is `u1` + pub(crate) fn is_bool(&self) -> bool { + matches!(self, Type::Numeric(NumericType::Unsigned { bit_size: 1 })) + } + /// Create a new signed integer type with the given amount of bits. pub(crate) fn signed(bit_size: u32) -> Type { Type::Numeric(NumericType::Signed { bit_size }) diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 795e06bceb9..76f8495c009 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -850,9 +850,9 @@ mod test { enable_side_effects u1 1 v3 = cast v0 as Field v4 = cast v1 as Field - v6 = unchecked_mul v3, Field 3 - v8 = unchecked_mul v4, Field 4 - v9 = unchecked_add v6, v8 + v6 = mul v3, Field 3 + v8 = mul v4, Field 4 + v9 = add v6, v8 return v9 } "; @@ -881,7 +881,7 @@ mod test { acir(inline) fn main f0 { b0(v0: u1, v1: u1): enable_side_effects v0 - v2 = unchecked_mul v1, v0 + v2 = mul v1, v0 constrain v2 == v0 v3 = not v0 enable_side_effects u1 1 @@ -916,9 +916,9 @@ mod test { v3 = not v0 v4 = cast v0 as Field v5 = cast v3 as Field - v7 = unchecked_mul v4, Field 5 - v8 = unchecked_mul v5, v2 - v9 = unchecked_add v7, v8 + v7 = mul v4, Field 5 + v8 = mul v5, v2 + v9 = add v7, v8 store v9 at v1 enable_side_effects u1 1 return @@ -954,17 +954,17 @@ mod test { v3 = not v0 v4 = cast v0 as Field v5 = cast v3 as Field - v7 = unchecked_mul v4, Field 5 - v8 = unchecked_mul v5, v2 - v9 = unchecked_add v7, v8 + v7 = mul v4, Field 5 + v8 = mul v5, v2 + v9 = add v7, v8 store v9 at v1 enable_side_effects v3 v10 = load v1 -> Field v11 = cast v3 as Field v12 = cast v0 as Field - v14 = unchecked_mul v11, Field 6 - v15 = unchecked_mul v12, v10 - v16 = unchecked_add v14, v15 + v14 = mul v11, Field 6 + v15 = mul v12, v10 + v16 = add v14, v15 store v16 at v1 enable_side_effects u1 1 return @@ -1066,32 +1066,32 @@ mod test { v3 = not v0 v4 = cast v0 as Field v5 = cast v3 as Field - v7 = unchecked_mul v4, Field 2 - v8 = unchecked_add v7, v5 - v9 = unchecked_mul v0, v1 + v7 = mul v4, Field 2 + v8 = add v7, v5 + v9 = mul v0, v1 enable_side_effects v9 v10 = not v9 v11 = cast v9 as Field v12 = cast v10 as Field - v14 = unchecked_mul v11, Field 5 - v15 = unchecked_mul v12, v8 - v16 = unchecked_add v14, v15 + v14 = mul v11, Field 5 + v15 = mul v12, v8 + v16 = add v14, v15 v17 = not v1 - v18 = unchecked_mul v0, v17 + v18 = mul v0, v17 enable_side_effects v18 v19 = not v18 v20 = cast v18 as Field v21 = cast v19 as Field - v23 = unchecked_mul v20, Field 6 - v24 = unchecked_mul v21, v16 - v25 = unchecked_add v23, v24 + v23 = mul v20, Field 6 + v24 = mul v21, v16 + v25 = add v23, v24 enable_side_effects v0 enable_side_effects v3 v26 = cast v3 as Field v27 = cast v0 as Field - v29 = unchecked_mul v26, Field 3 - v30 = unchecked_mul v27, v25 - v31 = unchecked_add v29, v30 + v29 = mul v26, Field 3 + v30 = mul v27, v25 + v31 = add v29, v30 enable_side_effects u1 1 return v31 }"; @@ -1446,9 +1446,9 @@ mod test { v2 = not v0 v3 = cast v0 as Field v4 = cast v2 as Field - v6 = unchecked_mul v3, Field 2 - v7 = unchecked_mul v4, v3 - v8 = unchecked_add v6, v7 + v6 = mul v3, Field 2 + v7 = mul v4, v3 + v8 = add v6, v7 enable_side_effects u1 1 return v8 } @@ -1491,7 +1491,7 @@ mod test { b0(v0: u1, v1: u1): enable_side_effects v0 v2 = cast v0 as Field - v4 = unchecked_mul v2, Field 2 + v4 = mul v2, Field 2 v5 = make_array [v4] : [Field; 1] enable_side_effects u1 1 return v5 diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index ee96d5d82c8..8c24b2ec458 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -346,7 +346,7 @@ fn test_binary() { let src = format!( " acir(inline) fn main f0 {{ - b0(v0: Field, v1: Field): + b0(v0: u32, v1: u32): v2 = {op} v0, v1 return }}