fix(mem2reg): Add the value in ArraySet to aliased_references

[aakoshh]

Description

Problem*

Resolves #8975
Fixes #8974

I also verified that NOIR_AST_FUZZER_SEED=0x8d7b02e200001000 cargo test -p noir_ast_fuzzer --test smoke no longer crashes.

Summary*

• Add the input value to ArraySet to the aliased_references.
• Change analyze_instruction so that if the instruction simplifies into a MakeArray instruction then it doesn't skip it, because the book keeping might be needed to keep alive stores to references used in the simplified instruction.

I'm not sure if this last step would need to be restricted to only a do a subset of the book keeping. What's sure if that if I allowed the analyse to go ahead as it is on all instruction types, then the compiler started crashing.

Additional Context

Originally I thought about adding the value to the instruction_input_references, but I re-read what @vezenovm commented about why we don't use this technique whenever a variable is an input to an instruction: he suggested that adding to this collection too aggressively would prevent the removal of stores that go into arrays that aren't used.

By contrast the aliased_references only prevents the removal of a store if there are outstanding instructions that rely on the value.

In this failure we had an SSA like this:

acir(inline) fn main f0 {
  b0():
    call f1()
    return u1 1
}
brillig(inline) fn func_1 f1 {
  b0():
    v0 = allocate -> &mut u1
    store u1 1 at v0
    v2 = make_array [v0] : [&mut u1; 1]
    call f2(v2)
    return
}
brillig(inline) fn func_2 f2 {
  b0(v0: [&mut u1; 1]):
    v1 = allocate -> &mut [&mut u1; 1]
    store v0 at v1
    v2 = allocate -> &mut u1
    store u1 0 at v2
    v4 = load v1 -> [&mut u1; 1]
    v6 = array_set v4, index u32 0, value v2
    store v6 at v1
    v7 = allocate -> &mut u32
    store u32 0 at v7
    jmp b1()
  b1():
    v8 = load v1 -> [&mut u1; 1]
    v9 = array_get v8, index u32 0 -> &mut u1
    v10 = load v9 -> u1
    jmpif v10 then: b2, else: b3
  b2():
    v11 = load v7 -> u32
    v12 = eq v11, u32 0
    jmpif v12 then: b4, else: b5
  b3():
    return
  b4():
    jmp b3()
  b5():
    jmp b1()
}

After mem2reg, the store u1 0 at v2 was gone, even though v6 = array_set v4, index u32 0, value v2 clearly uses v2.

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[aakoshh]

It still fails with --inliner-aggressiveness 0 --force-brillig:

❯ cargo run -q -p nargo_cli -- interpret --force --inliner-aggressiveness 0 --force-brillig --show-ssa-pass "step 6)" --show-ssa-pass "step 7)"
--- Interpreter result after Initial SSA:
Ok([Numeric(U1(true))])
---
--- Interpreter result after Removing Unreachable Functions (step 1):
Ok([Numeric(U1(true))])
---
--- Interpreter result after Defunctionalization (step 2):
Ok([Numeric(U1(true))])
---
--- Interpreter result after Inlining simple functions (step 3):
Ok([Numeric(U1(true))])
---
--- Interpreter result after Removing Paired rc_inc & rc_decs (step 4):
Ok([Numeric(U1(true))])
---
--- Interpreter result after Preprocessing Functions (step 5):
Ok([Numeric(U1(true))])
---
After Inlining (step 6):
brillig(inline) fn main f0 {
  b0():
    v0 = allocate -> &mut u1
    store u1 1 at v0
    v2 = make_array [v0] : [&mut u1; 1]
    v3 = allocate -> &mut [&mut u1; 1]
    store v2 at v3
    v4 = allocate -> &mut u1
    store u1 0 at v4
    v6 = load v3 -> [&mut u1; 1]
    v8 = array_set v6, index u32 0, value v4
    store v8 at v3
    v9 = allocate -> &mut u32
    store u32 0 at v9
    jmp b1()
  b1():
    v10 = load v3 -> [&mut u1; 1]
    v11 = array_get v10, index u32 0 -> &mut u1
    v12 = load v11 -> u1
    jmpif v12 then: b2, else: b3
  b2():
    v13 = load v9 -> u32
    v14 = eq v13, u32 0
    jmpif v14 then: b4, else: b5
  b3():
    return u1 1
  b4():
    jmp b3()
  b5():
    jmp b1()
}

--- Interpreter result after Inlining (step 6):
Ok([Numeric(U1(true))])
---
After Mem2Reg (step 7):
brillig(inline) fn main f0 {
  b0():
    v0 = allocate -> &mut u1
    v1 = make_array [v0] : [&mut u1; 1]
    v2 = allocate -> &mut [&mut u1; 1]
    v3 = allocate -> &mut u1
    v4 = make_array [v3] : [&mut u1; 1]
    store v4 at v2
    v5 = allocate -> &mut u32
    store u32 0 at v5
    jmp b1()
  b1():
    v7 = load v2 -> [&mut u1; 1]
    v8 = array_get v7, index u32 0 -> &mut u1
    v9 = load v8 -> u1
    jmpif v9 then: b2, else: b3
  b2():
    v10 = load v5 -> u32
    v11 = eq v10, u32 0
    jmpif v11 then: b4, else: b5
  b3():
    return u1 1
  b4():
    jmp b3()
  b5():
    jmp b1()
}

--- Interpreter result after Mem2Reg (step 7):
Err(Internal(UninitializedReferenceValueLoaded { value: "*v3 = None" }))
---
The application panicked (crashed).
Message:  Expected a non-error result: Internal(UninitializedReferenceValueLoaded { value: "*v3 = None" })
Location: tooling/nargo_cli/src/cli/interpret_cmd.rs:272

This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.
If there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml

[aakoshh]

In the above SSA, the array_set is optimised into a make_array, however because of that, the book keeping in analyze_instruction is skipped, which in this case means make_array [v3] did not keep around the store to v3 (originally store u1 0 at v4) 🤔

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Memory'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 00d61e7	Previous: 8a7046b	Ratio
private-kernel-inner	260.51 MB	202.19 MB	1.29
private-kernel-reset	339.03 MB	225.94 MB	1.50
rollup-base-private	1190 MB	507.26 MB	2.35
rollup-base-public	1350 MB	440.45 MB	3.07
rollup-block-root	7180 MB	1510 MB	4.75

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 7d49473	Previous: c686a3e	Ratio
rollup-merge	0.004 s	0.003 s	1.33
semaphore-depth-10	0.028 s	0.021 s	1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 8ea8883	Previous: 2cfc786	Ratio
private-kernel-reset	9.566 s	7.758 s	1.23

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 13f6998	Previous: 3838c69	Ratio
test_report_noir-lang_sha512_	24 s	15 s	1.60

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[asterite]

Looks good!