From d986bfc5aa8282f70a8c3584e17d483b68b00a21 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 14:07:58 -0400 Subject: [PATCH 01/16] add as_slice builtin function, add execution test --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 58 +++++++++++++++++++ .../noirc_evaluator/src/ssa/ir/instruction.rs | 4 ++ .../src/ssa/ir/instruction/call.rs | 10 ++++ noir_stdlib/src/array.nr | 10 +--- .../array_to_slice/Nargo.toml | 7 +++ .../array_to_slice/Prover.toml | 2 + .../array_to_slice/src/main.nr | 23 ++++++++ 7 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 test_programs/execution_success/array_to_slice/Nargo.toml create mode 100644 test_programs/execution_success/array_to_slice/Prover.toml create mode 100644 test_programs/execution_success/array_to_slice/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 8d4d0668534..9519e7565ba 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1663,6 +1663,64 @@ impl Context { }; Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } + Intrinsic::AsSlice => { + let elements = match self.convert_value(arguments[0], dfg) { + AcirValue::Array(values) => values, + _ => unreachable!("Non-array passed to as_slice method"), + }; + + let slice_length = self.acir_context.add_constant(elements.len()); + let (slice_contents, slice_typ, _) = + self.check_array_is_initialized(arguments[0], dfg)?; + assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); + + let slice = self.convert_value(slice_contents, dfg); + let mut new_elem_size = Self::flattened_value_size(&slice); + + let mut new_slice = elements.clone(); + self.slice_intrinsic_input(&mut new_slice, slice)?; + + let new_slice_val = AcirValue::Array(new_slice); + let result_block_id = self.block_id(&result_ids[1]); + self.initialize_array(result_block_id, new_elem_size, Some(new_slice_val.clone()))?; + // The previous slice length represents the index we want to write into. + // Write the elements we wish to push back directly. + // The slice's underlying array value should already be filled with dummy data + // to enable this write to be within bounds. + // The dummy data is either attached during SSA gen or in this match case for non-nested slices. + // These values can then be accessed due to the increased dynamic slice length. + let mut var_index = slice_length; + for elem in elements { + new_elem_size += Self::flattened_value_size(&elem); + self.array_set_value(&elem, result_block_id, &mut var_index)?; + } + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(&new_slice_val), + dfg, + )?) + } else { + None + }; + + let value_types = new_slice_val.flat_numeric_types(); + assert_eq!( + value_types.len(), + new_elem_size, + "ICE: Value types array must match new slice size" + ); + + let result = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: new_elem_size, + value_types, + element_type_sizes, + }); + Ok(vec![AcirValue::Var(slice_length, AcirType::field()), result]) + } Intrinsic::SlicePushBack => { // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 0b6c7074e45..709c87ee0f1 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -37,6 +37,7 @@ pub(crate) type InstructionId = Id; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub(crate) enum Intrinsic { ArrayLen, + AsSlice, AssertConstant, SlicePushBack, SlicePushFront, @@ -57,6 +58,7 @@ impl std::fmt::Display for Intrinsic { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Intrinsic::ArrayLen => write!(f, "array_len"), + Intrinsic::AsSlice => write!(f, "as_slice"), Intrinsic::AssertConstant => write!(f, "assert_constant"), Intrinsic::SlicePushBack => write!(f, "slice_push_back"), Intrinsic::SlicePushFront => write!(f, "slice_push_front"), @@ -89,6 +91,7 @@ impl Intrinsic { Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => true, Intrinsic::ArrayLen + | Intrinsic::AsSlice | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SlicePopBack @@ -109,6 +112,7 @@ impl Intrinsic { pub(crate) fn lookup(name: &str) -> Option { match name { "array_len" => Some(Intrinsic::ArrayLen), + "as_slice" => Some(Intrinsic::AsSlice), "assert_constant" => Some(Intrinsic::AssertConstant), "apply_range_constraint" => Some(Intrinsic::ApplyRangeConstraint), "slice_push_back" => Some(Intrinsic::SlicePushBack), diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 9349d58c4d9..8b800e0db54 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -84,6 +84,16 @@ pub(super) fn simplify_call( SimplifyResult::None } } + Intrinsic::AsSlice => { + let slice = dfg.get_array_constant(arguments[0]); + if let Some((slice, element_type)) = slice { + let slice_length = dfg.make_constant(slice.len().into(), Type::length_type()); + let new_slice = dfg.make_array(slice, element_type); + SimplifyResult::SimplifiedToMultiple(vec![slice_length, new_slice]) + } else { + SimplifyResult::None + } + } Intrinsic::SlicePushBack => { let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, element_type)) = slice { diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 3da4b649174..fd91d7dad9b 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -52,14 +52,8 @@ impl [T; N] { result } - // Converts an array into a slice. - pub fn as_slice(self) -> [T] { - let mut slice = []; - for elem in self { - slice = slice.push_back(elem); - } - slice - } + #[builtin(as_slice)] + pub fn as_slice(self) -> [T] {} // Apply a function to each element of an array, returning a new array // containing the mapped elements. diff --git a/test_programs/execution_success/array_to_slice/Nargo.toml b/test_programs/execution_success/array_to_slice/Nargo.toml new file mode 100644 index 00000000000..90c67b07b2b --- /dev/null +++ b/test_programs/execution_success/array_to_slice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_to_slice" +type = "bin" +authors = [""] +compiler_version = ">=0.24.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/array_to_slice/Prover.toml b/test_programs/execution_success/array_to_slice/Prover.toml new file mode 100644 index 00000000000..26fdbc19975 --- /dev/null +++ b/test_programs/execution_success/array_to_slice/Prover.toml @@ -0,0 +1,2 @@ +x = "0" +y = "1" diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr new file mode 100644 index 00000000000..06966f201f4 --- /dev/null +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -0,0 +1,23 @@ +// Converts an array into a slice. +fn as_slice_push(xs: [T; N]) -> [T] { + let mut slice = []; + for elem in xs { + slice = slice.push_back(elem); + } + slice +} + +fn main(x: Field, y: pub Field) { + let xs: [Field; 0] = []; + let ys: [Field; 1] = [1]; + let zs: [Field; 2] = [1, 2]; + let ws: [Field; 3] = [1; 3]; + let qs: [Field; 4] = [3, 2, 1, 0]; + + assert(x != y); + assert(xs.as_slice() == as_slice_push(xs)); + assert(ys.as_slice() == as_slice_push(ys)); + assert(zs.as_slice() == as_slice_push(zs)); + assert(ws.as_slice() == as_slice_push(ws)); + assert(qs.as_slice() == as_slice_push(qs)); +} From 462af6456db1013fada332aa56ab38debe035bde Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 15:22:06 -0400 Subject: [PATCH 02/16] wip debugging test failures, add dynamic array case to test --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 49 ++++++------------- .../array_to_slice/src/main.nr | 9 ++++ 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 9519e7565ba..55a3a3466a7 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1664,58 +1664,37 @@ impl Context { Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } Intrinsic::AsSlice => { - let elements = match self.convert_value(arguments[0], dfg) { - AcirValue::Array(values) => values, - _ => unreachable!("Non-array passed to as_slice method"), - }; - - let slice_length = self.acir_context.add_constant(elements.len()); - let (slice_contents, slice_typ, _) = + let (slice_contents, slice_typ, block_id) = self.check_array_is_initialized(arguments[0], dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); - let slice = self.convert_value(slice_contents, dfg); - let mut new_elem_size = Self::flattened_value_size(&slice); - - let mut new_slice = elements.clone(); - self.slice_intrinsic_input(&mut new_slice, slice)?; - - let new_slice_val = AcirValue::Array(new_slice); let result_block_id = self.block_id(&result_ids[1]); - self.initialize_array(result_block_id, new_elem_size, Some(new_slice_val.clone()))?; - // The previous slice length represents the index we want to write into. - // Write the elements we wish to push back directly. - // The slice's underlying array value should already be filled with dummy data - // to enable this write to be within bounds. - // The dummy data is either attached during SSA gen or in this match case for non-nested slices. - // These values can then be accessed due to the increased dynamic slice length. - let mut var_index = slice_length; - for elem in elements { - new_elem_size += Self::flattened_value_size(&elem); - self.array_set_value(&elem, result_block_id, &mut var_index)?; - } - let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(&new_slice_val), + None, dfg, )?) } else { None }; - let value_types = new_slice_val.flat_numeric_types(); - assert_eq!( - value_types.len(), - new_elem_size, - "ICE: Value types array must match new slice size" - ); + // TODO: remove this and the assert + let array_len = if !slice_typ.contains_slice_element() { + slice_typ.flattened_size() + } else { + self.flattened_slice_size(slice_contents, dfg) + }; + 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 slice_length = self.acir_context.add_constant(value_types.len()); + self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, - len: new_elem_size, + len: value_types.len(), value_types, element_type_sizes, }); diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 06966f201f4..879607f8f8a 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -14,10 +14,19 @@ fn main(x: Field, y: pub Field) { let ws: [Field; 3] = [1; 3]; let qs: [Field; 4] = [3, 2, 1, 0]; + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + let dynamic_expected: [Field] = [1000, 2, 1, 0]; + dynamic[x] = 1000; + assert(x != y); assert(xs.as_slice() == as_slice_push(xs)); assert(ys.as_slice() == as_slice_push(ys)); assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); + assert(dynamic.as_slice()[0] == dynamic_expected[0]); + assert(dynamic.as_slice()[1] == dynamic_expected[1]); + assert(dynamic.as_slice()[2] == dynamic_expected[2]); + assert(dynamic.as_slice()[3] == dynamic_expected[3]); + assert(dynamic.as_slice().len() == 4); } From 2b749e124e31419aeb48c821577ea29c5d53d84c Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 15:55:25 -0400 Subject: [PATCH 03/16] wip debugging error on valid index of result of as_slice --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 34 +++++++++++-------- .../array_to_slice/src/main.nr | 3 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 55a3a3466a7..554a3a9d930 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1239,15 +1239,16 @@ impl Context { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result<(), RuntimeError> { + ) -> Result, RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(source, &index_var)?; Ok::(AcirValue::Var(read, AcirType::field())) })?; - self.initialize_array(destination, array_len, Some(AcirValue::Array(init_values.into())))?; - Ok(()) + let array: im::Vector = init_values.into(); + self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?; + Ok(array) } fn get_flattened_index( @@ -1669,16 +1670,7 @@ impl Context { assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let result_block_id = self.block_id(&result_ids[1]); - let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { - Some(self.init_element_type_sizes_array( - &slice_typ, - slice_contents, - None, - dfg, - )?) - } else { - None - }; + let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { @@ -1686,11 +1678,23 @@ impl Context { } else { self.flattened_slice_size(slice_contents, dfg) }; - 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 slice_length = self.acir_context.add_constant(value_types.len()); - self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; + let array_elements = self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; + + assert!(array_elements.len() == value_types.len(), "AsSlice: unexpected length difference (2): {:?} != {:?}", array_elements.len(), value_types.len()); + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(&AcirValue::Array(array_elements)), + dfg, + )?) + } else { + None + }; let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 879607f8f8a..4f5594c6d11 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -15,7 +15,7 @@ fn main(x: Field, y: pub Field) { let qs: [Field; 4] = [3, 2, 1, 0]; let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - let dynamic_expected: [Field] = [1000, 2, 1, 0]; + let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; dynamic[x] = 1000; assert(x != y); @@ -24,6 +24,7 @@ fn main(x: Field, y: pub Field) { assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); + assert(dynamic.as_slice()[0] == dynamic_expected[0]); assert(dynamic.as_slice()[1] == dynamic_expected[1]); assert(dynamic.as_slice()[2] == dynamic_expected[2]); From 3da8474a6ef82d100fdc5195ecd32284f29e0b93 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 11 Mar 2024 21:47:18 +0000 Subject: [PATCH 04/16] update can_omit_element_sizes_array --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 554a3a9d930..1109767894d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1025,7 +1025,8 @@ impl Context { self.array_set_value(&store_value, result_block_id, &mut var_index)?; let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { - Some(self.init_element_type_sizes_array(&array_typ, array_id, None, dfg)?) + let acir_value = self.convert_value(array_id, dfg); + Some(self.init_element_type_sizes_array(&array_typ, array_id, Some(&acir_value), dfg)?) } else { None }; @@ -1258,7 +1259,9 @@ impl Context { var_index: AcirVar, dfg: &DataFlowGraph, ) -> Result { + dbg!(!can_omit_element_sizes_array(array_typ)); if !can_omit_element_sizes_array(array_typ) { + dbg!(array_typ.clone()); let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1670,7 +1673,7 @@ impl Context { assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let result_block_id = self.block_id(&result_ids[1]); - let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types(); + let acir_value = self.convert_value(slice_contents, dfg); // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { @@ -1678,24 +1681,24 @@ impl Context { } else { self.flattened_slice_size(slice_contents, dfg) }; - assert!(array_len == value_types.len(), "AsSlice: unexpected length difference: {:?} != {:?}", array_len, value_types.len()); - let slice_length = self.acir_context.add_constant(value_types.len()); - let array_elements = self.copy_dynamic_array(block_id, result_block_id, value_types.len())?; - - assert!(array_elements.len() == value_types.len(), "AsSlice: unexpected length difference (2): {:?} != {:?}", array_elements.len(), value_types.len()); + let slice_length = self.acir_context.add_constant(array_len); + self.copy_dynamic_array(block_id, result_block_id, array_len)?; let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(&AcirValue::Array(array_elements)), + Some(&acir_value), dfg, )?) } else { None }; + 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(), @@ -2309,11 +2312,9 @@ impl Context { // We can omit the element size array for arrays which don't contain arrays or slices. fn can_omit_element_sizes_array(array_typ: &Type) -> bool { - if array_typ.contains_slice_element() { - return false; - } - let Type::Array(types, _) = array_typ else { - panic!("ICE: expected array type"); + let types = match array_typ { + Type::Array(types, _) | Type::Slice(types) => types, + _ => panic!("ICE: expected array or slice type"), }; !types.iter().any(|typ| typ.contains_an_array()) From 197fd77fa7ba15ec663afc1a9c444add39025927 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 11 Mar 2024 19:22:52 -0400 Subject: [PATCH 05/16] remove dbg calls and cleanup, add brillig implementation for AsSlice, wip implementing convert_ssa_as_slice --- .../src/brillig/brillig_gen/brillig_block.rs | 30 +++++++++++++++++++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 18 +++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index c04d8475f08..76bd4bcae1f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -453,6 +453,15 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_array_len(arguments[0], result_variable.address, dfg); } } + Value::Intrinsic(Intrinsic::AsSlice) => { + let result_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + dfg.instruction_results(instruction_id)[0], + dfg, + ); + self.convert_ssa_as_slice(arguments[0], result_variable.address, dfg); + } Value::Intrinsic( Intrinsic::SlicePushBack | Intrinsic::SlicePopBack @@ -1488,6 +1497,27 @@ impl<'block> BrilligBlock<'block> { } } } + + /// Gets the "user-facing" slice from an array + /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. + fn convert_ssa_as_slice( + &mut self, + array_id: ValueId, + result_register: MemoryAddress, + dfg: &DataFlowGraph, + ) { + let array_variable = self.convert_ssa_value(array_id, dfg); + match array_variable { + BrilligVariable::BrilligArray(array) => { + let slice_var = self.brillig_context.array_to_vector(&array); + + self.brillig_context.allocate_array_instruction(result_register, slice_var.size); + } + _ => { + unreachable!("ICE: Cannot convert {array_variable:?} to slice") + } + } + } } /// Returns the type of the operation considering the types of the operands diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 1109767894d..65dd8821ae1 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1026,7 +1026,12 @@ impl Context { let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { let acir_value = self.convert_value(array_id, dfg); - Some(self.init_element_type_sizes_array(&array_typ, array_id, Some(&acir_value), dfg)?) + Some(self.init_element_type_sizes_array( + &array_typ, + array_id, + Some(&acir_value), + dfg, + )?) } else { None }; @@ -1259,9 +1264,7 @@ impl Context { var_index: AcirVar, dfg: &DataFlowGraph, ) -> Result { - dbg!(!can_omit_element_sizes_array(array_typ)); if !can_omit_element_sizes_array(array_typ) { - dbg!(array_typ.clone()); let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1675,13 +1678,11 @@ impl Context { let result_block_id = self.block_id(&result_ids[1]); let acir_value = self.convert_value(slice_contents, dfg); - // TODO: remove this and the assert let array_len = if !slice_typ.contains_slice_element() { slice_typ.flattened_size() } 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)?; @@ -1697,7 +1698,12 @@ impl Context { }; 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()); + 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, From a5f7d9e13266b7602496c035258b82b0d43bc183 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 13:34:00 -0400 Subject: [PATCH 06/16] wip debugging brillig as_slice implementation, rewrote implementation to match similar ones, recreated debugger error in execution test --- .../src/brillig/brillig_gen/brillig_block.rs | 59 +++++++++++++++---- .../array_to_slice/src/main.nr | 12 ++++ 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 76bd4bcae1f..db7abf5011d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -454,13 +454,8 @@ impl<'block> BrilligBlock<'block> { } } Value::Intrinsic(Intrinsic::AsSlice) => { - let result_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - dfg.instruction_results(instruction_id)[0], - dfg, - ); - self.convert_ssa_as_slice(arguments[0], result_variable.address, dfg); + let result_ids = dfg.instruction_results(instruction_id); + self.convert_ssa_as_slice(arguments[0], result_ids[0], result_ids[1], dfg); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1307,13 +1302,18 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary. fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> BrilligVariable { + dbg!("convert_ssa_value id: {:?}", value_id); let value_id = dfg.resolve(value_id); + dbg!("convert_ssa_value id2: {:?}", value_id); let value = &dfg[value_id]; + dbg!("convert_ssa_value value: {:?}", value); match value { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. + + dbg!("convert_ssa_value instruction: {:?}", value); self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { @@ -1335,11 +1335,13 @@ impl<'block> BrilligBlock<'block> { } } Value::Array { array, .. } => { + dbg!("convert_ssa_value array: {:?}", array); if let Some(variable) = self.variables.get_constant(value_id, dfg) { variable } else { let new_variable = self.variables.allocate_constant(self.brillig_context, value_id, dfg); + dbg!("convert_ssa_value new_variable: {:?}", new_variable); // Initialize the variable let pointer = match new_variable { @@ -1363,12 +1365,14 @@ impl<'block> BrilligBlock<'block> { ), }; + dbg!("convert_ssa_value pointer: {:?}", pointer); // Write the items // Allocate a register for the iterator let iterator_register = self.brillig_context.make_usize_constant(0_usize.into()); + dbg!("convert_ssa_value iterator_register: {:?}", iterator_register); for element_id in array.iter() { let element_variable = self.convert_ssa_value(*element_id, dfg); // Store the item in memory @@ -1383,6 +1387,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_register(iterator_register); + dbg!("convert_ssa_value new_variable 2: {:?}", new_variable); new_variable } } @@ -1503,15 +1508,49 @@ impl<'block> BrilligBlock<'block> { fn convert_ssa_as_slice( &mut self, array_id: ValueId, - result_register: MemoryAddress, + target_len: ValueId, + target_variable: ValueId, dfg: &DataFlowGraph, ) { let array_variable = self.convert_ssa_value(array_id, dfg); match array_variable { BrilligVariable::BrilligArray(array) => { - let slice_var = self.brillig_context.array_to_vector(&array); + dbg!("array: {:?}", array); + dbg!("array_id: {:?}", array_id); + dbg!("target_len: {:?}", target_len); + dbg!("target_variable: {:?}", target_variable); + dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + + let array_size_address = self.brillig_context.allocate_register(); + self.brillig_context.usize_const(array_size_address, array.size.into()); + + let len_variable = self.convert_ssa_value(target_len, dfg); + let len_address = len_variable.extract_single_addr(); + self.brillig_context.mov_instruction(len_address.address, array_size_address); + + let result_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + target_variable, + dfg, + ); + let target_vector = result_variable.extract_vector(); + + self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); + // We initialize the RC of the target vector to 1 + self.brillig_context.usize_const(target_vector.rc, 1_usize.into()); + + // Now we offset the target pointer by variables_to_insert.len() + let destination_copy_pointer = self.brillig_context.allocate_register(); + self.brillig_context.copy_array_instruction( + array.pointer, + destination_copy_pointer, + array_size_address, + ); + + self.brillig_context.deallocate_register(array_size_address); + self.brillig_context.deallocate_register(destination_copy_pointer); - self.brillig_context.allocate_array_instruction(result_register, slice_var.size); } _ => { unreachable!("ICE: Cannot convert {array_variable:?} to slice") diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 4f5594c6d11..7421d14440e 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,6 +7,16 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } +unconstrained fn brillig_as_slice(x: Field) -> bool { + let mut dynamic: [Field; 1] = [2]; + dynamic[x] = 999; + let brillig_expected: [Field; 1] = [999]; + let brillig_slice = dynamic.as_slice(); + + (brillig_slice[0] == brillig_expected[0]) & + (brillig_slice.len() == 1) +} + fn main(x: Field, y: pub Field) { let xs: [Field; 0] = []; let ys: [Field; 1] = [1]; @@ -30,4 +40,6 @@ fn main(x: Field, y: pub Field) { assert(dynamic.as_slice()[2] == dynamic_expected[2]); assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); + + assert(brillig_as_slice(x)); } From e8637a58b2d0bfd4ba7cf4328443eeacd07be0a3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 14:33:53 -0400 Subject: [PATCH 07/16] wip debugging brillig slice access error, fixed wrong initialization of brillig length variable --- .../src/brillig/brillig_gen/brillig_block.rs | 30 ++++++++----------- .../array_to_slice/src/main.nr | 22 +++++++++----- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index db7abf5011d..45ef9515d58 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1302,18 +1302,14 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary. fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> BrilligVariable { - dbg!("convert_ssa_value id: {:?}", value_id); let value_id = dfg.resolve(value_id); - dbg!("convert_ssa_value id2: {:?}", value_id); let value = &dfg[value_id]; - dbg!("convert_ssa_value value: {:?}", value); match value { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. - dbg!("convert_ssa_value instruction: {:?}", value); self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { @@ -1335,13 +1331,11 @@ impl<'block> BrilligBlock<'block> { } } Value::Array { array, .. } => { - dbg!("convert_ssa_value array: {:?}", array); if let Some(variable) = self.variables.get_constant(value_id, dfg) { variable } else { let new_variable = self.variables.allocate_constant(self.brillig_context, value_id, dfg); - dbg!("convert_ssa_value new_variable: {:?}", new_variable); // Initialize the variable let pointer = match new_variable { @@ -1365,14 +1359,12 @@ impl<'block> BrilligBlock<'block> { ), }; - dbg!("convert_ssa_value pointer: {:?}", pointer); // Write the items // Allocate a register for the iterator let iterator_register = self.brillig_context.make_usize_constant(0_usize.into()); - dbg!("convert_ssa_value iterator_register: {:?}", iterator_register); for element_id in array.iter() { let element_variable = self.convert_ssa_value(*element_id, dfg); // Store the item in memory @@ -1387,7 +1379,6 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_register(iterator_register); - dbg!("convert_ssa_value new_variable 2: {:?}", new_variable); new_variable } } @@ -1515,18 +1506,23 @@ impl<'block> BrilligBlock<'block> { let array_variable = self.convert_ssa_value(array_id, dfg); match array_variable { BrilligVariable::BrilligArray(array) => { - dbg!("array: {:?}", array); - dbg!("array_id: {:?}", array_id); - dbg!("target_len: {:?}", target_len); - dbg!("target_variable: {:?}", target_variable); - dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + // dbg!("array: {:?}", array); + // dbg!("array_id: {:?}", array_id); + // dbg!("target_len: {:?}", target_len); + // dbg!("target_variable: {:?}", target_variable); + // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + dbg!("array.size: {:?}", array.size); let array_size_address = self.brillig_context.allocate_register(); self.brillig_context.usize_const(array_size_address, array.size.into()); - let len_variable = self.convert_ssa_value(target_len, dfg); - let len_address = len_variable.extract_single_addr(); - self.brillig_context.mov_instruction(len_address.address, array_size_address); + let len_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + target_len, + dfg, + ); + self.brillig_context.mov_instruction(len_variable.address, array_size_address); let result_variable = self.variables.define_variable( self.function_context, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 7421d14440e..a702c85c8d3 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,14 +7,19 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } -unconstrained fn brillig_as_slice(x: Field) -> bool { - let mut dynamic: [Field; 1] = [2]; - dynamic[x] = 999; - let brillig_expected: [Field; 1] = [999]; +unconstrained fn brillig_as_slice(x: Field) -> Field { + let mut dynamic: [Field; 1] = [1]; + dynamic[x] = 2; + assert(dynamic[0] == 2); + print(dynamic); + print("\n"); + let brillig_slice = dynamic.as_slice(); + assert(brillig_slice.len() == 1); - (brillig_slice[0] == brillig_expected[0]) & - (brillig_slice.len() == 1) + print(brillig_slice[0]); + print("\n"); + brillig_slice[0] } fn main(x: Field, y: pub Field) { @@ -41,5 +46,8 @@ fn main(x: Field, y: pub Field) { assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); - assert(brillig_as_slice(x)); + let res = brillig_as_slice(x); + // print(res); + // print("\n"); + assert(res == 2); } From efa81dbd034538a3cb57dd7c03c6bdb3ae69b0ca Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 14:40:02 -0400 Subject: [PATCH 08/16] wip debugging: cleanup for second pair of eyes --- .../src/brillig/brillig_gen/brillig_block.rs | 5 +- .../array_to_slice/src/main.nr | 46 +++++++++---------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 45ef9515d58..d05659aec80 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1511,6 +1511,7 @@ impl<'block> BrilligBlock<'block> { // dbg!("target_len: {:?}", target_len); // dbg!("target_variable: {:?}", target_variable); // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); + dbg!("array: {:?}", array); dbg!("array.size: {:?}", array.size); let array_size_address = self.brillig_context.allocate_register(); @@ -1533,10 +1534,6 @@ impl<'block> BrilligBlock<'block> { let target_vector = result_variable.extract_vector(); self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const(target_vector.rc, 1_usize.into()); - - // Now we offset the target pointer by variables_to_insert.len() let destination_copy_pointer = self.brillig_context.allocate_register(); self.brillig_context.copy_array_instruction( array.pointer, diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index a702c85c8d3..6e814f7db5d 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -17,34 +17,34 @@ unconstrained fn brillig_as_slice(x: Field) -> Field { let brillig_slice = dynamic.as_slice(); assert(brillig_slice.len() == 1); - print(brillig_slice[0]); + print([brillig_slice[0]]); print("\n"); brillig_slice[0] } fn main(x: Field, y: pub Field) { - let xs: [Field; 0] = []; - let ys: [Field; 1] = [1]; - let zs: [Field; 2] = [1, 2]; - let ws: [Field; 3] = [1; 3]; - let qs: [Field; 4] = [3, 2, 1, 0]; - - let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; - dynamic[x] = 1000; - - assert(x != y); - assert(xs.as_slice() == as_slice_push(xs)); - assert(ys.as_slice() == as_slice_push(ys)); - assert(zs.as_slice() == as_slice_push(zs)); - assert(ws.as_slice() == as_slice_push(ws)); - assert(qs.as_slice() == as_slice_push(qs)); - - assert(dynamic.as_slice()[0] == dynamic_expected[0]); - assert(dynamic.as_slice()[1] == dynamic_expected[1]); - assert(dynamic.as_slice()[2] == dynamic_expected[2]); - assert(dynamic.as_slice()[3] == dynamic_expected[3]); - assert(dynamic.as_slice().len() == 4); + // let xs: [Field; 0] = []; + // let ys: [Field; 1] = [1]; + // let zs: [Field; 2] = [1, 2]; + // let ws: [Field; 3] = [1; 3]; + // let qs: [Field; 4] = [3, 2, 1, 0]; + // + // let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + // let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; + // dynamic[x] = 1000; + // + // assert(x != y); + // assert(xs.as_slice() == as_slice_push(xs)); + // assert(ys.as_slice() == as_slice_push(ys)); + // assert(zs.as_slice() == as_slice_push(zs)); + // assert(ws.as_slice() == as_slice_push(ws)); + // assert(qs.as_slice() == as_slice_push(qs)); + // + // assert(dynamic.as_slice()[0] == dynamic_expected[0]); + // assert(dynamic.as_slice()[1] == dynamic_expected[1]); + // assert(dynamic.as_slice()[2] == dynamic_expected[2]); + // assert(dynamic.as_slice()[3] == dynamic_expected[3]); + // assert(dynamic.as_slice().len() == 4); let res = brillig_as_slice(x); // print(res); From 84cfbc39e721705506e59bf226483e6131ea71c5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:25:03 -0400 Subject: [PATCH 09/16] got brillig tests passing using version of convert_ssa_array_set, cleanup as_slice test --- .../src/brillig/brillig_gen/brillig_block.rs | 131 ++++++++++++------ .../array_to_slice/src/main.nr | 50 +++---- 2 files changed, 108 insertions(+), 73 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index d05659aec80..07d7d9cf444 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -454,8 +454,27 @@ impl<'block> BrilligBlock<'block> { } } Value::Intrinsic(Intrinsic::AsSlice) => { + let source_variable = self.convert_ssa_value(arguments[0], dfg); let result_ids = dfg.instruction_results(instruction_id); - self.convert_ssa_as_slice(arguments[0], result_ids[0], result_ids[1], dfg); + + // this needs to be initialized for convert_ssa_as_slice + let _len_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[0], + dfg, + ); + let destination_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[1], + dfg, + ); + + self.convert_ssa_as_slice( + source_variable, + destination_variable, + ); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1498,57 +1517,79 @@ impl<'block> BrilligBlock<'block> { /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. fn convert_ssa_as_slice( &mut self, - array_id: ValueId, - target_len: ValueId, - target_variable: ValueId, - dfg: &DataFlowGraph, + source_variable: BrilligVariable, + destination_variable: BrilligVariable, ) { - let array_variable = self.convert_ssa_value(array_id, dfg); - match array_variable { - BrilligVariable::BrilligArray(array) => { - // dbg!("array: {:?}", array); - // dbg!("array_id: {:?}", array_id); - // dbg!("target_len: {:?}", target_len); - // dbg!("target_variable: {:?}", target_variable); - // dbg!("target_variable(convert): {:?}", self.convert_ssa_value(target_variable, dfg)); - dbg!("array: {:?}", array); - dbg!("array.size: {:?}", array.size); - - let array_size_address = self.brillig_context.allocate_register(); - self.brillig_context.usize_const(array_size_address, array.size.into()); - - let len_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - target_len, - dfg, - ); - self.brillig_context.mov_instruction(len_variable.address, array_size_address); + let destination_pointer = match destination_variable { + BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, + BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, + _ => unreachable!("ICE: as_slice returns non-array"), + }; - let result_variable = self.variables.define_variable( - self.function_context, - self.brillig_context, - target_variable, - dfg, - ); - let target_vector = result_variable.extract_vector(); - - self.brillig_context.allocate_array_instruction(target_vector.pointer, array_size_address); - let destination_copy_pointer = self.brillig_context.allocate_register(); - self.brillig_context.copy_array_instruction( - array.pointer, - destination_copy_pointer, - array_size_address, - ); + let reference_count = match source_variable { + BrilligVariable::BrilligArray(BrilligArray { rc, .. }) + | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, + _ => unreachable!("ICE: as_slice on non-array"), + }; - self.brillig_context.deallocate_register(array_size_address); - self.brillig_context.deallocate_register(destination_copy_pointer); + let (source_pointer, source_size_as_register) = match source_variable { + BrilligVariable::BrilligArray(BrilligArray { size, pointer, rc: _ }) => { + let source_size_register = self.brillig_context.allocate_register(); + self.brillig_context.usize_const(source_size_register, size.into()); + (pointer, source_size_register) + } + BrilligVariable::BrilligVector(BrilligVector { size, pointer, rc: _ }) => { + let source_size_register = self.brillig_context.allocate_register(); + self.brillig_context.mov_instruction(source_size_register, size); + (pointer, source_size_register) + } + _ => unreachable!("ICE: as_slice on non-array"), + }; + + let one = self.brillig_context.make_usize_constant(1_usize.into()); + let condition = self.brillig_context.allocate_register(); + + self.brillig_context.binary_instruction( + reference_count, + one, + condition, + BrilligBinaryOp::Field { op: BinaryFieldOp::Equals }, + ); + self.brillig_context.branch_instruction(condition, |ctx, cond| { + if cond { + // Reference count is 1, we can mutate the array directly + ctx.mov_instruction(destination_pointer, source_pointer); + } else { + // First issue a array copy to the destination + ctx.allocate_array_instruction(destination_pointer, source_size_as_register); + + ctx.copy_array_instruction( + source_pointer, + destination_pointer, + source_size_as_register, + ); } - _ => { - unreachable!("ICE: Cannot convert {array_variable:?} to slice") + }); + + match destination_variable { + BrilligVariable::BrilligArray(BrilligArray { rc: target_rc, .. }) => { + self.brillig_context.usize_const(target_rc, 1_usize.into()); } + BrilligVariable::BrilligVector(BrilligVector { + size: target_size, + rc: target_rc, + .. + }) => { + self.brillig_context.mov_instruction(target_size, source_size_as_register); + self.brillig_context.usize_const(target_rc, 1_usize.into()); + } + _ => unreachable!("ICE: as_slice on non-array"), } + + self.brillig_context.deallocate_register(source_size_as_register); + self.brillig_context.deallocate_register(one); + self.brillig_context.deallocate_register(condition); } } diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 6e814f7db5d..8fb7aa8d818 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -11,43 +11,37 @@ unconstrained fn brillig_as_slice(x: Field) -> Field { let mut dynamic: [Field; 1] = [1]; dynamic[x] = 2; assert(dynamic[0] == 2); - print(dynamic); - print("\n"); let brillig_slice = dynamic.as_slice(); assert(brillig_slice.len() == 1); - print([brillig_slice[0]]); - print("\n"); brillig_slice[0] } fn main(x: Field, y: pub Field) { - // let xs: [Field; 0] = []; - // let ys: [Field; 1] = [1]; - // let zs: [Field; 2] = [1, 2]; - // let ws: [Field; 3] = [1; 3]; - // let qs: [Field; 4] = [3, 2, 1, 0]; - // - // let mut dynamic: [Field; 4] = [3, 2, 1, 0]; - // let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; - // dynamic[x] = 1000; - // - // assert(x != y); - // assert(xs.as_slice() == as_slice_push(xs)); - // assert(ys.as_slice() == as_slice_push(ys)); - // assert(zs.as_slice() == as_slice_push(zs)); - // assert(ws.as_slice() == as_slice_push(ws)); - // assert(qs.as_slice() == as_slice_push(qs)); - // - // assert(dynamic.as_slice()[0] == dynamic_expected[0]); - // assert(dynamic.as_slice()[1] == dynamic_expected[1]); - // assert(dynamic.as_slice()[2] == dynamic_expected[2]); - // assert(dynamic.as_slice()[3] == dynamic_expected[3]); - // assert(dynamic.as_slice().len() == 4); + let xs: [Field; 0] = []; + let ys: [Field; 1] = [1]; + let zs: [Field; 2] = [1, 2]; + let ws: [Field; 3] = [1; 3]; + let qs: [Field; 4] = [3, 2, 1, 0]; + + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; + let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; + dynamic[x] = 1000; + + assert(x != y); + assert(xs.as_slice() == as_slice_push(xs)); + assert(ys.as_slice() == as_slice_push(ys)); + assert(zs.as_slice() == as_slice_push(zs)); + assert(ws.as_slice() == as_slice_push(ws)); + assert(qs.as_slice() == as_slice_push(qs)); + + assert(dynamic.as_slice()[0] == dynamic_expected[0]); + assert(dynamic.as_slice()[1] == dynamic_expected[1]); + assert(dynamic.as_slice()[2] == dynamic_expected[2]); + assert(dynamic.as_slice()[3] == dynamic_expected[3]); + assert(dynamic.as_slice().len() == 4); let res = brillig_as_slice(x); - // print(res); - // print("\n"); assert(res == 2); } From c29add0b0483cb5fc9f9b14b7ff0c28d0e76f8ed Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:40:31 -0400 Subject: [PATCH 10/16] remove debugging return --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 65dd8821ae1..7b58b88e162 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1245,7 +1245,7 @@ impl Context { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result, RuntimeError> { + ) -> Result<(), RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); @@ -1254,7 +1254,7 @@ impl Context { })?; let array: im::Vector = init_values.into(); self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?; - Ok(array) + Ok(()) } fn get_flattened_index( From fecda34fde7e06a2d66e6393ea247b57904b236d Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 12 Mar 2024 18:44:45 -0400 Subject: [PATCH 11/16] cargo clippy / fmt --- .../src/brillig/brillig_gen/brillig_block.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 98906d6450c..505fb471e8b 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -471,10 +471,7 @@ impl<'block> BrilligBlock<'block> { dfg, ); - self.convert_ssa_as_slice( - source_variable, - destination_variable, - ); + self.convert_ssa_as_slice(source_variable, destination_variable); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1328,7 +1325,7 @@ impl<'block> BrilligBlock<'block> { Value::Param { .. } | Value::Instruction { .. } => { // All block parameters and instruction results should have already been // converted to registers so we fetch from the cache. - + self.variables.get_allocation(self.function_context, value_id, dfg) } Value::NumericConstant { constant, typ } => { From b6feea0f3e593de51afeee73fbd28f26d5e50e7a Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 13:37:38 -0400 Subject: [PATCH 12/16] fix destination length init in brillig as_slice implementation, split out brillig and regular as_slice tests, cargo fmt/clippy --- .../src/brillig/brillig_gen/brillig_block.rs | 16 ++++++++++----- .../array_to_slice/src/main.nr | 20 +++---------------- .../brillig_array_to_slice/Nargo.toml | 7 +++++++ .../brillig_array_to_slice/Prover.toml | 1 + .../brillig_array_to_slice/src/main.nr | 18 +++++++++++++++++ 5 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 test_programs/execution_success/brillig_array_to_slice/Nargo.toml create mode 100644 test_programs/execution_success/brillig_array_to_slice/Prover.toml create mode 100644 test_programs/execution_success/brillig_array_to_slice/src/main.nr diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 505fb471e8b..67daf1b7b8f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -456,9 +456,7 @@ impl<'block> BrilligBlock<'block> { Value::Intrinsic(Intrinsic::AsSlice) => { let source_variable = self.convert_ssa_value(arguments[0], dfg); let result_ids = dfg.instruction_results(instruction_id); - - // this needs to be initialized for convert_ssa_as_slice - let _len_variable = self.variables.define_variable( + let destination_len_variable = self.variables.define_single_addr_variable( self.function_context, self.brillig_context, result_ids[0], @@ -470,8 +468,11 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - - self.convert_ssa_as_slice(source_variable, destination_variable); + self.convert_ssa_as_slice( + source_variable, + destination_len_variable, + destination_variable, + ); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -1515,6 +1516,7 @@ impl<'block> BrilligBlock<'block> { fn convert_ssa_as_slice( &mut self, source_variable: BrilligVariable, + destination_len_variable: SingleAddrVariable, destination_variable: BrilligVariable, ) { let destination_pointer = match destination_variable { @@ -1543,6 +1545,10 @@ impl<'block> BrilligBlock<'block> { _ => unreachable!("ICE: as_slice on non-array"), }; + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + let one = self.brillig_context.make_usize_constant(1_usize.into()); let condition = self.brillig_context.allocate_register(); diff --git a/test_programs/execution_success/array_to_slice/src/main.nr b/test_programs/execution_success/array_to_slice/src/main.nr index 8fb7aa8d818..4f5594c6d11 100644 --- a/test_programs/execution_success/array_to_slice/src/main.nr +++ b/test_programs/execution_success/array_to_slice/src/main.nr @@ -7,41 +7,27 @@ fn as_slice_push(xs: [T; N]) -> [T] { slice } -unconstrained fn brillig_as_slice(x: Field) -> Field { - let mut dynamic: [Field; 1] = [1]; - dynamic[x] = 2; - assert(dynamic[0] == 2); - - let brillig_slice = dynamic.as_slice(); - assert(brillig_slice.len() == 1); - - brillig_slice[0] -} - fn main(x: Field, y: pub Field) { let xs: [Field; 0] = []; let ys: [Field; 1] = [1]; let zs: [Field; 2] = [1, 2]; let ws: [Field; 3] = [1; 3]; let qs: [Field; 4] = [3, 2, 1, 0]; - + let mut dynamic: [Field; 4] = [3, 2, 1, 0]; let dynamic_expected: [Field; 4] = [1000, 2, 1, 0]; dynamic[x] = 1000; - + assert(x != y); assert(xs.as_slice() == as_slice_push(xs)); assert(ys.as_slice() == as_slice_push(ys)); assert(zs.as_slice() == as_slice_push(zs)); assert(ws.as_slice() == as_slice_push(ws)); assert(qs.as_slice() == as_slice_push(qs)); - + assert(dynamic.as_slice()[0] == dynamic_expected[0]); assert(dynamic.as_slice()[1] == dynamic_expected[1]); assert(dynamic.as_slice()[2] == dynamic_expected[2]); assert(dynamic.as_slice()[3] == dynamic_expected[3]); assert(dynamic.as_slice().len() == 4); - - let res = brillig_as_slice(x); - assert(res == 2); } diff --git a/test_programs/execution_success/brillig_array_to_slice/Nargo.toml b/test_programs/execution_success/brillig_array_to_slice/Nargo.toml new file mode 100644 index 00000000000..58157c38c26 --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_array_to_slice" +type = "bin" +authors = [""] +compiler_version = ">=0.25.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/brillig_array_to_slice/Prover.toml b/test_programs/execution_success/brillig_array_to_slice/Prover.toml new file mode 100644 index 00000000000..11497a473bc --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/Prover.toml @@ -0,0 +1 @@ +x = "0" diff --git a/test_programs/execution_success/brillig_array_to_slice/src/main.nr b/test_programs/execution_success/brillig_array_to_slice/src/main.nr new file mode 100644 index 00000000000..8f7fcf24bae --- /dev/null +++ b/test_programs/execution_success/brillig_array_to_slice/src/main.nr @@ -0,0 +1,18 @@ +unconstrained fn brillig_as_slice(x: Field) -> (u64, Field, Field) { + let mut dynamic: [Field; 1] = [1]; + dynamic[x] = 2; + assert(dynamic[0] == 2); + + let brillig_slice = dynamic.as_slice(); + assert(brillig_slice.len() == 1); + + (brillig_slice.len(), dynamic[0], brillig_slice[0]) +} + +fn main(x: Field) { + let (slice_len, dynamic_0, slice_0) = brillig_as_slice(x); + assert(slice_len == 1); + assert(dynamic_0 == 2); + assert(slice_0 == 2); +} + From 269bdda12f2eb2d2585fd431bb8db617e840a604 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 14:13:19 -0400 Subject: [PATCH 13/16] combine convert_ssa_as_slice with convert_ssa_array_set to pull in reference counting changes on master, cargo clippy / fmt --- .../src/brillig/brillig_gen/brillig_block.rs | 128 ++++-------------- 1 file changed, 28 insertions(+), 100 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 0e368e0d42f..02e8a494d4a 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -470,10 +470,10 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - self.convert_ssa_as_slice( + self.convert_ssa_array_set( source_variable, - destination_len_variable, destination_variable, + Err(destination_len_variable), ); } Value::Intrinsic( @@ -639,8 +639,7 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_array_set( source_variable, destination_variable, - index_register.address, - value_variable, + Ok((index_register.address, value_variable)), ); } Instruction::RangeCheck { value, max_bit_size, assert_message } => { @@ -819,23 +818,27 @@ impl<'block> BrilligBlock<'block> { /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. + /// + /// Skip setting a value and store the resulting length in the given SingleAddrVariable if + /// provided fn convert_ssa_array_set( &mut self, source_variable: BrilligVariable, destination_variable: BrilligVariable, - index_register: MemoryAddress, - value_variable: BrilligVariable, + index_and_value_or_len: Result<(MemoryAddress, BrilligVariable), SingleAddrVariable>, ) { + let method_str = if index_and_value_or_len.is_ok() { "array set" } else { "as_slice" }; + let destination_pointer = match destination_variable { BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: array set returns non-array"), + _ => unreachable!("ICE: {method_str} returns non-array"), }; let reference_count = match source_variable { BrilligVariable::BrilligArray(BrilligArray { rc, .. }) | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), }; let (source_pointer, source_size_as_register) = match source_variable { @@ -849,7 +852,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(source_size_register, size); (pointer, source_size_register) } - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), }; // Here we want to compare the reference count against 1. @@ -892,15 +895,24 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(target_size, source_size_as_register); self.brillig_context.usize_const(target_rc, 1_usize.into()); } - _ => unreachable!("ICE: array set on non-array"), + _ => unreachable!("ICE: {method_str} on non-array"), } - // Then set the value in the newly created array - self.store_variable_in_array( - destination_pointer, - SingleAddrVariable::new_usize(index_register), - value_variable, - ); + match index_and_value_or_len { + Ok((index_register, value_variable)) => { + // Then set the value in the newly created array + self.store_variable_in_array( + destination_pointer, + SingleAddrVariable::new_usize(index_register), + value_variable, + ); + } + Err(destination_len_variable) => { + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + } + } self.brillig_context.deallocate_register(source_size_as_register); self.brillig_context.deallocate_register(condition); @@ -1510,90 +1522,6 @@ impl<'block> BrilligBlock<'block> { } } } - - /// Gets the "user-facing" slice from an array - /// An array of structs with two fields would be stored as an 2 * array.len() array/vector. - fn convert_ssa_as_slice( - &mut self, - source_variable: BrilligVariable, - destination_len_variable: SingleAddrVariable, - destination_variable: BrilligVariable, - ) { - let destination_pointer = match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, - BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: as_slice returns non-array"), - }; - - let reference_count = match source_variable { - BrilligVariable::BrilligArray(BrilligArray { rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - _ => unreachable!("ICE: as_slice on non-array"), - }; - - let (source_pointer, source_size_as_register) = match source_variable { - BrilligVariable::BrilligArray(BrilligArray { size, pointer, rc: _ }) => { - let source_size_register = self.brillig_context.allocate_register(); - self.brillig_context.usize_const(source_size_register, size.into()); - (pointer, source_size_register) - } - BrilligVariable::BrilligVector(BrilligVector { size, pointer, rc: _ }) => { - let source_size_register = self.brillig_context.allocate_register(); - self.brillig_context.mov_instruction(source_size_register, size); - (pointer, source_size_register) - } - _ => unreachable!("ICE: as_slice on non-array"), - }; - - // we need to explicitly set the destination_len_variable - self.brillig_context - .mov_instruction(destination_len_variable.address, source_size_as_register); - - let one = self.brillig_context.make_usize_constant(1_usize.into()); - let condition = self.brillig_context.allocate_register(); - - self.brillig_context.binary_instruction( - reference_count, - one, - condition, - BrilligBinaryOp::Field { op: BinaryFieldOp::Equals }, - ); - - self.brillig_context.branch_instruction(condition, |ctx, cond| { - if cond { - // Reference count is 1, we can mutate the array directly - ctx.mov_instruction(destination_pointer, source_pointer); - } else { - // First issue a array copy to the destination - ctx.allocate_array_instruction(destination_pointer, source_size_as_register); - - ctx.copy_array_instruction( - source_pointer, - destination_pointer, - source_size_as_register, - ); - } - }); - - match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { rc: target_rc, .. }) => { - self.brillig_context.usize_const(target_rc, 1_usize.into()); - } - BrilligVariable::BrilligVector(BrilligVector { - size: target_size, - rc: target_rc, - .. - }) => { - self.brillig_context.mov_instruction(target_size, source_size_as_register); - self.brillig_context.usize_const(target_rc, 1_usize.into()); - } - _ => unreachable!("ICE: as_slice on non-array"), - } - - self.brillig_context.deallocate_register(source_size_as_register); - self.brillig_context.deallocate_register(one); - self.brillig_context.deallocate_register(condition); - } } /// Returns the type of the operation considering the types of the operands From f66772b0ed3f90744f54b05c907e89c461d04aa4 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 13 Mar 2024 15:15:54 -0400 Subject: [PATCH 14/16] move as_slice logic out of convert_ssa_array_set --- .../src/brillig/brillig_gen/brillig_block.rs | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 02e8a494d4a..a1d05972089 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -470,11 +470,17 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - self.convert_ssa_array_set( + let source_size_as_register = self.convert_ssa_array_set( source_variable, destination_variable, - Err(destination_len_variable), + None, + "as_slice".to_string(), ); + + // we need to explicitly set the destination_len_variable + self.brillig_context + .mov_instruction(destination_len_variable.address, source_size_as_register); + self.brillig_context.deallocate_register(source_size_as_register); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -635,12 +641,13 @@ impl<'block> BrilligBlock<'block> { dfg, ); self.validate_array_index(source_variable, index_register); - - self.convert_ssa_array_set( + let source_size_as_register = self.convert_ssa_array_set( source_variable, destination_variable, - Ok((index_register.address, value_variable)), + Some((index_register.address, value_variable)), + "array set".to_string(), ); + self.brillig_context.deallocate_register(source_size_as_register); } Instruction::RangeCheck { value, max_bit_size, assert_message } => { let value = self.convert_ssa_single_addr_value(*value, dfg); @@ -819,16 +826,15 @@ impl<'block> BrilligBlock<'block> { /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. /// - /// Skip setting a value and store the resulting length in the given SingleAddrVariable if - /// provided + /// Returns `source_size_as_register`, which is expected to be deallocated with: + /// `self.brillig_context.deallocate_register(source_size_as_register)` fn convert_ssa_array_set( &mut self, source_variable: BrilligVariable, destination_variable: BrilligVariable, - index_and_value_or_len: Result<(MemoryAddress, BrilligVariable), SingleAddrVariable>, - ) { - let method_str = if index_and_value_or_len.is_ok() { "array set" } else { "as_slice" }; - + opt_index_and_value: Option<(MemoryAddress, BrilligVariable)>, + method_str: String, + ) -> MemoryAddress { let destination_pointer = match destination_variable { BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, @@ -898,24 +904,17 @@ impl<'block> BrilligBlock<'block> { _ => unreachable!("ICE: {method_str} on non-array"), } - match index_and_value_or_len { - Ok((index_register, value_variable)) => { - // Then set the value in the newly created array - self.store_variable_in_array( - destination_pointer, - SingleAddrVariable::new_usize(index_register), - value_variable, - ); - } - Err(destination_len_variable) => { - // we need to explicitly set the destination_len_variable - self.brillig_context - .mov_instruction(destination_len_variable.address, source_size_as_register); - } + if let Some((index_register, value_variable)) = opt_index_and_value { + // Then set the value in the newly created array + self.store_variable_in_array( + destination_pointer, + SingleAddrVariable::new_usize(index_register), + value_variable, + ); } - self.brillig_context.deallocate_register(source_size_as_register); self.brillig_context.deallocate_register(condition); + source_size_as_register } pub(crate) fn store_variable_in_array_with_ctx( From 0f332185b24f2d2afb0e110670f41795f79104f7 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 15 Mar 2024 15:23:34 -0400 Subject: [PATCH 15/16] remove method string from ssa generation function (panic will unwind stack) --- .../src/brillig/brillig_gen/brillig_block.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 54f7f413134..99400b13528 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -468,12 +468,8 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); - let source_size_as_register = self.convert_ssa_array_set( - source_variable, - destination_variable, - None, - "as_slice".to_string(), - ); + let source_size_as_register = + self.convert_ssa_array_set(source_variable, destination_variable, None); // we need to explicitly set the destination_len_variable self.brillig_context @@ -643,7 +639,6 @@ impl<'block> BrilligBlock<'block> { source_variable, destination_variable, Some((index_register.address, value_variable)), - "array set".to_string(), ); self.brillig_context.deallocate_register(source_size_as_register); } @@ -841,18 +836,17 @@ impl<'block> BrilligBlock<'block> { source_variable: BrilligVariable, destination_variable: BrilligVariable, opt_index_and_value: Option<(MemoryAddress, BrilligVariable)>, - method_str: String, ) -> MemoryAddress { let destination_pointer = match destination_variable { BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: {method_str} returns non-array"), + _ => unreachable!("ICE: array_set SSA returns non-array"), }; let reference_count = match source_variable { BrilligVariable::BrilligArray(BrilligArray { rc, .. }) | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - _ => unreachable!("ICE: {method_str} on non-array"), + _ => unreachable!("ICE: array_set SSA on non-array"), }; let (source_pointer, source_size_as_register) = match source_variable { @@ -866,7 +860,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(source_size_register, size); (pointer, source_size_register) } - _ => unreachable!("ICE: {method_str} on non-array"), + _ => unreachable!("ICE: array_set SSA on non-array"), }; // Here we want to compare the reference count against 1. @@ -909,7 +903,7 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(target_size, source_size_as_register); self.brillig_context.usize_const(target_rc, 1_usize.into()); } - _ => unreachable!("ICE: {method_str} on non-array"), + _ => unreachable!("ICE: array_set SSA on non-array"), } if let Some((index_register, value_variable)) = opt_index_and_value { From c1ec91f9caaec6a9a1d3c5bb582762ed1efcebf4 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Fri, 15 Mar 2024 16:19:56 -0400 Subject: [PATCH 16/16] Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs Co-authored-by: Maxim Vezenov --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e9861bbbd82..5a4fa021f1f 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1253,7 +1253,7 @@ impl Context { Ok::(AcirValue::Var(read, AcirType::field())) })?; let array: im::Vector = init_values.into(); - self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?; + self.initialize_array(destination, array_len, Some(AcirValue::Array(array)))?; Ok(()) }