Repository navigation
jit: pcmap audit retirement — GUARDCAP + RESULT harnesses, inject trivia-twin cutover - #626
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe JIT dispatch code no longer captures or emits guard-capture audit data, compares guard and table depths, or asserts F3 selector consistency. Mid-body auditing remains unchanged. ChangesGuard-capture audit removal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 1c59993). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
youknowone
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63972ffefd
ℹ️ 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".
| let Some(result_color) = payload | ||
| .result_color_trivia_for_jitcode_pc(root_pc) | ||
| .map(|c| c as usize) | ||
| .filter(|&c| c != u16::MAX as usize) |
There was a problem hiding this comment.
When running the result-color census, this commit removes PYRE_PCMAP_RESULT_AUDIT entirely while this site is switching inject_root_call_result to the JitCode-PC trivia reader. The commit text says RESULT remains to certify a live legacy seam, but after this change setting PYRE_PCMAP_RESULT_AUDIT silently does nothing, so divergences between the new JitCode-PC reader and the Python-PC table around this migration will no longer be asserted. Please keep the result-color gate until the intended result audit retirement lands.
Useful? React with 👍 / 👎.
The guard-capture depth twin census reports 10808/10808 eq on both the plain and PYRE_P2_COMPILE dynasm corpus legs; production already reads the depth twins. Remove the gate, the probe writer, the depth comparison block, and the F3 selector assert. Assisted-by: Claude Assisted-by: Codex
Replace the backxlat_py_pc + result_color_at_pc_at production read with the codewriter-baked result-color trivia twin keyed by the carried root JitCode pc, declining on a missing or u16::MAX entry. The legacy comparison stays under PYRE_PCMAP_RESULT_AUDIT: twin vs backxlat consumer asserted equal over 1293 PYRE_P2_COMPILE fires (dynasm 216/216); the audit-on plain leg is green with zero fires. Assisted-by: Claude Assisted-by: Codex
Every production result-color read now uses the JitCode-pc-keyed twins; the four audit assert blocks (bridge-root parent, both inline-caller variants, inject_root_call_result) and the gate function are the last consumers of the comparison and are removed. The asserts held over 1293 PYRE_P2_COMPILE inject fires and the full audit-on corpus legs before removal. Assisted-by: Claude Assisted-by: Codex
…ia twin recipe_parent_frame_from_recipe and reconstruct_inline_recipe read the result-color slot through backxlat_py_pc + result_color_at_pc_at. Key both off the carried JitCode pc via result_color_trivia_for_jitcode_pc instead, dropping the py inversion. A whole-metadata verifier compared the twin against the legacy reader at every block-head resume coordinate of every jitcode the corpus builds — 9992 coordinates across 426 jitcodes, zero divergence — so the reads are unchanged. The now twin-vs-twin recipe_parent_result_color census block is removed; the shared recipe-resultcolor gate stays for the multi-frame callee site. Assisted-by: Claude Assisted-by: Codex
Summary
Rebased onto origin/main (#607 P2-bridge-compile default-ON) and closing out the pcmap audit scaffolding + the certifiable legacy
result_color_at_pc_atconsumers.PYRE_PCMAP_GUARDCAP_AUDIT— guard-capture depth twin census 10,808/10,808 eq on plain + P2 dynasm legs; production already on the depth twins (−74).residual_ref_call_dst_before(byte-native dst-register decode) the primary; the.or_elsemarker-miss fallback now reads the result-color trivia twin instead ofbackxlat_py_pc+result_color_at_pc_at.PYRE_PCMAP_RESULT_AUDIT— every production result-color read on the JitCode-pc twins; the four assert blocks + gate removed (−68).recipe_parent_frame_from_recipeandreconstruct_inline_recipekeyed off the carried JitCode pc viaresult_color_trivia_for_jitcode_pc, dropping the py inversion.Certification of commit 4 (the dormant-consumer flip)
Both consumers are corpus-dormant (0 fires even under
PYRE_P2_COMPILE), so a runtime fire census cannot certify them. Instead a throwaway whole-metadata verifier compared the trivia twin against the legacyresult_color_at_pc[backxlat_py_pc(off)]reader at every block-head resume coordinate of every jitcode the corpus builds: 9,992 coordinates across 426 jitcodes, zero divergence (control confirmed the harness performed all 9,992 comparisons — up to 266 per jitcode — not a silent no-op). The reads are structurally unchanged.The two inline-caller marker-miss fallbacks (
compute_inline_caller_frame/compute_nested_inline_caller_frame) stay: whenresume_marker_jit_pcis None there is no JitCode key for the twin, so they remain the residual keyless floor; the py-keyedresult_color_at_pctable cannot be fully retired until those (and the build-time twin builder) are addressed.Verification
check.pyafter each stage: dynasm 217/217, cranelift 217/217, wasm 216/216cargo test -p pyre-jit-trace -p pyre-jit --features dynasm: green🤖 Generated with Claude Code