chore(ssa refactor): Update how instruction result types are retrieved

crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs

@@ -2,7 +2,7 @@ use super::{
     basic_block::{BasicBlock, BasicBlockId},
     constant::{NumericConstant, NumericConstantId},
     function::Signature,
-    instruction::{Instruction, InstructionId},
+    instruction::{Instruction, InstructionId, InstructionResultType},
     map::{DenseMap, Id, SecondaryMap, TwoWayMap},
     types::Type,
     value::{Value, ValueId},
@@ -137,15 +137,15 @@ impl DataFlowGraph {
     pub(crate) fn make_instruction_results(
         &mut self,
         instruction_id: InstructionId,
-        ctrl_typevar: Type,
+        ctrl_typevars: Option<Vec<Type>>,
     ) -> &[ValueId] {
         // Clear all of the results instructions associated with this
         // instruction.
         self.results.get_mut(&instruction_id).expect("all instructions should have a `result` allocation when instruction was added to the DFG").clear();
 
         // Get all of the types that this instruction produces
         // and append them as results.
-        let typs = self.instruction_result_types(instruction_id, ctrl_typevar);
+        let typs = self.instruction_result_types(instruction_id, ctrl_typevars);
 
         for typ in typs {
             self.append_result(instruction_id, typ);
@@ -158,22 +158,33 @@ impl DataFlowGraph {
 
     /// Return the result types of this instruction.
     ///
-    /// For example, an addition instruction will return
-    /// one type which is the type of the operands involved.
-    /// This is the `ctrl_typevar` in this case.
+    /// In the case of Load, Call, and Intrinsic, the function's result
+    /// type may be unknown. In this case, the given ctrl_typevars are returned instead.
+    /// ctrl_typevars is taken in as an Option since it is common to omit them when getting
+    /// the type of an instruction that does not require them. Compared to passing an empty Vec,
+    /// Option has the benefit of panicking if it is accidentally used for a Call instruction,
+    /// rather than silently returning the empty Vec and continuing.
     fn instruction_result_types(
         &self,
         instruction_id: InstructionId,
-        ctrl_typevar: Type,
+        ctrl_typevars: Option<Vec<Type>>,
     ) -> Vec<Type> {
-        // Check if it is a call instruction. If so, we don't support that yet
-        let ins_data = &self.instructions[instruction_id];
-        match ins_data {
-            Instruction::Call { .. } => todo!("function calls are not supported yet"),
-            ins => ins.return_types(ctrl_typevar),
+        let instruction = &self.instructions[instruction_id];
+        match instruction.result_type() {
+            InstructionResultType::Known(typ) => vec![typ],
+            InstructionResultType::Operand(value) => vec![self.type_of_value(value)],
+            InstructionResultType::None => vec![],
+            InstructionResultType::Unknown => {
+                ctrl_typevars.expect("Control typevars required but not given")
+            }
         }
     }
 
+    /// Returns the type of a given value
+    pub(crate) fn type_of_value(&self, value: ValueId) -> Type {
+        self.values[value].get_type()
+    }
+
     /// Appends a result type to the instruction.
     pub(crate) fn append_result(&mut self, instruction_id: InstructionId, typ: Type) -> ValueId {
         let results = self.results.get_mut(&instruction_id).unwrap();
@@ -257,19 +268,15 @@ impl std::ops::IndexMut<BasicBlockId> for DataFlowGraph {
 #[cfg(test)]
 mod tests {
     use super::DataFlowGraph;
-    use crate::ssa_refactor::ir::{
-        instruction::Instruction,
-        types::{NumericType, Type},
-    };
+    use crate::ssa_refactor::ir::instruction::Instruction;
 
     #[test]
     fn make_instruction() {
         let mut dfg = DataFlowGraph::default();
         let ins = Instruction::Allocate { size: 20 };
         let ins_id = dfg.make_instruction(ins);
 
-        let num_results =
-            dfg.make_instruction_results(ins_id, Type::Numeric(NumericType::NativeField)).len();
+        let num_results = dfg.make_instruction_results(ins_id, None).len();
 
         let results = dfg.instruction_results(ins_id);
         assert_eq!(results.len(), num_results);

crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

@@ -105,23 +105,39 @@ impl Instruction {
         }
     }
 
-    /// Returns the types that this instruction will return.
-    pub(crate) fn return_types(&self, ctrl_typevar: Type) -> Vec<Type> {
+    /// Returns the type that this instruction will return.
+    pub(crate) fn result_type(&self) -> InstructionResultType {
         match self {
-            Instruction::Binary(_) => vec![ctrl_typevar],
-            Instruction::Cast(_, typ) => vec![*typ],
-            Instruction::Not(_) => vec![ctrl_typevar],
-            Instruction::Truncate { .. } => vec![ctrl_typevar],
-            Instruction::Constrain(_) => vec![],
-            Instruction::Call { .. } => vec![],
-            Instruction::Intrinsic { .. } => vec![],
-            Instruction::Allocate { .. } => vec![Type::Reference],
-            Instruction::Load { .. } => vec![ctrl_typevar],
-            Instruction::Store { .. } => vec![],
+            Instruction::Binary(binary) => binary.result_type(),
+            Instruction::Cast(_, typ) => InstructionResultType::Known(*typ),
+            Instruction::Allocate { .. } => InstructionResultType::Known(Type::Reference),
+            Instruction::Not(value) | Instruction::Truncate { value, .. } => {
+                InstructionResultType::Operand(*value)
+            }
+            Instruction::Constrain(_) | Instruction::Store { .. } => InstructionResultType::None,
+            Instruction::Load { .. } | Instruction::Call { .. } | Instruction::Intrinsic { .. } => {
+                InstructionResultType::Unknown
+            }
         }
     }
 }
 
+/// The possible return values for Instruction::return_types
+pub(crate) enum InstructionResultType {
+    /// The result type of this instruction matches that of this operand
+    Operand(ValueId),
+
+    /// The result type of this instruction is known to be this type - independent of its operands.
+    Known(Type),
+
+    /// The result type of this function is unknown and separate from its operand types.
+    /// This occurs for function and intrinsic calls.
+    Unknown,
+
+    /// This instruction does not return any results.
+    None,
+}
+
 /// These are operations which can exit a basic block
 /// ie control flow type operations
 ///
@@ -169,33 +185,53 @@ pub(crate) struct Binary {
     pub(crate) operator: BinaryOp,
 }
 
+impl Binary {
+    pub(crate) fn result_type(&self) -> InstructionResultType {
+        match self.operator {
+            BinaryOp::Eq | BinaryOp::Lt => InstructionResultType::Known(Type::bool()),
+            _ => InstructionResultType::Operand(self.lhs),
+        }
+    }
+}
+
 /// Binary Operations allowed in the IR.
+/// Aside from the comparison operators (Eq and Lt), all operators
+/// will return the same type as their operands.
+/// The operand types must match for all binary operators.
+/// All binary operators are also only for numeric types. To implement
+/// e.g. equality for a compound type like a struct, one must add a
+/// separate Eq operation for each field and combine them later with And.
 #[derive(Debug, PartialEq, Eq, Hash, Clone)]
 pub(crate) enum BinaryOp {
-    /// Addition of two types.
-    /// The result will have the same type as
-    /// the operands.
+    /// Addition of lhs + rhs.
     Add,
-    /// Subtraction of two types.
-    /// The result will have the same type as
-    /// the operands.
+    /// Subtraction of lhs - rhs.
     Sub,
-    /// Multiplication of two types.
-    /// The result will have the same type as
-    /// the operands.
+    /// Multiplication of lhs * rhs.
     Mul,
-    /// Division of two types.
-    /// The result will have the same type as
-    /// the operands.
+    /// Division of lhs / rhs.
     Div,
+    /// Modulus of lhs % rhs.
+    Mod,
     /// Checks whether two types are equal.
     /// Returns true if the types were equal and
     /// false otherwise.
     Eq,
-    /// Checks whether two types are equal.
-    /// Returns true if the types were not equal and
-    /// false otherwise.
-    Neq,
+    /// Checks whether the lhs is less than the rhs.
+    /// All other comparison operators should be translated
+    /// to less than. For example (a > b) = (b < a) = !(a >= b) = !(b <= a).
+    /// The result will always be a u1.
+    Lt,
+    /// Bitwise and (&)
+    And,
+    /// Bitwise or (|)
+    Or,
+    /// Bitwise xor (^)
+    Xor,
+    /// Shift lhs left by rhs bits (<<)
+    Shl,
+    /// Shift lhs right by rhs bits (>>)
+    Shr,
 }
 
 impl std::fmt::Display for BinaryOp {
@@ -206,7 +242,13 @@ impl std::fmt::Display for BinaryOp {
             BinaryOp::Mul => write!(f, "mul"),
             BinaryOp::Div => write!(f, "div"),
             BinaryOp::Eq => write!(f, "eq"),
-            BinaryOp::Neq => write!(f, "neq"),
+            BinaryOp::Mod => write!(f, "mod"),
+            BinaryOp::Lt => write!(f, "lt"),
+            BinaryOp::And => write!(f, "and"),
+            BinaryOp::Or => write!(f, "or"),
+            BinaryOp::Xor => write!(f, "xor"),
+            BinaryOp::Shl => write!(f, "shl"),
+            BinaryOp::Shr => write!(f, "shr"),
         }
     }
 }

crates/noirc_evaluator/src/ssa_refactor/ir/types.rs

@@ -38,6 +38,10 @@ impl Type {
         Type::Numeric(NumericType::Unsigned { bit_size })
     }
 
+    pub(crate) fn bool() -> Type {
+        Type::unsigned(1)
+    }
+
     pub(crate) fn field() -> Type {
         Type::Numeric(NumericType::NativeField)
     }

crates/noirc_evaluator/src/ssa_refactor/ir/value.rs

@@ -28,3 +28,13 @@ pub(crate) enum Value {
     /// This Value originates from a numeric constant
     NumericConstant { constant: NumericConstantId, typ: Type },
 }
+
+impl Value {
+    pub(crate) fn get_type(&self) -> Type {
+        match self {
+            Value::Instruction { typ, .. } => *typ,
+            Value::Param { typ, .. } => *typ,
+            Value::NumericConstant { typ, .. } => *typ,
+        }
+    }
+}

crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs

@@ -82,7 +82,7 @@ impl<'ssa> FunctionBuilder<'ssa> {
     /// which is always a Reference to the allocated data.
     pub(crate) fn insert_allocate(&mut self, size_to_allocate: u32) -> ValueId {
         let id = self.insert_instruction(Instruction::Allocate { size: size_to_allocate });
-        self.current_function.dfg.make_instruction_results(id, Type::Reference)[0]
+        self.current_function.dfg.make_instruction_results(id, None)[0]
     }
 
     /// Insert a Load instruction at the end of the current block, loading from the given address
@@ -91,7 +91,7 @@ impl<'ssa> FunctionBuilder<'ssa> {
     /// Returns the element that was loaded.
     pub(crate) fn insert_load(&mut self, address: ValueId, type_to_load: Type) -> ValueId {
         let id = self.insert_instruction(Instruction::Load { address });
-        self.current_function.dfg.make_instruction_results(id, type_to_load)[0]
+        self.current_function.dfg.make_instruction_results(id, Some(vec![type_to_load]))[0]
     }
 
     /// Insert a Store instruction at the end of the current block, storing the given element
@@ -100,12 +100,22 @@ impl<'ssa> FunctionBuilder<'ssa> {
         self.insert_instruction(Instruction::Store { address, value });
     }
 
-    /// Insert a Store instruction at the end of the current block, storing the given element
-    /// at the given address. Expects that the address points to a previous Allocate instruction.
-    /// Returns the result of the add instruction.
-    pub(crate) fn insert_add(&mut self, lhs: ValueId, rhs: ValueId, typ: Type) -> ValueId {
-        let operator = BinaryOp::Add;
+    /// Insert a binary instruction at the end of the current block.
+    /// Returns the result of the binary instruction.
+    pub(crate) fn insert_binary(
+        &mut self,
+        lhs: ValueId,
+        operator: BinaryOp,
+        rhs: ValueId,
+    ) -> ValueId {
         let id = self.insert_instruction(Instruction::Binary(Binary { lhs, rhs, operator }));
-        self.current_function.dfg.make_instruction_results(id, typ)[0]
+        self.current_function.dfg.make_instruction_results(id, None)[0]
+    }
+
+    /// Insert a not instruction at the end of the current block.
+    /// Returns the result of the instruction.
+    pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId {
+        let id = self.insert_instruction(Instruction::Not(rhs));
+        self.current_function.dfg.make_instruction_results(id, None)[0]
     }
 }

crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs

@@ -6,7 +6,9 @@ use noirc_frontend::monomorphization::ast::{self, LocalId, Parameters};
 use noirc_frontend::monomorphization::ast::{FuncId, Program};
 use noirc_frontend::Signedness;
 
+use crate::ssa_refactor::ir::instruction::BinaryOp;
 use crate::ssa_refactor::ir::types::Type;
+use crate::ssa_refactor::ir::value::ValueId;
 use crate::ssa_refactor::ssa_builder::SharedBuilderContext;
 use crate::ssa_refactor::{
     ir::function::FunctionId as IrFunctionId, ssa_builder::function_builder::FunctionBuilder,
@@ -123,6 +125,77 @@ impl<'a> FunctionContext<'a> {
             ast::Type::Vec(_) => Type::Reference,
         }
     }
+
+    /// Insert a unit constant into the current function if not already
+    /// present, and return its value
+    pub(super) fn unit_value(&mut self) -> Values {
+        self.builder.numeric_constant(0u128.into(), Type::Unit).into()
+    }
+
+    /// Insert a binary instruction at the end of the current block.
+    /// Converts the form of the binary instruction as necessary
+    /// (e.g. swapping arguments, inserting a not) to represent it in the IR.
+    /// For example, (a <= b) is represented as !(b < a)
+    pub(super) fn insert_binary(
+        &mut self,
+        mut lhs: ValueId,
+        operator: noirc_frontend::BinaryOpKind,
+        mut rhs: ValueId,
+    ) -> Values {
+        let op = convert_operator(operator);
+
+        if operator_requires_swapped_operands(operator) {
+            std::mem::swap(&mut lhs, &mut rhs);
+        }
+
+        let mut result = self.builder.insert_binary(lhs, op, rhs);
+
+        if operator_requires_not(operator) {
+            result = self.builder.insert_not(result);
+        }
+        result.into()
+    }
+}
+
+/// True if the given operator cannot be encoded directly and needs
+/// to be represented as !(some other operator)
+fn operator_requires_not(op: noirc_frontend::BinaryOpKind) -> bool {
+    use noirc_frontend::BinaryOpKind::*;
+    matches!(op, NotEqual | LessEqual | GreaterEqual)
+}
+
+/// True if the given operator cannot be encoded directly and needs
+/// to have its lhs and rhs swapped to be represented with another operator.
+/// Example: (a > b) needs to be represented as (b < a)
+fn operator_requires_swapped_operands(op: noirc_frontend::BinaryOpKind) -> bool {
+    use noirc_frontend::BinaryOpKind::*;
+    matches!(op, Greater | LessEqual)
+}
+
+/// Converts the given operator to the appropriate BinaryOp.
+/// Take care when using this to insert a binary instruction: this requires
+/// checking operator_requires_not and operator_requires_swapped_operands
+/// to represent the full operation correctly.
+fn convert_operator(op: noirc_frontend::BinaryOpKind) -> BinaryOp {
+    use noirc_frontend::BinaryOpKind;
+    match op {
+        BinaryOpKind::Add => BinaryOp::Add,
+        BinaryOpKind::Subtract => BinaryOp::Sub,
+        BinaryOpKind::Multiply => BinaryOp::Mul,
+        BinaryOpKind::Divide => BinaryOp::Div,
+        BinaryOpKind::Modulo => BinaryOp::Mod,
+        BinaryOpKind::Equal => BinaryOp::Eq,
+        BinaryOpKind::NotEqual => BinaryOp::Eq, // Requires not
+        BinaryOpKind::Less => BinaryOp::Lt,
+        BinaryOpKind::Greater => BinaryOp::Lt, // Requires operand swap
+        BinaryOpKind::LessEqual => BinaryOp::Lt, // Requires operand swap and not
+        BinaryOpKind::GreaterEqual => BinaryOp::Lt, // Requires not
+        BinaryOpKind::And => BinaryOp::And,
+        BinaryOpKind::Or => BinaryOp::Or,
+        BinaryOpKind::Xor => BinaryOp::Xor,
+        BinaryOpKind::ShiftRight => BinaryOp::Shr,
+        BinaryOpKind::ShiftLeft => BinaryOp::Shl,
+    }
 }
 
 impl SharedContext {