fix(ssa): Perform mem2reg before DIE

[aakoshh]

Description

Problem*

Resolves #8779

Summary*

Fixes the SSA pipeline to perform mem2reg before the DIE pass, as per the comments regarding the Store instruction in can_be_eliminated_if_unused.

Also added a post-check to DIE to see that no Load or Store instructions are left in it if the conditions indicate that Store can be removed.

Additional Context

The program in the bug ticket had this SSA:

After Check u128 mul overflow (1) (step 30):
acir(inline) predicate_pure fn main f0 {
  b0(v0: u32):
    v1 = allocate -> &mut u1
    store u1 1 at v1
    v3 = make_array [v1] : [&mut u1; 1]
    constrain v0 == u32 0, "Index out of bounds"
    v5 = load v1 -> u1                            	// src/main.nr:3:6
    return v5
}

After Dead Instruction Elimination (1) (step 31):
acir(inline) predicate_pure fn main f0 {
  b0(v0: u32):
    v1 = allocate -> &mut u1
    constrain v0 == u32 0, "Index out of bounds"
    v3 = load v1 -> u1                            	// src/main.nr:3:6
    return v3
}

The problem was that store u1 1 at v1 was removed by the DIE pass, even though it's used in v3 = load v1. The reason for this is:

Store instructions must be removed by DIE in acir code, any load
instructions should already be unused by that point.

Note that this check assumes that it is being performed after the flattening
pass and after the last mem2reg pass. This is currently the case for the DIE
pass where this check is done, but does mean that we cannot perform mem2reg
after the DIE pass.

So DIE assumes that mem2reg has figured out and eliminated any store followed by load and replaced e.g. v3 with the value in u1 1. However that wasn't the case here, and by removing the store, it prevented the mem2reg that followed from figuring it out, which meant the load survived until the end, and caused a panic during ACIR gen.

The array by this point was gone, but the error mentioned it anyway, but that's just because UnknownReference has this error message.

I don't know if there could be longer chains that would require multiple mem2reg passes, which is why I added the post-check. But since there already was a mem2reg after DIE, which is hampered by the removal of Store, I thought moving it up should only improve things.

Other moves

I wasn't sure if simplify_cfg shouldn't also be moved up:

• On one hand it says it removes blocks with no instructions, and I thought if DIE removed the last unused instruction, we might get more opportunities to simplify things.
• On the other hand, when we unify two blocks in mem2reg, if one is yet to be visited, it will make it forget all known values from the visited block, even if the one it merges with doesn't modify them. If simplification reduces the number of blocks, mem2reg has a better chance of keeping known values.

In the end I moved it as well, because otherwise the references test grows by 25% in Brillig opcode size.

I also had to move the new remove_unreachable_instructions before the DIE pass, so that the #8774 example doesn't fail on the new post-check that asserts that no Load should be left after DIE: in that case it's impossible to load from an empty array, so mem2reg couldn't get rid of it.

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.

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: e85d497074673e73f31d518b05decf977570ab86, compared to commit: b2c38af614e58554625886218ed8325b33a0590c

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
hashmap_inliner_max	+141 ❌	+0.27%
slices_inliner_min	+3 ❌	+0.08%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
hashmap_inliner_max	51,651 (+141)	+0.27%
slices_inliner_min	3,805 (+3)	+0.08%
uhashmap_inliner_max	144,538 (+113)	+0.08%
lambda_from_array_inliner_max	1,757 (+1)	+0.06%
lambda_from_array_inliner_min	1,757 (+1)	+0.06%
lambda_from_array_inliner_zero	1,757 (+1)	+0.06%

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: e85d497074673e73f31d518b05decf977570ab86, compared to commit: b2c38af614e58554625886218ed8325b33a0590c

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_max	+39 ❌	+0.34%
hashmap_inliner_max	+40 ❌	+0.23%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
uhashmap_inliner_max	11,628 (+39)	+0.34%
hashmap_inliner_max	17,318 (+40)	+0.23%
slices_inliner_min	2,170 (+3)	+0.14%
lambda_from_array_inliner_max	2,557 (+1)	+0.04%
lambda_from_array_inliner_min	2,557 (+1)	+0.04%
lambda_from_array_inliner_zero	2,557 (+1)	+0.04%

[github-actions]

Changes to circuit sizes

Generated at commit: e85d497074673e73f31d518b05decf977570ab86, compared to commit: b2c38af614e58554625886218ed8325b33a0590c

🧾 Summary (10% most significant diffs)

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_1144_1169_2399_6609	+4 ❌	+1.37%	+4 ❌	+0.08%

---

Full diff report 👇

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_1144_1169_2399_6609	296 (+4)	+1.37%	5,208 (+4)	+0.08%

[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: 5480889	Previous: 3838c69	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	182 s	141 s	1.29

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

CC: @TomAFrench

[jfecher]

LGTM.
And yes, the original expectation was for there to be no more loads before DIE runs and thus DIE could just freely remove all Stores.

[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: 3caf961	Previous: b2c38af	Ratio
rollup-base-public	18.48 s	13.8 s	1.34

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

CC: @TomAFrench