From 1399926d1c29211c8e7aa2c21e300aed00657b43 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 17 Jan 2025 20:55:09 +0000 Subject: [PATCH 1/4] fix: avoid creating unnecessary memory blocks --- compiler/noirc_evaluator/src/acir/mod.rs | 34 ++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index b573b7136f3..26e205ff8ba 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -3709,4 +3709,38 @@ mod test { } } } + + #[test] + fn does_not_generate_memory_blocks_without_dynamic_accesses() { + let src = " + acir(inline) fn main f0 { + b0(v0: [Field; 2]): + v2, v3 = call as_slice(v0) -> (u32, [Field]) + call f1(u32 2, v3) + v7 = array_get v0, index u32 0 -> Field + constrain v7 == Field 0 + return + } + + brillig(inline) fn foo f1 { + b0(v0: u32, v1: [Field]): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let (acir_functions, _brillig_functions, _, _) = ssa + .into_acir(&brillig, ExpressionWidth::default()) + .expect("Should compile manually written SSA into ACIR"); + + assert_eq!(acir_functions.len(), 1); + + // Check that no memory opcodes were emitted. + let main = &acir_functions[0]; + assert!(!main + .opcodes() + .iter() + .any(|opcode| matches!(opcode, Opcode::MemoryInit { .. } | Opcode::MemoryOp { .. }))); + } } From ac0a869c62e44f1bfd30a2cc38f3dbb2b0caf325 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 Jan 2025 22:28:00 +0000 Subject: [PATCH 2/4] feat: defer initializing memory block initialization when calling `as_slice` --- compiler/noirc_evaluator/src/acir/mod.rs | 73 ++++++++++-------------- 1 file changed, 30 insertions(+), 43 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 26e205ff8ba..fed6d4d8b00 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -1069,8 +1069,7 @@ impl<'a> Context<'a> { // Ensure that array id is fully resolved. let array = dfg.resolve(array); - let array_id = dfg.resolve(array); - let array_typ = dfg.type_of_value(array_id); + let array_typ = dfg.type_of_value(array); // Compiler sanity checks assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation"); let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else { @@ -1125,15 +1124,7 @@ impl<'a> Context<'a> { index: ValueId, store_value: Option, ) -> Result { - let array_id = dfg.resolve(array); - let array_typ = dfg.type_of_value(array_id); - // Compiler sanity checks - assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation"); - let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else { - unreachable!("ICE: expected array or slice type"); - }; - - match self.convert_value(array_id, dfg) { + match self.convert_value(array, dfg) { AcirValue::Var(acir_var, _) => { Err(RuntimeError::InternalError(InternalError::Unexpected { expected: "an array value".to_string(), @@ -2231,45 +2222,41 @@ impl<'a> Context<'a> { Intrinsic::AsSlice => { let slice_contents = arguments[0]; let slice_typ = dfg.type_of_value(slice_contents); - let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); - let result_block_id = self.block_id(&result_ids[1]); let acir_value = self.convert_value(slice_contents, dfg); + let (slice_length, result) = match acir_value { + AcirValue::Var(_, _) => { + unreachable!("ICE: cannot call `as_slice` on non-array type") + } + array @ AcirValue::Array(_) => { + let array_len = if !slice_typ.contains_slice_element() { + slice_typ.flattened_size() as usize + } else { + self.flattened_slice_size(slice_contents, dfg) + }; + (array_len, array) + } + AcirValue::DynamicArray(source_array) => { + let result_block_id = self.block_id(&result_ids[1]); + self.copy_dynamic_array( + source_array.block_id, + result_block_id, + source_array.len, + )?; - let array_len = if !slice_typ.contains_slice_element() { - slice_typ.flattened_size() as usize - } else { - self.flattened_slice_size(slice_contents, dfg) - }; - let slice_length = self.acir_context.add_constant(array_len); - self.copy_dynamic_array(block_id, result_block_id, array_len)?; + let array = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: source_array.len, + value_types: source_array.value_types, + element_type_sizes: source_array.element_type_sizes, + }); - let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { - Some(self.init_element_type_sizes_array( - &slice_typ, - slice_contents, - Some(&acir_value), - dfg, - )?) - } else { - None + (source_array.len, array) + } }; - let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); - assert!( - array_len == value_types.len(), - "AsSlice: unexpected length difference: {:?} != {:?}", - array_len, - value_types.len() - ); - - let result = AcirValue::DynamicArray(AcirDynamicArray { - block_id: result_block_id, - len: value_types.len(), - value_types, - element_type_sizes, - }); + let slice_length = self.acir_context.add_constant(slice_length); Ok(vec![AcirValue::Var(slice_length, AcirType::field()), result]) } Intrinsic::SlicePushBack => { From d04d477907b9525ebcb1235ba91a4c8ccdef3fae Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 Jan 2025 22:34:24 +0000 Subject: [PATCH 3/4] . --- .../regression_7062/Nargo.toml | 0 test_programs/execution_success/regression_7062/Prover.toml | 2 ++ .../regression_7062/src/main.nr | 6 ++++-- 3 files changed, 6 insertions(+), 2 deletions(-) rename test_programs/{compile_success_no_bug => execution_success}/regression_7062/Nargo.toml (100%) create mode 100644 test_programs/execution_success/regression_7062/Prover.toml rename test_programs/{compile_success_no_bug => execution_success}/regression_7062/src/main.nr (51%) diff --git a/test_programs/compile_success_no_bug/regression_7062/Nargo.toml b/test_programs/execution_success/regression_7062/Nargo.toml similarity index 100% rename from test_programs/compile_success_no_bug/regression_7062/Nargo.toml rename to test_programs/execution_success/regression_7062/Nargo.toml diff --git a/test_programs/execution_success/regression_7062/Prover.toml b/test_programs/execution_success/regression_7062/Prover.toml new file mode 100644 index 00000000000..08608e6b3ba --- /dev/null +++ b/test_programs/execution_success/regression_7062/Prover.toml @@ -0,0 +1,2 @@ +index = 1 +value = 1 diff --git a/test_programs/compile_success_no_bug/regression_7062/src/main.nr b/test_programs/execution_success/regression_7062/src/main.nr similarity index 51% rename from test_programs/compile_success_no_bug/regression_7062/src/main.nr rename to test_programs/execution_success/regression_7062/src/main.nr index c640062b45b..47e7593c0e6 100644 --- a/test_programs/compile_success_no_bug/regression_7062/src/main.nr +++ b/test_programs/execution_success/regression_7062/src/main.nr @@ -1,8 +1,10 @@ -fn main(args: [Field; 2]) { +fn main(value: Field, index: u32) { + let mut args = &[0, 1]; + args[index] = value; /// Safety: n/a unsafe { store(args) }; // Dummy test to remove the 'underconstraint bug' assert(args[0] + args[1] != 0); } -pub unconstrained fn store(_: [Field]) {} \ No newline at end of file +pub unconstrained fn store(_: [Field]) {} From 10770cf8602feb0c6c845d976c45aee5785faff8 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 Jan 2025 22:39:22 +0000 Subject: [PATCH 4/4] . --- compiler/noirc_evaluator/src/acir/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index fed6d4d8b00..a3c44e055b4 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -3725,9 +3725,6 @@ mod test { // Check that no memory opcodes were emitted. let main = &acir_functions[0]; - assert!(!main - .opcodes() - .iter() - .any(|opcode| matches!(opcode, Opcode::MemoryInit { .. } | Opcode::MemoryOp { .. }))); + assert!(!main.opcodes().iter().any(|opcode| matches!(opcode, Opcode::MemoryOp { .. }))); } }