diff --git a/acvm-repo/acir/src/native_types/expression/mod.rs b/acvm-repo/acir/src/native_types/expression/mod.rs index bcbc1aeaced..84e929e1bd9 100644 --- a/acvm-repo/acir/src/native_types/expression/mod.rs +++ b/acvm-repo/acir/src/native_types/expression/mod.rs @@ -166,6 +166,10 @@ impl Expression { Self::from_field(F::one()) } + pub fn is_one(&self) -> bool { + *self == Self::one() + } + /// Returns a `Witness` if the `Expression` can be represented as a degree-1 /// univariate polynomial. Otherwise returns `None`. /// diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 418653b5aef..c2f1fd54548 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -152,12 +152,12 @@ impl Context<'_> { /// only be computed dynamically, such as the type structure /// of non-homogenous arrays. fn internal_block_id(&mut self, value: &ValueId) -> BlockId { - if let Some(block_id) = self.internal_memory_blocks.get(value) { + if let Some(block_id) = self.element_type_sizes_blocks.get(value) { return *block_id; } let block_id = BlockId(self.max_block_id); self.max_block_id += 1; - self.internal_memory_blocks.insert(*value, block_id); + self.element_type_sizes_blocks.insert(*value, block_id); block_id } @@ -815,7 +815,6 @@ impl Context<'_> { Some(AcirValue::Array(init_values.into())), )?; - self.internal_mem_block_lengths.insert(element_type_sizes, element_type_sizes_len); Ok(element_type_sizes) } @@ -836,19 +835,10 @@ impl Context<'_> { unreachable!("ICE: element type size arrays are expected to be initialized"); } - let type_sizes_array_len = *self.internal_mem_block_lengths.get(inner_elem_type_sizes).ok_or_else(|| - InternalError::General { - message: format!("Array {array_id}'s inner element type sizes array does not have a tracked length"), - call_stack: self.acir_context.get_call_stack(), - } - )?; - self.copy_dynamic_array( - *inner_elem_type_sizes, - element_type_sizes, - type_sizes_array_len, - )?; - self.internal_mem_block_lengths.insert(element_type_sizes, type_sizes_array_len); - Ok(element_type_sizes) + // We can safely overwrite the memory block from the initial call to `self.internal_block_id(&array_id)` here. + // The type sizes array is never mutated so we can re-use it. + self.element_type_sizes_blocks.insert(array_id, *inner_elem_type_sizes); + Ok(*inner_elem_type_sizes) } _ => Err(InternalError::Unexpected { expected: "AcirValue::DynamicArray or AcirValue::Array".to_owned(), diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 47ab91c19ea..5eb9bf2e170 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -85,21 +85,14 @@ struct Context<'a> { /// Each acir memory block corresponds to a different SSA array. memory_blocks: HashMap, BlockId>, - /// Maps SSA values to a BlockId used internally + /// Maps SSA values to a BlockId used internally for computing the accurate flattened + /// index of non-homogenous arrays. + /// See [arrays] for more information about the purpose of the type sizes array. + /// /// A BlockId is an ACIR structure which identifies a memory block /// Each memory blocks corresponds to a different SSA value /// which utilizes this internal memory for ACIR generation. - internal_memory_blocks: HashMap, BlockId>, - - /// Maps an internal memory block to its length - /// - /// This is necessary to keep track of an internal memory block's size. - /// We do not need a separate map to keep track of `memory_blocks` as - /// the length is set when we construct a `AcirValue::DynamicArray` and is tracked - /// as part of the `AcirValue` in the `ssa_values` map. - /// The length of an internal memory block is determined before an array operation - /// takes place thus we track it separate here in this map. - internal_mem_block_lengths: HashMap, + element_type_sizes_blocks: HashMap, BlockId>, /// Number of the next BlockId, it is used to construct /// a new BlockId @@ -134,8 +127,7 @@ impl<'a> Context<'a> { acir_context, initialized_arrays: HashSet::default(), memory_blocks: HashMap::default(), - internal_memory_blocks: HashMap::default(), - internal_mem_block_lengths: HashMap::default(), + element_type_sizes_blocks: HashMap::default(), max_block_id: 0, data_bus: DataBus::default(), shared_context, @@ -256,6 +248,9 @@ impl<'a> Context<'a> { warnings.extend(return_warnings); warnings.extend(self.acir_context.warnings.clone()); + #[cfg(debug_assertions)] + acir_post_check(&self, &self.acir_context.acir_ir); + // Add the warnings from the alter Ssa passes Ok(self.acir_context.finish( input_witness, @@ -945,3 +940,25 @@ impl<'a> Context<'a> { self.acir_context.truncate_var(var, bit_size, max_bit_size) } } + +/// Check post ACIR generation properties +/// * No memory opcodes should be laid down that write to the internal type sizes array. +/// See [arrays] for more information on the type sizes array. +#[cfg(debug_assertions)] +fn acir_post_check(context: &Context<'_>, acir: &GeneratedAcir) { + use acvm::acir::circuit::Opcode; + for opcode in acir.opcodes() { + let Opcode::MemoryOp { block_id, op } = opcode else { + continue; + }; + if op.operation.is_one() { + // Check that we have no writes to the type size arrays + let is_type_sizes_array = + context.element_type_sizes_blocks.values().any(|id| id == block_id); + assert!( + !is_type_sizes_array, + "ICE: Writes to the internal type sizes array are forbidden" + ); + } + } +}