Repository navigation
jit: exempt LOAD_FAST_CHECK's null arm from the loop-body abort_permanent scan, and give markers an exact jitcode -> Python-PC inverse- #1023 - #1032
Merged
Conversation
`loop_body_abort_permanent_pc` scans to `code.len()` instead of the loop's back edge when the loop header is covered by an exception handler. No synthetic fixture responded to that widening: sweeping all 356 with the scan narrowed left `loops_compiled`, `bridges_compiled`, `loops_aborted` and `guard_failures` unchanged on every one. Three fixtures for the shape. Each has a hot loop inside a `try` and a post-loop tail that reads the loop variable, which the compiler spells LOAD_FAST_CHECK because the raise can reach the handler before the loop assigns it. LOAD_FAST_CHECK splits on a conditionally-bound slot and its null arm emits the `abort_permanent` the widened scan finds; the marker sits in a dead-end block reached only when the local really is unbound, and `unbound_tail` is the arm that reaches it. Delivery flavours covered: builtin container, int-specialized arithmetic guard, non-matching inner handler, inlined callee, a raise in the post-loop tail, a second loop after the handler, and two loops under one `try`. `PYRE_FBW_LOOPBODY_SCAN_LOOP_ONLY` drops the widening without a rebuild, the counterpart of the existing `PYRE_FBW_LOOPBODY_SCAN_FULL`. With it set, all three fixtures fail `jit-stats regression: guard_failures 20 -> 40/60/80`. Output is byte-identical across pypy3, python3.14 and all three backends, and the recorded baselines agree field-for-field across dynasm, cranelift and wasm. Assisted-by: Claude
`loop_body_abort_permanent_pc`'s doc comment names "the `DELETE_FAST` cleanup for `except X as e`" as the post-loop marker the widened scan catches. Across the shapes `bench/synth/loop_in_try_*` cover, every marker it catches is `LOAD_FAST_CHECK`'s null arm instead. A loop inside a `try` is what makes the tail read a loop variable through LOAD_FAST_CHECK rather than LOAD_FAST — the raise can reach the handler before the loop assigns the slot, so the slot is only conditionally bound on the join. The codewriter splits that opcode, compiles the bound arm, and sends the null arm to a dead-end block whose `abort_permanent` no bound-local run reaches. Also records why the marker cannot simply be exempted: the null arm's block is emitted after the whole body, and `py_floor_by_jit_pc` keys each Python PC to its first jitcode offset, so the floor lookup attributes the late block to `RETURN_VALUE` rather than to the LOAD_FAST_CHECK that owns it. Comment only; no behaviour change. Assisted-by: Claude
`PyErrorKind::GeneratorReturn` had six references in the tree: the variant, its `ExcKind::RuntimeError` mapping, three handlers in `eval_loop` / `handle_exception`, and a comment. No constructor. Nothing ever raised it, so the handlers were unreachable. `Instruction::ReturnGenerator` does not unwind: `return_generator` (eval.rs:4375) is a bare `push(w_none())` for the following POP_TOP, because the generator object is already built at call time in `call_user_function_with_eval`. Verified with cargo test --all --no-default-features --features dynasm (7396 tests), check.py --synthetic-only --backend dynasm (359/359), and the cpython_tests dynasm legs for test_generators, test_yield_from, test_coroutines, test_generator_stop and test_exceptions (no regressions). Assisted-by: Claude
…nent scan `loop_body_abort_permanent_pc` declined a whole frame on any `abort_permanent` in its scan window. `LOAD_FAST_CHECK` lowers to a `ptr_nonzero` split whose null arm ends in a dead-end block carrying that marker, so the scan refused frames over a conditionally-bound local read. Skip markers whose owning opcode is `LoadFastCheck`. Naming the owning opcode needs an exact jitcode-offset -> Python-PC inverse: `py_floor_by_jit_pc` keys each Python PC to its FIRST jitcode offset, and the null arm's block is emitted after the whole body, so the floor lookup resolves it to whichever PC last opened a segment. Record every `abort_permanent`'s `insns` index in `SSARepr` at `serialize_op`, carry it through the three `-live-` splicing passes and the `remove_repeated_live` remap, translate it in the `finish_with_positions_from` batch alongside `pc_map`, and publish it as `PyJitCodeMetadata::abort_permanent_py_pc_by_jit_pc`. `SSARepr::stream_positions_mut` replaces the per-table position rewrite loops in the splicing passes. Re-recorded the three `loop_in_try_*` baselines on all three backends: `loops_compiled` 1 -> 3/4/5, `guard_failures` 20 -> 60/80/100, identical across dynasm/cranelift/wasm. Output stays byte-exact against pypy3 and python3.14. Min-of-9 user+sys CPU, dynasm: 0.65s -> 0.19s, 1.33s -> 0.20s, 0.97s -> 0.18s. Assisted-by: Claude
Drops the separate `wasm-build` matrix job. The Linux job that runs pyre/check.py now builds `-p pyre-wasm --features web` before check.py's own wasm-host build, so both bindings share the release target directory instead of paying a second checkout, cache restore and LLBC download. Assisted-by: Claude
…W_DEBUG_ABORT The `[fbw-abort] start_pc=… abort_permanent_pc=…` line reported a bare jitcode offset. Resolve it through `abort_permanent_py_pc_by_jit_pc` and append `py_pc=<n> <Instruction>`. `marker_is_load_fast_check_null_arm`'s table lookup and CodeObject decode move into `abort_permanent_owner`, which both callers share; the predicate becomes a `matches!` on its result. The line only prints under `PYRE_FBW_DEBUG_ABORT`, which check.py never sets, and no fixture in the synthetic suite currently reaches this decline. Assisted-by: Claude
youknowone
marked this pull request as ready for review
August 4, 2026 13:50
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (20)
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 9790fef). The diff against the review base is empty; nothing to review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Self-review
Assisted-by