fix(SSA): disallow using lt with fields (#8585)

Author: asterite

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

@@ -341,7 +341,7 @@ mod test {
     use crate::ssa::ir::function::RuntimeType;
     use crate::ssa::ir::instruction::BinaryOp;
     use crate::ssa::ir::map::Id;
-    use crate::ssa::ir::types::Type;
+    use crate::ssa::ir::types::{NumericType, Type};
 
     #[test]
     fn simple_back_propagation() {
@@ -441,11 +441,11 @@ mod test {
     #[test]
     fn propagation_with_nested_loops() {
         // brillig fn main f0 {
-        //     b0(v0: Field, v1: Field):
+        //     b0(v0: u32, v1: u32):
         //       v3 = allocate
-        //       store Field 0 at v3
-        //       jmp b1(Field 0)
-        //     b1(v4: Field):
+        //       store u32 0 at v3
+        //       jmp b1(u32 0)
+        //     b1(v4: u32):
         //       v5 = lt v4, v0
         //       jmpif v5 then: b2, else: b3
         //     b3():
@@ -454,14 +454,14 @@ mod test {
         //     b2():
         //       v6 = mul v4, v4
         //       jmp b4(v0)
-        //     b4(v7: Field):
+        //     b4(v7: u32):
         //       v8 = lt v7, v1
         //       jmpif v8 then: b5, else: b6
         //     b6():
-        //       v16 = add v4, Field 1
+        //       v16 = add v4, u32 1
         //       jmp b1(v16)
         //     b5():
-        //       v10 = eq v7, Field 27
+        //       v10 = eq v7, u32 27
         //       v11 = not v10
         //       jmpif v11 then: b7, else: b8
         //     b7():
@@ -470,7 +470,7 @@ mod test {
         //       store v13 at v3
         //       jmp b8()
         //     b8():
-        //       v15 = add v7, Field 1
+        //       v15 = add v7, u32 1
         //       jmp b4(v15)
         //   }
 
@@ -487,18 +487,18 @@ mod test {
         let b7 = builder.insert_block();
         let b8 = builder.insert_block();
 
-        let v0 = builder.add_parameter(Type::field());
-        let v1 = builder.add_parameter(Type::field());
+        let v0 = builder.add_parameter(Type::unsigned(32));
+        let v1 = builder.add_parameter(Type::unsigned(32));
 
-        let v3 = builder.insert_allocate(Type::field());
+        let v3 = builder.insert_allocate(Type::unsigned(32));
 
-        let zero = builder.field_constant(0u128);
+        let zero = builder.numeric_constant(0u128, NumericType::Unsigned { bit_size: 32 });
         builder.insert_store(v3, zero);
 
         builder.terminate_with_jmp(b1, vec![zero]);
 
         builder.switch_to_block(b1);
-        let v4 = builder.add_block_parameter(b1, Type::field());
+        let v4 = builder.add_block_parameter(b1, Type::unsigned(32));
 
         let v5 = builder.insert_binary(v4, BinaryOp::Lt, v0);
 
@@ -512,15 +512,15 @@ mod test {
 
         builder.switch_to_block(b4);
 
-        let v7 = builder.add_block_parameter(b4, Type::field());
+        let v7 = builder.add_block_parameter(b4, Type::unsigned(32));
 
         let v8 = builder.insert_binary(v7, BinaryOp::Lt, v1);
 
         builder.terminate_with_jmpif(v8, b5, b6);
 
         builder.switch_to_block(b5);
 
-        let twenty_seven = builder.field_constant(27u128);
+        let twenty_seven = builder.numeric_constant(27u128, NumericType::Unsigned { bit_size: 32 });
         let v10 = builder.insert_binary(v7, BinaryOp::Eq, twenty_seven);
 
         let v11 = builder.insert_not(v10);
@@ -529,7 +529,7 @@ mod test {
 
         builder.switch_to_block(b7);
 
-        let v12 = builder.insert_load(v3, Type::field());
+        let v12 = builder.insert_load(v3, Type::unsigned(32));
 
         let v13 = builder.insert_binary(v12, BinaryOp::Add { unchecked: false }, v6);
 
@@ -539,7 +539,7 @@ mod test {
 
         builder.switch_to_block(b8);
 
-        let one = builder.field_constant(1u128);
+        let one = builder.numeric_constant(1u128, NumericType::Unsigned { bit_size: 32 });
         let v15 = builder.insert_binary(v7, BinaryOp::Add { unchecked: false }, one);
 
         builder.terminate_with_jmp(b4, vec![v15]);
@@ -552,7 +552,7 @@ mod test {
 
         builder.switch_to_block(b3);
 
-        let v17 = builder.insert_load(v3, Type::field());
+        let v17 = builder.insert_load(v3, Type::unsigned(32));
 
         builder.terminate_with_return(vec![v17]);
 

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

@@ -9,7 +9,8 @@ use super::{
     basic_block::{BasicBlock, BasicBlockId},
     function::{FunctionId, RuntimeType},
     instruction::{
-        Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction,
+        Binary, BinaryOp, Instruction, InstructionId, InstructionResultType, Intrinsic,
+        TerminatorInstruction,
     },
     integer::IntegerConstant,
     map::DenseMap,
@@ -223,11 +224,23 @@ impl DataFlowGraph {
         instruction_data: Instruction,
         ctrl_typevars: Option<Vec<Type>>,
     ) -> InstructionId {
+        self.validate_instruction(&instruction_data);
+
         let id = self.instructions.insert(instruction_data);
         self.make_instruction_results(id, ctrl_typevars);
         id
     }
 
+    fn validate_instruction(&self, instruction: &Instruction) {
+        if let Instruction::Binary(Binary { lhs, rhs: _, operator: BinaryOp::Lt }) = instruction {
+            // Assume rhs_type is the same as lhs_type
+            let lhs_type = self.type_of_value(*lhs);
+            if matches!(lhs_type, Type::Numeric(NumericType::NativeField)) {
+                panic!("Cannot use `lt` with field elements");
+            }
+        }
+    }
+
     /// Check if the function runtime would simply ignore this instruction.
     pub(crate) fn is_handled_by_runtime(&self, instruction: &Instruction) -> bool {
         match self.runtime() {
@@ -901,7 +914,10 @@ impl std::ops::Index<usize> for InsertInstructionResult<'_> {
 #[cfg(test)]
 mod tests {
     use super::DataFlowGraph;
-    use crate::ssa::ir::{instruction::Instruction, types::Type};
+    use crate::ssa::{
+        ir::{instruction::Instruction, types::Type},
+        ssa_gen::Ssa,
+    };
 
     #[test]
     fn make_instruction() {
@@ -912,4 +928,17 @@ mod tests {
         let results = dfg.instruction_results(ins_id);
         assert_eq!(results.len(), 1);
     }
+
+    #[test]
+    #[should_panic(expected = "Cannot use `lt` with field elements")]
+    fn disallows_comparing_fields_with_lt() {
+        let src = "
+        acir(inline) impure fn main f0 {
+          b0():
+            v2 = lt Field 1, Field 2
+            return
+        }
+        ";
+        let _ = Ssa::from_str(src);
+    }
 }

compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

@@ -236,7 +236,8 @@ impl BinaryOp {
             BinaryOp::Mul { .. } => Some(std::ops::Mul::mul),
             BinaryOp::Div => Some(std::ops::Div::div),
             BinaryOp::Eq => Some(|x, y| (x == y).into()),
-            BinaryOp::Lt => Some(|x, y| (x < y).into()),
+            // "less then" comparison is not supported for Fields
+            BinaryOp::Lt => None,
             // Bitwise operators are unsupported for Fields
             BinaryOp::Mod => None,
             BinaryOp::And => None,

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -1511,41 +1511,41 @@ mod test {
         let src = "
         acir(inline) fn main f0 {
           b0():
-            v0 = allocate -> &mut Field
-            store Field 0 at v0
-            v2 = allocate -> &mut Field
-            store Field 2 at v2
-            v4 = load v2 -> Field
-            v5 = lt v4, Field 2
+            v0 = allocate -> &mut u32
+            store u32 0 at v0
+            v2 = allocate -> &mut u32
+            store u32 2 at v2
+            v4 = load v2 -> u32
+            v5 = lt v4, u32 2
             jmpif v5 then: b4, else: b1
           b1():
-            v6 = load v2 -> Field
-            v8 = lt v6, Field 4
+            v6 = load v2 -> u32
+            v8 = lt v6, u32 4
             jmpif v8 then: b2, else: b3
           b2():
-            v9 = load v0 -> Field
-            v10 = load v2 -> Field
-            v12 = mul v10, Field 100
+            v9 = load v0 -> u32
+            v10 = load v2 -> u32
+            v12 = mul v10, u32 100
             v13 = add v9, v12
             store v13 at v0
-            v14 = load v2 -> Field
-            v16 = add v14, Field 1
+            v14 = load v2 -> u32
+            v16 = add v14, u32 1
             store v16 at v2
             jmp b3()
           b3():
             jmp b5()
           b4():
-            v17 = load v0 -> Field
-            v18 = load v2 -> Field
-            v20 = mul v18, Field 10
+            v17 = load v0 -> u32
+            v18 = load v2 -> u32
+            v20 = mul v18, u32 10
             v21 = add v17, v20
             store v21 at v0
-            v22 = load v2 -> Field
-            v23 = add v22, Field 1
+            v22 = load v2 -> u32
+            v23 = add v22, u32 1
             store v23 at v2
             jmp b5()
           b5():
-            v24 = load v0 -> Field
+            v24 = load v0 -> u32
             return v24
         }";
 
@@ -1572,10 +1572,10 @@ mod test {
         assert_ssa_snapshot!(ssa, @r"
         acir(inline) fn main f0 {
           b0():
-            v0 = allocate -> &mut Field
-            v1 = allocate -> &mut Field
+            v0 = allocate -> &mut u32
+            v1 = allocate -> &mut u32
             enable_side_effects u1 1
-            return Field 200
+            return u32 200
         }
         ");
     }

compiler/noirc_evaluator/src/ssa/opt/inlining.rs

@@ -807,19 +807,19 @@ mod test {
         let src = "
         acir(inline) fn main f0 {
           b0():
-            v2 = call f1(Field 5) -> Field
+            v2 = call f1(u32 5) -> u32
             return v2
         }
 
         acir(inline) fn factorial f1 {
-          b0(v1: Field):
-            v2 = lt v1, Field 1
+          b0(v1: u32):
+            v2 = lt v1, u32 1
             jmpif v2 then: b1, else: b2
           b1():
-            return Field 1
+            return u32 1
           b2():
-            v4 = sub v1, Field 1
-            v5 = call f1(v4) -> Field
+            v4 = sub v1, u32 1
+            v5 = call f1(v4) -> u32
             v6 = mul v1, v5
             return v6
         }
@@ -842,7 +842,7 @@ mod test {
           b5():
             jmp b6()
           b6():
-            return Field 120
+            return u32 120
         }
         ");
     }