diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 0c7427dc228..ad386b6e6df 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -14,7 +14,7 @@ use super::{ }, map::DenseMap, types::{NumericType, Type}, - value::{Value, ValueId}, + value::{Value, ValueId, resolve_value}, }; use acvm::{FieldElement, acir::AcirField}; @@ -383,47 +383,16 @@ impl DataFlowGraph { self.instructions[id] = instruction; } - /// Replaces all values in the given blocks with the values in the given map. - /// - /// This method should be preferred over `set_value_from_id` which might eventually be removed. - pub(crate) fn replace_values_in_blocks( + /// Replaces values in the given block terminator (if it has any) according to the given HashMap. + pub(crate) fn replace_values_in_block_terminator( &mut self, - blocks: impl Iterator, + block: BasicBlockId, values_to_replace: &HashMap, ) { - if values_to_replace.is_empty() { - return; - } - - let replacement_fn = |value_id| { - if let Some(replacement_id) = values_to_replace.get(&value_id) { - *replacement_id - } else { - value_id - } - }; - - for block in blocks { - // Replace in all the block's instructions - for instruction_id in self.blocks[block].instructions() { - let instruction = &mut self.instructions[*instruction_id]; - instruction.map_values_mut(replacement_fn); - - // Make sure we also replace the instruction results - let results = self.results.get_mut(instruction_id); - if let Some(results) = results { - for result in results { - if let Some(replacement_id) = values_to_replace.get(result) { - *result = *replacement_id; - } - } - } - } - - // Finally, the value might show up in a terminator - if self[block].terminator().is_some() { - self[block].unwrap_terminator_mut().map_values_mut(replacement_fn); - } + if self[block].terminator().is_some() { + self[block] + .unwrap_terminator_mut() + .map_values_mut(|value_id| resolve_value(values_to_replace, value_id)); } } @@ -603,30 +572,6 @@ impl DataFlowGraph { matches!(self.values[value].get_type().as_ref(), Type::Reference(_)) } - /// Replaces an instruction result with a fresh id. - pub(crate) fn replace_result( - &mut self, - instruction_id: InstructionId, - prev_value_id: ValueId, - ) -> ValueId { - let typ = self.type_of_value(prev_value_id); - let results = self.results.get_mut(&instruction_id).unwrap(); - let res_position = results - .iter() - .position(|&id| id == prev_value_id) - .expect("Result id not found while replacing"); - - let value_id = self.values.insert(Value::Instruction { - typ, - position: res_position, - instruction: instruction_id, - }); - - // Replace the value in list of results for this instruction - results[res_position] = value_id; - value_id - } - /// Returns all of result values which are attached to this instruction. pub(crate) fn instruction_results(&self, instruction_id: InstructionId) -> &[ValueId] { self.results.get(&instruction_id).expect("expected a list of Values").as_slice() diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index ee4adfe89e2..ce6eb6457fe 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -1,3 +1,4 @@ +use fxhash::FxHashMap as HashMap; use std::hash::{Hash, Hasher}; use acvm::acir::{BlackBoxFunc, circuit::ErrorSelector}; @@ -14,7 +15,7 @@ use super::{ dfg::DataFlowGraph, map::Id, types::{NumericType, Type}, - value::{Value, ValueId}, + value::{Value, ValueId, resolve_value}, }; pub(crate) mod binary; @@ -422,6 +423,13 @@ impl Instruction { } } + /// Replaces values present in this instruction with other values according to the given HashMap. + pub(crate) fn replace_values(&mut self, values_to_replace: &HashMap) { + if !values_to_replace.is_empty() { + self.map_values_mut(|value_id| resolve_value(values_to_replace, value_id)); + } + } + /// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction. /// Note that the returned instruction is fresh and will not have an assigned InstructionId /// until it is manually inserted in a DataFlowGraph later. diff --git a/compiler/noirc_evaluator/src/ssa/ir/value.rs b/compiler/noirc_evaluator/src/ssa/ir/value.rs index b6e9d987576..f2236964015 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/value.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/value.rs @@ -1,3 +1,4 @@ +use fxhash::FxHashMap as HashMap; use std::borrow::Cow; use acvm::FieldElement; @@ -71,3 +72,14 @@ impl Value { } } } + +pub(super) fn resolve_value( + values_to_replace: &HashMap, + value_id: ValueId, +) -> ValueId { + if let Some(replacement_id) = values_to_replace.get(&value_id) { + resolve_value(values_to_replace, *replacement_id) + } else { + value_id + } +} diff --git a/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs b/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs index 837a35126ac..29685ac9f90 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs @@ -1,7 +1,7 @@ use crate::ssa::{ ir::{ function::Function, - instruction::{Instruction, InstructionId, Intrinsic}, + instruction::{Instruction, Intrinsic}, types::{NumericType, Type}, value::Value, }, @@ -28,52 +28,45 @@ impl Ssa { impl Function { pub(crate) fn as_slice_optimization(&mut self) { - let known_slice_lengths = known_slice_lengths(self); - replace_known_slice_lengths(self, known_slice_lengths); - } -} + let mut values_to_replace = HashMap::default(); + + for block in self.reachable_blocks() { + let instruction_ids = self.dfg[block].take_instructions(); + for instruction_id in &instruction_ids { + let (target_func, first_argument) = { + let instruction = &mut self.dfg[*instruction_id]; + instruction.replace_values(&values_to_replace); -fn known_slice_lengths(func: &Function) -> HashMap { - let mut known_slice_lengths = HashMap::default(); - for block_id in func.reachable_blocks() { - let block = &func.dfg[block_id]; - for instruction_id in block.instructions() { - let (target_func, arguments) = match &func.dfg[*instruction_id] { - Instruction::Call { func, arguments } => (func, arguments), - _ => continue, - }; + let (target_func, arguments) = match &instruction { + Instruction::Call { func, arguments } => (func, arguments), + _ => continue, + }; - match &func.dfg[*target_func] { - Value::Intrinsic(Intrinsic::AsSlice) => { - let array_typ = func.dfg.type_of_value(arguments[0]); - if let Type::Array(_, length) = array_typ { - known_slice_lengths.insert(*instruction_id, length); - } else { - unreachable!("AsSlice called with non-array {}", array_typ); + (*target_func, arguments.first().copied()) + }; + + match &self.dfg[target_func] { + Value::Intrinsic(Intrinsic::AsSlice) => { + let first_argument = first_argument.unwrap(); + let array_typ = self.dfg.type_of_value(first_argument); + if let Type::Array(_, length) = array_typ { + let call_returns = self.dfg.instruction_results(*instruction_id); + let original_slice_length = call_returns[0]; + let known_length = + self.dfg.make_constant(length.into(), NumericType::length_type()); + values_to_replace.insert(original_slice_length, known_length); + } else { + unreachable!("AsSlice called with non-array {}", array_typ); + } } - } - _ => continue, - }; + _ => continue, + }; + } + + *self.dfg[block].instructions_mut() = instruction_ids; + self.dfg.replace_values_in_block_terminator(block, &values_to_replace); } } - known_slice_lengths -} - -fn replace_known_slice_lengths( - func: &mut Function, - known_slice_lengths: HashMap, -) { - known_slice_lengths.into_iter().for_each(|(instruction_id, known_length)| { - let call_returns = func.dfg.instruction_results(instruction_id); - let original_slice_length = call_returns[0]; - - // We won't use the new id for the original unknown length. - // This isn't strictly necessary as a new result will be defined the next time for which the instruction - // is reinserted but this avoids leaving the program in an invalid state. - func.dfg.replace_result(instruction_id, original_slice_length); - let known_length = func.dfg.make_constant(known_length.into(), NumericType::length_type()); - func.dfg.set_value_from_id(original_slice_length, known_length); - }); } #[cfg(test)] diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs index 9273453d6cb..58316eda686 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs @@ -29,10 +29,15 @@ impl Function { for block in &blocks { let block = *block; let mut instructions_to_remove = HashSet::default(); + let mut instruction_ids = self.dfg[block].take_instructions(); - for instruction_id in self.dfg[block].instructions() { - let instruction = &self.dfg[*instruction_id]; + for instruction_id in &instruction_ids { + if !values_to_replace.is_empty() { + let instruction = &mut self.dfg[*instruction_id]; + instruction.replace_values(&values_to_replace); + } + let instruction = &self.dfg[*instruction_id]; match instruction { // If this is a range_check instruction, associate the max bit size with the value Instruction::RangeCheck { value, max_bit_size, .. } => { @@ -66,16 +71,12 @@ impl Function { } } - if instructions_to_remove.is_empty() { - continue; + if !instructions_to_remove.is_empty() { + instruction_ids.retain(|instruction| !instructions_to_remove.contains(instruction)); } - - self.dfg[block] - .instructions_mut() - .retain(|instruction| !instructions_to_remove.contains(instruction)); + *self.dfg[block].instructions_mut() = instruction_ids; + self.dfg.replace_values_in_block_terminator(block, &values_to_replace); } - - self.dfg.replace_values_in_blocks(blocks.into_iter(), &values_to_replace); } }