JIT walker: direct dispatch for STORE_SUBSCR, CallMayForce, and Reraise - #158
Merged
Conversation
…execute_store_subscr wrapper Add `bh_execute_store_subscr` to `pyre-interpreter::opcode_ops` as a C-ABI bridge over `crate::pyopcode::execute_store_subscr::<PyFrame>` (whose `Result<StepResult<_>, PyError>` does not fit a residual_call's single- register Ref result slot). Errors propagate via `majit_metainterp::blackhole::BH_LAST_EXC_VALUE`, success returns non-zero. Register the bare `"execute_store_subscr"` path in `pyre_interpreter::jit_trace_fnaddrs()`. Without this entry the build-time codewriter falls back to `symbolic_fnaddr_for_path` for the `Instruction::StoreSubscr#28` arm's residual_call_r_r, mints a DefaultHasher 64-bit value, and `runtime_fnaddr_patch` leaves the hash in `JitCode.constants_i` because no runtime entry matches the bare path. The registration makes the patcher swap the build address for the wrapper's runtime address, satisfying sub-slice 4's 47-bit fnaddr-sanity gate. Assisted-by: Claude
- try_execute_residual_call_via_executor (jitcode_dispatch.rs:3565): clear
ctx.last_exc_value / last_exc_value_concrete on the success path so a
prior raising helper in the same walk does not leak past a later
non-raising residual call. Mirrors pyjitpl.py:1685-1690 implicit
clear_exception() in _opimpl_residual_call*'s no-raise tail.
- dispatch_residual_call_{iRd,iIRd,iIRFd}_kind: on the resid_raised
branch, after walker_record_guard_exception, return
DispatchOutcome::SubRaise { exc, exc_concrete } instead of falling
through to Continue. pyjitpl.py:2156-2168 handle_possible_exception
routes the raising arm through finishframe_exception() immediately;
walker continuing past this point recorded dead arm IR (e.g. the
arm's tail *_return) onto the exception path.
- select_residual_call_opcode doc comment (jitcode_dispatch.rs:3004):
the previous "PyPy do_residual_call always calls execute_varargs
regardless of EI branch" was an overstatement. OS_NOT_IN_TRACE
short-circuits via do_not_in_trace_call (pyjitpl.py:2003-2006) and
the release-gil branch runs through do_call_release_gil
(pyjitpl.py:3671-3681) instead. Updated the comment to enumerate
both narrow branches explicitly.
- dispatch_arm_via_blackhole doc (eval.rs:3013): added a "Latent gaps
before this path can be activated" section enumerating five
reviewer-flagged items (non-void return-type push not implemented,
bh.aborted ignored, jitdrivers_sd not seeded, BH_LAST_EXC_VALUE not
drained, hot-path lookup cost). Predicate is `false` everywhere
today so the function is unreachable; the doc surfaces what
activation requires.
Assisted-by: Claude
- trace_ctx::heapcache_invalidate_caches_varargs: env-gated eprintln dumping opnum, argboxes.len, EI extraeffect/forces_vorv/can_raise/plain_call/oopspec on every invalidation call. - state::opimpl_getfield_gc_i: env-gated eprintln dumping obj/field_index/ struct_ptr/descr_pure/cached/loaded before the cache-hit sanity assert when the values diverge. - trace_opcode::production_walker_handles + apply_walker_stack_effect: commented-out Instruction::StoreSubscr entries flag the sub-slice 5c activation site (strategy-aware specialization port pending). Probes default off (PYRE_PROBE_SUBSCR unset = zero impact). Assisted-by: Claude
…for 5c Sub-slice 5c step 2 instrumentation: log funcptr addr, dst_bank, r_args.len, and per-arg raw addrs whenever PYRE_PROBE_SUBSCR is set. Probe sits at the entry of dispatch_residual_call_iRd_kind so step 5's STORE_SUBSCR activation can capture the fnaddr/arg shape immediately as walker handles the opcode. PyObjectRef construction from the GcRef payload is deliberately deferred to step 4 (FrameOps lift); the probe stays at the raw-usize layer so the GcRef→PyObjectRef conversion has a single defined seam. Probe defaults off (PYRE_PROBE_SUBSCR unset = zero impact). Assisted-by: Claude
Sub-slice 5c step 3. Defines the 6-method surface that the strategy-aware
STORE_SUBSCR specialization helpers (`generated_list_setitem_by_strategy`,
`generated_list_setslice_same_len_by_strategy`, the `store_subscr_value`
gate sequence) emit through:
- value_type (Box.type query)
- generate_guard (multi-frame resume snapshot + guard record)
- implement_guard_value, guard_class, guard_int_object_value,
guard_list_strategy — all default impls composed over the two above.
`generate_guard` is the lone load-bearing method. MIFrame walks its
own parent_frames/orgpc/fallthrough_pc state; the walker reaches the
same semantic responsibility through walker_capture_snapshot_for_last_guard
+ the dispatch-time WalkContext register banks. Two distinct
implementations are unavoidable; the trait makes the call sites
interchangeable.
MIFrame impl delegates value_type/generate_guard to existing methods;
the four default impls are direct ports of the corresponding MIFrame
bodies and produce byte-identical recorded IR for the trait path.
The const-arm `flush_guard_not_invalidated` in `MIFrame::guard_class`
(trace_opcode.rs:4681) is documented as a default-impl deviation: it
fires only when a quasi-immut field read left a pending guard, and the
`store_subscr_value` precondition (concrete obj/key/value are direct
stack reads, not quasi-immut loads) excludes that path. Walker impl
in step 4 will assert the precondition before delegating.
WalkContext impl follows in step 4; `generated_*` helpers in
majit-translate/src/codegen.rs become generic over WalkerFrameOps in
step 5.
Gate dynasm+cranelift 41/41.
Assisted-by: Claude
…atures
Sub-slice 5c step 4.
Trait redesign: the earlier signature (`fn generate_guard(&mut self, ctx:
&mut TraceCtx, ...)`) was unsound for `WalkContext` because the caller
must split-borrow `self.trace_ctx` from `self`, and `MIFrame`/`WalkContext`
both reach the same `TraceCtx` transitively through `self` and the
explicit arg — the resulting double mut borrow can't be expressed in
safe Rust without the caller juggling raw pointers.
New shape: trait methods take `&mut self` only; `ctx` is reached
through `self.ctx_mut()` / `self.ctx()` accessors that each impl
provides. `MIFrame::ctx_mut` materialises the borrow via
`unsafe { &mut *self.ctx }` (the `*mut TraceCtx` raw pointer field);
`WalkContext::ctx_mut` returns its `self.trace_ctx` field. The
accessor is scoped to a single statement at each callsite so the
borrow ends before the next `self`-mut-call.
WalkContext impl:
- `generate_guard` records the guard via `self.trace_ctx.record_guard`
then delegates snapshot capture to `walker_capture_snapshot_for_last_guard`.
`flush_guard_not_invalidated` is intentionally omitted — the walker
STORE_SUBSCR path has no quasi-immut field reads, so no pending
GUARD_NOT_INVALIDATED can exist when the trait helpers run.
- `value_type` / `guard_class` / `guard_int_object_value` /
`guard_list_strategy` / `implement_guard_value` use the default
impls, which compose `ctx_mut` + `generate_guard` into byte-identical
emit sequences to the trait path.
MIFrame impl:
- `generate_guard` delegates to the existing
`MIFrame::generate_guard` method, preserving the full
`flush_guard_not_invalidated` / `parent_frames` /
`build_framestack_snapshot` plumbing for the trait dispatch leg.
- The raw pointer re-borrow inside the delegate is sound because
`self.ctx`'s lifetime invariant guarantees it points at the live
`TraceCtx` owned by the enclosing dispatch frame.
`walker_capture_snapshot_for_last_guard` lifted from `fn` to
`pub(crate) fn` so the walker impl can call it from another module.
Default impls of the 4 composed methods skip the const-arm
`flush_guard_not_invalidated` that `MIFrame::guard_class` performs at
trace_opcode.rs:4681; that flush fires only for quasi-immut-field
sequences, which `store_subscr_value` excludes by construction.
Gate dynasm+cranelift 41/41.
Assisted-by: Claude
Convert `trace_unbox_int_with_resume`, `trace_unbox_int_with_resume_descr`, `trace_unbox_long_with_resume`, `trace_unbox_float_with_resume` from `(frame: &mut MIFrame, ctx: &mut TraceCtx, ...)` to `(frame: &mut F, ...)` where `F: WalkerFrameOps`. Frame impl reaches ctx via `frame.ctx_mut()` / `frame.ctx()`; guard emit via `frame.generate_guard(opcode, args)` without explicit ctx arg. Update 22 callsites in codegen.rs + 1 internal in state.rs + 1 test. Sub-slice 5c step 5.1. Assisted-by: Claude
Convert `opimpl_check_resizable_neg_index`, `unbox_int_or_long_for_int_strategy`, `generated_list_setitem_by_strategy`, `generated_list_setslice_same_len_by_strategy`, `generated_store_subscr_value` from `(frame: &mut MIFrame, ctx: &mut TraceCtx, ...)` to `(frame: &mut F, ...)` where `F: WalkerFrameOps`. Ctx access goes through `frame.ctx_mut()` per statement; trait methods (`guard_class`, `guard_list_strategy`, `implement_guard_value`, `generate_guard`) drop the explicit ctx arg. Add `extern crate self as pyre_jit_trace` in lib.rs so generic bounds written for majit-translate's crate name (`pyre_jit_trace::walker_frame_ops`) resolve when codegen.rs is `include!`d into pyre-jit-trace's `generated*` modules. Update trait-leg `store_subscr_value` call sites in trace_opcode.rs (drop ctx arg; `with_ctx` closure ignores ctx). Sub-slice 5c steps 5.2 + 5.3 + 5.4. Assisted-by: Claude
Add `try_walker_store_subscr_specialization` invoked from
`dispatch_residual_call_iRd_kind` between the probe and the EI-decision
prologue. Gate conditions:
- `dst_bank == 'v'` (STORE_SUBSCR returns void)
- `r_args.len() == 3` (3-arg `[obj, key, value]` shape from
`codewriter.rs:7042 build_store_subscr_fn_residual_call_r_v_insn`)
- `PYRE_WALKER_STORE_SUBSCR_FNADDR=<hex|decimal>` env var matches
runtime funcptr (cross-crate plumbing of `bh_store_subscr_fn`
address from pyre-jit is deferred to 5.5b)
- All 3 `concrete_registers_r` shadow slots are `ConcreteValue::Ref(_)`
On all gates pass + `generated_store_subscr_value` returns true, calls
the helper concretely via raw fn-ptr cast to mutate the heap, then
returns `DispatchOutcome::Continue` skipping the blackbox CallN path.
Helper-raise (return non-zero) declines specialization → fallthrough to
existing dispatcher path emits `CallMayForce*` + `GuardException`/
`SubRaise`.
Sub-slice 5c step 5.5a. Gate 41/41×2 GREEN (hook inert without env var).
Assisted-by: Claude
`bh_store_subscr_fn` (call_jit.rs:3295) returns 1 on success and 0 on raise (exception stashed in `BH_LAST_EXC_VALUE`). The step 5.5a hook inverted this — declining on success and continuing on raise. Fix: invert the gate so `success == 0` declines (lets the dispatcher fall through to the standard `try_execute_residual_call_via_executor` + `GuardException` + `SubRaise` path) and `success != 0` proceeds with specialization. Note: this diverges from `jit_setitem` (opcode_ops.rs:225) which returns 0 on success and panics on error — the trait path's `emit_trace_call_int_typed(jit_setitem, ...)` assumes infallible trace-time recording. Walker uses the fail-soft `bh_store_subscr_fn` because residual_call execution is not a panic site. Assisted-by: Claude
Field carries the runtime address of `bh_store_subscr_fn` (`pyre-jit::cpu.store_subscr_fn` binding) so `try_walker_store_subscr_specialization` (step 5.5a) can recognise 3-arg `residual_call_r_v(store_subscr_fn, obj, key, value)` without the `PYRE_WALKER_STORE_SUBSCR_FNADDR` env var. Top-level entry points (`dispatch_via_miframe`, `dispatch_via_miframe_at_opcode_entry`) default to `None`; production plumbing of the address from `cpu.store_subscr_fn` lands in 5.5c. Sub-walk WalkContext constructions inherit the parent's `ctx.store_subscr_fn_addr` (3 sites). 102 test-fixture constructions default to `None` (env var still works as the fallback). Hook now reads field first, falls back to env var when `None`. Sub-slice 5c step 5.5b. Assisted-by: Claude
Move `bh_store_subscr_fn(obj, key, value) -> i64` from `pyre-jit/src/call_jit.rs:3295` to `pyre-interpreter/src/opcode_ops.rs` and register the address in `pyre_interpreter::jit_trace_fnaddrs()` under paths `pyre_interpreter::opcode_ops::bh_store_subscr_fn` + `pyre_interpreter::bh_store_subscr_fn`. Body unchanged. Update the `cpu.store_subscr_fn` binding at `pyre-jit/src/jit/cpu.rs:151` to the new path. Motivation: walker specialization (`try_walker_store_subscr_specialization` in pyre-jit-trace) needs to recognise the runtime address of the helper. Registering in `jit_trace_fnaddrs()` lets pyre-jit-trace look up the address via path key without taking a `pyre-jit-trace → pyre-jit` dep edge. Task #391. Assisted-by: Claude
Add `bh_store_subscr_fn_addr_cached()` helper that resolves the runtime address of `pyre_interpreter::opcode_ops::bh_store_subscr_fn` via `pyre_interpreter::jit_trace_fnaddrs()` linear scan with `OnceLock` caching. Production entry sites in `dispatch_via_miframe` and `dispatch_via_miframe_at_opcode_entry` now seed `WalkContext.store_subscr_fn_addr` from this lookup instead of `None`. With the field populated, `try_walker_store_subscr_specialization` fires without the `PYRE_WALKER_STORE_SUBSCR_FNADDR` env var. Hook remains inert for STORE_SUBSCR specifically (not in `production_walker_handles` yet — step 5.6), but ready for any non-StoreSubscr arm that embeds a 3-arg `residual_call_r_v( store_subscr_fn, ...)`. Gate 41/41×2 GREEN. Sub-slice 5c step 5.5c-d. Assisted-by: Claude
…gress) Sub-slice 5c step 5.6 attempted to enable `Instruction::StoreSubscr` in `production_walker_handles` + `apply_walker_stack_effect`. With the hook actively dispatching the specialized SETARRAYITEM_GC shape, gate runs to completion but nbody (~31× slowdown, 0.21s → 6.5s) and fannkuch (timeout >30s) regress — these are the two STORE_SUBSCR-hot benchmarks the specialization was meant to accelerate. Hook itself (`try_walker_store_subscr_specialization` + `bh_store_subscr_fn_addr_cached` plumbing) preserved in place; the remaining gap is in the walker leg of `generated_store_subscr_value` — likely the `WalkerFrameOps::generate_guard` snapshot delta vs the trait's `with_ctx + MIFrame::generate_guard` flush + parent-frame chain. Marker re-commented with the investigation note; activation deferred to sub-step 5.6b (snapshot / heapcache divergence root-cause). Gate 41/41×2 GREEN with marker off. Assisted-by: Claude
…e only)
5.6 activation surfaced SubRaise every trace iteration: walker dispatch
of `Instruction::StoreSubscr` walks the auto-generated arm jitcode
`int_copy, residual_call_r_r(bh_execute_store_subscr, frame), live,
ref_return`. `try_execute_residual_call_via_executor` matches the
`CallR` shape and concrete-executes `bh_execute_store_subscr(frame)`,
which casts `frame` to `*mut PyFrame` and calls
`pyopcode::execute_store_subscr(frame)`. That helper pops 3 values
from `PyFrame.locals_cells_stack_w` and feeds them to
`setitem(obj, key, value)`.
Walker LOAD_FAST / LOAD_CONST / etc. populate only MIFrame's symbolic
stack + concrete-shadow stack — not the concrete `PyFrame`'s
`locals_cells_stack_w`. `setfield_vable_i(vsd)` advances the
PyFrame's depth counter without writing the slots. When
`bh_execute_store_subscr` runs, the popped slots are uninitialized →
`bh_store_subscr_fn(null, null, null)` raises "store subscript on
null operand" → arm returns 0 → walker dispatch yields SubRaise →
trace aborts.
Repro (PYRE_PROBE_SUBSCR=1 MAJIT_LOG=1 on a tiny `a[0]=i` loop):
[PYRE_PROBE_SUBSCR] dispatch_residual_call_iRd_kind pc=3
dst_bank=r r_args.len=1 funcptr_addr=Some("0x...e6c8")
arg_addrs=[Some("<concrete_frame>")]
[jit][probe] instr=StoreSubscr trace=[
"int_copy", "residual_call_r_r", "live", "ref_return"]
[jit][abort-reason] StoreSubscr SubRaise
`...e6c8` resolves to `bh_execute_store_subscr`
(`pyre-interpreter/src/opcode_ops.rs:265`); the hash matches the
`nm` offset at `__ZN16pyre_interpreter10opcode_ops23bh_execute_store_subscr`.
Marker stays commented; comment expanded with the root cause.
Existing `try_walker_store_subscr_specialization` hook keyed on
`dst_bank == 'v' && r_args.len() == 3` (the codewriter trace IR
shape `build_store_subscr_fn_residual_call_r_v_insn`) never fires
against the arm jitcode's `_r_r` shape — they are two distinct
emission paths.
5.6b fix candidates left for future sub-slice:
(A) Populate the concrete `PyFrame` stack from walker LOAD/CONST
handlers (orthodox PyPy-parity but wide-scope).
(B) Intercept `StoreSubscr` at `dispatch_via_walker_for_opcode` entry
(before the arm walk) and route through `MIFrame::store_subscr_value`
using the symbolic stack — bypasses the arm jitcode entirely,
mirrors the trait path which also avoids concrete heap mutation
at trace time.
Gate dynasm 41/41 + cranelift 41/41 with marker off.
Assisted-by: Claude
Re-enable `Instruction::StoreSubscr` in `production_walker_handles` + intercept at `dispatch_via_walker_for_opcode` entry before the arm walk. Pops 3 values via `SharedOpcodeHandler::pop_value` (which updates symbolic + concrete-shadow stacks + vsd shadow through `MIFrame::pop_value`), delegates to the existing `MIFrame::store_subscr_value` which records the same specialized `guard_class + SETARRAYITEM_GC`-family IR shape via `generated_store_subscr_value` (or the `Call(jit_setitem, ...)` fallback via `trace_store_subscr`) that the trait dispatch already emits, then returns `StepResult::Continue` without entering the arm walk. `apply_walker_stack_effect` is not reached because the hook returns before the arm-walk match. The arm jitcode's `residual_call_r_r(bh_execute_store_subscr, frame)` shape — which would otherwise raise on the unpopulated `PyFrame.locals_cells_stack_w` slots — is bypassed entirely. Verified on dynasm release build: * `a[0]=i` smoke loop, n=2000 — correct output (1999) * `pyre/bench/nbody_50k.py` 1.10s → 0.73s (~33% improvement) * `pyre/bench/nbody.py` (n=500000) 4.51s → 4.04s * `pyre/bench/fannkuch.py` (n=9) timeout → 0.81s, output 8629/30 ×7 Gate dynasm 41/41 + cranelift 41/41 GREEN. The pre-existing 5.5a hook `try_walker_store_subscr_specialization` in `dispatch_residual_call_iRd_kind` remains dead (the codewriter-trace-IR `_r_v` shape it gates on never fires inside the arm jitcode); leaving it in place for now — the 141 cutover plan moves trace-IR emission to the walker and that hook will gain a reachable path then. Assisted-by: Claude
Deletions: - `dispatch_arm_via_blackhole` in `pyre-jit/src/eval.rs` - `production_blackhole_handles` in `pyre-jit-trace/src/trace_opcode.rs` and its re-export in `pyre-jit-trace/src/lib.rs` - `metainterp_jitcode_by_index` / `metainterp_jitcode_for_arm` / `metainterp_jitcode_for_instruction` and `METAINTERP_JITCODE_CACHE` thread-local in `pyre-jit-trace/src/jitcode_runtime.rs` `eval_loop_jit` walker-dispatched branch collapses to the BH_LAST_EXC_VALUE drain that surfaces a non-elidable residual_call's raised exception (kept because `try_execute_residual_call_via_executor` from Task #390 sub-slice 3 seeds the TLS on Err). Updates the stale Phase 5.B-replacement doc block on `try_fold_pure_call_via_executor` to point at `try_execute_residual_call_via_executor` as the orthodox covering function for non-elidable arms. Assisted-by: Claude
…a_executor when no active virtualizable Extends the executor match arm of `try_execute_residual_call_via_executor` from `Call*` / `CallLoopinvariant*` to also include `CallMayForce*` when `ctx.trace_ctx.standard_virtualizable_box()` returns None. With no active vinfo box, `vable_after_residual_call` (`pyjitpl.py:3349-3366` parity at `trace_opcode.rs:2646`) early-returns, so executing the helper here matches PyPy's `do_residual_call`'s forces-virtual branch (`pyjitpl.py:2017-2082`) without requiring a walker-side `walker_vable_after_residual_call`. Active-vable `CallMayForce*` continues to decline. Doc updated to spell out the gate. Assisted-by: Claude
…ise delegation
Thread `op_arg` into `dispatch_via_walker_for_opcode` (single call site
at `trace_code_step`) and add a `Instruction::Reraise { depth }` entry
hook that delegates to the existing `MIFrame::reraise` impl through
`OpcodeStepExecutor::reraise(self, depth.get(op_arg))`. The trait impl
reads `reraise_lasti` from `concrete_stack[stack_idx]` and seeds
`err.reraise_lasti` (`pyopcode.py:1357-1376` parity), so the walker
hook's propagated `Err(PyError)` carries the lasti through
`step_result.err().map(|e| e.reraise_lasti)` the same way the trait
dispatch leg does.
Add `Instruction::Reraise { .. }` to `production_walker_handles` and
drop the exclusion comment. `StoreSubscr` hook acquires `let _ = op_arg`
since `StoreSubscr` is a unit variant with no encoded arg.
Assisted-by: Claude
|
Warning Review limit reached
More reviews will be available in 10 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR refactors the JIT trace walker to enable STORE_SUBSCR residual-call specialization by introducing a ChangesSTORE_SUBSCR Walker Specialization
Sequence DiagramsequenceDiagram
participant walker as Walker
participant handler as trace_opcode handler
participant dispatch as dispatch_residual_call_iRd_kind
participant spec as try_walker_store_subscr_specialization
participant concrete as bh_store_subscr_fn
participant fallback as SubRaise path
walker->>handler: opcode STORE_SUBSCR
handler->>handler: try_walker_direct_opcode_dispatch
handler->>handler: pop obj, key, value from stack
handler->>handler: store_subscr_value (emit IR or trace)
handler-->>walker: StepResult::Continue
alt residual STORE_SUBSCR call needed
walker->>dispatch: dispatch_residual_call_iRd_kind
dispatch->>spec: try_walker_store_subscr_specialization
spec->>concrete: call bh_store_subscr_fn(obj, key, value)
alt success
concrete-->>spec: 1 (success)
spec->>dispatch: record IR, return Ok
else failure
concrete-->>spec: 0 or exception
spec->>dispatch: return SubRaise with last_exc_value
dispatch->>fallback: emit SubRaise with last_exc_value
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
youknowone
changed the title
youknowone
marked this pull request as ready for review
June 8, 2026 23:13
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ae065e3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+7061
to
+7068
| self.store_subscr_value( | ||
| obj.opref, | ||
| key.opref, | ||
| value.opref, | ||
| obj.concrete.to_pyobj(), | ||
| key.concrete.to_pyobj(), | ||
| value.concrete.to_pyobj(), | ||
| )?; |
There was a problem hiding this comment.
When StoreSubscr is allow-listed, eval_loop_jit marks the opcode as walker-dispatched and skips execute_opcode_step, so this direct branch becomes responsible for the real bytecode effect. It only pops the symbolic/concrete-shadow stack and calls store_subscr_value, which records trace IR (or a residual jit_setitem call) but does not execute setitem on the live object. During tracing, any list/dict item assignment handled here leaves the actual container unchanged, so subsequent bytecodes in the interpreter can read stale values.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-jit-trace/src/state.rs (1)
2262-2365:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun rustfmt on this helper block before merge.
cargo fmt --checkis already red in CI, and this newly edited block is part of the failure surface. Please format the file before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/state.rs` around lines 2262 - 2365, The new helper block (functions trace_unbox_int_with_resume, trace_unbox_int_with_resume_descr, trace_unbox_long_with_resume and trace_unbox_float_with_resume) is not rustfmt-formatted causing CI failure; run rustfmt (cargo fmt) on pyre/pyre-jit-trace/src/state.rs (or run cargo fmt --all) and commit the formatting changes so the functions and surrounding code match rustfmt style, or manually fix spacing/indentation to match rustfmt output before merging.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-interpreter/src/jit_fnaddr.rs`:
- Around line 342-375: Add a small regression check immediately after the
push_fnaddr/push_alias_pair calls to assert the string keys were registered
(verify "execute_store_subscr" and both
"pyre_interpreter::opcode_ops::bh_store_subscr_fn"/"pyre_interpreter::bh_store_subscr_fn"
aliases exist). Locate the block that calls push_fnaddr and push_alias_pair
(symbols: push_fnaddr, push_alias_pair, execute_store_subscr,
bh_store_subscr_fn) and add assertions that the registry/entries lookup contains
those exact keys (or call the existing jit_trace_fnaddrs()/lookup helper if
available) so a typo fails fast during tests. Ensure the assertions run in test
or debug builds only to avoid production panic.
In `@pyre/pyre-interpreter/src/opcode_ops.rs`:
- Around line 299-315: The bh_store_subscr_fn wrapper currently checks obj and
key for null but not value; add a null check for the value before calling
crate::baseobjspace::setitem so a null value produces a soft failure: create a
crate::PyError (e.g. PyErrorKind::TypeError with a descriptive message like
"store subscript on null operand"), set
majit_metainterp::blackhole::BH_LAST_EXC_VALUE to err.to_exc_object() as i64,
and return 0; this mirrors the existing obj/key handling and prevents forwarding
a raw null value into crate::baseobjspace::setitem.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs`:
- Around line 4918-4921: The new tuple-return formatting around the
DispatchOutcome::SubRaise return violates rustfmt; run cargo fmt and reformat
the return expressions (e.g., the return of (DispatchOutcome::SubRaise { exc,
exc_concrete }, op.next_pc)) so they conform to rustfmt style. Locate the return
statements in jitcode_dispatch.rs that return tuples containing DispatchOutcome
variants (such as DispatchOutcome::SubRaise) and op.next_pc, then run `cargo
fmt` (or apply rustfmt) to fix spacing/line-wrapping so CI `cargo fmt --check`
passes.
- Around line 4578-4615: The specialized STORE_SUBSCR IR is emitted by
generated_store_subscr_value(...) before the concrete helper bh_store_subscr_fn
is invoked, which can leave a successful IR on an exception path; either defer
calling generated_store_subscr_value until after bh_store_subscr_fn returns
success, or implement an explicit rollback that undoes the TraceCtx/heap
mutation when bh_store_subscr_fn reports a raise. Locate the sequence around
generated_store_subscr_value, the unsafe transmute/invocation of
bh_store_subscr_fn (store_subscr_fn), and the success check, and modify the flow
so IR emission/mutation only happens after success or ensure a precise inverse
undo of whatever generated_store_subscr_value changed before returning None.
In `@pyre/pyre-jit-trace/src/state.rs`:
- Around line 2283-2289: Replace the manual GUARD_CLASS emission with the
WalkerFrameOps helper: instead of calling
frame.ctx().heap_cache().is_class_known(...),
frame.generate_guard(OpCode::GuardClass, ...), and then
heap_cache_mut().class_now_known(...), call the frame's guard_class(...) method
(WalkerFrameOps::guard_class) which encapsulates the shared constant fast-path,
GuardNonnullClass behavior, and heap-cache update; update all occurrences (e.g.,
the blocks around functions referencing type_addr, obj and using
frame.generate_guard/heap_cache_mut().class_now_known at the shown sites and the
other locations noted) to use guard_class(obj, type_addr) so the trait contract
and centralized fast-path logic are preserved.
- Around line 2081-2091: The hot GETFIELD heap-cache path currently calls
std::env::var_os("PYRE_PROBE_SUBSCR") on every cached int-field hit; change the
logic to first compare loaded != cached_int and only if they differ consult the
probe flag, and avoid repeated env lookups by caching the probe value once
(e.g., a static/once_cell Lazy or similar) so the probe is read at startup and
then a cheap boolean is used in the branch that logs the sanity-int-mismatch
(refer to the variables loaded, cached_int and the current
std::env::var_os("PYRE_PROBE_SUBSCR") call in the GETFIELD fast path).
In `@pyre/pyre-jit-trace/src/trace_opcode.rs`:
- Around line 7055-7069: The STORE_SUBSCR direct-dispatch path only pops the
shadow stack via SharedOpcodeHandler::pop_value and fails to mirror those three
pops into the heap-backed concrete frame, which later readers
(concrete_valuestackdepth(), snapshot/depth paths) treat as authoritative; fix
by, after the three SharedOpcodeHandler::pop_value calls and before returning
from the Instruction::StoreSubscr branch, performing the equivalent three
pops/mutations on the concrete/heap-backed frame (i.e., call the frame's
concrete pop/remove API used elsewhere in this module) so the heap-backed frame
and shadow stack remain in sync (you can locate the relevant concrete pop helper
by searching for uses around concrete_valuestackdepth() / store_subscr_value and
reuse that to pop for obj, key, and value).
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 3176-3179: The multiline call to
pyre_interpreter::PyError::from_exc_object (passing bh_exc as
pyre_object::PyObjectRef) is not formatted to rustfmt expectations; run rustfmt
(cargo fmt) and reformat the Err(unsafe {
pyre_interpreter::PyError::from_exc_object( ... ) }) block so the argument and
closing parens align with rustfmt style (preserving the unsafe block and the
cast of bh_exc) ensuring cargo fmt --check passes for the
PyError::from_exc_object call.
- Around line 3149-3183: The walker-dispatch bypass logic is duplicated only in
eval_loop and missing in eval_loop_jit_bridge. Extract the walker-handled opcode
completion path into a shared helper that checks walker_dispatched_this_opcode,
skips execute_opcode_step when the walker already updated the frame, and drains
majit_metainterp::blackhole::BH_LAST_EXC_VALUE into
Err(pyre_interpreter::PyError::from_exc_object(...)) or returns
Ok(StepResult::Continue). Then call that same helper from both eval_loop and
eval_loop_jit_bridge immediately after walker dispatch/jit_merge_point_hook so
bridge execution mirrors the non-bridge path and does not double-apply opcode
effects.
---
Outside diff comments:
In `@pyre/pyre-jit-trace/src/state.rs`:
- Around line 2262-2365: The new helper block (functions
trace_unbox_int_with_resume, trace_unbox_int_with_resume_descr,
trace_unbox_long_with_resume and trace_unbox_float_with_resume) is not
rustfmt-formatted causing CI failure; run rustfmt (cargo fmt) on
pyre/pyre-jit-trace/src/state.rs (or run cargo fmt --all) and commit the
formatting changes so the functions and surrounding code match rustfmt style, or
manually fix spacing/indentation to match rustfmt output before merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 53ac5276-3403-4ac9-af0e-7da56b5eb31c
📒 Files selected for processing (13)
majit/majit-metainterp/src/trace_ctx.rsmajit/majit-translate/src/codegen.rspyre/pyre-interpreter/src/jit_fnaddr.rspyre/pyre-interpreter/src/opcode_ops.rspyre/pyre-jit-trace/src/jitcode_dispatch.rspyre/pyre-jit-trace/src/jitcode_runtime.rspyre/pyre-jit-trace/src/lib.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-jit-trace/src/walker_frame_ops.rspyre/pyre-jit/src/call_jit.rspyre/pyre-jit/src/eval.rspyre/pyre-jit/src/jit/cpu.rs
💤 Files with no reviewable changes (2)
- pyre/pyre-jit-trace/src/jitcode_runtime.rs
- pyre/pyre-jit/src/call_jit.rs
Comment on lines
+2283
to
+2289
| if !frame.ctx().heap_cache().is_class_known(obj) { | ||
| let type_const = frame.ctx_mut().const_int(type_addr); | ||
| frame.generate_guard(OpCode::GuardClass, &[obj, type_const]); | ||
| frame | ||
| .ctx_mut() | ||
| .heap_cache_mut() | ||
| .class_now_known(obj, type_addr); |
There was a problem hiding this comment.
Use WalkerFrameOps::guard_class here instead of open-coding GUARD_CLASS.
These helpers are now generic over WalkerFrameOps, but they bypass the trait’s class-guard contract and emit GuardClass directly. That drops the shared constant fast-path and the GuardNonnullClass/heap-cache behavior that WalkerFrameOps::guard_class() centralizes, so walker-side unboxing no longer matches the frame abstraction this refactor introduced.
Suggested fix
- if !frame.ctx().heap_cache().is_class_known(obj) {
- let type_const = frame.ctx_mut().const_int(type_addr);
- frame.generate_guard(OpCode::GuardClass, &[obj, type_const]);
- frame
- .ctx_mut()
- .heap_cache_mut()
- .class_now_known(obj, type_addr);
- }
+ frame.guard_class(
+ obj,
+ type_addr as usize as *const pyre_object::pyobject::PyType,
+ );Also applies to: 2322-2329, 2355-2361
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/state.rs` around lines 2283 - 2289, Replace the
manual GUARD_CLASS emission with the WalkerFrameOps helper: instead of calling
frame.ctx().heap_cache().is_class_known(...),
frame.generate_guard(OpCode::GuardClass, ...), and then
heap_cache_mut().class_now_known(...), call the frame's guard_class(...) method
(WalkerFrameOps::guard_class) which encapsulates the shared constant fast-path,
GuardNonnullClass behavior, and heap-cache update; update all occurrences (e.g.,
the blocks around functions referencing type_addr, obj and using
frame.generate_guard/heap_cache_mut().class_now_known at the shown sites and the
other locations noted) to use guard_class(obj, type_addr) so the trait contract
and centralized fast-path logic are preserved.
Comment on lines
+7055
to
+7069
| if matches!(instruction, Instruction::StoreSubscr) { | ||
| use pyre_interpreter::SharedOpcodeHandler; | ||
| let _ = op_arg; | ||
| let key = SharedOpcodeHandler::pop_value(self)?; | ||
| let obj = SharedOpcodeHandler::pop_value(self)?; | ||
| let value = SharedOpcodeHandler::pop_value(self)?; | ||
| self.store_subscr_value( | ||
| obj.opref, | ||
| key.opref, | ||
| value.opref, | ||
| obj.concrete.to_pyobj(), | ||
| key.concrete.to_pyobj(), | ||
| value.concrete.to_pyobj(), | ||
| )?; | ||
| return Ok(Some(pyre_interpreter::StepResult::Continue)); |
There was a problem hiding this comment.
Mirror the direct STORE_SUBSCR pops into the heap-backed frame.
Lines 7058-7069 only update the symbolic/shadow stack via pop_value. Line 2340 then refreshes that shadow from heap again on the next top-level opcode boundary, and snapshot/depth paths like Line 1033 and Line 2537 still treat concrete_valuestackdepth() as authoritative. That means a direct-dispatched STORE_SUBSCR can resurrect the pre-pop stack depth/slots on the very next opcode, corrupting later guard snapshots and resume state. Please either write the three pops back to the concrete frame before returning here, or keep the later refresh/depth readers off the heap for this shadow-only path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/trace_opcode.rs` around lines 7055 - 7069, The
STORE_SUBSCR direct-dispatch path only pops the shadow stack via
SharedOpcodeHandler::pop_value and fails to mirror those three pops into the
heap-backed concrete frame, which later readers (concrete_valuestackdepth(),
snapshot/depth paths) treat as authoritative; fix by, after the three
SharedOpcodeHandler::pop_value calls and before returning from the
Instruction::StoreSubscr branch, performing the equivalent three pops/mutations
on the concrete/heap-backed frame (i.e., call the frame's concrete pop/remove
API used elsewhere in this module) so the heap-backed frame and shadow stack
remain in sync (you can locate the relevant concrete pop helper by searching for
uses around concrete_valuestackdepth() / store_subscr_value and reuse that to
pop for obj, key, and value).
…yPy _opimpl_* entries Refactors the two direct-record opcode hooks (StoreSubscr 5.6b delegation, Reraise lasti propagation) out of the head of `dispatch_via_walker_for_opcode` into a dedicated `try_walker_direct_opcode_dispatch` helper. No functional change. The new function is the structural home for opcodes that bypass the auto-gen arm-jitcode walk and emit IR / produce concrete effects directly from the tracer, mirroring PyPy `MetaInterp.interpret`'s `pyjitpl.py:1346+ _opimpl_*` dispatch pattern where the tracer invokes `history.record*` and concrete-executes helpers inline rather than walking an intermediate jitcode representation. Future opcode ports land here as additional `if let Instruction::X = ...` arms, making the "walker emits IR directly like PyPy _opimpl_*" surface area a single named location. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 378375a99c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // always -1 and `Reraise { depth > 0 }` aborts the trace. | ||
| // Keep on trait dispatch until the walker can propagate the | ||
| // saved lasti. | ||
| | Instruction::Reraise { .. } // walker hook in dispatch_via_walker_for_opcode delegates to MIFrame::reraise which returns Err(PyError) with reraise_lasti seeded — `step_result.err().map(|e| e.reraise_lasti)` extraction works the same on either leg |
There was a problem hiding this comment.
When tracing reaches RERAISE, this allow-list entry makes eval_loop_jit set walker_dispatched_this_opcode and skip execute_opcode_step for the live frame. The new walker hook delegates to MIFrame::reraise, but that Err(PyError) is consumed by trace_code_step; the eval-loop skip path only checks BH_LAST_EXC_VALUE, which reraise never sets, so a hot exception handler can fall through as Continue instead of re-raising whenever tracing does not immediately replace execution with a compiled result.
Useful? React with 👍 / 👎.
Match the obj/key null branch by also rejecting a null value before forwarding to baseobjspace::setitem. Assisted-by: Claude
state.rs and jitcode_dispatch.rs probes sit on hot trace paths and called std::env::var_os on every cache hit / residual_call dispatch. Move the read into a OnceLock-backed probe_subscr_enabled() and gate the state.rs branch on (loaded != cached_int) first so the cached case bypasses the probe entirely. Assisted-by: Claude
…_miframe entry The dispatch_via_miframe path (test / shadow_walker fixture) hard-codes outer_jitcode_index = 0 and an empty outer_active_boxes; production dispatch via dispatch_via_miframe_at_opcode_entry seeds both from sym.jitcode and collect_outer_active_boxes. Leaving store_subscr_fn_addr populated here would let a specialized generated_store_subscr_value guard capture resume data pointing at the wrong outer frame, so set it to None on this entry. Assisted-by: Claude
…through SubRaise On a helper raise, drain BH_LAST_EXC_VALUE into ctx.last_exc_value / ctx.last_exc_value_concrete, record GuardException via walker_record_guard_exception, and return DispatchOutcome::SubRaise. Previously the function returned None on raise so the caller fell through to the generic residual-call dispatcher, which would record a second call op against the same opcode position even though the specialized IR was already emitted. Assisted-by: Claude
…t_bridge eval_loop_jit consults walker_dispatched_this_opcode after jit_merge_point_hook so production_walker_handles allow-listed opcodes skip execute_opcode_step (preventing double-mutation of the live frame) and drain BH_LAST_EXC_VALUE into PyError if a raise is pending. The bridge loop now does the same. Assisted-by: Claude
Owner
Author
|
Review triage round 2 — 6 commits pushed (b659ac3 .. 83528af). Applied
Deferred
— commented by Claude |
Assert jit_trace_fnaddrs() registers the bare execute_store_subscr key and both bh_store_subscr_fn module-path aliases. A typo in the registration would otherwise surface only at trace time. Assisted-by: Claude
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyre/pyre-jit-trace/src/jitcode_dispatch.rs (1)
4584-4629:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRaised
STORE_SUBSCRstill commits the no-call specialization path.
generated_store_subscr_value(...)has already replaced the residual call with direct mutation IR beforebh_store_subscr_fnreports failure. When this branch recordsGuardExceptionand returnsSubRaise, the trace no longer contains any recorded call that can raise at runtime, so replay can still execute the specialized mutations before deopting on a missing exception state. The raising sample needs to abandon specialization entirely here, or precisely roll back the emitted ops/heapcache state and fall back to the generic residual-call path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs` around lines 4584 - 4629, generated_store_subscr_value replaces the residual call with in-place mutation IR but the current branch still commits that specialization when bh_store_subscr_fn fails; change the failure path so it does not leave the specialized mutation applied — either undo the emitted ops/heapcache state or abandon the no-call specialization and fall back to the generic residual-call path. Specifically, when success == 0 and after reading BH_LAST_EXC_VALUE (use the same variables ctx.last_exc_value / ctx.last_exc_value_concrete and walker_record_guard_exception), ensure you revert whatever generated_store_subscr_value recorded (undo its emitted ops/heapcache or mark the specialization invalid) and then return a dispatch that routes to the generic call path instead of leaving the concrete mutation in place (so the runtime will perform the original helper call and preserve correct exception behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 3251-3277: The bridge branch advances the frame's last_instr via
frame.set_last_instr_from_next_instr(next_instr) too early so walker-raised
exceptions are handled using the post-decode PC; save the original last_instr
(or the pre-decode PC/opcode_pc) before calling set_last_instr_from_next_instr,
and restore frame.last_instr back to the opcode_pc (the seeded PC used by
eval_loop_jit) immediately before draining BH_LAST_EXC_VALUE / invoking the
bridge-side exception path (the walker_dispatched_this_opcode branch that
constructs Err from BH_LAST_EXC_VALUE) so handle_exception sees the correct
covering handler; apply this change inside the walker_dispatched_this_opcode arm
around the BH_LAST_EXC_VALUE check while keeping execute_opcode_step unchanged.
---
Duplicate comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs`:
- Around line 4584-4629: generated_store_subscr_value replaces the residual call
with in-place mutation IR but the current branch still commits that
specialization when bh_store_subscr_fn fails; change the failure path so it does
not leave the specialized mutation applied — either undo the emitted
ops/heapcache state or abandon the no-call specialization and fall back to the
generic residual-call path. Specifically, when success == 0 and after reading
BH_LAST_EXC_VALUE (use the same variables ctx.last_exc_value /
ctx.last_exc_value_concrete and walker_record_guard_exception), ensure you
revert whatever generated_store_subscr_value recorded (undo its emitted
ops/heapcache or mark the specialization invalid) and then return a dispatch
that routes to the generic call path instead of leaving the concrete mutation in
place (so the runtime will perform the original helper call and preserve correct
exception behavior).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6eee2711-262c-4296-9a7c-e9e568fffb12
📒 Files selected for processing (11)
pyre/check.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/jit_fnaddr.rspyre/pyre-interpreter/src/opcode_ops.rspyre/pyre-jit-trace/src/jitcode_dispatch.rspyre/pyre-jit-trace/src/lib.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-jit/src/call_jit.rspyre/pyre-jit/src/eval.rspyre/pyre-object/src/dictmultiobject.rs
💤 Files with no reviewable changes (1)
- pyre/pyre-jit/src/call_jit.rs
Comment on lines
3251
to
+3277
| // handle_bytecode: execute the bytecode on the concrete frame. | ||
| let next_instr = opcode_pc + 1; | ||
| frame.set_last_instr_from_next_instr(next_instr); | ||
| match execute_opcode_step(frame, code, instruction, op_arg, next_instr) { | ||
| let step_result = if walker_dispatched_this_opcode { | ||
| // Mirror `eval_loop_jit`'s walker-dispatched bypass — the | ||
| // walker arm already mutated the live PyFrame via | ||
| // `vable_setfield` → `synchronize_virtualizable`, and the | ||
| // compiled trace owns the rest of the bytecode's heap | ||
| // effects on replay. Running `execute_opcode_step` here | ||
| // would double-mutate `valuestackdepth` for the same opcode. | ||
| // Drain any pending raise from `BH_LAST_EXC_VALUE` so the | ||
| // exception handler runs against the bridge frame. | ||
| let bh_exc = majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| { | ||
| let v = c.get(); | ||
| c.set(0); | ||
| v | ||
| }); | ||
| if bh_exc != 0 { | ||
| Err(unsafe { | ||
| pyre_interpreter::PyError::from_exc_object(bh_exc as pyre_object::PyObjectRef) | ||
| }) | ||
| } else { | ||
| Ok(StepResult::Continue) | ||
| } | ||
| } else { | ||
| execute_opcode_step(frame, code, instruction, op_arg, next_instr) | ||
| }; |
There was a problem hiding this comment.
Restore frame.last_instr before bridge-side exception handling.
eval_loop_jit seeds the current opcode PC before routing exceptions because handle_exception consults that location to find the covering handler. This new bridge bypass only advances via set_last_instr_from_next_instr(next_instr), so a walker-raised exception is handled from the post-decode PC instead. On bridged EXTENDED_ARG instructions, that can miss the correct except range and let a catchable exception escape.
Suggested fix
// handle_bytecode: execute the bytecode on the concrete frame.
let next_instr = opcode_pc + 1;
+ frame.last_instr = pc as isize;
frame.set_last_instr_from_next_instr(next_instr);
let step_result = if walker_dispatched_this_opcode {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // handle_bytecode: execute the bytecode on the concrete frame. | |
| let next_instr = opcode_pc + 1; | |
| frame.set_last_instr_from_next_instr(next_instr); | |
| match execute_opcode_step(frame, code, instruction, op_arg, next_instr) { | |
| let step_result = if walker_dispatched_this_opcode { | |
| // Mirror `eval_loop_jit`'s walker-dispatched bypass — the | |
| // walker arm already mutated the live PyFrame via | |
| // `vable_setfield` → `synchronize_virtualizable`, and the | |
| // compiled trace owns the rest of the bytecode's heap | |
| // effects on replay. Running `execute_opcode_step` here | |
| // would double-mutate `valuestackdepth` for the same opcode. | |
| // Drain any pending raise from `BH_LAST_EXC_VALUE` so the | |
| // exception handler runs against the bridge frame. | |
| let bh_exc = majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| { | |
| let v = c.get(); | |
| c.set(0); | |
| v | |
| }); | |
| if bh_exc != 0 { | |
| Err(unsafe { | |
| pyre_interpreter::PyError::from_exc_object(bh_exc as pyre_object::PyObjectRef) | |
| }) | |
| } else { | |
| Ok(StepResult::Continue) | |
| } | |
| } else { | |
| execute_opcode_step(frame, code, instruction, op_arg, next_instr) | |
| }; | |
| // handle_bytecode: execute the bytecode on the concrete frame. | |
| let next_instr = opcode_pc + 1; | |
| frame.last_instr = pc as isize; | |
| frame.set_last_instr_from_next_instr(next_instr); | |
| let step_result = if walker_dispatched_this_opcode { | |
| // Mirror `eval_loop_jit`'s walker-dispatched bypass — the | |
| // walker arm already mutated the live PyFrame via | |
| // `vable_setfield` → `synchronize_virtualizable`, and the | |
| // compiled trace owns the rest of the bytecode's heap | |
| // effects on replay. Running `execute_opcode_step` here | |
| // would double-mutate `valuestackdepth` for the same opcode. | |
| // Drain any pending raise from `BH_LAST_EXC_VALUE` so the | |
| // exception handler runs against the bridge frame. | |
| let bh_exc = majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| { | |
| let v = c.get(); | |
| c.set(0); | |
| v | |
| }); | |
| if bh_exc != 0 { | |
| Err(unsafe { | |
| pyre_interpreter::PyError::from_exc_object(bh_exc as pyre_object::PyObjectRef) | |
| }) | |
| } else { | |
| Ok(StepResult::Continue) | |
| } | |
| } else { | |
| execute_opcode_step(frame, code, instruction, op_arg, next_instr) | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit/src/eval.rs` around lines 3251 - 3277, The bridge branch
advances the frame's last_instr via
frame.set_last_instr_from_next_instr(next_instr) too early so walker-raised
exceptions are handled using the post-decode PC; save the original last_instr
(or the pre-decode PC/opcode_pc) before calling set_last_instr_from_next_instr,
and restore frame.last_instr back to the opcode_pc (the seeded PC used by
eval_loop_jit) immediately before draining BH_LAST_EXC_VALUE / invoking the
bridge-side exception path (the walker_dispatched_this_opcode branch that
constructs Err from BH_LAST_EXC_VALUE) so handle_exception sees the correct
covering handler; apply this change inside the walker_dispatched_this_opcode arm
around the BH_LAST_EXC_VALUE check while keeping execute_opcode_step unchanged.
youknowone
added a commit
that referenced
this pull request
Jun 11, 2026
The rebased full-body-walk commits predate two APIs the #158/#390 base added to pyre-jit-trace: - `WalkContext.store_subscr_fn_addr`: the inline-call sub-walk constructor in `try_walker_inline_user_call` propagates it from the parent context, and the six `#[cfg(test)]` WalkContext fixtures that the release build does not compile set it to `None`. - `walker_capture_snapshot_for_last_guard` now returns `Result<(), DispatchError>`; the base's `WalkerFrameOps::generate_guard` for `WalkContext` calls it in a `()` context with no abort channel, so the snapshot is best-effort there (the STORE_SUBSCR specialization that reaches it is env-gated and dead in production). Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 11, 2026
try_execute_residual_call_via_executor declined Type::Void residual calls by pushing them onto VOID_DEFER_LOG and committing enclosing-scope stores at the primary loop close. The may-force-safe gate (from #158) declines the active-vable CallMayForce* subset (STORE_SUBSCR) before reaching this branch, so the void branch no longer fires across the bench suite. Replace the void branch with a bare `return None` so the residual op is recorded symbolically and the compiled loop applies the side effect once per iteration -- matching the may-force-decline path and the trait path's net effect. do_residual_call (pyjitpl.py:2040/2104/2123) executes void calls eagerly during tracing, but the full-body walk is symbolic and does not advance the interpreter, so the compiled loop re-runs the traced iteration; eager execution there would double-apply the heap mutation. Remove the dead VOID_DEFER_LOG, VoidDeferEvent, void_defer_push_store/push_register/commit_at_close, and the jit_merge_point register push and loop-close commit call sites. Rename void_defer_reset to bool_box_truth_reset, its only remaining job. Verified: FBW-all-levers output byte-identical to the trait path on all 44 benches (incl. nbody enclosing-scope store, fannkuch, list ops); check.py dynasm 44/44 + cranelift 44/44; pyre-jit-trace 250 + majit-metainterp 1365 unit tests pass. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 12, 2026
The rebased full-body-walk commits predate two APIs the #158/#390 base added to pyre-jit-trace: - `WalkContext.store_subscr_fn_addr`: the inline-call sub-walk constructor in `try_walker_inline_user_call` propagates it from the parent context, and the six `#[cfg(test)]` WalkContext fixtures that the release build does not compile set it to `None`. - `walker_capture_snapshot_for_last_guard` now returns `Result<(), DispatchError>`; the base's `WalkerFrameOps::generate_guard` for `WalkContext` calls it in a `()` context with no abort channel, so the snapshot is best-effort there (the STORE_SUBSCR specialization that reaches it is env-gated and dead in production). Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 12, 2026
try_execute_residual_call_via_executor declined Type::Void residual calls by pushing them onto VOID_DEFER_LOG and committing enclosing-scope stores at the primary loop close. The may-force-safe gate (from #158) declines the active-vable CallMayForce* subset (STORE_SUBSCR) before reaching this branch, so the void branch no longer fires across the bench suite. Replace the void branch with a bare `return None` so the residual op is recorded symbolically and the compiled loop applies the side effect once per iteration -- matching the may-force-decline path and the trait path's net effect. do_residual_call (pyjitpl.py:2040/2104/2123) executes void calls eagerly during tracing, but the full-body walk is symbolic and does not advance the interpreter, so the compiled loop re-runs the traced iteration; eager execution there would double-apply the heap mutation. Remove the dead VOID_DEFER_LOG, VoidDeferEvent, void_defer_push_store/push_register/commit_at_close, and the jit_merge_point register push and loop-close commit call sites. Rename void_defer_reset to bool_box_truth_reset, its only remaining job. Verified: FBW-all-levers output byte-identical to the trait path on all 44 benches (incl. nbody enclosing-scope store, fannkuch, list ops); check.py dynasm 44/44 + cranelift 44/44; pyre-jit-trace 250 + majit-metainterp 1365 unit tests pass. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
The rebased full-body-walk commits predate two APIs the #158/#390 base added to pyre-jit-trace: - `WalkContext.store_subscr_fn_addr`: the inline-call sub-walk constructor in `try_walker_inline_user_call` propagates it from the parent context, and the six `#[cfg(test)]` WalkContext fixtures that the release build does not compile set it to `None`. - `walker_capture_snapshot_for_last_guard` now returns `Result<(), DispatchError>`; the base's `WalkerFrameOps::generate_guard` for `WalkContext` calls it in a `()` context with no abort channel, so the snapshot is best-effort there (the STORE_SUBSCR specialization that reaches it is env-gated and dead in production). Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
try_execute_residual_call_via_executor declined Type::Void residual calls by pushing them onto VOID_DEFER_LOG and committing enclosing-scope stores at the primary loop close. The may-force-safe gate (from #158) declines the active-vable CallMayForce* subset (STORE_SUBSCR) before reaching this branch, so the void branch no longer fires across the bench suite. Replace the void branch with a bare `return None` so the residual op is recorded symbolically and the compiled loop applies the side effect once per iteration -- matching the may-force-decline path and the trait path's net effect. do_residual_call (pyjitpl.py:2040/2104/2123) executes void calls eagerly during tracing, but the full-body walk is symbolic and does not advance the interpreter, so the compiled loop re-runs the traced iteration; eager execution there would double-apply the heap mutation. Remove the dead VOID_DEFER_LOG, VoidDeferEvent, void_defer_push_store/push_register/commit_at_close, and the jit_merge_point register push and loop-close commit call sites. Rename void_defer_reset to bool_box_truth_reset, its only remaining job. Verified: FBW-all-levers output byte-identical to the trait path on all 44 benches (incl. nbody enclosing-scope store, fannkuch, list ops); check.py dynasm 44/44 + cranelift 44/44; pyre-jit-trace 250 + majit-metainterp 1365 unit tests pass. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
PR #158 review chatgpt-codex P1 / coderabbit #9 flagged that the walker direct-dispatch hook records IR without invoking the helper to mutate the live container. Surface comparison with pyjitpl.py:1346 _opimpl_residual_call* makes that look like a parity gap. Extend the comment block with the structural explanation: pyre's tracing burst runs on a FrameBox snapshot (trace.rs:52-77), while the live frame is mutated by the regular execute_opcode_step that eval_loop_jit runs after jit_merge_point_hook returns (the trace trigger iteration) and by compiled-trace replay (subsequent iterations). Adding the helper call inside the walker hook double- mutates the live list, regressing list_index_update / dict_lookup_update. The PyPy-equivalent arrangement here is record-IR- only; the deeper unification belongs with the eval_loop_jit / MetaInterp::interpret epic. Pointer to try_walker_store_subscr_specialization (now execute-then- record) for the sister path that DOES invoke the helper, reached only once the codewriter emits the _r_v/iRd residual-call shape. Doc-only change. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
* refresh_virtualizable_shadow_from_heap: reserve trailing identity slot
`virtualizable_values`'s last entry stores the standard-vable identity
(`virtualizable_boxes[-1]` in RPython terms, see comment at
`virtualizable_box_at:1589`). `synchronize_virtualizable` already stops at
`static_count + sum(lengths)`; the read path now mirrors that bound so a
short or misaligned shadow (e.g. only the identity slot present, or
fewer data slots than the schema implies) cannot overwrite the
identity from heap fields.
Use `values.len().saturating_sub(1)` as the upper bound for both the
static-fields loop and the array-items cursor.
* pyframe free-function helpers: also bind bare-path aliases
`pyframe_get_pycode`, `ncells`, and `npure_cellvars` carry
`#[elidable_cannot_raise]` and are called both qualified (`pyframe::name`
from other modules) and unqualified (`name(...)` inside `pyframe.rs`
itself, e.g. pyframe.rs:783/1136/1143/1717).
`target_to_path` for a `FunctionPath` returns the segments verbatim, so
the bare call resolves to the 1-segment CallPath `["<name>"]` while a
cross-module qualified call resolves to `["pyframe", "<name>"]`. The
prior `push_fnaddr` registered only the 2-segment shape, so
`has_cannot_raise_assertion` missed the bare-path call sites and the
descrs stayed `EF_ELIDABLE_CAN_RAISE`.
Switch to `push_alias_pair` with both the 3-segment input
`pyre_interpreter::pyframe::<name>` and the 2-segment input
`pyre_interpreter::<name>`; `register_macro_helper_trace_fnaddr` strips
the leading segment of each, yielding the two CallPath shapes the
codewriter actually asks for.
* full-body walk: speculative int/float specialization for BINARY_OP/COMPARE_OP
Recognition vehicle (cross-crate: pyre-jit-trace does not depend on
pyre-jit, so fnaddr matching is unavailable):
- effectinfo: add OopSpecIndex::BinaryOp=200 / CompareOp=201, out of the
upstream OS_* range.
- flatten: build_residual_call_ir_r_insn_from_operands tags the BINARY_OP
/ COMPARE_OP helper calldescr EffectInfo with the oopspec; pin test
updated.
Full-body walk (PYRE_FULL_BODY_WALK, default-off):
- trace: run_perfn_walk / full_body_walk_trace drive dispatch_via_miframe
over the per-CodeObject JitCode with per-fn descr pool, loop-input
register seeding, and CloseLoop→CloseLoopWithArgs mapping.
- executor: execute_may_force_call runs a may-force helper concretely and
reports (boxed_result_i64, exc_value); tests for non-raising, raising,
and stale-exception-clear paths.
- jitcode_dispatch: dispatch_residual_call_iIRd_kind recognizes the
oopspec and re-emits the specialization instead of an opaque
CALL_MAY_FORCE. Int binary (Add/Sub/Mul ovf, And/Or/Xor, FloorDiv/Mod
via call_typed_with_effect_pure ll_int_py_div/mod with rhs==0 and
INT_MIN/-1 guards, Lshift ovfcheck, Rshift large-shift const-fold), int
compare (6 ops, non-fused bool box), float binary (Add/Sub/Mul/TrueDiv +
int operand CastIntToFloat), float compare (6 ops). Shared gates
walker_execute_may_force_boxed / walker_concrete_ref_object and unbox
helpers walker_unbox_int / walker_unbox_float / walker_coerce_operand_to_float.
- jitcode_runtime: overlay the full wellknown_bh_insns() universe onto the
byte→opname decode table so runtime JitCodes decode every reserved BC_*
byte.
- shadow_walker: pass authoritative=false (shadow validation must not
re-execute may-force calls).
check.py dynasm 39/39; pyre-jit-trace 232/0. Walker remains default-off;
flag-on resume (snapshot jitcode_pc coordinate) is the remaining blocker.
* full-body walk: per-opcode snapshot coordinate + branch-guard resume target
Add FULL_BODY_SNAPSHOT_SYM thread-local + FullBodySnapshotSymGuard so
walker_capture_snapshot_for_last_guard, when run from dispatch_via_miframe,
derives the snapshot's Python pc from the guard op's jitcode pc via
python_pc_for_jitcode_pc (inverse pc_map) and reads liveness from the live
walk register banks (ctx.registers_i/r/f) at that pc.
Parameterize collect_outer_active_boxes with regs_i/regs_r/regs_f slices so
the full-body walk reads its fresh top_regs banks instead of sym.registers_*.
In the goto_if_not/iL handler, pass other_target (the not-taken-in-trace
branch destination) to walker_capture_snapshot_for_last_guard instead of
op.pc, matching trace_opcode.rs resume_pc = other_target: GuardTrue resumes
at the jump target, GuardFalse at the fall-through.
PYRE_FULL_BODY_WALK=1 int_loop produces 19999999900000000 (was
20000000100000000); check.py dynasm 39/39.
* full-body walk: resolve branch-guard target through register-renaming trampoline
goto_if_not's other_target can land on a synthetic ref_copy*; goto
renaming block (emit_trampoline_for_multi_pred_link) that carries no
Python pc. resolve_branch_target_through_trampoline follows such a chain
to the py-boundary destination before stamping the guard snapshot, so
python_pc_for_jitcode_pc no longer maps the trampoline offset to a wrong
Python opcode. Consumed only on the full-body-walk snapshot path
(FULL_BODY_SNAPSHOT_SYM set); default-off control flow uses the
unresolved target unchanged.
* full-body walk: forward-skip Python trivia at branch-guard resume coordinate
The inverse-pc_map can map a resolved branch target to a Python trivia
instruction's jitcode region (e.g. a NOT_TAKEN block). A resume
coordinate must be a real opcode boundary, matching the trait path which
resumes branches at semantic_fallthrough_pc / jump_target_forward (both
forward-skip trivia). skip_python_trivia_forward advances the stamped
py_pc past Cache/ExtendedArg/Resume/Nop/NotTaken to the next opcode, so
the resume reader's backward trivia backtrack is a no-op instead of
walking a NOT_TAKEN coordinate back to the preceding branch opcode (whose
block-entry liveness differs and desynced the snapshot box-count against
the resume liveness, reading past section.values and crashing).
bool_compare full-body-walk now matches default (5809236); check.py
dynasm 39/39; int/float/nested unaffected.
* full-body walk: fold LoadConst residual_call to constant ref
Add OopSpecIndex::LoadConst (202) and tag the load_const_from_code helper
calldescr with it in build_residual_call_ir_r_single_ref_plain_insn_from_operands.
In dispatch_residual_call_iIRd_kind, when the call carries the LoadConst
oopspec and both the const index and code-pointer operands are concrete,
materialize co_consts[idx] (mirroring bh_load_const_fn) into a const_ref and
suppress the residual.
flag-on int_loop/float_loop/nested_loop now complete in 2.7-5.5s (were
10-36s / check.py timeout). check.py dynasm 39/39.
* full-body walk: bridge resume uses section pc, not vable last_instr
The full-body walk sets the concrete frame's last_instr once at the loop
header and never advances it per opcode, so decode_and_restore_guard_failure's
next_instr() (derived from the vable last_instr field) carries the header pc
for a mid-body guard. resume_in_blackhole already resumes at the per-frame
section pc (ResumedFrame.py_pc), which is correct; the bridge-compile path
read next_instr() instead and started tracing from the loop header, double-
executing the loop body before the guarded branch.
Prefer the innermost resumed-frame section pc when it disagrees with
next_instr(); for the trait tracer they always match (no-op there).
float_arithmetic flag-on now matches (was 883662 vs 998193). check.py
dynasm 39/39.
* full-body walk: skip concrete execution of void-result residual calls
try_execute_residual_call_via_walker ran every Call* concretely, including
void-result CallN/CallMayForceN (STORE_SUBSCR, list.append, dict setitem).
Those carry no value the walk specializes on; executing one only commits
its heap side effect. Since the loop body the walk traces is re-executed by
the loop itself, the committed mutation lands twice on the live heap — the
traced iteration is counted twice (list_index_update +192, dict_lookup_update
+3905). Return early when call_descr.result_type() is Void, leaving the side
effect to the loop, mirroring the trait tracer which records these
symbolically and never touches the real heap.
PYRE_FULL_BODY_WALK=1: list_index_update / dict_lookup_update / list_append_pop
now match the default backend; float_arithmetic unchanged. dynasm 39/39.
* full-body walk: abort trace on may-force call with concrete-NULL Ref arg
A user-function call whose callee is loaded from a local (or is a closure)
folds to a specialized direct-entry call: funcptr = the callee entry, with
the PUSH_NULL self-slot baked as ptr(0x0). try_execute_residual_call_via_walker
skips such a call (its NULL-receiver SEGV guard), leaving the result
unresolved, and the baked NULL arg makes the compiled call pass NULL where
the entry needs the callee's globals/closure → NULL result at runtime
(closures / locals-bound callees called in a loop returned NULL). The trait
tracer declines this shape and aborts; mirror it.
walker_abort_if_mayforce_null_ref_arg returns DispatchError::
MayForceNullRefArgUnsupported when a CallMayForce{R,I,F} carries a Ref arg
folded to GcRef(0); the driver maps the error to TraceAction::Abort so the
loop falls back to the interpreter instead of compiling a wrong trace.
Wired into the iRd / iIRd / iIRFd residual-call dispatchers.
PYRE_FULL_BODY_WALK=1: closures / function_calls / recursion /
class_attrs_methods / inheritance_dispatch now match the default backend;
24/25 synth benches match (exceptions pre-existing). dynasm 39/39,
pyre-jit-trace 232/0.
* full-body walk: abort cross-loop cut when inner merge point shape differs from jump args
The walker's jit_merge_point handler registers the inner merge point from
the JitCode merge-point reds ([frame, ec], len 2 for the portal
jitdriver), while the loop close rebuilds jump_args via close_loop_args_at
with the full virtualizable expansion (locals + stack). When the trace
starts before the loop header (loop_header_pc != start_pc) a cross-loop
cut replaces the trace inputargs with the reds-only merge-point boxes,
but next_iteration_args keeps the expanded length; for loops that
virtualize a heap object (fannkuch list, nbody body tuples) the unroll
peel then reads an inputarg slot past the cut list end and panics in
inputarg_type_at_strict.
Detect the shape mismatch at close and abort the full-body walk so the
portal falls back to the trait tracer. Loops without a heap virtual have
matching shapes and still close under the walk.
dynasm + cranelift check.py 39/39 each; pyre-jit-trace 232/0. fannkuch /
nbody flag-on now produce correct output instead of panicking.
* full-body walk: abort may-force trace in functions with exception handlers; fix walker_unbox class_now_known arg type
A may-force residual call that can raise emits GUARD_NO_EXCEPTION. When
that guard fails at runtime the deopt bridge must resume into the
function's exception handler, but the walker's guard resume snapshot does
not seed the standing exception value, so the bridge dereferences a NULL
exception object (GuardClass ldr [x0], x0=0) and segfaults
(exceptions.py, N>=~1000). Abort the walk when the jitcode body contains
any catch_exception op so the portal falls back to the trait tracer,
which resumes the handler correctly. Bodies without catch_exception
(nbody / fannkuch / loop benches) are unaffected and still compile under
the walk.
Also fix walker_unbox_int / walker_unbox_float passing GcRef to
HeapCache::class_now_known, whose parameter is i64; the type error was
latent because incremental compilation had cached the stale typecheck.
dynasm + cranelift check.py 39/39 each; pyre-jit-trace 232/0. exceptions.py
flag-on now matches flag-off (1125000) instead of SIGSEGV; nbody / loop
benches unchanged under flag-on.
* full-body-walk: bail InvalidLoop on next_iteration_args longer than inputargs
Replace the trace.rs CloseLoop cross-loop-cut shape-mismatch abort with an
optimizer-level InvalidLoop bail.
The CloseLoop guard returned TraceAction::Abort after the authoritative walk
had already executed the loop body concretely, so loops with observable
side-effects (nested_loop's print) ran twice. The unroll peel's typed_inputargs
loop now panics InvalidLoop when next_iteration_args has an entry past the cut
inputarg list end, routing through compile_loop_body's catch (interpreter
fallback) without re-executing the body, instead of inputarg_type_at_strict
hard-panicking the worker thread.
* full-body-walk: don't concretely execute LoadConst residual calls
try_execute_residual_call_via_walker skips calls whose descr carries the
LoadConst oopspec. The LOAD_CONST helper has a dedicated fold in the
residual_call dispatchers that materializes co_consts[idx] when both the
const index and the code pointer (frame.pycode) are concrete; when that fold
declines, the residual is recorded so the loop computes it at runtime from
the live frame's pycode. An inlined-callee sub-walk does not seed
frame.pycode concrete, so the fold declines, and concretely executing the
recorded residual passed the unseeded code pointer to bh_load_const_fn, which
dereferenced it via w_code_get_ptr and faulted (SIGSEGV when a float-returning
helper with a co_const is inlined into a hot loop).
* optimizeopt: fold standard-virtualizable getarrayitem read-after-write
optimize_getarrayitem_gc folds a constant-index read on the standard
virtualizable array field to the box tracked in vstate.arrays (seeded
from the inputarg layout, updated by mirror_setarrayitem), mirroring the
static-field fold in optimize_getfield_gc. The matching setarrayitem is
left emitted, so no heap write is dropped; only the redundant read is
removed. Adds VirtualizableTracker::tracked_array_element.
* jitcode_dispatch: precise may-force exception-handler abort gate
walker_abort_if_protected_may_force aborted the full-body walk for any
may-force-can-raise call whenever the body contained any catch_exception.
Resolve the call's Python PC via python_pc_for_jitcode_pc (the inverse
pc_map already used by walker_capture_snapshot_for_last_guard) reached
through FULL_BODY_SNAPSHOT_SYM, then lookup_exceptiontable on the
CodeObject's exceptiontable: abort only when that Python opcode is
actually covered by a handler. Calls with no covering handler raise out
of the frame and are safe to walk. Falls back to the whole-body scan
when no full-body sym pointer is set.
flag-on exceptions.py=1125000 and raise_catch_loop=1142858 (both match
cpython); check.py 39/39 dynasm; pyre-jit-trace 232/0.
* WIP: FBW #67/#62 experiments (candidate-b shape fix, boxing/subscr oopspec, graceful abort)
effectinfo: add OopSpecIndex::BoxInt (203) and StoreSubscr (204) tags.
trace_ctx: graceful-abort try_set_opref_concrete (returns false when no
frontend op/inputarg is recorded; routes through recorder.set_concrete_at
per the #123 canonical-op-identity model, no BoxPool side-table).
jitdriver: TraceContinuationSuspendGuard around may-force concrete execution.
optimizer: InvalidLoop bail on next_iteration_args longer than inputargs.
jitcode_dispatch: append_virtualizable_boxes merge-point shape fix +
entry_py_pc snapshot substitution + BoxInt/StoreSubscr walker dispatch.
trace: deferred-void-store event log + no-loop entry seeding.
flatten: box_int/store_subscr calldescr tagging.
Experimental; proven worse than the clean InvalidLoop-bail baseline.
FBW-flag-gated (PYRE_FULL_BODY_WALK, default off); inert at default env.
Assisted-by: Claude
* jitcode_dispatch: advance resume coordinate past may-force call for after_residual_call guards
The full-body-walk guard-snapshot helper mapped a guard's jitcode op_pc back
to the Python opcode CONTAINING it and resumed there. For GUARD_NOT_FORCED /
GUARD_NO_EXCEPTION emitted after a may-force residual call, that opcode is the
call's own opcode (e.g. STORE_SUBSCR); the call already ran in compiled code
and consumed its Python stack operands, so resuming at it re-executes the call
from a coordinate whose stack no longer holds them.
Split walker_capture_snapshot_for_last_guard into an _impl taking
after_residual_call, add walker_capture_snapshot_after_residual_call, and route
the four may-force guard sites (release-gil + iRd dispatcher GUARD_NOT_FORCED /
GUARD_NO_EXCEPTION) through it. When set, the resume coordinate advances via
semantic_fallthrough_pc to the next executable opcode, matching
capture_resumedata(after_residual_call=True) (pyjitpl.py:2599-2603).
FBW-gated (PYRE_FULL_BODY_WALK, default off). check.py 39/39 flag-off; flag-on
int/nested/list-read loops unaffected.
Assisted-by: Claude
* compile_loop_body: convert SpeculativeError to InvalidLoop bail
The optimize catch in compile_loop_body handled only InvalidLoop; a
SpeculativeError (constant_fold of an ill-typed speculative heap access,
raised via panic_any at optimizeopt/mod.rs:534) fell through to
resume_unwind, propagating the panic across the JIT FFI boundary and
crashing the interpreter. unroll.py:119-123 maps SpeculativeError to
InvalidLoop; treat both the same here so the optimized trace is
abandoned and tracing continues, rather than re-raising.
Assisted-by: Claude
* eval: full-body-walk guard failures resume via blackhole, skip bridge
Under PYRE_FULL_BODY_WALK, handle_fail now returns ResumeInBlackhole for
every guard failure instead of compiling and attaching a bridge. The
full-body walk sets the concrete frame's last_instr once at the loop
header and never advances it per opcode, so a guard's per-PC resume
coordinate and partial operand stack are reconstructed precisely only by
the blackhole's rd_numb consume_one_section decode. A bridge compiled
and attached from such a resume produces compiled code that mishandles
the resumed state — a hot in-place list-swap loop dropped one iteration
at the round an inner-loop guard's failure counter reached
trace_eagerness. Forcing the blackhole path matches the behavior proven
correct under the diagnostic no-bridge mode.
The flag is cached in a OnceLock because handle_fail is hot on
recursion-heavy guard chains; a per-call env read regressed
fib_recursive past its timeout. The trait tracer (flag-off) is
unaffected.
Assisted-by: Claude
* jitcode_dispatch tests: add raw_descrs/is_authoritative_executor to two WalkContext literals
The two vable-box-not-seeded test constructions predate the
raw_descrs/is_authoritative_executor WalkContext fields; set them to the
production defaults (RawDescrPool::Global, false).
Assisted-by: Claude
* full-body walk: abort may-force trace in any exception-handler body
walker_abort_if_protected_may_force returned Ok(()) unconditionally under
the full-body walk, relying on bridge handler-resume seeding to supply the
standing exception value. That seeding does not cover the multi-frame
shape where an inlined callee raises and the caller's handler reads the
exception (synth/exceptions: may_fail raises, main catches and reads
e.args[0]): the exception-path bridge bakes a NULL exception object and the
compiled GuardClass dereferences x0 == 0 (SIGSEGV).
Restore the conservative whole-body gate for the full-body walk too: abort
to the trait tracer whenever the walked body contains any catch_exception.
Bodies without a handler still compile under the walk.
flag-off check.py 41/41; flag-on synth/exceptions no longer crashes
(1125000, matches flag-off) and the flag-on suite is 34/41 (was 33/41 with
the crash), no other bench regressed.
Assisted-by: Claude
* walker int/float specialization: exclude bool operands
walker_int_specialization_operands and walker_float_specialization_operands
accepted bool operands because is_int returns true for W_BoolObject. The
specialization then unboxed them via the int path (GuardClass INT_TYPE +
GetfieldGcPureI intval@16), but W_BoolObject has its own &BOOL_TYPE vtable
and a 1-byte boolval field, not W_IntObject's 8-byte intval at offset 16.
A boolean compare result used as an arithmetic operand (e.g. `x = a < b;
s = s + x`) was therefore miscompiled under PYRE_FULL_BODY_WALK: the
GuardClass guarded the wrong class and the getfield read 7 bytes past
boolval.
Reject bool in both specializations so such operands route through the
generic residual call, which forces the correct value.
Assisted-by: Claude
* walker list subscr/store specialization: exclude bool index and value
try_walker_specialize_subscr and try_walker_specialize_store_subscr gated
the index operand (and the int-storage store value) on is_int, which is
true for W_BoolObject. A bool index (lst[a < b]) or bool value stored into
an int-storage list was unboxed via the int path (GuardClass INT_TYPE +
getfield intval@16), reading the 1-byte boolval as an 8-byte intval and
guarding the wrong class.
Reject bool in both gates so these operands route through the generic
residual call.
Assisted-by: Claude
* full-body walk: abort branch guard whose resume target keeps an operand-stack temp
A goto_if_not branch guard resuming at a target with operand-stack depth
> 0 — the short-circuit `and`/`or`, conditional expression, and chained
comparison shapes, where CPython keeps the tested value on the value
stack across the branch — corrupts a loop-carried slot on guard-failure
deopt: the single-frame snapshot does not model the kept temp on the
not-taken arm. Detect this via the resume-target stack depth
(branch_resume_target_stack_depth, full-body-walk only) and surface
DispatchError::BranchGuardKeptStackUnsupported, mapped to AbortPermanent
→ interpreter fallback. Plain while/if branches resume at depth 0 and
still compile.
flag-off check.py 41/41; flag-on 34/41 (unchanged, remaining 7 are perf
timeouts); pyre-jit-trace 234/0.
Assisted-by: Claude
* full-body walk: elide dead box_bool when compare result is consumed only by the branch
A COMPARE specialization (try_walker_specialize_compare_op_int/_float)
emits the raw truth plus a box_bool CallR that lands a W_Bool in the Ref
dst. When the only consumer is the immediately-following is_true
(POP_JUMP_IF_*), that op folds to the raw truth (bool_box_truth), leaving
the box dead — but it is a non-pure CallR the optimizer cannot DCE, so it
runs every loop iteration.
compare_box_provably_dead does a forward JitCode lookahead proving the
boxed Ref dst is read by exactly one op (an is_true residual), never
overwritten before it, the scan ends at a goto_if_not, and the dst color
is dead at both branch arms (so the guard snapshot cannot capture the
marker). When proven, the box is elided: write the truth into the Ref dst
as a marker and record bool_box_truth(truth, truth) so the is_true fold
resolves it. Any other shape (escape to a local, arithmetic use, second
reader, register reuse, kept-on-stack short-circuit) falls back to
emitting the real box. Full-body-walk only.
flag-off check.py 41/41; flag-on 34/41 → 36/41 (int_loop now passes,
nested_loop improved); pyre-jit-trace 234/0. Escape cases (x=i<n; s=s+x),
bool family, and plain while/if verified flag-on == cpython.
Assisted-by: Claude
* full-body walk: add env-gated call-inlining recognition probe
dispatch_residual_call_iRd_kind gains a PYRE_DIAG_INLINE_RECOG-gated
diagnostic (authoritative-executor path only) that reads each Ref arg's
concrete value, detects a pure-Python function (FUNCTION_TYPE, via
is_function + ob_type), resolves its CodeObject, and reports whether a
per-fn JitCode is present in the setup-time and codewriter stores.
On inline_helper the probe identifies the callable at arg#1 of every
call_fn and reports both stores MISS: pyre builds per-fn JitCode lazily,
so the callee JitCode is absent at caller-trace time. The on-demand build
path jitcode_for (state.rs:593) was measured to build+install these
callees successfully.
No behavior change without the env var (single bool guard per call_fn).
Assisted-by: Claude
* full-body walk: extend inline recognition probe with call_fn arg layout
The PYRE_DIAG_INLINE_RECOG probe now reports nargs, a per-arg concrete
kind shape, and the callable index. On inline_helper the layout is
[ctx, callable, positional_args..]: callable at index 1, positional args
from index 2 (1-arg call → nargs 3, 2-arg call → nargs 4).
Assisted-by: Claude
* full-body walk: add sub_jitcode_body_for_code callee-body obtain helper
sub_jitcode_body_for_code(code) builds (via jitcode_for) and views a
callee per-fn JitCode as a SubJitCodeBody. Unlike the per-function
sub_jitcode_lookup (indexed by the current function's descr pool), this
resolves an arbitrary runtime callable's code, which call inlining needs.
The Arc<PyJitCode> is program-lived in the append-only jitcodes store, so
the 'static slice extension matches the per-fn arm-entry borrow extension
at trace.rs:363.
The PYRE_DIAG_INLINE_RECOG probe now exercises it, reporting each callee's
register-bank shape (inline_helper: add/mul body 27 bytes regs_r=17,i=1;
compute 96 bytes regs_r=19). Gated; flag-off 41/41 unchanged.
Assisted-by: Claude
* full-body walk: dev-gated user-function call inline sub-walk (PYRE_FBW_INLINE)
try_walker_inline_user_call sub-walks a recognized user-function call_fn in
place of the residual call: exact-positional, closure-free callees only;
binds r_args[2..] to callee locals [0..nparams]; runs walk() over the callee
per-fn JitCode (sub_jitcode_body_for_code); writes SubReturn to the call dst;
routes SubRaise through try_catch_exception_at. Sub-walk DispatchErrors
propagate as a trace abort (sound; never mixes inlined + residual emission).
Guards inside resume to the caller CALL boundary via the inherited
single-frame snapshot.
Gated by PYRE_FBW_INLINE (default off); production flag-on path unchanged,
flag-off 41/41. Validated correct on a 200k-iter inline_helper variant
(matches CPython/flag-off/flag-on = 647933338). Currently perf-negative
(0.53s vs 0.35s gate-off) — the single-frame-resume inline yields a correct
but slower trace; trace-quality / multi-frame-resume work is the next step.
Assisted-by: Claude
* full-body walk: diagnose inline sub-walk trace aborts (PYRE_FBW_INLINE_DIAG)
Add a gated log of the callee sub-walk DispatchError. On inline_helper the
dev-gated inline produces correct output but aborts 25 traces; the errors
pinpoint two callee frame-setup gaps the bare sub-WalkContext skips:
VableBoxNotSeeded / VableArrayDescrMalformed (callee frame vable not seeded)
and ResidualCallDescrNotCallDescr (a nested call indexes the parent descr
pool, not the callee's own pool). These are the next steps for the inline to
compile rather than fall back to the interpreter.
Assisted-by: Claude
* full-body walk: decode-dump callee body under PYRE_FBW_INLINE_DIAG
Dump the callee per-fn JitCode opcodes. Confirms the inline-abort root: the
add callee reads its params via getfield_vable_r at pc 3 (a virtualizable
frame access), not from registers. So the register-binding sub-walk does not
match how a per-fn JitCode reads locals; the callee virtualizable frame must
be seeded with the args (build_pending_inline_frame-style) rather than
binding callee registers.
Assisted-by: Claude
* full-body walk: build real callee frame + per-fn descr pool for inline sub-walk
The callee per-fn JitCode is portal-shaped: LOAD_FAST lowers to
getfield_vable_r(r0=frame, localsplus[i]), so the inline sub-walk needs a
genuine heap frame at r0 (read via the _nonstandard_virtualizable ->
GETFIELD_GC fallback), not register-bound args. try_walker_inline_user_call
now builds the concrete callee frame (PyFrame::try_new_for_call_with_closure_
and_globals_obj over the concrete positional args + the caller frame's
execution context) and emits the symbolic callee-frame OpRef via the
one_arg_callee_frame_helper / callee_frame_helper(n) helpers, then seeds
callee_regs_r[0] / callee_concrete_r[0] with it.
The callee body resolves its d/j descr operands through its OWN per-fn pool,
not the caller's. sub_jitcode_descr_pool_for_code (memoized per code ptr,
mirror of trace.rs:363-400) builds the callee's adapted descr_refs +
RawDescrPool::PerFn + sub_jitcode_lookup; the sub_wc now uses them.
Moves the PYRE_FBW_INLINE sub-walk abort past VableBoxNotSeeded (no frame at
r0) and ResidualCallDescrNotCallDescr (wrong descr pool). Remaining blocker:
VableArrayDescrMalformed at the LOAD_FAST sites — the callee jitcode lacks the
(VableArray, Array) descr pair. Dev-gated PYRE_FBW_INLINE (default off);
flag-off check.py 41/41.
Assisted-by: Claude
* full-body walk: diagnose VableArrayDescrMalformed in inline sub-walk
Add a PYRE_FBW_INLINE_DIAG-gated dump at the (VableArray, Array) pair
mismatch arm of vable_array_descrs_from_jitcode + a RawDescrPool::len()
helper. On inline_helper the callee sub-walk reports pool_len=22,
field_idx=21 resolving to the body's BinaryOp Call descr and array_idx=22
out of range — the decoded descr indices point at the tail of the callee
pool, not a vable-array pair, indicating a descr-operand-offset/routing
mismatch in the callee body decode rather than an absent descr.
Dev-gated; no production path change.
Assisted-by: Claude
* full-body walk: seed callee entry registers per setup_call convention
Straight-line callee entry (pc=0, no governing loop header) places the
jitdriver args [pycode, frame, ec] at r0/r1/r2 (trace.rs:341-345), and the
body reads its vable from r1 — not r0. The inline sub-walk previously seeded
only r0=frame, so the body's getfield_vable_r read r1 (NONE) and aborted
VableBoxNotSeeded. Seed all three (pycode=const_ref(w_code), frame=callee
frame OpRef, ec=const_ref(exec_ctx)) with concretes, and require >=3 Ref
register slots.
With this the PYRE_FBW_INLINE sub-walk runs the whole callee body with zero
decode / descr / vable aborts (VableBoxNotSeeded + VableArrayDescrMalformed
both gone). The next blocker is multi-frame guard resume: a callee-emitted
GuardValue has resume_pos=-1 (store_final_boxes_in_guard panic) because the
inline guards lack a snapshot modelling the caller+callee frame stack
(#124/#65 family). Dev-gated PYRE_FBW_INLINE; default off.
Assisted-by: Claude
* full-body walk: emit callable guard_value + no-exception guard for inline
pyjitpl recursive_call specializes the inlined body on the exact callable via
guard_value(callable) before building the callee frame, and emits
GUARD_NO_EXCEPTION right after the frame-build helper
(build_pending_inline_frame:5986 + 6153). The inline sub-walk now emits both
(with walker_capture_snapshot_for_last_guard), and pre-checks frame-helper
availability before any IR so the unsupported-arity path bails cleanly.
Advances the PYRE_FBW_INLINE trace build further (the callable-specialization
GuardValue now carries a snapshot). One orphan GuardValue (resume_pos=-1)
remains at store_final_boxes_in_guard time, emitted by a non-walker-record
path (may-force promotion / multi-frame resume integration, the (4) frontier
documented at jitcode_dispatch.rs:4756). Dev-gated PYRE_FBW_INLINE; default
off.
Assisted-by: Claude
* full-body-walk inline: capture resume snapshot for helper-internal promote guards
The `_nonstandard_virtualizable` check in `TraceCtx`'s vable_* helpers
records a PTR_EQ + `promote_int` GuardValue internally for a frame that is
not the standard virtualizable, then `emit_force_virtualizable` records
GETFIELD_GC / PTR_NE / COND_CALL after it. Only an inlined callee's heap
frame is non-standard (the production main frame is the standard
virtualizable, so the check short-circuits and emits nothing), so the
walker never attached a resume snapshot to that guard and
`store_final_boxes_in_guard` reached it with `rd_resume_position == -1`.
Add Recorder::set_last_guard_op_resume_position (stamps the most-recent
guard op, skipping the non-guard ops recorded after the promote) and the
matching History::capture_snapshot_for_last_guard_op_with_vable_vref;
`set_last_op_resume_position` could not be reused because the guard is not
the last op once emit_force_virtualizable runs.
In the walker, gate a guard-count delta check (TraceCtx::num_guards)
around each vable_* call via INLINE_SUBWALK_CAPTURE_BOUNDARY (set only
during a `try_walker_inline_user_call` sub-walk). When a guard was emitted,
capture a single-frame snapshot at the caller's CALL boundary
(outer_active_boxes / entry_py_pc) — re-executing the call on deopt — and
stamp it on the guard op. The same flag routes the existing
walker_capture_snapshot_for_last_guard away from the callee-pc-via-outer-
pc_map mapping for inline sub-walks.
Dev-gated by PYRE_FBW_INLINE; no-op when the flag is unset (one num_guards
read). check.py flag-off 41/41, majit-metainterp 1277, pyre-jit-trace 234.
Assisted-by: Claude
* full-body-walk inline: seed callee registers with args (fast-path convention)
The inlined callee body is entered at pc=0 and reads its parameters
straight from registers_r[0..nparams], not via getfield_vable against a
heap frame. Decode of add/mul/square/compute confirms the body is
`ref_copy r0->r0 / ref_copy r1->r1 / residual_call(r0,r1) / ref_return r0`
— the can_skip_traced_callee_frame convention (build_pending_inline_frame:
`sym.registers_r = args.to_vec()`; same seeding dispatch_inline_call_dr_kind
uses). The previous code built a heap callee frame and seeded
[pycode@r0, frame@r1, ec@r2] (the portal-entry convention), so the callee
computed `pycode + frame` and failed at runtime with "TypeError: call
failed".
Replace the heap-frame build + frame-build helper + GuardNoException with
direct arg seeding: callee_regs_r[i] = r_args[2+i], concretes likewise.
Keep the guard_value(callable) specialization. A callee that materializes
a frame (extra locals → getfield_vable on an unseeded register) aborts
cleanly with VableBoxNotSeeded -> trait fallback, never a miscompile.
ih_tiny (single-level), ih_small (nested compute->add->square), and
bench/inline_helper.py now all produce correct output under
PYRE_FBW_INLINE. Trace quality / compile (the traces still abort or
guard-thrash, so no perf win yet) is the remaining #62 work. check.py
flag-off 41/41.
Assisted-by: Claude
* optimizeopt: migrate tracked_array_element to BoxRef array source
resolve_array_source takes &BoxRef after the box-identity migration, but
its read-path caller tracked_array_element still took OpRef and the
optimize_getarrayitem_gc call site passed an out-of-scope `array_ref`.
Under --no-default-features --features dynasm this failed to compile
(E0308 expected &BoxRef found OpRef; E0425 array_ref not in scope).
Thread the BoxRef through tracked_array_element symmetrically with the
already-migrated mirror_setarrayitem write path.
Assisted-by: Claude
* FBW: dev-gated LOAD_GLOBAL cell-cache fold (PYRE_FBW_LOADGLOBAL_FOLD)
Add OopSpecIndex::LoadGlobal (205) and tag the load_global_fn calldescr in
build_load_global_fn_insn_from_operands. The full-body walker's
dispatch_residual_call_iIRd_kind recognizes the tag and, when namei /
namespace / promoted pycode are concrete, emits the cell-cache fast path
(quasiimmut_field + record_known_result + elidable jit_namespace_cell_lookup,
plus getfield on an ObjectMutableCell) mirroring the trait load_global_value,
folding the per-iteration module-global read to a loop-invariant cell pointer.
The reader is gated three ways: is_authoritative_executor, no catch_exception
in the body (load_global_fn is CallFlavor::Plain/can-raise; a handler-bearing
body must keep falling back via walker_abort_if_protected_may_force), and the
PYRE_FBW_LOADGLOBAL_FOLD env flag (default off). When enabled, a folded
function callee that is then CALLed mis-resolves through the in-progress FBW
call-inlining path and yields wrong output for global-function-call loops, so
the fold stays default-off until that path lands.
Adds pyframe::load_name_from_code (co_names[idx] accessor).
Default behavior unchanged: check.py dynasm 41/41 flag-off, 35/41 flag-on;
majit-ir 303+5, pyre-jit-trace lib 234.
Assisted-by: Claude
* FBW: route bridge traces through full_body_walk_trace; skip merge-point for bridges
Revert the trait-bridge hybrid: trace_bytecode no longer gates the walker on
`!ctx.is_bridge_trace`, so under PYRE_FULL_BODY_WALK every trace, main and
bridge, goes through full_body_walk_trace (single tracer).
full_body_walk_trace skips add_merge_point when ctx.is_bridge_trace: a bridge
resumes at a mid-loop pc, not a loop header, so it registers no portal entry
signature there.
flag-off check.py 41/41; flag-on default 35-36/41 (both unchanged — the edits
are inert unless PYRE_FBW_BRIDGE enables guard-fail bridges, default-off). The
bridge float-accumulator resume bug (#67) is unchanged: setup_bridge_sym leaves
an unboxed loop-carried scalar's localsplus slot at its stale trace-time box.
Assisted-by: Claude
* FBW: publish last_instr=py_pc-1 to vable shadow at walker guard snapshot
walker_capture_snapshot_for_last_guard_impl resolves the guard's resume
py_pc but did not update the `last_instr` static vable scalar before
building the snapshot boxes. The walker walks JitCode and never crosses
`set_orgpc`, so `virtualizable_boxes[last_instr]` kept the value the trace
seed / previous `close_loop_args_at` override wrote (the loop-header pc).
The blackhole / vable-sync resume reads that scalar into `frame.last_instr`,
so a mid-body guard resumed at the loop header instead of its own opcode.
Mirror `MIFrame::publish_last_instr_to_vable`: publish `py_pc-1` into the
shadow (gated on `owns_virtualizable_shadow`) before
`build_snapshot_vable_vref_boxes`, and defer that build until after the
publish. Make `mirror_vable_static_to_boxes` pub(crate).
Verified: flag-on+PYRE_FBW_BRIDGE spectral_norm bridge resume now writes
frame ni=94 (was 13), matching the trait. check.py dynasm 41/41,
pyre-jit-trace lib 234/0.
Assisted-by: Claude
* FBW: emit intermediate merge-point vable→heap writeback at walker register branch
The walker `jit_merge_point` register branch returned `Continue` without
the vable→heap writeback that the trait's `close_loop_args_at`
(trace_opcode.rs:2875-2950) runs at every merge point: override the vable
`last_instr` scalar to `merge_pc - 1`, mirror it into the
`virtualizable_boxes` shadow, and on a reduced (reds-only) target LABEL
call `gen_writeback_vable_to_heap`. The walker's loop-close path already
routes through `close_loop_args_at` (run_perfn_walk) so it emitted the
final writeback, but an intermediate loop header reached mid-trace — e.g.
a bridge re-entering the inner loop — skipped it, so the compiled body
left the heap PyFrame `last_instr`/array at the trace-seed state.
With the writeback emitted, the flag-on+PYRE_FBW_BRIDGE walker bridge for a
nested while loop now carries the intermediate GcStore(_, 32, merge_pc-1)
and its op count matches the trait (pre-opt 75→105, compiled 36→46).
check.py 41/41 dynasm + 41/41 cranelift.
Assisted-by: Claude
* FBW: restore live frame to guard resume state after bridge trace
`trace_and_compile_from_bridge` set the live frame's `last_instr` to the
guard `resume_pc` before tracing so that a `BridgeCompiled` outcome
re-enters `eval_loop_jit` at the guard point — the
`ContinueRunningNormally` arm runs the interpreter forward from the live
frame's `last_instr` / value stack without reconstructing it.
The trait tracer interprets a private `snapshot_for_tracing` copy and
never touches the live frame. The full-body walker executes may-force
residual calls concretely through the shared execution context during
the walk, which advanced the live frame's `last_instr` to a walked
opcode (a jitcode-region pc) and stepped its locals (a loop counter) to
the body's concrete state. The post-bridge interpreter then resumed
mid-body (value-stack underflow) or past a dropped loop iteration
(off-by-one-iteration result) under PYRE_FULL_BODY_WALK=1 +
PYRE_FBW_BRIDGE=1.
Snapshot the resume state (last_instr, valuestackdepth,
locals_cells_stack_w) before the walk and restore it afterward via the
new `PyFrame::restore_resume_state_from`. The trait path is a no-op
(it never mutates the live frame).
Verified: nested-loop bridges f(1200,3)=2158200, f(2000,10)=89955000,
spectralnorm(50)=1.2741938369830932 all correct and deterministic under
flag-on+bridge; check.py 41/41 dynasm + 41/41 cranelift ×2.
Assisted-by: Claude
* FBW: recover guard-resume section pc from build_resumed_frames return
decode_and_restore_guard_failure now binds build_resumed_frames's return
and reads the innermost resumed frame's py_pc to derive the resume
coordinate, replacing a read of the removed LAST_GUARD_FRAMES thread-local.
Assisted-by: Claude
* compile_bridge: give up when terminal JUMP arg count differs from target LABEL
A bridge whose terminal JUMP targets an already-compiled loop must supply
as many args as that loop's LABEL; the backend regalloc asserts
arglocs.len() == target_arglocs.len(). When a full-body walk closes a bridge
against an outer-loop LABEL grown by an unroll short preamble, the counts can
disagree. Return false (giveup) before the backend stage so blackhole resume
handles it, instead of letting the backend panic. The check is skipped when
target_arglocs is empty (not-yet-compiled target).
Assisted-by: Claude
* optimizeopt: wrap virtualize test op args in BoxRef::from_opref
The tracked_array_element BoxRef-array-source migration left one
virtualize.rs unit test passing `OpRef` where `Op::with_descr`,
`setarg`, and `get_box_replacement` now take/return `BoxRef`, so the
lib-test build did not compile. Wrap the args in `BoxRef::from_opref`,
matching the already-migrated sibling test in the same file.
Assisted-by: Claude
* pyjitpl: give up cross-loop-CUT self loop with NULL-fed class-guarded slot
A cross-loop CUT synthesizes a self-loop LABEL whose inputargs include
valuestack temporaries promoted from the inner merge point via the
escaped-ref BFS. When the optimized body class-guards such a slot
(GuardClass / GuardNonnullClass / GuardNonnull) while the closing Jump
feeds back a Const NULL for that slot, the LABEL/JUMP contract is
self-inconsistent: the loop dereferences NULL on its back edge. The
no-unroll retry path (pyjitpl.py:3044-3054) carries no virtual state to
reject it, the way virtualstate.py:595-606 _generate_guards_knownclass
would. Detect the inconsistency in compile_loop_body and return
CompileOutcome::Aborted (give up to the blackhole, as the retry-failure
path does) instead of installing the loop. Gated on
cut_inner_green_key.is_some() so the full-body-walk-off path is unchanged.
This removes the fannkuch SIGSEGV under
PYRE_FULL_BODY_WALK=1 PYRE_FBW_BRIDGE=1; the giveup fires on slot 24 and
the output matches the trait-dispatch reference.
Assisted-by: Claude
* flatten: LoadGlobal residual_call tests expect oopspecindex=LoadGlobal
build_load_global_fn_insn_from_operands tags the calldescr EffectInfo
with OopSpecIndex::LoadGlobal. Three tests still constructed their
expected EffectInfo via effect_info_for_call_flavor(CallFlavor::Plain),
which leaves oopspecindex None, so they failed once the workspace test
build could run. Update the expected EffectInfo in
build_load_global_fn_residual_call_ir_r_insn_emits_residual_call_ir_r,
build_load_global_fn_with_const_pycode_uses_const_ref_for_code_only, and
build_load_global_fn_residual_call_ir_r_insn_matches_flatten_of_residual_call_op
to set oopspecindex = LoadGlobal, mirroring the LoadConst sibling test.
Assisted-by: Claude
* FBW LOAD_GLOBAL fold: index co_names by namei >> 1
try_walker_load_global_cell_fold passed the raw LOAD_GLOBAL oparg to
load_name_from_code as the co_names index. The oparg is
(name_idx << 1) | push_null_flag, so the fold resolved the wrong name for
any global whose load carries the push-NULL flag (e.g. a global at
co_names[0] read co_names[1]), folding the in-loop callee to a different
global's cell. bh_load_global_fn already derives the index as namei >> 1;
mirror that in the fold.
Reword the dev-gate comment: the fold is correct and reaches production
parity for global-function-call loops with PYRE_FBW_INLINE; the gate stays
pending Phase-5 full-FBW-bench validation.
Assisted-by: Claude
* FBW inline: bound recursive inlining at max_unroll_recursion (=7)
try_walker_inline_user_call had no recursion bound, so a self-recursive
callee (fib) unrolled its exponentially branching call tree at trace
time, never terminating. Add a thread-local inline stack of callee
w_code pointers with an RAII guard around the sub-walk; bail to a
residual call (Ok(None)) once a callee is already FBW_MAX_INLINE_RECURSION
deep, mirroring the trait tracer's recursive_depth >= max_unroll_recursion
gate. fib_recursive under PYRE_FBW_INLINE now completes with correct
output instead of exploding; loop benches stay at trait parity.
Assisted-by: Claude
* FBW inline: decline frame-materializing callees instead of aborting trace
The fast-path inline seeds only r0..nparams; a callee that reads its
params from the virtualizable frame (any *_vable_* op, emitted when a
local must survive a sub-call) hit VableBoxNotSeeded mid sub-walk, and
that Err propagated up to abort the *entire* enclosing loop trace. With
PYRE_FBW_INLINE on, inline_helper's loop therefore never compiled and
ran fully interpreted.
Add callee_fast_path_inlinable: pre-scan the callee body for vable ops
before recording any IR and return Ok(None) (residual call) when found,
mirroring the orthodox should_inline=False -> do_residual_call path.
inline_helper under PYRE_FBW_INLINE now compiles with zero aborts; loop
benches stay at trait parity.
Assisted-by: Claude
* FBW inline: resolve callee frame-static reads to constants; defer iIRd arg-bound past folds
try_walker_inline_user_call's sub-walk seeds only the callee's positional
arg registers, leaving its portal frame box unseeded. A callee that reads a
scalar getfield_vable_r off that frame (pycode field 1 / w_globals field 5,
for LOAD_CONST / LOAD_GLOBAL setup) aborted the trace with VableBoxNotSeeded,
so callee_fast_path_inlinable declined every *_vable_* callee.
Resolve those reads to the callee's compile-time constants (w_code,
function_get_globals_obj) via a thread-local InlineCalleeConsts stack pushed
for the sub-walk lifetime, mirroring the codewriter non-portal branch
(codewriter.rs:6720-6732 / :7347-7369). callee_fast_path_inlinable now admits
a callee whose only vable ops are getfield_vable_r on field 1/5.
In dispatch_residual_call_iIRd_kind, move ensure_residual_call_args_bound
after the LoadConst/LoadGlobal folds: a folded call records nothing, so an
inlined callee's load_global passing its own unseeded portal_frame_reg no
longer trips the bound check.
With PYRE_FBW_INLINE + PYRE_FBW_LOADGLOBAL_FOLD, inline_helper inlines
compute/square/add/mul fully (0 aborts) and matches trait timing (was >235x
slower). check.py dynasm+cranelift 41/41; pyre-jit-trace tests pass.
Assisted-by: Claude
* FBW: execute iIRFd may-force residual via walker, matching iRd/iIRd
dispatch_residual_call_iIRFd_kind recorded the may-force residual and ran
try_fold_pure_call_via_executor but omitted the
try_execute_residual_call_via_walker call that the iRd and iIRd
dispatchers make after the pure-fold. The boxes3 (i++r++f) arglist shares
the same do_residual_call body as the r and ir shapes, so the float-arg /
float-return shape now executes concretely and stamps the result the same
way. Void-result stores are deferred inside the helper's
result_type()==Void arm, so a compiled loop re-run does not double-apply
the store.
The irf residual shape is not emitted by reachable pyre paths today: the
execute path records 0 hits across float_loop/int_loop/fib_loop/
nested_loop/nbody_200 under PYRE_FULL_BODY_WALK=1, so this is
behavior-neutral on the current bench suite. Gate dynasm 41/41 +
cranelift 41/41, pyre-jit-trace 242 tests.
Assisted-by: Claude
* pyrex: suppress SpeculativeError panic message like InvalidLoop
The panic hook silenced InvalidLoop (a caught control-flow panic) but not
SpeculativeError, which the optimizer raises to decline a speculative heap
fold. compile_loop / compile_bridge catch SpeculativeError and abandon the
trace (blackhole fallback = correct result, rc=0), but the default hook
still printed "panicked at optimizeopt/mod.rs" to stderr during the unwind.
check.py treats any "panicked" in stderr as a failure, so a fannkuch loop
compile that gracefully declined a fold under FBW+PYRE_FBW_BRIDGE was
reported as a crash though it ran correctly (8629/30, internal_compile_
panics=0).
Suppress SpeculativeError in the hook symmetrically with InvalidLoop; both
are silent exceptions per unroll.py:119-123. A genuinely uncaught one still
aborts with a nonzero exit, so only the false crash message is removed.
Gate: default check.py dynasm 41/41 + cranelift 41/41.
Assisted-by: Claude
* FBW inline: decline callees with internal branches (non-leaf)
callee_fast_path_inlinable admitted any callee whose body had no
unsupported vable op, but the inline convention only resumes a guard
inside the callee at the caller's CALL boundary via the inherited
single-frame snapshot — sound only for straight-line leaves where
re-executing the whole call on deopt reproduces the state.
A callee with an internal conditional branch (goto_if_not / switch)
emits a branch guard whose fail snapshot must resume into the callee
mid-body. Under PYRE_FBW_INLINE+PYRE_FBW_LOADGLOBAL_FOLD the single-frame
model then serialised a resume section whose liveness shape disagreed
with the encoded stream: a folded branch operand numbered TAGINT in a
slot the outer liveness reported as a ref, crashing blackhole resume
with "resume.rs decode_ref: unexpected tag 1" (synth/function_calls,
mix's `if a & 1`). Decline such callees pre-flight so the call lowers to
an ordinary residual call (correct) until the multi-frame resume
coordinate is ported (#68).
Branchless leaves (inline_helper's add/mul/square/compute) stay inlined,
so inline_helper keeps parity (0.22s, 985375007). function_calls now
completes correctly (320103199572). Full flip-candidate sweep
(FULL_BODY_WALK+BRIDGE+INLINE+LOADGLOBAL_FOLD) dynasm 39/41, remaining 2
fails are perf-only (fib_recursive timeout, nbody 5.1x). Default gate
dynasm 41/41 + cranelift 41/41.
Assisted-by: Claude
* FBW: compile loop-free portal exits via TraceAction::Finish (gated)
full_body_walk_trace mapped every walk outcome except CloseLoop to
TraceAction::Abort, so a top-level *_return that reaches
done_with_this_frame (DispatchOutcome::Terminate, no back-edge) never
compiled: fib_recursive aborted and ran interpreted.
Under PYRE_FBW_CALL_ASSEMBLER the three top-level return arms route
through fbw_terminate_with_finish instead of ctx.finish(): re-box the
result to Type::Ref, record the vable store-back + GUARD_NOT_FORCED_2,
and stash the finish value in a thread-local. full_body_walk_trace
resets it at walk start and, on Terminate, builds TraceAction::Finish
from it, letting the compile pipeline record the FINISH -- the same
split the trait StepResult::Return path uses. Gate off leaves the
ctx.finish() + Terminate->Abort path byte-identical.
fib_recursive then compiles under PYRE_FULL_BODY_WALK (loops_compiled
0->1, loops_aborted 5->0) and matches trait output under MAJIT_STRICT.
Add the missing raw_descrs / is_authoritative_executor fields to the
drive_int_between test's WalkContext literal.
Assisted-by: Claude
* FBW: abort non-standard-virtualizable callee Finish portals to trait
A callee compiled as its own Finish portal (reached via
call_user_function_with_eval) can access its frame through a vable_* op
that finds it to be a non-standard virtualizable, emitting an internal
promote GuardValue + force store-back. Those internal ops carry a resume
snapshot / FieldDescr only on the inline sub-walk path; on the own-portal
compile path the optimizer's store_final_boxes_in_guard /
optimize_setfield_gc invariants trip.
walker_capture_inline_nonstandard_vable_guard now distinguishes the two:
the inline sub-walk still captures the snapshot, while an own-portal walk
that emitted such a guard returns NonStandardVableFinishPortalUnsupported,
aborting the trace to the trait interpreter (AbortPermanent). The method
runs interpreted until the own-portal callee frame is registered as the
standard virtualizable (a perf follow-up).
Exposed by the Finish-portal compile path (PYRE_FBW_CALL_ASSEMBLER):
synth/inheritance_dispatch's polymorphic value() methods previously
panicked in the optimizer; they now produce correct output. The full
flip-candidate sweep (FBW + all levers + CALL_ASSEMBLER) is 45/45
stdout-match, 0 panic; default check.py dynasm + cranelift 41/41.
Assisted-by: Claude
* FBW walker: guard unseeded vable array register; gate inline on call_fn oopspec
getarrayitem_vable / setarrayitem_vable / arraylen_vable read the vable Ref
register without the is_none() check the scalar getfield/setfield handlers
carry, so an unseeded OpRef::None (raw() == u32::MAX) flowed into the
metainterp vable path and resized the heapcache flag vector to ~16 GiB
instead of aborting. Add the VableBoxNotSeeded guard to all three plus a
unit test.
try_walker_inline_user_call inlined any _r_* residual whose 2nd ref arg is a
function, including store_subscr_fn (StoreSubscr oopspec): `d[f] = v` with a
1-arg function key f was mis-inlined as f(v), skipping the store. Gate the
inline on ei.oopspecindex == None, the only iRd-routed helper shape without
an oopspec (the genuine call_fn / call_fn_N).
Assisted-by: Claude
* FBW sub-descr pool: key cache by payload identity and retain the Arc
SUB_DESCR_POOL_CACHE keyed by code ptr only and cached 'static views of the
payload's exec.descrs without retaining the Arc<PyJitCode>. A merge-point
refinement that replaces the payload in place for the same code ptr left the
cache returning descrs derived from the superseded payload (or dangling if
the old Arc had no other holder). Key on (code, payload) identity and retain
Arc::clone(&pjc) in the entry so a refined payload yields a fresh, correct
pool and the borrowed slices stay valid.
Assisted-by: Claude
* optimizeopt: invalidate tracked vable array on variable-index setarrayitem
optimize_setarrayitem_gc only mirrored constant-index writes into
vstate.arrays; a non-constant-index SETARRAYITEM_GC left the tracked slots
unchanged, so a later constant-index GETARRAYITEM_GC could fold to a value
the variable-index write may have overwritten. Add
VirtualizableTracker::invalidate_array and call it on the variable-index
path, mirroring force_lazy_setarrayitem(can_cache=False). Add a regression
test alongside the read-after-write fold test.
Assisted-by: Claude
* review nits: doc link, must_use guard, probe void-defer reset
- history.rs: fix the intra-doc link to recorder::Trace (no Recorder type).
- jitdriver.rs: mark TraceContinuationSuspendGuard #[must_use] so a bare
enter() statement cannot construct-then-drop the suspend guard.
- trace.rs: reset the void-defer log and Finish payload on the diagnostic
probe_walk_perfn_jitcode path, which discards its trace via cut_trace but
never runs through the production tracer's entry reset.
Assisted-by: Claude
* EffectInfo: recognize FBW helper calls via pyre_helper, not oopspecindex
The full-body walker recognized the binary_op / compare / load_const /
box_int / store_subscr / load_global helper residual_calls by tagging their
EffectInfo with OopSpecIndex variants 200-205. has_oopspec() returns true
when oopspecindex != None, so production trait-dispatch traces carrying these
descrs were misclassified: the dynasm reghint _consider_real_call pass
skipped their real-call register hints, and the CanRaise members (load_const,
box_int, load_global) sat outside the _OS_CANRAISE set.
Add EffectInfo.pyre_helper: PyreHelperKind and move recognition onto it;
oopspecindex stays None for these helpers, so has_oopspec() and the OS_*
universe match origin/main. The CallDescrStub cache keys on the whole
EffectInfo and pyre_helper joins PartialEq/Hash, so distinct helper descrs
stay interned distinctly. Migrate the flatten.rs producers, the
jitcode_dispatch.rs walker consumers, and the try_walker_inline_user_call
inline gate; initialize pyre_helper: None in the front-end and metainterp EI
builders. Remove variants 200-205 from OopSpecIndex.
Also document BOOL_BOX_TRUTH as a pyre-only runtime reconstruction of the
jtransform optimize_goto_if_not fusion and note its reset boundary.
Assisted-by: Claude
* pyframe: document bridge-resume restore scope
restore_resume_state_from restores last_instr, valuestackdepth, and
locals_cells_stack_w — the only virtualizable fields the full-body walk
mutates on the live frame. Document that pycode / w_globals / debugdata are
intentionally not restored: frame-invariant or debug-only, never written on
the live frame during bridge tracing.
Assisted-by: Claude
* FBW: log walker abort reason under PYRE_FBW_DEBUG_ABORT
full_body_walk_trace mapped every DispatchError and non-loop-closing
DispatchOutcome to TraceAction::Abort/AbortPermanent without surfacing
which one; the metainterp abort log (pyjitpl/mod.rs:6348) reports only
the green key and permanence. Add a cached PYRE_FBW_DEBUG_ABORT gate that
prints the DispatchError variant, the non-loop-closing outcome, and the
ungated-portal-exit / run_perfn_walk-None cases at each Abort arm.
Default off, no production effect.
Assisted-by: Claude
* optimizeopt tests: adapt virtualizable regression tests to rebased BoxRef API
virtualize.rs regression tests (read-after-write fold, variable-index
invalidate): append `.to_opref()` to `get_box_replacement` (now returns
BoxRef) and pass the OpRc argument to `propagate_forward`.
Assisted-by: Claude
* jitcode_dispatch: handle ConcreteValue::Bool at two pc-map match sites
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at
two walker-only match sites, leaving them non-exhaustive:
- arraylen_vable_via_metainterp: a Bool concrete is not a vable struct
pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort).
- diagnose_inline_recognition: maps it to 'b' in the shape string.
Assisted-by: Claude
* FBW: direct CALL_ASSEMBLER for self-recursive call (dev-gated PYRE_FBW_REC_CA)
The full-body-walk residual_call dispatcher emitted a generic may-force
CallMayForceR for a self-recursive call, which re-enters the callee
through the func-entry residency door: one frame build + entry-bridge per
recursive call. fib_recursive ran ~30x slower than the trait tracer,
which instead records CallAssemblerR (direct assembler->assembler jump to
the callee's own loop/pending token).
Add try_walker_call_assembler_self_recursive, invoked from
dispatch_residual_call_iRd_kind after the inline path declines. It mirrors
the trait CALL_ASSEMBLER sequence: build the callee PyFrame inline via
emit_new_pyframe_inline_self_recursive (Branch A: single positional int
local, ncells==0, non-global-storing), then FORCE_TOKEN/vable setup,
call_assembler_red_only_ref to the resolved loop or pending token,
KEEPALIVE, escaped-frame heapcache invalidation, the result writeback
(Ref dst as-is, Int dst unboxed) BEFORE the guards — mirroring
make_result_of_lastop (pyjitpl.py:2077) and the in-tree sibling residual
path (write at jitcode_dispatch.rs:6856-6857, contract at 6844-6849) so a
raising/forcing deopt's after-call resume snapshot reads the recorded dst
OpRef instead of a stale box or NONE — then GUARD_NOT_FORCED and
GUARD_NO_EXCEPTION with after-call resume snapshots.
Every precondition (single int arg, exact self-recursion via
callee-code==portal-code, resolvable token, available portal sym) bails
with Ok(None) before recording any IR, so an unmet shape falls back to the
proven residual path. Gated by PYRE_FBW_REC_CA; default-off, production
trait path unaffected.
fib(30) FBW+REC_CA user-CPU at parity with trait (~0.12s); resolves the
sole fib_recursive flip blocker (was ~30x via the func-entry door).
Assisted-by: Claude
* FBW: reconcile walker with #158 base APIs after rebase
The rebased full-body-walk commits predate two APIs the #158/#390 base
added to pyre-jit-trace:
- `WalkContext.store_subscr_fn_addr`: the inline-call sub-walk
constructor in `try_walker_inline_user_call` propagates it from the
parent context, and the six `#[cfg(test)]` WalkContext fixtures that
the release build does not compile set it to `None`.
- `walker_capture_snapshot_for_last_guard` now returns
`Result<(), DispatchError>`; the base's `WalkerFrameOps::generate_guard`
for `WalkContext` calls it in a `()` context with no abort channel, so
the snapshot is best-effort there (the STORE_SUBSCR specialization that
reaches it is env-gated and dead in production).
Assisted-by: Claude
* FBW: record void residual calls symbolically; drop VOID_DEFER_LOG
try_execute_residual_call_via_executor declined Type::Void residual calls
by pushing them onto VOID_DEFER_LOG and committing enclosing-scope stores
at the primary loop close. The may-force-safe gate (from #158) declines
the active-vable CallMayForce* subset (STORE_SUBSCR) before reaching this
branch, so the void branch no longer fires across the bench suite.
Replace the void branch with a bare `return None` so the residual op is
recorded symbolically and the compiled loop applies the side effect once
per iteration -- matching the may-force-decline path and the trait path's
net effect. do_residual_call (pyjitpl.py:2040/2104/2123) executes void
calls eagerly during tracing, but the full-body walk is symbolic and does
not advance the interpreter, so the compiled loop re-runs the traced
iteration; eager execution there would double-apply the heap mutation.
Remove the dead VOID_DEFER_LOG, VoidDeferEvent,
void_defer_push_store/push_register/commit_at_close, and the
jit_merge_point register push and loop-close commit call sites. Rename
void_defer_reset to bool_box_truth_reset, its only remaining job.
Verified: FBW-all-levers output byte-identical to the trait path on all
44 benches (incl. nbody enclosing-scope store, fannkuch, list ops);
check.py dynasm 44/44 + cranelift 44/44; pyre-jit-trace 250 +
majit-metainterp 1365 unit tests pass.
Assisted-by: Claude
* FBW: object-storage list getitem arm in try_walker_specialize_subscr
try_walker_specialize_subscr handled only int/float list storage (sid 1/2),
declining object-storage reads to the generic CALL_MAY_FORCE residual. Add
the sid=0 arm, mirroring generated_list_getitem_by_strategy: guard_class LIST
+ guard_value(strategy==0) + unbox index + IntLt bounds guard (length via
list_length_descr) + getfield(list_items_descr) + getarrayitem_gc_r against
the Ptr(GcArray(OBJECTPTR)) items block, reading the element Ref directly.
The bounds-length descriptor is selected per strategy.
Under PYRE_FULL_BODY_WALK, nbody object reads (bodies[i]) now specialize,
which makes the loaded element concrete so its float-storage reads (b[k])
also specialize; the BINARY_SUBSCR CALL_MAY_FORCE residuals and the
associated GuardNoException / FORCE_TOKEN drop to zero.
Assisted-by: Claude
* FBW: source merge-point reds from sym.frame/execution_context
The walker jit_merge_point handler registered reds read from the walk
register file (ri/rr/rf), while the loop-close path rebuilds jump args
via close_loop_args_at, which reads sym.frame / sym.execution_context.
When a register slot held a const-folded alias (e.g. a ConstPtr for the
execution context), the registered green_boxes and the close-side reds
diverged in box identity. A cross-loop cut (cut_trace_from_with_consts)
then saw the close-side box as an escaped inputarg and appended it as an
extra inputarg, producing a loop entry layout that
extend_compiled_live_values could never satisfy — every interpreter
entry aborted with reason=extend-live-values.
Override the frame/ec slots of the registration reds with sym.frame and
sym.execution_context (the same source the close path uses) when the
red signature matches the portal shape (rr.len()==2, no ints/floats).
Also add a MAJIT_LOG-gated [jit][cut-escape] diagnostic in
cut_trace_from_with_consts when an original inputarg has no pool
constant and is appended as an extra inputarg.
nbody 50k under PYRE_FULL_BODY_WALK + all levers: guard failures
51181 -> 1427, loop entry aborts 263 -> 14, runtime 1.38s -> 0.59s,
output byte-identical to the trait tracer across all 18 benches.
Assisted-by: Claude
* FBW: emit loop_header at backward jumps; gate jit_merge_point on it
The per-CodeObject codewriter emitted only jit_merge_point at loop-header
PCs. Upstream lowers can_enter_jit at each backward-JUMP site to a
loop_header op (jtransform.py:1714-1723) and opimpl_jit_merge_point
(pyjitpl.py:1547-1562) fires reached_loop_header only when a preceding
loop_header stamped seen_loop_header_for_jdindex; a fall-through arrival
returns early unless the green key already has compiled targets.
Without the gate, the FBW walker treated every jit_merge_point execution
as a loop crossing. On nbody the first trace at the inner physics header
(pc=38) closed a degenerate "inner exhausted, outer increment, fresh
iterator" loop at its fall-through re-arrival, occupying the inner
loop's green key. The trait tracer (backward-jump-triggered
close_loop_args) instead cuts at the outer header, leaving the inner key
free to re-heat into a specialized retrace.
Changes:
- codewriter.rs: JumpBackward / JumpBackwardNoInterrupt arms emit a
loop_header graph op (Constant(jdindex)) before the goto when a portal
jitdriver is registered; new emit_loop_header helper records the graph
op and serializes …
youknowone
added a commit
that referenced
this pull request
Jul 1, 2026
Flip ProducedShortOp.res and ProducedShortOp.same_as_source from BoxRef to Operand, along with the coupled PreambleOp (AbstractShortOp) same_as_source field. res is always a producer-bound or const operand (materialize_operand_at on the preview; res.bound_op()-rooted exported entries per #173), never position-only. Retype the record/add channel to carry Operand: record_imported_preamble_use, record_preamble_use, add_tracked_preamble_op, and ShortPreambleBuilder::new's short_boxes parameter. Replace materialize_box_at with materialize_operand_at at the five ProducedShortOp construction sites in mod.rs. Contain at the principled BoxRef boundaries via to_boxref (Rc::ptr_eq-stable on the bound producer): exported_infos (#158), info::PreambleOp / ImportedShortPureOp (#182/#183), the position-only PreambleOp.res channel, and label_args: Vec<BoxRef>. Delete the optimizer same_as_source position-remap block: an Operand live-tracks its producer's already-remapped Op.pos through the carried Rc<Op>, so there is no separate position Cell to rewrite; the position-only else-branch was dead since #173 roots the producer. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 2, 2026
Flip ProducedShortOp.res and ProducedShortOp.same_as_source from BoxRef to Operand, along with the coupled PreambleOp (AbstractShortOp) same_as_source field. res is always a producer-bound or const operand (materialize_operand_at on the preview; res.bound_op()-rooted exported entries per #173), never position-only. Retype the record/add channel to carry Operand: record_imported_preamble_use, record_preamble_use, add_tracked_preamble_op, and ShortPreambleBuilder::new's short_boxes parameter. Replace materialize_box_at with materialize_operand_at at the five ProducedShortOp construction sites in mod.rs. Contain at the principled BoxRef boundaries via to_boxref (Rc::ptr_eq-stable on the bound producer): exported_infos (#158), info::PreambleOp / ImportedShortPureOp (#182/#183), the position-only PreambleOp.res channel, and label_args: Vec<BoxRef>. Delete the optimizer same_as_source position-remap block: an Operand live-tracks its producer's already-remapped Op.pos through the carried Rc<Op>, so there is no separate position Cell to rewrite; the position-only else-branch was dead since #173 roots the producer. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 2, 2026
Flip ProducedShortOp.res and ProducedShortOp.same_as_source from BoxRef to Operand, along with the coupled PreambleOp (AbstractShortOp) same_as_source field. res is always a producer-bound or const operand (materialize_operand_at on the preview; res.bound_op()-rooted exported entries per #173), never position-only. Retype the record/add channel to carry Operand: record_imported_preamble_use, record_preamble_use, add_tracked_preamble_op, and ShortPreambleBuilder::new's short_boxes parameter. Replace materialize_box_at with materialize_operand_at at the five ProducedShortOp construction sites in mod.rs. Contain at the principled BoxRef boundaries via to_boxref (Rc::ptr_eq-stable on the bound producer): exported_infos (#158), info::PreambleOp / ImportedShortPureOp (#182/#183), the position-only PreambleOp.res channel, and label_args: Vec<BoxRef>. Delete the optimizer same_as_source position-remap block: an Operand live-tracks its producer's already-remapped Op.pos through the carried Rc<Op>, so there is no separate position Cell to rewrite; the position-only else-branch was dead since #173 roots the producer. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 2, 2026
…xVar.var_box to Operand (#321) * optimizeopt: ProducedShortOp res/same_as_source to Operand Flip ProducedShortOp.res and ProducedShortOp.same_as_source from BoxRef to Operand, along with the coupled PreambleOp (AbstractShortOp) same_as_source field. res is always a producer-bound or const operand (materialize_operand_at on the preview; res.bound_op()-rooted exported entries per #173), never position-only. Retype the record/add channel to carry Operand: record_imported_preamble_use, record_preamble_use, add_tracked_preamble_op, and ShortPreambleBuilder::new's short_boxes parameter. Replace materialize_box_at with materialize_operand_at at the five ProducedShortOp construction sites in mod.rs. Contain at the principled BoxRef boundaries via to_boxref (Rc::ptr_eq-stable on the bound producer): exported_infos (#158), info::PreambleOp / ImportedShortPureOp (#182/#183), the position-only PreambleOp.res channel, and label_args: Vec<BoxRef>. Delete the optimizer same_as_source position-remap block: an Operand live-tracks its producer's already-remapped Op.pos through the carried Rc<Op>, so there is no separate position Cell to rewrite; the position-only else-branch was dead since #173 roots the producer. Assisted-by: Claude * optimizeopt: ImportedShortAlias.same_as_source to Operand Flip ImportedShortAlias.same_as_source from BoxRef to Operand. Every construction site is producer-bound: production reads op.arg(0) (already an Operand op-arg) and the three test mints use rooted_resop_operand, so the field never carries a position-only box. This deletes a round-trip that the BoxRef field forced: used_imported_short_aliases stored op.arg(0).to_boxref(), and emit_alias_same_as_for_imports rebuilt the SameAs arg via Operand::from_boxref(&same_as_source). With the field typed Operand both conversions are dropped — op.arg(0) is stored directly and cloned into the emitted op. The assert on .to_opref() is unchanged (Operand carries it). Assisted-by: Claude * optimizeopt: bind IndexVar.var_box + rename_op to producers Flip `IndexVar.var_box: Option<BoxRef>` to `Option<Operand>` and bind the dormant-vectorizer producer sites, draining the last E5b (#175) residual. dependency.rs: - `var_box` field + `new_boxed` param BoxRef -> Operand; `get_or_create` passes the bound `arg_box.clone()` instead of `arg_box.to_boxref()`. - `get_operations` carries `var` as an Operand directly: `first_var` and the chained references bind a synthetic producer via `Operand::bound_from_opref` (to_opref-identical) instead of `BoxRef::from_opref`, dropping the three `Operand::from_boxref(&var_box(..))` wraps. Removes the BoxRef import (dependency.rs is now BoxRef-free). The constant arg stays `from_opref` (a ConstInt, sheds inline). guard.rs `rename_op`: `Operand::from_opref(replacement)` -> `bound_from_opref`. The renamer replacement is a producer position, so `from_opref` would panic on it; bind a synthetic producer carrying the same pos, matching the sibling guard-strengthening sites. Both paths are vectorizer-only (vec_all default false), so check.py cannot exercise them; correct by construction, mirroring the landed `bound_from_opref` siblings. Gate: dynasm-lib 1370 / cranelift-lib 1368 / ir 333 / check.py 167/167 both backends. Assisted-by: Claude * optimizeopt: flip util::args_eq/args_hash to Operand `args_eq` / `args_hash` / `hash_arg` take `&[Option<Operand>]` instead of `&[Option<BoxRef>]` (util.py:100-122 parity port). `Operand::same_box` carries the same const-by-value / producer-by-identity semantics, and `hash_arg` keeps the value special-case (`Operand::Const` hashes by pointer, so equal fresh consts must hash via `const_value` to preserve the args_eq/args_hash contract). Tests use `Operand::const_` / `bound_from_opref`. Drops the last BoxRef use in util.rs (import removed). These helpers have no production caller (the optimizer's CSE compares via `same_box` directly); kept as a faithful port. Lib-authoritative: dynasm-lib 1370 / cranelift-lib 1368 / ir 333. Assisted-by: Claude * optimizeopt: short-box map keys via materialize_operand_at `produce_arg` and `materialize_one` resolved the produced_short_boxes / boxes_in_production key with `let key = ctx.materialize_box_at(x); let okey = Operand::from_boxref(&key)`. Collapse both to `let okey = ctx.materialize_operand_at(x)` — byte-identical (`materialize_operand_at` is `from_boxref(&materialize_box_at(..))` with the same mutation side-effect), dropping two `from_boxref` bridges and the intermediate BoxRef locals. Gate: dynasm-lib 1370 / cranelift-lib 1368 / check.py 168/168 both backends. Assisted-by: Claude * optimizeopt: ShortPreamble position fields to OpRef Flip ShortPreamble.{inputargs, used_boxes, jump_args, phase1_inputargs} from Vec<BoxRef>/Option<Vec<BoxRef>> to flat Vec<OpRef>. These are cross-phase position-domain channels whose payload is a bare producer or const position. Re-home walk_const_ptr_refs_mut to forward inline OpRef::ConstPtr GcRefs directly over &mut slices instead of BoxRef::walk_const_ptr_refs; the old path round-tripped the same GcRef through a BoxKind::Const cell, so forwarding is unchanged. Drop the from_opref re-mints at producers and the to_opref extraction at consumers across shortpreamble.rs / unroll.rs / optimizer.rs. The ExtendedShortPreambleBuilder.used_boxes field stays BoxRef, so re-mint the positions at that boundary. Assisted-by: Claude * optimizeopt: ShortPreamble builder position fields to OpRef Flip the ShortPreambleBuilder-feeding position channels from BoxRef to flat OpRef: - ShortPreambleBuilder.known_constants (VecSet) and .used_boxes; ExtendedShortPreambleBuilder.used_boxes. - OptContext.exported_short_inputargs and ExportedState.short_inputargs. - initialize_imported_short_preamble_builder[_from_short_boxes] short_inputargs parameter. Re-home the const GC walk to inline OpRef::ConstPtr forwarding (visit_oprefs / visit_opref_set over &mut). Retire the temporary re-mint bridge C1 left: used_boxes assigns short_preamble.used_boxes directly; the produce loop pushes produced.preamble_op.pos.get(). Assisted-by: Claude * optimizeopt: PreambleOp.res to Operand PreambleOp.res is identity-bearing at the export boundary — its value feeds ProducedShortOp.res through Operand::from_boxref, which panics on a position-only box, so production res is always producer-bound or const. Carry it as Operand (matching ProducedShortOp.res) rather than a flat OpRef, preserving the producer Rc identity. - Export reads (build_short_preamble_struct, optimizer preview-to-export) become entry.res.clone() / produced.res.clone(). - Drop the optimizer res position-remap block: a bound/const operand live-tracks its producer's already-remapped op.pos, like same_as_source. - Construction: ctx sites use materialize_operand_at; the ctx-less collector builder uses Operand::bound_from_opref. - GC walk routes through Operand::walk_const_ptr_refs (Const cell get/visit/set), consistent with the sibling same_as_source field. - Tests use rooted_resop_operand / Operand::from_boxref. Assisted-by: Claude * optimizeopt: ExportedState position fields to OpRef Flip the three pure position-domain ExportedState fields end_args, renamed_inputargs, and runtime_boxes from Vec<BoxRef> to flat Vec<OpRef>. Their readers already resolve via to_opref (trace inputarg reconstruction in pyjitpl, high-water scan, remap), so storing OpRef drops the constructor from_opref re-mints and the reader round-trips. Re-home the const GC walk to inline OpRef::ConstPtr forwarding (visit_oprefs over &mut); high-water and export remap read the position directly (remap_opref). next_iteration_args and the exported_infos keys stay BoxRef — they carry cross-peel ptr_eq identity and convert together in a later slice. Assisted-by: Claude
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.
Fix #122
Summary
dispatch_via_walker_for_opcodepops 3 values and delegates toMIFrame::store_subscr_value, bypassing the arm-jitcode walk that pops from a concrete PyFrame stack the walker never populates.try_execute_residual_call_via_executoradmitsCallMayForce*when no active virtualizable (matchesdo_residual_call's force-virtual branch with no vinfo).MIFrame::reraise, which seedserr.reraise_lastifromconcrete_stackso the dispatcher's lasti extraction works the same on either leg.dispatch_via_walker_for_opcodegains anop_argparameter.dispatch_arm_via_blackhole+production_blackhole_handles+METAINTERP_JITCODE_CACHEdeleted; the walker executor is now the sole non-elidable residual_call mechanism.build_pyre_production_bh_builder+blackhole_control_opcodesretained for the resume path.try_walker_direct_opcode_dispatchextracted as the named entry point for opcodes that emit IR directly from the tracer.Gate: dynasm 44/44 + cranelift 44/44 ×2 backends.
Self-review
Prompt & Model
Model:
Prompt:
Answer
No behavior-level PyPy parity regressions found versus origin/main.
The new early SubRaise returns in pyre/pyre-jit-trace/src/jitcode_dispatch.rs:4653 move Pyre closer to PyPy: handle_possible_exception() records GUARD_EXCEPTION
and immediately calls finishframe_exception() in rpython/jit/metainterp/pyjitpl.py:3404-3417, so later arm bytecode should not be walked.
No new behavior mismatch found.
Two new comments are misleading:
pyre/pyre-jit-trace/src/jitcode_dispatch.rs:3582 says residual-call success clears exception state at pyjitpl.py:1685-1690; upstream clears before execution in
MIFrame.execute_varargs() at pyjitpl.py:1941-1943, and in the forces branch at pyjitpl.py:2010.
The release-GIL comment in the residual-call inventory overstates direct_call_release_gil: upstream concrete execution happens first via
executor.execute_varargs(... CALL_MAY_FORCE_) at pyjitpl.py:2023-2041; direct_call_release_gil() later records CALL_RELEASE_GIL_ at pyjitpl.py:3700-3705.
pyre/pyre-jit-trace/src/jitcode_dispatch.rs:3493 still does not execute every residual call. It declines non-supported opcodes, non-const funcptrs, unpatched/
high fnaddrs, over-arity calls, missing concrete args, and NULL refs. PyPy executes residual calls at trace-record time in pyjitpl.py:1995-2126.
If that helper returns None, Pyre still does not mirror upstream clear_exception() for that residual call. The patch only clears on Some(Ok(_)).
CallMayForce* and CallReleaseGil* concrete execution remain excluded from the walker executor path, while PyPy’s forces branch concretely executes first.
Existing documented gaps remain: OS_JIT_FORCE_VIRTUAL, direct_libffi_call, full virtualizable/vref after-call handling, direct assembler-call handling, and
exact multi-frame guard snapshots.
pyre/pyre-interpreter/src/opcode_ops.rs:231 is a Rust ABI bridge, not a line-by-line PyPy shape. PyPy’s STORE_SUBSCR just performs space.setitem and returns
through interpreter control flow; Pyre needs a C-ABI callable that publishes exceptions through BH_LAST_EXC_VALUE.
The bridge returns 1/0, while the opcode-arm JitCode has a Ref-shaped StepResult::Continue return. This is acceptable only because production opcode-entry
dispatch treats any SubReturn as StepResult::Continue and ignores the returned Ref.
The BH_LAST_EXC_VALUE TLS, runtime fnaddr patching, and 47-bit fnaddr sanity gate are implementation-language/runtime adaptations, not RPython structures.
Single-frame walker snapshots are a deliberate Pyre adaptation because helper/sub-jitcodes are not blackhole-resumable Python frames.
Summary by CodeRabbit
Performance
Bug Fixes
Debugging
Other