Skip to content

jit: resolve the jd1 novable unpackiterable-drain live blackhole resume and enable it by default #751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 15 commits into from
Jul 25, 2026
Merged

jit: resolve the jd1 novable unpackiterable-drain live blackhole resume and enable it by default #751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7462e6d
jit: update the trace heap cache after the setarrayitem_gc trace-walk…
youknowone Jul 23, 2026
24376f3
jit: thread per-jitdriver novable vinfo through blackhole resume
youknowone Jul 23, 2026
7065a0f
jit: resolve novable drain resume against build-time jitcode/liveness…
youknowone Jul 23, 2026
96fd9f5
jit: drop orphaned multi-frame exc reads; null-guard raw_code lookup
youknowone Jul 24, 2026
788be67
jit: collapse the jd1 drain append into one registered dont_look_insi…
youknowone Jul 24, 2026
57c49a0
test: guard the jd1 drain-match fusion fires on the real lowered drain
youknowone Jul 24, 2026
f5e87ea
jit: enable jd1 unpackiterable-drain and live-enter by default
youknowone Jul 24, 2026
0f70aa9
style: rustfmt the jd1 drain fnaddr registrations and novable resume …
youknowone Jul 24, 2026
e61a158
jit: accept the StopIteration discriminant in the drain-match fuse
youknowone Jul 25, 2026
c22137d
jit: skip the pre-registration placeholder in the vinfo driver resolver
youknowone Jul 25, 2026
8a07143
jit: park a non-StopIteration jd1 drain error for the caller loop
youknowone Jul 25, 2026
4167366
front: pass evalue for the write-only etype exceptblock link arg
youknowone Jul 25, 2026
a2f21d0
blackhole: register the canonical inline_call_* bytes and decline sym…
youknowone Jul 25, 2026
827034b
jit: route the walker's residual calls through the residual-call host…
youknowone Jul 25, 2026
8f10cc2
jit: decode guard-metadata `-live-` against the compiling driver's ow…
youknowone Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions majit/majit-backend-cranelift/src/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7690,6 +7690,9 @@ pub struct CraneliftBackend {
trace_counter: u64,
next_trace_id: Option<u64>,
next_header_pc: Option<u64>,
/// `Backend::set_next_frame_value_count_fn` — the compiling driver's
/// `-live-` decoder for the `rd_numb` reads below.
next_frame_value_count_fn: Option<fn(i32, i32) -> usize>,
Comment on lines +7693 to +7695

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find every call site of set_next_frame_value_count_fn to confirm whether
# it is reset to None before every compile for a driver that has no override,
# or only ever set once for jd1 without a matching reset for jd0.
rg -n -B3 -A3 'set_next_frame_value_count_fn' --type=rust

Repository: youknowone/pyre

Length of output: 153

🏁 Script executed:

#!/bin/bash
set -u

echo "== candidate files =="
fd -a 'compiler\.rs$|jitdriver\.rs$' . | sed 's#^\./##' | head -100

echo "== symbols =="
rg -n -B2 -A2 'next_frame_value_count_fn|set_next_frame_value_count_fn|next_trace_id|next_header_pc|collect_guards|JitDriverStaticData|frame_value_count_fn' --type=rust . | head -300

Repository: youknowone/pyre

Length of output: 25337

🏁 Script executed:

#!/bin/bash
set -u

echo "== exact occurrences =="
rg -n -B3 -A3 'next_frame_value_count_fn|set_next_frame_value_count_fn|frame_value_count_fn' --type=rust .

echo "== compiler.rs relevant implementation =="
sed -n '7680,7705p' majit/majit-backend-cranelift/src/compiler.rs
sed -n '7915,7928p' majit/majit-backend-cranelift/src/compiler.rs
sed -n '8405,8496p' majit/majit-backend-cranelift/src/compiler.rs
sed -n '15380,15410p' majit/majit-backend-cranelift/src/compiler.rs

echo "== jitdriver.rs relevant doc/field =="
sed -n '410,470p' majit/majit-metainterp/src/jitdriver.rs

Repository: youknowone/pyre

Length of output: 38243

🏁 Script executed:

#!/bin/bash
set -u

echo "== call sites near cranelift calls =="
sed -n '8390,8505p' majit/majit-backend-cranelift/src/compiler.rs

echo "== pyjitpl call sites excerpts =="
sed -n '6080,6098p' majit/majit-metainterp/src/pyjitpl.rs
sed -n '7140,7150p' majit/majit-metainterp/src/pyjitpl.rs

echo "== backend trait definition context =="
sed -n '1750,1782p' majit/majit-backend/src/lib.rs

Repository: youknowone/pyre

Length of output: 9310

Consume next_frame_value_count_fn for the next compilation.

set_next_frame_value_count_fn stores the value and collect_guards reads self.next_frame_value_count_fn; unlike the sibling one-shot fields, it is not cleared. Pass .take() instead of keeping it in the CraneliftBackend state so a driver override cannot persist into later compilations.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@majit/majit-backend-cranelift/src/compiler.rs` around lines 7693 - 7695,
Update the next-compilation handling for next_frame_value_count_fn so
collect_guards consumes it with Option::take rather than borrowing or retaining
the stored callback. Preserve set_next_frame_value_count_fn’s setter behavior
while ensuring the override is cleared after one compilation and cannot affect
subsequent compilations.

registered_call_assembler_tokens: IndexSet<u64>,
registered_call_assembler_bridge_traces: IndexSet<u64>,
/// llmodel.py: self.vtable_offset — byte offset for vtable in objects.
Expand Down Expand Up @@ -7917,6 +7920,7 @@ impl CraneliftBackend {
trace_counter: 1,
next_trace_id: None,
next_header_pc: None,
next_frame_value_count_fn: None,
registered_call_assembler_tokens: IndexSet::new(),
registered_call_assembler_bridge_traces: IndexSet::new(),
// llmodel.py:64-69: vtable_offset is None when gcremovetypeptr is
Expand Down Expand Up @@ -8487,6 +8491,7 @@ impl CraneliftBackend {
caller_layout,
&constants_i64,
attached_descrs,
self.next_frame_value_count_fn,
)?;
// RPython jitframe layout parity: ref_root slots start AFTER all
// output slots. max_output_slots must be >= inputs.len() so that
Expand Down Expand Up @@ -14404,6 +14409,10 @@ fn collect_guards(
caller_layout: Option<&ExitRecoveryLayout>,
_constants: &indexmap::IndexMap<u32, i64>,
attached_descrs: majit_backend::AttachedDescrPtrs,
// `Backend::set_next_frame_value_count_fn` — the compiling driver's
// `-live-` decoder for the `rd_numb` reads below. `None` falls back to the
// process-global callback.
frame_value_count_fn: Option<fn(i32, i32) -> usize>,
) -> Result<(), BackendError> {
let type_index = OpTypeIndex::new(inputargs, ops);
let (type_overrides, op_def_positions) = build_type_overrides(ops, &type_index);
Expand Down Expand Up @@ -14541,7 +14550,7 @@ fn collect_guards(
(op.resolved_rd_numb(), op.resolved_rd_consts())
{
use majit_ir::resumedata::{get_frame_value_count_fn, rebuild_from_numbering};
let fvc = get_frame_value_count_fn();
let fvc = frame_value_count_fn.or_else(get_frame_value_count_fn);
let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> =
fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize);
let num_virtuals = op.resolved_rd_virtuals().map_or(0, |v| v.len());
Expand Down Expand Up @@ -14588,7 +14597,7 @@ fn collect_guards(
let rd_vi = op.resolved_rd_virtuals();
use majit_ir::resumedata::{self, RebuiltValue, rebuild_from_numbering};
let rd_consts_ref: &[majit_ir::Const] = &rd_consts_data;
let fvc = majit_ir::resumedata::get_frame_value_count_fn();
let fvc = frame_value_count_fn.or_else(majit_ir::resumedata::get_frame_value_count_fn);
let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> =
fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize);
let num_virtuals = rd_vi.as_ref().map_or(0, |v| v.len());
Expand Down Expand Up @@ -15386,6 +15395,10 @@ impl majit_backend::Backend for CraneliftBackend {
self.next_header_pc = Some(header_pc);
}

fn set_next_frame_value_count_fn(&mut self, fvc: Option<fn(i32, i32) -> usize>) {
self.next_frame_value_count_fn = fvc;
}

fn set_done_with_this_frame_descr_void(&mut self, descr: majit_ir::DescrRef) {
self.descr_attachments
.write()
Expand Down
77 changes: 77 additions & 0 deletions majit/majit-backend/src/call_stub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,87 @@ pub fn collect_call_args_positional(
out
}

/// Dispatch a residual call described by `arg_classes`, picking the signature
/// strategy the active backend supports.
///
/// `bh_call_*_dispatch` transmutes the funcptr to an `extern "C" fn` guessed
/// from the *bucketed* `(int, float)` arity. That is sound only on a C ABI that
/// tolerates a signature mismatch: SysV/AAPCS pass the surplus in registers the
/// callee ignores, and a `usize` parameter is register-width either way. wasm32
/// has neither property — `call_indirect` type-checks the callee's declared
/// type on every call, and a pointer parameter is `i32` where the transmute
/// says `i64` — so a mistyped guess traps with `indirect call type mismatch`
/// instead of silently working.
///
/// Where a host trampoline is installed (`set_residual_host_call`, wasm32) the
/// call must therefore go through it with the *positional* argument list.
/// [`collect_call_args`] discards the interleaving `arg_classes` encodes, so
/// the choice cannot be recovered downstream: it belongs here, at the last
/// point that still holds `arg_classes`.
///
/// # Safety
/// On the transmute path, `func` must match the ABI [`collect_call_args`]
/// derives from `arg_classes` — see [`bh_call_i_dispatch`].
pub unsafe fn bh_call_i_by_classes(
func: usize,
arg_classes: &str,
args_i: Option<&[i64]>,
args_r: Option<&[i64]>,
args_f: Option<&[i64]>,
) -> i64 {
if let Some(hook) = residual_host_call() {
let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f);
return hook(func, &args);
}
let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f);
unsafe { bh_call_i_dispatch(func, &int_args, &float_args) }
}

/// f64-returning parallel of [`bh_call_i_by_classes`].
///
/// # Safety
/// See [`bh_call_i_by_classes`].
pub unsafe fn bh_call_f_by_classes(
func: usize,
arg_classes: &str,
args_i: Option<&[i64]>,
args_r: Option<&[i64]>,
args_f: Option<&[i64]>,
) -> f64 {
if let Some(hook) = residual_host_call() {
let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f);
// The trampoline returns an f64 callee result as its raw bits.
return f64::from_bits(hook(func, &args) as u64);
}
let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f);
unsafe { bh_call_f_dispatch(func, &int_args, &float_args) }
}

/// Result-discarding parallel of [`bh_call_i_by_classes`].
///
/// # Safety
/// See [`bh_call_i_by_classes`].
pub unsafe fn bh_call_v_by_classes(
func: usize,
arg_classes: &str,
args_i: Option<&[i64]>,
args_r: Option<&[i64]>,
args_f: Option<&[i64]>,
) {
if let Some(hook) = residual_host_call() {
let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f);
let _ = hook(func, &args);
return;
}
let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f);
unsafe { bh_call_v_dispatch(func, &int_args, &float_args) }
}

/// A host-provided trampoline that performs a residual call by reflecting the
/// callee's real signature, rather than transmuting the raw funcptr to a
/// statically-guessed `extern "C" fn`.
///

/// `func_ptr` is the raw callee address (a table index on wasm32); `args` is
/// the positional argument list (floats as raw bits). The return value is the
/// callee result as a 64-bit pattern (Void callees return 0; Ref returns the
Expand Down
71 changes: 34 additions & 37 deletions majit/majit-backend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,18 @@ pub trait Backend: Send {
/// this header PC to synthesised exit recovery layouts.
fn set_next_header_pc(&mut self, _header_pc: u64) {}

/// The compiling driver's override for the `jitcode.py:147 enumerate_vars`
/// frame box count, when a backend decodes the guards' `rd_numb` itself to
/// build exit layouts.
///
/// Carried on `JitDriverStaticData::frame_value_count_fn`: a driver whose
/// frames are numbered outside the process-global liveness pool must not
/// decode against it, and the wrong pool decodes *successfully* with a
/// mistyped count rather than failing. `None` (the default) leaves the
/// backend on the global callback
/// (`majit_ir::resumedata::set_frame_value_count_fn`).
fn set_next_frame_value_count_fn(&mut self, _fvc: Option<fn(i32, i32) -> usize>) {}

/// `compile.py:665-674` `make_and_attach_done_descrs([self, cpu])` —
/// per-result-type `DoneWithThisFrame*` singleton shared with
/// `MetaInterpStaticData`. Attached once per CPU instance, matching
Expand Down Expand Up @@ -2692,20 +2704,17 @@ pub trait Backend: Send {
if func == 0 {
return 0;
}
if let Some(hook) = crate::call_stub::residual_host_call() {
let args = crate::call_stub::collect_call_args_positional(
// SAFETY: `func` is a valid funcptr matching the ABI recovered from
// `calldescr.arg_classes`.
unsafe {
crate::call_stub::bh_call_i_by_classes(
func as usize,
&calldescr.arg_classes,
args_i,
args_r,
args_f,
);
return hook(func as usize, &args);
)
}
Comment on lines +2707 to 2717

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add ABI regression coverage for the shared residual-call path.

Please add or verify tests covering mixed I/R/F argument classes and Int, Ref, Float, and Void returns, including a real GC-pointer return through bh_call_r and empty/None argument buckets. This should validate class ordering and pointer-return handling after replacing the previous dispatch path.

Also applies to: 2733-2742, 2759-2768, 2784-2793

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@majit/majit-backend/src/lib.rs` around lines 2707 - 2717, Extend the
regression tests for the shared residual-call path around the dispatch branches
invoking bh_call_i_by_classes, bh_call_r, and the other return handlers. Cover
mixed I/R/F argument classes, Int, Ref, Float, and Void returns, empty or None
argument buckets, and a real GC-pointer return through bh_call_r; verify class
ordering and pointer-return handling remain correct.

let (int_args, float_args) =
crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f);
// SAFETY: `func` is a valid funcptr matching the (ints, floats) arity
// recovered from `calldescr.arg_classes`.
unsafe { crate::call_stub::bh_call_i_dispatch(func as usize, &int_args, &float_args) }
}
/// model.py:268 bh_call_r(func, args_i, args_r, args_f, calldescr).
/// `llmodel.py:818 bh_call_r`: GCREF-returning parallel — a host pointer
Expand All @@ -2721,20 +2730,16 @@ pub trait Backend: Send {
if func == 0 {
return GcRef::NULL;
}
if let Some(hook) = crate::call_stub::residual_host_call() {
let args = crate::call_stub::collect_call_args_positional(
// SAFETY: see `bh_call_i`.
let raw = unsafe {
crate::call_stub::bh_call_i_by_classes(
func as usize,
&calldescr.arg_classes,
args_i,
args_r,
args_f,
);
return GcRef(hook(func as usize, &args) as usize);
}
let (int_args, float_args) =
crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f);
// SAFETY: see `bh_call_i`.
let raw =
unsafe { crate::call_stub::bh_call_i_dispatch(func as usize, &int_args, &float_args) };
)
};
GcRef(raw as usize)
}
/// model.py:270 bh_call_f(func, args_i, args_r, args_f, calldescr).
Expand All @@ -2751,20 +2756,16 @@ pub trait Backend: Send {
if func == 0 {
return 0.0;
}
if let Some(hook) = crate::call_stub::residual_host_call() {
let args = crate::call_stub::collect_call_args_positional(
// SAFETY: see `bh_call_i`.
unsafe {
crate::call_stub::bh_call_f_by_classes(
func as usize,
&calldescr.arg_classes,
args_i,
args_r,
args_f,
);
// The trampoline returns an f64 callee result as its raw bits.
return f64::from_bits(hook(func as usize, &args) as u64);
)
}
let (int_args, float_args) =
crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f);
// SAFETY: see `bh_call_i`.
unsafe { crate::call_stub::bh_call_f_dispatch(func as usize, &int_args, &float_args) }
}
/// model.py:272 bh_call_v(func, args_i, args_r, args_f, calldescr).
/// `llmodel.py:834 bh_call_v`: void-typed dispatch so a genuinely void
Expand All @@ -2780,20 +2781,16 @@ pub trait Backend: Send {
if func == 0 {
return;
}
if let Some(hook) = crate::call_stub::residual_host_call() {
let args = crate::call_stub::collect_call_args_positional(
// SAFETY: see `bh_call_i`.
unsafe {
crate::call_stub::bh_call_v_by_classes(
func as usize,
&calldescr.arg_classes,
args_i,
args_r,
args_f,
);
let _ = hook(func as usize, &args);
return;
)
}
let (int_args, float_args) =
crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f);
// SAFETY: see `bh_call_i`.
unsafe { crate::call_stub::bh_call_v_dispatch(func as usize, &int_args, &float_args) }
}

// ── model.py: additional bh_* helpers ──
Expand Down
Loading
Loading