Skip to content

refactor(asm)!: collapse linking and cut fused dispatch overhead - #162

Merged
siyul-park merged 5 commits into
mainfrom
feature/keystone-patterns
Jul 30, 2026
Merged

refactor(asm)!: collapse linking and cut fused dispatch overhead#162
siyul-park merged 5 commits into
mainfrom
feature/keystone-patterns

Conversation

@siyul-park

@siyul-park siyul-park commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

Continues the in-flight asm/prof refactor to a working state, then takes one
pass of quality -> performance -> quality over the runtime.

asm linking collapses onto a single code block. Assembler.Build returns the
finished machine code for one block, so the Code container, its external
relocation table, and the multi-entry Link contract are gone. Buffer owns the
write/execute transition around every install rather than exposing
Unseal/Append/Seal. Register allocation folds into the rewriter that already
rewrote virtual operands, so one linear-scan pass owns the physical bank, the
spill frame, and the label rebasing it forces.

prof keeps one counter table per JIT metric family, keyed by the row's label
set, with rows ordering themselves through a self-referencing constraint instead
of an interface type assertion per comparison.

Fused handlers check stack room once. A fused source stays in a temporary
instead of being pushed, so a fused handler grows the operand stack by exactly
one slot no matter how many sources it folded. Emitting a source's overflow check
only when it pushes on its own removes 5,424 generated lines and every duplicated
check.

Performance

Interleaved A/B, median of five, threaded mode:

Kernel before after delta
BranchTree(96) 959.3 ns 917.9 ns -4.3%
TypedArraySum(256) 6,446 ns 6,182 ns -4.1%
IterativeFib(30) 736.6 ns 709.9 ns -3.6%
Sieve(256) 16,342 ns 15,882 ns -2.8%

Native and adaptive modes are unchanged within noise. No kernel regressed.

docs/benchmarks.md carries a full re-measured cross-runtime table (90 rows).

Rejected simplification

Narrowing dispatch to reload the frame's handler table only on a frame
transition measured 15% slower (724 -> 843 ns on IterativeFib(30)): the
branch misprediction costs more than the unconditional slice reload. Reverted;
the unconditional reload stays.

Testing

  • make check (generated-code parity, tidy, fmt, vet, tests, arm64 cross-build)
  • go test -race ./...
  • coverage 78.8%, above the 72.8% baseline
  • cross-builds verified on darwin/linux/windows x amd64/arm64
  • two new specs in interp/interp_test.go pin the fused stack-room contract
  • internal/jitcheck is a temporary black-box harness proving the JIT still
    lowers, spills, links, and executes through interp's public API after the
    asm collapse

Breaking changes

asm.Link takes one []byte and returns one Callable. asm.Code,
asm.LabelOp, OpPseudoLabel, and RegMask.PopFirst/Count are removed.
Both are within the asm/prof surface where public API changes are in scope.

Summary by CodeRabbit

  • New Features

    • Added direct assembly and linking of generated machine-code buffers.
    • Added clear errors for unresolved labels and invalid assembly inputs.
    • Preserved callable entry points when executable buffers grow.
  • Improvements

    • Improved ARM64 branch relaxation and register allocation for wide live ranges and recursive calls.
    • Reduced redundant interpreter stack checks while preserving overflow and bounds validation.
    • Improved fused-operation stack handling and JIT execution reliability.
  • Documentation

    • Reworked coding, testing, benchmarking, and JIT guidance with updated performance results and clearer requirements.

Assembler.Build now returns the finished machine code for one block, so
the Code container, its external relocation table, and the multi-entry
Link contract are gone. Link installs those bytes into a Buffer and binds
them through the ABI, and Buffer owns the write/execute transition around
every install instead of exposing Unseal/Append/Seal to callers.

Register allocation moves into the rewriter that already rewrote virtual
operands, so one linear-scan pass owns the physical bank, the spill
frame, and the label rebasing it forces. A build containing a back-edge
runs without a spill frame, since linear-scan lifetimes only describe a
forward-only stream.

prof keeps one counter table per JIT metric family, each keyed by the
row's label set, and rows order themselves through a self-referencing
constraint rather than an interface type assertion per comparison.

Drop asm.Code, asm.LabelOp, OpPseudoLabel, RegMask.PopFirst/Count, and
the memory ptr/within helpers left unused by the collapse.

BREAKING CHANGE: asm.Link takes one []byte and returns one Callable.
A fused source stays in a temporary instead of being pushed, so a fused
handler grows the operand stack by exactly one slot no matter how many
sources it folded. Emitting a source's overflow check only when it pushes
on its own, and checking the consumer's net push once, removes 5,424
generated lines and every duplicated check.

Bounds, segmentation, and underflow checks stay per source. Trapping
arithmetic still materializes its operands on the stack, so it keeps a
check per push.

Interleaved A/B, median of five, threaded mode: BranchTree(96) -4.3%,
TypedArraySum(256) -4.1%, IterativeFib(30) -3.6%, Sieve(256) -2.8%.
Native and adaptive modes are unchanged within noise.
Compile emits one handler per code byte, so a rethreaded table describes
the same function at the same length as the one already installed.
Copying into that table instead of replacing it rewires every frame
currently executing the function, which drops the frame scan that
existed only to repoint each active frame at the new slice.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a32bead-e417-4f0e-acef-f411cc3bb4ee

📥 Commits

Reviewing files that changed from the base of the PR and between b0e6c43 and 0f655b0.

📒 Files selected for processing (42)
  • .claude/CLAUDE.md
  • AGENTS.md
  • asm/arch.go
  • asm/arm64/abi_arm64_test.go
  • asm/arm64/frame_test.go
  • asm/arm64/instr_test.go
  • asm/arm64/relax_test.go
  • asm/assembler.go
  • asm/assembler_test.go
  • asm/buffer.go
  • asm/buffer_test.go
  • asm/code.go
  • asm/instr.go
  • asm/instr_test.go
  • asm/link.go
  • asm/link_test.go
  • asm/memory.go
  • asm/memory_stub.go
  • asm/operand.go
  • asm/operand_test.go
  • asm/reg.go
  • asm/reg_test.go
  • asm/regalloc.go
  • asm/rewriter.go
  • docs/README.md
  • docs/benchmarks.md
  • docs/coding-patterns.md
  • docs/instruction-set.md
  • docs/jit-internals.md
  • docs/symbol-naming-audit.md
  • docs/testing.md
  • internal/cmd/geninterp/lower.go
  • internal/jitcheck/fixture_test.go
  • internal/jitcheck/jitcheck_test.go
  • interp/interp.go
  • interp/interp_test.go
  • interp/jit.go
  • interp/jit_arm64.go
  • interp/threaded.go
  • prof/collector.go
  • prof/jit.go
  • prof/jit_metrics.go

📝 Walkthrough

Walkthrough

The PR replaces relocation-based assembler linking with direct byte encoding and single-callable installation, restructures ARM64 register rewriting, updates interpreter fusion and JIT execution paths, refactors profiling counters, and rewrites repository coding and testing guidance.

Changes

Assembler and executable-code pipeline

Layer / File(s) Summary
Encoding contracts and public assembler surface
asm/assembler.go, asm/instr.go, asm/operand.go, asm/reg.go, asm/arch.go
Assembler builds resolved byte slices, removes external label entry handling, simplifies instruction helpers, and narrows several exported APIs.
Buffer installation and single-code linking
asm/buffer.go, asm/link.go, asm/memory*.go, asm/*_test.go
Buffers install executable bytes through direct permission transitions, retain old mappings after growth, and asm.Link returns one callable.
ARM64 rewriting and exported-contract validation
asm/rewriter.go, asm/arm64/*_test.go, asm/*_test.go
Register allocation, spilling, label injection, and ARM64 relaxer tests are reworked around exported package contracts.

Interpreter and JIT execution

Layer / File(s) Summary
Fusion stack-room validation
internal/cmd/geninterp/lower.go, interp/threaded.go, interp/interp_test.go, docs/instruction-set.md
Stack-room checks are separated from other checks and fused-source behavior is tested for net capacity validation and pre-read overflow.
In-place handler rethreading
interp/interp.go
Compiled handlers are copied into installed slices while exits and stubs are updated; redundant handler overflow checks are removed.
JIT lowering integration and runtime fixtures
interp/jit.go, interp/jit_arm64.go, internal/jitcheck/*
JIT lowering uses updated labels and byte-oriented linking, with runtime fixtures covering loops, recursion, and wide live ranges.

JIT profiling metrics

Layer / File(s) Summary
Counter storage and metric rendering
prof/collector.go, prof/jit.go, prof/jit_metrics.go
Metric counters use stable registration, separate maps, shared aggregation helpers, lookup-table labels, and generic row ordering.

Repository guidance and documentation

Layer / File(s) Summary
Normative coding and workflow guidance
.claude/CLAUDE.md, AGENTS.md, docs/coding-patterns.md, docs/testing.md, docs/README.md
Repository workflow, coding, testing, buffer invariant, and documentation-routing guidance is rewritten and renumbered.
Runtime and assembler documentation
docs/instruction-set.md, docs/jit-internals.md, docs/symbol-naming-audit.md
Fusion, ARM64 JIT, branch relaxation, and private symbol documentation is updated.
Benchmark records
docs/benchmarks.md
Benchmark methodology, measurements, summaries, and result tables are refreshed.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

  • siyul-park/minivm#115 — It concerns overlapping interpreter and JIT runtime areas modified by this PR.

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/keystone-patterns

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.39955% with 171 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.13%. Comparing base (b0e6c43) to head (0f655b0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
asm/rewriter.go 59.52% 75 Missing and 10 partials ⚠️
prof/jit_metrics.go 67.96% 31 Missing and 2 partials ⚠️
asm/assembler.go 67.27% 17 Missing and 1 partial ⚠️
asm/buffer.go 0.00% 17 Missing ⚠️
asm/instr.go 74.19% 6 Missing and 2 partials ⚠️
interp/interp.go 0.00% 4 Missing ⚠️
interp/jit.go 0.00% 4 Missing ⚠️
asm/link.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #162      +/-   ##
==========================================
+ Coverage   30.29%   32.13%   +1.84%     
==========================================
  Files          86       85       -1     
  Lines       59886    55920    -3966     
==========================================
- Hits        18141    17970     -171     
+ Misses      40467    36813    -3654     
+ Partials     1278     1137     -141     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@siyul-park
siyul-park marked this pull request as ready for review July 30, 2026 14:04
@siyul-park
siyul-park merged commit 894824f into main Jul 30, 2026
7 checks passed
@siyul-park
siyul-park deleted the feature/keystone-patterns branch July 30, 2026 14:04
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.

1 participant