diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 158ce86a8d6..a6fbd2d1dd0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -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); + } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs index d4ef509f73e..4325ec3df1c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs @@ -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 = 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); }