Skip to content

chore: simplify Expression in mem2reg #9599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asterite merged 2 commits into from
Aug 21, 2025
Merged

chore: simplify Expression in mem2reg #9599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2158590
chore: simplify Expression in mem2reg
asterite Aug 21, 2025
670d0cd
Merge branch 'master' into ab/mem2reg_simplify_expression
asterite Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,11 @@ impl<'f> PerFunctionContext<'f> {
let first = first.expect("All parameters alias at least themselves or we early return");

let expression = Expression::Other(first);
let previous = references.aliases.insert(expression.clone(), aliases.clone());
let previous = references.aliases.insert(expression, aliases.clone());
assert!(previous.is_none());

aliases.for_each(|alias| {
let previous = references.expressions.insert(alias, expression.clone());
let previous = references.expressions.insert(alias, expression);
assert!(previous.is_none());
});
}
Expand Down Expand Up @@ -534,7 +534,7 @@ impl<'f> PerFunctionContext<'f> {
});
references.mark_value_used(array, self.inserter.function);

let expression = Expression::ArrayElement(Box::new(Expression::Other(array)));
let expression = Expression::ArrayElement(array);

if let Some(aliases) = references.aliases.get_mut(&expression) {
aliases.insert(result);
Expand All @@ -549,7 +549,7 @@ impl<'f> PerFunctionContext<'f> {
let result = self.inserter.function.dfg.instruction_results(instruction)[0];
let array = *array;

let expression = Expression::ArrayElement(Box::new(Expression::Other(array)));
let expression = Expression::ArrayElement(array);

let mut aliases = if let Some(aliases) = references.aliases.get_mut(&expression)
{
Expand All @@ -566,7 +566,7 @@ impl<'f> PerFunctionContext<'f> {

aliases.unify(&references.get_aliases_for_value(*value));

references.expressions.insert(result, expression.clone());
references.expressions.insert(result, expression);
references.aliases.insert(expression, aliases);

// Similar to how we remember that we used a value in a `Store` instruction,
Expand All @@ -593,8 +593,8 @@ impl<'f> PerFunctionContext<'f> {
if Self::contains_references(typ) {
let array = self.inserter.function.dfg.instruction_results(instruction)[0];

let expr = Expression::ArrayElement(Box::new(Expression::Other(array)));
references.expressions.insert(array, expr.clone());
let expr = Expression::ArrayElement(array);
references.expressions.insert(array, expr);
let aliases = references.aliases.entry(expr).or_insert(AliasSet::known_empty());

self.add_array_aliases(elements, aliases);
Expand All @@ -606,7 +606,7 @@ impl<'f> PerFunctionContext<'f> {

if Self::contains_references(&result_type) {
let expr = Expression::Other(result);
references.expressions.insert(result, expr.clone());
references.expressions.insert(result, expr);
references.aliases.insert(
expr,
AliasSet::known_multiple(vec![*then_value, *else_value].into()),
Expand Down Expand Up @@ -643,7 +643,7 @@ impl<'f> PerFunctionContext<'f> {
fn set_aliases(&self, references: &mut Block, address: ValueId, new_aliases: AliasSet) {
let expression =
references.expressions.entry(address).or_insert(Expression::Other(address));
let aliases = references.aliases.entry(expression.clone()).or_default();
let aliases = references.aliases.entry(*expression).or_default();
*aliases = new_aliases;
}

Expand Down
16 changes: 7 additions & 9 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ pub(super) struct Block {
/// An `Expression` here is used to represent a canonical key
/// into the aliases map since otherwise two dereferences of the
/// same address will be given different ValueIds.
#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
#[derive(Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
pub(super) enum Expression {
Dereference(Box<Expression>),
ArrayElement(Box<Expression>),
Dereference(ValueId),
ArrayElement(ValueId),
Other(ValueId),
}

Expand Down Expand Up @@ -90,7 +90,7 @@ impl Block {

fn set_value(&mut self, address: ValueId, value: ReferenceValue) {
let expression = self.expressions.entry(address).or_insert(Expression::Other(address));
let aliases = self.aliases.entry(expression.clone()).or_default();
let aliases = self.aliases.entry(*expression).or_default();

if aliases.is_unknown() {
// uh-oh, we don't know at all what this reference refers to, could be anything.
Expand Down Expand Up @@ -119,7 +119,7 @@ impl Block {
if let Some(existing) = self.expressions.get(value_id) {
assert_eq!(existing, expression, "Expected expressions for {value_id} to be equal");
} else {
self.expressions.insert(*value_id, expression.clone());
self.expressions.insert(*value_id, *expression);
}
}

Expand All @@ -130,10 +130,8 @@ impl Block {
continue;
}
}
let expression = expression.clone();

self.aliases
.entry(expression)
.entry(*expression)
.and_modify(|aliases| aliases.unify(new_aliases))
.or_insert_with(|| new_aliases.clone());
}
Expand Down Expand Up @@ -162,7 +160,7 @@ impl Block {
if let Some(known_address) = self.get_known_value(address) {
self.expressions.insert(result, Expression::Other(known_address));
} else {
let expression = Expression::Dereference(Box::new(Expression::Other(address)));
let expression = Expression::Dereference(address);
self.expressions.insert(result, expression);
// No known aliases to insert for this expression... can we find an alias
// even if we don't have a known address? If not we'll have to invalidate all
Expand Down
Loading