jit: #369 pc-word flip — resume frame collapses to RPython 2-word shape

[youknowone]

Summary

Collapses the JIT resume frame's dual coordinate into RPython's single-pc shape.
Before this stack the pyre resume frame carried both the Python bytecode pc
word and a redundant jitcode_pc twin side-channel (a per-frame 3rd rd_numb
word). RPython's resume.py numbers 2 words/frame (jitcode_index, pc)
where pc is the JitCode byte offset. This stack makes the pc word the JitCode
offset and physically removes the twin.

Phases

• P1 (encode completion) — every production resume frame now carries a real
  jitcode coordinate or is skeleton; deleted the production-dead portal-bridge
  PyJitCode mode (no undefined-coordinate frame class remains).
• P3 (losslessness census) — certified back-translation jitcode→py at the
  three python-resume readers; the flip-obstruction rows were all shown benign
  (single-frame guards whose exact resume pc survives in the virtualizable
  last_instr).
• P4 (the flip) — encode writers store the resolved JitCode offset into the
  pc word; decode py_pc-readers back-translate; single-frame resume_pc sourced
  from vable ni. Landed gated, then flipped default-ON (s3). Full A/B
  green with the gate OFF and ON.
• P5 (collapse) — removed the gate and audit scaffolding (s0/s1),
  collapsed the decode callbacks and readers onto the pc word (s2/s3a),
  derived the bridge-resume gates and recipe/carrier twin sources from frame.pc
  (s3b/s3c), and finally deleted the rd_numb 3rd word (s5): the frame
  header is now (jitcode_index, pc). Both encoders and all three decoders drop
  the twin word in lockstep; SnapshotFrame.jitcode_pc / RebuiltFrame.jitcode_pc
  and the *_with_jitcode_pc constructors are gone. The encode-side twin
  computation is retained — it still feeds resolve_resume_pc_with_jitcode_pc,
  which bakes the resume coordinate into the surviving pc word.

Verification

• pyre/check.py --backend dynasm 180/180, --backend cranelift 180/180.
• cargo test -p pyre-jit-trace 279/0-fail, cargo test -p majit-metainterp 1465/0-fail.
• cargo fmt --check + git diff --check clean.

Follow-up (not in this PR)

S4 — deleting the pyre-side ReconstructRecipe.jitcode_pc and
BridgeInlineCarrier.root_jitcode_pc twins — is deferred. It is not
byte-identical and has two genuine blockers (carrier root_pc offset consumed
as py_pc; recipe.pc is a py_pc and the twin is the recipe's only offset),
so it needs a real offset field on the recipe first and its own census + A/B.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Improvements

  • Simplified resume-state metadata and frame-position handling for more consistent JIT resume and guard-recovery behavior.
  • Improved translation between JIT offsets and interpreter positions during frame reconstruction.
  • Unified stack-depth, liveness, and snapshot handling across resume paths.

• Bug Fixes

  • Corrected frame-value liveness calculations and encoded resume-data offsets.
  • Improved restoration of guard failures and blackhole execution state.

• Cleanup

  • Removed obsolete portal-bridge behavior and diagnostic audit logging.

[coderabbitai]

Walkthrough

Changes

Resume coordinate simplification

Layer / File(s)	Summary
Resume-data contracts and encoding majit/majit-ir/..., majit/majit-metainterp/src/{resume,history,recorder,compile}.rs, majit/majit-metainterp/tests/*	Resume frames, callbacks, snapshots, encoded layouts, and tests no longer carry jitcode_pc.
Trace snapshot coordinate construction pyre/pyre-jit-trace/src/{jitcode_dispatch,trace,trace_opcode}.rs	Snapshot PC words and stack depths now use direct JitCode resolution and concrete frame state.
JitCode coordinate and bridge resolution pyre/pyre-jit-trace/src/state.rs	Pc-only translation, liveness, semantic-map, constant-slot, and inline-reconstruction helpers replace carried-coordinate paths.
JitCode mode and audit cleanup pyre/pyre-jit-trace/src/{canonical_bridge,lib,liveness,pyjitcode}.rs	Portal-bridge construction and related audit and diagnostic plumbing are removed or updated.
Runtime resume reconstruction pyre/pyre-jit/src/{call_jit,eval}.rs	Blackhole and guard-failure paths resolve and back-translate resume coordinates using (jitcode_index, pc).

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Trace
  participant ResumeData
  participant Runtime
  participant PyJitCode
  Trace->>ResumeData: encode jitcode_index and pc
  ResumeData->>Runtime: decode frame coordinates
  Runtime->>PyJitCode: resolve resume PC and liveness
  PyJitCode-->>Runtime: return resolved frame state

Loading

Possibly related issues

• youknowone/pyre#369 — Directly covers removal of the carried jitcode_pc side channel across resume-data and runtime paths.
• youknowone/pyre#368 — Addresses the coupled resume-coordinate and jitcode_pc plumbing changes.

Possibly related PRs

• youknowone/pyre#365 — Overlaps in removing carried JitCode-PC data from resume-coordinate resolution.
• youknowone/pyre#455 — Touches the same resume-data encoding and callback interpretation.
• youknowone/pyre#501 — Shares changes to blackhole and resume-data reconstruction signatures.

Suggested reviewers: lifthrasiir

Poem

A rabbit hops through frames of code,
And drops the carried PC load.
Two numbers guide the trail anew,
Liveness follows, crisp and true.
The bridge sheds audits, light and bright—
Resume paths now leap just right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: collapsing resume frames from three words to the RPython two-word shape.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rewrite-tracer

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 04ad46b).

Files in the reviewed diff

majit/majit-backend-cranelift/src/compiler.rs
majit/majit-ir/src/resumedata.rs
majit/majit-metainterp/src/blackhole.rs
majit/majit-metainterp/src/compile.rs
majit/majit-metainterp/src/history.rs
majit/majit-metainterp/src/jitdriver.rs
majit/majit-metainterp/src/lib.rs
majit/majit-metainterp/src/optimizeopt/mod.rs
majit/majit-metainterp/src/pyjitpl.rs
majit/majit-metainterp/src/pyjitpl/dispatch.rs
majit/majit-metainterp/src/recorder.rs
majit/majit-metainterp/src/resume.rs
majit/majit-metainterp/tests/jit_driver_runtime_parity.rs
majit/majit-metainterp/tests/resume_parity.rs
pyre/pyre-jit-trace/src/canonical_bridge.rs
pyre/pyre-jit-trace/src/jitcode_dispatch.rs
pyre/pyre-jit-trace/src/lib.rs
pyre/pyre-jit-trace/src/liveness.rs
pyre/pyre-jit-trace/src/pyjitcode.rs
pyre/pyre-jit-trace/src/state.rs
pyre/pyre-jit-trace/src/trace.rs
pyre/pyre-jit-trace/src/trace_opcode.rs
pyre/pyre-jit/src/call_jit.rs
pyre/pyre-jit/src/eval.rs

1. Regressions to PyPy parity introduced by this patch

• pyre/pyre-jit/src/eval.rs:7425 ↔ rpython/jit/metainterp/resume.py:1049: single-frame guard recovery now selects ni and discards the decoded snapshot PC. RPython always resumes each rebuilt frame at the encoded pc (f.setup_resume_at_op(pc)). This regresses main’s behavior, which used the decoded innermost snapshot PC whenever it differed from ni; a stale virtualizable last_instr can now resume a single-frame bridge at the loop header rather than the guard’s recorded position.

2. Other mismatches introduced by this patch

• pyre/pyre-jit-trace/src/trace_opcode.rs:4131 ↔ rpython/jit/metainterp/resume.py:249, with equivalent new fallbacks at pyre/pyre-jit-trace/src/trace_opcode.rs:4081, pyre/pyre-jit-trace/src/jitcode_dispatch.rs:11130, 11894, 11929, 12483, and 12495: after changing the serialized pc word to mean a JitCode offset, failed translation falls back to serializing the original Python PC (unwrap_or(...py_pc)). RPython’s numbering always stores a valid JitCode PC and the reader passes it directly to setposition/setup_resume_at_op; a Python instruction index is not a valid substitute and can select different liveness or fail JitCode startpoint assertions.

3. Pre-existing mismatches (already present before this patch)

• pyre/pyre-jit/src/eval.rs:8029 ↔ pypy/interpreter/pyframe.py:128: every rebuilt inline section is assigned frame_ptr = vable_frame_ptr, collapsing all resumed frames onto the portal frame. PyPy retains a separate frame whose get_w_globals() comes from that frame; this loses callee frame identity.

• pyre/pyre-jit/src/eval.rs:8059 ↔ pypy/interpreter/pyframe.py:128: inner resumed frames explicitly use vable_ns (“Inner frames share the chain virtualizable’s namespace”) instead of their own frame globals. Thus an inlined callee’s LOAD_GLOBAL can resolve in the caller namespace.

• pyre/pyre-jit-trace/src/trace_opcode.rs:4316 ↔ rpython/jit/metainterp/virtualizable.py:86: virtualizable snapshot slots reuse frame-register OpRefs, so numbering deduplicates them. RPython’s read_boxes() creates distinct boxes with wrap() for every virtualizable field/array item.

• pyre/pyre-jit-trace/src/trace_opcode.rs:4553 ↔ rpython/jit/metainterp/virtualizable.py:94: an unavailable virtualizable array slot is encoded as NULL rather than as the actual wrapped field value. RPython serializes every array element; the later pyre-only refetch is not part of resume-data semantics.

• pyre/pyre-jit-trace/src/trace_opcode.rs:2510 ↔ pypy/module/pypyjit/interp_jit.py:87: pyre publishes last_instr = orgpc - 1, while PyPy’s JIT driver uses next_instr as the current dispatch coordinate. The source itself documents this as an unresolved semantic mismatch; it was present before this patch.

4. Structural adaptations

• pyre/pyre-jit-trace/src/trace_opcode.rs:4128 ↔ rpython/jit/metainterp/resume.py:249: converting CPython instruction coordinates to generated JitCode offsets before numbering is a necessary compiler-boundary adaptation. It restores the canonical two-word resume-frame layout (jitcode_index, jitcode_pc) where resolution succeeds.

• majit/majit-ir/src/resumedata.rs:528 ↔ rpython/jit/metainterp/resume.py:928: removing pyre’s third per-frame jitcode_pc word is structurally aligned with RPython’s read_jitcode_pos_pc(), which reads exactly two items.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40f83ed432

ℹ️ 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".

[chatgpt-codex-connector]

Translate root pc before building bridge carrier

When resume_data.frames.len() > 1, this root_pc is later consumed as a Python bytecode PC by the bridge-carrier path (e.g. trace_bytecode sets last_instr from it, and drive_outer_continuation_and_map calls result_color_at_pc_at / resume_jitcode_pc_for(root_pc)). After this change frame.pc is the JitCode byte offset, so multi-frame guard failures now root the caller continuation at a JitCode offset whenever that offset differs from the Python PC. Please back-translate this value for root_pc and keep the raw offset only in root_jitcode_pc.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@pyre/pyre-jit-trace/src/state.rs`:
- Around line 764-797: Guard negative inputs before the i32-to-usize conversions
in python_pc_for_jitcode_pc_public and skip_python_trivia_forward_public,
returning None for values below zero. Update backxlat_py_pc to detect a negative
pc_word and return its existing decoded fallback immediately, preserving the
current fallback behavior for lookup failures.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 703afaf9-8cd9-4d7b-8e79-00bf159a67fd

📥 Commits

Reviewing files that changed from the base of the PR and between 51a2c2c and 40f83ed.

📒 Files selected for processing (24)

• majit/majit-backend-cranelift/src/compiler.rs
• majit/majit-ir/src/resumedata.rs
• majit/majit-metainterp/src/blackhole.rs
• majit/majit-metainterp/src/compile.rs
• majit/majit-metainterp/src/history.rs
• majit/majit-metainterp/src/jitdriver.rs
• majit/majit-metainterp/src/lib.rs
• majit/majit-metainterp/src/optimizeopt/mod.rs
• majit/majit-metainterp/src/pyjitpl.rs
• majit/majit-metainterp/src/pyjitpl/dispatch.rs
• majit/majit-metainterp/src/recorder.rs
• majit/majit-metainterp/src/resume.rs
• majit/majit-metainterp/tests/jit_driver_runtime_parity.rs
• majit/majit-metainterp/tests/resume_parity.rs
• pyre/pyre-jit-trace/src/canonical_bridge.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch.rs
• pyre/pyre-jit-trace/src/lib.rs
• pyre/pyre-jit-trace/src/liveness.rs
• pyre/pyre-jit-trace/src/pyjitcode.rs
• pyre/pyre-jit-trace/src/state.rs
• pyre/pyre-jit-trace/src/trace.rs
• pyre/pyre-jit-trace/src/trace_opcode.rs
• pyre/pyre-jit/src/call_jit.rs
• pyre/pyre-jit/src/eval.rs

💤 Files with no reviewable changes (6)

• pyre/pyre-jit-trace/src/lib.rs
• pyre/pyre-jit-trace/src/canonical_bridge.rs
• majit/majit-metainterp/tests/jit_driver_runtime_parity.rs
• majit/majit-metainterp/src/recorder.rs
• majit/majit-metainterp/src/lib.rs
• majit/majit-metainterp/src/pyjitpl/dispatch.rs

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all call sites of the new pc-translation helpers pass a
# non-negative/already-validated pc word, and inspect the callee
# implementations' tolerance for out-of-range offsets.
rg -nP -C4 '\b(backxlat_py_pc|python_pc_for_jitcode_pc_public|skip_python_trivia_forward_public)\s*\(' --type=rust
rg -nP -C6 'fn (python_pc_for_jitcode_pc|skip_python_trivia_forward)\s*\(' --type=rust pyre/pyre-jit-trace/src/jitcode_dispatch.rs

Repository: youknowone/pyre

Length of output: 153

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant region and nearby call sites.
printf '\n== state.rs outline ==\n'
ast-grep outline pyre/pyre-jit-trace/src/state.rs --view expanded | sed -n '1,220p'

printf '\n== helper call sites ==\n'
rg -n -C 3 'backxlat_py_pc|python_pc_for_jitcode_pc_public|skip_python_trivia_forward_public' pyre/pyre-jit-trace/src

printf '\n== jitcode_dispatch helper definitions ==\n'
rg -n -C 6 'fn (python_pc_for_jitcode_pc|skip_python_trivia_forward)\s*\(' pyre/pyre-jit-trace/src/jitcode_dispatch.rs

Repository: youknowone/pyre

Length of output: 15342

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Focused probe for the pc-translation path and data-flow around it.
sed -n '760,805p' pyre/pyre-jit-trace/src/state.rs
printf '\n== nearby reconstruct_inline_recipe call site ==\n'
sed -n '5808,5830p' pyre/pyre-jit-trace/src/state.rs
printf '\n== all references to can_decode_live_vars in state.rs ==\n'
rg -n -C 2 'can_decode_live_vars|resolve_resume_pc_with_jitcode_pc|frame_pc_is_resolved_offset_at|frame_value_count_at|bridge_semantic_maps_at_with_jitcode_pc|const_ref_slots_at_pc_at' pyre/pyre-jit-trace/src/state.rs

Repository: youknowone/pyre

Length of output: 9971

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify the suspected Rust cast behavior with a tiny standalone probe.
python3 - <<'PY'
import sys
vals = [-1, -2, 0, 1, 2147483647]
for v in vals:
    u = v & ((1 << (8 * sys.getsizeof(0) - 1)) - 1)
    print(v)
PY

Repository: youknowone/pyre

Length of output: 174

---

Guard negative pc words before converting to usize.

python_pc_for_jitcode_pc_public and skip_python_trivia_forward_public cast i32 inputs to usize without a sign check. A negative pc_word/raw_py_pc wraps to a huge offset before reaching jitcode_dispatch, so backxlat_py_pc should reject < 0 itself instead of relying on every caller to pre-screen. Keep the fallback path, but return it early for negative input.

Proposed fix

 pub fn backxlat_py_pc(jitcode_index: i32, pc_word: i32) -> i32 {
     let fallback = majit_ir::resumedata::decode_resume_pc(pc_word).0;
+    if pc_word < 0 {
+        return fallback;
+    }
     python_pc_for_jitcode_pc_public(jitcode_index, pc_word)
         .and_then(|raw_py_pc| skip_python_trivia_forward_public(jitcode_index, raw_py_pc))
         .map(|(py_pc, _)| py_pc)
         .unwrap_or(fallback)
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	/// Resolve a stored JitCode offset back to its Python instruction coordinate.
	pub fn python_pc_for_jitcode_pc_public(jitcode_index: i32, offset: i32) -> Option<i32> {
	let payload = pyjitcode_for_jitcode_index(jitcode_index)?;
	Some(
	crate::jitcode_dispatch::python_pc_for_jitcode_pc(&payload.metadata, offset as usize)
	as i32,
	)
	}
	/// Advance a Python instruction coordinate past resume trivia when code is available.
	pub fn skip_python_trivia_forward_public(
	jitcode_index: i32,
	raw_py_pc: i32,
	) -> Option<(i32, bool)> {
	let payload = pyjitcode_for_jitcode_index(jitcode_index)?;
	if payload.code_ptr.is_null() {
	return Some((raw_py_pc, false));
	}
	let code = unsafe { &*payload.code_ptr };
	Some((
	crate::jitcode_dispatch::skip_python_trivia_forward(code, raw_py_pc as usize) as i32,
	true,
	))
	}
	/// Translate a resume-frame pc word to a Python instruction coordinate.
	pub fn backxlat_py_pc(jitcode_index: i32, pc_word: i32) -> i32 {
	let fallback = majit_ir::resumedata::decode_resume_pc(pc_word).0;
	python_pc_for_jitcode_pc_public(jitcode_index, pc_word)
	.and_then(|raw_py_pc| skip_python_trivia_forward_public(jitcode_index, raw_py_pc))
	.map(|(py_pc, _)| py_pc)
	.unwrap_or(fallback)
	}
	/// Translate a resume-frame pc word to a Python instruction coordinate.
	pub fn backxlat_py_pc(jitcode_index: i32, pc_word: i32) -> i32 {
	let fallback = majit_ir::resumedata::decode_resume_pc(pc_word).0;
	if pc_word < 0 {
	return fallback;
	}
	python_pc_for_jitcode_pc_public(jitcode_index, pc_word)
	.and_then(|raw_py_pc| skip_python_trivia_forward_public(jitcode_index, raw_py_pc))
	.map(|(py_pc, _)| py_pc)
	.unwrap_or(fallback)
	}

🤖 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 `@pyre/pyre-jit-trace/src/state.rs` around lines 764 - 797, Guard negative
inputs before the i32-to-usize conversions in python_pc_for_jitcode_pc_public
and skip_python_trivia_forward_public, returning None for values below zero.
Update backxlat_py_pc to detect a negative pc_word and return its existing
decoded fallback immediately, preserving the current fallback behavior for
lookup failures.