Skip to content

Commit

Permalink
Only look up the entry for mutation if it would change something
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Jan 14, 2025
1 parent ef1a6f8 commit df5b88d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
14 changes: 14 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/alias_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ impl AliasSet {
}
}

/// Returns true if calling `unify` would change something in this alias set.
///
/// This is an optimization to avoid having to look up an entry ready to be modified in the [Block](crate::ssa::opt::mem2reg::block::Block),
/// because doing so would involve calling `Arc::make_mut` which clones the entry, ready for modification.
pub(super) fn should_unify(&self, other: &Self) -> bool {
if let (Some(self_aliases), Some(other_aliases)) = (&self.aliases, &other.aliases) {
// `unify` would extend `self_aliases` with `other_aliases`, so if `other_aliases` is a subset, then nothing would happen.
!other_aliases.is_subset(self_aliases)
} else {
// `unify` would set `aliases` to `None`, so if it's not `Some`, then nothing would happen.
self.aliases.is_some()
}
}

/// Inserts a new alias into this set if it is not unknown
pub(super) fn insert(&mut self, new_alias: ValueId) {
if let Some(aliases) = &mut self.aliases {
Expand Down
6 changes: 6 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ impl Block {
}

for (expression, new_aliases) in &other.aliases {
// If nothing would change, then don't call `.entry(...).and_modify(...)` as it involves creating more `Arc`s.
if let Some(aliases) = self.aliases.get(expression) {
if !aliases.should_unify(new_aliases) {
continue;
}
}
let expression = expression.clone();

self.aliases
Expand Down

0 comments on commit df5b88d

Please sign in to comment.