Skip to content

Commit

Permalink
fix: Reduce memory usage in mem2reg (#7053)
Browse files Browse the repository at this point in the history
Co-authored-by: Ary Borenszweig <[email protected]>
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
3 people authored Jan 14, 2025
1 parent aa7b91c commit a0ffedf
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
16 changes: 15 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/mem2reg/alias_set.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use vec_collections::VecSet;
use vec_collections::{AbstractVecSet, VecSet};

use crate::ssa::ir::value::ValueId;

Expand Down Expand Up @@ -55,6 +55,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 a0ffedf

Please sign in to comment.