jit: root the blackhole virtualizable_ptr slot, drop the re-entrant trace-too-long teardown, and give the exception-edge bridge its discarded-frame traceback nodes - #972
Conversation
WalkthroughThe PR adds traceback recording for discarded inline frames, separates nested ChangesJIT runtime control flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ExceptionEdge
participant JitcodeDispatch
participant TracebackHook
participant PythonException
ExceptionEdge->>JitcodeDispatch: publish discarded resume levels
JitcodeDispatch->>TracebackHook: record discarded levels
TracebackHook->>PythonException: attach traceback frames
sequenceDiagram
participant TraceWalker
participant QuasiImmut
participant AbortState
participant Blackhole
TraceWalker->>QuasiImmut: check invalidation baseline
QuasiImmut->>TraceWalker: report forced invalidation
TraceWalker->>AbortState: stage ABORT_FORCE_QUASIIMMUT
TraceWalker->>Blackhole: adopt forward resume
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
https://github.com/youknowone/pyre/blob/689a883d60febb1816857657fdf6f04d0e8d3490/pyre-object/src/quasiimmut.rs#L261-L262
When one thread is tracing while another mutates any watched module dictionary or type, this process-global counter advances and the tracing thread interprets the unrelated mutation as its own jit_force_quasi_immutable, aborting after its next opcode; sustained mutations can therefore prevent otherwise independent loops from compiling. The single TLS baseline also cannot represent nested same-thread traces: a nested trace resets it and can hide a force already performed by the outer residual call. Track the event on a stack/per-trace context tied to the walk that executed the write rather than using a process-wide odometer.
AGENTS.md reference: AGENTS.md:L194-L196
ℹ️ 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".
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 95e9437). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
689a883 to
5050873
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5050873852
ℹ️ 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".
| FBW_FORCED_QUASIIMMUT_BASE | ||
| .with(|c| pyre_object::quasiimmut::forced_quasi_immut_count() != c.get()) |
There was a problem hiding this comment.
When Python threads trace concurrently, this thread-local baseline is compared with FORCED_QUASI_IMMUTS, a process-global atomic incremented by every watcher sweep in quasiimmut.rs. A mutation of any watched module or type on thread B therefore makes thread A return ForceQuasiImmut at its next safe opcode even when A neither performed that write nor depends on that object; sustained cross-thread mutations can repeatedly abort and eventually retire otherwise valid hot traces. Attribute the event to the active trace/execution context instead of using the process-wide counter.
AGENTS.md reference: AGENTS.md:L157-L162
Useful? React with 👍 / 👎.
youknowone
changed the title
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/09c232b4ac432dc42b7865dcc774b4d87711c880/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs#L2345-L2349
When an inlined callee executes a non-pure residual before reaching its FOR_ITER (for example, mutates an object and then enters a for loop), fbw_abort_nested_unjournaled_residual now allows that earlier residual because the loop-bearing scan was removed, and this later consume-time check aborts only after the mutation has happened. The effect-count gate then cannot install the outer rewind leg, while the LoopBearingCalleeInlineUnsupported epilogue keeps the legacy replay for unjournaled effects, so the caller re-executes the callee and applies that mutation twice. Detect the reached loop before allowing preceding irreversible residuals, or provide a forward-resume path that does not replay them.
https://github.com/youknowone/pyre/blob/09c232b4ac432dc42b7865dcc774b4d87711c880/pyre-jit/src/call_jit.rs#L859-L865
When an exception-guard bridge unwinds through an inlined callee to an outer handler, this creates a fresh empty frame from only the code and globals rather than reconstructing the frame encoded in the guard resume data. The resulting traceback exposes a different tb_frame whose arguments, locals, cells, value stack, and back-reference are missing, so code inspecting exc.__traceback__.tb_frame.f_locals observes fabricated state. Preserve and materialize each discarded level's actual per-frame resume state instead of reducing it to (w_code, py_pc).
AGENTS.md reference: AGENTS.md:L24-L30
https://github.com/youknowone/pyre/blob/09c232b4ac432dc42b7865dcc774b4d87711c880/pyre-jit/src/call_jit.rs#L842-L848
For multi-frame exception bridges, each resume_coords entry is a next-instruction coordinate—the same function uses py_pc.saturating_sub(1) for exception-table lookup and set_last_instr_from_next_instr for the live frame—but this callback decodes py_pc directly and later records it as tb_lasti. Consequently discarded traceback nodes point at the instruction after the raising/calling opcode, producing incorrect line information; a bare RERAISE is also missed and gains a spurious traceback node. Convert the coordinate to the preceding opcode before both the reraise check and traceback recording.
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/src/jit/codewriter.rs (1)
4449-4456: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared "group by insn_idx" helper.
after_call_groups(Lines 4470-4485) duplicates the same linear-scan grouping pattern asgroups(Lines 4449-4456): iterate,.find(|(idx, _)| *idx == insn_idx), push-or-insert. Factor this into one helper, for examplefn group_by_insn_idx(pairs: impl Iterator<Item = (usize, usize)>) -> Vec<(usize, Vec<usize>)>, and call it twice. This removes the duplicated logic and keeps both call sites in sync if the grouping strategy changes later.♻️ Proposed helper extraction
+fn group_by_insn_idx(pairs: impl Iterator<Item = (usize, usize)>) -> Vec<(usize, Vec<usize>)> { + let mut groups: Vec<(usize, Vec<usize>)> = Vec::new(); + for (py_pc, insn_idx) in pairs { + if let Some(entry) = groups.iter_mut().find(|(idx, _)| *idx == insn_idx) { + entry.1.push(py_pc); + } else { + groups.push((insn_idx, vec![py_pc])); + } + } + groups +} + - let mut groups: Vec<(usize, Vec<usize>)> = Vec::new(); - for (py_pc, &insn_idx) in live_markers.iter().enumerate() { - if let Some(entry) = groups.iter_mut().find(|(idx, _)| *idx == insn_idx) { - entry.1.push(py_pc); - } else { - groups.push((insn_idx, vec![py_pc])); - } - } + let groups = group_by_insn_idx( + live_markers.iter().enumerate().map(|(py_pc, &insn_idx)| (py_pc, insn_idx)), + );- let mut after_call_groups: Vec<(usize, Vec<usize>)> = Vec::new(); - for (py_pc, anchor) in after_call_post_merge.iter().enumerate() { - let Some(insn_idx) = *anchor else { continue }; - if per_pc_markers.contains(&insn_idx) { - continue; - } - if let Some(entry) = after_call_groups - .iter_mut() - .find(|(idx, _)| *idx == insn_idx) - { - entry.1.push(py_pc); - } else { - after_call_groups.push((insn_idx, vec![py_pc])); - } - } + let after_call_groups = group_by_insn_idx( + after_call_post_merge + .iter() + .enumerate() + .filter_map(|(py_pc, anchor)| anchor.map(|insn_idx| (py_pc, insn_idx))) + .filter(|(_, insn_idx)| !per_pc_markers.contains(insn_idx)), + );Also applies to: 4468-4485
🤖 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/jit/codewriter.rs` around lines 4449 - 4456, Extract the duplicated insn_idx grouping logic into a shared helper near the grouping code, such as group_by_insn_idx accepting an iterator of (usize, usize) pairs and returning Vec<(usize, Vec<usize>)>. Replace both the groups construction and after_call_groups construction with calls to this helper, preserving their existing input ordering and output 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-trace/src/state.rs`:
- Around line 4456-4465: In the trace setup around
current_quasiimmut_field_value and ensure_quasi_immut_installed, invoke
ensure_quasi_immut_installed(ctx, obj, field_index) before reading the
quasi-immutable value. Preserve the existing calls and maintain strict
line-by-line structural parity with interpreter semantics.
In `@pyre/pyre-jit-trace/src/trace.rs`:
- Around line 3721-3745: Update the blackhole adoption failure handling around
try_adopt_blackhole and try_adopt_single_frame_blackhole so an effectful
ForceQuasiImmut adoption failure is treated as fatal, matching the existing
TraceTooLong behavior. Apply this consistently to both single-frame and
multi-frame adoption paths, preventing an uncommitted walk from falling back to
legacy replay after the force-causing residual has executed.
In `@pyre/pyre-jit/src/call_jit.rs`:
- Around line 826-849: Update record_discarded_level_traceback to derive the
current-bytecode coordinate from the full-frame py_pc before decoding
RaiseVarargs/Reraise, updating frame.last_instr, or calculating source-line
information. Use that adjusted coordinate for bare-reraise detection and
frame/source-line accounting, while retaining the original full-frame resume
coordinate where record_application_traceback requires it.
---
Outside diff comments:
In `@pyre/pyre-jit/src/jit/codewriter.rs`:
- Around line 4449-4456: Extract the duplicated insn_idx grouping logic into a
shared helper near the grouping code, such as group_by_insn_idx accepting an
iterator of (usize, usize) pairs and returning Vec<(usize, Vec<usize>)>. Replace
both the groups construction and after_call_groups construction with calls to
this helper, preserving their existing input ordering and output 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 Plus
Run ID: dd196fd4-1563-4d02-a515-b71bb7a22119
📒 Files selected for processing (25)
majit/majit-metainterp/src/blackhole.rsmajit/majit-metainterp/src/lib.rsmajit/majit-metainterp/src/pyjitpl.rspyre/bench/synth/exception_reraise_tb_depth_jitstress.cranelift.jitstatspyre/bench/synth/exception_reraise_tb_depth_jitstress.dynasm.jitstatspyre/bench/synth/exception_reraise_tb_depth_jitstress.wasm.jitstatspyre/bench/synth/inline_subwalk_user_iterator.cranelift.jitstatspyre/bench/synth/inline_subwalk_user_iterator.dynasm.jitstatspyre/bench/synth/str_search_index_bounds.wasm.jitstatspyre/check.pypyre/pyre-interpreter/src/pyopcode.rspyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace.rspyre/pyre-jit/src/call_jit.rspyre/pyre-jit/src/eval.rspyre/pyre-jit/src/jit/codewriter.rspyre/pyre-object/src/celldict.rspyre/pyre-object/src/dictmultiobject.rspyre/pyre-object/src/quasiimmut.rspyre/pyre-object/src/typeobject.rs
…indow `BlackholeInterpreter::run` roots every chain level's register bank and, for a level with a `virtualizable_info`, the array-field slots inside its virtualizable. `virtualizable_ptr` itself was neither: it is a bare copy of the frame red the level was bound to (`call_jit.rs` binds it from `registers_r[portal_red]`), so a collection forwarded the register and left the copy naming the pre-move address. A virtualizable in the nursery makes that visible. A compiled trace allocates an inlined callee's `PyFrame` through its own `NewWithVtable`, which the GC rewriter lowers to a nursery allocation, so a minor collection inside `run_inner` relocates it. The propagation loop then hands the stale pointer to `record_application_traceback`, which stores it into `PyTraceback.frame` — a slot `pytraceback_object_custom_trace` forwards whenever `try_gc_owns_object` holds, and that predicate is a plain address-range test (`is_valid_gc_object && (nursery.contains || oldgen.contains)`), so the vacated block passes and the next minor collection reads a data word as a type id. Register the slot instead of the value, the shape `blackhole_from_resumedata` already applies to the resume reader's own `virtualizable_ptr` for the chain-build window. `cargo test --all --no-default-features --features dynasm` and `pyre/check.py` (dynasm 357/357, cranelift 357/357, wasm 353/353) are green, and the 342-fixture synth corpus is byte-identical before and after. Assisted-by: Claude
…ady committed `LoopBearingCalleeInlineUnsupported` is handled by two independent blocks in the walk epilogue: the carrier block, which can take `CalleeRebuild` (resume INSIDE the rebuilt callee, past what it applied), and the `FBW_ABORT_OUTER_RESUME` block, which rewinds the outer frame to its CALL and re-executes it. Nothing kept the two apart — the second block's gates are inclusion tests on `fbw_executed_nonpure_residual` / `fbw_has_unjournaled_effect`, and `walk_end_resume_provable` samples `FBW_EXECUTED_EFFECT_COUNT`, which the rebuilt callee's plain interpretation never bumps. When both fire the callee body runs twice. Read `WALK_END_FLUSH_COMMITTED` first and reset the latch instead. Same `MidBodyDecline::AfterRun` argument the carrier block already applies to its own entry-carrier fallback. Assisted-by: Claude
`filter_liveness_in_place` applied the LV∩SSA retain — keep only the pcdep frame-slot colors plus the portal reds — to the per-PC `-live-` markers only. The after-residual-call markers named by `after_call_post_merge` kept the raw SSA-live set, which includes Ref colors no trace-time writer populates. Group those markers alongside the per-PC ones and run them through the same narrowing, adding the preceding call's own Ref result register to the retained set (`get_list_of_active_boxes` names it at `ord(self.bytecode[self.pc - 1])`, pyjitpl.py:186). `marker_pcdep` publication stays per-PC-marker-only. `original_markers` is keyed by insn index instead of py_pc so both marker classes read their own pre-mutation snapshot. Measured over pyre/bench/** (404 scripts, dynasm): declines at `collect_callee_active_boxes` 5 -> 1; `fbw_abort_nested_residual` denies unchanged at 16. check.py: dynasm 358/358, cranelift 358/358; wasm keeps the two `exception_reraise_tb_depth_*` jit-stats failures that reproduce unchanged at the branch base. Assisted-by: Claude
`blackhole_if_trace_too_long` runs in the tracer's own stepping loop (`pyjitpl.py:2861-2867 _interpret`, which drives `framestack[-1].run_one_step()`); `pyframe.py dispatch_bytecode` has no such call. Pyre's tracer is the walker and its walk loop already runs the check, so the copy in `eval_loop_jit`'s opcode dispatch was a second caller of the same teardown. That copy tore down `MetaInterp.tracing` from a re-entrant interpreter run: a residual call executed inside an inline sub-walk runs Python through `eval_loop_jit`, whose per-step check read the OUTER trace's op count, found it over the limit, and moved that `TraceCtx` out of the shared slot via `abort_trace_live`. The in-flight walk then kept recording through a `&mut TraceCtx` whose recorder buffer had been freed. `bench/synth/trace_too_long_inline_multiframe.py` aborts in libmalloc under `PYRE_FBW_NESTED_RESID_ABORT`-equivalent conditions; after this change it exits 0 and matches `PYRE_NO_JIT=1` byte for byte. RPython cannot reach the same state: `warmstate.py:437-441 bound_reached` builds a fresh `MetaInterp` per trace attempt, so a nested JIT entry never touches the outer attempt's history. Assisted-by: Claude
… resumes past
`route_exc_edge` (`call_jit.rs`) takes a raise that unwinds clear out of every
inlined callee into the live frame's own handler, and re-points the live frame at
that handler. Its own comment states what that costs: "that unwind discards the
callee frames outright, so there is no inlined framestack left to rebuild". The
walk that follows starts flat, so the only node it records is the catching
frame's.
`pyopcode.py:148 pytraceback.record_application_traceback` runs BEFORE the `:152`
exception-table lookup, so a frame the unwind only passes through contributes a
node exactly like the one that catches. Upstream gets that for free: it resumes
onto a rebuilt MIFrame stack and the unwind is traced code running each level's
own recorder. Pyre synthesizes that loop, and this route synthesized it for one
level only.
`resume_coords[1..]` is exactly the set of discarded levels, so publish it at the
routing point and emit one node per level at the handler entry, innermost-first —
both recorders prepend, so emission order is the chain read outermost-first. The
coordinate the resume data carries is a PYTHON pc, not the jitcode pc the two
existing recorders translate from, hence the third hook arity;
`record_discarded_level_traceback` fabricates the node's frame from the code
object as `record_inline_traceback_for_recording` does, the level's own `PyFrame`
having stayed virtual in the compiled trace. Emitted as IR because
`trace_and_compile_from_bridge` runs once and every later failure of this class
enters the compiled bridge directly.
Latent behind the `CalleeReplaySafety::DeferredCall` arm, which residualizes the
intermediate call instead of inlining it. With that arm forced off,
`bench/synth/gc_bug_bridge_flavor_traceback_names.py` printed
`('T', 'a_bridge_two_classes', 'leaf_two')` alongside the correct shape — the
`mid_two` frame dropped — and now matches `PYRE_NO_JIT=1`; a three-level variant
(`driver`/`deep_a`/`deep_b`/`deep_c`/`leaf`) matches too.
`cargo test --all --no-default-features --features dynasm`: 101 binaries, 0
failed. `pyre/check.py` dynasm 15 / cranelift 15 / wasm 9 failed — keyed on
(fixture, backend, reason) that set adds NOTHING to `origin/main`'s own 41 and
drops the two `exception_reraise_tb_depth_jitstress` entries this branch fixes.
Assisted-by: Claude
`quasiimmut.py:124-125 QuasiImmutDescr.__init__` calls `get_current_qmut_instance` first and `get_current_constant_fieldvalue` second. `record_quasiimmut_field` had the two in the opposite order, so a write landing between them moved the field with no watcher installed: nothing invalidated and nothing bumped the force counter, and the trace kept a value that was already stale. Without a GIL that window is a real interleaving. Assisted-by: Claude
`record_discarded_level_traceback` received `py_pc` straight out of `resume_coords`, which is a `next_instr`-style coordinate — the same one `exc_table_offset` converts with `saturating_sub(1)` and the live frame converts with `set_last_instr_from_next_instr`. All three consumers below it want the instruction that RAN: `decode_instruction_at` for the bare-reraise test, `frame.last_instr`, and `record_application_traceback`'s `tb_lasti`. So the node named the instruction AFTER the raising or calling opcode: the traceback line was one instruction late, and a bare `RERAISE` decoded as whatever follows it and gained a node the `RaiseWithExplicitTraceback` rule says it must not have. Convert once at entry and use that for all three. Assisted-by: Claude
`filter_liveness_in_place` grew a second copy of the "push onto the entry with this `insn_idx`, or start one" loop when the after-residual-call markers began being narrowed alongside the per-PC ones. Extract `group_py_pcs_by_insn` and call it twice; the after-call site's skip of a marker already folded onto a per-PC group becomes a `filter_map` on the input iterator. Assisted-by: Claude
09c232b to
95e9437
Compare
youknowone
changed the title
|
Review dispositions after the rebase onto Fixed
Confirmed, and it is pre-existing — filed, not fixed here
Moot after the rebase
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/95e9437b591a45fd7aace1bf23611a10977d9c55/pyre-jit/src/call_jit.rs#L868-L872
When a discarded inlined callee has free variables, passing None as outer_func makes createframe_obj return the documented “directly executed code object may not contain free variables” error; the else { return; } then silently omits that callee's traceback node, recreating the missing-frame bug this path is intended to fix. More generally, a newly initialized frame also loses the discarded frame's locals. Preserve or materialize the per-frame resume state rather than reconstructing from only (w_code, py_pc).
AGENTS.md reference: AGENTS.md:L24-L41
https://github.com/youknowone/pyre/blob/95e9437b591a45fd7aace1bf23611a10977d9c55/pyre-jit-trace/src/jitcode_dispatch/mod.rs#L600-L605
When this exception-edge walk enters the handler but later aborts or declines, this concrete callback has already prepended every discarded-frame node to the live exception. Unlike record_bridge_handler_entry_traceback, these mutations are never added to FBW_TRACEBACK_STORE_JOURNAL, so rollback removes only the catching-frame node; blackhole replay then records the discarded frames again and exposes duplicate traceback entries. Journal each concrete attachment, or avoid applying it until the walk commits.
ℹ️ 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".
1 participant
Eight commits on top of
main. Each is independently reproducible.1.
virtualizable_ptrwas a bare copy across a collectionBlackholeInterpreter::runroots every chain level's register bank and, for alevel with a
virtualizable_info, the array-field slots inside itsvirtualizable.
virtualizable_ptritself was neither — it is a bare copy of theframe red the level was bound to, so a collection forwarded the register and left
the copy naming the pre-move address. A compiled trace allocates an inlined
callee's
PyFramethrough its ownNewWithVtable, which the GC rewriter lowersto a nursery allocation, so a minor collection inside
run_innerrelocates it;the propagation loop then stores the stale pointer into
PyTraceback.frame,whose custom trace forwards it on the next collection and reads a data word as a
type id. Register the slot, the shape
blackhole_from_resumedataalready appliesfor the chain-build window.
2. Two epilogue blocks both handled the same abort, running the callee twice
LoopBearingCalleeInlineUnsupportedis handled by the carrier block (which cantake
CalleeRebuild, resuming INSIDE the rebuilt callee) and by theFBW_ABORT_OUTER_RESUMEblock (which rewinds the outer frame to its CALL andre-executes it). Nothing kept them apart: the second block's gates are inclusion
tests on
fbw_executed_nonpure_residual/fbw_has_unjournaled_effect, andwalk_end_resume_provablesamplesFBW_EXECUTED_EFFECT_COUNT, which the rebuiltcallee's plain interpretation never bumps. Read
WALK_END_FLUSH_COMMITTEDfirstand reset the latch.
3. The after-residual-call
-live-markers were never narrowedget_list_of_active_boxesreads the marker AFTER the call wheneverin_a_callor
after_residual_callholds and the one before the op otherwise(
pyjitpl.py:194-198);compute_livenessis one uniform pass over every-live-. Group the after-call markers alongside the per-PC ones so both markerfamilies go through the same pass, adding the preceding call's own Ref result
register (
pyjitpl.py:186). Declines atcollect_callee_active_boxes5 → 1 overpyre/bench/**.4. A re-entrant interpreter run tore down the outer trace
blackhole_if_trace_too_longruns in the tracer's own stepping loop(
pyjitpl.py:2861-2867 _interpret);pyframe.py dispatch_bytecodehas no suchcall. Pyre's walk loop already runs the check, so the copy in
eval_loop_jit'sopcode dispatch was a second caller of the same teardown — and a residual call
executed inside an inline sub-walk runs Python through
eval_loop_jit, whoseper-step check read the OUTER trace's op count and moved that
TraceCtxout ofthe shared slot. The in-flight walk then kept recording through a
&mut TraceCtxwhose recorder buffer had been freed. RPython cannot reach this:
warmstate.py:437-441 bound_reachedbuilds a freshMetaInterpper trace attempt.5. The exception-edge bridge emitted one traceback node for a whole unwind
route_exc_edgere-points the live frame at its own handler and discards theinlined callee frames outright, so the flat walk that follows records only the
catching frame's node.
pyopcode.py:148 record_application_tracebackruns BEFOREthe
:152exception-table lookup, so a frame the unwind only passes throughcontributes a node exactly like the one that catches.
resume_coords[1..]isexactly the discarded set: publish it at the routing point and emit one node per
level at the handler entry, innermost-first.
6. The quasi-immut watcher was installed after the value was read
quasiimmut.py:124-126ordersself.qmut = get_current_qmut_instance(...)beforeself.constantfieldbox = self.get_current_constant_fieldvalue(). Reading firstleaves a window in which the field moves with no watcher installed, so nothing
invalidates, nothing bumps the force counter, and the trace keeps a value that is
already stale. Without a GIL that window is a real interleaving. (Ordering fix on
top of #977's install.)
7. The discarded level's traceback pc was off by one instruction
record_discarded_level_tracebackreceivedpy_pcstraight out ofresume_coords, anext_instr-style coordinate — the same oneexc_table_offsetconverts with
saturating_sub(1)and the live frame converts withset_last_instr_from_next_instr. Its three consumers all want the instructionthat RAN, so nodes named the instruction AFTER the raising or calling opcode and a
bare
RERAISEdecoded as whatever follows it, gaining a node theRaiseWithExplicitTracebackrule forbids. (Raised by both reviewers on this PR.)8. One grouping helper for both marker families
filter_liveness_in_placehad grown a second copy of the "push onto the entrywith this
insn_idx, or start one" loop. Extractgroup_py_pcs_by_insn.(Raised by CodeRabbit on this PR.)
Withdrawn: the FOR_ITER framestack-scan deletion
The earlier version replaced
fbw_inline_callee_hazardous' loop-bearingframestack scan with a screen at the
for_iter_nextconsume, on the measurementthat "the deleted scan claimed no abort the other two arms do not" — declines at
that site 16 → 14, synth corpus byte-identical.
That measurement counted declines at the fbw site, not the aborts the admitted
callee goes on to cause, and it held only because the
-live-Ref-bank retainnarrowed away the colors that make
collect_callee_active_boxesdecline.Once #973 deleted that retain, the same change takes
synth/inline_subwalk_user_iteratorfromloops_aborted=1(main's recordedbaseline, which main still meets) to 5, and panics on wasm with
blackhole recursive_call: jitdrivers_sd[0] carries no portal runner. Measuredby reverting
fbw_state.rs/residual_call.rsto main's version: 5 → 1.foriter_exempt_shared_generator, the fixture that change existed to fix, passeson main's version either way — the framestack scan masks that bug too. So the
commit, its repair, and the baseline re-record that recorded all three of their
consequences are withdrawn rather than re-baselined.
Status
cargo fmt --all -- --checkclean;cargo test --all --no-default-features --features dynasmgreen (101 suites).All six failures are inherited from
main, verified locally by reverse-applyingthis branch's whole diff and re-running each fixture on the resulting tree — the
numbers are identical, not merely the fixture names:
synth/exception_args_virtual(all 3 backends)loops_aborted 0 -> 3,guard_failures 401 -> 1002synth/list_length_hint_validate(dynasm, cranelift)loops_aborted 14 -> 34,guard_failures 828 -> 4923synth/pickle_terminal_raise_resume(wasm)loops_aborted 54 -> 59