From e0454e7f713d32aef985c966a6d58c8a74c31450 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 26 Jul 2023 09:16:18 +0000 Subject: [PATCH 1/8] initialise arrays returned by brillig --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) 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 b0ade9419fe..11d6388b182 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -332,6 +332,11 @@ impl Context { assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA"); for result in result_ids.iter().zip(output_values) { + if let AcirValue::Array(values) = &result.1 { + let block_id = BlockId(dfg.resolve(*result.0).to_usize() as u32); + let values: Vec = values.iter().map(|i| i.clone()).collect(); + self.initialize_array(block_id, values.len(), Some(&values)); + } self.ssa_values.insert(*result.0, result.1); } } From 549d54d8641fbe7e061ad9f5911c5dfca686c415 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 26 Jul 2023 09:20:33 +0000 Subject: [PATCH 2/8] clippy --- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 5ef4707f9a9..b224c1f19aa 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 @@ -97,7 +97,7 @@ pub(crate) struct AcirContext { /// Two-way map that links `AcirVar` to `AcirVarData`. /// /// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`. - vars: HashMap, + pub vars: HashMap, /// An in-memory representation of ACIR. /// @@ -1025,7 +1025,7 @@ impl AcirContext { /// Enum representing the possible values that a /// Variable can be given. #[derive(Debug, Eq, Clone)] -enum AcirVarData { +pub enum AcirVarData { Witness(Witness), Expr(Expression), Const(FieldElement), From 37fb8aef6b94c26633e40cc2cf8b596469a5753e Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 26 Jul 2023 10:47:26 +0000 Subject: [PATCH 3/8] revert bad commit --- .../src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 b224c1f19aa..5ef4707f9a9 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 @@ -97,7 +97,7 @@ pub(crate) struct AcirContext { /// Two-way map that links `AcirVar` to `AcirVarData`. /// /// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`. - pub vars: HashMap, + vars: HashMap, /// An in-memory representation of ACIR. /// @@ -1025,7 +1025,7 @@ impl AcirContext { /// Enum representing the possible values that a /// Variable can be given. #[derive(Debug, Eq, Clone)] -pub enum AcirVarData { +enum AcirVarData { Witness(Witness), Expr(Expression), Const(FieldElement), From 48e65bab5cfb75158feaa615cecbb286525ca72c Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 26 Jul 2023 10:51:29 +0000 Subject: [PATCH 4/8] correct clippy --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 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 11d6388b182..f123634b33a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -334,7 +334,7 @@ impl Context { for result in result_ids.iter().zip(output_values) { if let AcirValue::Array(values) = &result.1 { let block_id = BlockId(dfg.resolve(*result.0).to_usize() as u32); - let values: Vec = values.iter().map(|i| i.clone()).collect(); + let values: Vec = values.iter().cloned().collect(); self.initialize_array(block_id, values.len(), Some(&values)); } self.ssa_values.insert(*result.0, result.1); From 74bcaf65d73c7373d3e35486f35cf6192b0f5a3a Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 26 Jul 2023 14:21:47 +0000 Subject: [PATCH 5/8] add unit test --- .../brillig_unitialised_arrays/Nargo.toml | 5 +++++ .../brillig_unitialised_arrays/Prover.toml | 2 ++ .../brillig_unitialised_arrays/src/main.nr | 13 +++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml new file mode 100644 index 00000000000..b6626a67e19 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml @@ -0,0 +1,2 @@ +x = 1 +y = 0 diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr new file mode 100644 index 00000000000..e0efbad1f42 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr @@ -0,0 +1,13 @@ + +fn main(x: Field, y: Field) -> pub Field { + let notes = create_notes(x, y); + sum_x(notes, x, y) +} + +fn sum_x(notes: [Field; 2], x: Field, y: Field) -> Field { + notes[x] + notes[y] +} + +unconstrained fn create_notes(x: Field, y: Field) -> [Field; 2] { + [x,y] +} \ No newline at end of file From a9718dd212b878d56d7404209123c25ec4d2f586 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 27 Jul 2023 10:28:34 +0000 Subject: [PATCH 6/8] dot not initialise nested arrays --- .../acir_gen/acir_ir/acir_variable.rs | 34 +++++++++++++++---- .../src/ssa_refactor/acir_gen/mod.rs | 5 +-- 2 files changed, 30 insertions(+), 9 deletions(-) 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 2fdfb0bd10f..8df4376c2af 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 @@ -1014,7 +1014,13 @@ impl AcirContext { block_id: BlockId, len: usize, optional_values: Option<&[AcirValue]>, - ) { + ) -> bool { + // Nested will be true if the array we want to initialise contains nested arrays + // in that case, we do not initialize the array + // Note that array initialization is only required for dynamic array accesses + // which does not support nested arrays for now. So not initializing the array + // is not an issue as long as the array is not used with witness indices + let mut nested = false; // If the optional values are supplied, then we fill the initialized // array with those values. If not, then we fill it with zeros. let initialized_values = match optional_values { @@ -1023,13 +1029,27 @@ impl AcirContext { let zero_witness = self.var_to_witness(zero); vec![zero_witness; len] } - Some(optional_values) => vecmap(optional_values, |value| { - let value = value.clone().into_var(); - self.var_to_witness(value) - }), + Some(optional_values) => { + let mut witness_values = Vec::new(); + for value in optional_values { + let value = match value { + AcirValue::Var(_, _) => value.clone().into_var(), + AcirValue::Array(_) | AcirValue::DynamicArray(_) => { + nested = true; + break; + } + }; + witness_values.push(self.var_to_witness(value)); + } + witness_values + } }; - - self.acir_ir.opcodes.push(Opcode::MemoryInit { block_id, init: initialized_values }); + if nested { + false + } else { + self.acir_ir.opcodes.push(Opcode::MemoryInit { block_id, init: initialized_values }); + true + } } } 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 f123634b33a..e3d7d5c3195 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -597,8 +597,9 @@ impl Context { /// Initializes an array with the given values and caches the fact that we /// have initialized this array. fn initialize_array(&mut self, array: BlockId, len: usize, values: Option<&[AcirValue]>) { - self.acir_context.initialize_array(array, len, values); - self.initialized_arrays.insert(array); + if self.acir_context.initialize_array(array, len, values) { + self.initialized_arrays.insert(array); + } } /// Remember the result of an instruction returning a single value From 10da101384d72ea759d82c5cc0c0834ddd33806d Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 11 Sep 2023 14:31:52 +0000 Subject: [PATCH 7/8] allow unitialised nested arrays --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 0fc10ee33c9..c245b19a01f 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1206,19 +1206,31 @@ impl AcirContext { ) -> Result<(), InternalError> { // If the optional values are supplied, then we fill the initialized // array with those values. If not, then we fill it with zeros. + let mut nested = false; let initialized_values = match optional_values { None => { let zero = self.add_constant(FieldElement::zero()); let zero_witness = self.var_to_witness(zero)?; vec![zero_witness; len] } - Some(optional_values) => try_vecmap(optional_values, |value| { - let value = value.clone().into_var()?; - self.var_to_witness(value) - })?, + Some(optional_values) => { + let mut values = Vec::new(); + for value in optional_values { + if let Ok(some_value) = value.clone().into_var() { + values.push(self.var_to_witness(some_value)?); + } else { + nested = true; + break; + } + } + values + } }; + // we do not initialize nested arrays. This means that non-const indexes are not supported for nested arrays + if !nested { + self.acir_ir.push_opcode(Opcode::MemoryInit { block_id, init: initialized_values }); + } - self.acir_ir.push_opcode(Opcode::MemoryInit { block_id, init: initialized_values }); Ok(()) } } From 53c252de2ad96b76abfaaa4a17d155d66e0ecd3a Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 11 Sep 2023 16:51:46 +0000 Subject: [PATCH 8/8] use correct block_id and move the test case to test folder --- .../brillig_unitialised_arrays/Nargo.toml | 7 +++++++ .../brillig_unitialised_arrays/Prover.toml | 0 .../brillig_unitialised_arrays/src/main.nr | 0 .../brillig_unitialised_arrays/Nargo.toml | 5 ----- crates/noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Nargo.toml rename crates/nargo_cli/tests/{test_data_ssa_refactor => execution_success}/brillig_unitialised_arrays/Prover.toml (100%) rename crates/nargo_cli/tests/{test_data_ssa_refactor => execution_success}/brillig_unitialised_arrays/src/main.nr (100%) delete mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml diff --git a/crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Nargo.toml b/crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Nargo.toml new file mode 100644 index 00000000000..c7045d0b816 --- /dev/null +++ b/crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_unitialised_arrays" +type = "bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml b/crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Prover.toml similarity index 100% rename from crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Prover.toml rename to crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/Prover.toml diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr b/crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/src/main.nr similarity index 100% rename from crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/src/main.nr rename to crates/nargo_cli/tests/execution_success/brillig_unitialised_arrays/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml deleted file mode 100644 index e0b467ce5da..00000000000 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_unitialised_arrays/Nargo.toml +++ /dev/null @@ -1,5 +0,0 @@ -[package] -authors = [""] -compiler_version = "0.1" - -[dependencies] \ No newline at end of file diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs index 5daac89de60..81c79eda064 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -416,7 +416,7 @@ impl Context { for result in result_ids.iter().zip(output_values) { if let AcirValue::Array(values) = &result.1 { - let block_id = BlockId(dfg.resolve(*result.0).to_usize() as u32); + let block_id = self.block_id(&dfg.resolve(*result.0)); let values: Vec = values.iter().cloned().collect(); self.initialize_array(block_id, values.len(), Some(&values))?; }