From 4625979da71392d4b83b94a56e71468274cfd1d1 Mon Sep 17 00:00:00 2001 From: Joss Date: Tue, 30 May 2023 22:34:10 +0100 Subject: [PATCH 01/15] chore(ssa refactor): acir-gen arrays in main --- .../simple_array_param/Nargo.toml | 5 + .../simple_array_param/src/main.nr | 12 ++ .../src/ssa_refactor/abi_gen/mod.rs | 48 +++++- .../src/ssa_refactor/acir_gen/acir_ir.rs | 1 + .../acir_gen/acir_ir/acir_variable.rs | 29 +++- .../ssa_refactor/acir_gen/acir_ir/errors.rs | 6 + .../ssa_refactor/acir_gen/acir_ir/memory.rs | 145 ++++++++++++++++++ .../src/ssa_refactor/acir_gen/mod.rs | 144 +++++++++++++---- 8 files changed, 356 insertions(+), 34 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr create mode 100644 crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Nargo.toml new file mode 100644 index 00000000000..670888e37cd --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.6.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr new file mode 100644 index 00000000000..ec1a7372d14 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr @@ -0,0 +1,12 @@ +// This program tests: +// - the allocation of virtual arrays for array params to main +// - load instructions for such arrays + +struct Foo { + bar: Field, + baz: bool, +} + +fn main(xs : [Field; 2], foos: [Foo; 3]) -> pub Field { + xs[0] + foos[1].bar + foos[2].baz +} 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 5117cda3604..52acd2d0bb1 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use acvm::acir::native_types::Witness; use iter_extended::{btree_map, vecmap}; -use noirc_abi::{Abi, AbiParameter, FunctionSignature}; +use noirc_abi::{Abi, AbiParameter, AbiType, FunctionSignature}; /// Traverses the parameters to the program to infer the lengths of any arrays that occur. /// @@ -13,8 +13,50 @@ use noirc_abi::{Abi, AbiParameter, FunctionSignature}; /// 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_lengths(_abi_params: &[AbiParameter]) -> Vec { - Vec::new() +pub(crate) fn collate_array_lengths(abi_params: &[AbiParameter]) -> Vec { + let mut acc = Vec::new(); + for abi_param in abi_params { + collate_array_lengths_recursive(&mut acc, &abi_param.typ); + } + acc +} + +fn collate_array_lengths_recursive(acc: &mut Vec, abi_type: &AbiType) { + match abi_type { + AbiType::Array { length, typ: elem_type } => { + match elem_type.as_ref() { + AbiType::Array { .. } => { + unreachable!("2D arrays are not supported"); + } + AbiType::Struct { .. } => { + // monomorphization converts arrays of structs into an array per flattened + // struct field. + let array_count = elem_type.field_count(); + for _ in 0..array_count { + acc.push(*length as usize); + } + } + AbiType::String { .. } => { + unreachable!("Arrays od strings are not supported"); + } + AbiType::Boolean | AbiType::Field | AbiType::Integer { .. } => { + // Simple 1D array + acc.push(*length as usize); + } + } + } + AbiType::Struct { fields } => { + for (_, field_type) in fields { + collate_array_lengths_recursive(acc, field_type); + } + } + AbiType::String { length } => { + acc.push(*length as usize); + } + AbiType::Boolean | AbiType::Field | AbiType::Integer { .. } => { + // Do not produce arrays + } + } } /// Arranges a function signature and a generated circuit's return witnesses into a 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 ae183aa962f..7a755210e2b 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,3 +1,4 @@ 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 9ef420f6c35..29d3c142fb2 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,6 +1,10 @@ use crate::ssa_refactor::ir::types::NumericType; -use super::{errors::AcirGenError, generated_acir::GeneratedAcir}; +use super::{ + errors::AcirGenError, + generated_acir::GeneratedAcir, + memory::{ArrayId, Memory}, +}; use acvm::{ acir::native_types::{Expression, Witness}, FieldElement, @@ -34,6 +38,8 @@ pub(crate) struct AcirContext { /// Maps an `AcirVar` to its known bit size. variables_to_bit_sizes: HashMap, + /// Maps the elements of virtual arrays to their `AcirVar` elements + memory: Memory, } impl AcirContext { @@ -336,6 +342,27 @@ impl AcirContext { self.acir_ir } + pub(crate) fn allocate_array(&mut self, size: usize) -> ArrayId { + self.memory.allocate(size) + } + + pub(crate) fn array_store( + &mut self, + array_id: ArrayId, + index: usize, + element: AcirVar, + ) -> Result<(), AcirGenError> { + self.memory.constant_set(array_id, index, element) + } + + pub(crate) fn array_load( + &mut self, + array_id: ArrayId, + index: usize, + ) -> Result { + self.memory.constant_get(array_id, index) + } + /// Adds `Data` into the context and assigns it a Variable. /// /// Variable can be seen as an index into the context. 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 1373a54fe65..810208a6e83 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,7 +1,10 @@ +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 }, } impl AcirGenError { @@ -15,6 +18,9 @@ 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:?}") + } } } } 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 new file mode 100644 index 00000000000..aa4da4b6d3a --- /dev/null +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs @@ -0,0 +1,145 @@ +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] } + } + + pub(crate) fn with_default(size: usize, default: AcirVar) -> Array { + Self { elements: vec![Some(default); size] } + } +} + +#[derive(Debug, Default)] +/// Memory used to represent Arrays +pub(crate) struct Memory { + arrays: Vec, +} + +impl Memory { + pub(crate) fn new() -> Self { + Self { arrays: Vec::new() } + } + + /// Allocates an array of size `size`. + /// The elements in the array are not zero initialized + /// + /// TODO: Check if this method is needed, ie allocation without a default + pub(crate) fn allocate(&mut self, size: usize) -> ArrayId { + let array = Array::new(size); + self.add_array(array) + } + pub(crate) fn allocate_with_default( + &mut self, + size: usize, + default_element: AcirVar, + ) -> ArrayId { + let array = Array::with_default(size, default_element); + self.add_array(array) + } + + fn add_array(&mut self, array: Array) -> ArrayId { + let id = self.arrays.len(); + self.arrays.push(array); + ArrayId(id) + } + + fn mut_array(&mut self, array_id: ArrayId) -> &mut Array { + &mut self.arrays[array_id.0] + } + + /// 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> { + // Check if the index is larger than the array size + let array = self.mut_array(array_id); + let array_size = array.size(); + + if index >= array_size { + return Err(AcirGenError::IndexOutOfBounds { 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( + &mut self, + array_id: ArrayId, + index: usize, + ) -> Result { + // Check if the index is larger than the array size + let array = self.mut_array(array_id); + let array_size = array.size(); + + if index >= array_size { + return Err(AcirGenError::IndexOutOfBounds { index, array_size }); + } + + let element = array.elements[index]; + + match element { + Some(element) => Ok(element), + None => { + // The element was never initialized + Err(AcirGenError::UninitializedElementInArray { index, array_id }) + } + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct ArrayId(usize); + +#[cfg(test)] +mod tests { + use crate::ssa_refactor::acir_gen::acir_ir::{errors::AcirGenError, memory::Memory}; + + #[test] + fn smoke_api_get_uninitialized_element_ou() { + let mut memory = Memory::new(); + + 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::new(); + + 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 92720d30706..b3c0f1b833d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -2,7 +2,10 @@ use std::collections::HashMap; -use self::acir_ir::acir_variable::{AcirContext, AcirVar}; +use self::acir_ir::{ + acir_variable::{AcirContext, AcirVar}, + memory::ArrayId, +}; use super::{ abi_gen::collate_array_lengths, ir::{ @@ -31,21 +34,22 @@ struct Context { /// 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>, + ssa_value_to_array_address: HashMap, /// acir_context: AcirContext, } impl Ssa { pub(crate) fn into_acir(self, main_function_signature: FunctionSignature) -> GeneratedAcir { - let _param_array_lengths = collate_array_lengths(&main_function_signature.0); + let param_array_lengths = collate_array_lengths(&main_function_signature.0); let context = Context::default(); - context.convert_ssa(self) + context.convert_ssa(self, ¶m_array_lengths) } } impl Context { /// Converts SSA into ACIR - fn convert_ssa(mut self, ssa: Ssa) -> GeneratedAcir { + fn convert_ssa(mut self, ssa: Ssa, param_array_lengths: &[usize]) -> GeneratedAcir { assert_eq!( ssa.functions.len(), 1, @@ -55,9 +59,7 @@ impl Context { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; - for param_id in entry_block.parameters() { - self.convert_ssa_block_param(*param_id, dfg); - } + self.convert_ssa_block_params(entry_block.parameters(), dfg, param_array_lengths); for instruction_id in entry_block.instructions() { self.convert_ssa_instruction(*instruction_id, dfg); @@ -68,31 +70,59 @@ impl Context { self.acir_context.finish() } - /// Adds and binds an AcirVar for each numeric block parameter - fn convert_ssa_block_param(&mut self, param_id: ValueId, dfg: &DataFlowGraph) { - 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.acir_context.add_variable(); - if matches!(numeric_type, NumericType::Signed { .. } | NumericType::Unsigned { .. }) - { - self.acir_context - .numeric_cast_var(acir_var, &numeric_type) - .expect("invalid range constraint was applied {numeric_type}"); + /// 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_array_lengths: &[usize], + ) { + let mut param_array_lengths_iter = param_array_lengths.iter(); + 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.acir_context.add_variable(); + if matches!( + numeric_type, + NumericType::Signed { .. } | NumericType::Unsigned { .. } + ) { + self.acir_context + .numeric_cast_var(acir_var, &numeric_type) + .expect("invalid range constraint was applied {numeric_type}"); + } + self.ssa_value_to_acir_var.insert(*param_id, acir_var); + } + Type::Reference => { + let array_length = param_array_lengths_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.acir_context.add_variable(); + self.acir_context + .array_store(array_id, index, acir_var) + .expect("invalid array store"); + } + } + _ => { + unreachable!( + "ICE: Params to the program should only contains numerics and arrays" + ) } - self.ssa_value_to_acir_var.insert(param_id, acir_var); - } - Type::Reference => { - todo!("Create an abstract array holding an AcirVar for each element of the reference and bind its ArrayId to the reference."); - } - _ => { - unreachable!("ICE: Params to the program should only contains numerics and arrays") } } + assert_eq!( + param_array_lengths_iter.next(), + None, + "ICE: more arrays in abi than in block params" + ); } /// Converts an SSA instruction into its ACIR representation @@ -100,9 +130,15 @@ impl Context { let instruction = &dfg[instruction_id]; match instruction { Instruction::Binary(binary) => { - let result_acir_var = self.convert_ssa_binary(binary, dfg); let result_ids = dfg.instruction_results(instruction_id); assert_eq!(result_ids.len(), 1, "Binary ops have a single result"); + if self.value_is_array_address(binary.lhs) + || self.value_is_array_address(binary.rhs) + { + self.track_array_address(result_ids[0], binary, dfg); + return; + } + let result_acir_var = self.convert_ssa_binary(binary, dfg); self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); } Instruction::Constrain(value_id) => { @@ -115,6 +151,17 @@ impl Context { assert_eq!(result_ids.len(), 1, "Cast ops have a single result"); self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); } + Instruction::Load { address } => { + let (array_id, index) = self + .ssa_value_to_array_address + .get(address) + .expect("ICE: Load from undeclared array"); + let result_acir_var = + self.acir_context.array_load(*array_id, *index).expect("invalid array load"); + let result_ids = dfg.instruction_results(instruction_id); + assert_eq!(result_ids.len(), 1, "Load ops have a single result"); + self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); + } _ => todo!("{instruction:?}"), } } @@ -149,6 +196,10 @@ impl Context { /// parameters. This is because block parameters are converted before anything else, and /// because instructions results are converted when the corresponding instruction is /// encountered. (An instruction result cannot be referenced before the instruction occurs.) + /// + /// 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 { let value = &dfg[value_id]; if let Some(acir_var) = self.ssa_value_to_acir_var.get(&value_id) { @@ -200,4 +251,37 @@ impl Context { _ => unimplemented!("The cast operation is only valid for integers."), } } + + /// Returns true if the value has been declared as an array address + fn value_is_array_address(&self, value_id: ValueId) -> bool { + self.ssa_value_to_array_address.contains_key(&value_id) + } + + /// 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 constant = &dfg[*constant]; + let field_element = constant.value(); + let further_offset = + field_element.try_to_u64().expect("ICE: array arithmetic doesn't fit in u64") + as usize; + offset + further_offset + } + _ => unreachable!("Invalid array address arithmetic operand"), + }; + self.ssa_value_to_array_address.insert(value_id, (*array_id, new_offset)); + } } From 839179cd1342cfe0f8cf90971c793a5fd03f5178 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 30 May 2023 22:39:16 +0100 Subject: [PATCH 02/15] Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs --- .../noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index aa4da4b6d3a..5abaedfd15a 100644 --- 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 @@ -118,7 +118,7 @@ mod tests { use crate::ssa_refactor::acir_gen::acir_ir::{errors::AcirGenError, memory::Memory}; #[test] - fn smoke_api_get_uninitialized_element_ou() { + fn smoke_api_get_uninitialized_element_out() { let mut memory = Memory::new(); let array_size = 10; From 5053c525c69254260dd393c2694c1f840e0c1d36 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 07:49:59 +0100 Subject: [PATCH 03/15] chore(ssa refactor): PR suggestions --- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 1 + .../noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs | 1 + 2 files changed, 2 insertions(+) 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 29d3c142fb2..fa8a7e92807 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 @@ -342,6 +342,7 @@ impl AcirContext { 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) } 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 index 5abaedfd15a..9174c63e325 100644 --- 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 @@ -110,6 +110,7 @@ impl Memory { } } +/// Pointer to an allocated `Array` #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct ArrayId(usize); From 95b5fbbb961990ac4e2297d9fd159f51548baf01 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 08:21:46 +0100 Subject: [PATCH 04/15] chore(ssa refactor): simplify test into something I know the input syntax for :-/ --- .../simple_array_param/Prover.toml | 1 + .../simple_array_param/src/main.nr | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Prover.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Prover.toml new file mode 100644 index 00000000000..66f0b9ccc1c --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/Prover.toml @@ -0,0 +1 @@ +xs = [0, 1] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr index ec1a7372d14..8dc9b138496 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/simple_array_param/src/main.nr @@ -2,11 +2,6 @@ // - the allocation of virtual arrays for array params to main // - load instructions for such arrays -struct Foo { - bar: Field, - baz: bool, -} - -fn main(xs : [Field; 2], foos: [Foo; 3]) -> pub Field { - xs[0] + foos[1].bar + foos[2].baz +fn main(xs : [Field; 2]) -> pub Field { + xs[1] } From ef5439d556c1091c205fe970af4f4c27adf9d73e Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 08:23:31 +0100 Subject: [PATCH 05/15] chore(ssa refactor): rm todo (now has issue) --- .../noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs | 2 -- 1 file changed, 2 deletions(-) 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 index 9174c63e325..f81d498ff2c 100644 --- 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 @@ -37,8 +37,6 @@ impl Memory { /// Allocates an array of size `size`. /// The elements in the array are not zero initialized - /// - /// TODO: Check if this method is needed, ie allocation without a default pub(crate) fn allocate(&mut self, size: usize) -> ArrayId { let array = Array::new(size); self.add_array(array) From 502ab1a8b6c4e46efd0ad6c0f4b7ca93afb19a1e Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 08:26:02 +0100 Subject: [PATCH 06/15] chore(ssa refactor): rm unused --- .../src/ssa_refactor/acir_gen/acir_ir/memory.rs | 12 ------------ 1 file changed, 12 deletions(-) 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 index f81d498ff2c..bfe27fc97d3 100644 --- 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 @@ -18,10 +18,6 @@ impl Array { pub(crate) fn new(size: usize) -> Array { Self { elements: vec![None; size] } } - - pub(crate) fn with_default(size: usize, default: AcirVar) -> Array { - Self { elements: vec![Some(default); size] } - } } #[derive(Debug, Default)] @@ -41,14 +37,6 @@ impl Memory { let array = Array::new(size); self.add_array(array) } - pub(crate) fn allocate_with_default( - &mut self, - size: usize, - default_element: AcirVar, - ) -> ArrayId { - let array = Array::with_default(size, default_element); - self.add_array(array) - } fn add_array(&mut self, array: Array) -> ArrayId { let id = self.arrays.len(); From f3c8fed307d6eb1035963e1b9eeca560cbb5fffa Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 08:38:51 +0100 Subject: [PATCH 07/15] chore(ssa refactor): abi_gen comment --- crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 52acd2d0bb1..ab13fd4006e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/abi_gen/mod.rs @@ -21,6 +21,13 @@ pub(crate) fn collate_array_lengths(abi_params: &[AbiParameter]) -> Vec { acc } +/// The underlying recursive implementation of `collate_array_lengths` +/// +/// 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_lengths_recursive(acc: &mut Vec, abi_type: &AbiType) { match abi_type { AbiType::Array { length, typ: elem_type } => { @@ -37,7 +44,7 @@ fn collate_array_lengths_recursive(acc: &mut Vec, abi_type: &AbiType) { } } AbiType::String { .. } => { - unreachable!("Arrays od strings are not supported"); + unreachable!("Arrays of strings are not supported"); } AbiType::Boolean | AbiType::Field | AbiType::Integer { .. } => { // Simple 1D array From 9ce570587a344061c2b06d17d54bc50a6cfe6360 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 09:02:26 +0100 Subject: [PATCH 08/15] chore(ssa refactor): split convert_ssa_load --- .../src/ssa_refactor/acir_gen/mod.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 6bf8296ed95..3c5e0829634 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -155,12 +155,7 @@ impl Context { (vec![result_ids[0]], vec![result_acir_var]) } Instruction::Load { address } => { - let (array_id, index) = self - .ssa_value_to_array_address - .get(address) - .expect("ICE: Load from undeclared array"); - let result_acir_var = - self.acir_context.array_load(*array_id, *index).expect("invalid array load"); + let result_acir_var = self.convert_ssa_load(address); let result_ids = dfg.instruction_results(instruction_id); assert_eq!(result_ids.len(), 1, "Load ops have a single result"); (vec![result_ids[0]], vec![result_acir_var]) @@ -257,6 +252,13 @@ impl Context { } } + /// 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") + } + /// Returns true if the value has been declared as an array address fn value_is_array_address(&self, value_id: ValueId) -> bool { self.ssa_value_to_array_address.contains_key(&value_id) From 36a19ab54043038778c29e90d4d8420c771b91f1 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 09:09:53 +0100 Subject: [PATCH 09/15] chore(ssa refactor): more comments --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 3c5e0829634..71f16f24b4a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -34,8 +34,14 @@ struct Context { /// 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>, - ssa_value_to_array_address: HashMap, + /// 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, + /// Manages and builds the `AcirVar`s to which the converted SSA values refer. acir_context: AcirContext, } From 667adcf9107497109408dbd71648d84ab55c36d1 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 09:18:06 +0100 Subject: [PATCH 10/15] chore(ssa refactor): more comments --- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 4 ++++ 1 file changed, 4 insertions(+) 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 fa8a7e92807..5b0c0867cca 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 @@ -347,6 +347,7 @@ impl AcirContext { self.memory.allocate(size) } + /// Stores the given `AcirVar` at the specified address in memory pub(crate) fn array_store( &mut self, array_id: ArrayId, @@ -356,6 +357,9 @@ impl AcirContext { self.memory.constant_set(array_id, index, element) } + /// 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, From bbc467a81a379f3d155d6ab28fda17355c71e424 Mon Sep 17 00:00:00 2001 From: Joss Date: Wed, 31 May 2023 16:23:41 +0100 Subject: [PATCH 11/15] chore(ssa refactor): hookup alloc + store instructions --- .../src/ssa_refactor/acir_gen/mod.rs | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) 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 71f16f24b4a..d6ebbdcf08d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -143,11 +143,12 @@ impl Context { || self.value_is_array_address(binary.rhs) { self.track_array_address(result_ids[0], binary, dfg); - return; + (Vec::new(), Vec::new()) + } else { + let result_acir_var = self.convert_ssa_binary(binary, dfg); + self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); + (vec![result_ids[0]], vec![result_acir_var]) } - let result_acir_var = self.convert_ssa_binary(binary, dfg); - self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); - (vec![result_ids[0]], vec![result_acir_var]) } Instruction::Constrain(value_id) => { let constrain_condition = self.convert_ssa_value(*value_id, dfg); @@ -160,6 +161,17 @@ impl Context { assert_eq!(result_ids.len(), 1, "Cast ops have a single result"); (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); + assert_eq!(result_ids.len(), 1, "Allocate ops have a single result"); + 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); @@ -258,6 +270,15 @@ impl Context { } } + /// 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"); + } + /// Returns the `AcirVar` that was previously stored at the given address. fn convert_ssa_load(&mut self, address: &ValueId) -> AcirVar { let (array_id, index) = From 62e4f282e20fcb3d1ce99789661c6b5e17e29ede Mon Sep 17 00:00:00 2001 From: Joss Date: Thu, 1 Jun 2023 12:44:18 +0100 Subject: [PATCH 12/15] chore(ssa refactor): rm assert --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 - 1 file changed, 1 deletion(-) 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 b81c2aee023..7e0ef4e45e9 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -187,7 +187,6 @@ impl Context { Instruction::Allocate { size } => { let array_id = self.acir_context.allocate_array(*size as usize); let result_ids = dfg.instruction_results(instruction_id); - assert_eq!(result_ids.len(), 1, "Allocate ops have a single result"); self.ssa_value_to_array_address.insert(result_ids[0], (array_id, 0)); (Vec::new(), Vec::new()) } From f47a75f1a393ed250b639ec5694d6156d5d9f65a Mon Sep 17 00:00:00 2001 From: Joss Date: Thu, 1 Jun 2023 12:50:17 +0100 Subject: [PATCH 13/15] chore(ssa refactor): rm double insert --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 - 1 file changed, 1 deletion(-) 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 7e0ef4e45e9..009570f8997 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -187,7 +187,6 @@ impl Context { 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 } => { From 84831d9c9d4ded4bda4b4bde350983090aa59a93 Mon Sep 17 00:00:00 2001 From: Joss Date: Thu, 1 Jun 2023 12:52:27 +0100 Subject: [PATCH 14/15] Revert "chore(ssa refactor): rm double insert" This reverts commit f47a75f1a393ed250b639ec5694d6156d5d9f65a. --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 + 1 file changed, 1 insertion(+) 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 009570f8997..7e0ef4e45e9 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -187,6 +187,7 @@ impl Context { 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 } => { From 1317ad1cd9db0c92a960cd32cf5fe689cb5850ec Mon Sep 17 00:00:00 2001 From: Joss Date: Thu, 1 Jun 2023 12:54:00 +0100 Subject: [PATCH 15/15] chore(ssa refactor): rm double insert --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 - 1 file changed, 1 deletion(-) 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 7e0ef4e45e9..185745659d4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -146,7 +146,6 @@ impl Context { (Vec::new(), Vec::new()) } else { let result_acir_var = self.convert_ssa_binary(binary, dfg); - self.ssa_value_to_acir_var.insert(result_ids[0], result_acir_var); (vec![result_ids[0]], vec![result_acir_var]) } }