Skip to content

cranelift: route OP_WINDOW / OP_WINDOW_VIEW through extern helpers#386

Merged
danieljohnmorris merged 2 commits into
mainfrom
fix/cranelift-window-view-unbail
May 18, 2026
Merged

cranelift: route OP_WINDOW / OP_WINDOW_VIEW through extern helpers#386
danieljohnmorris merged 2 commits into
mainfrom
fix/cranelift-window-view-unbail

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

Cranelift's JIT used to bail out (JitCallError::NotEligible) on any function containing OP_WINDOW or OP_WINDOW_VIEW, forcing entire hot paths back to the VM dispatcher. The reason: the VM dispatcher's OP_WINDOW emits HeapObj::ListView strides, and Cranelift's inlined LISTGET / FOREACHPREP / FOREACHNEXT fast paths read Vec metadata at fixed offsets that would UB on a view's struct layout.

Fix: emit calls to the existing jit_window / jit_window_view extern helpers in both arms. Both helpers always return owning HeapObj::Lists (never views), so subsequent inlined foreach reads operate on the expected Vec layout and remain sound. We trade the VM dispatcher's per-stride ListView win for the much larger JIT-compiled outer loop, which is exactly the trade bio-canonical-shape flt p (window k xs) needs.

Repro before / after

f xs:L n>L (L n);window 3 xs on --run-cranelift:

  • before: silently fell back to VM dispatcher (no error, but the surrounding function never benefited from JIT).
  • after: compiles in JIT, full hot path runs native.

What's in the diff

  • cranelift: route OP_WINDOW / OP_WINDOW_VIEW through extern helpers (src/vm/jit_cranelift.rs) — replaces the return None bail with calls to helpers.window (3 args) and helpers.window_view (5 args). OP_WINDOW_VIEW is a two-word op; skip_next = true consumes the data word holding the n register, mirroring OP_SLC / OP_LST / OP_MSET / OP_RGXSUB. Stale audit comments referencing the bail in OP_FOREACHPREP / OP_LISTGET remain accurate (no view reaches them under JIT because the helpers materialise to Lists).
  • test: cover OP_WINDOW / OP_WINDOW_VIEW on cranelift JIT (tests/regression_window_cranelift.rs, examples/window-cranelift-jit.ilo) — 5 cross-engine regression tests across tree / VM / cranelift covering plain window, foreach-over-window, the fused flt p (window n xs) shape (OP_WINDOW_VIEW), size-1 windows, and n>len. Example file pins the same shapes for the engine harness.

Test plan

  • cargo test --release --features cranelift --test regression_window_cranelift — 5/5 pass
  • cargo test --release --features cranelift — full suite green
  • cargo fmt --check clean
  • cargo clippy --release --features cranelift --all-targets -- -D warnings clean
  • Smoke: ilo --run-cranelift 'main xs:L n>L n;ws=window 3 xs;hd ws' 1,2,3,4,5[1, 2, 3]

Follow-ups

  • A future PR can teach the inlined LISTGET / FOREACHPREP / FOREACHNEXT paths to detect HeapObj::ListView via a [ptr+0] == 1 discriminant guard, fall back to the listget helper on mismatch, and then let OP_WINDOW emit views directly. That recovers the per-stride ListView win that the VM dispatcher currently has exclusively, on top of JIT-compiled outer loops. Out of scope here; the helper-routing path is the smaller, safer first step.

Both arms previously bailed JIT compilation (`return None`) because the
VM dispatcher's OP_WINDOW emits `HeapObj::ListView` strides and the
inlined LISTGET / FOREACHPREP / FOREACHNEXT fast paths read Vec metadata
at fixed offsets that would UB on a view's struct layout. Any function
touching window opcodes fell back to the VM dispatcher.

Fix: emit calls to the existing `jit_window` / `jit_window_view` extern
helpers. Both helpers always return owning `HeapObj::List`s (no views),
so subsequent inlined foreach reads operate on the expected Vec layout
and remain sound. The function compiles, the rest of the hot path runs
JIT-native (notably bio-canonical-shape `flt p (window k xs)`), and we
trade the VM dispatcher's per-stride ListView win for the much larger
JIT-compiled outer loop.

OP_WINDOW_VIEW is a two-word encoding; the data word at ip+1 holds the
n register in its A field. `skip_next = true` consumes it, mirroring
OP_SLC / OP_LST / OP_MSET / OP_RGXSUB.
Cross-engine regression test exercises every shape that previously bailed
JIT compilation: plain `window`, foreach over a window result, and the
fused `flt p (window n xs)` shape that triggers OP_WINDOW_VIEW. Each
asserts parity across `--run-tree`, `--run-vm`, and `--run-cranelift`.

Companion `examples/window-cranelift-jit.ilo` pins the same shapes for
the engine harness so future agents see the now-correct behaviour.
@danieljohnmorris danieljohnmorris merged commit 9cf36ed into main May 18, 2026
4 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/cranelift-window-view-unbail branch May 18, 2026 12:21
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

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