diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 55c11e06ca3..16334052d27 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -957,13 +957,39 @@ impl<'a> Context<'a> { return Ok(()); } - let (new_index, new_value) = - self.convert_array_operation_inputs(array, dfg, index, store_value)?; + // Get an offset such that the type of the array at the offset is the same as the type at the 'index' + // If we find one, we will use it when computing the index under the enable_side_effect predicate + // If not, array_get(..) will use a fallback costing one multiplication in the worst case. + // cf. https://github.com/noir-lang/noir/pull/4971 + let array_id = dfg.resolve(array); + let array_typ = dfg.type_of_value(array_id); + // For simplicity we compute the offset only for simple arrays + let is_simple_array = dfg.instruction_results(instruction).len() == 1 + && can_omit_element_sizes_array(&array_typ); + let offset = if is_simple_array { + let result_type = dfg.type_of_value(dfg.instruction_results(instruction)[0]); + match array_typ { + Type::Array(item_type, _) | Type::Slice(item_type) => item_type + .iter() + .enumerate() + .find_map(|(index, typ)| (result_type == *typ).then_some(index)), + _ => None, + } + } else { + None + }; + let (new_index, new_value) = self.convert_array_operation_inputs( + array, + dfg, + index, + store_value, + offset.unwrap_or_default(), + )?; if let Some(new_value) = new_value { self.array_set(instruction, new_index, new_value, dfg, mutable_array_set)?; } else { - self.array_get(instruction, array, new_index, dfg)?; + self.array_get(instruction, array, new_index, dfg, offset.is_none())?; } Ok(()) @@ -1053,7 +1079,7 @@ impl<'a> Context<'a> { /// - new_index is the index of the array. ACIR memory operations work with a flat memory, so we fully flattened the specified index /// in case we have a nested array. The index for SSA array operations only represents the flattened index of the current array. /// Thus internal array element type sizes need to be computed to accurately transform the index. - /// - predicate_index is 0, or the index if the predicate is true + /// - predicate_index is offset, or the index if the predicate is true /// - new_value is the optional value when the operation is an array_set /// When there is a predicate, it is predicate*value + (1-predicate)*dummy, where dummy is the value of the array at the requested index. /// It is a dummy value because in the case of a false predicate, the value stored at the requested index will be itself. @@ -1063,14 +1089,18 @@ impl<'a> Context<'a> { dfg: &DataFlowGraph, index: ValueId, store_value: Option, + offset: usize, ) -> Result<(AcirVar, Option), RuntimeError> { let (array_id, array_typ, block_id) = self.check_array_is_initialized(array, dfg)?; let index_var = self.convert_numeric_value(index, dfg)?; let index_var = self.get_flattened_index(&array_typ, array_id, index_var, dfg)?; - let predicate_index = - self.acir_context.mul_var(index_var, self.current_side_effects_enabled_var)?; + // predicate_index = index*predicate + (1-predicate)*offset + let offset = self.acir_context.add_constant(offset); + let sub = self.acir_context.sub_var(index_var, offset)?; + let pred = self.acir_context.mul_var(sub, self.current_side_effects_enabled_var)?; + let predicate_index = self.acir_context.add_var(pred, offset)?; let new_value = if let Some(store) = store_value { let store_value = self.convert_value(store, dfg); @@ -1171,12 +1201,14 @@ impl<'a> Context<'a> { } /// Generates a read opcode for the array + /// `index_side_effect == false` means that we ensured `var_index` will have a type matching the value in the array fn array_get( &mut self, instruction: InstructionId, array: ValueId, mut var_index: AcirVar, dfg: &DataFlowGraph, + mut index_side_effect: bool, ) -> Result { let (array_id, _, block_id) = self.check_array_is_initialized(array, dfg)?; let results = dfg.instruction_results(instruction); @@ -1195,7 +1227,7 @@ impl<'a> Context<'a> { self.data_bus.call_data_map[&array_id] as i128, )); let new_index = self.acir_context.add_var(offset, bus_index)?; - return self.array_get(instruction, call_data, new_index, dfg); + return self.array_get(instruction, call_data, new_index, dfg, index_side_effect); } } @@ -1204,7 +1236,28 @@ impl<'a> Context<'a> { !res_typ.contains_slice_element(), "ICE: Nested slice result found during ACIR generation" ); - let value = self.array_get_value(&res_typ, block_id, &mut var_index)?; + let mut value = self.array_get_value(&res_typ, block_id, &mut var_index)?; + + if let AcirValue::Var(value_var, typ) = &value { + let array_id = dfg.resolve(array_id); + let array_typ = dfg.type_of_value(array_id); + if let (Type::Numeric(numeric_type), AcirType::NumericType(num)) = + (array_typ.first(), typ) + { + if numeric_type.bit_size() <= num.bit_size() { + // first element is compatible + index_side_effect = false; + } + } + // Fallback to multiplication if the index side_effects have not already been handled + if index_side_effect { + // Set the value to 0 if current_side_effects is 0, to ensure it fits in any value type + value = AcirValue::Var( + self.acir_context.mul_var(*value_var, self.current_side_effects_enabled_var)?, + typ.clone(), + ); + } + } self.define_result(dfg, instruction, value.clone()); diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 48036580d29..d72ad487f66 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -166,6 +166,14 @@ impl Type { other => panic!("element_types: Expected array or slice, found {other}"), } } + + pub(crate) fn first(&self) -> Type { + match self { + Type::Numeric(_) | Type::Function => self.clone(), + Type::Reference(typ) => typ.first(), + Type::Slice(element_types) | Type::Array(element_types, _) => element_types[0].first(), + } + } } /// Composite Types are essentially flattened struct or tuple types. diff --git a/test_programs/execution_success/regression_struct_array_conditional/Nargo.toml b/test_programs/execution_success/regression_struct_array_conditional/Nargo.toml new file mode 100644 index 00000000000..a0587210464 --- /dev/null +++ b/test_programs/execution_success/regression_struct_array_conditional/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_struct_array_conditional" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_struct_array_conditional/Prover.toml b/test_programs/execution_success/regression_struct_array_conditional/Prover.toml new file mode 100644 index 00000000000..ef97f9d482a --- /dev/null +++ b/test_programs/execution_success/regression_struct_array_conditional/Prover.toml @@ -0,0 +1,18 @@ +y = 1 +z = 1 + +[[x]] +value = "0x23de33be058ce5504e1ade738db8bdacfe268fa9dbde777092bf1d38519bdf59" +counter = "10" +dummy = "0" + +[[x]] +value = "3" +counter = "2" +dummy = "0" + +[[x]] +value = "2" +counter = "0" +dummy = "0" + diff --git a/test_programs/execution_success/regression_struct_array_conditional/src/main.nr b/test_programs/execution_success/regression_struct_array_conditional/src/main.nr new file mode 100644 index 00000000000..17502a9fe50 --- /dev/null +++ b/test_programs/execution_success/regression_struct_array_conditional/src/main.nr @@ -0,0 +1,38 @@ +struct foo { + value: Field, + counter: u8, + dummy: u8, +} +struct bar { + dummy: [u8;3], + value: Field, + counter: u8, +} +struct bar_field { + dummy: [Field;3], + value: Field, + counter: u8, +} +fn main(x: [foo; 3], y: u32, z: u32) -> pub u8 { + let a = [y, z, x[y].counter as u32]; + let mut b = [bar { value: 0, counter: 0, dummy: [0; 3] }; 3]; + let mut c = [bar_field { value: 0, counter: 0, dummy: [0; 3] }; 3]; + for i in 0..3 { + b[i].value = x[i].value; + b[i].counter = x[i].counter; + b[i].dummy[0] = x[i].dummy; + c[i].value = x[i].value; + c[i].counter = x[i].counter; + c[i].dummy[0] = x[i].dummy as Field; + } + if z == 0 { + // offset + assert(y as u8 < x[y].counter); + assert(y <= a[y]); + // first element is compatible + assert(y as u8 < b[y].counter); + // fallback + assert(y as u8 < c[y].counter); + } + x[0].counter +}