jit: the inlined-callee frame image and the escape attribution behind VableEscapedDuringResidualCall - #1015
Conversation
|
Warning Review limit reached
Next review available in: 20 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 (13)
WalkthroughThe PR adds ChangesInline callee frame handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ResidualCall
participant InlineCalleeFrame
participant VableState
participant EscapeDiagnostics
ResidualCall->>InlineCalleeFrame: resolve and publish callee Python PC
ResidualCall->>VableState: prepare virtualizable and virtual-reference state
VableState->>InlineCalleeFrame: restore frame slots after escape
ResidualCall->>EscapeDiagnostics: classify portal and published-callee escapes
EscapeDiagnostics->>InlineCalleeFrame: attribute the subsequent force
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 0a8520d4f8
ℹ️ 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".
| // forcing also resets the token before writing fields | ||
| // (`resume.py:1405-1408`). Therefore a matched guard reports the | ||
| // escape even when the flush declined. | ||
| return true; |
There was a problem hiding this comment.
When an inlined callee exposes its published frame but the full and locals-only flushes decline (for example, because a live virtualizable entry has no concrete value), this unconditional true now clears the portal token and raises VableEscapedDuringResidualCall without committing a resume PC. If build_multi_frame_miframe also cannot reconstruct the missing state, the epilogue falls back to replaying from the walk entry even though the residual call already ran, so a non-idempotent callee can apply its effects twice. Keep redirected callee escapes conditional on a committed/constructible forward-resume image rather than forcing first and hoping reconstruction succeeds.
AGENTS.md reference: AGENTS.md:L24-L30
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 05df83c). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@AGENTS.md`:
- Around line 204-206: Specify the shell language on the fenced code block
containing the PYPYLOG command by changing its opening fence to use the sh
identifier, while leaving the command and closing fence unchanged.
In `@pyre/pyre-wasm-runner/src/main.rs`:
- Line 546: Update the standard MAJIT_STATS emission path alongside RING_BASE so
it reads slots 6–10 and publishes the fbw_escape_split and fbw_force_attrib
counters consistently with native output. Require the diagnostic export used for
these counters and fail when it is unavailable; do not limit this handling to
the verbose PYRE_WASM_JIT_STATS reader.
🪄 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 Plus
Run ID: 0cb53639-5320-4c73-a3cb-06f8c7d8a572
📒 Files selected for processing (13)
AGENTS.mdpyre/bench/frame_inlined_callee_own_image_regression.pypyre/check.pypyre/pyre-interpreter/src/module/sys/vm.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rspyre/pyre-jit-trace/src/trace.rspyre/pyre-jit/src/eval.rspyre/pyre-jit/src/lib.rspyre/pyre-wasm-runner/src/main.rspyre/pyrex/src/lib.rs
| ``` | ||
| PYPYLOG=jit-summary:- pypy3 pyre/bench/synth/<fixture>.py | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the fenced code-block language.
Line 204 starts a shell command block without a language identifier. Add sh to satisfy markdownlint.
Proposed fix
-```
+```sh
PYPYLOG=jit-summary:- pypy3 pyre/bench/synth/<fixture>.py</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@AGENTS.md` around lines 204 - 206, Specify the shell language on the fenced
code block containing the PYPYLOG command by changing its opening fence to use
the sh identifier, while leaving the command and closing fence unchanged.
Source: Linters/SAST tools
| // `pyre_jit_trace::trace::fbw_diag`. | ||
| if let Ok(fbw) = instance.get_typed_func::<u32, u64>(&mut store, "pyre_fbw_diag") { | ||
| const RING_BASE: u32 = 6; | ||
| const RING_BASE: u32 = 11; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Publish the new FBW counters under Wasm MAJIT_STATS.
Line 546 only updates the verbose PYRE_WASM_JIT_STATS reader. The standard MAJIT_STATS block does not read slots 6-10, so Wasm output omits fbw_escape_split and fbw_force_attrib while native output publishes them. Emit the same records from the MAJIT_STATS path and fail if the required diagnostic export is unavailable.
🤖 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-wasm-runner/src/main.rs` at line 546, Update the standard
MAJIT_STATS emission path alongside RING_BASE so it reads slots 6–10 and
publishes the fbw_escape_split and fbw_force_attrib counters consistently with
native output. Require the diagnostic export used for these counters and fail
when it is unavailable; do not limit this handling to the verbose
PYRE_WASM_JIT_STATS reader.
`PYPYLOG=jit-summary:- pypy3 <fixture>` runs most `pyre/bench/synth` scripts unmodified, so a behavioural "is this ours or upstream's?" question is one command rather than a source-reading exercise. The section names the counters to read, says a differing counter is a pointer to a JIT hint rather than the answer itself, and records the case that prompted it: the `getframe_inline_subwalk_multiframe` GUARD_NOT_FORCED count, held to be unfixable by construction, against an oracle reporting `forcings: 0` / `virtualizables forced: 0` and `pypy/module/sys/vm.py:41`'s `@jit.look_inside_iff(lambda space, depth: jit.isconstant(depth))`. The "When in doubt" grep under data-structure parity now points at it for behavioural questions. Assisted-by: Claude
`pypy/module/sys/vm.py` has two functions: `:30 _getframe`, carrying `@unwrap_spec(depth=int)` and the negative-depth ValueError, over `:42 getframe`, carrying the non-hidden chain walk and `@jit.look_inside_iff(lambda space, depth: jit.isconstant(depth))`. Pyre had one anonymous closure holding both. Split into `sys_getframe` and `getframe`, and add `is_builtin_getframe_function` in the shape of `builtins::is_builtin_len_function`. The registration was a closure, so no identity predicate could name it; a named `fn` is the prerequisite for any walker arm that wants to reproduce the walk rather than call it. Behaviour is unchanged, deliberately: the walk keeps both virtualizable forces (`gettopframe()` before it, `force_frame` after), which upstream does not take because a constant depth is traced through instead. Their cost is now recorded where they are: each call trips `vable_after_residual_call`, which is 138 of the synth corpus's 219 `loops_aborted`, against `abort: vable escape: 0` and `forcings: 0` on the same fixtures under real pypy3. The doc comment also records why a constant-depth fold does not reclaim those 138 -- 70 are inside inlined callees whose virtual frame still carries `last_instr = -1`, so folding would compile a `_getframe().f_lineno` that reports the `def` line where the abort today falls back and answers correctly. `cargo test --all --no-default-features --features dynasm` green; `check.py --backend dynasm` 370/370; the getframe fixtures' abort counts and `PYRE_NO_JIT=1` outputs are unchanged. Assisted-by: Claude
`frame_lineno_mid_replay_regression` reads through `sys._getframe(1)` from a callee whose caller is the virtualizable. The new guard reads through `sys._getframe(0)` from a callee that is itself inlined, so the frame handed out is materialised through a `jit.virtual_ref` rather than being the virtualizable: it carries the `last_instr = -1` sentinel from `helpers.rs` and its locals region is not what the escape flush writes, that flush being keyed on the virtualizable. Three reads are pinned — `f_lineno`, `sorted(f_locals)`, and the three combined. Measured against a build with `sys._getframe`'s two `force_frame` calls moved to the `f_lasti` / `f_lineno` getsets: the synthetic suite stayed at 369/370 on dynasm and cranelift and 366/366 on wasm while `f_lineno` reported the `def` line, `f_locals` grew a second entry, and the combined read exited 139. This guard exits 139 on that build and passes on dynasm, cranelift and wasm at HEAD, as do cpython, pypy3 and `PYRE_NO_JIT=1`. Assisted-by: Claude
…y-force boundaries `maybe_walker_vable_and_vrefs_before_residual_call` now takes the current jitcode pc and, when `inline_callee_consts` is present, records `SetfieldGc(callee_frame, const(py_pc))` under `pyframe_next_instr_descr()` plus `heapcache_setfield_cached`. The pc is mapped through the CALLEE's own jitcode metadata (`consts.jitcode_index` -> `pyjitcode_for_jitcode_index` -> `python_pc_for_jitcode_pc`); the sub-walk inherits `fbw_mode` from the outer ctx, so `snapshot_sym` names the portal's jitcode and cannot map a callee pc. Before this, the only `last_instr` store the trace ever recorded on such a frame was the construction `-1` (`helpers.rs:1376-1380`, `:1493-1496`), so a materialised inline-callee frame carried `-1` and `offset2lineno(code, -1)` answered the `def` line. `pyopcode.py:200 dispatch_bytecode` writes `self.last_instr` at every bytecode; for a frame that is not `virtualizable_boxes[-1]`, `pyjitpl.py:1120-1146 _nonstandard_virtualizable` and `:1189-1193 _opimpl_setfield_vable` make that a plain recorded `setfield_gc` on the virtual frame. Recording it once per may-force residual boundary is that store after the optimizer has coalesced it. The default configuration is unchanged: the `VableEscapedDuringResidualCall` abort still routes these shapes to the interpreter, so the recorded pc is not yet observable in production. Measured with the abort suppressed, an inlined callee reading `sys._getframe(0).f_lineno` goes from `[2, 4]` to `[4]`. check.py dynasm 372/372; cargo fmt clean. Assisted-by: Claude
… committed `flush_active_frame_escape` returned `flushed || expected == frame as usize`, so on the redirected (`escaped_published`) arm a declined flush reported "not escaped" and the force never happened. Return true whenever the guard matched. Upstream decides the escape from the vable token state alone, with no image write involved: `virtualizable.py:231-246 tracing_after_residual_call` returns true iff `force_now` (`:248-255`) cleared the token — "The values in the virtualizable are always correct during tracing. We only need to reset vable_token to TOKEN_NONE as a marker" — and `pyjitpl.py:3373-3390 vable_after_residual_call` raises ABORT_ESCAPE on that state unconditionally. Runtime forcing also resets the token before writing fields (`resume.py:1405-1408`). Same for a vref'd inlined callee (`virtualref.py:157-167`). The declined-flush bookkeeping is unchanged: the locals region is still written (`flush_locals_region_to_frame`), no resume pc is committed, and the undo stays armed. No behaviour change at the default configuration — the escape flush declines 0 times across r1-r5 and the synthetic corpus, so `flushed` was already true on every reporting path. Measured with a decline forced to fire, the abort count returns from 0 to 10 and a side-effect-plus-frame-read callee still applies its effect exactly once (30000/30000), i.e. the replay double-apply the previous comment described does not occur. check.py dynasm 372/372, cranelift 372/372; cargo fmt clean. Assisted-by: Claude
…shed-callee disjunct
`flush_active_frame_escape` matches on `expected == frame as usize ||
escaped_published` and collapses both into one bool, so the share of the
`VableEscapedDuringResidualCall` class carried by each disjunct was unmeasured.
Adds three `fbw_diag` counters classifying each matched escape, and two more
attributing the token clear at the force site in `force_pyframe` — that clear is
what the post-residual probe reads as an escape and turns into the abort, so its
attribution is 1:1 with the aborts. Both are exported through the `MAJIT_STATS`
jit-stats block.
Measured, dynasm release, 356 synthetic fixtures:
loops_aborted 207
attributed escape forces 138 (66.7% of aborts)
by_portal 93
by_callee_only 45 (32.6% of the class, 21.7% of aborts)
Per fixture the attribution sums to the abort count exactly (r1 5+5=10,
r3 5+10=15), which is the check that the counters name aborts rather than calls
to the flush.
`RING_BASE` moves 6 -> 11 for the five new slots; `LEN` is derived from it, and
the wasm runner's duplicated copy is updated to match.
check.py dynasm 372/372; cargo fmt clean.
Assisted-by: Claude
… frame The strict fresh-frame fold in `vable_ops.rs` returned `Ok(Continue)` for a `setfield_vable` / `setarrayitem_vable` on the inline callee's own frame register, emitting neither an IR op nor a concrete store, so the live PyFrame kept whatever image it had at entry. Carry the callee's concrete `PyFrame*` and its seeded frame box on `CalleeLocalsShadow`, set both where the sub-walk resolves `callee_portal_frame_reg`, and have both fold arms store through via `store_live_frame_static_int` / `store_live_frame_array_slot`. `store_live_frame_array_slot` already ignores non-Ref values and slots past the array end, so the array arm keeps a `debug_assert!` rather than a release panic on a value shape the corpus does not exercise. r1-r5 answers, per-fixture abort counts and the corpus attribution (loops_aborted=207, by_portal=93, by_callee_only=45 over 356 synth fixtures) are unchanged; committed `.jitstats` baselines do not move. Assisted-by: Claude
…ces the frame Once a residual call has forced the frames, the inline callee's folded local stores exist only in `CalleeLocalsShadow` — the trace carries no op for them, so anything reading the callee frame after the force sees the entry image. Add `disarm_folded_inline_callee_after_escape`, called from the `forced` arm of `try_execute_residual_call_via_executor`: it replays the shadow's slots in index order through `vable_setarrayitem_indexed` against the seeded callee frame box, captures the resulting guards, and then clears `fold_frame_reg` so subsequent stores on that frame take the ordinary recorded path. It is inert unless the walk is an inline sub-walk with a seeded frame box whose portal frame register still holds it. r1-r5 answers, per-fixture abort counts and the corpus attribution (loops_aborted=207, by_portal=93, by_callee_only=45) are unchanged; committed `.jitstats` baselines do not move; dynasm check.py 372/372. Assisted-by: Claude
…ts to its frame `LiveLastInstrGuard::enter` retargeted the frame to the inline callee's concrete frame but kept the outer walk's `vstack_cur_pypc`, so the pc and the frame named different code. A sub-walk never advances that mirror (`vstack_valid` is false while `PYRE_FBW_CALLEE_VSTACK` is off), so the value published onto the callee frame was a constant 0 and a frame reader inside the callee reported the function's first line. Resolve the pc against the callee's own metadata instead, via a new `inline_callee_py_pc` that reuses the `python_pc_for_jitcode_pc` lookup `maybe_record_inline_callee_last_instr` already performs. The outer-frame arm is unchanged. Measured with the escape abort suppressed for the published-callee disjunct (a local experiment, not part of this commit), this is what turns `sys._getframe(0).f_lineno` inside an inlined callee from the `def` line into the reading line, and `f_lasti` from 0 into the executing offset. At the default configuration it is behaviour-neutral: r1-r5 answers unchanged, corpus loops_aborted=207 / by_portal=93 / by_callee_only=45 unchanged over 356 synth fixtures, dynasm check.py 372/372, committed `.jitstats` baselines unmoved. Assisted-by: Claude
…ites `fb00dfbf8ce` moved `jitcode_dispatch/diag.rs:160 python_pc_for_jitcode_pc` to `py_coord.rs:37 containing_py_pc_for_jitcode_pc`, body byte-identical. `inline_callee_py_pc` and `maybe_record_inline_callee_last_instr` were written against the old name, and rebasing onto that commit produced no textual conflict, so the tip did not compile: two E0425s at `residual_call.rs:3557` and `:3582`. All three `prepare Charon/LLBC` jobs failed there, charon building `pyre-jit` compiling `pyre-jit-trace`, and every downstream job skipped. Point both sites at `crate::py_coord::containing_py_pc_for_jitcode_pc`. check.py dynasm 372/372, cranelift 372/372, wasm 368/368, all on a re-extracted `.ullbc`; `cargo test --all --no-default-features --features dynasm` and `cargo fmt --all -- --check` clean; `cargo check --workspace --all-targets` clean under `--features dynasm` and `--no-default-features --features cranelift`. Assisted-by: Claude
0a8520d to
05df83c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05df83cf11
ℹ️ 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".
| // `pyre_jit_trace::trace::fbw_diag`. | ||
| if let Ok(fbw) = instance.get_typed_func::<u32, u64>(&mut store, "pyre_fbw_diag") { | ||
| const RING_BASE: u32 = 6; | ||
| const RING_BASE: u32 = 11; |
There was a problem hiding this comment.
When diagnostics are collected through pyre-wasm-runner, slots 6–10 are now reserved for the escape split and force attribution, but this decoder only moves RING_BASE past them and never reads or prints them. Native runs expose the same counters through the new fbw_escape_split and fbw_force_attrib lines in pyrex/src/lib.rs, so WASM MAJIT_STATS output silently omits the commit's new attribution data and cannot be compared with the native backends. Add the five slots to the WASM diagnostic output before decoding the ring.
Useful? React with 👍 / 👎.
The rebase onto main (through #1015 and #1019) moved structural jit-stats on 26 synthetic fixtures. #1019 re-recorded only the eight dynasm baselines; the cranelift/wasm baselines of the same fixtures and the remaining drifted fixtures are re-recorded here from a 3-backend measurement. No correctness or output change; the recorded values reproduced exactly across the record run and a full verification run (dynasm 373/373, cranelift 373/373, wasm 369/369 green). Assisted-by: Claude
… the test_asyncio hang) (#1016) * jit: env-gated report and decline switches for the raw-color result-slot fallback `inject_root_call_result`'s raw-color fallback is documented as load-bearing and simultaneously wrong. `PYRE_INJECT_RCA` reports each engagement with its coordinate; `PYRE_INJECT_FALLBACK_DECLINE` declines the bridge compile instead of engaging it, as the A/B control for attributing a wrong-slot injection. Both are off in production; the gated branches are the only added code. Assisted-by: Claude * jit: keep constructor inlining out of parented guard snapshots A guard emitted inside an inlined `__init__` resumed through a parented snapshot chain ([caller paused after the CALL, `__init__` frame]), so the blackhole delivered `__init__`'s return value — None — into the caller's CALL dst register. `typeobject.py descr_call` discards `__init__`'s result and returns the instance; the flattened constructor inline has no frame that can reconstruct that discard at a mid-`__init__` pause. Two changes in `try_walker_inline_resolved_user_call`: * a constructor callee that is not strict-straight-line returns `Ok(None)` (residual instantiation) instead of entering the forward-branch multiframe path — and before the arm that would decline the enclosing key to interpretation; * `strict_seed` excludes constructors, so a straight-line `__init__` inlines through the ordinary single-frame path whose guards collapse to the caller boundary (deopt re-executes the whole instantiation call). This was the cause of the test.test_asyncio hang/error cascade: a warmed `BaseEventLoop._call_soon` trace inlined `events.Handle(...)` including its branchy `__init__`; flipping the loop debug flag failed the `get_debug()` bool guard inside the inlined body and every subsequent `call_soon` produced `handle = None` (`AttributeError: 'NoneType' object has no attribute '_source_traceback'`, lost wakeups, event loop parked in kevent forever). New fixture `synth/type_call_inline_init_branch_deopt` reproduces the shape deterministically (verified failing on the unfixed build) with converged jitstats baselines on all three backends. Verified: check.py dynasm 372/372, cranelift 372/372, wasm 368/368; cpython_tests gate 47 PASS / 0 regressions (test.test_asyncio included); full test_asyncio suite completes in ~2min (one intermittent `_run_once` `self`-slot corruption class remains, tracked separately). Assisted-by: Claude * cpython_tests: widen the rust-panic reason cap to 200 chars The 80-char cap cut a CI panic reason at "...blackhole.rs:6", dropping the line number. The sibling signal/abort branch already keeps 200 chars. Assisted-by: Claude * check.py: re-record 38 jitstats baselines after the rebase onto main The rebase onto main (through #1015 and #1019) moved structural jit-stats on 26 synthetic fixtures. #1019 re-recorded only the eight dynasm baselines; the cranelift/wasm baselines of the same fixtures and the remaining drifted fixtures are re-recorded here from a 3-backend measurement. No correctness or output change; the recorded values reproduced exactly across the record run and a full verification run (dynasm 373/373, cranelift 373/373, wasm 369/369 green). Assisted-by: Claude * extra_tests: satisfy the runner's OK convention in seven new parity scripts The parity runner passes a script only when it exits 0 AND its last stdout line is exactly "OK". Five of #950's scripts printed nothing, one printed lowercase "ok", and newmemoryview_omitted_geometry imported __pypy__ unconditionally, failing under the CPython reference runner. These never ran in CI before because the check.py step ahead of the parity step was red on main. Assisted-by: Claude * builtins: BaseExceptionGroup and ExceptionGroup basicsize 96 per CPython 3.14.6 CPython 3.14.6 grew PyBaseExceptionGroupObject by one pointer slot, so __basicsize__ moved 88 -> 96 for BaseExceptionGroup and ExceptionGroup. Update cpython314_builtin_abi and the type_members_python314 expected table to the 3.14.6 values (measured; the two groups are the only changed entries, flags unchanged). CI's reference CPython is 3.14.6 while local homebrew is 3.14.5, which is why this only failed in CI. Assisted-by: Claude
1 participant
What this is
Nine commits on the path to removing
VableEscapedDuringResidualCall(the port ofpyjitpl.py:3389 vable_after_residual_call→SwitchToBlackhole(ABORT_ESCAPE)), which isthe single largest abort class in the synthetic corpus: 138 of 219
loops_aborted,against
abort: vable escape: 0andforcings: 0on the same fixtures under realpypy37.3.22.Eight of the nine are behaviour-neutral at the default configuration — they build the
frame image that the abort's removal will expose, plus the counters that measure it. The
ninth is a new self-check guard. No committed
.jitstatsbaseline moves.The commits
AGENTS.md: run the PyPy oracle (PYPYLOG=jit-summary:- pypy3 <fixture>) as the first step for a behavioural orthodoxy questionsys._getframeintosys_getframe(the@unwrap_specwrapper) andgetframe(the non-hidden chain walk), matchingpypy/module/sys/vm.py:30/:42, and addis_builtin_getframe_functioncheck.pyself-check guard: an inlined callee reading its own frame image (_getframe(0)), the shapeframe_lineno_mid_replay_regressiondoes not coverpyopcode.py:200 dispatch_bytecode's per-bytecodelast_instrstorevirtualizable.py:231-246 tracing_after_residual_call)MAJIT_STATSsetfield_vable/setarrayitem_vablethrough to the live frameLiveLastInstrGuardretargets to its frameWhat the counters now say
dynasm release, 356 synthetic fixtures:
Per fixture the attribution sums to that fixture's abort count exactly (r1
5+5=10,r3
5+10=15) — the check that the counters name aborts rather than calls to the flush.Why the abort itself is not removed here
Removing it is measured (
207 → 128) but regresses one fixture, and this PR does notcarry a change that would need a
.jitstatsre-record to look green.getframe_inline_subwalk_multiframeevaluatessys._getframe(2).f_locals["base"] + x.Suppressing the published-callee disjunct takes it from
loops_aborted=15 / loops_compiled=0 / guard_failures=0to1 / 2 / **8948**— the loop compiles and thendeopts on every iteration. The compiled loop holds exactly three
CALL_MAY_FORCE, eachwith its own
GUARD_NOT_FORCED:v35sys._getframe(2)v42.f_locals(jit_getattr)v48["base"]All 8947 deopts report
fail=4— the guard afterv42, not after_getframe.GUARD_NOT_FORCEDis one word compare,cmp [jf_descr], 0, and the only writer ofjf_descrisRunner::force, so the guard cannot tell the portal force from the vrefforce. Folding
_getframealone therefore just moves the deopt one op later:.f_localslowers tojit_getattr, which is#[jit_may_force], and its getset bodycalls
force_frame_before_locals_readunconditionally on the same portal frame.PyPy has neither residual:
pypy/module/sys/vm.py:41carries@jit.look_inside_iff(lambda space, depth: jit.isconstant(depth)), and itsf_localsread is ordinary looked-inside RPython whose virtualizable force
jtransform.py:2164-2172deletes. Thejit-log-optoracle for this fixture showsforce_token()×2,ptr_eq(p41, p0)+guard_trueand no call at all.So the remaining work is transparency, not gate-tuning: the constant-depth
_getframearm and a non-forcing
f_localsread. Tracked as follow-ups; this PR lands the frameimage they both depend on.
Verification
Rebased onto
origin/main(9c3888c4),.ullbcre-extracted, all three backends measuredon the rebased tip:
check.py --backend dynasm372/372,cranelift372/372,wasm368/368cargo test --all --no-default-features --features dynasm— cleancargo fmt --all -- --check— clean.jitstatsbaselines unmoved;getframe_inline_subwalk_multiframestays atloops_aborted=15 / loops_compiled=0 / guard_failures=0 / bridges_compiled=0bench/frame_*_regression.pyself-check guards passr1–r10frame-read repros agree withPYRE_NO_JIT=1, CPython andpypy3, with twoexceptions:
r3andr8readsys._getframe(1).f_linenofrom an inlined callee andreport the caller's
defline. Each commit here was measured to leave ther1–r5answers unchanged, and no corpus fixture covers the shape. Filed as a separate
follow-up; not addressed by this branch.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
sys._getframe()behavior, including frame-depth validation and reliable traversal.Diagnostics
Tests
Documentation