feat(brillig): Phase 1 — linear-scan register allocator - #13313
Draft
aakoshh wants to merge 15 commits into
Draft
Conversation
The `LiveIntervals` module (program-point numbering, per-value `[def, last_use]` intervals, register-pressure queries) was fully `#[cfg(test)]`-gated as scaffolding. Un-gate the `impl` and its accessors so production code — the forthcoming linear-scan `Allocator` — can build and read intervals. No behavior change: the module is still unused outside tests (covered by the existing `#![allow(dead_code)]`), and all interval unit tests pass unchanged. Milestone A of the Phase 1 linear-scan work (design/register_allocation.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ar-scan) Make the per-function allocator choice polymorphic behind a `FunctionAllocator<R>` enum (`Greedy | LinearScan`), both implementing the existing `Allocator` trait, so the driver dispatches without knowing which strategy runs. A `use_linear_scan_allocator` flag on `BrilligOptions` selects between them (default greedy); `FunctionContext::new_with_allocator` constructs the chosen variant from the same liveness/coalescing/spill inputs. `LinearScanAllocator` is a delegating scaffold over `GreedyAllocator` for now, so the whole seam — flag, polymorphic field, construction, globals `into_allocations` path — is exercised end-to-end and proven behavior-preserving before the plan-based internals land. The non-trait lifecycle methods (`into_allocations`, test inspectors) live on the enum, keeping the `Allocator` trait free of allocator-specific surface. Tests compile representative functions (no-spill, cross-block, spilling) with both allocators and assert byte-identical bytecode. Milestone B of Phase 1 linear scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the global assignment that the linear-scan allocator will serve: `assign()` runs textbook linear scan (Poletto & Sarkar) over the value `LiveIntervals` and produces a `Plan` mapping each value to a fixed `Home` — one register index for its whole life, or a spill slot. The value-register capacity is `usable_registers - min_live_count`. Reserving `min_live_count` registers leaves every instruction room for its working set (operands/result/scratch, which reuse one another up to the per-instruction floor), so bounding register-homed values to that capacity guarantees codegen never overflows the frame. Overflow spills the furthest-`last_use` value whole. Because a home is fixed for a value's entire range, a cross-block value that gets a register keeps it across the boundary with no spill/reload — the reduction in spill traffic over greedy's blanket cross-block spilling, and the point of the fixed-home model. Pure data-structure pass, not yet wired into codegen (guarded by `allow(dead_code)` until the allocator consumes it). Unit tests cover the soundness invariant (interfering values never share a register index; indices within capacity), the no-spill and all-spill boundaries, and that a cross-block value keeps its register. Milestone C1 of Phase 1 linear scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `LiveRanges`: per-value liveness as a *list* of disjoint program-point ranges that preserves holes, the precision `LiveIntervals` deliberately drops by collapsing to one `[def, last_use]`. Holes are the floor for a competitive linear-scan allocator: a value dead across a hole frees its register for a value that never co-resides with it (sharing separated by a hole), and only live-but-displaced values are pressure-spilled. Without holes a linear scan over-estimates register pressure and loses to the greedy allocator, which already accounts for dead-after points via `max_live_count`. Ranges are derived by punching holes into each value's contiguous interval: a block whose whole point-range lies strictly inside the interval and in which the value is neither live-in nor live-out is a hole (a value used/defined in an interior block is necessarily live-in there, so the live-in/live-out test alone finds dead interior blocks). Reuses the existing interval + block-granular liveness, so params/constants are handled correctly. Tests: straight-line code has a single range per value equal to its collapsed interval; a value used only in a deep branch with an early-sorted dead sibling branch splits into two ranges across the hole. Milestone C5 of Phase 1 linear scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (no pressure) Replace C1's whole-value placeholder with the real per-point model: a `Plan` of per-value location timelines (`Segment` = a point range in one `Location`, either a register index or the value's fixed slot). Register location varies over time (splitting); the spill slot is fixed. `assign` runs linear scan over the hole-aware `LiveRanges`, scanning each of a value's ranges as an interval but **reclaiming the value's previous register** when a later range begins if it is still free. So a value with a hole on its own runtime path keeps one register (no spurious split), while a value dead across a divergent-path hole releases its register for another value to share during the hole. This lands the no-pressure case (returns `None` when a point needs more registers than capacity — pressure spilling to the fixed slot is layered next). All behind the off-by-default flag, so mainline is untouched. Tests: straight-line code assigns registers soundly (no two interfering values share a register over overlapping points); a cross-block value keeps a single register the whole time (the win greedy lacks); over-capacity pressure reports unplaceable for now. Milestone C6a of Phase 1 linear scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the delegating scaffold with the real read-only `LinearScanAllocator`: it holds only the precomputed `Plan` + program-point maps and answers every `Allocator` method as a lookup (no register pool, no online decisions), exactly the read-only endgame the design doc describes. `FunctionContext` builds the plan and, when the function fits the value capacity, runs linear scan; otherwise it falls back to greedy so every function still compiles (pressure spilling lands next). Key correctness points shaken out end-to-end: - Entry-block parameters are pre-colored fixed intervals: the calling convention places argument `i` in register `i`, so `assign` reserves `[0, n_params)` in parameter order rather than letting the scan reorder them (this was a real execution bug — swapped operands). - Value capacity is `usable - (min_live_count + SPILL_MARGIN)`, matching greedy's cushion, since `min_live_count` under-counts the true scratch peak (parallel moves, on-demand constants). Tight frames therefore decline linear scan and fall back. The driver needed **no changes** — with no pressure the allocator returns concrete registers and no spill/reload actions, so the existing consumer works as-is. Scratch note (deviation from the doc, flagged for review): scratch is drawn from a reserved high register band (value homes occupy `[0, capacity)`, scratch the rest) rather than the doc's shadow-complement. Same guarantee (scratch never collides with a value home), simpler, and no driver pool-sync; the dynamic shadow-backed pool is a register-efficiency refinement (tracked as E2-proper). Tests execute a function with each allocator and assert identical results (no-spilling, cross-block, and small-frame fallback). Full `noirc_evaluator` suite green (greedy is still the default). Milestone C3+E2+C4 (no-pressure) of Phase 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t + frame-size bugs Broad validation (forced linear-scan across the whole execute suite) surfaced two real bugs, now fixed; the no-pressure pipeline then passes corpus-wide. - Decline split plans. Register scarcity can force a value into a second register even without slot spilling (reclaim fails when a divergent-path hole's occupant overlaps the value's revival). Realizing that split needs a move the no-pressure allocator does not emit, so `try_build` now declines any plan where a value's timeline is not a single register and falls back to greedy. (Was miscompiling: a Field value read as u32.) Pressure spilling with proper moves/resolution handles splits next. - Reserve only the registers used, not the whole capacity. The scratch band previously reserved `[0, capacity)`, inflating the frame's high-water mark to the full capacity even for tiny functions — which ballooned recursive call frames and overflowed the stack. Now reserve only up to the highest value register the plan actually uses, so the frame stays as small as the values require. Also add `NOIR_BRILLIG_LINEAR_SCAN` as an A/B override to force the allocator corpus-wide for validation without threading the option through every entry point. Result: with linear scan forced, the execute suite is 9076/9080 genuine passes, 0 real failures; the 4 diffs are cosmetic recursion-depth counts in expected-failure stderr snapshots (different frame size -> overflow at a different depth). Greedy remains the default, so the mainline is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Changes to circuit sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
Repair the two `cargo doc` warnings (broken/redundant intra-doc links) that fail CI under `-D warnings`: - `GreedyAllocator::new`'s doc pointed at `FunctionContext::new`, now `#[cfg(test)]`; point it at `new_with_allocator` (the production constructor). - Drop the redundant explicit target on the `LiveRanges` link and refresh the stale `linear_scan` module doc (it still described the delegating scaffold) to the current read-only plan-based allocator with greedy fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend `assign` to spill under register pressure instead of declining. When no register is free at a point, it evicts the longest-lived interval — an active one if it outlives the incoming range, else the incoming range itself — to that value's fixed spill slot, splitting the evicted value's timeline into a register segment followed by a slot segment. Plans therefore now mix `Register` and `Slot` locations; a value can be both over its life. Each spilled value gets one fixed slot (packing non-interfering values into shared slots is a later refinement). This is the algorithmic core of the win — keeping the values that fit in registers across blocks while spilling only the overflow — as opposed to greedy's blanket cross-block spill. Not yet served end-to-end: `try_build` still declines any plan with a slot/split segment (a reload needs a register drawn from the shadow-backed scratch pool, which isn't built yet), so spilling functions continue to fall back to greedy — no behaviour change. Serving spill/reload actions + edge resolution is the next step. Adds `ProgramPoint::pred` for the segment split. Unit test: capacity-2 pressure spills soundly (register-resident values never share a register over overlapping points; every value has a home at each live point). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n point `LiveIntervals::adjust_constant_defs` extended every constant's def back to its allocation block's entry, on the assumption (stated in the old doc comment) that constants are codegen'd at block entry. They are not: `initialize_constants` materializes a constant lazily, just before its use, at the `ConstantAllocation` location (an instruction or terminator). The interval therefore claimed a constant was live — and register-resident — over a stretch where its `const` opcode had not yet run. Set the def to the actual materialization point instead. This yields accurate live ranges (e.g. `register_pressure_linear` drops from an inflated 4 to 3, since the two constants are no longer counted live from block entry) and is a prerequisite for spilling constants soundly: a spill now stores data that has actually been computed. The bug was latent because `LiveIntervals`/`LiveRanges` were `#[cfg(test)]`-only until the linear-scan allocator started consuming them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ator The linear-scan allocator now realizes a precomputed plan at codegen time: it keeps a residency mirror and, per definition/use, returns where a value lives. `assign` produces plans with interval splitting (register↔slot spills, register→register revival across holes), but `try_build` currently declines any plan whose values change register or spill, and falls back to greedy — serving only single-register plans. That already beats greedy, which permanently spills every cross-block value; linear scan keeps them in a register (holes/divergent-branch revivals in the same register are re-seeded in the mirror, their data surviving on the taken path). Enabling the declined cases needs an edge/merge resolution phase for split intervals (Wimmer & Franz §6) that is not yet built; see the decline in `try_build` for the rationale. Also: - The `Allocator` seam's `use_variable`/`reserve_scratch` take an `Option<InstructionId>` so a plan-based allocator can resolve the program point of a use; greedy ignores it, the driver threads a `current_instruction` cursor, and terminator operands pass `None` (made resident up front by `before_terminator`). - `resolve_edge` routes a dead block-param (passed by a predecessor but never read in its block) to its home register instead of panicking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Nothing in CI ran the linear-scan allocator — it is off by default and only selected via `NOIR_BRILLIG_LINEAR_SCAN`. Generate a `brillig_small_stack_linear_scan_execution_success` module alongside the existing (greedy) small-stack module: the same execution-success programs, forced to Brillig with `--max-stack-frame-size 64`, but with the linear-scan allocator enabled. The small frame maximizes register pressure, so this is where the allocator's placement and its greedy fallback are stressed. Keeps that path green as its capability grows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aakoshh
added
bench-show
…try snapshots Three related changes to the linear-scan allocator, all in `linear_scan.rs`: - **Fix a double-allocation.** A value defined and used at the same program point — which now happens for constants, whose def is their materialization point (its first use) — was processed by `assign` in both the uses step (reloaded as if a spilled operand) and the range-starts step (defined), allocating it twice and leaking its first register out of the free set. That leak forced later values to change register across a hole, and the read-only serving then re-seeded them at the wrong register (`Bit size ... does not match` / `no entry found` at runtime). The uses step now skips a value whose range starts at this point; it is defined by the range-starts step. Exposed only after constant defs moved to the materialization point. - **Serve register-only plans, not just single-register.** With the double-allocation fixed, a value never spuriously changes register across a hole; a genuine same-register revival after a hole is re-seeded by `make_resident` (its data survives on the taken path). `try_build` now declines only plans that use a spill slot — pressure spilling needs merge resolution, still to come — and serves everything else, including holed cross-block values greedy always spills. - **Seed block entries from precomputed `im` snapshots.** `Plan` now stores, per block, the values register-resident at its entry (value -> register index) as structurally-shared `im` maps, built in one sweep over the segments. `begin_block` seeds the residency mirror from its block's snapshot in O(live-at-entry) instead of scanning every value's timeline, which was O(values) per block (quadratic over a function). Also computes `next_use` once in `allocate`. Corpus (`NOIR_BRILLIG_LINEAR_SCAN=1`, execute suite): 9574/9578, 0 serving bugs, only the 4 known-benign recursion-depth stderr-snapshot diffs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…locator Serve pressure-spilled and interval-split plans, not just single-register ones. Values whose plan location diverges across a CFG edge are routed through their spill slot uniformly at every block boundary: excluded from the block-entry seed (reloaded on demand), saved before each terminator, and — when they are block parameters — delivered by resolve_edge to the slot rather than a register. This is what makes slot-canonical loop-carried values correct. The only case still declined to greedy is a value whose location diverges across an edge but has no spill slot (a register-to-register move, which is not emitted). Validated: NOIR_BRILLIG_LINEAR_SCAN=1 execute corpus 9574/9578 (the 4 diffs are pre-existing benign recursion-depth counts in expected-failure stderr snapshots, flag-on only, not in CI); small-stack CI survey 498/498. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A value whose plan location diverges across a CFG edge but was never spilled (so it has no slot) was the last case declined to greedy. Rather than emit the textbook register-to-register move (Wimmer & Franz §6) — which would need critical-edge splitting and parallel-move handling — give each such value a fresh spill slot and route it through the existing slot-canonical machinery. A store+reload round-trip instead of a move, but no new edge machinery. The only decline left is pressure beyond value_capacity, which greedy (which blanket-spills cross-block values) still covers. Red->green: lambda_from_dynamic_if, regression_1144_1169_2399_6609, and vector_dynamic_index declined under --max-stack-frame-size 64 before this and are served after (verified by instrumentation); all three run under the small-stack CI survey. Full corpus NOIR_BRILLIG_LINEAR_SCAN=1 unchanged at 9574/9578 (4 benign recursion-depth snapshot diffs, flag-on only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
I'm afraid my subscription now ended, so I'm shutting down the machines. I didn't have time to start the Validation phase, which would have shown at various stack sizes what the resulting bytecode size and potentially executed opcodes look like. The one unfinished task on this PR was to make temporary register allocation work off the shadow register. The latest idea was that The fact that execution is green so far suggests that the design has merit, if you ever want to pick this up, I hope it wasn't wasted effort. |
aakoshh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Resolved
Phase 1 of the register-allocation plan in #13302 (tracking issue #11638): a linear-scan register allocator for Brillig, running behind the pluggable
Allocatorseam extracted in Phase 0.5 (#13310).What's landed
LiveRanges): per-value liveness as a list of ranges, preserving the holes thatLiveIntervalscollapses. A hole is a divergent branch (dead-in and dead-out of an interior block), verified against Wimmer & Franz §3 — the!live_outclause is what distinguishes it from a straight-line gap. A value dead across a hole frees its register for a non-overlapping value.assign→Plan): linear scan over the hole-aware ranges producing a per-value location timeline. Pre-colors entry-block parameters to their ABI registers; reclaims a value's previous register across a hole; and (for the pressure case) evicts the furthest-next-use value to a fixed slot and reloads it at its next use. A result never aliases its instruction's operands' registers (codegen claims the result register before reading operands).LinearScanAllocator): realizes thePlanat codegen time via a small residency mirror — everyAllocatormethod is a lookup returning where a value lives and theActions to get it there. For single-register plans this emits no moves; a value revived in the same register after a hole is re-seeded in the mirror (its data survives on the taken path). The seam'suse_variable/reserve_scratchnow take anOption<InstructionId>so a plan-based allocator can resolve the program point of a use; greedy ignores it.try_buildcomputes the set of such divergent values and routes each through its spill slot uniformly at every block boundary: excluded from the block-entry seed (reloaded on demand), saved before every terminator (a newAction::Savethat stores but keeps the register), and — when the value is a block parameter — delivered byresolve_edgeto the slot rather than a register (this is what makes slot-canonical loop-carried values correct). Slot stores are harmless on every outgoing edge (own slot, SSA-immutable), so no critical-edge splitting is needed. A value in the same register on every edge pays nothing — only genuinely divergent values take the slot round-trip.adjust_constant_defs): a constant's def is now its materialization point (theConstantAllocationlocation), not the block entry — codegen materializes constants lazily at their use, so the old block-entry extension over-claimed liveness. This is a standalone correctness fix (first commit) and a prerequisite for spilling constants soundly.use_linear_scan_allocatorBrilligOptionsflag (plus aNOIR_BRILLIG_LINEAR_SCANenv override for A/B and CI) chooses the allocator per function. The only functions the current scope cannot serve — pressure beyondvalue_capacity— fall back to greedy, so every function still compiles correctly.brillig_small_stack_linear_scan_execution_successtest module runs the execution-success programs forced to Brillig at--max-stack-frame-size 64with the linear-scan allocator enabled — the small frame maximizes pressure, stressing placement and the greedy fallback. Nothing in CI ran linear scan before.What's pending
value_capacity(usable − min_live_count − SPILL_MARGIN) still falls back to greedy. Shrinking or retiring the staticSPILL_MARGINcushion — via shadow-backed dynamic scratch below — would remove most of these and is what would let greedy be retired entirely.max(live + scratch)rather thanmax(live) + max(scratch). Would also letreserve_scratchhand codegen an exact, plan-consistent temp set, turning the currently test-checked "scratch demand matches codegen" contract into a structural one.Validation
cargo nextest run -p noirc_evaluator— 1847 passed (greedy is the default; mainline unchanged).cargo clippyandcargo fmt --checkclean.NOIR_BRILLIG_LINEAR_SCAN=1 cargo nextest run -p nargo_cli --test execute --no-fail-fast— 9574/9578 genuine passes, 0 serving bugs (now with full spill/split serving enabled). The 4 diffs are cosmetic recursion-depth counts in expected-failurestderrsnapshots (a different frame size overflows at a different depth), flag-on only and not in CI, not miscompilations.brillig_small_stack_linear_scan_execution_successmodule — 498/498.User Documentation
Check one:
PR Checklist
cargo fmt.🤖 Generated with Claude Code