diff --git a/Cargo.lock b/Cargo.lock index 20bb8a7ca0d..0227b1698bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -473,6 +473,15 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "bitmaps" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "031043d04099746d8db04daf1fa424b2bc8bd69d92b25962dcde24da39ab64a2" +dependencies = [ + "typenum", +] + [[package]] name = "bitvec" version = "1.0.1" @@ -1618,6 +1627,20 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "im" +version = "15.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0acd33ff0285af998aaf9b57342af478078f53492322fafc47450e09397e0e9" +dependencies = [ + "bitmaps", + "rand_core 0.6.4", + "rand_xoshiro", + "sized-chunks", + "typenum", + "version_check", +] + [[package]] name = "indenter" version = "0.3.3" @@ -2021,6 +2044,7 @@ version = "0.6.0" dependencies = [ "acvm", "arena", + "im", "iter-extended", "noirc_abi", "noirc_errors", @@ -2369,6 +2393,15 @@ dependencies = [ "getrandom", ] +[[package]] +name = "rand_xoshiro" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f97cdb2a36ed4183de61b2f824cc45c9f1037f28afe0a322e9fff4c108b5aaa" +dependencies = [ + "rand_core 0.6.4", +] + [[package]] name = "rayon" version = "1.7.0" @@ -2968,6 +3001,16 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f27f6278552951f1f2b8cf9da965d10969b2efdea95a6ec47987ab46edfe263a" +[[package]] +name = "sized-chunks" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16d69225bde7a69b235da73377861095455d298f2b970996eec25ddbb42b3d1e" +dependencies = [ + "bitmaps", + "typenum", +] + [[package]] name = "slab" version = "0.4.8" diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 601bacd278b..25948ba1784 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -44,7 +44,7 @@ pub(crate) struct ProveCommand { verifier_name: String, /// Verify proof after proving - #[arg(short, long)] + #[arg(long)] verify: bool, #[clap(flatten)] diff --git a/crates/noirc_evaluator/Cargo.toml b/crates/noirc_evaluator/Cargo.toml index 2e82d34d2dd..b2a01b142d0 100644 --- a/crates/noirc_evaluator/Cargo.toml +++ b/crates/noirc_evaluator/Cargo.toml @@ -16,6 +16,7 @@ iter-extended.workspace = true thiserror.workspace = true num-bigint = "0.4" num-traits = "0.2.8" +im = "15.1" [dev-dependencies] rand="0.8.5" diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 776f7c992ab..827a221b15b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -29,7 +29,6 @@ pub mod ssa_gen; /// form and performing optimizations there. When finished, /// convert the final SSA into ACIR and return it. pub(crate) fn optimize_into_acir(program: Program, allow_log_ops: bool) -> GeneratedAcir { - let func_signature = program.main_function_signature.clone(); let ssa = ssa_gen::generate_ssa(program).print("Initial SSA:"); let brillig = ssa.to_brillig(); ssa.inline_functions() @@ -44,7 +43,7 @@ pub(crate) fn optimize_into_acir(program: Program, allow_log_ops: bool) -> Gener .print("After Mem2Reg:") .fold_constants() .print("After Constant Folding:") - .into_acir(func_signature, brillig, allow_log_ops) + .into_acir(brillig, allow_log_ops) } /// Compiles the Program into ACIR and applies optimizations to the arithmetic gates diff --git a/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs index 400744fda11..db39b1c8110 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs @@ -2,92 +2,7 @@ use std::collections::BTreeMap; use acvm::acir::native_types::Witness; use iter_extended::{btree_map, vecmap}; -use noirc_abi::{Abi, AbiParameter, AbiType, FunctionSignature}; - -/// Traverses the parameters to the program to infer the lengths of any arrays that occur. -/// -/// This is needed for the acir_gen pass, because while the SSA representation of the program -/// knows the positions at which any arrays occur in the parameters to main, it does not know the -/// lengths of said arrays. -/// -/// This function returns the lengths ordered such as to correspond to the ordering used by the -/// SSA representation. This allows the lengths to be consumed as array params are encountered in -/// the SSA. -pub(crate) fn collate_array_info(abi_params: &[AbiParameter]) -> Vec<(usize, AbiType)> { - let mut acc = Vec::new(); - for abi_param in abi_params { - collate_array_info_recursive(&mut acc, &abi_param.typ); - } - acc -} - -/// The underlying recursive implementation of `collate_array_info` -/// -/// This does a depth-first traversal of the abi until an array (or string) is encountered, at -/// which point arrays are handled differently depending on the element type: -/// - arrays of fields, integers or booleans produce an array of the specified length -/// - arrays of structs produce an array of the specified length for each field of the flatten -/// struct (which reflects a simplification made during monomorphization) -fn collate_array_info_recursive(acc: &mut Vec<(usize, AbiType)>, abi_type: &AbiType) { - match abi_type { - AbiType::Array { length, typ: elem_type } => { - let elem_type = elem_type.as_ref(); - match elem_type { - AbiType::Array { .. } => { - unreachable!("2D arrays are not supported"); - } - AbiType::Struct { .. } => { - // monomorphization converts arrays of structs into an array per flattened - // struct field. - let mut destructured_array_types = Vec::new(); - flatten_abi_type_recursive(&mut destructured_array_types, elem_type); - for abi_type in destructured_array_types { - acc.push((*length as usize, abi_type)); - } - } - AbiType::String { .. } => { - unreachable!("Arrays of strings are not supported"); - } - AbiType::Boolean | AbiType::Field | AbiType::Integer { .. } => { - // Simple 1D array - acc.push((*length as usize, elem_type.clone())); - } - } - } - AbiType::Struct { fields } => { - for (_, field_type) in fields { - collate_array_info_recursive(acc, field_type); - } - } - AbiType::String { length } => { - // Strings are implemented as u8 arrays - let element_type = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 }; - acc.push((*length as usize, element_type)); - } - AbiType::Boolean | AbiType::Field | AbiType::Integer { .. } => { - // Do not produce arrays - } - } -} - -/// Used for flattening a struct into its ordered constituent field types. This is needed for -/// informing knowing the bit widths of any array sets that were destructured from an array of -/// structs. For this reason, any array encountered within this function are considered to be -/// nested within a struct and are therefore disallowed. This is acceptable because this function -/// will only be applied to structs which have been found in an array. -fn flatten_abi_type_recursive(acc: &mut Vec, abi_type: &AbiType) { - match abi_type { - AbiType::Array { .. } | AbiType::String { .. } => { - unreachable!("2D arrays are unsupported") - } - AbiType::Boolean | AbiType::Integer { .. } | AbiType::Field => acc.push(abi_type.clone()), - AbiType::Struct { fields } => { - for (_, field_type) in fields { - flatten_abi_type_recursive(acc, field_type); - } - } - } -} +use noirc_abi::{Abi, AbiParameter, FunctionSignature}; /// Arranges a function signature and a generated circuit's return witnesses into a /// `noirc_abi::Abi`. diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs index 7a755210e2b..ae183aa962f 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir.rs @@ -1,4 +1,3 @@ pub(crate) mod acir_variable; pub(crate) mod errors; pub(crate) mod generated_acir; -pub(crate) mod memory; diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index 4bf6a77e6d2..a6d7ae157d3 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -1,12 +1,9 @@ +use crate::ssa_refactor::acir_gen::AcirValue; use crate::ssa_refactor::ir::types::Type as SsaType; use crate::ssa_refactor::ir::{instruction::Endian, map::TwoWayMap, types::NumericType}; use acvm::acir::brillig_vm::Opcode as BrilligOpcode; -use super::{ - errors::AcirGenError, - generated_acir::GeneratedAcir, - memory::{ArrayId, Memory}, -}; +use super::{errors::AcirGenError, generated_acir::GeneratedAcir}; use acvm::{ acir::{ circuit::opcodes::FunctionInput, @@ -16,7 +13,7 @@ use acvm::{ FieldElement, }; use iter_extended::vecmap; -use std::{borrow::Cow, collections::HashMap, hash::Hash}; +use std::{borrow::Cow, hash::Hash}; #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] /// High level Type descriptor for Variables. @@ -31,8 +28,12 @@ use std::{borrow::Cow, collections::HashMap, hash::Hash}; pub(crate) struct AcirType(NumericType); impl AcirType { + pub(crate) fn new(typ: NumericType) -> Self { + Self(typ) + } + /// Returns the bit size of the underlying type - fn bit_size(&self) -> u32 { + pub(crate) fn bit_size(&self) -> u32 { match self.0 { NumericType::Signed { bit_size } => bit_size, NumericType::Unsigned { bit_size } => bit_size, @@ -45,10 +46,17 @@ impl AcirType { AcirType(NumericType::Unsigned { bit_size: 1 }) } } + impl From for AcirType { fn from(value: SsaType) -> Self { + AcirType::from(&value) + } +} + +impl<'a> From<&'a SsaType> for AcirType { + fn from(value: &SsaType) -> Self { match value { - SsaType::Numeric(numeric_type) => AcirType(numeric_type), + SsaType::Numeric(numeric_type) => AcirType(*numeric_type), _ => unreachable!("The type {value} cannot be represented in ACIR"), } } @@ -72,11 +80,6 @@ pub(crate) struct AcirContext { /// then the `acir_ir` will be populated to assert this /// addition. acir_ir: GeneratedAcir, - - /// Maps an `AcirVar` to its type. - variables_to_types: HashMap, - /// Maps the elements of virtual arrays to their `AcirVar` elements - memory: Memory, } impl AcirContext { @@ -159,21 +162,15 @@ impl AcirContext { } /// Returns an `AcirVar` that is the XOR result of `lhs` & `rhs`. - pub(crate) fn xor_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { - let lhs_bit_size = *self - .variables_to_types - .get(&lhs) - .expect("ICE: XOR applied to field type, this should be caught by the type system"); - let rhs_bit_size = *self - .variables_to_types - .get(&lhs) - .expect("ICE: XOR applied to field type, this should be caught by the type system"); - assert_eq!(lhs_bit_size, rhs_bit_size, "ICE: Operands to XOR require equal bit size"); - - let outputs = self.black_box_function(BlackBoxFunc::XOR, vec![lhs, rhs])?; - let result = outputs[0]; - self.variables_to_types.insert(result, lhs_bit_size); - Ok(result) + pub(crate) fn xor_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + typ: AcirType, + ) -> Result { + let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)]; + let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs)?; + Ok(outputs[0]) } /// Returns an `AcirVar` that is the AND result of `lhs` & `rhs`. @@ -183,31 +180,25 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, ) -> Result { - let outputs = self.black_box_function(BlackBoxFunc::AND, vec![lhs, rhs])?; - let result = outputs[0]; - self.variables_to_types.insert(result, typ); - Ok(result) + let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)]; + let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?; + Ok(outputs[0]) } /// Returns an `AcirVar` that is the OR result of `lhs` & `rhs`. - pub(crate) fn or_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { - let lhs_type = *self - .variables_to_types - .get(&lhs) - .expect("all variables should have a type attached to them"); - let rhs_type = *self - .variables_to_types - .get(&lhs) - .expect("all variables should have a type attached to them"); - assert_eq!(lhs_type, rhs_type, "types in or expressions should be the same"); - let bit_size = lhs_type.bit_size(); - - let result = if bit_size == 1 { + pub(crate) fn or_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + typ: AcirType, + ) -> Result { + let bit_size = typ.bit_size(); + if bit_size == 1 { // Operands are booleans // a + b - ab let sum = self.add_var(lhs, rhs)?; let mul = self.mul_var(lhs, rhs)?; - self.sub_var(sum, mul)? + self.sub_var(sum, mul) } else { // Implement OR in terms of AND // max - ((max - a) AND (max -b)) @@ -216,15 +207,10 @@ impl AcirContext { let max = self.add_constant(FieldElement::from((1_u128 << bit_size) - 1)); let a = self.sub_var(max, lhs)?; let b = self.sub_var(max, rhs)?; - // We track the bit sizes of these intermediaries so that blackbox input generation - // infers them correctly. - self.variables_to_types.insert(a, lhs_type); - self.variables_to_types.insert(b, lhs_type); - let output = self.black_box_function(BlackBoxFunc::AND, vec![a, b])?; - self.sub_var(max, output[0])? - }; - self.variables_to_types.insert(result, lhs_type); - Ok(result) + let inputs = vec![AcirValue::Var(a, typ), AcirValue::Var(b, typ)]; + let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?; + self.sub_var(max, outputs[0]) + } } /// Constrains the `lhs` and `rhs` to be equal. @@ -365,14 +351,8 @@ impl AcirContext { /// /// `x` must be a 1-bit integer (i.e. a boolean) pub(crate) fn not_var(&mut self, x: AcirVar) -> AcirVar { - assert_eq!( - self.variables_to_types.get(&x), - Some(&AcirType::boolean()), - "ICE: NOT op applied to non-boolean type" - ); - let data = &self.vars[x]; // Since `x` can only be 0 or 1, we can derive NOT as 1 - x - match data { + match &self.vars[x] { AcirVarData::Const(constant) => { self.add_data(AcirVarData::Expr(&Expression::one() - &Expression::from(*constant))) } @@ -413,6 +393,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, + bit_size: u32, ) -> Result<(AcirVar, AcirVar), AcirGenError> { let predicate = Expression::one(); @@ -422,18 +403,8 @@ impl AcirContext { let lhs_expr = lhs_data.to_expression(); let rhs_expr = rhs_data.to_expression(); - let lhs_bit_size = self.variables_to_types.get(&lhs).expect("euclidean division cannot be made on variables with no known bit size. This should have been caught by the frontend").bit_size(); - let rhs_bit_size = self.variables_to_types.get(&rhs).expect("euclidean division cannot be made on variables with no known bit size. This should have been caught by the frontend").bit_size(); - - assert_eq!( - lhs_bit_size, rhs_bit_size, - // This makes the assumption that the bit size is the last known integer - // type for this variable and that we are not getting the smallest range for example. - "Euclidean division can only be applied to variables of the same type" - ); - let (quotient, remainder) = - self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, lhs_bit_size, &predicate)?; + self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, bit_size, &predicate)?; let quotient_var = self.add_data(AcirVarData::Witness(quotient)); let remainder_var = self.add_data(AcirVarData::Witness(remainder)); @@ -446,10 +417,12 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, + bit_size: u32, ) -> Result { - let (_, remainder) = self.euclidean_division_var(lhs, rhs)?; + let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size)?; Ok(remainder) } + /// Returns an `AcirVar` that is constrained to be `lhs >> rhs`. /// /// We convert right shifts to divisions, so this is equivalent to @@ -514,7 +487,6 @@ impl AcirContext { // integer, but a function requires the parameter to be a Field. } } - self.variables_to_types.insert(variable, AcirType(*numeric_type)); Ok(variable) } @@ -532,24 +504,25 @@ impl AcirContext { Ok(self.add_data(AcirVarData::Expr(result_expr))) } + /// Returns an `AcirVar` which will be `1` if lhs >= rhs /// and `0` otherwise. - fn more_than_eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + fn more_than_eq_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + bit_size: u32, + ) -> Result { let lhs_data = &self.vars[lhs]; let rhs_data = &self.vars[rhs]; let lhs_expr = lhs_data.to_expression(); let rhs_expr = rhs_data.to_expression(); - let lhs_type = self.variables_to_types.get(&lhs).expect("comparisons cannot be made on variables with no known max bit size. This should have been caught by the frontend"); - let rhs_type = self.variables_to_types.get(&rhs).expect("comparisons cannot be made on variables with no known max bit size. This should have been caught by the frontend"); - // TODO: check what happens when we do (a as u8) >= (b as u32) // TODO: The frontend should shout in this case - assert_eq!(lhs_type, rhs_type, "types in a more than eq comparison should be the same"); - let is_greater_than_eq = - self.acir_ir.more_than_eq_comparison(&lhs_expr, &rhs_expr, lhs_type.bit_size())?; + self.acir_ir.more_than_eq_comparison(&lhs_expr, &rhs_expr, bit_size)?; Ok(self.add_data(AcirVarData::Witness(is_greater_than_eq))) } @@ -560,10 +533,11 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, + bit_size: u32, ) -> Result { // Flip the result of calling more than equal method to // compute less than. - let comparison = self.more_than_eq_var(lhs, rhs)?; + let comparison = self.more_than_eq_var(lhs, rhs, bit_size)?; let one = self.add_constant(FieldElement::one()); self.sub_var(one, comparison) // comparison_negated @@ -574,24 +548,26 @@ impl AcirContext { pub(crate) fn black_box_function( &mut self, name: BlackBoxFunc, - mut inputs: Vec, + mut inputs: Vec, ) -> Result, AcirGenError> { // Separate out any arguments that should be constants let constants = match name { BlackBoxFunc::Pedersen => { // The last argument of pedersen is the domain separator, which must be a constant let domain_var = - inputs.pop().expect("ICE: Pedersen call requires domain separator"); + inputs.pop().expect("ICE: Pedersen call requires domain separator").into_var(); + let domain_constant = self.vars[domain_var] .as_constant() .expect("ICE: Domain separator must be a constant"); + vec![domain_constant] } _ => vec![], }; // Convert `AcirVar` to `FunctionInput` - let inputs = self.prepare_inputs_for_black_box_func_call(&inputs)?; + let inputs = self.prepare_inputs_for_black_box_func_call(inputs)?; // Call Black box with `FunctionInput` let outputs = self.acir_ir.call_black_box(name, inputs, constants); @@ -601,10 +577,7 @@ impl AcirContext { // // We do not apply range information on the output of the black box function. // See issue #1439 - let outputs_var = - vecmap(&outputs, |witness_index| self.add_data(AcirVarData::Witness(*witness_index))); - - Ok(outputs_var) + Ok(vecmap(&outputs, |witness_index| self.add_data(AcirVarData::Witness(*witness_index)))) } /// Black box function calls expect their inputs to be in a specific data structure (FunctionInput). @@ -612,51 +585,21 @@ impl AcirContext { /// This function will convert `AcirVar` into `FunctionInput` for a blackbox function call. fn prepare_inputs_for_black_box_func_call( &mut self, - inputs: &[AcirVar], + inputs: Vec, ) -> Result, AcirGenError> { let mut witnesses = Vec::new(); for input in inputs { - let var_data = &self.vars[input]; - - // Intrinsics only accept Witnesses. This is not a limitation of the - // intrinsics, its just how we have defined things. Ideally, we allow - // constants too. - let expr = var_data.to_expression(); - let witness = self.acir_ir.get_or_create_witness(&expr); - - // Fetch the number of bits for this variable - // If it has never been constrained before, then we will - // encounter None, and so we take the max number of bits for a - // field element. - let num_bits = match self.variables_to_types.get(input) { - Some(typ) => { - // In Noir, we specify the number of bits to take from the input - // by doing the following: - // - // ``` - // call_intrinsic(x as u8) - // ``` - // - // The `as u8` specifies that we want to take 8 bits from the `x` - // variable. - // - // There were discussions about the SSA IR optimizing out range - // constraints. We would want to be careful with it here. For example: - // - // ``` - // let x : u32 = y as u32 - // call_intrinsic(x as u64) - // ``` - // The `x as u64` is redundant since we know that `x` fits within a u32. - // However, since the `x as u64` line is being used to tell the intrinsic - // to take 64 bits, we cannot remove it. - - typ.bit_size() - } - None => FieldElement::max_num_bits(), - }; - - witnesses.push(FunctionInput { witness, num_bits }); + for (input, typ) in input.flatten() { + let var_data = &self.vars[input]; + + // Intrinsics only accept Witnesses. This is not a limitation of the + // intrinsics, its just how we have defined things. Ideally, we allow + // constants too. + let expr = var_data.to_expression(); + let witness = self.acir_ir.get_or_create_witness(&expr); + let num_bits = typ.bit_size(); + witnesses.push(FunctionInput { witness, num_bits }); + } } Ok(witnesses) } @@ -673,7 +616,8 @@ impl AcirContext { input_var: AcirVar, radix_var: AcirVar, limb_count_var: AcirVar, - ) -> Result, AcirGenError> { + result_element_type: AcirType, + ) -> Result, AcirGenError> { let radix = self.vars[&radix_var].as_constant().expect("ICE: radix should be a constant").to_u128() as u32; @@ -687,13 +631,16 @@ impl AcirContext { let limbs = self.acir_ir.radix_le_decompose(input_expr, radix, limb_count)?; - let mut limb_vars = vecmap(limbs, |witness| self.add_data(AcirVarData::Witness(witness))); + let mut limb_vars = vecmap(limbs, |witness| { + let witness = self.add_data(AcirVarData::Witness(witness)); + AcirValue::Var(witness, result_element_type) + }); if endian == Endian::Big { limb_vars.reverse(); } - Ok(limb_vars) + Ok(vec![AcirValue::Array(limb_vars.into())]) } /// Returns `AcirVar`s constrained to be the bit decomposition of the provided input @@ -702,13 +649,16 @@ impl AcirContext { endian: Endian, input_var: AcirVar, limb_count_var: AcirVar, - ) -> Result, AcirGenError> { + result_element_type: AcirType, + ) -> Result, AcirGenError> { let two_var = self.add_constant(FieldElement::from(2_u128)); - self.radix_decompose(endian, input_var, two_var, limb_count_var) + self.radix_decompose(endian, input_var, two_var, limb_count_var, result_element_type) } /// Prints the given `AcirVar`s as witnesses. - pub(crate) fn print(&mut self, input: Vec) -> Result<(), AcirGenError> { + pub(crate) fn print(&mut self, input: Vec) -> Result<(), AcirGenError> { + let input = Self::flatten_values(input); + let witnesses = vecmap(input, |acir_var| { let var_data = &self.vars[acir_var]; let expr = var_data.to_expression(); @@ -718,42 +668,32 @@ impl AcirContext { Ok(()) } - /// Terminates the context and takes the resulting `GeneratedAcir` - pub(crate) fn finish(self) -> GeneratedAcir { - self.acir_ir - } - - /// Allocates an array of size `size` and returns a pointer to the array in memory. - pub(crate) fn allocate_array(&mut self, size: usize) -> ArrayId { - self.memory.allocate(size) - } - - /// Stores the given `AcirVar` at the specified address in memory - pub(crate) fn array_store( - &mut self, - array_id: ArrayId, - index: usize, - element: AcirVar, - ) -> Result<(), AcirGenError> { - self.memory.constant_set(array_id, index, element) + /// Flatten the given Vector of AcirValues into a single vector of only variables. + /// Each AcirValue::Array in the vector is recursively flattened, so each element + /// will flattened into the resulting Vec. E.g. flatten_values([1, [2, 3]) == [1, 2, 3]. + fn flatten_values(values: Vec) -> Vec { + let mut acir_vars = Vec::with_capacity(values.len()); + for value in values { + Self::flatten_value(&mut acir_vars, value); + } + acir_vars } - /// Gets the last stored `AcirVar` at the specified address in memory. - /// - /// This errors if nothing was previously stored at the address. - pub(crate) fn array_load( - &mut self, - array_id: ArrayId, - index: usize, - ) -> Result { - self.memory.constant_get(array_id, index) + /// Recursive helper for flatten_values to flatten a single AcirValue into the result vector. + pub(crate) fn flatten_value(acir_vars: &mut Vec, value: AcirValue) { + match value { + AcirValue::Var(acir_var, _) => acir_vars.push(acir_var), + AcirValue::Array(array) => { + for value in array { + Self::flatten_value(acir_vars, value); + } + } + } } - /// Gets all `AcirVar` elements currently stored at the array. - /// - /// This errors if nothing was previously stored any element in the array. - pub(crate) fn array_load_all(&self, array_id: ArrayId) -> Result, AcirGenError> { - self.memory.constant_get_all(array_id) + /// Terminates the context and takes the resulting `GeneratedAcir` + pub(crate) fn finish(self) -> GeneratedAcir { + self.acir_ir } /// Adds `Data` into the context and assigns it a Variable. diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs index f37abbc38e9..cd856192fe9 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/errors.rs @@ -1,10 +1,7 @@ -use super::memory::ArrayId; - #[derive(Debug, PartialEq, Eq, Clone)] pub(crate) enum AcirGenError { InvalidRangeConstraint { num_bits: u32 }, IndexOutOfBounds { index: usize, array_size: usize }, - UninitializedElementInArray { index: usize, array_id: ArrayId }, UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32 }, } @@ -19,9 +16,6 @@ impl AcirGenError { AcirGenError::IndexOutOfBounds { index, array_size } => { format!("Index out of bounds, array has size {array_size}, but index was {index}") } - AcirGenError::UninitializedElementInArray { index, array_id } => { - format!("The element at index {index} was never set in array {array_id:?}") - } AcirGenError::UnsupportedIntegerSize { num_bits, max_num_bits } => { format!("Integer sized {num_bits} is over the max supported size of {max_num_bits}") } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs deleted file mode 100644 index 3cbf70bfdea..00000000000 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs +++ /dev/null @@ -1,122 +0,0 @@ -use crate::ssa_refactor::ir::map::{DenseMap, Id}; - -use super::{acir_variable::AcirVar, errors::AcirGenError}; - -#[derive(Debug, Default)] -pub(crate) struct Array { - /// Elements in the array. - /// None represents the element not being set yet - /// - /// The size of this vector cannot be changed once set. - /// This follows the behavior of an Array as opposed to a Vector. - elements: Vec>, -} - -impl Array { - pub(crate) fn size(&self) -> usize { - self.elements.len() - } - - pub(crate) fn new(size: usize) -> Array { - Self { elements: vec![None; size] } - } -} - -#[derive(Debug, Default)] -/// Memory used to represent Arrays -pub(crate) struct Memory { - arrays: DenseMap, -} - -impl Memory { - /// Allocates an array of size `size`. - /// The elements in the array are not zero initialized - pub(crate) fn allocate(&mut self, size: usize) -> ArrayId { - let array = Array::new(size); - self.arrays.insert(array) - } - - /// Sets an element at the array that `ArrayId` points to. - /// The index must be constant in the Noir program. - pub(crate) fn constant_set( - &mut self, - array_id: ArrayId, - index: usize, - element: AcirVar, - ) -> Result<(), AcirGenError> { - let array = &mut self.arrays[array_id]; - Self::check_bounds(index, array.size())?; - - array.elements[index] = Some(element); - - Ok(()) - } - - /// Gets an element at the array that `ArrayId` points to. - /// The index must be constant in the Noir program. - pub(crate) fn constant_get( - &self, - array_id: ArrayId, - index: usize, - ) -> Result { - let array = &self.arrays[array_id]; - Self::check_bounds(index, array.size())?; - - array.elements[index].ok_or(AcirGenError::UninitializedElementInArray { index, array_id }) - } - - /// Gets all elements at the array that `ArrayId` points to. - /// - /// This returns an error if any of the array's elements have not been initialized. - pub(crate) fn constant_get_all(&self, array_id: ArrayId) -> Result, AcirGenError> { - let array = &self.arrays[array_id]; - let mut elements = Vec::new(); - for index in 0..array.size() { - elements.push(self.constant_get(array_id, index)?); - } - Ok(elements) - } - - /// Check if the index is larger than the array size - fn check_bounds(index: usize, array_size: usize) -> Result<(), AcirGenError> { - if index < array_size { - Ok(()) - } else { - Err(AcirGenError::IndexOutOfBounds { index, array_size }) - } - } -} - -/// Pointer to an allocated `Array` -pub(crate) type ArrayId = Id; - -#[cfg(test)] -mod tests { - use crate::ssa_refactor::acir_gen::acir_ir::{errors::AcirGenError, memory::Memory}; - - #[test] - fn smoke_api_get_uninitialized_element_out() { - let mut memory = Memory::default(); - - let array_size = 10; - let index = 0; - - let array_id = memory.allocate(array_size); - - let element = memory.constant_get(array_id, index); - // Should get an error because we are trying to get an element which has not been initialized - // yet. - assert_eq!(element, Err(AcirGenError::UninitializedElementInArray { index, array_id })); - } - #[test] - fn smoke_api_out_of_bounds() { - let mut memory = Memory::default(); - - let array_size = 10; - let array_id = memory.allocate(array_size); - - let element = memory.constant_get(array_id, array_size); - // Should get an index out of bounds error - assert_eq!(element, Err(AcirGenError::IndexOutOfBounds { index: array_size, array_size })); - } -} diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 78823fc330c..537b08ddd93 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -3,12 +3,10 @@ use std::collections::HashMap; use self::acir_ir::{ - acir_variable::{AcirContext, AcirVar}, + acir_variable::{AcirContext, AcirType, AcirVar}, errors::AcirGenError, - memory::ArrayId, }; use super::{ - abi_gen::collate_array_info, ir::{ dfg::DataFlowGraph, function::RuntimeType, @@ -23,7 +21,7 @@ use super::{ }; use crate::brillig::{artifact::BrilligArtifact, Brillig}; use acvm::FieldElement; -use noirc_abi::{AbiType, FunctionSignature, Sign}; +use iter_extended::vecmap; pub(crate) use acir_ir::generated_acir::GeneratedAcir; @@ -39,58 +37,44 @@ struct Context { /// AcirVar per SSA value. Before creating an `AcirVar` /// for an SSA value, we check this map. If an `AcirVar` /// already exists for this Value, we return the `AcirVar`. - ssa_value_to_acir_var: HashMap, AcirVar>, - /// Maps SSA values to array addresses (including index offset). - /// - /// When converting parameters the of main, `ArrayId`s are gathered and stored with an offset - /// of 0. When the use of these stored values are detected for address arithmetic, the results - /// of such instructions are stored, in effect capturing any further values that refer to - /// addresses. - ssa_value_to_array_address: HashMap, + ssa_values: HashMap, AcirValue>, + /// Manages and builds the `AcirVar`s to which the converted SSA values refer. acir_context: AcirContext, } -impl Ssa { - pub(crate) fn into_acir( - self, - main_function_signature: FunctionSignature, - brillig: Brillig, - allow_log_ops: bool, - ) -> GeneratedAcir { - let param_arrays_info: Vec<_> = collate_array_info(&main_function_signature.0) - .iter() - .map(|(size, abi_type)| (*size, numeric_type_for_abi_array_element_type(abi_type))) - .collect(); +#[derive(Debug, Clone)] +pub(crate) enum AcirValue { + Var(AcirVar, AcirType), + Array(im::Vector), +} - let context = Context::default(); - context.convert_ssa(self, ¶m_arrays_info, brillig, allow_log_ops) +impl AcirValue { + fn into_var(self) -> AcirVar { + match self { + AcirValue::Var(var, _) => var, + AcirValue::Array(_) => panic!("Called AcirValue::into_var on an array"), + } + } + + fn flatten(self) -> Vec<(AcirVar, AcirType)> { + match self { + AcirValue::Var(var, typ) => vec![(var, typ)], + AcirValue::Array(array) => array.into_iter().flat_map(AcirValue::flatten).collect(), + } } } -/// Gives the equivalent ssa numeric type for the given abi type. We are dealing in the context of -/// arrays - hence why only numerics are supported. -fn numeric_type_for_abi_array_element_type(abi_type: &AbiType) -> NumericType { - match abi_type { - AbiType::Boolean => NumericType::Unsigned { bit_size: 1 }, - AbiType::Integer { sign, width } => match sign { - Sign::Signed => NumericType::Signed { bit_size: *width }, - Sign::Unsigned => NumericType::Unsigned { bit_size: *width }, - }, - AbiType::Field => NumericType::NativeField, - _ => unreachable!("Non-numeric cannot be array element"), +impl Ssa { + pub(crate) fn into_acir(self, brillig: Brillig, allow_log_ops: bool) -> GeneratedAcir { + let context = Context::default(); + context.convert_ssa(self, brillig, allow_log_ops) } } impl Context { /// Converts SSA into ACIR - fn convert_ssa( - mut self, - ssa: Ssa, - param_array_info: &[(usize, NumericType)], - brillig: Brillig, - allow_log_ops: bool, - ) -> GeneratedAcir { + fn convert_ssa(mut self, ssa: Ssa, brillig: Brillig, allow_log_ops: bool) -> GeneratedAcir { assert_eq!( ssa.functions.len(), 1, @@ -100,7 +84,7 @@ impl Context { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; - self.convert_ssa_block_params(entry_block.parameters(), dfg, param_array_info); + self.convert_ssa_block_params(entry_block.parameters(), dfg); for instruction_id in entry_block.instructions() { self.convert_ssa_instruction(*instruction_id, dfg, &ssa, &brillig, allow_log_ops); @@ -111,51 +95,34 @@ impl Context { self.acir_context.finish() } - /// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array - /// element. At the same time `ArrayId`s are bound for any references within the params. - fn convert_ssa_block_params( - &mut self, - params: &[ValueId], - dfg: &DataFlowGraph, - param_arrays_info: &[(usize, NumericType)], - ) { - let mut param_arrays_info_iter = param_arrays_info.iter(); + /// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array element. + fn convert_ssa_block_params(&mut self, params: &[ValueId], dfg: &DataFlowGraph) { for param_id in params { - let value = &dfg[*param_id]; - let param_type = match value { - Value::Param { typ, .. } => typ, - _ => unreachable!("ICE: Only Param type values should appear in block parameters"), - }; - match param_type { - Type::Numeric(numeric_type) => { - let acir_var = self.add_numeric_input_var(numeric_type); - self.ssa_value_to_acir_var.insert(*param_id, acir_var); - } - Type::Reference => { - let (array_length, numeric_type) = param_arrays_info_iter - .next() - .expect("ICE: fewer arrays in abi than in block params"); - let array_id = self.acir_context.allocate_array(*array_length); - self.ssa_value_to_array_address.insert(*param_id, (array_id, 0)); - for index in 0..*array_length { - let acir_var = self.add_numeric_input_var(numeric_type); - self.acir_context - .array_store(array_id, index, acir_var) - .expect("invalid array store"); + let typ = dfg.type_of_value(*param_id); + let value = self.convert_ssa_block_param(&typ); + self.ssa_values.insert(*param_id, value); + } + } + + fn convert_ssa_block_param(&mut self, param_type: &Type) -> AcirValue { + match param_type { + Type::Numeric(numeric_type) => { + let typ = AcirType::new(*numeric_type); + AcirValue::Var(self.add_numeric_input_var(numeric_type), typ) + } + Type::Array(element_types, length) => { + let mut elements = im::Vector::new(); + + for _ in 0..*length { + for element in element_types.iter() { + elements.push_back(self.convert_ssa_block_param(element)); } } - _ => { - unreachable!( - "ICE: Params to the program should only contains numerics and arrays" - ) - } + + AcirValue::Array(elements) } + _ => unreachable!("ICE: Params to the program should only contains numbers and arrays"), } - assert_eq!( - param_arrays_info_iter.next(), - None, - "ICE: more arrays in abi than in block params" - ); } /// Creates an `AcirVar` corresponding to a parameter witness to appears in the abi. A range @@ -184,28 +151,20 @@ impl Context { ) { let instruction = &dfg[instruction_id]; - let (results_id, results_vars) = match instruction { + match instruction { Instruction::Binary(binary) => { - let result_ids = dfg.instruction_results(instruction_id); - if Self::value_is_array_address(result_ids[0], dfg) { - self.track_array_address(result_ids[0], binary, dfg); - (Vec::new(), Vec::new()) - } else { - let result_acir_var = self - .convert_ssa_binary(binary, dfg) - .expect("add Result types to all methods so errors bubble up"); - (vec![result_ids[0]], vec![result_acir_var]) - } + let result_acir_var = self + .convert_ssa_binary(binary, dfg) + .expect("add Result types to all methods so errors bubble up"); + self.define_result_var(dfg, instruction_id, result_acir_var); } Instruction::Constrain(value_id) => { - let constrain_condition = self.convert_ssa_value(*value_id, dfg); + let constrain_condition = self.convert_numeric_value(*value_id, dfg); self.acir_context.assert_eq_one(constrain_condition); - (Vec::new(), Vec::new()) } Instruction::Cast(value_id, typ) => { let result_acir_var = self.convert_ssa_cast(value_id, typ, dfg); - let result_ids = dfg.instruction_results(instruction_id); - (vec![result_ids[0]], vec![result_acir_var]) + self.define_result_var(dfg, instruction_id, result_acir_var); } Instruction::Call { func, arguments } => { let result_ids = dfg.instruction_results(instruction_id); @@ -220,7 +179,7 @@ impl Context { // Generate the brillig code of the function let code = BrilligArtifact::default().link(&brillig[*id]); self.acir_context.brillig(code); - (result_ids.to_vec(), Vec::new()) + // TODO: Set result values } } } @@ -230,66 +189,103 @@ impl Context { arguments, dfg, allow_log_ops, + result_ids, ); - if Self::value_is_array_address(result_ids[0], dfg) { - // Some intrinsics return arrays - these require an allocation - if result_ids.len() != 1 { - todo!("Complex return type encountered. Restructuring required to provide info on how to repackage result"); - } - let array_id = self.acir_context.allocate_array(outputs.len()); - self.ssa_value_to_array_address.insert(result_ids[0], (array_id, 0)); - for (index, element) in outputs.iter().enumerate() { - self.acir_context - .array_store(array_id, index, *element) - .expect("add Result types to all methods so errors bubble up"); - } - (Vec::new(), Vec::new()) - } else { - (result_ids.to_vec(), outputs) + + // Issue #1438 causes this check to fail with intrinsics that return 0 + // results but the ssa form instead creates 1 unit result value. + // assert_eq!(result_ids.len(), outputs.len()); + + for (result, output) in result_ids.iter().zip(outputs) { + self.ssa_values.insert(*result, output); } } _ => unreachable!("expected calling a function"), } } Instruction::Not(value_id) => { - let boolean_var = self.convert_ssa_value(*value_id, dfg); + let boolean_var = self.convert_numeric_value(*value_id, dfg); let result_acir_var = self.acir_context.not_var(boolean_var); - - let result_ids = dfg.instruction_results(instruction_id); - (vec![result_ids[0]], vec![result_acir_var]) - } - Instruction::Allocate { size } => { - let array_id = self.acir_context.allocate_array(*size as usize); - let result_ids = dfg.instruction_results(instruction_id); - self.ssa_value_to_array_address.insert(result_ids[0], (array_id, 0)); - (Vec::new(), Vec::new()) - } - Instruction::Store { address, value } => { - self.convert_ssa_store(address, value, dfg); - (Vec::new(), Vec::new()) - } - Instruction::Load { address } => { - let result_acir_var = self.convert_ssa_load(address); - let result_ids = dfg.instruction_results(instruction_id); - (vec![result_ids[0]], vec![result_acir_var]) + self.define_result_var(dfg, instruction_id, result_acir_var); } Instruction::Truncate { value, bit_size, max_bit_size } => { - let var = self.convert_ssa_value(*value, dfg); - let result_ids = dfg.instruction_results(instruction_id); + let var = self.convert_numeric_value(*value, dfg); let result_acir_var = self .acir_context .truncate_var(var, *bit_size, *max_bit_size) .expect("add Result types to all methods so errors bubble up"); - (vec![result_ids[0]], vec![result_acir_var]) + self.define_result_var(dfg, instruction_id, result_acir_var); + } + Instruction::ArrayGet { array, index } => { + self.handle_array_operation(instruction_id, *array, *index, None, dfg); + } + Instruction::ArraySet { array, index, value } => { + self.handle_array_operation(instruction_id, *array, *index, Some(*value), dfg); + } + Instruction::Allocate => { + unreachable!("Expected all allocate instructions to be removed before acir_gen") + } + Instruction::Store { .. } => { + unreachable!("Expected all store instructions to be removed before acir_gen") + } + Instruction::Load { .. } => { + unreachable!("Expected all load instructions to be removed before acir_gen") + } + } + } + + /// Handles an ArrayGet or ArraySet instruction. + /// To set an index of the array (and create a new array in doing so), pass Some(value) for + /// store_value. To just retrieve an index of the array, pass None for store_value. + fn handle_array_operation( + &mut self, + instruction: InstructionId, + array: ValueId, + index: ValueId, + store_value: Option, + dfg: &DataFlowGraph, + ) { + let array = self.convert_array_value(array, dfg); + let index = dfg + .get_numeric_constant(index) + .expect("Expected array index to be a known constant") + .try_to_u64() + .expect("Expected array index to fit into a u64") as usize; + + let value = match store_value { + Some(store_value) => { + let store_value = self.convert_value(store_value, dfg); + AcirValue::Array(array.update(index, store_value)) } + None => array[index].clone(), }; - // Map the results of the instructions to Acir variables - for (result_id, result_var) in results_id.into_iter().zip(results_vars) { - self.ssa_value_to_acir_var.insert(result_id, result_var); - } + self.define_result(dfg, instruction, value); + } + + /// Remember the result of an instruction returning a single value + fn define_result( + &mut self, + dfg: &DataFlowGraph, + instruction: InstructionId, + result: AcirValue, + ) { + let result_ids = dfg.instruction_results(instruction); + self.ssa_values.insert(result_ids[0], result); + } + + /// Remember the result of instruction returning a single numeric value + fn define_result_var( + &mut self, + dfg: &DataFlowGraph, + instruction: InstructionId, + result: AcirVar, + ) { + let result_ids = dfg.instruction_results(instruction); + let typ = dfg.type_of_value(result_ids[0]).into(); + self.define_result(dfg, instruction, AcirValue::Var(result, typ)); } /// Converts an SSA terminator's return values into their ACIR representations @@ -309,9 +305,7 @@ impl Context { // The return value may or may not be an array reference. Calling `flatten_value_list` // will expand the array if there is one. - let return_acir_vars = self - .flatten_value_list(return_values, dfg) - .expect("add Result types to all methods so errors bubble up"); + let return_acir_vars = self.flatten_value_list(return_values, dfg); for acir_var in return_acir_vars { self.acir_context.return_var(acir_var); @@ -331,21 +325,46 @@ impl Context { /// It is not safe to call this function on value ids that represent addresses. Instructions /// involving such values are evaluated via a separate path and stored in /// `ssa_value_to_array_address` instead. - fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> AcirVar { + fn convert_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> AcirValue { let value = &dfg[value_id]; - if let Some(acir_var) = self.ssa_value_to_acir_var.get(&value_id) { - return *acir_var; + if let Some(acir_value) = self.ssa_values.get(&value_id) { + return acir_value.clone(); } - let acir_var = match value { - Value::NumericConstant { constant, .. } => self.acir_context.add_constant(*constant), + + let acir_value = match value { + Value::NumericConstant { constant, typ } => { + AcirValue::Var(self.acir_context.add_constant(*constant), typ.into()) + } + Value::Array { array, .. } => { + let elements = array.iter().map(|element| self.convert_value(*element, dfg)); + AcirValue::Array(elements.collect()) + } Value::Intrinsic(..) => todo!(), Value::Function(..) => unreachable!("ICE: All functions should have been inlined"), Value::Instruction { .. } | Value::Param { .. } => { unreachable!("ICE: Should have been in cache {value:?}") } }; - self.ssa_value_to_acir_var.insert(value_id, acir_var); - acir_var + self.ssa_values.insert(value_id, acir_value.clone()); + acir_value + } + + fn convert_array_value( + &mut self, + value_id: ValueId, + dfg: &DataFlowGraph, + ) -> im::Vector { + match self.convert_value(value_id, dfg) { + AcirValue::Var(acir_var, _) => panic!("Expected an array value, found: {acir_var:?}"), + AcirValue::Array(array) => array, + } + } + + fn convert_numeric_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> AcirVar { + match self.convert_value(value_id, dfg) { + AcirValue::Var(acir_var, _) => acir_var, + AcirValue::Array(array) => panic!("Expected a numeric value, found: {array:?}"), + } } /// Processes a binary operation and converts the result into an `AcirVar` @@ -354,20 +373,20 @@ impl Context { binary: &Binary, dfg: &DataFlowGraph, ) -> Result { - let lhs = self.convert_ssa_value(binary.lhs, dfg); - let rhs = self.convert_ssa_value(binary.rhs, dfg); + let lhs = self.convert_numeric_value(binary.lhs, dfg); + let rhs = self.convert_numeric_value(binary.rhs, dfg); let binary_type = self.type_of_binary_operation(binary, dfg); - match binary_type { + match &binary_type { Type::Numeric(NumericType::Unsigned { bit_size }) | Type::Numeric(NumericType::Signed { bit_size }) => { // Conservative max bit size that is small enough such that two operands can be // multiplied and still fit within the field modulus. This is necessary for the // truncation technique: result % 2^bit_size to be valid. let max_integer_bit_size = FieldElement::max_num_bits() / 2; - if bit_size > max_integer_bit_size { + if *bit_size > max_integer_bit_size { return Err(AcirGenError::UnsupportedIntegerSize { - num_bits: bit_size, + num_bits: *bit_size, max_num_bits: max_integer_bit_size, }); } @@ -375,21 +394,24 @@ impl Context { _ => {} } + let binary_type = AcirType::from(binary_type); + let bit_count = binary_type.bit_size(); + 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::Div => self.acir_context.div_var(lhs, rhs, binary_type.into()), + BinaryOp::Div => self.acir_context.div_var(lhs, rhs, binary_type), // Note: that this produces unnecessary constraints when // this Eq instruction is being used for a constrain statement BinaryOp::Eq => self.acir_context.eq_var(lhs, rhs), - BinaryOp::Lt => self.acir_context.less_than_var(lhs, rhs), - BinaryOp::Shl => self.acir_context.shift_left_var(lhs, rhs, binary_type.into()), - BinaryOp::Shr => self.acir_context.shift_right_var(lhs, rhs, binary_type.into()), - BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs), - BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type.into()), - BinaryOp::Or => self.acir_context.or_var(lhs, rhs), - BinaryOp::Mod => self.acir_context.modulo_var(lhs, rhs), + BinaryOp::Lt => self.acir_context.less_than_var(lhs, rhs, bit_count), + BinaryOp::Shl => self.acir_context.shift_left_var(lhs, rhs, binary_type), + BinaryOp::Shr => self.acir_context.shift_right_var(lhs, rhs, binary_type), + BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, binary_type), + BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type), + BinaryOp::Or => self.acir_context.or_var(lhs, rhs, binary_type), + BinaryOp::Mod => self.acir_context.modulo_var(lhs, rhs, bit_count), } } @@ -413,13 +435,14 @@ impl Context { match (lhs_type, rhs_type) { // Function type should not be possible, since all functions // have been inlined. - (Type::Function, _) | (_, Type::Function) => { + (_, Type::Function) | (Type::Function, _) => { unreachable!("all functions should be inlined") } (_, Type::Reference) | (Type::Reference, _) => { - unreachable!( - "operations involving references are handled separately in `convert_ssa_instruction`." - ) + unreachable!("References are invalid in binary operations") + } + (_, Type::Array(..)) | (Type::Array(..), _) => { + unreachable!("Arrays are invalid in binary operations") } // Unit type currently can mean a 0 constant, so we return the // other type. @@ -442,7 +465,7 @@ impl Context { /// Returns an `AcirVar` that is constrained to be fn convert_ssa_cast(&mut self, value_id: &ValueId, typ: &Type, dfg: &DataFlowGraph) -> AcirVar { - let variable = self.convert_ssa_value(*value_id, dfg); + let variable = self.convert_numeric_value(*value_id, dfg); match typ { Type::Numeric(numeric_type) => self @@ -462,28 +485,40 @@ impl Context { arguments: &[ValueId], dfg: &DataFlowGraph, allow_log_ops: bool, - ) -> Vec { - let inputs = self - .flatten_value_list(arguments, dfg) - .expect("add Result types to all methods so errors bubble up"); + result_ids: &[ValueId], + ) -> Vec { match intrinsic { - Intrinsic::BlackBox(black_box) => self - .acir_context - .black_box_function(black_box, inputs) - .expect("add Result types to all methods so errors bubble up"), + Intrinsic::BlackBox(black_box) => { + let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); + + let vars = self + .acir_context + .black_box_function(black_box, inputs) + .expect("add Result types to all methods so errors bubble up"); + + Self::convert_vars_to_values(vars, dfg, result_ids) + } Intrinsic::ToRadix(endian) => { - // inputs = [field, radix, limb_size]; (see noir_stdlib/src/field.nr) + let field = self.convert_value(arguments[0], dfg).into_var(); + let radix = self.convert_value(arguments[1], dfg).into_var(); + let limb_size = self.convert_value(arguments[2], dfg).into_var(); + let result_type = Self::array_element_type(dfg, result_ids[0]); + self.acir_context - .radix_decompose(endian, inputs[0], inputs[1], inputs[2]) + .radix_decompose(endian, field, radix, limb_size, result_type) .expect("add Result types to all methods so errors bubble up") } Intrinsic::ToBits(endian) => { - // inputs = [field, bit_size]; (see noir_stdlib/src/field.nr) + let field = self.convert_value(arguments[0], dfg).into_var(); + let bit_size = self.convert_value(arguments[1], dfg).into_var(); + let result_type = Self::array_element_type(dfg, result_ids[0]); + self.acir_context - .bit_decompose(endian, inputs[0], inputs[1]) + .bit_decompose(endian, field, bit_size, result_type) .expect("add Result types to all methods so errors bubble up") } Intrinsic::Println => { + let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); if allow_log_ops { self.acir_context .print(inputs) @@ -495,81 +530,85 @@ impl Context { } } + /// Given an array value, return the numerical type of its element. + /// Panics if the given value is not an array or has a non-numeric element type. + fn array_element_type(dfg: &DataFlowGraph, value: ValueId) -> AcirType { + match dfg.type_of_value(value) { + Type::Array(elements, _) => { + assert_eq!(elements.len(), 1); + (&elements[0]).into() + } + _ => unreachable!("Expected array type"), + } + } + /// Maps an ssa value list, for which some values may be references to arrays, by inlining /// the `AcirVar`s corresponding to the contents of each array into the list of `AcirVar`s /// that correspond to other values. - fn flatten_value_list( - &mut self, - arguments: &[ValueId], - dfg: &DataFlowGraph, - ) -> Result, AcirGenError> { - let mut acir_vars = Vec::new(); + fn flatten_value_list(&mut self, arguments: &[ValueId], dfg: &DataFlowGraph) -> Vec { + let mut acir_vars = Vec::with_capacity(arguments.len()); for value_id in arguments { - if Self::value_is_array_address(*value_id, dfg) { - let (array_id, index) = self - .ssa_value_to_array_address - .get(value_id) - .expect("ICE: Call argument of undeclared array"); - assert_eq!(index, &0, "ICE: Call arguments only accept arrays in their entirety"); - let elements = self.acir_context.array_load_all(*array_id)?; - acir_vars.extend(elements); - } else { - acir_vars.push(self.convert_ssa_value(*value_id, dfg)); - } + let value = self.convert_value(*value_id, dfg); + AcirContext::flatten_value(&mut acir_vars, value); } - Ok(acir_vars) - } - - /// Stores the `AcirVar` corresponding to `value` at the `ArrayId` and index corresponding to - /// `address`. - fn convert_ssa_store(&mut self, address: &ValueId, value: &ValueId, dfg: &DataFlowGraph) { - let element_var = self.convert_ssa_value(*value, dfg); - let (array_id, index) = - self.ssa_value_to_array_address.get(address).expect("ICE: Load from undeclared array"); - self.acir_context.array_store(*array_id, *index, element_var).expect("invalid array load"); + acir_vars } - /// Returns the `AcirVar` that was previously stored at the given address. - fn convert_ssa_load(&mut self, address: &ValueId) -> AcirVar { - let (array_id, index) = - self.ssa_value_to_array_address.get(address).expect("ICE: Load from undeclared array"); - self.acir_context.array_load(*array_id, *index).expect("invalid array load") + fn bit_count(&self, lhs: ValueId, dfg: &DataFlowGraph) -> u32 { + match dfg.type_of_value(lhs) { + Type::Numeric(NumericType::Signed { bit_size }) => bit_size, + Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size, + Type::Numeric(NumericType::NativeField) => FieldElement::max_num_bits(), + _ => 0, + } } - /// Returns true if the value has been declared as an array address - fn value_is_array_address(value_id: ValueId, dfg: &DataFlowGraph) -> bool { - dfg.type_of_value(value_id) == Type::Reference + /// Convert a Vec into a Vec using the given result ids. + /// If the type of a result id is an array, several acirvars are collected into + /// a single AcirValue::Array of the same length. + fn convert_vars_to_values( + vars: Vec, + dfg: &DataFlowGraph, + result_ids: &[ValueId], + ) -> Vec { + let mut vars = vars.into_iter(); + vecmap(result_ids, |result| { + let result_type = dfg.type_of_value(*result); + Self::convert_var_type_to_values(&result_type, &mut vars) + }) } - /// Takes a binary instruction describing array address arithmetic and stores the result. - fn track_array_address(&mut self, value_id: ValueId, binary: &Binary, dfg: &DataFlowGraph) { - if binary.operator != BinaryOp::Add { - unreachable!("ICE: Array address arithmetic only supports Add"); - } - let lhs_address = self.ssa_value_to_array_address.get(&binary.lhs); - let rhs_address = self.ssa_value_to_array_address.get(&binary.rhs); - let ((array_id, offset), other_value_id) = match (lhs_address, rhs_address) { - (Some(address), None) => (address, binary.rhs), - (None, Some(address)) => (address, binary.lhs), - (Some(_), Some(_)) => unreachable!("ICE: Addresses cannot be added"), - (None, None) => unreachable!("ICE: One operand must be an address"), - }; - let other_value = &dfg[other_value_id]; - let new_offset = match other_value { - Value::NumericConstant { constant, .. } => { - let further_offset = - constant.try_to_u64().expect("ICE: array arithmetic doesn't fit in u64") - as usize; - offset + further_offset + /// Recursive helper for convert_vars_to_values. + /// If the given result_type is an array of length N, this will create an AcirValue::Array with + /// the first N elements of the given iterator. Otherwise, the result is a single + /// AcirValue::Var wrapping the first element of the iterator. + fn convert_var_type_to_values( + result_type: &Type, + vars: &mut impl Iterator, + ) -> AcirValue { + match result_type { + Type::Array(elements, size) => { + let mut element_values = im::Vector::new(); + for _ in 0..*size { + for element_type in elements.iter() { + let element = Self::convert_var_type_to_values(element_type, vars); + element_values.push_back(element); + } + } + AcirValue::Array(element_values) } - _ => unreachable!("Invalid array address arithmetic operand"), - }; - self.ssa_value_to_array_address.insert(value_id, (*array_id, new_offset)); + typ => { + let var = vars.next().unwrap(); + AcirValue::Var(var, typ.into()) + } + } } } #[cfg(test)] mod tests { + use std::rc::Rc; + use acvm::{ acir::{ circuit::Opcode, @@ -581,7 +620,7 @@ mod tests { use crate::{ brillig::Brillig, ssa_refactor::{ - ir::{function::RuntimeType, map::Id}, + ir::{function::RuntimeType, map::Id, types::Type}, ssa_builder::FunctionBuilder, }, }; @@ -592,20 +631,22 @@ mod tests { fn returns_body_scoped_arrays() { // fn main { // b0(): - // v0 = alloc 1 - // store v0, Field 1 - // return v0 + // return [Field 1] // } let func_id = Id::test_new(0); let mut builder = FunctionBuilder::new("func".into(), func_id, RuntimeType::Acir); - let v0 = builder.insert_allocate(1); - let const_one = builder.field_constant(FieldElement::one()); - builder.insert_store(v0, const_one); - builder.terminate_with_return(vec![v0]); + + let one = builder.field_constant(FieldElement::one()); + + let element_type = Rc::new(vec![Type::field()]); + let array = builder.array_constant(im::Vector::unit(one), element_type); + + builder.terminate_with_return(vec![array]); + let ssa = builder.finish(); let context = Context::default(); - let acir = context.convert_ssa(ssa, &[], Brillig::default(), false); + let acir = context.convert_ssa(ssa, Brillig::default(), false); let expected_opcodes = vec![Opcode::Arithmetic(&Expression::one() - &Expression::from(Witness(1)))]; diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 47872b7468a..ad4f8f3ad4e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::{collections::HashMap, rc::Rc}; use crate::ssa_refactor::ir::instruction::SimplifyResult; @@ -8,8 +8,8 @@ use super::{ instruction::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, }, - map::{DenseMap, Id}, - types::Type, + map::DenseMap, + types::{CompositeType, Type}, value::{Value, ValueId}, }; @@ -157,21 +157,30 @@ impl DataFlowGraph { /// Set the value of value_to_replace to refer to the value referred to by new_value. pub(crate) fn set_value_from_id(&mut self, value_to_replace: ValueId, new_value: ValueId) { - let new_value = self.values[new_value]; + let new_value = self.values[new_value].clone(); self.values[value_to_replace] = new_value; } /// Creates a new constant value, or returns the Id to an existing one if /// one already exists. pub(crate) fn make_constant(&mut self, constant: FieldElement, typ: Type) -> ValueId { - if let Some(id) = self.constants.get(&(constant, typ)) { + if let Some(id) = self.constants.get(&(constant, typ.clone())) { return *id; } - let id = self.values.insert(Value::NumericConstant { constant, typ }); + let id = self.values.insert(Value::NumericConstant { constant, typ: typ.clone() }); self.constants.insert((constant, typ), id); id } + /// Create a new constant array value from the given elements + pub(crate) fn make_array( + &mut self, + array: im::Vector, + element_type: Rc, + ) -> ValueId { + self.make_value(Value::Array { array, element_type }) + } + /// Gets or creates a ValueId for the given FunctionId. pub(crate) fn import_function(&mut self, function: FunctionId) -> ValueId { if let Some(existing) = self.functions.get(&function) { @@ -265,7 +274,7 @@ impl DataFlowGraph { } /// Add a parameter to the given block - pub(crate) fn add_block_parameter(&mut self, block_id: BasicBlockId, typ: Type) -> Id { + pub(crate) fn add_block_parameter(&mut self, block_id: BasicBlockId, typ: Type) -> ValueId { let block = &mut self.blocks[block_id]; let position = block.parameters().len(); let parameter = self.values.insert(Value::Param { block: block_id, position, typ }); @@ -275,7 +284,7 @@ impl DataFlowGraph { /// Returns the field element represented by this value if it is a numeric constant. /// Returns None if the given value is not a numeric constant. - pub(crate) fn get_numeric_constant(&self, value: Id) -> Option { + pub(crate) fn get_numeric_constant(&self, value: ValueId) -> Option { self.get_numeric_constant_with_type(value).map(|(value, _typ)| value) } @@ -283,10 +292,23 @@ impl DataFlowGraph { /// Returns None if the given value is not a numeric constant. pub(crate) fn get_numeric_constant_with_type( &self, - value: Id, + value: ValueId, ) -> Option<(FieldElement, Type)> { - match self.values[value] { - Value::NumericConstant { constant, typ } => Some((constant, typ)), + match &self.values[value] { + Value::NumericConstant { constant, typ } => Some((*constant, typ.clone())), + _ => None, + } + } + + /// Returns the Value::Array associated with this ValueId if it refers to an array constant. + /// Otherwise, this returns None. + pub(crate) fn get_array_constant( + &self, + value: ValueId, + ) -> Option<(im::Vector, Rc)> { + match &self.values[value] { + // Vectors are shared, so cloning them is cheap + Value::Array { array, element_type } => Some((array.clone(), element_type.clone())), _ => None, } } @@ -397,7 +419,7 @@ mod tests { #[test] fn make_instruction() { let mut dfg = DataFlowGraph::default(); - let ins = Instruction::Allocate { size: 20 }; + let ins = Instruction::Allocate; let ins_id = dfg.make_instruction(ins, None); let results = dfg.instruction_results(ins_id); diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 98e277c0fa2..a9767bc3777 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -94,17 +94,21 @@ pub(crate) enum Instruction { /// Allocates a region of memory. Note that this is not concerned with /// the type of memory, the type of element is determined when loading this memory. - /// - /// `size` is the size of the region to be allocated by the number of FieldElements it - /// contains. Note that non-numeric types like Functions and References are counted as 1 field - /// each. - Allocate { size: u32 }, + /// This is used for representing mutable variables and references. + Allocate, /// Loads a value from memory. Load { address: ValueId }, /// Writes a value to memory. Store { address: ValueId, value: ValueId }, + + /// Retrieve a value from an array at the given index + ArrayGet { array: ValueId, index: ValueId }, + + /// Creates a new array with the new value at the given index. All other elements are identical + /// to those in the given array. This will not modify the original array. + ArraySet { array: ValueId, index: ValueId, value: ValueId }, } impl Instruction { @@ -117,13 +121,16 @@ impl Instruction { pub(crate) fn result_type(&self) -> InstructionResultType { match self { Instruction::Binary(binary) => binary.result_type(), - Instruction::Cast(_, typ) => InstructionResultType::Known(*typ), + Instruction::Cast(_, typ) => InstructionResultType::Known(typ.clone()), Instruction::Allocate { .. } => InstructionResultType::Known(Type::Reference), Instruction::Not(value) | Instruction::Truncate { value, .. } => { InstructionResultType::Operand(*value) } + Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array), Instruction::Constrain(_) | Instruction::Store { .. } => InstructionResultType::None, - Instruction::Load { .. } | Instruction::Call { .. } => InstructionResultType::Unknown, + Instruction::Load { .. } | Instruction::ArrayGet { .. } | Instruction::Call { .. } => { + InstructionResultType::Unknown + } } } @@ -143,7 +150,7 @@ impl Instruction { rhs: f(binary.rhs), operator: binary.operator, }), - Instruction::Cast(value, typ) => Instruction::Cast(f(*value), *typ), + Instruction::Cast(value, typ) => Instruction::Cast(f(*value), typ.clone()), Instruction::Not(value) => Instruction::Not(f(*value)), Instruction::Truncate { value, bit_size, max_bit_size } => Instruction::Truncate { value: f(*value), @@ -155,11 +162,17 @@ impl Instruction { func: f(*func), arguments: vecmap(arguments.iter().copied(), f), }, - Instruction::Allocate { size } => Instruction::Allocate { size: *size }, + Instruction::Allocate => Instruction::Allocate, Instruction::Load { address } => Instruction::Load { address: f(*address) }, Instruction::Store { address, value } => { Instruction::Store { address: f(*address), value: f(*value) } } + Instruction::ArrayGet { array, index } => { + Instruction::ArrayGet { array: f(*array), index: f(*index) } + } + Instruction::ArraySet { array, index, value } => { + Instruction::ArraySet { array: f(*array), index: f(*index), value: f(*value) } + } } } @@ -170,9 +183,10 @@ impl Instruction { match self { Instruction::Binary(binary) => binary.simplify(dfg), Instruction::Cast(value, typ) => { - match (*typ == dfg.type_of_value(*value)).then_some(*value) { - Some(value) => SimplifiedTo(value), - _ => None, + if let Some(value) = (*typ == dfg.type_of_value(*value)).then_some(*value) { + SimplifiedTo(value) + } else { + None } } Instruction::Not(value) => { @@ -186,9 +200,10 @@ impl Instruction { } Value::Instruction { instruction, .. } => { // !!v => v - match &dfg[*instruction] { - Instruction::Not(value) => SimplifiedTo(*value), - _ => None, + if let Instruction::Not(value) = &dfg[*instruction] { + SimplifiedTo(*value) + } else { + None } } _ => None, @@ -202,6 +217,32 @@ impl Instruction { } None } + Instruction::ArrayGet { array, index } => { + let array = dfg.get_array_constant(*array); + let index = dfg.get_numeric_constant(*index); + + if let (Some((array, _)), Some(index)) = (array, index) { + let index = + index.try_to_u64().expect("Expected array index to fit in u64") as usize; + assert!(index < array.len()); + SimplifiedTo(array[index]) + } else { + None + } + } + Instruction::ArraySet { array, index, value } => { + let array = dfg.get_array_constant(*array); + let index = dfg.get_numeric_constant(*index); + + if let (Some((array, element_type)), Some(index)) = (array, index) { + let index = + index.try_to_u64().expect("Expected array index to fit in u64") as usize; + assert!(index < array.len()); + SimplifiedTo(dfg.make_array(array.update(index, *value), element_type)) + } else { + None + } + } Instruction::Truncate { .. } => None, Instruction::Call { .. } => None, Instruction::Allocate { .. } => None, diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs index 7a4651056d1..2829c6768b8 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs @@ -67,7 +67,11 @@ fn value(function: &Function, id: ValueId) -> String { } Value::Function(id) => id.to_string(), Value::Intrinsic(intrinsic) => intrinsic.to_string(), - _ => id.to_string(), + Value::Array { array, .. } => { + let elements = vecmap(array, |element| value(function, *element)); + format!("[{}]", elements.join(", ")) + } + Value::Param { .. } | Value::Instruction { .. } => id.to_string(), } } @@ -144,10 +148,22 @@ pub(crate) fn display_instruction( Instruction::Call { func, arguments } => { writeln!(f, "call {}({})", show(*func), value_list(function, arguments)) } - Instruction::Allocate { size } => writeln!(f, "alloc {size} fields"), + Instruction::Allocate => writeln!(f, "allocate"), Instruction::Load { address } => writeln!(f, "load {}", show(*address)), Instruction::Store { address, value } => { writeln!(f, "store {} at {}", show(*value), show(*address)) } + Instruction::ArrayGet { array, index } => { + writeln!(f, "array_get {}, index {}", show(*array), show(*index)) + } + Instruction::ArraySet { array, index, value } => { + writeln!( + f, + "array_set {}, index {}, value {}", + show(*array), + show(*index), + show(*value) + ) + } } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs index e00c25a257c..d4a3946375a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs @@ -1,3 +1,7 @@ +use std::rc::Rc; + +use iter_extended::vecmap; + /// A numeric type in the Intermediate representation /// Note: we class NativeField as a numeric type /// though we also apply limitations to it, such as not @@ -14,7 +18,7 @@ pub(crate) enum NumericType { } /// All types representable in the IR. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub(crate) enum Type { /// Represents numeric types in the IR, including field elements Numeric(NumericType), @@ -22,6 +26,9 @@ pub(crate) enum Type { /// A reference to some value, such as an array Reference, + /// An immutable array value with the given element type and length + Array(Rc, usize), + /// A function that may be called directly Function, @@ -45,17 +52,31 @@ impl Type { Type::unsigned(1) } + /// Creates the char type, represented as u8. + pub(crate) fn char() -> Type { + Type::unsigned(8) + } + /// Creates the native field type. pub(crate) fn field() -> Type { Type::Numeric(NumericType::NativeField) } } +/// Composite Types are essentially flattened struct or tuple types. +/// Array types may have these as elements where each flattened field is +/// included in the array sequentially. +pub(crate) type CompositeType = Vec; + impl std::fmt::Display for Type { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Type::Numeric(numeric) => numeric.fmt(f), Type::Reference => write!(f, "reference"), + Type::Array(element, length) => { + let elements = vecmap(element.iter(), |element| element.to_string()); + write!(f, "[{}; {length}]", elements.join(", ")) + } Type::Function => write!(f, "function"), Type::Unit => write!(f, "unit"), } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs index a77916ee841..30468a0a669 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/value.rs @@ -1,3 +1,5 @@ +use std::rc::Rc; + use acvm::FieldElement; use crate::ssa_refactor::ir::basic_block::BasicBlockId; @@ -6,14 +8,14 @@ use super::{ function::FunctionId, instruction::{InstructionId, Intrinsic}, map::Id, - types::Type, + types::{CompositeType, Type}, }; pub(crate) type ValueId = Id; /// Value is the most basic type allowed in the IR. /// Transition Note: A Id is similar to `NodeId` in our previous IR. -#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone)] +#[derive(Debug, PartialEq, Eq, Hash, Clone)] pub(crate) enum Value { /// This value was created due to an instruction /// @@ -35,6 +37,9 @@ pub(crate) enum Value { /// This Value originates from a numeric constant NumericConstant { constant: FieldElement, typ: Type }, + /// Represents a constant array value + Array { array: im::Vector, element_type: Rc }, + /// This Value refers to a function in the IR. /// Functions always have the type Type::Function. /// If the argument or return types are needed, users should retrieve @@ -51,9 +56,10 @@ impl Value { /// Retrieves the type of this Value pub(crate) fn get_type(&self) -> Type { match self { - Value::Instruction { typ, .. } => *typ, - Value::Param { typ, .. } => *typ, - Value::NumericConstant { typ, .. } => *typ, + Value::Instruction { typ, .. } => typ.clone(), + Value::Param { typ, .. } => typ.clone(), + Value::NumericConstant { typ, .. } => typ.clone(), + Value::Array { element_type, array } => Type::Array(element_type.clone(), array.len()), Value::Function { .. } => Type::Function, Value::Intrinsic { .. } => Type::Function, } diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 05ff22808f6..51ce5601e12 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -947,7 +947,7 @@ mod test { let c1 = builder.add_parameter(Type::bool()); let c4 = builder.add_parameter(Type::bool()); - let r1 = builder.insert_allocate(1); + let r1 = builder.insert_allocate(); let store_value = |builder: &mut FunctionBuilder, value: u128| { let value = builder.field_constant(value); @@ -957,9 +957,8 @@ mod test { let println = builder.import_intrinsic_id(Intrinsic::Println); let call_println = |builder: &mut FunctionBuilder, block: u128| { - let zero = builder.field_constant(0u128); let block = builder.field_constant(block); - let load = builder.insert_load(r1, zero, Type::field()); + let load = builder.insert_load(r1, Type::field()); builder.insert_call(println, vec![block, load], Vec::new()); }; @@ -1003,14 +1002,11 @@ mod test { builder.terminate_with_jmp(b9, vec![]); switch_and_print(&mut builder, b9, 9); - let zero = builder.field_constant(0u128); - let load = builder.insert_load(r1, zero, Type::field()); + let load = builder.insert_load(r1, Type::field()); builder.terminate_with_return(vec![load]); let ssa = builder.finish().flatten_cfg().mem2reg(); - println!("{ssa}"); - // Expected results after mem2reg removes the allocation and each load and store: // // fn main f0 { diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 29466cba346..0346dcee166 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -203,10 +203,14 @@ impl<'function> PerFunctionContext<'function> { unreachable!("All Value::Params should already be known from previous calls to translate_block. Unknown value {id} = {value:?}") } Value::NumericConstant { constant, typ } => { - self.context.builder.numeric_constant(*constant, *typ) + self.context.builder.numeric_constant(*constant, typ.clone()) } Value::Function(function) => self.context.builder.import_function(*function), Value::Intrinsic(intrinsic) => self.context.builder.import_intrinsic_id(*intrinsic), + Value::Array { array, element_type } => { + let elements = array.iter().map(|value| self.translate_value(*value)).collect(); + self.context.builder.array_constant(elements, element_type.clone()) + } }; self.values.insert(id, new_value); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs index 6c0fa81d014..4166512f695 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs @@ -103,7 +103,7 @@ impl PerBlockContext { Instruction::Load { address } => { if let Some(address) = self.try_const_address(*address, dfg) { if let Some(last_value) = self.last_stores.get(&address) { - let last_value = dfg[*last_value]; + let last_value = dfg[*last_value].clone(); loads_to_substitute.push((*instruction_id, last_value)); } else { self.failed_substitutes.insert(address); @@ -138,7 +138,7 @@ impl PerBlockContext { .instruction_results(*instruction_id) .first() .expect("ICE: Load instructions should have single result"); - dfg.set_value(result_value, *new_value); + dfg.set_value(result_value, new_value.clone()); } // Delete load instructions @@ -227,14 +227,17 @@ impl PerBlockContext { #[cfg(test)] mod tests { + use std::rc::Rc; + use acvm::FieldElement; + use im::vector; use crate::ssa_refactor::{ ir::{ basic_block::BasicBlockId, dfg::DataFlowGraph, function::RuntimeType, - instruction::{BinaryOp, Instruction, Intrinsic, TerminatorInstruction}, + instruction::{Instruction, Intrinsic, TerminatorInstruction}, map::Id, types::Type, }, @@ -245,25 +248,30 @@ mod tests { fn test_simple() { // fn func() { // b0(): - // v0 = alloc 2 - // v1 = add v0, Field 1 - // store v1, Field 1 - // v2 = add v0, Field 1 - // v3 = load v1 - // return v3 + // v0 = allocate + // store [Field 1, Field 2] in v0 + // v1 = load v0 + // v2 = array_get v1, index 1 + // return v2 // } let func_id = Id::test_new(0); let mut builder = FunctionBuilder::new("func".into(), func_id, RuntimeType::Acir); - let v0 = builder.insert_allocate(2); - let const_one = builder.field_constant(FieldElement::one()); - let v1 = builder.insert_binary(v0, BinaryOp::Add, const_one); - builder.insert_store(v1, const_one); - // v2 is created internally by builder.insert_load - let v3 = builder.insert_load(v0, const_one, Type::field()); - builder.terminate_with_return(vec![v3]); + let v0 = builder.insert_allocate(); + let one = builder.field_constant(FieldElement::one()); + let two = builder.field_constant(FieldElement::one()); - let ssa = builder.finish().mem2reg(); + let element_type = Rc::new(vec![Type::field()]); + let array = builder.array_constant(vector![one, two], element_type.clone()); + + builder.insert_store(v0, array); + let v1 = builder.insert_load(v0, Type::Array(element_type, 2)); + let v2 = builder.insert_array_get(v1, one, Type::field()); + builder.terminate_with_return(vec![v2]); + + let ssa = builder.finish().mem2reg().fold_constants(); + + println!("{ssa}"); let func = ssa.main(); let block_id = func.entry_block(); @@ -275,33 +283,29 @@ mod tests { TerminatorInstruction::Return { return_values } => return_values.first().unwrap(), _ => unreachable!(), }; - assert_eq!(func.dfg[*ret_val_id], func.dfg[const_one]); + assert_eq!(func.dfg[*ret_val_id], func.dfg[two]); } #[test] fn test_simple_with_call() { // fn func { // b0(): - // v0 = alloc 2 - // v1 = add v0, Field 1 - // store v1, Field 1 - // v2 = add v0, Field 1 - // v3 = load v1 - // v4 = call f0, v0 - // return v3 + // v0 = allocate + // store v0, Field 1 + // v1 = load v0 + // v2 = call f0(v0) + // return v1 // } let func_id = Id::test_new(0); let mut builder = FunctionBuilder::new("func".into(), func_id, RuntimeType::Acir); - let v0 = builder.insert_allocate(2); - let const_one = builder.field_constant(FieldElement::one()); - let v1 = builder.insert_binary(v0, BinaryOp::Add, const_one); - builder.insert_store(v1, const_one); - // v2 is created internally by builder.insert_load - let v3 = builder.insert_load(v0, const_one, Type::field()); + let v0 = builder.insert_allocate(); + let one = builder.field_constant(FieldElement::one()); + builder.insert_store(v0, one); + let v1 = builder.insert_load(v0, Type::field()); let f0 = builder.import_intrinsic_id(Intrinsic::Println); builder.insert_call(f0, vec![v0], vec![Type::Unit]); - builder.terminate_with_return(vec![v3]); + builder.terminate_with_return(vec![v1]); let ssa = builder.finish().mem2reg(); @@ -315,21 +319,21 @@ mod tests { TerminatorInstruction::Return { return_values } => return_values.first().unwrap(), _ => unreachable!(), }; - assert_eq!(func.dfg[*ret_val_id], func.dfg[const_one]); + assert_eq!(func.dfg[*ret_val_id], func.dfg[one]); } #[test] fn test_simple_with_return() { // fn func { // b0(): - // v0 = alloc 1 + // v0 = allocate // store v0, Field 1 // return v0 // } let func_id = Id::test_new(0); let mut builder = FunctionBuilder::new("func".into(), func_id, RuntimeType::Acir); - let v0 = builder.insert_allocate(2); + let v0 = builder.insert_allocate(); let const_one = builder.field_constant(FieldElement::one()); builder.insert_store(v0, const_one); builder.terminate_with_return(vec![v0]); @@ -370,36 +374,35 @@ mod tests { fn multiple_blocks() { // fn main { // b0(): - // v0 = alloc 1 - // store v0, Field 5 + // v0 = allocate + // store Field 5 in v0 // v1 = load v0 // jmp b1(v1): // b1(v2: Field): // v3 = load v0 - // store v0, Field 6 + // store Field 6 in v0 // v4 = load v0 // return v2, v3, v4 // } let main_id = Id::test_new(0); let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Acir); - let v0 = builder.insert_allocate(1); + let v0 = builder.insert_allocate(); let five = builder.field_constant(5u128); builder.insert_store(v0, five); - let zero = builder.field_constant(0u128); - let v1 = builder.insert_load(v0, zero, Type::field()); + let v1 = builder.insert_load(v0, Type::field()); let b1 = builder.insert_block(); builder.terminate_with_jmp(b1, vec![v1]); builder.switch_to_block(b1); let v2 = builder.add_block_parameter(b1, Type::field()); - let v3 = builder.insert_load(v0, zero, Type::field()); + let v3 = builder.insert_load(v0, Type::field()); let six = builder.field_constant(6u128); builder.insert_store(v0, six); - let v4 = builder.insert_load(v0, zero, Type::field()); + let v4 = builder.insert_load(v0, Type::field()); builder.terminate_with_return(vec![v2, v3, v4]); @@ -409,7 +412,7 @@ mod tests { // Expected result: // fn main { // b0(): - // v0 = alloc 1 + // v0 = allocate // store v0, Field 5 // jmp b1(Field 5): // Optimized to constant 5 // b1(v2: Field): diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index ac022c9a9a7..902278e286a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -1,3 +1,5 @@ +use std::rc::Rc; + use acvm::FieldElement; use crate::ssa_refactor::ir::{ @@ -14,6 +16,7 @@ use super::{ dfg::InsertInstructionResult, function::RuntimeType, instruction::{InstructionId, Intrinsic}, + types::CompositeType, }, ssa_gen::Ssa, }; @@ -104,6 +107,15 @@ impl FunctionBuilder { self.numeric_constant(value.into(), Type::field()) } + /// Insert an array constant into the current function with the given element values. + pub(crate) fn array_constant( + &mut self, + elements: im::Vector, + element_types: Rc, + ) -> ValueId { + self.current_function.dfg.make_array(elements, element_types) + } + /// Returns the type of the given value. pub(crate) fn type_of_value(&self, value: ValueId) -> Type { self.current_function.dfg.type_of_value(value) @@ -154,8 +166,8 @@ impl FunctionBuilder { /// 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 { - self.insert_instruction(Instruction::Allocate { size: size_to_allocate }, None).first() + pub(crate) fn insert_allocate(&mut self) -> ValueId { + self.insert_instruction(Instruction::Allocate, None).first() } /// Insert a Load instruction at the end of the current block, loading from the given offset @@ -165,13 +177,7 @@ impl FunctionBuilder { /// 'offset' is in units of FieldElements here. So loading the fourth FieldElement stored in /// an array will have an offset of 3. /// Returns the element that was loaded. - pub(crate) fn insert_load( - &mut self, - mut address: ValueId, - offset: ValueId, - type_to_load: Type, - ) -> ValueId { - address = self.insert_binary(address, BinaryOp::Add, offset); + pub(crate) fn insert_load(&mut self, address: ValueId, type_to_load: Type) -> ValueId { self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load])).first() } @@ -222,6 +228,27 @@ impl FunctionBuilder { self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() } + /// Insert an instruction to extract an element from an array + pub(crate) fn insert_array_get( + &mut self, + array: ValueId, + index: ValueId, + element_type: Type, + ) -> ValueId { + let element_type = Some(vec![element_type]); + self.insert_instruction(Instruction::ArrayGet { array, index }, element_type).first() + } + + /// Insert an instruction to create a new array with the given index replaced with a new value + pub(crate) fn insert_array_set( + &mut self, + array: ValueId, + index: ValueId, + value: ValueId, + ) -> ValueId { + self.insert_instruction(Instruction::ArraySet { array, index, value }, None).first() + } + /// Terminates the current block with the given terminator instruction fn terminate_block_with(&mut self, terminator: TerminatorInstruction) { self.current_function.dfg.set_block_terminator(self.current_block, terminator); 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 602c5a79030..e20a54ba8cc 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::rc::Rc; use std::sync::{Mutex, RwLock}; use iter_extended::vecmap; @@ -146,7 +147,7 @@ impl<'a> FunctionContext<'a> { /// Allocate a single slot of memory and store into it the given initial value of the variable. /// Always returns a Value::Mutable wrapping the allocate instruction. pub(super) fn new_mutable_variable(&mut self, value_to_store: ValueId) -> Value { - let alloc = self.builder.insert_allocate(1); + let alloc = self.builder.insert_allocate(); self.builder.insert_store(alloc, value_to_store); let typ = self.builder.type_of_value(value_to_store); Value::Mutable(alloc, typ) @@ -186,7 +187,10 @@ impl<'a> FunctionContext<'a> { pub(super) fn convert_non_tuple_type(typ: &ast::Type) -> Type { match typ { ast::Type::Field => Type::field(), - ast::Type::Array(_, _) => Type::Reference, + ast::Type::Array(len, element) => { + let element_types = Self::convert_type(element).flatten(); + Type::Array(Rc::new(element_types), *len as usize) + } ast::Type::Integer(Signedness::Signed, bits) => Type::signed(*bits), ast::Type::Integer(Signedness::Unsigned, bits) => Type::unsigned(*bits), ast::Type::Bool => Type::unsigned(1), @@ -289,8 +293,147 @@ impl<'a> FunctionContext<'a> { } } - /// Mutate lhs to equal rhs - pub(crate) fn assign(&mut self, lhs: Values, rhs: Values) { + /// Extract the given field of the tuple by reference. Panics if the given Values is not + /// a Tree::Branch or does not have enough fields. + pub(super) fn get_field_ref(tuple: &Values, field_index: usize) -> &Values { + match tuple { + Tree::Branch(trees) => &trees[field_index], + Tree::Leaf(value) => { + unreachable!("Tried to extract tuple index {field_index} from non-tuple {value:?}") + } + } + } + + /// Replace the given field of the tuple with a new one. Panics if the given Values is not + /// a Tree::Branch or does not have enough fields. + pub(super) fn replace_field(tuple: Values, field_index: usize, new_value: Values) -> Values { + match tuple { + Tree::Branch(mut trees) => { + trees[field_index] = new_value; + Tree::Branch(trees) + } + Tree::Leaf(value) => { + unreachable!("Tried to extract tuple index {field_index} from non-tuple {value:?}") + } + } + } + + /// Retrieves the given function, adding it to the function queue + /// if it is not yet compiled. + pub(super) fn get_or_queue_function(&mut self, id: FuncId) -> Values { + let function = self.shared_context.get_or_queue_function(id); + self.builder.import_function(function).into() + } + + /// Extracts the current value out of an LValue. + /// + /// Goal: Handle the case of assigning to nested expressions such as `foo.bar[i1].baz[i2] = e` + /// while also noting that assigning to arrays will create a new array rather than mutate + /// the original. + /// + /// Method: First `extract_current_value` must recurse on the lvalue to extract the current + /// value contained: + /// + /// v0 = foo.bar ; allocate instruction for bar + /// v1 = load v0 ; loading the bar array + /// v2 = add i1, baz_index ; field offset for index i1, field baz + /// v3 = array_get v1, index v2 ; foo.bar[i1].baz + /// + /// Method (part 2): Then, `assign_new_value` will recurse in the opposite direction to + /// construct the larger value as needed until we can `store` to the nearest + /// allocation. + /// + /// v4 = array_set v3, index i2, e ; finally create a new array setting the desired value + /// v5 = array_set v1, index v2, v4 ; now must also create the new bar array + /// store v5 in v0 ; and store the result in the only mutable reference + /// + /// The returned `LValueRef` tracks the current value at each step of the lvalue. + /// This is later used by `assign_new_value` to construct a new updated value that + /// can be assigned to an allocation within the LValueRef::Ident. + /// + /// This is operationally equivalent to extract_current_value_recursive, but splitting these + /// into two separate functions avoids cloning the outermost `Values` returned by the recursive + /// version, as it is only needed for recursion. + pub(super) fn extract_current_value(&mut self, lvalue: &ast::LValue) -> LValue { + match lvalue { + ast::LValue::Ident(ident) => LValue::Ident(self.ident_lvalue(ident)), + ast::LValue::Index { array, index, .. } => self.index_lvalue(array, index).2, + ast::LValue::MemberAccess { object, field_index } => { + let (old_object, object_lvalue) = self.extract_current_value_recursive(object); + let object_lvalue = Box::new(object_lvalue); + LValue::MemberAccess { old_object, object_lvalue, index: *field_index } + } + } + } + + /// Compile the given identifier as a reference - ie. avoid calling .eval() + fn ident_lvalue(&self, ident: &ast::Ident) -> Values { + match &ident.definition { + ast::Definition::Local(id) => self.lookup(*id), + other => panic!("Unexpected definition found for mutable value: {other}"), + } + } + + /// Compile the given `array[index]` expression as a reference. + /// This will return a triple of (array, index, lvalue_ref) where the lvalue_ref records the + /// structure of the lvalue expression for use by `assign_new_value`. + fn index_lvalue( + &mut self, + array: &ast::LValue, + index: &ast::Expression, + ) -> (ValueId, ValueId, LValue) { + let (old_array, array_lvalue) = self.extract_current_value_recursive(array); + let old_array = old_array.into_leaf().eval(self); + let array_lvalue = Box::new(array_lvalue); + let index = self.codegen_non_tuple_expression(index); + (old_array, index, LValue::Index { old_array, index, array_lvalue }) + } + + fn extract_current_value_recursive(&mut self, lvalue: &ast::LValue) -> (Values, LValue) { + match lvalue { + ast::LValue::Ident(ident) => { + let variable = self.ident_lvalue(ident); + (variable.clone(), LValue::Ident(variable)) + } + ast::LValue::Index { array, index, element_type, location: _ } => { + let (old_array, index, index_lvalue) = self.index_lvalue(array, index); + let element = self.codegen_array_index(old_array, index, element_type); + (element, index_lvalue) + } + ast::LValue::MemberAccess { object, field_index: index } => { + let (old_object, object_lvalue) = self.extract_current_value_recursive(object); + let object_lvalue = Box::new(object_lvalue); + let element = Self::get_field_ref(&old_object, *index).clone(); + (element, LValue::MemberAccess { old_object, object_lvalue, index: *index }) + } + } + } + + /// Assigns a new value to the given LValue. + /// The LValue can be created via a previous call to extract_current_value. + /// This method recurs on the given LValue to create a new value to assign an allocation + /// instruction within an LValue::Ident - see the comment on `extract_current_value` for more + /// details. + /// When first-class references are supported the nearest reference may be in any LValue + /// variant rather than just LValue::Ident. + pub(super) fn assign_new_value(&mut self, lvalue: LValue, new_value: Values) { + match lvalue { + LValue::Ident(references) => self.assign(references, new_value), + LValue::Index { old_array, index, array_lvalue } => { + let rvalue = new_value.into_leaf().eval(self); // TODO + let new_array = self.builder.insert_array_set(old_array, index, rvalue); + self.assign_new_value(*array_lvalue, new_array.into()); + } + LValue::MemberAccess { old_object, index, object_lvalue } => { + let new_object = Self::replace_field(old_object, index, new_value); + self.assign_new_value(*object_lvalue, new_object); + } + } + } + + /// Given an lhs containing only references, create a store instruction to store each value of + /// rhs into its corresponding value in lhs. + fn assign(&mut self, lhs: Values, rhs: Values) { match (lhs, rhs) { (Tree::Branch(lhs_branches), Tree::Branch(rhs_branches)) => { assert_eq!(lhs_branches.len(), rhs_branches.len()); @@ -310,13 +453,6 @@ impl<'a> FunctionContext<'a> { } } } - - /// Retrieves the given function, adding it to the function queue - /// if it is not yet compiled. - pub(super) fn get_or_queue_function(&mut self, id: FuncId) -> Values { - let function = self.shared_context.get_or_queue_function(id); - self.builder.import_function(function).into() - } } /// True if the given operator cannot be encoded directly and needs @@ -399,3 +535,10 @@ impl SharedContext { next_id } } + +/// Used to remember the results of each step of extracting a value from an ast::LValue +pub(super) enum LValue { + Ident(Values), + Index { old_array: ValueId, index: ValueId, array_lvalue: Box }, + MemberAccess { old_object: Values, index: usize, object_lvalue: Box }, +} 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 cdf8a0e7100..386648c87f0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -2,6 +2,8 @@ mod context; mod program; mod value; +use std::rc::Rc; + pub(crate) use program::Ssa; use context::SharedContext; @@ -14,7 +16,12 @@ use self::{ value::{Tree, Values}, }; -use super::ir::{function::RuntimeType, instruction::BinaryOp, types::Type, value::ValueId}; +use super::ir::{ + function::RuntimeType, + instruction::BinaryOp, + types::{CompositeType, Type}, + value::ValueId, +}; /// Generates SSA for the given monomorphized program. /// @@ -102,8 +109,8 @@ impl<'a> FunctionContext<'a> { match literal { ast::Literal::Array(array) => { let elements = vecmap(&array.contents, |element| self.codegen_expression(element)); - let element_type = Self::convert_type(&array.element_type); - self.codegen_array(elements, element_type) + let element_types = Self::convert_type(&array.element_type).flatten(); + self.codegen_array(elements, element_types) } ast::Literal::Integer(value, typ) => { let typ = Self::convert_non_tuple_type(typ); @@ -116,7 +123,7 @@ impl<'a> FunctionContext<'a> { let elements = vecmap(string.as_bytes(), |byte| { self.builder.numeric_constant(*byte as u128, Type::field()).into() }); - self.codegen_array(elements, Tree::Leaf(Type::field())) + self.codegen_array(elements, vec![Type::char()]) } } } @@ -130,24 +137,17 @@ impl<'a> FunctionContext<'a> { /// stored the same as the array [1, 2, 3, 4]. /// /// The value returned from this function is always that of the allocate instruction. - fn codegen_array(&mut self, elements: Vec, element_type: Tree) -> Values { - let size = element_type.size_of_type() * elements.len(); - let array = self.builder.insert_allocate(size.try_into().unwrap_or_else(|_| { - panic!("Cannot allocate {size} bytes for array, it does not fit into a u32") - })); - - // Now we must manually store all the elements into the array - let mut i = 0u128; + fn codegen_array(&mut self, elements: Vec, element_types: CompositeType) -> Values { + let mut array = im::Vector::new(); + for element in elements { element.for_each(|element| { - let address = self.make_offset(array, i); let element = element.eval(self); - self.builder.insert_store(address, element); - i += 1; + array.push_back(element); }); } - array.into() + self.builder.array_constant(array, Rc::new(element_types)).into() } fn codegen_block(&mut self, block: &[Expression]) -> Values { @@ -178,7 +178,8 @@ impl<'a> FunctionContext<'a> { fn codegen_index(&mut self, index: &ast::Index) -> Values { let array = self.codegen_non_tuple_expression(&index.collection); - self.codegen_array_index(array, &index.index, &index.element_type, true) + let index_value = self.codegen_non_tuple_expression(&index.index); + self.codegen_array_index(array, index_value, &index.element_type) } /// This is broken off from codegen_index so that it can also be @@ -190,27 +191,19 @@ impl<'a> FunctionContext<'a> { fn codegen_array_index( &mut self, array: super::ir::value::ValueId, - index: &ast::Expression, + index: super::ir::value::ValueId, element_type: &ast::Type, - load_result: bool, ) -> Values { - let base_offset = self.codegen_non_tuple_expression(index); - - // base_index = base_offset * type_size + // base_index = index * type_size let type_size = Self::convert_type(element_type).size_of_type(); let type_size = self.builder.field_constant(type_size as u128); - let base_index = self.builder.insert_binary(base_offset, BinaryOp::Mul, type_size); + let base_index = self.builder.insert_binary(index, BinaryOp::Mul, type_size); let mut field_index = 0u128; Self::map_type(element_type, |typ| { let offset = self.make_offset(base_index, field_index); field_index += 1; - if load_result { - self.builder.insert_load(array, offset, typ) - } else { - self.builder.insert_binary(array, BinaryOp::Add, offset) - } - .into() + self.builder.insert_array_get(array, offset, typ).into() }) } @@ -362,7 +355,7 @@ impl<'a> FunctionContext<'a> { let mut values = self.codegen_expression(&let_expr.expression); if let_expr.mutable { - values.map_mut(|value| { + values = values.map(|value| { let value = value.eval(self); Tree::Leaf(self.new_mutable_variable(value)) }); @@ -379,38 +372,13 @@ impl<'a> FunctionContext<'a> { } fn codegen_assign(&mut self, assign: &ast::Assign) -> Values { - let lhs = self.codegen_lvalue(&assign.lvalue); + let lhs = self.extract_current_value(&assign.lvalue); let rhs = self.codegen_expression(&assign.expression); - self.assign(lhs, rhs); + self.assign_new_value(lhs, rhs); self.unit_value() } - fn codegen_lvalue(&mut self, lvalue: &ast::LValue) -> Values { - match lvalue { - ast::LValue::Ident(ident) => { - // Do not .eval the Values here! We do not want to load from any references within - // since we want to return the references instead - match &ident.definition { - ast::Definition::Local(id) => self.lookup(*id), - other => panic!("Unexpected definition found for mutable value: {other}"), - } - } - ast::LValue::Index { array, index, element_type, location: _ } => { - // Note that unlike the Ident case, we're .eval'ing the array here. - // This is because arrays are already references and thus a mutable reference - // to an array would be a Value::Mutable( Value::Mutable ( address ) ), and we - // only need the inner mutable value. - let array = self.codegen_lvalue(array).into_leaf().eval(self); - self.codegen_array_index(array, index, element_type, false) - } - ast::LValue::MemberAccess { object, field_index } => { - let object = self.codegen_lvalue(object); - Self::get_field(object, *field_index) - } - } - } - 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 02011adbaa8..0637641528e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs @@ -26,7 +26,7 @@ pub(super) enum Tree { /// used internally by functions in the ssa ir and should thus be isolated /// to a given function. If used outisde their function of origin, the IDs /// would be invalid. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] pub(super) enum Value { Normal(IrValueId), @@ -41,10 +41,7 @@ impl Value { pub(super) fn eval(self, ctx: &mut FunctionContext) -> IrValueId { match self { Value::Normal(value) => value, - Value::Mutable(address, typ) => { - let offset = ctx.builder.field_constant(0u128); - ctx.builder.insert_load(address, offset, typ) - } + Value::Mutable(address, typ) => ctx.builder.insert_load(address, typ), } }