Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 63 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2538,4 +2538,67 @@ mod tests {
";
assert_ssa_does_not_change(src, Ssa::mem2reg);
}

#[test]
fn regression_10070() {
// Here v6 and v7 aliases v2 expression.
// When storing to v3 we may modify value referenced by v2 depending on the taken branch
// This must invalidate v8's value previously set.
let src = "
brillig(inline) fn main f0 {
b0(v0: [&mut Field; 1], v1: u1):
v3 = allocate -> &mut Field
v4 = allocate -> &mut Field
jmpif v1 then: b1, else: b2
b1():
v7 = array_set v0, index u32 0, value v3
jmp b3(v7)
b2():
v6 = array_set v0, index u32 0, value v4
jmp b3(v6)
b3(v2: [&mut Field; 1]):
v8 = array_get v2, index u32 0 -> &mut Field
store Field 1 at v8
store Field 2 at v3
store Field 3 at v4
v12 = load v8 -> Field
return v12
}
";
assert_ssa_does_not_change(src, Ssa::mem2reg);
}

#[test]
fn regression_10020() {
// v14 = add v12, v13 is NOT replaced by v13 = add v12, Field 1
let src = "
acir(inline) predicate_pure fn main f0 {
b0():
v1 = allocate -> &mut Field
store Field 0 at v1
v3 = allocate -> &mut Field
store Field 0 at v3
v4 = make_array [v1, v3] : [&mut Field; 2]
v5 = allocate -> &mut Field
store Field 0 at v5
jmp b1(u32 0)
b1(v0: u32):
v7 = eq v0, u32 0
jmpif v7 then: b2, else: b3
b2():
v9 = array_get v4, index v0 -> &mut Field
store Field 1 at v9
store Field 2 at v1
v12 = load v5 -> Field
v13 = load v9 -> Field
v14 = add v12, v13
store v14 at v5
v16 = unchecked_add v0, u32 1
jmp b1(v16)
b3():
v8 = load v5 -> Field
return v8
}";
assert_ssa_does_not_change(src, Ssa::mem2reg);
}
}
34 changes: 34 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,40 @@ impl Block {
}
}

// Issue https://github.com/noir-lang/noir/issues/10070
// When setting the value at `address`, it may impact any expression having unknown aliases
// because the `address` could reference such expressions.
// This is what happens in the issue linked above. An array of references is modified in two branches
// and as a result the parameter of the block joining the two branches is set to unknown aliases in `handle_terminator()`.
// Any value set here may be done on an address to the unknown aliases. Since we do no know them, we must conservatively
// invalidate their values.
// Furthermore, if the aliases are known and include the modified `address`, then we must invalidate the aliases values as well.
let addresses_to_invalidate: Vec<ValueId> = self
.expressions
.iter()
.filter_map(|(other_address, other_expression)| {
// Skip the address we're storing to, since its value is known
if *other_address == address {
return None;
}
if let Some(other_aliases) = self.aliases.get(other_expression) {
if other_aliases.is_unknown() {
// other_address with unknown alias set could alias anything and must be conservatively invalidated
return Some(*other_address);
}
if other_aliases.iter().any(|alias| alias == address) {
//other_address's alias set contains the address we're storing to, so it must be invalidated
return Some(*other_address);
}
}
None
})
.collect();
// Invalidate addresses with possible aliasing to `address`
for addr in addresses_to_invalidate {
self.references.remove(&addr);
}

// We always know address points to value
self.set_reference_value(address, value);
}
Expand Down
Loading