Blackhole resume image: sharpen the roll-back gate, latch on bridge walks, and close the two decline families - #1064
Conversation
|
Important Review skippedToo many files! This PR contains 110 files, which is 10 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (110)
You can disable this status message by setting the 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 |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 4d07c85). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
|
Rebased onto Full Zero jit-stats rows moved on any backend. The two reds are both inherited from 1. 2. -# pyre-check: max-pypy-ratio=336
+# pyre-check: max-pypy-ratio=36
+# pyre-check: min-pypy-ratio=4.15The gate moved 9.3x, in a "re-record max-pypy-ratio from four accumulated suite The local number is also not adjudicable: the host was at load average 31–32 with — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64fdc8d14b
ℹ️ 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".
| // same breath, so `journaled <= effects` holds for every walk. | ||
| let effects = crate::jitcode_dispatch::fbw_executed_effect_count(); | ||
| debug_assert!(journaled <= effects); | ||
| let unrecoverable = effects.saturating_sub(journaled); |
There was a problem hiding this comment.
Account for residual bumps on journaled appends
When a LIST_APPEND falls through to the generic residual executor, this subtraction only removes the journal push from effects, but that same append already bumped the odometer once as a non-pure Void residual before fbw_append_journal_push bumps it again. On an uncommitted walk where rollback rewinds the append, effects - journaled is still 1, so fbw_rolled_back_with_effects is reported even though no irreversible effect remains, forcing affected jitstats baselines to fail/re-record spuriously.
Useful? React with 👍 / 👎.
|
Rebased onto New since the last push:
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/01823aae921f5342b3613e615d2ba7577f1b5eaf/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs#L6354
Clear helper transparency when entering Python callees
When a root-level canonical helper reaches a CallFn for a real Python function, this flag is copied unchanged into that function's WalkContext by inline_call.rs:4141-4145. Consequently, walker_capture_multi_frame_inline_snapshot takes the transparent-helper branch at resume_snapshot.rs:2104 and serializes only parent_frames, omitting the live Python callee; the post-step trace-limit/latch logic is skipped for it as well. A guard failure or abort inside such a callback can therefore resume without that callee's own jitcode, globals, and locals. Keep the flag on generated helper contexts, but clear it at the boundary that constructs a real Python callee frame.
AGENTS.md reference: AGENTS.md:L32-L42
ℹ️ 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".
|
Three more commits, all in the same mechanism as §8: the walk vstack mirror's
After these, the whole 373-fixture synth corpus produces one blackhole image-build decline. Still open — deliberately not "fixed" by widening the predicateThat last one is
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44eb8f4820
ℹ️ 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".
| // image and fell back to entry replay — the outcome the | ||
| // transparent-helper exclusions at the abort-coordinate claim | ||
| // and the post-step trace-limit check exist to prevent. | ||
| transparent_helper_subwalk: true, |
There was a problem hiding this comment.
Preserve the helper coordinate when propagating aborts
When a canonical helper aborts after a partial effectful descent, this flag prevents the helper walk from claiming the abort and bubbles its DispatchError to the enclosing Python walk. That walk then calls latch_abort_blackhole(ctx, error.stop_pc(), ...), but stop_pc() is still an offset in the helper JitCode while ctx and the resulting MIFrame belong to the outer JitCode; if that numeric offset happens to have valid outer liveness, adoption resumes the outer frame at an unrelated instruction and can skip or repeat effects. This is reachable through the explicitly retained effectful OrthodoxSubWalkTraceUnsupported arm, so the propagated abort must retain the helper's own frame/JitCode coordinate rather than being latched against the caller.
AGENTS.md reference: AGENTS.md:L32-L42
Useful? React with 👍 / 👎.
|
Follow-up: the one blackhole-image decline left open at the end of the previous round is closed, and the RCA recorded for it was wrong.
The codewriter already states this exact hazard next to Fix — two halves, both needed (either alone still arms the region):
Result: Verification — one full
One trap worth flagging for anyone reading the diag output: the fixture a decline is attributed to need not be the code that declines. The offender here was — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afaf7346bd
ℹ️ 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".
| .unwrap_or(0) as usize | ||
| }; | ||
| let handler_depth = if twin_populated { | ||
| let handler_py = handler_entry_py_pc(code_ptr, ctx.vstack_cur_pypc, floor_py); |
There was a problem hiding this comment.
Derive the handler from the actual raising coordinate
When the outer mirror has already become invalid, this function deliberately revives it, but vstack_cur_pypc is frozen at the last successfully reconciled opcode because step_vstack_mirror returns immediately while invalid. With nested exception regions, that stale opcode can belong to an outer handler while the actual raise is caught by an inner one, so this lookup selects the outer handler instead of the handler identified by handler_jit_pc; the mirror is then seeded with the wrong Python coordinate and stack depth, corrupting later guard snapshots. Pass the actual raising Python coordinate from the catch-routing code, or validate the exception-table target against handler_jit_pc, rather than consulting the stale mirror coordinate.
AGENTS.md reference: AGENTS.md:L14-L19
Useful? React with 👍 / 👎.
|
Acting on the two results that came back after the last push: the Codex parity review, and the red Codex review — §1 reclassified, §2 empty, §3 split into a fix and a deferral§1
Measured rather than argued: a probe at the gate over the 373 synth fixtures shows exactly one fixture reaches the exemption at all, ending at 118 ops against a limit of 100. Committed as the comment at the gate plus a regression test that walks one The remaining gap — pyre cannot resume a blackhole at a helper pc, where PyPy's blackhole interpreter runs any jitcode — is the structural adaptation the comment there already stated. §3 Fixed: Deferred, with the blocker named: §2 was empty. §4's three structural adaptations each already carry their citation in code ( The red ubuntu leg is a clamped denominator, and main is red the same way
The same fixture across runs of the same leg:
The numerator is flat and matches main's; the denominator halved onto the timer floor and the ratio doubled. Three more facts put it in the base's column:
Not re-recorded and the gate not raised: nothing real would be blessed and the next run picks a different victim. The only fix that restores a measurement is scaling those fixtures' N until pypy leaves the floor, which is a suite-wide change with its own trade-off (for some fixtures the true ratio is worse than the noisy one) — filed rather than folded in here. Verification
Full
No — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5073a3b786
ℹ️ 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".
| // of the same arm. Without it the two above ask for the loop to be | ||
| // re-traced and force-finished while its callers may still inline it, | ||
| // so the next attempt can rebuild the very trace that overflowed. | ||
| warm_state.disable_noninlinable_function(merge_key); |
There was a problem hiding this comment.
Select the oversized callee before disabling inlining
When an overlong full-body trace is caused by an inlined callee, this always applies DONT_TRACE_HERE to the root merge key while leaving the oversized callee eligible for inlining, so callers stop inlining the wrong function and the next root trace can reproduce the same overflow. Upstream first calls find_biggest_function() and disables that callee; it invokes prepare_trace_segmenting()—including this root-key operation—only when no inlined culprit exists (rpython/jit/metainterp/pyjitpl.py:2817-2846). Preserve that branch here rather than unconditionally treating every walker overflow as a root-sized trace.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
…ack gate `fbw_diag::ROLLED_BACK_WITH_EFFECTS` gated on `!committed && effects > 0`. `effects` is the gh#467 executed-effect odometer, and three of its four bump sites are journal pushes -- `fbw_store_journal_push`, `fbw_append_journal_push` and `fbw_cell_store_journal_push`. `fbw_store_journal_rollback` undoes all three on the same non-committed exit, before the epilogue reads the odometer, and never decrements it, so those walks were counted as irreversible. A fixture whose ten uncommitted walks carry only journaled effects reported 10; it now reports 0, with the same ten walks still in the census. The epilogue reads the three journal lengths where it read the store journal's and gates on `effects - journaled`. The odometer is written in two places, `fbw_state.rs` `set(0)` and the bump, and the `set(0)` sits inside `fbw_store_journal_reset`, which clears all three journals in the same function; `journaled <= effects` therefore holds for every walk and a `debug_assert!` pins it. `journal=` in the `[fbw-census]` line was the store journal's length alone, so an append-journal effect printed `effects=1 journal=0`, indistinguishable from a residual effect. The line prints `jstore=`, `jappend=`, `jcell=` and `unrecoverable=`. The wasm ring keeps one summed `journaled` field, width unchanged. The store-journal rollback can fail to restore a slot when a later residual shrank the list. That was reported only under the debug gate and counted nowhere, so the subtraction would have absorbed it. It now bumps `fbw_diag::STORE_JOURNAL_ROLLBACK_FAILED`, slot 11, moving `RING_BASE` to 12 in the producer and in the wasm runner's decoder. Both runners print it as `fbw_store_journal_rollback_failed` and it joins `JITSTATS_BADNESS_FIELDS`. Five of the six fixtures carrying a nonzero baseline keep their value: set_hash_protocol and raise_reg_unbound_jitstress at `unrecoverable=7`, recursive_forced_frame_kept_stack at `unrecoverable=3`, global_store_plain_dict_globals and pickle_terminal_raise_resume unchanged. `ca_bridge_multiframe_resume_double_call` on wasm falls 1 -> 0: its only uncommitted walk is `end=LoopBearingCalleeInlineUnsupport effects=1 journaled=1`, on a run reporting `fbw_store_journal_rollback_failed=0`. Assisted-by: Claude
…k too `residual_call.rs`'s single-frame blackhole latch carried `!ctx.trace_ctx.is_bridge_trace` as one of seven conjuncts from a default-off rollout gate. It was the only latch producer a bridge vable escape had: the abort-path `latch_abort_blackhole` is gated on `error.leaves_complete_image()`, whose variant list does not contain `VableEscapedDuringResidualCall`, so a bridge escape reached the epilogue with no image and its caller replayed the region. The comment directly above the conjunct already argues that upstream has no counterpart to either of its siblings -- ABORT_ESCAPE goes to `run_blackhole_interp_to_cancel_tracing` and runs forward, never replays. Two of the six recorded instances of the roll-back-with-effects population go to zero on all three backends. `set_hash_protocol`'s walk moves from `end=CompileTracePending committed=false leg=0 bridge=true effects=7` to `committed=true leg=1`, so those seven effects are applied once instead of twice. `recursive_forced_frame_kept_stack` moves the same way; its walk is `bridge=false` and the change reaches it through the bridge walks in the same run, which now adopt. Two fixtures for a shape the corpus did not contain: of 138 forced vable escapes across the synth corpus, none were `bridge=true`. A hot loop whose rare arm guard-fails past the trace-eagerness threshold earns a bridge, and `sys._getframe(0)` inside the bridge body forces the virtualizable without entering a Python frame. `getframe_bridge_force_plain` adopts 20 single-frame images; `getframe_bridge_force_after_store` puts an un-journaled store ahead of the forcing call, which leaves the escape's mirror slot unresolved, and records `fbw_rolled_back_with_effects=20` on the replay path. Neither fixture pinned anything without a counter: no gated field moved between the two arms, because nothing counted a successful adoption. `fbw_diag` slots 12 and 13 tally single- and multi-frame blackhole adoptions -- `RING_BASE` moves to 14 in the producer and in the wasm runner's decoder -- and both runners print them as `fbw_blackhole_adopted_single_frame` and `fbw_blackhole_adopted_multi_frame`. They join `JITSTATS_SNAPSHOT_FIELDS` and `JITSTATS_REGRESSION_ON_FALL`, where a fall is the regression: the walk stopped handing the interpreter an image. Recording them pins 110 single-frame and 56 multi-frame adoptions across 16 existing fixtures that nothing gated before. Assisted-by: Claude
`cargo fmt --check` flagged fbw_state.rs:922; the call fits in 100 columns. Assisted-by: Claude
… site `build_multi_frame_miframe` carried three `s2dbg!`-labelled decline arms while its seven real exits were unlabelled, five of them bare `?` on an `Option` — three of those buried in the lvalue of an assignment (`*miframe.int_values.get_mut(color)? = ...`). `?` desugars to a compiler-generated `return` whose residual for `Option` is `Option<Infallible>`, so it carries no payload and no call-site identity: the count of declines existed, the distribution did not. Each is now a `let ... else` carrying a message. The `s2dbg!` definition moves above the empty-framestack check so that exit can log too. `latch_abort_blackhole` takes a `&'static str` origin naming its call site and threads it into `capture_vstack_mirror_image`, `multi_frame_blackhole_preflight`, and `build_multi_frame_miframe`; every message from those functions is prefixed with it. The three call sites pass "mod2730", "mod2786", "bridge1361"; the two escape-flush sites pass "escape-flush". The framestack-empty label also prints the framestack depth and `transparent_helper_subwalk`. Every message stays behind `fbw_debug_abort_enabled()`. No condition, control-flow branch, or return value changes. Assisted-by: Claude
`run_sub_jitcode_walk` built its sub-context with `inline_subwalk: true` and inherited `transparent_helper_subwalk` from the caller, so a descent from a root-level Python walk ran with the flag false and the session framestack empty. Two sites read that combination: * the abort-coordinate claim in `jitcode_dispatch::walk` consumed the session's one-shot claim at a helper pc; * the post-step trace-limit check ran inside the helper and called `latch_abort_blackhole`, which takes its `inline_subwalk` arm and there calls `build_multi_frame_miframe` — which declines immediately on an empty framestack. The abort therefore latched no blackhole image and fell back to entry replay. Both sites already carry `!transparent_helper_subwalk` exclusions written for exactly this case. Set the flag on the sub-context `run_sub_jitcode_walk` builds, which is the single point every canonical helper descent passes through, and drop the `nested_helper_entry.is_some()` assignment at the builtin-gateway call site that only covered helpers entered from another sub-walk. Measured over the 373-fixture synth corpus with `PYRE_FBW_DEBUG_ABORT=1`: `mf-build-miframe` declines 97 -> 0 (94 from the trace-limit latch, 3 from the error-unwind latch); the 56 image builds that already succeeded are unchanged. `check.py` dynasm 1 / cranelift 1 / wasm 4 failed, the same rows with the same numbers as before the change. Assisted-by: Claude
`reconcile_vstack_at_boundary` computed `cfg_successor` from the previous opcode's fallthrough and branch targets only, so a boundary reporting the py_pc the walk is already on failed the test and armed the out-of-order permutation region. One Python opcode's jitcode expansion can carry more than one boundary marker, so this fires on ordinary code. Inside that region the per-op reconcile is replaced by `ShadowReseed`, which clears every operand-stack mirror slot and re-sources them from the virtualizable shadow; a slot the shadow cannot source stays `OpRef::NONE` and `capture_vstack_mirror_image` declines on it. The repeat is already modelled by the `layout_only_boundary` arm — the observed depth matches neither successor, so it preserves the surviving slots — and that arm is the one the reorder region excludes. Accept a same-py_pc boundary as a CFG successor. A genuine self-branch is unaffected: `target_pc` reports it, so the existing clause accepts it. `getframe_bridge_force_after_store` then captures the escape image it could not build before: `fbw_rolled_back_with_effects` 20 -> 0 and `fbw_blackhole_adopted_single_frame` 0 -> 20 on dynasm, cranelift and wasm alike — the walk resumes forward instead of rolling back and letting the legacy entry replay apply its un-journaled `box.n = i` store a second time. Baselines re-recorded on all three backends and the fixture header updated to describe what it now pins. Corpus-wide `mirror-slot ... unresolved` latch declines 23 -> 2 (`PYRE_FBW_DEBUG_ABORT=1` over the 373 synth fixtures). `check.py` cranelift 1 / wasm 4 failed, all base-owned rows with unchanged numbers; dynasm 0. Assisted-by: Claude
Three of the lines the `[latch-decline]` / `[s2-build-decline]` / `[s2-adopt-decline]` macros carry are not refusals, and a census that groups by that tag counts them as such. * `build_multi_frame_miframe`'s closing `BUILT multi-frame depth=N` is the build's success report. It moves to a sibling macro tagged `[s2-build-ok]` (56 lines over the synth corpus). * `multi_frame_blackhole_preflight`'s `pf-root-caps` line was emitted ahead of the four-way capability test, so it also announced every passing preflight, and it re-evaluated all four predicates that the test then evaluated again. It becomes an `else if` chain naming the first capability that refuses, reported only on the refusal; the evaluation order and short-circuiting are unchanged. The corpus has no refusal, so those 11 lines are gone. * `try_adopt_multi_frame_blackhole`'s `chain root=… levels=[…]` is emitted before the root-identity check and reports recovered state on both paths. It moves to `[s2-adopt-state]` (56 lines, matching the 56 builds). The same function's `no latched multi-frame image` is a real decline but the ordinary one: `try_adopt_blackhole` tries this arm before the single-frame arm, so every single-frame latch produces one. The message says so. Over the 373 synth fixtures the decline-tagged population falls from 269 to 155, of which 153 are that fallthrough. The remaining two are the `mirror-slot … unresolved` pair. No gate, condition or return value changes; every line stays behind `fbw_debug_abort_enabled()`. Assisted-by: Claude
`reconcile_vstack_at_boundary` cleared `vstack_reorder_ceiling` at the end of the call, after the boundary's class had already been applied. The comment on that clear says the region lasts "until the walk passes both endpoints", but the boundary that passes them still ran under `ShadowReseed`. That exiting boundary is an ordinary sequential step: its previous opcode produced `vstack_last_ref`, and `ResultToTos` would land it on the new TOS. `ShadowReseed` instead clears the mirror and re-sources from the virtualizable shadow, which cannot recover a slot whose only producer is a walk register -- a LOAD_NAME/LOAD_FAST temp is never written through to the portal array. The slot stays `OpRef::NONE` and `capture_vstack_mirror_image` declines the whole image. Move the clear ahead of the arming below. The order matters: a boundary that both passes the old ceiling and is itself out of order now opens a new region instead of running unprotected. `trace_too_long_effect_replay` reaches its trace-limit latch with a complete mirror: over the 373 synth fixtures the `mirror-slot ... unresolved` declines fall 2 -> 1, no other decline shape moves, and the 56 multi-frame image builds are unchanged. `check.py` dynasm 1 / cranelift 1 / wasm 4 failed, the same base-owned rows with the same numbers as before the change. The remaining one is a different cause: an unwind to an exception handler is not modelled as a CFG successor, so it arms the region across the whole handler body. Assisted-by: Claude
`reconcile_vstack_at_boundary`'s `cfg_successor` asked `liveness::target_pc`, which reads a branch delta out of the instruction's own operand. Under the zero-cost exception tables a handler target is not in the opcode, it is in `co_exceptiontable`, so an unwind from a covered pc to its handler matched no clause and armed the out-of-order permutation region. Arming it there is expensive: inside the region every boundary is forced to `ShadowReseed`, which clears the operand-stack mirror and re-sources from the virtualizable shadow, and the shadow cannot recover a slot whose only producer is a walk register. The ceiling is `max(prev, new)`, so an unwind backwards to a handler covers the whole handler body. `liveness::exception_target_pc` reads the covering entry's target through `pycode::lookup_exceptiontable`, converting byte offsets to code-unit indices at both ends as the codewriter's decode sites do. Across 40 exception-carrying synth fixtures the new clause is the only one that accepts the boundary on 62 of 10939 reconcile events -- 62 permutation regions that no longer open. No corpus decline moves: the one remaining `mirror-slot ... unresolved` is a `53 -> 35` transition that is not a successor of 53 under any of the four clauses (`Reraise` has no fallthrough and no branch operand, and the handler covering 53 targets 54), so it is tracked separately. The `PYRE_VSTACK_DIAG` line prints the successor set it decided against, `succ=(ft=..,br=..,exc=..)`. Without it an arming cannot be told apart from a real permutation. `check.py`: dynasm 390/390, cranelift 1 failed, wasm 4 failed -- the base-owned rows, numbers unchanged. Assisted-by: Claude
`vstack_enter_exception_handler` took the handler's Python pc from the floor segment of the catch target's JitCode offset. A catch target is an out-of-line landing block — `decode_exception_catch_sites` mints one per covered py_pc and the dispatch loop emits them all after the body — so it carries no py pivot of its own and the floor answers with whatever segment it was laid out inside. On the corpus's last blackhole-image decline that named the `RERAISE` ending the handler (py 53) where the walk went on to report the handler's `PUSH_EXC_INFO` entry (py 35): wrong coordinate, wrong depth, and a backwards boundary that armed the reorder region over the whole handler body. The codewriter states this same hazard next to `abort_permanent_py_pc_by_jit_pc`. Read the coordinate from `co_exceptiontable` instead — the unwind target of the entry covering the raising opcode. `lookup_exceptiontable` keeps the last matching entry, the rule `decode_exception_catch_sites` uses to pick the landing block, so the two agree by construction. The depth follows the resolved pc; the JitCode-offset-keyed twin can only answer for the floor segment, so it stays on that path with its audit. The landing block's own bytes lower no Python opcode. `vstack_handler_landing_py` holds the handler-entry coordinate while the walk is inside them, so they no longer read as a boundary either. `[vstack-handler]` reports both coordinates; `[vstack-reconcile]` names the code object and whether the walk is a sub-walk. Over the 373 synth fixtures `latch-decline` goes 1 -> 0. Assisted-by: Claude
`note_root_trace_too_long` carries the warm-state half of `prepare_trace_segmenting` (pyjitpl.py:2833) for the walker, which holds `&mut TraceCtx` and so cannot take `MetaInterp` to run the method itself. Its merge-point arm ran two of the three calls upstream makes there (pyjitpl.py:2843-2846): `trace_next_iteration` and `mark_force_finish_tracing`, but not `dont_trace_here`. Those two ask for the loop to be re-traced and force-finished while its callers may still inline it, so the next attempt can rebuild the trace that overflowed. `MetaInterp::prepare_trace_segmenting`, the per-opcode path's copy of the same arm, already makes all three. Assisted-by: Claude
The post-step trace-limit check (pyjitpl.py:2865 `_interpret`) is skipped inside a canonical helper descent, which has no blackhole entry point to abort at. Nothing said what still bounds the trace, and nothing tested it. The exemption defers the abort rather than dropping it, bounded by one descent. `run_sub_jitcode_walk` is the only site that sets the flag and the Python-callee sub-walk only inherits it, while every walk root is built with it clear, so an exempt descent always has a non-exempt Python frame above it; and `is_too_long` is a `num_ops > trace_limit` comparison over the shared `TraceCtx`, not an edge, so it still holds when that frame regains the check. Measured over the 373 synth fixtures: two reach the exemption at all, `trace_too_long_inline_multiframe` ending at 100 ops against a limit of 70 and `trace_too_long_effect_replay` at 115 against 100. The test walks one `inline_call_r_v/dR` past the limit under both settings of the flag — the enclosing frame aborts at its own pc, the descent finishes its body. Dropping the flag from the gate fails it. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/4d07c854c13fe2d50790bfba0ec2c7e295b1fe6d/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs#L6376
Clear helper transparency before entering Python callees
When a canonical helper invokes an inline Python function, this flag is inherited by that function's sub_wc through ..ctx.fbw_mode in try_walker_inline_resolved_user_call. Consequently, guards in the real Python callee take walker_capture_transparent_helper_snapshot (resume_snapshot.rs:2138), which serializes only parent_frames and omits the active callee's jitcode, locals, and register banks. A guard failure inside such a callee therefore resumes without that frame; keep this flag true only for the helper body and clear it when constructing an actual Python-callee context.
AGENTS.md reference: AGENTS.md:L32-L42
ℹ️ 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".
…t_doubled `fbw_blackhole_adopted_single_frame` reads 3 where the baseline had no entry for it. `loops_compiled=4` and `bridges_compiled=3` are unchanged, so the trace shape is the same and what moved is that the walk now adopts the blackhole resume image instead of falling back to the replay from the traced region's entry. Attributed by measuring both arms with the same command, `check.py --backend wasm --synthetic-only --synthetic-pattern exception_reused_object_tb_not_doubled`: with `ff503b5d746` reverse-applied in place the bench reports ALL PASSED against the existing baseline, and with it restored it reports the 0 -> 3 change. The control arm took 2m32s against the treatment arm's 4s, which is the wasm module being relinked rather than reused. The counter arrived with #1064 and this bench's baselines were last recorded at `da5e6fb38c7` (#1059), so absence from the baseline did not by itself say which of the two it was. No CI job runs `--backend wasm`, so the wasm baselines are not gated there either. The other four keys the re-record adds -- fbw_blackhole_adopted_multi_frame, fbw_store_journal_rollback_failed, field_pos_attached_misplaced, field_pos_spec_misplaced -- are counters that did not exist at #1059 and are pinned at 0 here for the first time. The dynasm and cranelift baselines are not re-recorded: both backends still report ALL PASSED for this bench. Assisted-by: Claude
…t_doubled `fbw_blackhole_adopted_single_frame` reads 3 where the baseline had no entry for it. `loops_compiled=4` and `bridges_compiled=3` are unchanged, so the trace shape is the same and what moved is that the walk now adopts the blackhole resume image instead of falling back to the replay from the traced region's entry. Attributed by measuring both arms with the same command, `check.py --backend wasm --synthetic-only --synthetic-pattern exception_reused_object_tb_not_doubled`: with `ff503b5d746` reverse-applied in place the bench reports ALL PASSED against the existing baseline, and with it restored it reports the 0 -> 3 change. The control arm took 2m32s against the treatment arm's 4s, which is the wasm module being relinked rather than reused. The counter arrived with #1064 and this bench's baselines were last recorded at `da5e6fb38c7` (#1059), so absence from the baseline did not by itself say which of the two it was. No CI job runs `--backend wasm`, so the wasm baselines are not gated there either. The other four keys the re-record adds -- fbw_blackhole_adopted_multi_frame, fbw_store_journal_rollback_failed, field_pos_attached_misplaced, field_pos_spec_misplaced -- are counters that did not exist at #1059 and are pinned at 0 here for the first time. The dynasm and cranelift baselines are not re-recorded: both backends still report ALL PASSED for this bench. Assisted-by: Claude
… on a non-measurement (#1095) * jit: stamp the qmut abort's own subwalk coordinate, and re-seed a live-NULL operand slot A walk that executed residual side effects and then fails to commit its end state falls back to the legacy replay from the traced region's entry, which runs those residuals a second time. Two shapes reached that fallback; both show up under `PYRE_FBW_CENSUS=1` as `committed=false effects>0`. `WalkSession::abort_in_subwalk` is sticky — `claim_abort_coordinate` only ever sets it — so an inline sub-walk abort the walk recovered from left it true for every later abort in the same trace attempt, and `flush_qmut_abort_state`'s gate then read a root-frame abort as a callee coordinate. The `ForceQuasiImmutable` raise in `dispatch_residual_call_iRd_kind` now stamps it from `fbw_mode.inline_subwalk` at the raise point, as the two kept-stack branch-guard raises already do. `reseed_vstack_from_shadow` rejected a NULL const-ptr shadow slot outright, because a NULL there can also mean a slot the portal never wrote. It now accepts one carrying the `virtualizable_live_null_slots` marker, which records that the last executed store into that slot wrote a NULL. PUSH_NULL's `self_or_null` sentinel is such a slot and stays live across the whole callable/args/kwargs build ahead of a CALL; the reorder region re-seeds the mirror in the middle of that build, and the rejected slot made `capture_vstack_mirror_image` refuse the image, leaving an escape inside the call with no blackhole resume. `capture_vstack_mirror_image`'s decline line gains the Python pc and the mirror boxes. The LoadName cell-fold gate comment is rewritten to the measured state: with the gate lifted the `bench/synth` corpus is output-correct, and what fails is `exception_reraise_tb_depth_jitstress` at 13.0x against its 4x pypy gate plus four benches' jit-stats. Measured with the gate lifted, in-place arms: `iter57/real_exception` 100003 -> 100000, `exception_reentry_guard_finally_residual` `leaked 4 reentry 2` -> `leaked 0 reentry 0`. Assisted-by: Claude * jit: record why reseed_vstack_from_callee_shadow keeps its NULL const-ptr rejection The callee-shadow reseed is the structural twin of `reseed_vstack_from_shadow` and rejects a NULL const-ptr the same way, but its source is a sparse `HashMap`, where a present key is already the write-witness the dense virtualizable array needed a per-slot side table to supply. So the clause discards a proven write whose value happens to be PUSH_NULL's `self_or_null`. Measured before writing this: dropping the clause leaves `check.py --backend dynasm` at 386/386 with no jit-stats movement and no baseline change, so the corpus does not distinguish the two behaviours. Behaviour unchanged; the comment records the asymmetry and the measurement. Assisted-by: Claude * rework.md: refresh the audit against the current tree The findings were measured on `pc-map` on 2026-07-05. Re-measured on `ec-wiring` at base 58fcd37, thirteen of the fifteen issues the document tracks are closed and the priority order has inverted. F1: gh#366/367/368/369 closed; `metadata.pc_map` and `resume_jitcode_pc_for` have zero hits and `resume::SnapshotFrame.pc` is the JitCode byte offset. The surviving `pc_map` matches are the compile-time exit-recovery `Vec<usize>` in jit/codewriter.rs and jit/flatten.rs, a different thing. Residue recorded: recorder.rs's SnapshotFrame doc still describes the deleted translation, py_pc is stored rather than derived, and build_state_field_snapshot stamps the JitCode offset into py_pc (unproven, needs a repro). F2: verified done — `is_full_body_walk`, `PYRE_FULL_BODY_WALK` and `OpcodeHandler for MIFrame` have zero hits each. F3: regressed to 15 registrations against MAX_EXTRA_ROOT_WALKERS = 16; the 16th caller hits `panic!("capacity exceeded")` at startup. F4: gh#346 and gh#373 closed, coverage still landing (#1065); abort_permanent unchanged in scale, but the exit criterion is the census, not a match count. F5: gate-triage.md now exists but the population grew from 119 matches to 245 distinct PYRE_* identifiers. Sequencing amended to WS3 > WS2 > WS1-residue > WS4. Assisted-by: Claude * rework.md: correct the F5 gate count to a reproducible measurement The refresh recorded 245 distinct `PYRE_*` identifiers against the audit's original 119. That figure does not reproduce: tracked `*.rs` holds 131 distinct identifiers, all tracked files 174, and 548 raw matches. The quantity comparable to the original "distinct `PYRE_*` env gates" is the set of names actually read from the environment, which is 126. The command is now stated in the document so the number can be re-derived, along with the three other counts it is easy to confuse it with. Assisted-by: Claude * check.py: do not fail a ratio gate whose baseline is clamped to the floor `_exec_time` clamps a startup-subtracted time to `EXEC_TIME_FLOOR_S` so ratios cannot divide by ~0. When the pypy baseline lands there, the ratio is `pyre_exec / EXEC_TIME_FLOOR_S` and the ceiling it is compared against is an absolute wall-clock budget of `ceiling * EXEC_TIME_FLOOR_S` seconds, fitted on whichever host wrote the header. The comparison table already marks those ratios `~` and prints "ratio is not a measurement"; the gate failed the run on them anyway. `failed_bound` now returns None whenever the baseline is clamped, instead of requiring the backend to be at the floor as well. Only the ceiling changes behaviour: the floor arms at `exec_baseline >= FLOOR_GATE_MIN_BASELINE_S`, which a clamped baseline is always under. The gate can therefore only pass more than before, never fail more. The `[... clamped to floor; ratio not a measurement]` suffix in `_gate_fail_detail` is unreachable once a clamped baseline returns no bound, and is removed; the `~` legend states the consequence instead. Three consecutive `main` runs failed this way on three different fixtures across two runners: global_cell_shortpreamble_hot 24.1x > 19x and class_reassign_hot 49.2x > 47x on ubuntu-24.04, reentrant_key_eq_mutation 10.3x > 5x on macos-latest (runs 31079972573, 31080288895). Discriminator, cranelift, `class_reassign_hot` with its ceiling temporarily set to 1: the previous check.py reports SLOWER "exec 0.13s > pypy 0.01s ratio 27.0x > gate 1x [pypy exec clamped to floor; ratio not a measurement]", this one reports PASS. With the same ceiling of 1 on seqiter_tuple_error_parity, whose pypy exec is a measurement, this check.py still reports SLOWER at 18.3x — the ceiling is untouched wherever the baseline is real. The three fixtures above pass with their own ceilings restored. Assisted-by: Claude * posix: correct which stat rejection precedes the platform's dir_fd check `stat_entry` consults `HAVE_FSTATAT` while unwrapping `dir_fd`, above the descriptor branch, so the descriptor+`dir_fd` conflict is unreachable where `fstatat` does not exist. The comment claimed both fd-conflict rejections come first. #1081 corrected the same claim in `extra_tests/parity_tests/os_stat_file_descriptor.py` and cites `_DirFD_Unavailable` (`interp_posix.py:285-292`) for it; this is the statement of it that sits next to the code. Assisted-by: Claude * bench: re-record the wasm jit-stats for exception_reused_object_tb_not_doubled `fbw_blackhole_adopted_single_frame` reads 3 where the baseline had no entry for it. `loops_compiled=4` and `bridges_compiled=3` are unchanged, so the trace shape is the same and what moved is that the walk now adopts the blackhole resume image instead of falling back to the replay from the traced region's entry. Attributed by measuring both arms with the same command, `check.py --backend wasm --synthetic-only --synthetic-pattern exception_reused_object_tb_not_doubled`: with `ff503b5d746` reverse-applied in place the bench reports ALL PASSED against the existing baseline, and with it restored it reports the 0 -> 3 change. The control arm took 2m32s against the treatment arm's 4s, which is the wasm module being relinked rather than reused. The counter arrived with #1064 and this bench's baselines were last recorded at `da5e6fb38c7` (#1059), so absence from the baseline did not by itself say which of the two it was. No CI job runs `--backend wasm`, so the wasm baselines are not gated there either. The other four keys the re-record adds -- fbw_blackhole_adopted_multi_frame, fbw_store_journal_rollback_failed, field_pos_attached_misplaced, field_pos_spec_misplaced -- are counters that did not exist at #1059 and are pinned at 0 here for the first time. The dynasm and cranelift baselines are not re-recorded: both backends still report ALL PASSED for this bench. Assisted-by: Claude * bench: restore bridges_compiled and guard_failures on three synth baselines `9d2fff92649` (#1063) re-recorded 993 jit-stats baselines. All but four gained only the two new `field_pos_*_misplaced=0` keys; three changed a value: binary_int_overflow_local_resume bridges 5 -> 6 guards 647 -> 686 exc_bridge_entry_guard_not_removed bridges 4 -> 5 guards 809 -> 1009 list_append_write_barrier_gc bridges 5 -> 6 guards 1345 -> 1562 Five runs report the pre-#1063 values and none reports the recorded ones: dynasm, cranelift and wasm here, and `main`'s own CI on ubuntu-24.04 and macos-latest at 9d2fff9 -- run 31139317566, jobs 92747505633 and 92748753166, on a tree carrying no commit from this branch. The three benches fail identically on all three backends in each of them. Only those two keys are restored; #1063's two added keys stay. The fourth bench it revalued, getattribute_override_no_bind, is left as recorded: it passes here and in that CI run, so its new values do reproduce. Assisted-by: Claude
Follow-up to #1056. The counter that PR turned into a CI gate was measuring the wrong
thing, and the conjunct it left open — which I closed there as "measured, no effect" —
turns out to fix two of the six instances the gate names.
1. The gated predicate counted effects that get rolled back
fbw_diag::ROLLED_BACK_WITH_EFFECTSgated on!committed && effects > 0.effectsisthe gh#467 executed-effect odometer, and three of its four bump sites are journal
pushes —
fbw_store_journal_push,fbw_append_journal_push,fbw_cell_store_journal_push.fbw_store_journal_rollbackundoes all three on the samenon-committed exit, before the epilogue reads the odometer, and never decrements it.
Only the fourth site is irreversible — which is what the gate's own documentation in
check.pyalready claimed the counter names.A fixture whose ten uncommitted walks carry only journaled effects reported 10. It
now reports 0, with the same ten walks still printed by the census.
The predicate is
!committed && (effects - journaled) > 0. The subtraction cannotunder-report: the odometer is written in exactly two places tree-wide, and the
set(0)sits inside
fbw_store_journal_reset, which clears all three journals in the samefunction. So
journaled <= effectsholds for every walk; adebug_assert!pins it.journal=in the census covered one journal of threeIt was
fbw_store_journal_len()alone. The append and cell journals had no lengthaccessor at all, so an append-journal effect printed
effects=1 journal=0—indistinguishable from a real residual effect. #1056's claim that the six recorded
instances were irreversible "because journal=0" did not hold, and I made it. The line
now prints
jstore=/jappend=/jcell=separately plusunrecoverable=.The hole the sharpening would have opened
fbw_store_journal_rollbackcan fail to restore a slot when a later residual shrank thelist. That was reported only under the debug gate and counted nowhere, so after
subtracting, such a walk would read
unrecoverable=0and escape the gate entirely. Itnow bumps
fbw_diag::STORE_JOURNAL_ROLLBACK_FAILED, printed by both runners and gated inJITSTATS_BADNESS_FIELDS.One baseline falls:
ca_bridge_multiframe_resume_double_callon wasm, 1 → 0. Its onlyuncommitted walk is
end=LoopBearingCalleeInlineUnsupport effects=1 journaled=1, on arun reporting
fbw_store_journal_rollback_failed=0— that counter, not the fall, is theevidence for the re-record.
2. The bridge vable-escape conjunct — #1056's "no effect" reading was wrong
residual_call.rs's single-frame blackhole latch carried!ctx.trace_ctx.is_bridge_traceas one of seven conjuncts from a default-off rollout gate. It is the only latch
producer a bridge vable escape has: the abort-path
latch_abort_blackholeis gated onerror.leaves_complete_image(), whose variant list does not containVableEscapedDuringResidualCall. So a bridge escape reached the epilogue with no imageand its caller replayed the region. The comment directly above the conjunct already
argues that upstream has no counterpart to either of its siblings — ABORT_ESCAPE goes to
run_blackhole_interp_to_cancel_tracingand runs forward, never replays.Two of the six recorded instances go to zero, on all three backends.
set_hash_protocol's walk moves fromend=CompileTracePending committed=false leg=0 bridge=true effects=7tocommitted=true leg=1, so those seven effects are applied onceinstead of twice.
recursive_forced_frame_kept_stackmoves the same way; its walk isbridge=falseand the change reaches it through the bridge walks in the same run, whichnow adopt.
Why #1056 got this wrong, and what changed
Two compounding measurement faults, both mine:
test.test_asyncio: two completing runs (2697tests) gave 5 and 0, and
PYTHONHASHSEED=0does not fix it. Every pyre subprocess alsoprints its own
[jit-stats]into the merged stderr (25-28 child lines per run) andcheck.pykeeps the last value per key, so a run killed before the parent exits isscored on a child's number.
reverted the source and did not rebuild. The giveaway is
[latch-decline]printingbetween
[force-shape] bridge=trueand[fbw-escape], a window whose only call sitesits inside the reverted
if.Redone with a rebuild per arm and a deterministic fixture, then confirmed by the corpus
above.
3. Two fixtures for a shape the corpus did not contain
Of 138 forced vable escapes across the synth corpus, none were
bridge=true. A hotloop whose rare arm guard-fails past the trace-eagerness threshold earns a bridge, and
sys._getframe(0)inside the bridge body forces the virtualizable without entering aPython frame — 20 of 20 forces are the target shape, against 11 of 18 on asyncio.
getframe_bridge_force_plainadopts 20 single-frame images.getframe_bridge_force_after_storeputs an un-journaled store ahead of the forcingcall, which leaves the escape's mirror slot unresolved, and records
fbw_rolled_back_with_effects=20on the replay path.Structural fact worth recording: the forcing residual can never bump the odometer itself —
the force branch returns 66 lines before the bump — so a bridge escape needs a second,
earlier effectful op to register at all. That is what the second fixture adds.
4. Neither fixture pinned anything without a counter
No gated field moved between the two arms, because nothing counted a successful
adoption.
fbw_diaghad slots for walks, roll-backs, latch declines and escapeattribution, and none for the machinery doing its job.
Slots 12 and 13 tally single- and multi-frame blackhole adoptions. Both runners print
them as
fbw_blackhole_adopted_single_frame/fbw_blackhole_adopted_multi_frame; theyjoin
JITSTATS_SNAPSHOT_FIELDSandJITSTATS_REGRESSION_ON_FALL, where a fall is theregression — the walk stopped handing the interpreter an image. Recording them pins 110
single-frame and 56 multi-frame adoptions across 16 existing fixtures that nothing gated
before.
5. Not done: a dedicated latch/adopt counter pair
Considered and rejected. The two-binary fixture A/B answers the question with no
per-latch bookkeeping, and the design that was drafted was wrong in three places — most
plainly,
try_adopt_blackholehas four call sites, so draining a mark at one of themunder-counts.
6. Every decline exit of the latch now carries a label and a call site
build_multi_frame_miframehad threes2dbg!-labelled decline arms and sevenunlabelled real exits, five of them a bare
?on anOption— three of those buriedin the lvalue of an assignment (
*miframe.int_values.get_mut(color)? = …).?desugarsto a compiler-generated
returnwhoseOptionresidual isOption<Infallible>, so itcarries no payload and no call-site identity: the count of declines existed, the
distribution did not. Each becomes a
let … elsewith a message, andlatch_abort_blackholetakes a&'static strorigin naming its call site and threads itinto
capture_vstack_mirror_image,multi_frame_blackhole_preflightandbuild_multi_frame_miframe.Everything stays behind
fbw_debug_abort_enabled(); no condition, branch or return valuechanges. The point is that it makes a census possible:
Four minutes over all 373 synth fixtures, and the whole corpus produces only four
distinct decline shapes. Two of them were one-line predicate bugs. In both, the in-code
comment explained the arm that was taken — not why the predicate choosing it was right.
7.
transparent_helper_subwalkwas tied to the wrong predicaterun_sub_jitcode_walkis the single point every canonical helper descent passes through.It built its sub-context with
inline_subwalk: trueand inheritedtransparent_helper_subwalk, so a descent from a root-level Python walk ran with the flagfalse and the session framestack empty. The builtin-gateway call site set it separately as
nested_helper_entry.is_some(), which isSomeonly when the helper is entered fromanother sub-walk — so it never covered the root-level case either.
inline_subwalk && framestack.is_empty()sendslatch_abort_blackholeinto itsmulti-frame arm, where
build_multi_frame_miframedeclines on its first line. The abortlatched no image and fell back to entry replay. Both reading sites — the abort-coordinate
claim in
jitcode_dispatch::walkand the post-step trace-limit check — already carried!transparent_helper_subwalkexclusions written for exactly this case. The flag neverreached them.
mf-build-miframedeclines 97 → 0 (94 from the trace-limit latch, 3 from theerror-unwind latch); the 56 image builds that already succeeded are unchanged.
8. A same-
py_pcboundary is not an out-of-order block transitionreconcile_vstack_at_boundarycomputedcfg_successorfrom the previous opcode'sfallthrough and branch targets only. One Python opcode's jitcode expansion can carry more
than one boundary marker, so a boundary reporting the pc the walk is already on failed
both clauses and armed
vstack_reorder_ceilingon the spot. Inside that region theper-op reconcile is replaced by
ShadowReseed, which clears every operand-stack mirrorslot and re-sources from the virtualizable shadow; a slot the shadow cannot source stays
OpRef::NONEandcapture_vstack_mirror_imagedeclines on it — including slots theopcode never touched.
The repeat is already modelled correctly by the
layout_only_boundaryarm (the observeddepth matches neither successor, so it preserves the surviving slots), and that arm is
precisely the one the reorder region excludes. A genuine self-branch is unaffected:
target_pcreports it, so the existing clause already accepts it.getframe_bridge_force_after_store— the fixture §3 added — then captures the escapeimage it could not build, on dynasm, cranelift and wasm alike:
fbw_rolled_back_with_effectsfbw_blackhole_adopted_single_frameSo the walk resumes forward instead of rolling back and letting the legacy entry replay
apply its un-journaled
box.n = istore a second time. This overturns §3's own reading— the fixture header said the store "is also what leaves the escape's operand-stack mirror
slot unresolved", i.e. that the 20 were inherent to the shape. They were not. The header
is rewritten to describe what the fixture now pins, and
fbw_rolled_back_with_effectsback above zero there is now a tripwire for this capture breaking again.
Corpus-wide
mirror-slot … unresolveddeclines 23 → 2. After §7 and §8 the onlyimage-build declines left in the whole synth corpus are those 2;
multi_frame_blackhole_preflightrejects nothing at all.9–11. The same mechanism, twice more — and the diagnostic that found it
§8 is one instance of a general rule worth stating: the mirror's out-of-order
permutation region is armed by
!cfg_successor, and inside it every boundary is forcedto
ShadowReseed, which clears the mirror and re-sources from the virtualizable shadow.The shadow cannot recover a slot whose only producer is a walk register. So every edge
cfg_successorfails to model costs a resume image. The predicate is not a heuristicknob; it is the CFG successor set.
jit: stop tagging blackhole-image successes as declines. Three lines the[latch-decline]/[s2-build-decline]/[s2-adopt-decline]macros carry are notrefusals —
build_multi_frame_miframe's closingBUILT multi-frame(56),multi_frame_blackhole_preflight'spf-root-caps, emitted ahead of the four-way testso it announced every passing preflight and re-evaluated all four predicates the test
then evaluated again (11), and
try_adopt_multi_frame_blackhole'schain root=…,emitted before the root-identity check (56). A census grouping by tag counted all 123 as
capability gaps. Success/state lines get their own tags;
pf-root-capsbecomes anelse ifchain naming the first refusing capability and prints only then, withevaluation order and short-circuiting unchanged; the genuine-but-ordinary
no latched multi-frame imagenow says it is the dispatch fallthrough to thesingle-frame arm. Decline-tagged population 269 → 155, of which 153 are that
fallthrough.
jit: end the vstack reorder region before the boundary that leaves it. The clearran at the end of the call, after the class had been applied, so the boundary that
"passes both endpoints" — its own comment's wording — still ran under
ShadowReseed,although it is an ordinary sequential step whose previous opcode really did produce
vstack_last_ref. Moved ahead of the arming, in that order, so a boundary that bothpasses the old ceiling and is itself out of order opens a new region instead of running
unprotected. Corpus
mirror-slot … unresolveddeclines 2 → 1.jit: model the exception-table handler edge as a CFG successor.target_pcreads abranch delta out of the opcode's operand, so under the zero-cost exception tables it
cannot see a handler edge.
liveness::exception_target_pcreads the covering entrythrough
pycode::lookup_exceptiontable. Across 40 exception-carrying fixtures it is theonly clause accepting the boundary on 62 of 10939 reconcile events — 62 permutation
regions that no longer open.
PYRE_VSTACK_DIAGnow prints the successor set thedecision was made against,
succ=(ft=..,br=..,exc=..).After these the whole 373-fixture synth corpus produces one image-build decline.
12. The catch target's py coordinate — the last decline, and a refuted RCA
The earlier round left that decline open with a recorded root cause: a
53 → 35transition that matched no
cfg_successorclause, narrowed to a stalevstack_cur_pypcfrom the bridge sub-walk seed. That RCA was wrong, and the diagnostic that refuted it
also refuted the frame it was reasoned in: the offending
<module>isapp_atexit.py,not the fixture's own module — every fixture runs it, only this one's
trace_limit=70aborts inside it.
vstack_enter_exception_handlerresolved the handler's Python pc from the floorsegment of the catch target's JitCode offset. A catch target is an out-of-line
landing block:
decode_exception_catch_sitesmints one per coveredpy_pcand thedispatch loop emits them all after the body, so they carry no py pivot and the floor
answers with whatever segment they happened to be laid out inside. It named the
RERAISEthat ends the handler (py 53) where the walk went on to report the handler's
PUSH_EXC_INFOentry (py 35) — wrong coordinate, wrong depth, and a backwards boundarythat armed the reorder region across the whole handler body. The codewriter states this
same hazard verbatim next to
abort_permanent_py_pc_by_jit_pc(codewriter.rs:14294):"
py_floor_by_jit_pccannot serve … a marker in a block emitted after the whole bodyfloors to whichever PC last opened a segment." Same trap, second consumer.
Two halves, both needed — either alone still arms the region:
handler_entry_py_pcreadsco_exceptiontable— the unwind target of the entrycovering the raising opcode.
lookup_exceptiontablekeeps the last matching entry,the rule
decode_exception_catch_sitesuses to pick the landing block, so the twoagree by construction. The depth follows the resolved pc; the JitCode-offset-keyed
twin can only answer for the floor segment, so it stays on that path with its audit.
WalkContext::vstack_handler_landing_pyholds the handler-entry coordinate whilethe walk is inside the landing block's bytes, which lower no Python opcode and so must
not read as a boundary. The hold clears the moment the walk reports the mirror's own
coordinate, so it lasts exactly as far as the landing block.
Corpus
latch-decline1 → 0. It also fixed a second handler entry in the samefixture (
floor_py=147against a realhandler_py=190) that had been arming a regionwithout producing a decline.
[vstack-reconcile]now names the code object and whetherthe walk is a sub-walk, and
[vstack-handler]prints the adopted coordinate next to thefloor answer — without those two fields the arming cannot be attributed at all.
Verification
pyre/check.py, rebuilt and re-run on the current base (1de95e0d321), once perbehaviour change. Final tree: dynasm 390/390, cranelift 1 failed / 389 passed, wasm 4
failed / 382 passed.
cargo test -p pyre-jit-trace: 342 + 11 + 1 passed, 0 failed.Every failure is base-owned, with numbers unchanged from before these commits:
pickle_ctor_args— jit: exception-path and iteration inlining, with an object-strategy args_w #1033 tightenedmax-pypy-ratio336 → 36; the pypy denominator is0.02s and other full
check.pyruns were live on this host throughout, so the ratio isnot adjudicable here. It read 40.7x / 37.9x / pass on dynasm across three runs of
this branch, and 46.7x on cranelift in the final run. The fixture is red on main's own
CI.
guard_failuresrows (closure_per_call470→468,exception_traceback_frame_lineno820→819,gc_iterator_source_drop613→614,recursive_call_frame_relocation649→648) — a macOS-vs-ubuntu divergence introduced bymain's own recent commits, byte-identical here to the set measured before these
commits. main's CI is green on these at that tip, so re-recording them from macOS
would break ubuntu; they are left at their committed values.
Nothing in this branch moves a row that is not re-recorded in it.