Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions acvm-repo/acir/src/native_types/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ impl<F: AcirField> Expression<F> {
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`.
///
Expand Down
22 changes: 6 additions & 16 deletions compiler/noirc_evaluator/src/acir/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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(),
Expand Down
45 changes: 31 additions & 14 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,14 @@ struct Context<'a> {
/// Each acir memory block corresponds to a different SSA array.
memory_blocks: HashMap<Id<Value>, 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<Id<Value>, 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<BlockId, usize>,
element_type_sizes_blocks: HashMap<Id<Value>, BlockId>,

/// Number of the next BlockId, it is used to construct
/// a new BlockId
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<FieldElement>) {
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"
);
}
}
}
Loading