Skip to content

feat(compiler): add specialization-aware CFG analysis - #89

Draft
thiremani wants to merge 16 commits into
masterfrom
codex/specialization-aware-cfg
Draft

feat(compiler): add specialization-aware CFG analysis#89
thiremani wants to merge 16 commits into
masterfrom
codex/specialization-aware-cfg

Conversation

@thiremani

@thiremani thiremani commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a cold recursive-specialization discovery fuse that rejects the 257th active frame before cache allocation without limiting flat specialization breadth
  • split the template CFG pass into structural-only validation and run effect-sensitive dataflow once per settled type specialization
  • reuse specialization call-graph topology for effect settlement and CFG publication while caching complete direct-callee keys separately
  • publish immutable CFG diagnostics before FuncInfo.Settled, then replay and deduplicate diagnostics from each script's reachable specialization closure
  • preserve print reads, scalar companion variants, simultaneous assignment, ReadsSeed, and sparse MustWrite/MayWrite behavior
  • delete the duplicated syntactic effect classifiers and mark PIR Step 2B complete in the durable plan

Fixes #79.

Why

Step 2A made write and yield effects solver-owned, but the existing CFG still performed a syntax-approximate pass over every template and only analyzed the script after type solving. Function specializations never received effect-aware CFG analysis, cached specializations could not replay diagnostics, and recursive specialization discovery could grow without reaching SCC settlement.

This change makes CFG diagnostics specialization-aware and extends the invariant that a settled specialization has published effects, complete direct-call edges, and a non-nil immutable CFG result.

The recursive guard is an operational fuse for cold, unsettled inference work. It is not a limit on total functions, total specializations, array rank, or flat/deep acyclic code. Settled cache hits do not consume it. Unreachable .pt templates intentionally receive structural checks only; dead-store and write-after-write diagnostics begin when a specialization becomes reachable.

Review guide

  1. Recursive discovery guardcompiler/solver.go and the specialization guard tests in compiler/solver_test.go. The guard starts at the earliest repeated template in the active inference path, caps that recursive region at 256 frames, rejects before FuncCache allocation, and emits one bounded diagnostic even when later sibling calls reuse an unsettled key.
  2. Structural template checkscompiler/codecompiler.go and the structural portion of compiler/cfg.go. Templates are checked once for type-independent errors; reads are validated before let destinations are published.
  3. Typed CFG dataflow — the specialization/script analyzers and event transfers in compiler/cfg.go. Event order is explicit reads, ReadsSeed, then sparse writes; MustWrite and MayWrite retain their distinct kill behavior.
  4. Graph and atomic publicationcompiler/effects.go, compiler/solver.go, and compiler/types.go. Batch-local primary edges feed SCC settlement; complete persistent direct edges feed replay. CFG results are staged and published before any Settled bit is flipped.
  5. Script replaycompiler/scriptcompiler.go and compiler/cfg_replay_test.go. Replay is deterministic, specialization-key visited, source-diagnostic deduplicated, and limited to the script's reachable closure.

The large deletion in compiler/cfg.go removes the syntax-only effect classifiers whose responsibility is now owned by solver-published StatementEffects and YieldEffects.

Validation

  • eval "$(python3 scripts/llvm_env.py --shell)" && go test -race ./lexer ./parser ./compiler -count=1
  • python3 test.py --leak-check — 74 passed, zero detected leaks
  • git diff --check 92fd5ac..HEAD

Record the growth-guard prerequisite, settled-specialization CFG lifecycle, cache publication rules, replay semantics, migration hazards, and acceptance tests for PIR Step 2B.
Cap each script solve at 256 unique unsettled specializations before shared-cache allocation, while settled warm hits remain free. Report a bounded active specialization-key chain and preserve finite polymorphic recursion by making the budget the sole termination guarantee.

Add direct, mutual, retry, fixed-closure, scalar-companion, and lifecycle regressions; align the PIR and Step 2B plans with the sound budget-only policy.
Split template validation into structural checks and run effect-sensitive dataflow for each settled specialization. Cache immutable CFG diagnostics and complete direct-call edges, then replay the reachable closure deterministically for each typed script.

Reuse specialization graph topology for effect settlement and CFG publication, preserve print and ReadsSeed events, and remove the duplicated syntactic effect classifiers.
Record the implemented Step 2B lifecycle, cache invariants, event ordering, and validation status in the durable PIR plan. Remove the temporary execution plan now that its completion criteria pass; it remains available in Git history.
@thiremani

Copy link
Copy Markdown
Owner Author

Review

Reviewed at 2a40e28 against master (92fd5ac). I built both compilers and reproduced every behavioral claim below; go test ./compiler and python3 test.py (74/74) both pass on the branch, so nothing in the existing suite regresses.

Verdict. The central idea is right and the pass split is genuinely necessary — template-time syntactic effect classification was an approximation, and this PR retires the whole mirrored classifier family (funcDestWriteKinds/funcValueMaySkip/funcNodeMayNotYield/callRootMaySkip/hasRangeExpr) in favor of solver-owned effects. That deletion is the strongest part of the change. It is not minimal: the specialization budget is an independent change bolted on, and there are two user-visible diagnostic defects that I think block merge. Suggested path: split the budget commit out, fix the replay duplication, then this is close.


Blocking

1. The 256-specialization budget rejects ordinary valid programs

compiler/solver.go:16 bounds unique unsettled specializations per Solve, but the hazard it is documented to guard (docs/Pluto IR Plan.md, "Finite specialization closure") is expanding recursive type growth. Those aren't the same thing, so the guard fires on flat, finite, entirely valid code.

Repro — 300 distinct one-line functions, no recursion, no type growth:

lib.pt:   res = F0(x)
              res = x + 0
          ... (300 of them)
main.spt: acc = 0
          acc = acc + F0(acc)
          ... (300 of them)
          acc
master: ✅ Successfully built binary for script: main
PR89:   /main.spt:258:13:specialization budget exhausted (limit 256 per Solve);
        active signature chain: F256(I64)

257 ordinary functions is not a large program, so this is a hard false positive on a real limit. Two secondary problems visible in that message: the "active signature chain" degenerates to a single frame here, so the diagnostic points at nothing actionable; and because settled specializations are free (solver.go:2674, solver.go:2574) while FuncCache is module-wide, whether b.spt compiles can depend on whether a.spt warmed the cache first — i.e. on filename order.

The budget should count what it claims to bound. Either charge only specializations reachable through a growing signature chain, or key the bound to nesting depth of the discovery stack rather than the flat unique count.

2. A single .pt diagnostic is now emitted once per specialization, and once per script

replaySpecializationCFGNode dedupes on the mangled specialization (compiler/scriptcompiler.go:81), but AnalyzeSpecialization (compiler/cfg.go:291) derives diagnostics from the shared template body. Every specialization therefore carries its own copy of the same template-line error.

lib.pt:   res = Foo(x)
              t = x + 1
              res = x
main.spt: a = Foo(1)
          b = Foo(2.0)
          a, b
master: /lib.pt:2:5:value assigned to "t" is never used
PR89:   /lib.pt:2:5:value assigned to "t" is never used
        /lib.pt:2:5:value assigned to "t" is never used

This also fires for a single call site whenever ScalarCallVariantEnsured adds the scalar companion as a second replay key (compiler/effects.go:562) — m = 1:4 / v = Sq(m) prints the body's dead store twice.

Separately, two scripts calling one noisy .pt function report it once each, and each copy is trailed by error compiling scriptFile .../a.spt for script a — blaming a .spt for a .pt defect. Master reported it once under error while compiling code module.

TestDiamondCFGReplayIsOnceAndDeterministic (compiler/cfg_replay_test.go) can't catch either case: it asserts pointer identity for a shared node, and each specialization allocates fresh *CompileError values.

Fix: dedupe the accumulated slice on (file, line, column, msg) in replaySpecializationCFG. Sorting by that same key before returning would also fix note 3 below, in one pass.

3. Unreachable .pt templates lose dataflow checking entirely — including exit code

Acknowledged in the PR body, and I agree with the tradeoff for reachable code. But the consequence is stronger than "structural checks only":

master: /lib.pt:2:5:value assigned to "unused" is never used   → exit 1
PR89:   ✅ Successfully built binary for script: main          → exit 0

A library-only package gets zero WAW/dead-store analysis and a clean build. That deserves a line in the PR description, not just the design doc. Also, TestUnreachableFunctionDataflowDiagnosticsAreDeferred (compiler/cfg_test.go:49) asserts require.Empty and calls this deferred — nothing ever re-runs those checks. Rename it to say dropped.


Structure, reuse, minimality

Well factored. SpecializationCFGResult as an immutable cached value (compiler/types.go) with staging installed before any Settled = true is a clean contract, and settleSpecializationBatch (compiler/solver.go:2647) reads clearly. The ensureScalarCallVariant rewrite is load-bearing rather than drive-by — ScalarCallVariantEnsured is the only signal collectSpecializationCallEdges has for the distinct companion edge — so it belongs here.

Not minimal. Commit 3e89704 (the budget) touches only solver.go, solver_test.go, and docs, and none of settleSpecializationBatch, buildSpecializationCallGraph, AnalyzeSpecialization, or replaySpecializationCFG call into it. It's a separable PR, and given blocking item 1 it needs its own design discussion anyway.

New duplication. validateFuncTemplate (compiler/cfg.go:204) and typedForwardPass (compiler/cfg.go:314) run the identical five-step walk — collectStatementReads → append reads.ErrorsvalidateStructuralRead per event → validateStructuralWrite per non-discard target → publishTargets — differing only in the inputNames map. On master these were shared through checkRead/checkWrite inside processForwardEvents, so this is a step backwards on checklist item 3. Extract validateStructuralStatement(stmt, reads, inputs) covering the error-append plus the two validation loops — but not publishTargets, since typedForwardPass deliberately publishes after typedStatementEvents and the ReadsSeed assertion would go vacuous otherwise.

Compounding it: typedForwardPass(..., structural bool) has exactly two call sites, each passing a literal. One function meaning two things by call-site boolean is worth avoiding; at minimum rename it validateStructure.

Abstractions that don't earn their keep.

  • activeSpecialization (solver.go:146) is a struct with one string field.
  • collectDirectCallees (effects.go:556) is a wrapper that exists to drop a return value.
  • The settled specialization %s has no CFG result panic appears four times for an invariant with one producer. The copy in callBodyOutputEffects (effects.go:293) is the odd one — that function returns BodyOutputEffects and doesn't touch CFG. Keep the one at the replay boundary. Six lines above it, f := analyzer.compiler.FuncCache[mangled] then f.Settled (effects.go:290) is an unguarded nil deref — inconsistent failure modes for the same class of invariant.

Readability. validateStatementEffect (cfg.go:374) restates properties deriveLet establishes by construction 20 lines away, in eight inline panics, on every statement of every specialization. A property test over deriveLet plus the bounds check would carry the same guarantee with less noise. Some of the new comments are design prose that belongs in the plan doc rather than the source.


Behavior changes worth stating in the PR body

tests/math/acc.spt — deleting reversedHalf = 0.0. This is a user-visible strictness change, not a test tidy-up. Verified: master compiles the original file, the branch rejects it:

/acc.spt:3:1:unconditional assignment to "reversedHalf" overwrites a previous value that was never used…
/acc.spt:1:1:value assigned to "reversedHalf" is never used

The mechanism is exactly the intended one — master's callRootMaySkip treated every non-builtin bare call root as possibly-skipping, and typed MustWrite now kills the preceding store. The diagnostic is correct and consistent with rejecting x = 1 / x = 2; note res = 10 / res = Acc(res, 1:6) still compiles because the seed is read as an argument. Only seeds of pure call outputs break, and those are genuinely useless.

But no test on the branch pins the new strictness for a direct call output — the existing unconditional assignment cases are collectors, array masks, and operator-fed calls, all of which already passed on master. Worth one negative case in cfg_test.go's error table. Same story for the bare h added in compiler_test.go.

One doc inconsistency: docs/Pluto IR Plan.md still says the forward rule "cures today's conditional-write false positive, which currently forces tests to interleave reads" — while this PR adds an interleaved read for a new true positive. That paragraph now reads as the opposite of what shipped.


Smaller notes

  • compiler/scriptcompiler.go:53 — replayed diagnostics follow the script's DFS discovery order, and land after the script's own errors, so output is no longer grouped by file (main.spt error, then lib.pt error). Master emitted .pt diagnostics in code-module source order; TestFunctionDiagnosticsFollowSourceOrder pinned that and was deleted here. Sorting the replayed slice by (file, line, column) restores it.
  • compiler/solver.go:219formatSpecializationChain truncates when len(parts) > 8 but always emits 9 parts, so a 9-frame chain hides exactly one frame and replaces it with a longer "1 specializations omitted" placeholder. Truncate only when it actually saves output, and fix the plural. The count arithmetic itself is correct.
  • compiler/solver.go:191 — the truncation bounds frame count but not frame size; for type-growing recursion each retained frame's demangled name grows with depth, so the message can reach tens of KB. Cap rendered length in specializationDisplay.
  • specializationDisplay runs eagerly (full DemangleParsed + Sprintf + Join) on every non-settled walk, for a message that fires only on exhaustion. Store mangled names and render lazily.
  • compiler/solver.go:2672 — the recordSpecializationDiscovery call in TypeFunc is effectively unreachable as a discovery site: every production entry is preceded by InferFuncTypes recording the same mangled and returning early on failure. It duplicates accounting with a worse blame token (template.Token vs ce.Function.Token) and makes the budget look like it has two enforcement points.
  • compiler/cfg_test.gorunCFGTest now drives sc.Compile(), and ScriptCompiler.Compile returns solver errors before building the CFG, which is why three expectations flipped from variable "y" has not been defined to undefined identifier: y. Those cases no longer exercise CFG.validateStructuralRead at all. Also, no test observes the budget diagnostic through sc.Compile() — every budget test reaches in via ts.specializationLimit or ts.InferFuncTypes.

Bound only the active recursive inference region before cache allocation, so broad and acyclic specialization sets remain unrestricted. Keep the guard an honest cold-discovery resource fuse and add coverage for wide cycles, retries, warm tails, and flat breadth.
Preserve immutable diagnostics on every specialization while presenting each source-identical defect once per script replay. Keep specialization-key traversal separate so unique scalar or typed diagnostics are never hidden.
Keep read and formatting validation pure until typed statement events have been constructed, then publish destinations at the caller. Add reachable-specialization and proven MustWrite regressions and remove an unnecessary ABI test seed.
Document the recursive-region resource fuse, structural-only unreachable templates, proven MustWrite strictness, and source-stable diagnostic set union across reachable specializations.
@thiremani

Copy link
Copy Markdown
Owner Author

Follow-up review — 97d0f53d

Re-reviewed the four fix commits (2a40e28..97d0f53d) and re-ran every repro from my earlier comment against a build of the new head. All three blocking issues are fixed, and the budget fix is a real redesign rather than a threshold bump. One new regression, minor, introduced by a178449.

Repro 2a40e28 97d0f53d
300 flat non-recursive functions budget exhausted ✅ builds
Foo(1) + Foo(2.0) shared dead store printed 2× printed 1×
scalar companion, single call site printed 2× printed 1×
self-expanding recursion caught caught, controlled
mutual expanding recursion (Ping/Pong) caught
valid finite recursion (Fact(10)) ✅ builds
trace message size ~16 KB 1358 B

go test -race ./lexer ./parser ./compiler and python3 test.py (74/74) both pass.

Scoping the limit to a recursive inference region is the right call, and I think it's sound rather than merely stricter-in-practice: with finitely many templates, any chain deeper than the template count must repeat a template (pigeonhole), so an expanding recursion always establishes a region and cannot outrun the bound — while flat breadth and deep acyclic chains cost nothing. TestRecursiveSpecializationLimitAllowsFlatBreadthColdAndWarm and TestWideMutualGrowthHitsOneRecursiveRegionLimit pin both halves of that.

Two things you did better than what I suggested: dropping the h = 0.0 seed in compiler_test.go outright rather than adding the interleaved read I proposed; and turning TestUnreachableFunctionDataflowDiagnosticsAreDeferred into TestFunctionDataflowDiagnosticsWaitForReachableSpecialization plus an assertion that the diagnostics really do appear once the specialization is reachable — that makes the name true instead of just honest.


New: guard trip emits a spurious "is not converging" error per extra call in the statement

a178449 removed both the else if !f.Settled && !recordSpecializationDiscovery(...) { return f } short-circuit from InferFuncTypes and the guard call from TypeFunc, leaving allowSpecializationAllocation (compiler/solver.go:2637) as the only check — and it fires only on the uncached (!ok) path.

Once specializationGuardFailed is set, every cached-but-unsettled specialization reached later in the same statement is re-walked in full. New allocations at the bottom are denied silently (solver.go:204), so no new error is appended, the len(ts.Errors) > errsAtEntry early return doesn't fire, and TypeScriptFunc ends up with unresolved outputs and Converging == false — hitting the !ts.Converging branch at solver.go:2694 and appending a second diagnostic that contradicts the first and blames the template.

lib.pt:   res = Grow(x)
              res = Grow([x])
main.spt: v = Grow(1) + Grow([1])
          v
97d0f53d: /lib.pt:2:11:recursive specialization resource limit exceeded (limit 256 …)
          /lib.pt:1:7:Function Grow is not converging. Check for cyclic recursion and
                      that each function has a base case      ← spurious
2a40e28:  /lib.pt:2:11:specialization budget exhausted (limit 256 per Solve); …

Isolation: one call → 1 error; two calls in separate statements → 1 error (Solve aborts between statements); four calls in one statement → 3 spurious errors. So the count is calls-in-statement − 1, and each one costs a full ~256-frame re-walk.

It only degrades an already-failing compile, hence minor — but the second message actively misdirects, pointing at "check for a base case" when the real cause is the resource limit already reported one line above.

Fix: restore the short-circuit on the cached path — in InferFuncTypes, after the if !ok block, if ts.specializationGuardFailed && !f.Settled { return f }. Alternatively suppress the non-convergence diagnostic in TypeScriptFunc (solver.go:2694-2701) when the guard has tripped. Worth a regression test with two script-level calls to the growing function in one statement asserting exactly one error — TestUnboundedSpecializationGrowthHitsRecursiveLimitWithActiveChain misses it because it uses a single call with require.Len(ts.Errors, 1).


Still open (all minor, all fine to decline)

  • A shared .pt defect is still reported once per calling script, each trailed by error compiling scriptFile .../a.spt — inherent to per-script error lists now that dedup is per-script. Only worth revisiting if the duplicate output annoys in practice.
  • Replayed diagnostics still aren't sorted by (file, line, column), so output reads main.spt error → lib.pt error rather than grouped by file.
  • collectDirectCallees (effects.go:556) is still a wrapper that exists to drop a return value.
  • "has no CFG result" still appears 4× for a one-producer invariant, including in callBodyOutputEffects (effects.go:293), which returns BodyOutputEffects and never reads CFG.
  • effects.go:290 still derefs FuncCache[mangled] unguarded, six lines above three carefully worded ICE panics.
  • validateStatementEffect still carries 9 inline panics restating what deriveLet establishes by construction.

Nothing above blocks. With the specializationGuardFailed short-circuit restored, I'd call this ready to come out of draft.

(One candidate finding — that the local key := in the new dedup loop shadows the package-level key() — I checked and dismissed: that shadow already exists three times on master (compiler.go, codecompiler.go, solver.go), so the new code follows the existing convention.)

Return cached unsettled functions immediately after the recursive discovery fuse reports an error. This prevents sibling calls in the same statement from appending contradictory non-convergence diagnostics.
@thiremani

Copy link
Copy Markdown
Owner Author

Sign-off — 5ff5074

Verified the fix and re-ran the full regression set. Ready to come out of draft from my side.

5ff5074 is the minimal correct fix. The guard-failed short-circuit sits after the if !ok block, so it can only affect the cached path — on the uncached path allowSpecializationAllocation has already returned true, which means specializationGuardFailed cannot be set there. Confirmed against my repro:

lib.pt:   res = Grow(x)
              res = Grow([x])
main.spt: v = Grow(1) + Grow([1]) + Grow([[1]]) + Grow([[[1]]])
97d0f53d 5ff5074
spurious not converging errors 3 0
total diagnostics (2-call form) 2 1

TestRecursiveSpecializationFailureStopsSiblingRewalks pins exactly this shape with require.NotContains(..., "not converging"). go test -race ./lexer ./parser ./compiler and python3 test.py (74/74) pass locally, matching CI.

On the limits — agreed, and I checked each claim

Every one holds up empirically on 5ff5074:

  • 300 unrelated specializations compile — cold and warm; TestRecursiveSpecializationLimitAllowsFlatBreadthColdAndWarm really does use specializationCount = 300.
  • Acyclic call depth is not limited — a 300-deep chain F0 → F1 → … → F299 builds fine.
  • Array rank is not limited — a rank-200 literal builds fine.
  • Frame 257 is the rejection point — the emitted chain reports 249 specializations omitted out of 257 frames, consistent with a limit of 256.

So 256 is not too small, and the fuse's scope is exactly as described.

Fixes #79 is accurate. I confirmed the original repro still hangs on master (CPU-bound, killed at 60s, no output) and now terminates on this branch with a single controlled diagnostic.

The layered plan is right, and I have evidence the remaining layers are load-bearing rather than theoretical: a rank-2000 nested literal (Id([[[…1…]]])) does not complete within 90s on 5ff5074. That is a genuinely different vector from #79 — no recursive template, so the frame fuse correctly never engages — which is precisely the gap #90 describes. Agreed too that C ABI / linker symbol limits are the wrong safeguard: they fail late, after the compiler has already done the expensive work, and the diagnostic would point at the linker rather than the source.

One thing worth writing into #90 explicitly: a rank cap of 32 or 64 narrows currently-accepted behavior — rank 200 compiles today. Almost certainly nobody cares, but it's a deliberate narrowing rather than pure hardening, so it deserves a line in the issue and a note in the release notes rather than being discovered by whoever first trips it.

Tracking issues

#90, #91, and #92 between them cover every open item from my earlier reviews — #91 takes the diagnostic ordering and the per-script attribution trailer, #92 takes the repeated has no CFG result panics, the unguarded FuncCache deref at effects.go:290, and the assertion density in validateStatementEffect. Splitting them out rather than growing this PR is the right call.

Nothing blocking left.

Return read events directly and report format-specifier diagnostics through the CFG error sink. Preserve the read-before-destination publication order without a parallel result wrapper.
Split function binding classification and body validation into focused helpers, reducing validateFuncTemplate complexity. Mark impossible CFG block and global-scope underflows as internal compiler errors.
Treat every parsed function parameter as an input, matching the parser invariant that parameter and output names cannot overlap. Remove the unreachable membership guard and use parameter-oriented naming throughout structural CFG validation.
Build parameter and output membership sets at their only call site now that no additional binding policy is involved.
Remove the redundant nil guard from AnalyzeSpecialization; the solver validates and dereferences every walked specialization before CFG settlement.
Treat scripts as zero-input, zero-output templates before typed dataflow so the forward pass no longer needs a structural-validation mode. Validate sparse statement-effect shape at solver publication, leaving CFG to enforce only missing facts, invalid write states, and seed scope.
Continue typed CFG dataflow after structural script diagnostics by reusing the structural pass's explicit reads in a fresh scope. This reports independent dead-store and write-after-write findings without duplicating format errors, and reports every missing dynamic specifier binding in one compile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recursive call with a growing argument type hangs the compiler

1 participant