Skip to content

perf(ir/lower): tail-call fusion — :return of a single-use :call → TAIL_CALL - #649

Merged
mparrett merged 3 commits into
nooga:mainfrom
nnunley:salvage-ir-tailcall-fusion
Aug 18, 2026
Merged

perf(ir/lower): tail-call fusion — :return of a single-use :call → TAIL_CALL#649
mparrett merged 3 commits into
nooga:mainfrom
nnunley:salvage-ir-tailcall-fusion

Conversation

@nnunley

@nnunley nnunley commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Tail-call fusion emits TAIL_CALL for a single-use :call in return position, enabling frame reuse.

Test coverage

  • test/ir_tailcall_fusion.lg: bytecode/IR parity, opcode falsifier, and try-frame isolation coverage

Base

Rebased directly onto main at a759f8655eb8.

Verification

  • make generate twice: byte-identical generated.sums, generated.manifest, and core_compiled.lgb
  • make check-generated: passed
  • go test ./pkg/ir ./test: passed
  • make ir-stress-gate: passed; 2505/2516 native-lowering fixtures, 11 known baseline failures
  • Push gates: compiled-output guard, Go tests, compatibility suite, and IR stress ratchet passed

Bench ratchet

make bench-ratchet ran in the exclusive benchmark lane on the candidate and on main@a759f8655eb8 using Apple M3 / Go 1.26.5. Both runs returned exit 2 against the Go 1.26.3 baseline with the same two benchmark groups over budget.

Candidate vs main control wall time:

  • compiler init: 1.98 ms vs 2.02 ms (-2.0%)
  • IR compile, bytecode: 20.4 ms vs 20.9 ms (-2.4%)
  • IR compile, gogen_ir: 14.4 ms vs 14.6 ms (-1.4%)

Reported deterministic candidate-minus-main deltas:

  • compiler: 0 allocs/op, -3 bytes/op
  • IR compile, bytecode: 0 allocs/op
  • IR compile, gogen_ir: +46 allocs/op, -5,349 bytes/op

@mparrett mparrett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You flag that this includes [2/4]; the part worth pinning down is which order costs what. Merge #648 first and this rebases cleanly — git reports skipped previously applied commit 37ccb333 and leaves one commit. The other direction is messier: merge this first and #648 does reduce to an empty diff, but only after resolving a pkg/rt/generated.sums conflict by hand. So #648 first, and worth retargeting this PR's base to #648's branch rather than main — right now GitHub shows reviewers the combined diff and both PRs read MERGEABLE independently, which hides the dependency you documented in the body.

The fusion pairs well with where #620 is heading. That issue was rewritten to stack on #645, and its remaining goal is to change what OP_TAIL_CALL does: reuse the current frame instead of allocating a child. This PR increases how much of the IR path reaches that opcode, so more code inherits the constant-space win once #620 lands.

I went looking for an interaction with #638 — a callee-less terminator becoming OP_TAIL_CALL, with the VM then faulting as it resolves f.nth(arity) — and there isn't one. The fusion predicate fires on a :return whose ref is a :call, and a :call always carries fn+args with aux as argc, so a fused emission always has a callee in the right slot. A function-level recur never reaches it: build-recur adds a :tail-call terminator directly, not a :return, so the predicate can't see it before or after #642. I confirmed #638's repro fails identically on main and on this branch. Recording it here so the question doesn't get re-raised.

@mparrett mparrett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of the current head: the fusion implementation looks sound, the isolated delta against #648 changes the lowered sequence from INVOKE, RETURN to TAIL_CALL, RETURN, targeted tests pass, and required checks are green.

One remaining coverage gap: test/ir_tailcall_fusion.lg only asserts result parity, so it still passes if the fusion is removed; strict mode is also off, so fallback can hide a lowering failure. Please compile the IR fixtures under *ir-compile-strict* and add a disassembly assertion for :TAIL_CALL (and absence of :INVOKE). Also, the line-77 comment that the plain compiler uses regular INVOKE is inaccurate—the plain compiler already emits TAIL_CALL in tail position.

The stack dependency remains: expecting rebase/refresh after #648 lands.

nnunley added a commit to nnunley/let-go that referenced this pull request Aug 5, 2026
@nnunley
nnunley force-pushed the salvage-ir-tailcall-fusion branch 2 times, most recently from 616ee44 to f6e3646 Compare August 5, 2026 01:23
nnunley added a commit to nnunley/let-go that referenced this pull request Aug 5, 2026
nnunley added a commit to nnunley/let-go that referenced this pull request Aug 11, 2026
regen: resolve generated artifacts after main rebase
@nnunley
nnunley force-pushed the salvage-ir-tailcall-fusion branch from f6e3646 to 4d25a20 Compare August 11, 2026 01:44
@mparrett

mparrett commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Non-blocking, same class of ask as #719 just got: the fusion's try-safety is a documented-but-untested invariant. The gate comment says a fusable :call can never sit in a guarded region because the IR models try bodies as inner closures — true today, but nothing pins it, and test/ir_tailcall_fusion.lg has no try/catch case. If the IR ever moves try bodies into the same frame, the fusion would emit a TAIL_CALL past a live handler with every existing test still green. A fixture asserting a tail-position call inside a try body does NOT fuse would make the invariant structural instead of commented.

nnunley added a commit to nnunley/let-go that referenced this pull request Aug 13, 2026
regen: resolve generated artifacts after main rebase
@nnunley
nnunley force-pushed the salvage-ir-tailcall-fusion branch from 4d25a20 to cf43894 Compare August 13, 2026 21:55
nnunley added a commit to nnunley/let-go that referenced this pull request Aug 14, 2026
regen: resolve generated artifacts after main rebase
@nnunley
nnunley force-pushed the salvage-ir-tailcall-fusion branch from cf43894 to 2d17900 Compare August 14, 2026 23:48
@nnunley

nnunley commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased and updated at 2d1790064aca. The requested “try-body call must not fuse” assertion would encode the wrong invariant: try bodies and handlers are isolated into inner function templates, so the outer frame owns :try and has zero calls, while an unguarded call in the inner frame can safely become TAIL_CALL. The replacement regression asserts that full structure: outer :try/handler metadata, zero outer calls, one inner call, TAIL_CALL, and no INVOKE. Final pre-push Go, compatibility, and IR-stress gates pass.

@mparrett mparrett added the perf-repeat Run the repeat A/B (variance-reduced) perf check label Aug 17, 2026
…IL_CALL

Extracted from nooga#625 [3/4]. In lower-block!, when a block's :return wraps a
single-use :call that is the block's last live instruction, emit TAIL_CALL
directly (RETURN kept for shape parity) instead of CALL+RETURN. Stacks on the
RPO/block-junk correctness fixes [2/4]; parity-checked against the plain
bytecode compiler.
regen: resolve generated artifacts after main rebase
@nnunley
nnunley force-pushed the salvage-ir-tailcall-fusion branch from 2d17900 to 7ef2a89 Compare August 18, 2026 02:13

@mparrett mparrett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head 7ef2a89. The prior strict-mode, opcode-shape, comment-correction, and try-frame-isolation asks are addressed; targeted and repository short-mode tests pass, generated artifacts are clean, and current CI is green. No blocking findings.

@mparrett mparrett added the review-priority/high Review/merge first: ready or unblocks the stack label Aug 18, 2026
@mparrett

Copy link
Copy Markdown
Collaborator

Verified the merge result locally before merging. What the checks covered, plus two ways this PR's fixture reports a pass that asserts nothing.

Shape. Head 7ef2a897, merge-base a759f865, which is the current tip of main, so this goes in as a fast-forward. There is no three-way merge, so the merge=sums driver never runs and the sharp edge in #747 is not reachable from this PR.

Generated artifacts. make check-generated and make check-generated-manifest both pass. make generate from the PR tree leaves the working tree clean, and the recomputed digest matches the committed generated.sums byte for byte. That is the check that failed after #641 and #733 and needed #749 and #750 to repair by hand; it does not reproduce here.

Tests and builds. go test -short -count=1 ./... is clean. Cross-builds pass for linux/amd64, darwin/arm64, js/wasm, plan9/amd64 and wasip1/wasm.

The 2026-08-04 coverage gap is closed. That review noted the fixture still passed with the fusion removed. It no longer does. Reverting pkg/rt/core/ir/lower.lg to main, regenerating, then running the fixture:

FAIL (pos? (count-op tail-in-let-ir :TAIL_CALL))
FAIL (zero? (count-op tail-in-let-ir :INVOKE))
FAIL (pos? (count (filter (fn [op] (= :TAIL_CALL op)) body-ops)))
FAIL (zero? (count (filter (fn [op] (= :INVOKE op)) body-ops)))
Tests: 3 Pass: 21 Fail: 4

Two ways to get a meaningless pass here. Neither detects a missing fusion:

  1. lg test/ir_tailcall_fusion.lg exits 0 without asserting anything; it only defines the deftests. The harness is TestRunner in test/language_test.go, which walks test/*.lg and calls (run-tests) per file.
  2. Reverting lower.lg without make generate leaves the previous lowering inside core_compiled.lgb, so the built binary still fuses. The negative control only bites after regeneration.

Two pre-existing items on main that I checked and am not raising against this PR: gofmt -l reports test/e2e/native_entry_matrix_test.go unformatted, which this PR does not touch and the lint job passes over; and the boot-budget smoke in make build fails on main as well on this machine, with cold medians near 8.4ms against the 8ms budget and around 5.3ms warm.

@mparrett
mparrett merged commit b3f68d6 into nooga:main Aug 18, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-repeat Run the repeat A/B (variance-reduced) perf check review-priority/high Review/merge first: ready or unblocks the stack

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants