From cc6add979fc1f2816d1294754a9b966ab30905dc Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 25 Apr 2023 11:33:24 -0500 Subject: [PATCH 1/4] Implement binary instructions --- .../src/ssa_refactor/ir/instruction.rs | 74 ++++++++++++++----- .../ssa_builder/function_builder.rs | 21 ++++-- .../src/ssa_refactor/ssa_gen/context.rs | 74 +++++++++++++++++++ .../src/ssa_refactor/ssa_gen/mod.rs | 50 ++++++++++--- .../src/ssa_refactor/ssa_gen/value.rs | 1 + 5 files changed, 186 insertions(+), 34 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 442f1dbd47e..4d0305a03d5 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -170,32 +170,66 @@ pub(crate) struct Binary { } /// 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, + /// Bitiwse or (|) + Or, + /// Bitwise xor (^) + Xor, + /// Shift lhs left by rhs bits (<<) + Shl, + /// Shift lhs right by rhs bits (>>) + Shr, +} + +impl From for BinaryOp { + fn from(value: noirc_frontend::BinaryOpKind) -> Self { + match value { + noirc_frontend::BinaryOpKind::Add => todo!(), + noirc_frontend::BinaryOpKind::Subtract => todo!(), + noirc_frontend::BinaryOpKind::Multiply => todo!(), + noirc_frontend::BinaryOpKind::Divide => todo!(), + noirc_frontend::BinaryOpKind::Equal => todo!(), + noirc_frontend::BinaryOpKind::NotEqual => todo!(), + noirc_frontend::BinaryOpKind::Less => todo!(), + noirc_frontend::BinaryOpKind::LessEqual => todo!(), + noirc_frontend::BinaryOpKind::Greater => todo!(), + noirc_frontend::BinaryOpKind::GreaterEqual => todo!(), + noirc_frontend::BinaryOpKind::And => todo!(), + noirc_frontend::BinaryOpKind::Or => todo!(), + noirc_frontend::BinaryOpKind::Xor => todo!(), + noirc_frontend::BinaryOpKind::ShiftRight => todo!(), + noirc_frontend::BinaryOpKind::ShiftLeft => todo!(), + noirc_frontend::BinaryOpKind::Modulo => todo!(), + } + } } impl std::fmt::Display for BinaryOp { @@ -206,7 +240,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"), } } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs index c76d2943abe..7911aa2988a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs @@ -100,12 +100,23 @@ 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, + typ: Type, + ) -> ValueId { let id = self.insert_instruction(Instruction::Binary(Binary { lhs, rhs, operator })); self.current_function.dfg.make_instruction_results(id, typ)[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, typ: Type) -> ValueId { + let id = self.insert_instruction(Instruction::Not(rhs)); + self.current_function.dfg.make_instruction_results(id, typ)[0] + } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs index 32133feea13..bdb732551c7 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/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,78 @@ impl<'a> FunctionContext<'a> { ast::Type::Vec(_) => Type::Reference, } } + + 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); + } + + // TODO: Rework how types are stored. + // They should be on values rather than on instruction results + let typ = Type::field(); + let mut result = self.builder.insert_binary(lhs, op, rhs, typ); + + if operator_requires_not(operator) { + result = self.builder.insert_not(result, typ); + } + 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) +} + +/// Lossily (!) 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 { diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index 2f9c6646282..3b469ad9664 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -12,7 +12,10 @@ use self::{ value::{Tree, Values}, }; -use super::{ir::types::Type, ssa_builder::SharedBuilderContext}; +use super::{ + ir::{instruction::BinaryOp, types::Type, value::ValueId}, + ssa_builder::SharedBuilderContext, +}; pub(crate) fn generate_ssa(program: Program) { let context = SharedContext::new(program); @@ -58,6 +61,17 @@ impl<'a> FunctionContext<'a> { } } + /// Codegen any non-tuple expression so that we can unwrap the Values + /// tree to return a single value for use with most SSA instructions. + fn codegen_non_tuple_expression(&mut self, expr: &Expression) -> ValueId { + match self.codegen_expression(expr) { + Tree::Branch(branches) => { + panic!("codegen_non_tuple_expression called on tuple {branches:?}") + } + Tree::Leaf(value) => value.eval(), + } + } + fn codegen_ident(&mut self, _ident: &ast::Ident) -> Values { todo!() } @@ -103,7 +117,7 @@ impl<'a> FunctionContext<'a> { array } else { let offset = self.builder.numeric_constant((i as u128).into(), Type::field()); - self.builder.insert_add(array, offset, Type::field()) + self.builder.insert_binary(array, BinaryOp::Add, offset, Type::field()) }; self.builder.insert_store(address, value.eval()); i += 1; @@ -113,16 +127,22 @@ impl<'a> FunctionContext<'a> { array.into() } - fn codegen_block(&mut self, _block: &[Expression]) -> Values { - todo!() + fn codegen_block(&mut self, block: &[Expression]) -> Values { + let mut result = self.unit_value(); + for expr in block { + result = self.codegen_expression(expr); + } + result } fn codegen_unary(&mut self, _unary: &ast::Unary) -> Values { todo!() } - fn codegen_binary(&mut self, _binary: &ast::Binary) -> Values { - todo!() + fn codegen_binary(&mut self, binary: &ast::Binary) -> Values { + let lhs = self.codegen_non_tuple_expression(&binary.lhs); + let rhs = self.codegen_non_tuple_expression(&binary.rhs); + self.insert_binary(lhs, binary.operator, rhs) } fn codegen_index(&mut self, _index: &ast::Index) -> Values { @@ -141,12 +161,17 @@ impl<'a> FunctionContext<'a> { todo!() } - fn codegen_tuple(&mut self, _tuple: &[Expression]) -> Values { - todo!() + fn codegen_tuple(&mut self, tuple: &[Expression]) -> Values { + Tree::Branch(vecmap(tuple, |expr| self.codegen_expression(expr))) } - fn codegen_extract_tuple_field(&mut self, _tuple: &Expression, _index: usize) -> Values { - todo!() + fn codegen_extract_tuple_field(&mut self, tuple: &Expression, index: usize) -> Values { + match self.codegen_expression(tuple) { + Tree::Branch(mut trees) => trees.remove(index), + Tree::Leaf(value) => { + unreachable!("Tried to extract tuple index {index} from non-tuple {value:?}") + } + } } fn codegen_call(&mut self, _call: &ast::Call) -> Values { @@ -165,7 +190,8 @@ impl<'a> FunctionContext<'a> { todo!() } - fn codegen_semi(&mut self, _semi: &Expression) -> Values { - todo!() + fn codegen_semi(&mut self, expr: &Expression) -> Values { + self.codegen_expression(expr); + self.unit_value() } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs index c3911d367c1..83a5d15c904 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs @@ -2,6 +2,7 @@ use crate::ssa_refactor::ir::function::FunctionId as IrFunctionId; use crate::ssa_refactor::ir::types::Type; use crate::ssa_refactor::ir::value::ValueId as IrValueId; +#[derive(Debug)] pub(super) enum Tree { Branch(Vec>), Leaf(T), From 378b35a10e1fe3c6052b9dcfac53b3b0b049f966 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 25 Apr 2023 11:43:57 -0500 Subject: [PATCH 2/4] Cleanup PR --- .../src/ssa_refactor/ir/instruction.rs | 25 +------------------ .../src/ssa_refactor/ssa_gen/context.rs | 4 ++- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 4d0305a03d5..9b5aeb9388c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -199,7 +199,7 @@ pub(crate) enum BinaryOp { Lt, /// Bitwise and (&) And, - /// Bitiwse or (|) + /// Bitwise or (|) Or, /// Bitwise xor (^) Xor, @@ -209,29 +209,6 @@ pub(crate) enum BinaryOp { Shr, } -impl From for BinaryOp { - fn from(value: noirc_frontend::BinaryOpKind) -> Self { - match value { - noirc_frontend::BinaryOpKind::Add => todo!(), - noirc_frontend::BinaryOpKind::Subtract => todo!(), - noirc_frontend::BinaryOpKind::Multiply => todo!(), - noirc_frontend::BinaryOpKind::Divide => todo!(), - noirc_frontend::BinaryOpKind::Equal => todo!(), - noirc_frontend::BinaryOpKind::NotEqual => todo!(), - noirc_frontend::BinaryOpKind::Less => todo!(), - noirc_frontend::BinaryOpKind::LessEqual => todo!(), - noirc_frontend::BinaryOpKind::Greater => todo!(), - noirc_frontend::BinaryOpKind::GreaterEqual => todo!(), - noirc_frontend::BinaryOpKind::And => todo!(), - noirc_frontend::BinaryOpKind::Or => todo!(), - noirc_frontend::BinaryOpKind::Xor => todo!(), - noirc_frontend::BinaryOpKind::ShiftRight => todo!(), - noirc_frontend::BinaryOpKind::ShiftLeft => todo!(), - noirc_frontend::BinaryOpKind::Modulo => todo!(), - } - } -} - impl std::fmt::Display for BinaryOp { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs index bdb732551c7..8f7b4e3de9a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -126,6 +126,8 @@ impl<'a> FunctionContext<'a> { } } + /// 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() } @@ -173,7 +175,7 @@ fn operator_requires_swapped_operands(op: noirc_frontend::BinaryOpKind) -> bool matches!(op, Greater | LessEqual) } -/// Lossily (!) converts the given operator to the appropriate BinaryOp. +/// 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. From 841118e5d51b993caed2512dc5eaa3fe80e3cb4c Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 25 Apr 2023 13:44:19 -0400 Subject: [PATCH 3/4] Change how instruction result types are handled --- .../src/ssa_refactor/ir/dfg.rs | 43 +++++++++------- .../src/ssa_refactor/ir/instruction.rs | 49 ++++++++++++++----- .../src/ssa_refactor/ir/types.rs | 4 ++ .../src/ssa_refactor/ir/value.rs | 10 ++++ .../ssa_builder/function_builder.rs | 11 ++--- .../src/ssa_refactor/ssa_gen/context.rs | 7 +-- .../src/ssa_refactor/ssa_gen/mod.rs | 4 +- 7 files changed, 85 insertions(+), 43 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index c21fc2c3f35..769bde404d8 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/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,7 +137,7 @@ impl DataFlowGraph { pub(crate) fn make_instruction_results( &mut self, instruction_id: InstructionId, - ctrl_typevar: Type, + ctrl_typevars: Option>, ) -> &[ValueId] { // Clear all of the results instructions associated with this // instruction. @@ -145,7 +145,7 @@ impl DataFlowGraph { // 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 { - // 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,10 +268,7 @@ impl std::ops::IndexMut 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() { @@ -268,8 +276,7 @@ mod tests { 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); diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 9b5aeb9388c..dcab6e04006 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/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 { + /// 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,6 +185,15 @@ 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. diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs index 888d7d128d1..8a0f825a117 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs +++ b/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) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs index 537eabb0cab..a559522fadd 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs +++ b/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, + } + } +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs index 7911aa2988a..66db5e3e530 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs +++ b/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 @@ -107,16 +107,15 @@ impl<'ssa> FunctionBuilder<'ssa> { lhs: ValueId, operator: BinaryOp, rhs: ValueId, - typ: Type, ) -> 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, typ: Type) -> ValueId { + 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, typ)[0] + self.current_function.dfg.make_instruction_results(id, None)[0] } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs index 8f7b4e3de9a..f76a6675077 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -148,13 +148,10 @@ impl<'a> FunctionContext<'a> { std::mem::swap(&mut lhs, &mut rhs); } - // TODO: Rework how types are stored. - // They should be on values rather than on instruction results - let typ = Type::field(); - let mut result = self.builder.insert_binary(lhs, op, rhs, typ); + let mut result = self.builder.insert_binary(lhs, op, rhs); if operator_requires_not(operator) { - result = self.builder.insert_not(result, typ); + result = self.builder.insert_not(result); } result.into() } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index 3b469ad9664..553b5eb2218 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -116,8 +116,8 @@ impl<'a> FunctionContext<'a> { let address = if i == 0 { array } else { - let offset = self.builder.numeric_constant((i as u128).into(), Type::field()); - self.builder.insert_binary(array, BinaryOp::Add, offset, Type::field()) + let offset = self.builder.field_constant(i as u128); + self.builder.insert_binary(array, BinaryOp::Add, offset) }; self.builder.insert_store(address, value.eval()); i += 1; From 8cb18bfb37be14002d0429f19cabc9582635476d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 25 Apr 2023 14:29:59 -0400 Subject: [PATCH 4/4] Reorganize make_instruction flow a bit --- .../src/ssa_refactor/ir/dfg.rs | 34 +++++++++---------- .../ssa_builder/function_builder.rs | 27 ++++++++------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 769bde404d8..54ffd5a05f6 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -110,11 +110,19 @@ impl DataFlowGraph { } /// Inserts a new instruction into the DFG. - /// This does not add the instruction to the block or populate the instruction's result list - pub(crate) fn make_instruction(&mut self, instruction_data: Instruction) -> InstructionId { + /// This does not add the instruction to the block. + /// Returns the id of the new instruction and its results. + /// + /// Populates the instruction's results with the given ctrl_typevars if the instruction + /// is a Load, Call, or Intrinsic. Otherwise the instruction's results will be known + /// by the instruction itself and None can safely be passed for this parameter. + pub(crate) fn make_instruction( + &mut self, + instruction_data: Instruction, + ctrl_typevars: Option>, + ) -> InstructionId { let id = self.instructions.insert(instruction_data); - // Create a new vector to store the potential results for the instruction. - self.results.insert(id, Default::default()); + self.make_instruction_results(id, ctrl_typevars); id } @@ -134,14 +142,12 @@ impl DataFlowGraph { /// Attaches results to the instruction, clearing any previous results. /// /// Returns the results of the instruction - pub(crate) fn make_instruction_results( + fn make_instruction_results( &mut self, instruction_id: InstructionId, ctrl_typevars: Option>, - ) -> &[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(); + ) { + self.results.insert(instruction_id, Default::default()); // Get all of the types that this instruction produces // and append them as results. @@ -150,10 +156,6 @@ impl DataFlowGraph { for typ in typs { self.append_result(instruction_id, typ); } - - self.results.get_mut(&instruction_id) - .expect("all instructions should have a `result` allocation when instruction was added to the DFG") - .as_slice() } /// Return the result types of this instruction. @@ -274,11 +276,9 @@ mod tests { 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, None).len(); + let ins_id = dfg.make_instruction(ins, None); let results = dfg.instruction_results(ins_id); - assert_eq!(results.len(), num_results); + assert_eq!(results.len(), 1); } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs index 66db5e3e530..b30ff11c2e1 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/function_builder.rs @@ -3,7 +3,7 @@ use acvm::FieldElement; use crate::ssa_refactor::ir::{ basic_block::BasicBlockId, function::{Function, FunctionId}, - instruction::{Binary, BinaryOp, Instruction, InstructionId}, + instruction::{Binary, BinaryOp, Instruction}, types::Type, value::ValueId, }; @@ -71,18 +71,21 @@ impl<'ssa> FunctionBuilder<'ssa> { self.numeric_constant(value.into(), Type::field()) } - fn insert_instruction(&mut self, instruction: Instruction) -> InstructionId { - let id = self.current_function.dfg.make_instruction(instruction); + fn insert_instruction( + &mut self, + instruction: Instruction, + ctrl_typevars: Option>, + ) -> &[ValueId] { + let id = self.current_function.dfg.make_instruction(instruction, ctrl_typevars); self.current_function.dfg.insert_instruction_in_block(self.current_block, id); - id + self.current_function.dfg.instruction_results(id) } /// Insert an allocate instruction at the end of the current block, allocating the /// given amount of field elements. Returns the result of the allocate instruction, /// 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, None)[0] + self.insert_instruction(Instruction::Allocate { size: size_to_allocate }, None)[0] } /// Insert a Load instruction at the end of the current block, loading from the given address @@ -90,14 +93,13 @@ impl<'ssa> FunctionBuilder<'ssa> { /// a single value. Loading multiple values (such as a tuple) will require multiple loads. /// 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, Some(vec![type_to_load]))[0] + self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load]))[0] } /// 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. pub(crate) fn insert_store(&mut self, address: ValueId, value: ValueId) { - self.insert_instruction(Instruction::Store { address, value }); + self.insert_instruction(Instruction::Store { address, value }, None); } /// Insert a binary instruction at the end of the current block. @@ -108,14 +110,13 @@ impl<'ssa> FunctionBuilder<'ssa> { operator: BinaryOp, rhs: ValueId, ) -> ValueId { - let id = self.insert_instruction(Instruction::Binary(Binary { lhs, rhs, operator })); - self.current_function.dfg.make_instruction_results(id, None)[0] + let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); + self.insert_instruction(instruction, 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] + self.insert_instruction(Instruction::Not(rhs), None)[0] } }