diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index e559aed2458..664dcaf0003 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -180,10 +180,7 @@ impl<'f> PerFunctionContext<'f> { } let mut all_terminator_values = HashSet::default(); - let mut per_func_block_params: HashSet = HashSet::default(); for (block_id, references) in self.blocks.iter_mut() { - let block_params = self.inserter.function.dfg.block_parameters(*block_id); - per_func_block_params.extend(block_params.iter()); let terminator = self.inserter.function.dfg[*block_id].unwrap_terminator(); terminator.for_each_value(|value| { all_terminator_values.insert(value); @@ -203,12 +200,8 @@ impl<'f> PerFunctionContext<'f> { // This rule does not apply to reference parameters, which we must also check for before removing these stores. for (_, block) in self.blocks.iter() { for (store_address, store_instruction) in block.last_stores.iter() { - let store_alias_used = self.is_store_alias_used( - store_address, - block, - &all_terminator_values, - &per_func_block_params, - ); + let store_alias_used = + self.is_store_alias_used(store_address, block, &all_terminator_values); let is_dereference = block .expressions @@ -231,7 +224,6 @@ impl<'f> PerFunctionContext<'f> { store_address: &ValueId, block: &Block, all_terminator_values: &HashSet, - per_func_block_params: &HashSet, ) -> bool { let reference_parameters = self.reference_parameters(); @@ -241,10 +233,6 @@ impl<'f> PerFunctionContext<'f> { return true; } - if per_func_block_params.contains(&alias) { - return true; - } - // Is any alias of this address an input to some function call, or a return value? if self.instruction_input_references.contains(&alias) { return true; @@ -495,20 +483,12 @@ impl<'f> PerFunctionContext<'f> { references.aliases.insert(Expression::Other(result), AliasSet::known(result)); } Instruction::ArrayGet { array, .. } => { - let result = self.inserter.function.dfg.instruction_results(instruction)[0]; - let array = *array; let array_typ = self.inserter.function.dfg.type_of_value(array); if array_typ.contains_reference() { self.instruction_input_references .extend(references.get_aliases_for_value(array).iter()); references.mark_value_used(array, self.inserter.function); - - let expression = Expression::ArrayElement(array); - - if let Some(aliases) = references.aliases.get_mut(&expression) { - aliases.insert(result); - } } } Instruction::ArraySet { array, value, .. } => { @@ -516,29 +496,6 @@ impl<'f> PerFunctionContext<'f> { let element_type = self.inserter.function.dfg.type_of_value(*value); if element_type.contains_reference() { - let result = self.inserter.function.dfg.instruction_results(instruction)[0]; - let array = *array; - - let expression = Expression::ArrayElement(array); - - let mut aliases = if let Some(aliases) = references.aliases.get_mut(&expression) - { - aliases.clone() - } else if let Some((elements, _)) = - self.inserter.function.dfg.get_array_constant(array) - { - let aliases = references.collect_all_aliases(elements); - self.set_aliases(references, array, aliases.clone()); - aliases - } else { - AliasSet::unknown() - }; - - aliases.unify(&references.get_aliases_for_value(*value)); - - references.expressions.insert(result, expression); - references.aliases.insert(expression, aliases); - // Similar to how we remember that we used a value in a `Store` instruction, // take note that it was used in the `ArraySet`. If this instruction is not // going to be removed at the end, we shall keep the stores to this value as well. @@ -698,12 +655,6 @@ impl<'f> PerFunctionContext<'f> { } } TerminatorInstruction::Return { return_values, .. } => { - // We need to appropriately mark each alias of a reference as being used as a return terminator argument. - // This prevents us potentially removing a last store from a preceding block or is altered within another function. - for return_value in return_values { - self.instruction_input_references - .extend(references.get_aliases_for_value(*return_value).iter()); - } // Removing all `last_stores` for each returned reference is more important here // than setting them all to unknown since no other block should // have a block with a Return terminator as a predecessor anyway. diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs index ffe93609a9f..78be142e475 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs @@ -1,10 +1,6 @@ use std::borrow::Cow; -use crate::ssa::ir::{ - function::Function, - instruction::{Instruction, InstructionId}, - value::ValueId, -}; +use crate::ssa::ir::{function::Function, instruction::InstructionId, value::ValueId}; use super::alias_set::AliasSet; @@ -174,27 +170,13 @@ impl Block { /// last stores anyway, we don't inherit them from predecessors, so if one /// block stores to an address and a descendant block loads it, this mechanism /// does not affect the candidacy of the last store in the predecessor block. - fn keep_last_stores_for(&mut self, address: ValueId, function: &Function) { - self.keep_last_store(address, function); + fn keep_last_stores_for(&mut self, address: ValueId) { + self.last_stores.remove(&address); - for alias in (*self.get_aliases_for_value(address)).clone().iter() { - self.keep_last_store(alias, function); - } - } - - /// Forget the last store to an address, to remove it from the set of instructions - /// which are candidates for removal at the end. Also marks the values in the last - /// store as used, now that we know we want to keep them. - fn keep_last_store(&mut self, address: ValueId, function: &Function) { - if let Some(instruction) = self.last_stores.remove(&address) { - // Whenever we decide we want to keep a store instruction, we also need - // to go through its stored value and mark that used as well. - match &function.dfg[instruction] { - Instruction::Store { value, .. } => { - self.mark_value_used(*value, function); - } - other => { - unreachable!("last_store held an id of a non-store instruction: {other:?}") + if let Some(expr) = self.expressions.get(&address) { + if let Some(aliases) = self.aliases.get(expr) { + for alias in aliases.iter() { + self.last_stores.remove(&alias); } } } @@ -203,7 +185,7 @@ impl Block { /// Mark a value (for example an address we loaded) as used by forgetting the last store instruction, /// which removes it from the candidates for removal. pub(super) fn mark_value_used(&mut self, value: ValueId, function: &Function) { - self.keep_last_stores_for(value, function); + self.keep_last_stores_for(value); // We must do a recursive check for arrays since they're the only Values which may contain // other ValueIds. @@ -214,18 +196,6 @@ impl Block { } } - /// Collect all aliases used by the given value list - pub(super) fn collect_all_aliases( - &self, - values: impl IntoIterator, - ) -> AliasSet { - let mut aliases = AliasSet::known_empty(); - for value in values { - aliases.unify(&self.get_aliases_for_value(value)); - } - aliases - } - pub(super) fn get_aliases_for_value(&self, value: ValueId) -> Cow { if let Some(expression) = self.expressions.get(&value) { if let Some(aliases) = self.aliases.get(expression) {