From f3f6fbe45254ea206b778d191861498eef880064 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Wed, 2 Aug 2023 21:38:28 +0200 Subject: [PATCH] chore: Decouple acir blockid from ssa valueid (#2103) Decouple acir blokid from ssa valueid Co-authored-by: Tom French --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs index f1e71922b0b..25a0c2ee2e8 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -55,6 +55,15 @@ struct Context { /// This set is used to ensure that a MemoryOp opcode is only pushed to the circuit /// if there is already a MemoryInit opcode. initialized_arrays: HashSet, + + /// Maps SSA values to BlockId + /// A BlockId is an ACIR structure which identifies a memory block + /// Each acir memory block corresponds to a different SSA array. + memory_blocks: HashMap, BlockId>, + + /// Number of the next BlockId, it is used to construct + /// a new BlockId + max_block_id: u32, } #[derive(Clone)] @@ -139,6 +148,8 @@ impl Context { current_side_effects_enabled_var, acir_context, initialized_arrays: HashSet::new(), + memory_blocks: HashMap::new(), + max_block_id: 0, } } @@ -221,7 +232,7 @@ impl Context { match &value { AcirValue::Var(_, _) => (), AcirValue::Array(values) => { - let block_id = BlockId(param_id.to_usize() as u32); + let block_id = self.block_id(param_id); let v = vecmap(values, |v| v.clone()); self.initialize_array(block_id, values.len(), Some(&v))?; } @@ -264,6 +275,18 @@ impl Context { } } + /// Get the BlockId corresponding to the ValueId + /// If there is no matching BlockId, we create a new one. + fn block_id(&mut self, value: &ValueId) -> BlockId { + if let Some(block_id) = self.memory_blocks.get(value) { + return *block_id; + } + let block_id = BlockId(self.max_block_id); + self.max_block_id += 1; + self.memory_blocks.insert(*value, block_id); + block_id + } + /// Creates an `AcirVar` corresponding to a parameter witness to appears in the abi. A range /// constraint is added if the numeric type requires it. /// @@ -500,7 +523,7 @@ impl Context { dfg: &DataFlowGraph, ) -> Result<(), RuntimeError> { let array = dfg.resolve(array); - let block_id = BlockId(array.to_usize() as u32); + let block_id = self.block_id(&array); if !self.initialized_arrays.contains(&block_id) { match &dfg[array] { Value::Array { array, .. } => { @@ -548,11 +571,9 @@ impl Context { ) -> Result<(), InternalError> { // Fetch the internal SSA ID for the array let array = dfg.resolve(array); - let array_ssa_id = array.to_usize() as u32; - // Use the SSA ID to create a block ID - // There is currently a 1-1 mapping from array SSA ID to block ID - let block_id = BlockId(array_ssa_id); + // Use the SSA ID to get or create its block ID + let block_id = self.block_id(&array); // Every array has a length in its type, so we fetch that from // the SSA IR. @@ -586,8 +607,7 @@ impl Context { .instruction_results(instruction) .first() .expect("Array set does not have one result"); - let result_array_id = result_id.to_usize() as u32; - let result_block_id = BlockId(result_array_id); + let result_block_id = self.block_id(result_id); // Initialize the new array with the values from the old array let init_values = try_vecmap(0..len, |i| {