From ade259b614f6c11fadbe19b4ec84592c55f76c41 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 26 Jul 2026 01:52:48 +0900 Subject: [PATCH] jit(fbw): drop the unwired PendingInlineFrame vref residue; record _MULTIFRAME's measured corpus coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PendingInlineFrame::drop_frame_opref` was constructed at one site, always `None`, and never read. Its comment said `None` "skips the opimpl_virtual_ref emission in push_inline_frame"; `push_inline_frame` (`majit-metainterp/src/compile.rs`) is a depth guard that pushes a `(code_ptr, pc)` greenkey and contains no vref code, and the emission the comment names was never built. `InlineTraceStepAction` — the enum that would have carried a `PendingInlineFrame` into such a push — is declared and referenced nowhere in the tree. Remove both. Three comments corrected against the call graph rather than restated: `MetaInterpreter::push_inline_frame` (the identifier `MetaInterpreter` occurs exactly once in the tree, inside that comment); `opimpl_virtual_ref`'s "Called from metainterp push_inline_frame" (its only callers, in both the `majit-metainterp` and `pyre-jit-trace` ports, are `#[test]`, so `virtualref_boxes` stays empty and no live trace records a VIRTUAL_REF); and `push_inline_frame stamps parent_frames.first().pending_result_*` (`pending_result_stack_idx` is never assigned a `Some` value anywhere). gate-triage §1 records a measurement of the multi-frame path: the vable-escape latch site was instrumented and all 318 benchmarks (`pyre/bench` + `pyre/bench/synth`) run under `PYRE_FBW_MULTIFRAME=1`. The site is reached in 3 benches, 5 events each, and all 15 have `inline_subwalk=false` — every one takes the single-frame arm. `build_multi_frame_miframe` is never called, so the image is never latched and `try_adopt_multi_frame_blackhole` never sees a candidate. §4's `_BLACKHOLE_RESUME` row is amended: its retirement was conditioned on `_MULTIFRAME` landing, which that measurement makes unevaluable. Gates from the workspace root: `pyre-jit-trace` + `majit-metainterp` unit tests green (1399 / 294 / …), `check.py --backend dynasm` 313/313. Assisted-by: Claude --- pyre/gate-triage.md | 28 +++++++++++++++++++++++++--- pyre/pyre-jit-trace/src/state.rs | 27 +++++++++++---------------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index ab30b46c896..493b16fb065 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -118,7 +118,7 @@ upstream lines: | gate | orthodox side | outcome | |---|---|---| | PYRE_FBW_VABLE_SCALAR_CA | **OFF** | **RETIRED** — the ON design contradicts upstream | -| PYRE_FBW_MULTIFRAME | **ON** | keep; the ON path is the port, and it is unfinished | +| PYRE_FBW_MULTIFRAME | **ON** | keep; the ON path is the port, it is unfinished, and §1 measures it as never reached by the corpus | | PYRE_FBW_CALLEE_VSTACK | NEITHER | keep OFF; see §5 | The walker's default-ON `PYRE_FBW_*` cluster was retired separately in #757. @@ -187,7 +187,29 @@ keeps inlined callee frames unmaterialized (`fbw_strict_fold_frame_reg`, upstream does not have — a consequence of pyre's virtual-callee-frame inlining — plus per-frame vable binding, since `PyjitplBlackholeFrameConfig` stamps one shared `virtualizable_ptr` onto every frame in the chain and the adopt writes -only `last_instr`. +only `last_instr`. A third item sits below both: `try_adopt_multi_frame_blackhole` +(`pyre-jit-trace/src/trace.rs`) declines outright when the recovered chain is not +rooted at the walked frame, and names the `jit.virtual_ref` emit at the inline +push as the prerequisite. That emit does not exist — `opimpl_virtual_ref` / +`_finish` are ported in both `majit-metainterp/src/pyjitpl.rs` and +`pyre-jit-trace/src/state.rs`, and **neither has a caller outside a `#[test]`**, +so `virtualref_boxes` is empty and no live trace records a `VIRTUAL_REF`. + +**Measured 2026-07-25: the multi-frame path has no corpus coverage.** The +vable-escape latch site was instrumented and all **318** benchmarks +(`pyre/bench` + `pyre/bench/synth`) run under `PYRE_FBW_MULTIFRAME=1`. The site +is reached in **3 benches** (`getframe_escape_flush_writethrough_regression`, +`synth/getframe_inlined_callee_own_frame`, `synth/getframe_stored_fback_walk`), +5 events each, and **all 15 have `inline_subwalk=false`** — every one takes the +single-frame arm and adopts. `build_multi_frame_miframe` is therefore never +called, the image is never latched, and the adopt never sees a candidate. So +flipping `_MULTIFRAME` ON is a no-op across the corpus, none of the three items +above is exercised, and any port of them would be unvalidatable until a +benchmark that reaches `inline_subwalk=true` at a vable escape exists. Building +that benchmark is the prerequisite for the rest. Note the multi-frame latch is +nested inside `single_frame_blackhole_resume_enabled()`, so it also requires +`_BLACKHOLE_RESUME` to stay ON. The pre-existing `[s2-gate]` eprintln (under +`PYRE_FBW_DEBUG_ABORT`) already reports `inline_subwalk` at that site. ## §2 — Not gates (11): Rust identifiers, not env vars @@ -229,7 +251,7 @@ OFF path is a needed safety net. Retire at the listed trigger (A7). | var | subsystem | retire when | |---|---|---| -| PYRE_FBW_BLACKHOLE_RESUME | single-frame resume-past-escape (#754) | flipped default-ON 2026-07-25; retire once the multi-frame twin (`_MULTIFRAME`) lands and the escape/replay fallback is no longer the only other leg | +| PYRE_FBW_BLACKHOLE_RESUME | single-frame resume-past-escape (#754) | flipped default-ON 2026-07-25; retirement was conditioned on the multi-frame twin (`_MULTIFRAME`) landing, but §1 now measures that twin as having zero corpus coverage, so the condition is unevaluable — keep the gate and re-open the question only once a benchmark reaches `inline_subwalk=true` at a vable escape | | PYRE_TWO_PHASE_RTYPE, PYRE_TUPLE_PER_SHAPE_CLASSDEF | rtyper prepass / per-shape tuple classdef | WS2 / #346 rtyper epic | | PYRE_ORIGINAL_BOXES | greens++reds original_boxes index shape | box-identity #202 / resume F1 | | PYRE_MIR_FRAMESTATE | framestate-threaded MIR lowering | MIR front-end #176/#181/#346 | diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 809af426fae..173f338525f 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -5491,7 +5491,12 @@ impl PyreSym { /// records VIRTUAL_REF(box, cindex), and pushes /// [virtualbox, vrefbox] onto virtualref_boxes. /// -/// Called from metainterp push_inline_frame (executioncontext.enter parity). +/// Upstream's caller is `executioncontext.py:89 enter`, which the tracer +/// reaches by tracing through the interpreter's own frame-entry code. The +/// pyre walker builds its inline levels itself and never traces `enter`, so +/// this has no caller yet and `virtualref_boxes` stays empty in every live +/// trace. Wiring it is a prerequisite for the multi-frame blackhole adopt +/// (`try_adopt_multi_frame_blackhole`, `trace.rs`). pub(crate) fn opimpl_virtual_ref( ctx: &mut TraceCtx, sym: &mut PyreSym, @@ -11953,13 +11958,11 @@ mod tests { /// (`rpython/jit/metainterp/pyjitpl.py`) for pyre. RPython constructs /// and pushes the callee `MIFrame` directly inside `perform_call`; pyre /// returns this struct from the trace step so the framestack mutation -/// happens in `MetaInterpreter::push_inline_frame` after the trace -/// handler releases its borrow on `MetaInterpFrame`. No upstream -/// counterpart. +/// happens after the trace handler releases its borrow on +/// `MetaInterpFrame`. No upstream counterpart. pub struct PendingInlineFrame { pub sym: PyreSym, pub concrete_frame: pyre_interpreter::pyframe::PyFrame, - pub drop_frame_opref: Option, pub green_key: u64, /// Raw `(code_ptr, target_pc)` greenkey components for element- /// wise recursion-depth comparison. `green_key` above is the u64 @@ -12038,8 +12041,9 @@ pub(crate) fn recover_inline_callee_globals(code_ptr: *const ()) -> pyre_object: /// the bridge's root concrete frame. /// /// `parent_frames` is the OUTER chain (immediate parent first) the drain -/// builds from the framestack; `push_inline_frame` stamps -/// `parent_frames.first().pending_result_*` with the caller result slot. +/// builds from the framestack. The caller result slot travels in this +/// struct's own `caller_result_stack_idx` / `caller_result_type`; nothing +/// stamps `parent_frames.first().pending_result_*`, which stays `None`. pub(crate) fn assemble_bridge_inline_pending( ctx: &mut TraceCtx, recipe: &ReconstructRecipe, @@ -12149,10 +12153,6 @@ pub(crate) fn assemble_bridge_inline_pending( PendingInlineFrame { sym, concrete_frame, - // No virtual_ref for a reconstructed frame: the forward trace's - // virtual_ref was already finished/encoded; None skips the - // opimpl_virtual_ref emission in push_inline_frame. - drop_frame_opref: None, // The reconstructed frame represents the same inlined call the // forward trace pushed at function entry; match its (code, 0) // greenkey identity for recursion-depth + inline-position tracking. @@ -12315,11 +12315,6 @@ pub(crate) fn setup_reconstructed_callee_frame( Some((pending, argboxes_r)) } -pub enum InlineTraceStepAction { - Trace(TraceAction), - PushFrame(PendingInlineFrame), -} - pub fn execute_inline_residual_call( frame: &mut pyre_interpreter::pyframe::PyFrame, nargs: usize,