Skip to content

jit: eliminate the guard-fail resume-decode backxlat inverse (jitcode-blackhole Slice 3') #727

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 8 commits into from
Jul 23, 2026
Merged

jit: eliminate the guard-fail resume-decode backxlat inverse (jitcode-blackhole Slice 3') #727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
53e6e56
jit: carry forward per-frame py_pc through resume data + audit twin (…
youknowone Jul 22, 2026
13138a7
jit: cut guard-fail resume to the forward py_pc, drop the decode-side…
youknowone Jul 22, 2026
c72b3c9
jit: fix resume-data unit tests for the forward py_pc frame header
youknowone Jul 22, 2026
9f1bc8b
jit: address forward py_pc review findings (symmetric abort + dedup t…
youknowone Jul 22, 2026
d495cbb
jit: source inlined-callee resumed-frame namespace from its own code …
youknowone Jul 22, 2026
e623a80
jit: dedup op-start pred resume twins on jitcode offset via BTreeMap
youknowone Jul 22, 2026
7dae04a
jit: add after-residual fallthrough-py forward twin under audit (jitc…
youknowone Jul 23, 2026
8d08ccd
jit: cut after-residual fallthrough resume coordinate to the forward …
youknowone Jul 23, 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
3 changes: 3 additions & 0 deletions majit/majit-backend/src/resume_value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ pub struct FrameInfo {
/// populates it with the Python bytecode PC because pyre's tracer
/// records Python bytecode rather than JitCode.
pub pc: u64,
/// Forward-carried Python instruction PC. `-1` is the no-snapshot
/// sentinel and must round-trip through every resume encoder unchanged.
pub py_pc: i32,
/// Mapping from slot index to a tagged resume source.
pub slot_map: Vec<FrameSlotSource>,
}
Expand Down
15 changes: 12 additions & 3 deletions majit/majit-ir/src/resumedata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ pub struct RebuiltFrame {
pub jitcode_index: i32,
/// resume.py:250 `pc` — the JitCode byte offset.
pub pc: i32,
/// Forward-carried Python instruction PC; `-1` is the no-snapshot
/// sentinel paired with `pc == -1`.
pub py_pc: i32,
Comment on lines +358 to +360

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 23, 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.

P2 Badge Thread carried py_pc into inline bridge reconstruction

When a multi-frame guard resumes at a JitCode PC whose forward Python PC differs from backxlat_py_pc (the trivia/after-residual/branch coordinates this field is meant to preserve), the bridge-reconstruction path still ignores RebuiltFrame.py_pc: reconstruct_inline_recipe derives py_pc with backxlat_py_pc(frame.jitcode_index, frame.pc) in pyre/pyre-jit-trace/src/state.rs:5927, and the bridge carrier still re-inverts root/recipe PCs in trace.rs:588, trace.rs:1048, and trace.rs:1237. That makes the new decoded field unused for inline bridge setup, so it can compute stack depth or pending-result slots from the wrong Python opcode and either drain otherwise valid bridges or rebuild an inlined callee at the wrong resume point. Please carry this py_pc through the carrier/recipe and use it in those consumers instead of the inverse.

Useful? React with 👍 / 👎.

pub values: Vec<RebuiltValue>,
}

Expand Down Expand Up @@ -430,14 +433,14 @@ pub fn decode_tagged_value(
/// Decode rd_numb back into vable/vref values and per-frame tagged values.
///
/// resume.py:249-253, resume.py:1049-1055: RPython encodes frames as
/// `jitcode_index, pc, [tagged_values...]` and uses jitcode liveness
/// `jitcode_index, pc, py_pc, [tagged_values...]` and uses jitcode liveness
/// (`get_current_position_info`) at the decode site to know how many
/// values each frame has.
///
/// `frame_value_count`: when `Some(f)`, `f(jitcode_index, pc)`
/// returns the number of tagged values for that frame (RPython parity:
/// liveness-driven decode). When `None`, all remaining items after
/// `(jitcode_index, pc)` are consumed as a single frame (backward-compat for
/// `(jitcode_index, pc, py_pc)` are consumed as a single frame (backward-compat for
/// callers that only ever see single-frame data).
///
/// `fail_arg_types`: parent guard's per-failarg type vector. resume.py:1245
Expand Down Expand Up @@ -490,7 +493,7 @@ pub fn rebuild_from_numbering(
));
}

// resume.py:1049-1055: frame section — jitcode_index, pc, [tagged_values...].
// Pyre M73: frame section — jitcode_index, pc, py_pc, [tagged_values...].
// RPython uses consume_one_section → enumerate_vars(liveness) to split frames.
let mut frames = Vec::new();
while reader.items_read < total_size as usize && reader.has_more() {
Expand All @@ -500,6 +503,11 @@ pub fn rebuild_from_numbering(
} else {
0
};
let py_pc = if reader.has_more() && reader.items_read < total_size as usize {
reader.next_item()
} else {
-1
};
let box_count = if let Some(f) = &frame_value_count {
// RPython parity: liveness-driven frame boundary.
f(jitcode_index, pc)
Expand All @@ -524,6 +532,7 @@ pub fn rebuild_from_numbering(
frames.push(RebuiltFrame {
jitcode_index,
pc,
py_pc,
values,
});
}
Expand Down
5 changes: 3 additions & 2 deletions majit/majit-metainterp/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ pub(crate) fn build_guard_metadata<T: AsRef<majit_ir::Op>>(
// as separate sections. Do not merge vable_array entries into
// the innermost frame slots here.
for frame in frames.iter() {
builder.push_frame(frame.jitcode_index, frame.pc as u64);
builder.push_frame(frame.jitcode_index, frame.pc as u64, frame.py_pc);
let mut slot_idx = 0usize;
for val in &frame.values {
add_slot(&mut builder, slot_idx, val);
Expand All @@ -595,7 +595,7 @@ pub(crate) fn build_guard_metadata<T: AsRef<majit_ir::Op>>(
}
} else {
// No rd_numb: single frame, 1:1 mapping (fail_args[i] → state[i]).
builder.push_frame(0, pc);
builder.push_frame(0, pc, -1);
let num_slots = op
.getfailargs()
.map(|fa| fa.len())
Expand Down Expand Up @@ -2625,6 +2625,7 @@ mod tests {
framestack: vec![SnapshotFrame {
jitcode_index: 0,
pc: 8,
py_pc: 8,
boxes: vec![OpRef::input_arg_int(1).into()],
}],
};
Expand Down
29 changes: 24 additions & 5 deletions majit/majit-metainterp/src/history.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,7 @@ impl TreeLoop {
.map(|f| crate::recorder::SnapshotFrame {
jitcode_index: f.jitcode_index,
pc: f.pc,
py_pc: f.py_pc,
boxes: f.boxes.iter().map(&remap_tagged).collect(),
})
.collect(),
Expand Down Expand Up @@ -2219,6 +2220,13 @@ impl TraceCtx {
/// resolve to a known `Box.type`; constants must have a recorded
/// value. Misses are bookkeeping bugs and panic, not silent
/// fallbacks.
/// `py_pc == pc` here: this convenience serves jitdrivers whose
/// interpreter pc already *is* the JitCode pc — the native meta-tracing
/// clients (and unit tests) that have no CPython-bytecode layer, so the
/// two coordinates coincide and no JitCode→Python translation applies.
/// The pyre CPython-bytecode path never uses this shortcut; it carries a
/// distinct forward Python pc through
/// `capture_snapshot_for_last_guard_with_vable_vref`.
pub fn capture_snapshot_for_last_guard(
&mut self,
active_boxes: &[OpRef],
Expand All @@ -2229,6 +2237,7 @@ impl TraceCtx {
active_boxes,
jitcode_index,
pc,
pc,
&[],
&[],
);
Expand All @@ -2254,6 +2263,7 @@ impl TraceCtx {
active_boxes: &[OpRef],
jitcode_index: u32,
pc: u32,
py_pc: u32,
vable_boxes: &[crate::recorder::SnapshotTagged],
vref_boxes: &[crate::recorder::SnapshotTagged],
) {
Expand All @@ -2263,6 +2273,7 @@ impl TraceCtx {
frames: vec![crate::recorder::SnapshotFrame {
jitcode_index,
pc,
py_pc,
boxes,
}],
vable_boxes: vable_boxes.to_vec(),
Expand All @@ -2281,6 +2292,7 @@ impl TraceCtx {
active_boxes: &[OpRef],
jitcode_index: u32,
pc: u32,
py_pc: u32,
vable_boxes: &[crate::recorder::SnapshotTagged],
vref_boxes: &[crate::recorder::SnapshotTagged],
) {
Expand All @@ -2289,6 +2301,7 @@ impl TraceCtx {
frames: vec![crate::recorder::SnapshotFrame {
jitcode_index,
pc,
py_pc,
boxes,
}],
vable_boxes: vable_boxes.to_vec(),
Expand All @@ -2314,7 +2327,10 @@ impl TraceCtx {
/// (RPython's `_number_boxes` does this implicitly via the memo
/// table; pyre's `Snapshot.encode` does the same in
/// `resume.rs:1898 _number_boxes`).
pub fn capture_snapshot_for_last_guard_multi_frame(&mut self, frames: &[(u32, u32, &[OpRef])]) {
pub fn capture_snapshot_for_last_guard_multi_frame(
&mut self,
frames: &[(u32, u32, u32, &[OpRef])],
) {
self.capture_snapshot_for_last_guard_multi_frame_with_vable_vref(frames, &[], &[]);
}

Expand All @@ -2328,18 +2344,19 @@ impl TraceCtx {
/// it sees in the trace-time MIFrame stack.
pub fn capture_snapshot_for_last_guard_multi_frame_with_vable_vref(
&mut self,
frames: &[(u32, u32, &[OpRef])],
frames: &[(u32, u32, u32, &[OpRef])],
vable_boxes: &[crate::recorder::SnapshotTagged],
vref_boxes: &[crate::recorder::SnapshotTagged],
) {
let recorder_frames: Vec<crate::recorder::SnapshotFrame> = frames
.iter()
.map(|(jitcode_index, pc, boxes)| {
.map(|(jitcode_index, pc, py_pc, boxes)| {
// The pc word is a raw JitCode offset.
let encoded = self.encode_snapshot_boxes(boxes);
crate::recorder::SnapshotFrame {
jitcode_index: *jitcode_index,
pc: *pc,
py_pc: *py_pc,
boxes: encoded,
}
})
Expand All @@ -2362,18 +2379,19 @@ impl TraceCtx {
/// paused caller chain is available for a full `Snapshot.frames`.
pub fn capture_snapshot_for_last_guard_op_multi_frame_with_vable_vref(
&mut self,
frames: &[(u32, u32, &[OpRef])],
frames: &[(u32, u32, u32, &[OpRef])],
vable_boxes: &[crate::recorder::SnapshotTagged],
vref_boxes: &[crate::recorder::SnapshotTagged],
) {
let recorder_frames: Vec<crate::recorder::SnapshotFrame> = frames
.iter()
.map(|(jitcode_index, pc, boxes)| {
.map(|(jitcode_index, pc, py_pc, boxes)| {
// The pc word is a raw JitCode offset.
let encoded = self.encode_snapshot_boxes(boxes);
crate::recorder::SnapshotFrame {
jitcode_index: *jitcode_index,
pc: *pc,
py_pc: *py_pc,
boxes: encoded,
}
})
Expand Down Expand Up @@ -2842,6 +2860,7 @@ impl TraceCtx {
frames: vec![crate::recorder::SnapshotFrame {
jitcode_index: 0,
pc: self.last_traced_pc as u32,
py_pc: self.last_traced_pc as u32,
boxes: Vec::new(),
}],
vable_boxes: Vec::new(),
Expand Down
32 changes: 25 additions & 7 deletions majit/majit-metainterp/src/optimizeopt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use std::collections::VecDeque;

pub type SnapshotBoxes = Vec<Option<Vec<SnapshotBox>>>;
pub type SnapshotFrameSizes = Vec<Option<Vec<usize>>>;
pub type SnapshotFramePcs = Vec<Option<Vec<(i32, i32)>>>;
pub type SnapshotFramePcs = Vec<Option<Vec<(i32, i32, i32)>>>;
type OpRefFxIndexMap<V> = indexmap::IndexMap<OpRef, V, FxBuildHasher>;

pub(crate) fn snapshot_get<T>(store: &[Option<T>], pos: i32) -> Option<&T> {
Expand Down Expand Up @@ -754,7 +754,7 @@ pub struct OptContext {
/// resume.py:243-247 _number_boxes consumes vref_array as a section
/// after vable_array. opencoder.py:767 records vref_boxes here.
pub snapshot_vref_boxes: SnapshotBoxes,
/// Per-guard per-frame (jitcode_index, pc) from tracing-time snapshots.
/// Per-guard per-frame (jitcode_index, pc, py_pc) from tracing-time snapshots.
pub snapshot_frame_pcs: SnapshotFramePcs,
/// optimizer.py:34 `self.inputargs = inputargs` parity.
/// Typed InputArg OpRefs; slot `i` is `OpRef::input_arg_typed(i, tp)`.
Expand Down Expand Up @@ -6244,14 +6244,32 @@ impl OptContext {
for (i, &size) in sizes.iter().enumerate() {
let end = (offset + size).min(snapshot_boxes.len());
let frame_boxes: Vec<SnapshotBox> = snapshot_boxes[offset..end].to_vec();
let (jitcode_index, pc) = frame_pcs.get(i).copied().unwrap_or((0, 0));
frames.push((jitcode_index, pc, frame_boxes));
let (jitcode_index, pc, py_pc) = frame_pcs.get(i).copied().unwrap_or((0, 0, 0));
frames.push(crate::resume::SnapshotFrame {
jitcode_index,
pc,
py_pc,
boxes: frame_boxes,
});
offset = end;
}
Snapshot::multi_frame_boxes(frames)
Snapshot {
vable_array: Vec::new(),
vref_array: Vec::new(),
framestack: frames,
}
} else {
let (jitcode_index, pc) = frame_pcs.first().copied().unwrap_or((0, 0));
Snapshot::single_frame_boxes(jitcode_index, pc, snapshot_boxes.clone())
let (jitcode_index, pc, py_pc) = frame_pcs.first().copied().unwrap_or((0, 0, 0));
Snapshot {
vable_array: Vec::new(),
vref_array: Vec::new(),
framestack: vec![crate::resume::SnapshotFrame {
jitcode_index,
pc,
py_pc,
boxes: snapshot_boxes.clone(),
}],
}
};
// pyjitpl.py:2588: vable_array stores virtualizable_boxes.
// ni/vsd are constants (TAGINT/TAGCONST) so they don't affect
Expand Down
2 changes: 1 addition & 1 deletion majit/majit-metainterp/src/optimizeopt/optimizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ pub struct Optimizer {
/// section. opencoder.py:767 create_top_snapshot records vref_boxes
/// alongside vable_boxes.
pub snapshot_vref_boxes: SnapshotBoxes,
/// Per-guard per-frame (jitcode_index, pc) from tracing-time snapshots.
/// Per-guard per-frame (jitcode_index, pc, py_pc) from tracing-time snapshots.
pub snapshot_frame_pcs: SnapshotFramePcs,
/// Phase 1 emit ops carried into Phase 2's lookup surface (6).
///
Expand Down
2 changes: 1 addition & 1 deletion majit/majit-metainterp/src/optimizeopt/unroll.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ pub struct UnrollOptimizer {
/// (resume.py:243-247 vref_array — _number_boxes consumes them
/// after the virtualizable array).
pub snapshot_vref_boxes: SnapshotBoxes,
/// Per-guard per-frame (jitcode_index, pc) from tracing-time snapshots.
/// Per-guard per-frame (jitcode_index, pc, py_pc) from tracing-time snapshots.
pub snapshot_frame_pcs: SnapshotFramePcs,
/// pyjitpl.py:2289 all_descrs: dense list indexed by descr_index.
/// Threaded through inner Optimizer instances for inline registration.
Expand Down
5 changes: 3 additions & 2 deletions majit/majit-metainterp/src/pyjitpl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,10 +552,10 @@ fn snapshot_map_from_trace_snapshots(
// AND vref_array. resume.py:243-247 _number_boxes consumes
// vref_array as a separate section after vable_array.
let vref_boxes: Vec<SnapshotBox> = snap.vref_boxes.iter().map(&tagged_to_box).collect();
let frame_pcs: Vec<(i32, i32)> = snap
let frame_pcs: Vec<(i32, i32, i32)> = snap
.frames
.iter()
.map(|f| (f.jitcode_index as i32, f.pc as i32))
.map(|f| (f.jitcode_index as i32, f.pc as i32, f.py_pc as i32))
.collect();
let id = id as i32;
snapshot_insert(&mut box_map, id, boxes);
Expand Down Expand Up @@ -20465,6 +20465,7 @@ mod tests {
frames: vec![crate::recorder::SnapshotFrame {
jitcode_index: 0,
pc: 123,
py_pc: 123,
boxes: vec![crate::recorder::SnapshotTagged::Box(
OpRef::int_op(0),
majit_ir::Type::Int,
Expand Down
1 change: 1 addition & 0 deletions majit/majit-metainterp/src/pyjitpl/dispatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8036,6 +8036,7 @@ pub fn build_state_field_snapshot(
snapshot_frames.push(crate::recorder::SnapshotFrame {
jitcode_index,
pc: frame.pc as u32,
py_pc: frame.pc as u32,

@coderabbitai coderabbitai Bot Jul 22, 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 | 🔴 Critical | 🏗️ Heavy lift

Store the forward-mapped Python PC, not frame.pc.

frame.pc is already the JIT/resume PC stored on Line 8038. Copying it into py_pc collapses two distinct coordinate systems, so guard-failure reconstruction may resume the interpreter at the wrong Python bytecode location—especially for inlined frames. Use the frame value populated from codewriter marker data instead, preserving -1 for synthetic frames where required.

As per coding guidelines, the generated JIT must preserve interpreter semantics; a coordinate mismatch is a generation defect, not an acceptable porting difference.

🤖 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-metainterp/src/pyjitpl/dispatch.rs` at line 8039, Update the
guard-failure frame reconstruction around the py_pc assignment to store the
forward-mapped Python PC from the codewriter marker data rather than frame.pc,
which is the JIT/resume PC. Preserve the existing -1 value for synthetic frames
and keep the separate JIT PC assignment unchanged.

Source: Coding guidelines

boxes,
});
}
Expand Down
3 changes: 3 additions & 0 deletions majit/majit-metainterp/src/recorder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ pub struct SnapshotFrame {
/// context; the runtime translates `py_pc` through `pc_map` at resume
/// time until pyre's walker-as-tracer epic lands.
pub pc: u32,
/// Forward-carried Python instruction PC for this JitCode position.
/// `u32::MAX` is the no-snapshot sentinel paired with `pc == -1`.
pub py_pc: u32,
/// Tagged references to the live boxes in this frame.
pub boxes: Vec<SnapshotTagged>,
}
Expand Down
Loading
Loading