chore(ssa): Refactor flattening

[aakoshh]

Description

Problem*

Trying to improve clarity for the audit.

Summary*

• Some of the methods returned "next" blocks which were unused. Removed these to avoid the confusion about what drives the algorithm.
• Added a WorkList which maintains a separate stack and set of blocks, to avoid O(n) inclusion checks for deeply nested branching.
• Format docs.
• Replace arguments_stack: Vec<_> with next_arguments: Option<_> to get rid of a bug that did not manifest itself in practice, but didn't make any sense either (see below).
• Break up handle_instruction_side_effects to make it easier to see what it does with calls/intrinsics/blackbox functions.
• Make the matches in handle_*_side_effects exhaustive so we don't miss handling any new instruction/intrinsic.
• Add the absence ConstrainNotEqual a pre-condition.
• Fix the passing on of local_allocations from the then_branch to the else_branch and then back to the previous context so that we don't pass out the else_branch allocations, but rather restore what was before. The effect is that some loading of previous reference values can be saved, where we are storing to references allocated on a branch before a nested branch made it forget its history. Added test to demonstrate.

Additional Context

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]

⚠️ 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: da6281c	Previous: 585175e	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	229 s	180 s	1.27
test_report_zkpassport_noir-ecdsa_	2 s	1 s	2

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

CC: @TomAFrench

[aakoshh]

There seems to be a bug in the handling of arguments_stack

• In inline_block we pop it unconditionally.
• In handle_terminator we push 1 item: empty for JmpIf and the actual args for Jmp
• handle_terminator enqueues [then_block, else_block, exit_block]
• handle_terminator calls then_stop or else_stop , and the latter calls inline_branch_end to prepare the args for the exit block
• inline_branch_end pops 2 items from arguments_stack and replaces them with the merged arguments, which seem to be assumed to already be in the inserters cache.

The following is a rundown of the basic_jmpif test:

b0(v0: u1):                     // flatten:    args_stack = []
  jmpif v0 then: b1, else: b2   // hndl_trm: push(args_stack = [], args = [])  -> [[]]
b1():                           // inln_blk: pop (args_stack = [[]])           -> ([], Some([]))
  jmp b3(Field 3)               // hndl_trm: push(args_stack = [], args = [3]) -> [[3]]
b2():                           // inln_blk: pop (args_stack = [[3]])          -> ([], Some([3]))
  jmp b3(Field 4)               // hndl_trm: push(args_stack = [], args = [4]) -> [[4]]
                                // brch_end: pop (args_stack = [[4]])          -> ([], Some([4]))
                                // brch_end: pop (args_stack = [])             -> ([], None))
                                // brch_end: push(args_stack = [v0*3+!v0*4])   -> [[v0*3+!v0*4]]
b3(v1: Field):                  // inln_blk: pop (args_stack = [[v0*3+!v0*4]]) -> ([], Some([[v0*3+!v0*4]]))
  return v1

So the else block consumes the arguments pushed by the then branch and treats it as its own, which is surely not correct. However, it doesn't have parameters, so this is not causing any issues, and in the end it actually looks up the arguments of the then terminator, to do the merge, at which point those values are already in the inserter.

The else handler pops two items from the arguments stack, which doesn't seem correct either, but because it always pushes one argument, the exit block will always see something.

I simplified this by replacing the arguments_stack: Vec<_> with a next_arguments: Option<_> and only pushing/consuming in blocks which have non-empty blocks, so there should only ever be 1 item in there.

[github-actions]

Changes to circuit sizes

Generated at commit: 1c5b83f6874f1efaf13ab31fcbac29f3495d253a, compared to commit: 585175e56f2c34f225fe6ac87a91f4962c61553d

🧾 Summary (10% most significant diffs)

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_5252	-264 ✅	-1.06%	+534 ❌	+0.70%

---

Full diff report 👇

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_5252	24,531 (-264)	-1.06%	76,351 (+534)	+0.70%

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​types/​prettier@​3.0.0
	@​types/​pako@​2.0.4
	@​types/​mocha@​10.0.10

View full report

[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: 3b97880	Previous: 585175e	Ratio
private-kernel-reset	9.914 s	7.716 s	1.28

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

CC: @TomAFrench

[aakoshh]

On thing I forgot to touch on in the end was that inline_block mentions "track slice capacities" but I didn't see any evidence of this.