From fed37bfa113cf317ab1ec8b115ce81ec8134103f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 2 Sep 2026 22:20:47 +0900 Subject: [PATCH 1/5] interp: carry StepResult::CloseLoop's jump_args as an Option `PyFrame::close_loop` filled the field with `vec![]`. In a jitcode that `Vec::new` is a residual call whose path the translator has no binding for, so a walk that reaches a back edge through `close_loop` stops there; the field is written at five sites and read at none. Assisted-by: Claude --- pyre/pyre-interpreter/src/eval.rs | 2 +- pyre/pyre-interpreter/src/pyopcode.rs | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 771093443c5..f119a61f91b 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -3660,7 +3660,7 @@ impl ControlFlowOpcodeHandler for PyFrame { // Signal a back-edge to the main eval_loop, which handles // JIT counting and compiled code execution via try_back_edge_jit. Ok(StepResult::CloseLoop { - jump_args: vec![], + jump_args: None, loop_header_pc: target, }) } diff --git a/pyre/pyre-interpreter/src/pyopcode.rs b/pyre/pyre-interpreter/src/pyopcode.rs index 7156a26465d..ac6a129e928 100644 --- a/pyre/pyre-interpreter/src/pyopcode.rs +++ b/pyre/pyre-interpreter/src/pyopcode.rs @@ -127,7 +127,14 @@ pub enum StepResult { Continue, Return(V), CloseLoop { - jump_args: Vec, + /// The arguments [`ControlFlowOpcodeHandler::close_loop_args`] + /// supplied, carried as an `Option` so a handler that has none does + /// not have to build an empty `Vec` for the field. The distinction + /// costs nothing at the reader — the back edge is the report itself, + /// and no consumer reads these — while an owned empty `Vec` costs an + /// allocation on every back edge and, in a jitcode, a residual call to + /// `Vec::new` that the translator has no binding for. + jump_args: Option>, loop_header_pc: usize, }, Yield(V), @@ -523,7 +530,7 @@ pub trait ControlFlowOpcodeHandler: SharedOpcodeHandler { fn close_loop(&mut self, target: usize) -> Result, PyError> { match self.close_loop_args(target)? { Some(args) => Ok(StepResult::CloseLoop { - jump_args: args, + jump_args: Some(args), loop_header_pc: target, }), None => Ok(StepResult::Continue), From 04c287bdd18875e20d31e9b6a94ff9c0d98ed512 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 3 Sep 2026 01:27:22 +0900 Subject: [PATCH 2/5] majit-translate: seat a payload enum's variant fields after the inherited discriminant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A payload enum is modelled as RPython's sum-type subclass layout: the base carries `__discriminant`, each variant subclass carries its own fields and inherits the base's byte 0. When `layout_for_target` returns `None` the variant's field offsets fall back to default packing from byte 0, so the first payload field is placed on the discriminant's byte. `pyre_interpreter::pyopcode::StepResult` has no recorded layout. Its `CloseLoop` variant was `{jump_args@0, loop_header_pc@8}` and `Return` was `{__pos_0@0}`, so `close_loop`'s jitcode wrote discriminant 2 to byte 0 and overwrote it with `jump_args` on the next op; `execute_jump_backward` read the null back as tag 0 and reported `Continue`. The explicit sum shell that seats the tag at 0 and the payload from 8 was reached by an allowlist of two type names, `core::result::Result` and `core::option::Option`. Compute the condition those two share instead: apply the shell when the base registered a `__discriminant` row the variant inherits and the host layout would seat a payload field inside the tag's bytes, taking an absent layout as overlapping since default packing starts at byte 0. The guard reuses `!fieldless && !tag_recorded_but_unspellable`, the same test the base row's own registration uses, so an `I128`/`U128` tag — which registers no base row — keeps byte 0 for its payload. `int_type_byte_width` maps the tag spelling Charon records beside the offset to a byte count; an unrecorded width answers a machine word. Assisted-by: Claude --- majit/majit-translate/src/front/mir.rs | 65 +++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/majit/majit-translate/src/front/mir.rs b/majit/majit-translate/src/front/mir.rs index ccff7801aa7..d6c463ed1d8 100644 --- a/majit/majit-translate/src/front/mir.rs +++ b/majit/majit-translate/src/front/mir.rs @@ -1395,6 +1395,19 @@ fn register_synthetic_positional_metadata( /// A fixed 16 would truncate any shell that ever records more than one payload /// word. The floor keeps a tag-only shell at the full `[tag | payload]` width, /// matching the `size.max(16)` the codewriter applies to the same shell. +/// Byte width of a Rust integer spelling, for the enum tag `TypeLayout` +/// records beside its offset (`discriminant_int_type`). Anything unspelled +/// answers a machine word, which is the conservative direction: it widens the +/// tag's byte range and so decides more enums need the explicit sum shell. +fn int_type_byte_width(int_ty: &str) -> u64 { + match int_ty { + "i8" | "u8" => 1, + "i16" | "u16" => 2, + "i32" | "u32" => 4, + _ => 8, + } +} + fn sum_shell_size(field_offsets: &std::collections::HashMap) -> u64 { field_offsets .values() @@ -1812,8 +1825,56 @@ fn derive_program_metadata( // base tag stays at offset 0 either way, so the shell only // moves the payload of an Option this front end actually // builds. - let explicit_sum_shell = - name == "core::result::Result" || name == "core::option::Option"; + // + // Those two names were the cases this shell was first written + // for, but the condition they share is physical rather than + // nominal: whenever the host layout seats a variant's payload + // field on the tag's own bytes, the constructor's payload store + // overwrites the tag it just wrote. Asking the layout catches + // every such enum instead of the two that were noticed. + // + // `pyre_interpreter::pyopcode::StepResult` — what every opcode + // handler returns — is one of them. Charon gives + // `CloseLoop.jump_args` and `Return.__pos_0` offset 0, the tag's + // own offset, so `close_loop`'s jitcode stamped discriminant 2 + // and then overwrote it with `jump_args`; `execute_jump_backward` + // read the null back as tag 0 and reported `Continue`, and the + // portal took its `Continue` arm on every back edge. A walk of + // the portal could therefore never reach `loop_header`, and no + // portal trace could close. + // The shell is needed exactly when the base registered a + // `__discriminant` row that the variant subclass inherits at + // byte 0 (`rclass.py:499-518`): the payload must then start + // after it. `tag_recorded_but_unspellable` registers no base + // row at all, so a payload at 0 there aliases nothing. + let base_has_discriminant = !fieldless && !tag_recorded_but_unspellable; + // An unrecorded tag width reads as a machine word: the wider + // guess only moves more enums onto the explicit shell, which + // is the representation that cannot alias in the first place. + let tag_offset = enum_layout + .as_ref() + .and_then(|l| l.discriminant_offset()) + .unwrap_or(0); + let tag_size = enum_layout + .as_ref() + .and_then(|l| l.discriminant_int_type()) + .map_or(8, int_type_byte_width); + let payload_seats_on_tag = match enum_layout.as_ref() { + // Charon recorded no layout for this decl — the case that + // hid this bug. The variant offsets then fall back to + // default packing from byte 0, which is the inherited + // discriminant's byte, on every payload enum in the set. + None => true, + Some(l) => variants.iter().enumerate().any(|(vidx, v)| { + (0..v.fields.len()).any(|i| { + l.field_offset(vidx, i) + .is_some_and(|off| off >= tag_offset && off < tag_offset + tag_size) + }) + }), + }; + let explicit_sum_shell = name == "core::result::Result" + || name == "core::option::Option" + || (base_has_discriminant && payload_seats_on_tag); // Register the enum BASE in `exact_layouts`: a single // `__discriminant` field at the tag's real byte position // (`discriminator.Branch.offset` via `discriminant_offset`). From 0019f2bfb6b05f40693381c87f8ac7099a5a8ca3 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 3 Sep 2026 01:27:43 +0900 Subject: [PATCH 3/5] majit: print MAJIT_PCSEQ branches at every frame depth, tagged with the jitcode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pcseq_branch` returned early unless `frames.len() == 1`, so only the root jitcode's own switches and branches printed and the whole descent was silent. A portal walk reporting `Continue` on a back edge then gave no reading between the portal's arm and the opcode handler, while a static decode of that chain read correct at every hop. Drop the depth gate and add `d=` to each line. The line that named the divergence — `execute_jump_backward`'s own `switch` on the `StepResult` discriminant — sits three frames down. Assisted-by: Claude --- majit/gate-triage.md | 2 +- majit/majit-metainterp/src/lib.rs | 18 ++++++++++++------ majit/majit-metainterp/src/pyjitpl/dispatch.rs | 11 ++++++----- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/majit/gate-triage.md b/majit/gate-triage.md index 9e5035ab015..52b3ef680a2 100644 --- a/majit/gate-triage.md +++ b/majit/gate-triage.md @@ -386,7 +386,7 @@ cover the condition they diagnose. - Read sites: 1 — `majit/majit-metainterp/src/lib.rs` - Accessor: `pcseq_enabled()` -- What it does: Prints the control-flow decisions a walk of the portal jitcode makes: the interpreter pc every `jit_merge_point` visit carries, and — for the root jitcode frame alone — every `switch`, every `goto_if_not`, and every `loop_header`. A walk that records without ever closing gives the same reading at the top (`seen_loop_header_for_jdindex` stays -1 at every merge point) whether the `loop_header` op was never reached or was reached and declined, and the two call for opposite fixes; the branch lines name which edge diverted the walk. +- What it does: Prints the control-flow decisions a walk makes: the interpreter pc every `jit_merge_point` visit carries, and every `switch`, `goto_if_not` and `loop_header`, each tagged with the jitcode that owns it and its frame depth (`d=`). A walk that records without ever closing gives the same reading at the top (`seen_loop_header_for_jdindex` stays -1 at every merge point) whether the `loop_header` op was never reached or was reached and declined, and the two call for opposite fixes; the branch lines name which edge diverted the walk, at whatever depth it happened. - Retirement condition: **UNRECORDED** — owed by this gate's owner. ### `MAJIT_PORTAL_INLINE` diff --git a/majit/majit-metainterp/src/lib.rs b/majit/majit-metainterp/src/lib.rs index f87fef8e03c..ed1b998a714 100644 --- a/majit/majit-metainterp/src/lib.rs +++ b/majit/majit-metainterp/src/lib.rs @@ -494,18 +494,24 @@ pub fn mptrace_enabled() -> bool { *FLAG.get_or_init(|| std::env::var_os("MAJIT_MPTRACE").is_some()) } -/// Prints the control-flow decisions a walk of the portal jitcode makes: the -/// interpreter pc every `jit_merge_point` visit carries, and — for the root -/// jitcode frame alone — every `switch`, every `goto_if_not`, and every -/// `loop_header`. +/// Prints the control-flow decisions a walk makes: the interpreter pc every +/// `jit_merge_point` visit carries, and every `switch`, `goto_if_not` and +/// `loop_header`, each tagged with the jitcode that owns it and its frame +/// depth (`d=`). /// /// A walk that records without ever closing gives the same reading at the top /// (`seen_loop_header_for_jdindex` stays -1 at every merge point) whether the /// `loop_header` op was never reached or was reached and declined, and the two /// call for opposite fixes. The branch lines name which edge diverted the walk, /// so the answer is the sequence itself rather than an inference from its -/// absence. Restricted to the root frame because the ops in question are the -/// portal's own; an inlined callee's branches would bury them. +/// absence. +/// +/// Every depth, not the root frame alone. Restricting it to the root was how +/// this was first written, and it cost a whole build to learn better: the +/// portal reported `Continue` on a back edge whose entire callee chain read +/// correct when decoded statically, and the line that settled it was +/// `execute_jump_backward`'s own `switch` — three frames down. A reading taken +/// only at the top says which arm the portal took and nothing about why. pub fn pcseq_enabled() -> bool { static FLAG: std::sync::OnceLock = std::sync::OnceLock::new(); *FLAG.get_or_init(|| std::env::var_os("MAJIT_PCSEQ").is_some()) diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index a45b7277c8a..1111aa410f0 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -2324,16 +2324,17 @@ where /// to follow are the portal's, and the callees run orders of magnitude /// more of them. fn pcseq_branch(&mut self, kind: &str, opcode_pc: usize, value: i64, target: Option) { - if !crate::pcseq_enabled() || self.frames.len() != 1 { + if !crate::pcseq_enabled() { return; } + let depth = self.frames.len(); let name = &self.frames.current_mut().jitcode.name; match target { - Some(t) => { - eprintln!("@@@PCSEQ {kind} jitcode={name} pc={opcode_pc} value={value} -> {t}") - } + Some(t) => eprintln!( + "@@@PCSEQ {kind} d={depth} jitcode={name} pc={opcode_pc} value={value} -> {t}" + ), None => eprintln!( - "@@@PCSEQ {kind} jitcode={name} pc={opcode_pc} value={value} -> fallthrough" + "@@@PCSEQ {kind} d={depth} jitcode={name} pc={opcode_pc} value={value} -> fallthrough" ), } } From 344d9ec969f227a84ce19b62c75e75e5aa430862 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 3 Sep 2026 01:27:44 +0900 Subject: [PATCH 4/5] jit, pyrex, wasm: publish the portal's can_enter_jit tallies as a counter export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every portal instrument is a `std::env::var` read — `PYRE_PORTAL_METATRACE`, `PYRE_PORTAL_METATRACE_ENTRY`, `_SKIP`, `MAJIT_PCSEQ` — and the wasm guest has no environment, so none of them can fire there. Add `PORTAL_DIAG`, two counters read through one `pyre_jit::eval::portal_diag(slot)` by both consumers: the native `[jit-stats] portal_diag` line from `maybe_print_jit_stats`, and the `pyre_jit_portal_diag` / `pyre_jit_portal_diag_len` exports the wasm runner prints as the same line. Exported rather than imported, as `pyre_jit_bridge_diag` already is, because an import shifts the JIT's function-index space. Both bumps sit inside `can_enter_jit`'s body, which `jtransform` rewrites into the `loop_header` operation and so never traces. `pyre-jit` is one of the four LLBC crates, so a counter in `eval_loop_jit`'s `CloseLoop` arm is lowered into the portal jitcode, and `AtomicU64::fetch_add` is outside the LLBC set: it becomes a symbolic residual between the arm and its `loop_header`, which no `pyre-jit` path can bind. `portal_activation_bracketed` is reached from `funccall_valuestack` with no `dont_look_inside` or may-force boundary, so it carries no counter either. The runner reports a slot the guest exports but its own legend does not name as `slot`, rather than dropping it as the `bridge_diag` mirror would. `can_enter_jit` and `can_enter_jit_taken` are absent from `JITSTATS_SNAPSHOT_FIELDS`, so no recorded baseline carries them. Assisted-by: Claude --- pyre/pyre-jit/src/eval.rs | 67 +++++++++++++++++++++++++++++++ pyre/pyre-wasm-runner/src/main.rs | 21 ++++++++++ pyre/pyre-wasm/src/lib.rs | 27 +++++++++++++ pyre/pyrex/src/lib.rs | 13 ++++++ 4 files changed, 128 insertions(+) diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 04f65a40c20..2ff396a3d53 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -6268,6 +6268,10 @@ impl PyPyJitDriver { let green_key = driver.resolve_cell_key(green_key_hash, || { pyre_jit_trace::driver::make_green_key_typed(loop_pycode, next_instr, is_being_profiled) }); + // Safe to tally here and nowhere upstream of it: `jtransform` rewrites + // the `can_enter_jit` call into the `loop_header` operation, so this + // body is never part of a traced graph. See `PORTAL_DIAG_LABELS`. + portal_diag_bump(0); if portal_metatrace_enabled() && PORTAL_METATRACE_SEEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed) >= portal_metatrace_skip() @@ -6297,6 +6301,7 @@ impl PyPyJitDriver { return false; }; set_pending_loop_exit(ec, loop_result); + portal_diag_bump(1); true } } @@ -7154,6 +7159,68 @@ fn jd1_experiment_enabled() -> bool { }) } +/// Legend for [`PORTAL_DIAG`], positionally. +/// +/// The portal's own decisions are otherwise unobservable on wasm32: every +/// existing portal knob (`PYRE_PORTAL_METATRACE`, `MAJIT_PCSEQ`) is a +/// `std::env::var` read, and the guest has no environment, so those probes are +/// dead code there rather than silent evidence. A counter the host reads back +/// through an export is the shape that carries out of the guest — the same +/// channel `majit_backend_wasm::BRIDGE_DIAG` and the full-body-walk census +/// already use. +/// +/// Every tally is bumped from code the portal graph does **not** contain. +/// That constraint is not stylistic: `pyre-jit` is one of the four LLBC crates, +/// so a counter placed in `eval_loop_jit`'s `CloseLoop` arm is lowered into the +/// portal jitcode, and `AtomicU64::fetch_add` is outside the LLBC set — it +/// would become a symbolic residual sitting between the arm and its +/// `loop_header`, which no `pyre-jit` path can ever bind (every production +/// fnaddr comes from `pyre_interpreter::jit_trace_fnaddrs`, and that table +/// cannot name a crate above it). It would block the portal walk *and* regress +/// any production trace that inlines a nested activation. `can_enter_jit`'s +/// body is safe for the opposite reason: `jtransform` rewrites the call into +/// the `loop_header` operation, so the body is never traced. +/// +/// That rules out the sites the arm's own decisions would need. The two +/// filters sit in the arm itself, and `portal_activation_bracketed` is reached +/// from `funccall_valuestack` with no `dont_look_inside` or may-force boundary +/// in between, so none of them is provably outside a traced graph. This line +/// counts what it can count safely rather than what would read most directly. +pub const PORTAL_DIAG_LABELS: &[&str] = &[ + // `can_enter_jit` was reached (warmspot.py:446). Only `eval_loop_jit`'s + // `StepResult::CloseLoop` arm calls it, and only after both of that arm's + // filters passed, so a run with back edges and `can_enter_jit=0` says the + // arm never completed — which is what a clobbered `StepResult` discriminant + // looks like from outside the trace. + "can_enter_jit", + // …and it answered true: the driver took the trace or ran compiled code. + "can_enter_jit_taken", +]; + +/// Portal decision tallies, indexed by [`PORTAL_DIAG_LABELS`]. +/// +/// `Relaxed` throughout: these are diagnostics, no other state is ordered +/// against them, and the back edge they sit on is hot. +pub static PORTAL_DIAG: [std::sync::atomic::AtomicU64; PORTAL_DIAG_LABELS.len()] = + [const { std::sync::atomic::AtomicU64::new(0) }; PORTAL_DIAG_LABELS.len()]; + +#[inline] +fn portal_diag_bump(slot: usize) { + if let Some(cell) = PORTAL_DIAG.get(slot) { + cell.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + } +} + +/// One portal tally, by [`PORTAL_DIAG_LABELS`] index; out of range reads 0. +/// +/// The wasm export and the native `[jit-stats]` line share this reader so the +/// two backends report the same numbers from the same place. +pub fn portal_diag(slot: usize) -> u64 { + PORTAL_DIAG + .get(slot) + .map_or(0, |cell| cell.load(std::sync::atomic::Ordering::Relaxed)) +} + fn portal_metatrace_enabled() -> bool { static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); *ENABLED.get_or_init(|| std::env::var("PYRE_PORTAL_METATRACE").as_deref() == Ok("1")) diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index 27f461188bd..dff8541a729 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -911,6 +911,27 @@ fn run(module_path: &Path, source: &str, script: &Path) -> Result { } eprintln!("[jit-stats] bridge_diag {}", parts.join(" ")); } + // The portal's own decision tallies. POSITIONAL MIRROR of + // `pyre_jit::eval::PORTAL_DIAG_LABELS`, but unlike `bridge_diag` above + // the guest also exports the slot count, so a slot added there without + // a label here is reported as `slot` rather than silently dropped. + if let Ok(diag) = instance.get_typed_func::(&mut store, "pyre_jit_portal_diag") { + let labels = ["can_enter_jit", "can_enter_jit_taken"]; + let guest_len = instance + .get_typed_func::<(), u32>(&mut store, "pyre_jit_portal_diag_len") + .ok() + .and_then(|f| f.call(&mut store, ()).ok()) + .unwrap_or(labels.len() as u32) as usize; + let mut parts = Vec::new(); + for i in 0..guest_len.max(labels.len()) { + let n = diag.call(&mut store, i as u32).unwrap_or(0); + match labels.get(i) { + Some(lbl) => parts.push(format!("{lbl}={n}")), + None => parts.push(format!("slot{i}={n}")), + } + } + eprintln!("[jit-stats] portal_diag {}", parts.join(" ")); + } if let Ok(geometry) = instance.get_typed_func::(&mut store, "pyre_jit_inline_geometry_diag") { diff --git a/pyre/pyre-wasm/src/lib.rs b/pyre/pyre-wasm/src/lib.rs index 359e8aa2f17..ef11a4c5c32 100644 --- a/pyre/pyre-wasm/src/lib.rs +++ b/pyre/pyre-wasm/src/lib.rs @@ -572,6 +572,33 @@ pub extern "C" fn pyre_jit_mc_diag(i: u32) -> u64 { majit_metainterp::mc_diag(i as usize) } +/// Diagnostic-only: the portal's own decision tallies +/// (`pyre_jit::eval::PORTAL_DIAG`, legend in `PORTAL_DIAG_LABELS`). +/// +/// Every other portal instrument is an env-gated print — `PYRE_PORTAL_METATRACE`, +/// `MAJIT_PCSEQ` — and the guest has no environment, so none of them can fire +/// here. This export is the only channel that says whether the wasm portal was +/// entered, whether any back edge reached its `CloseLoop` arm, and which filter +/// consumed the ones that did. Exported rather than imported for the same +/// reason as `pyre_jit_bridge_diag`: an import would shift the JIT's own +/// function-index space. +/// +/// `..._len` is exported beside the values so the runner can report a slot this +/// crate added but its own legend does not name, instead of dropping it. The +/// existing `bridge_diag` mirror has no such guard and relies on both sides +/// being edited together. +#[cfg(all(target_arch = "wasm32", feature = "wasm-host"))] +#[unsafe(no_mangle)] +pub extern "C" fn pyre_jit_portal_diag(i: u32) -> u64 { + pyre_jit::eval::portal_diag(i as usize) +} + +#[cfg(all(target_arch = "wasm32", feature = "wasm-host"))] +#[unsafe(no_mangle)] +pub extern "C" fn pyre_jit_portal_diag_len() -> u32 { + pyre_jit::eval::PORTAL_DIAG_LABELS.len() as u32 +} + /// Diagnostic-only: the full-body-walk decline census /// (`pyre_jit_trace::jitcode_dispatch::census_entries`), which names the /// `DispatchError` variant behind every aborted walk. The map is always diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index c0858c52e33..0ab7891b33c 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -1084,6 +1084,19 @@ fn maybe_print_jit_stats() { "[jit-stats] mc_diag {} all_descrs={all_descrs_len}", majit_metainterp::mc_diag_summary() ); + // The portal's own decisions, joined against the labels declared beside + // the counters (`pyre_jit::eval::PORTAL_DIAG_LABELS`). Read from the same + // reader the wasm export calls, so the two backends' lines diff field by + // field. This is the only portal instrument that works on wasm at all: the + // rest are `std::env::var` reads, and the guest has no environment. + { + let portal: Vec = pyre_jit::eval::PORTAL_DIAG_LABELS + .iter() + .enumerate() + .map(|(i, lbl)| format!("{lbl}={}", pyre_jit::eval::portal_diag(i))) + .collect(); + eprintln!("[jit-stats] portal_diag {}", portal.join(" ")); + } // How many of the rehydrated descr pool a run actually names, under // `PYRE_DESCR_DEMAND`. Off by default; `(0, pool)` means the probe is off. let (descr_demanded, descr_pool) = pyre_jit::descr_demand_summary(); From b552ab5a9acd37f36d885148365d88d5c0d432d4 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 3 Sep 2026 08:31:22 +0900 Subject: [PATCH 5/5] majit, jit: name the portal runner the code dispatches, and drop pcseq_branch's root-frame claim `pcseq_branch`'s doc still described the depth gate the previous commit removed ("Print one control-flow edge of the root jitcode frame", "Silent for an inlined callee"). `get_jitcode_calldescr` takes the address of `bh_portal_runner(&[i64], &[i64], &[i64]) -> i64` and pairs it with a `"r"` -> `'r'` descr, and its comment reads as if that pair were how pyre calls a Python function. It is not dispatched: `bhimpl_recursive_call_*` reaches the portal through `get_portal_runner` (`bh_portal_runner_c`, `"iirrr"`), and pyre's `inline_call` handlers read their callee from a build-time descr pool whose `fnaddr` comes from `JitCodeBuilder::set_native_entry`, which a runtime CodeObject jitcode is never in. Say that, and say which of the two functions the descr describes. `executor.rs`'s Float arm named `bh_portal_runner(all_i, all_r, all_f)` as what `portal_runner_adr` holds; `call_jit.rs:2819` sets it to `bh_portal_runner_c`. The i64-return contract the comment exists to pin holds for both. Assisted-by: Claude --- majit/majit-metainterp/src/executor.rs | 8 +++--- .../majit-metainterp/src/pyjitpl/dispatch.rs | 12 ++++----- pyre/pyre-jit/src/jit/call.rs | 25 +++++++++++-------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/majit/majit-metainterp/src/executor.rs b/majit/majit-metainterp/src/executor.rs index 8c7715c4684..30fd01360a6 100644 --- a/majit/majit-metainterp/src/executor.rs +++ b/majit/majit-metainterp/src/executor.rs @@ -330,10 +330,10 @@ pub fn execute_varargs( // `funcbox.2` as a hand-written or `#[jit_module]`-generated // function pointer with i64-return ABI: // * `do_recursive_call` (`pyjitpl.rs`) sets funcbox.2 - // to `targetjitdriver_sd.portal_runner_adr`. Pyre's - // portal entry is `bh_portal_runner(all_i, all_r, all_f) - // -> i64` (pyre-jit/src/call_jit.rs); it never - // declares an f64 return. + // to `targetjitdriver_sd.portal_runner_adr`, which is + // `bh_portal_runner_c(i64, i64, i64, i64, i64) -> i64` + // (pyre-jit/src/call_jit.rs); it never declares an f64 + // return. // * `#[jit_module]` (majit-macros/src/lib.rs) emits a // Float helper's `concrete_ptr` as `extern "C" fn(...) // -> i64` with the f64 result pre-packed via diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index 1111aa410f0..e42a69510d4 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -2315,14 +2315,14 @@ where ); } - /// Print one control-flow edge of the root jitcode frame under - /// `MAJIT_PCSEQ` (see `crate::pcseq_enabled`). + /// Print one control-flow edge under `MAJIT_PCSEQ` + /// (see `crate::pcseq_enabled`). /// /// `value` is the concrete word the edge was decided on and `target` the - /// cursor it moves to, so a run's lines read as the walk's own path - /// through the portal. Silent for an inlined callee: the ops this exists - /// to follow are the portal's, and the callees run orders of magnitude - /// more of them. + /// cursor it moves to, so a run's lines read as the walk's own path. + /// `d=` is the frame depth and `jitcode=` the frame that owns the edge: + /// an inlined callee prints too, because the edge that diverts a portal + /// walk is routinely one of theirs. fn pcseq_branch(&mut self, kind: &str, opcode_pc: usize, value: i64, target: Option) { if !crate::pcseq_enabled() { return; diff --git a/pyre/pyre-jit/src/jit/call.rs b/pyre/pyre-jit/src/jit/call.rs index 11ffd35b026..32212a49179 100644 --- a/pyre/pyre-jit/src/jit/call.rs +++ b/pyre/pyre-jit/src/jit/call.rs @@ -438,16 +438,21 @@ impl CallControl { /// return (fnaddr, calldescr) /// ``` /// - /// Note: pyre's blackhole calls every Python - /// function through one `bh_portal_runner(frame: ref) -> ref` - /// stub — the C-ABI is identical for every CodeObject because the - /// portal runner unwraps the frame and dispatches dynamically. So - /// `(fnaddr, calldescr)` is constant across all graphs in pyre, - /// while RPython has one pair per `FUNC` type because lltype-typed - /// `direct_call` ops can carry varying signatures. Keeping the - /// method shape preserves the call.py:167 - /// `(fnaddr, calldescr) = self.get_jitcode_calldescr(graph)` flow - /// even though both values are constants here. + /// The pair is constant across every graph, where RPython has one per + /// `FUNC` type because lltype-typed `direct_call` ops carry varying + /// signatures. Keeping the method shape preserves the call.py:167 + /// `(fnaddr, calldescr) = self.get_jitcode_calldescr(graph)` flow even so. + /// + /// Nothing dispatches the address it returns. `bhimpl_recursive_call_*` + /// reaches the portal through `get_portal_runner`, which answers + /// `bh_portal_runner_c` with the `"iirrr"` descr that really describes it; + /// and where upstream's `bhimpl_inline_call_*` calls + /// `cpu.bh_call_*(jitcode.fnaddr, …)` (blackhole.py:1279-1320), pyre reads + /// its callee out of a build-time descr pool whose `fnaddr` comes from + /// `JitCodeBuilder::set_native_entry`, and a CodeObject jitcode minted here + /// is never in that pool. So this stamps a non-null placeholder, and the + /// descr beside it describes `bh_portal_runner_c`, not the slice-taking + /// `bh_portal_runner` whose address it takes. pub fn get_jitcode_calldescr(&self, _graph: *const CodeObject) -> (i64, BhCallDescr) { let fnaddr = crate::call_jit::bh_portal_runner as *const () as usize as i64; let calldescr = BhCallDescr::from_arg_classes(