chore(brillig): Assert that the stack frame is large enough for the max number of registers needed by any instruction

[aakoshh]

Problem Resolved

One of the tasks the design doc proposed in #13302

Goal

Convert the latent, reactive "Stack frame too deep" panic in Stack::allocate_register
into an upfront diagnostic. An instruction's operands and scratch registers must all be
register-resident at the same program point — no allocator can spill around them — so there
is a hard lower bound on max_stack_frame_size. If the layout is configured below that bound
we should say so clearly instead of failing deep inside codegen once an allocation overflows.

Summary of Changes

Three files (+182 / -7):

• compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs

  • New field VariableLiveness::min_live_count, computed in the same walk as
    max_live_count (compute_max_live_count).
  • Per instruction: max(distinct inputs, results) + scratch, folded with max across all
    instructions. scratch comes from the existing instruction_scratch_demand.
  • Inputs come from instruction_min_inputs: for most instructions the distinct operands from
    variables_used_in_instruction (a HashSet, so repeated operands like add v0, v0 count
    once; function values excluded). MakeArray is an exception — see below.
  • Four unit tests: floor is per-instruction (not the peak), scratch is included, operands
    are deduplicated, and the floor does not scale with MakeArray length.

• compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs

  • FunctionContext::new now asserts
    min_live_count <= max_stack_frame_size - Stack::START_OFFSET
    with a clear message ("needs at least N registers, but only M are usable …").
  • Two tests: the assertion fires for a signed lt in a too-small frame; it accepts a frame
    whose usable slots meet the floor.

• compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs

  • Stack::START_OFFSET made pub(crate) so the floor can be compared against usable
    slots (frame − START_OFFSET), not the raw frame. START_OFFSET = 2 (sp[0] = previous
    stack pointer, sp[1] = per-frame spill base pointer).

Why max(inputs, results) + scratch, and why against usable slots

The first draft used inputs + results + scratch <= max_stack_frame_size (matching the
design doc's shorthand). Review pushed back: comparing against the full frame rather than
the usable slots relies on the reserved-slot count (2) coincidentally covering the
over-count, with no real relationship to why a result can reuse an input register.

To settle it, a frame-size sweep measured the smallest frame each single instruction compiles
in without the "Stack frame too deep" panic (usable = frame − 2):

instruction	inputs	results	scratch	usable floor	i+r+s	max(i,r)+s
checked add	2	1	1	4	4	3
checked mul	2	1	4	7	7	6
signed lt	2	1	3	6	6	5
signed div	2	1	8	11	11	10
array_get	2	1	1	4	4	3

Findings:

1. For every instruction that actually panics (scratch-bearing), the true usable floor is
   inputs + results + scratch exactly — the result does not reuse an operand register
   when there is a post-op check/scratch that keeps operands and result live together.
2. Trivial ops (unchecked_add, store) never panic; they silently emit
   possibly-fragile code at absurdly small frames, so their apparent floor of 1 is an artifact,
   not a real capability. Result-register reuse (the "alias trick") happens only here.
3. The original i+r+s <= frame had a real false negative: signed lt panics at frame 7,
   but 6 <= 7 passed the assertion. The +START_OFFSET slack was masking under-configuration.

The exact rule i+r+s <= frame − START_OFFSET would be tightest, but it breaks two existing
compile-only regression tests that deliberately use sub-floor frames — notably
brillig_spill_does_not_cause_transient_spill_leak, whose bug requires the frame-4
("2-register layout") to reproduce, so bumping it is not an option.

Decision: max(inputs, results) + scratch compared against frame − START_OFFSET. Rationale:

• Principled — compares a reuse-aware demand against the actually-usable slots.
• Safe — it is a lower bound on the true floor. For an assertion, false positives
  (rejecting a frame codegen could handle → breaking valid compilation) are far worse than
  false negatives (missing a bad config → falling through to the existing panic, i.e. status
  quo). Under-counting is the safe direction.
• Keeps all tests green.

Documented caveat: max(i,r)+s under-counts the true floor by results (≈1) for
scratch-bearing instructions, because the "results reuse operands" assumption is empirically
false when scratch is present. So the assertion catches gross under-configuration but leaves a
±1 gap to the existing codegen panic. FunctionContext::new therefore validates only this
floor, not full register pressure.

The MakeArray exception: streamed inputs

MakeArray breaks the "all operands must be register-resident together" assumption: its
elements are written to the heap one at a time (codegen_make_array →
initialize_constant_array_comptime converts and stores each element in turn; the repeating-item
runtime loop only ever materializes a single item's subitems, subitem_to_repeat_variables). So
a large array literal never needs all of its elements in registers at once.

Counting every distinct element as a simultaneous input therefore massively over-stated the floor
and caused a false positive: test_programs/execution_success/brillig_large_array failed to
compile (min_live_count exceeded the frame). Fix: instruction_min_inputs special-cases
MakeArray to typ.element_types().len() — the per-item subitem count (1 for a scalar array,
tuple-arity for tuple arrays), which is the true simultaneous bound and matches the size of
subitem_to_repeat_variables. Regression test: min_live_count_does_not_scale_with_make_array_length;
verified against all 16 brillig_large_array variants (including the small-stack one).

Is MakeArray the only exception? It is the only instruction whose operands are streamed to
the heap rather than held in registers, so it is the only one where the distinct-operand
count over-states the floor. (Call also marshals many arguments, but into contiguous
stack-frame slots that codegen_call already bounds-checks separately, so counting those is
not a false-positive source.)

MakeArray scratch. codegen_make_array reserves ensure_register_capacity(4) = the result
register plus three transient temporaries (items_pointer, write_pointer, codegen temp). The result
is an SSA value already counted in the live set, so instruction_scratch_demand returns 3 for
MakeArray, consistent with how it accounts for every other instruction's codegen temporaries.
The floor for a scalar-element array is therefore max(1 per-item, 1 result) + 3 = 4, matching the
ensure_register_capacity(4) reservation, and independent of the element count.

This scratch also feeds max_live_count, so two max_live_count MakeArray tests move from
4 → 7 (their earlier expectation counted the elements + result but omitted the codegen
temporaries). brillig_spill_batch_reduces_consecutive_spill_opcodes (a MakeArray at exactly 4
usable slots) still passes — floor 4 ≤ 4 — confirming this is not a false positive.

The result-register reuse mechanism (and a needless spill it exposes)

How two live-through operands and a result fit in only 2 usable registers:

• codegen_binary allocates the result register first, then reloads operands, then emits
  the op (brillig_binary.rs).
• Where inputs are reloaded: convert_ssa_binary → convert_ssa_single_addr_value
  (brillig_block.rs:1069) → convert_ssa_value (brillig_block.rs:982), whose
  is_spilled → reload_spilled_value (brillig_block.rs:417) emits the spill-slot load.
• spill_value (brillig_block.rs:288) stores the value to a slot, then frees the
  register — the value survives in memory.
• With only 2 usable slots the pre-allocated result register necessarily coincides with one
  operand's register at emit time; the alias is safe because a Brillig binary opcode reads
  both sources before writing the destination.

Needless spill of the empty result register

Real codegen of v2 = v0 + v1 at frame=4 (2 usable slots, both operands live-through):

 3: store sp[2] at sp[1]          ## (1) spill INPUT v0  → free a slot for the result register
 6: store sp[3] at @<slot 1>      ##     spill INPUT v1  → free a slot to reload v0
 7: sp[3] = load  sp[1]           ##     reload v0 into sp[3]      (left operand)
10: store sp[2] at @<slot 2>      ## (2) spill the RESULT register sp[2] — it holds no data yet!
13: sp[2] = load  @<slot 1>       ##     reload v1 into sp[2]      (right operand)
14: sp[2] = add   sp[3], sp[2]    ## (3) v2 = v0 + v1 → sp[2] (the result register)

Trace:

1. The result register is allocated first; ensure_register_capacity spills the LRU value,
   which is input v0 (line 3). Result v2 takes sp[2].
2. Reloading the second operand needs a slot back. At that point registers hold
   {v2 (result, empty), v0 (just reloaded)}; the LRU victim is v2, so line 10 stores the
   result register while it contains no live data.
3. Line 14 overwrites that register with the actual sum.

So lines 8–10 (const + add + store) are ~3 wasted opcodes spilling an empty register.
Root cause: the victim choice uses LRU recency, not next use — v2's next use is
imminent (line 14) but its last touch is older than v0's reload, so the recency proxy wrongly
ranks the about-to-be-written result as the coldest register.

This is exactly the behavior the design doc targets: the plan-based allocator reserves room for
inputs + results + scratch together (before_instruction) so results draw a pre-reserved slot
and never evict a sibling, and its furthest-next-use victim rule cannot evict them (their next
use is immediate). It is wasteful, not incorrect — the stale transient-spill record is cleaned
up by machinery already covered by brillig_spill_does_not_cause_transient_spill_leak and
..._reclaims_slot_when_it_dies.

Possible cheap follow-up (independent of the allocator rewrite): teach spill_value to skip
the store when the victim is a defined-but-not-yet-written result.

User Documentation

Check one:

• No user documentation needed.
• Documented in docs/.
• [For Experimental Features] Documentation tracking issue created:

PR Checklist

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

[aakoshh]

Hm, the fact that https://github.com/noir-lang/noir/actions/runs/28937560624/job/85853942060?pr=13306 fails suggest my assumption that inputs need to fit at the same time don't hold, or maybe not for every type of instruction.

Ah yes, make_array can reload individual items in a loop. This also has a bearing in the Allocator trait in the design doc.

[aakoshh]

Updated the design doc in 595d645