jit: resolve a walk's coordinates from the frame and operand offset that produced them; gate the vendored CPython suite - #1111
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR adds a Darwin ARM64 CPython suite gate and updates inline JIT frame, coordinate, ChangesCPython suite validation
Inline JIT correctness
Iterator state parity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Check
participant Dynasm
participant CPythonSuite
Check->>Dynasm: run synthetic benchmarks
Check->>CPythonSuite: run vendored CPython tests
CPythonSuite-->>Check: return module results and output
Check->>Check: enforce suite and module timeouts
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 624d547875
ℹ️ 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".
| } => crate::state::pyjitcode_for_jitcode_index(jitcode_index).map(|jc| { | ||
| crate::py_coord::containing_py_pc_for_jitcode_pc(&jc.metadata, op_pc) as usize + 1 |
There was a problem hiding this comment.
Preserve the frame identity through FOR_ITER delivery
When an inlined callee's FOR_ITER body has the same numeric Python offset as a loop in the outer live frame, this conversion discards jitcode_index and returns only that offset. The downstream stash matching in fbw_state.rs consequently aliases the two loops, and deliver_inflight_foriter_item validates and pushes the callee's item against the outer frame, allowing a guard abort to resume the caller with the wrong item/stack. Keep the JitCode/frame identity through matching and delivery, or refuse legacy delivery for a non-root frame.
AGENTS.md reference: AGENTS.md:L32-L42
Useful? React with 👍 / 👎.
| if not args.no_cpython_suite and not args.synthetic_only: | ||
| print() | ||
| print(bold("vendored CPython suite")) | ||
| chk.run_cpython_suite() |
There was a problem hiding this comment.
Run the CPython gate only once on macOS CI
In the inspected .github/workflows/pyre-ci.yml, pyre-check-macos reuses the shared step that invokes default pyre/check.py, while the separate cpython-tests job already runs pyre/cpython_tests/run.py with the same dynasm baseline. Making this stage unconditional by default on darwin-arm64 therefore executes the roughly five-minute, 104-module gate twice on every CI run, doubling the expensive macOS work without adding coverage; disable it in one lane or remove the redundant standalone job.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs (1)
4083-4187: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftGate nested fresh-coordinate derivation on the non-subwalk case.
When
ctx.fbw_mode.inline_subwalkis true,ctx.fbw_mode.snapshot_symremains the outer/root sym whilecall_site_py_pcuses that sym’s jitcode metadata with the callee’sop.pc. Ifcollect_outer_active_boxesreturns no boxes at any nested inline Call, this branch can map the nested offset through the root JitCode. Use the same!ctx.fbw_mode.inline_subwalkguard as the abort-coordinate path.🤖 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/jitcode_dispatch/inline_call.rs` around lines 4083 - 4187, Restrict the fresh CALL-site coordinate derivation in the outer-active-boxes initialization to the non-subwalk case, using the same !ctx.fbw_mode.inline_subwalk condition as the abort-coordinate path. When inline_subwalk is true, always inherit ctx.outer_active_boxes and its associated coordinate fields instead of calling collect_outer_active_boxes or mapping op.pc through snapshot_sym metadata.
🤖 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.
Outside diff comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs`:
- Around line 4083-4187: Restrict the fresh CALL-site coordinate derivation in
the outer-active-boxes initialization to the non-subwalk case, using the same
!ctx.fbw_mode.inline_subwalk condition as the abort-coordinate path. When
inline_subwalk is true, always inherit ctx.outer_active_boxes and its associated
coordinate fields instead of calling collect_outer_active_boxes or mapping op.pc
through snapshot_sym metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d9d00a7-91f9-4c3b-bd3d-cd8a05110ff6
📒 Files selected for processing (13)
pyre/check.pypyre/extra_tests/parity_tests/foriter_body_call_abort_operand_stack.pypyre/extra_tests/parity_tests/inline_callee_locals_across_guard.pypyre/extra_tests/parity_tests/iterator_setstate_python314.pypyre/extra_tests/parity_tests/nested_inline_caller_lineno.pypyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/diag.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/specialize.rspyre/pyre-jit-trace/src/jitcode_dispatch/tests.rspyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit d7eb409). 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)
4. Structural adaptations
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs (1)
4115-4186: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve inherited caller state for nested inline subwalks.
When
ctx.outer_active_boxes.is_empty()andctx.fbw_mode.inline_subwalkis true, this branch maps the intermediate calleeop.pcthroughctx.fbw_mode.snapshot_sym::jitcode. That produces an invalid caller coordinate and can publish the wronglast_instror resume header; skip collapse-box capture here and use the inheritedctxfields plusinherited_caller_py_pcinstead.Proposed fix
- ) = if ctx.outer_active_boxes.is_empty() { + ) = if ctx.outer_active_boxes.is_empty() && !ctx.fbw_mode.inline_subwalk {Add a nested-inline regression case where the outer caller has no active boxes. Before commit, run
cargo check --features dynasm,cargo test --features dynasm, and all eight required benchmarks.🤖 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/jitcode_dispatch/inline_call.rs` around lines 4115 - 4186, When ctx.outer_active_boxes.is_empty() and ctx.fbw_mode.inline_subwalk is true, bypass the snapshot_sym/jitcode call-site coordinate derivation and collect_outer_active_boxes call; instead preserve the inherited ctx outer fields together with inherited_caller_py_pc. Add a nested-inline regression case covering an outer caller with no active boxes, then run the requested dynasm checks, tests, and benchmarks.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs`:
- Around line 4115-4186: When ctx.outer_active_boxes.is_empty() and
ctx.fbw_mode.inline_subwalk is true, bypass the snapshot_sym/jitcode call-site
coordinate derivation and collect_outer_active_boxes call; instead preserve the
inherited ctx outer fields together with inherited_caller_py_pc. Add a
nested-inline regression case covering an outer caller with no active boxes,
then run the requested dynasm checks, tests, and benchmarks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 453949ed-84b4-4322-9b98-a43140b36e00
📒 Files selected for processing (12)
pyre/check.pypyre/extra_tests/parity_tests/inline_callee_locals_across_guard.pypyre/extra_tests/parity_tests/iterator_setstate_python314.pypyre/extra_tests/parity_tests/nested_inline_caller_lineno.pypyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/diag.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/specialize.rspyre/pyre-jit-trace/src/jitcode_dispatch/tests.rspyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs
`iterator_setstate_python314.py` tested the reject side of the cursor's machine-word range with `1 << 100` alone, 37 bits past the boundary. Add `sys.maxsize + 1` and `-sys.maxsize - 2`, and the accept side the fixture had no case for: `-sys.maxsize - 1` and `sys.maxsize` are the extremes that do fit, and each lands where any other value of its sign lands. The generic `__getitem__` iterator is the exception on the positive side, having no length to clamp against. Assisted-by: Claude
A `vendored CPython suite` stage runs `pyre/cpython_tests/run.py` for the dynasm backend against `pyre/cpython_tests/baseline.json`, the comparison `.github/workflows/pyre-ci.yml`'s `CPython suite (gate)` job already makes. `--no-cpython-suite` skips it, as does `--synthetic-only`. The baseline holds one verdict per module per backend and was observed on darwin-arm64, which is why that job pins `runs-on: macos-latest`; on any other host the stage prints what it skipped instead of counting as a pass. The verdict comes from the runner's `N to run,` line rather than its "no regressions" text: a selection that comes out empty prints every counter as zero and still exits 0. Assisted-by: Claude
…al frame `folded_store_is_observable_local` read the local/operand-stack boundary from `fbw_mode.snapshot_sym`, which describes the outermost portal frame, while a `setarrayitem_vable_*` slot indexes the frame the op names — the callee's inside an inline sub-walk. A callee with more locals than its caller had every local at or above the caller's count classified as operand stack, so its `STORE_FAST` folded away with no `SETARRAYITEM_GC` and a guard resuming inside the callee read the slot back as NULL, leaving the next call in the callee one positional argument short. `CalleeLocalsShadow` carries the inline level's `CodeObject`, set by `try_walker_inline_resolved_user_call` and `drive_bridge_frame_subwalk`. `active_frame_nlocals` resolves the boundary from it inside a sub-walk; the fold gate, the `value.is_none()` TOS recovery and the operand-stack push mirror read through it. The push mirror's method-form LOAD_ATTR decode moves to `active_frame_code`, which resolves the same way: it decoded `vstack_cur_pypc`, a callee pc, against the outer code object. Adds `parity_tests/inline_callee_locals_across_guard.py`, covering a callee with more locals than its caller and one with fewer. Assisted-by: Claude
Three walker sites read a frame-relative quantity off `fbw_mode.snapshot_sym`, which names the outermost portal frame, while the quantity belonged to the frame the walk was actually executing. `vable_ops` split a vable-array slot on `fbw_mode.inline_subwalk`. That flag is also set for a canonical-helper descent and for a recursive root closure driven through `run_sub_jitcode_walk`, neither of which resolves a `CalleeLocalsShadow`; both walk the frame the sym already describes, so keying on the flag left them with no local/operand-stack split at all. Key `active_frame_code` / `active_frame_nlocals` on shadow ownership instead. `fbw_foriter_body_from_op_pc` paired a `for_iter_next` residual's JitCode offset with the portal jitcode's index. Inside an inline sub-walk the offset is the callee's, so `inflight_foriter_body_pc` resolved it through the caller's pc tables and answered with a Python pc belonging to neither loop. Take the identity from `inline_callee_consts`, the resolution `build_multi_frame_miframe` already applies to its innermost frame. `InflightForiterBody::Jit`'s `outer_jitcode_index: u32` becomes `jitcode_index: i32`, so an unresolvable callee stays the documented conservative refusal. `try_walker_inline_resolved_user_call` derived `inline_caller_py_pc` by mapping the CALL's own `op.pc` through the portal's pc tables. One inline level down that offset is the intermediate callee's, and the result is published as the portal frame's temporary `last_instr`, so a frame read during a residual in the innermost callee reported a line the call was not made from. Inherit `fbw_mode.inline_caller_py_pc` when the walk is already a sub-walk. Adds `parity_tests/nested_inline_caller_lineno.py` and a unit test pinning the FOR_ITER body identity to the callee's jitcode. Assisted-by: Claude
`InflightForiterBody::Jit` carries `jitcode_index: i32` since #1111, which also made the identity negative when unresolvable. The census `code_ptr` resolution still destructured the former `outer_jitcode_index: u32` and cast it, so the crate stopped compiling once both sides met. `raw_code_for_jitcode_index` indexes with the value, so a negative index misses and the census keeps the live frame's code. Assisted-by: Claude
`InflightForiterBody::Jit` carries `jitcode_index: i32` since #1111, which also made the identity negative when unresolvable. The census `code_ptr` resolution still destructured the former `outer_jitcode_index: u32` and cast it, so the crate stopped compiling once both sides met. `raw_code_for_jitcode_index` indexes with the value, so a negative index misses and the census keeps the live frame's code. Assisted-by: Claude
`InflightForiterBody::Jit` carries `jitcode_index: i32` since #1111, which also made the identity negative when unresolvable. The census `code_ptr` resolution still destructured the former `outer_jitcode_index: u32` and cast it, so the crate stopped compiling once both sides met. `raw_code_for_jitcode_index` indexes with the value, so a negative index misses and the census keeps the live frame's code. Assisted-by: Claude
…ts, and a measured FOR_ITER gate widening (#1103) * list: give the unused typed strategy an empty array, not a block `build_list_storage` called `IntArray::from_vec` and `FloatArray::from_vec` unconditionally, and `try_alloc_typed_items_block` clamps `cap` to 1 into the old-gen `try_gc_alloc_stable_raw`, so every list allocated two blocks whose strategy never reads them. The trace emitters leave those fields null: `emit_empty_list_inline` and `emit_object_list_inline` set only `length` / `items` / `strategy`, and `emit_typed_list_inline` writes one typed pair. Add `IntArray::empty()` / `FloatArray::empty()` and use them where emptiness is statically known — `build_list_storage`'s non-matching arms, `switch_to_object_strategy`, `w_list_clear`. `switch_to_correct_strategy` keeps `from_vec`, since its twin `emit_promote_empty_list_inline` emits a capacity-1 block and seeds the capacity getfield cache with 1. `base()` takes `wrapping_add`, so the null block yields the items offset — a non-null, 8-aligned address `from_raw_parts` accepts at length zero. `list_object_custom_trace` skips the ownership query on a null typed block. Assisted-by: Claude * optimizeopt: answer an unwritten field of a virtual with its typed zero virtualize.py:184-190 optimize_GETFIELD_GC_* resolves a field the virtual has never been written to through optimizer.new_const(fielddescr). Pyre carried only the written-field arm, so such a read fell through to OptEarlyForce, which forces every argument of a non-exempt operation and materialised the struct along with everything its fields reach. The array counterpart was already in place: NEW_ARRAY_CLEAR seeds every slot with the typed zero at creation (virtualize.py:27-35, info.py:507-514). typeptr keeps its own arm. heaptracker.py:66 excludes it from the virtual field set and the block above answers it from the descr vtable, so a struct whose descr carries no vtable must not fold its class pointer to null. Assisted-by: Claude * mapdict: keep builtin storage on user subclasses Restore exact int and bool objects to 24 bytes, Unicode objects to 64 bytes, and tuple objects to 40 bytes. Add distinct user-subclass layouts carrying mapdict map and storage fields, with their own GC types and traces. Select the wider layouts from builtin subclass constructors and resolve mapdict field descriptors from each concrete carrier layout. Keep the specialized attribute load guarded by the subclass map and storage descriptors. Record the wasm guard-count changes caused by the restored exact-object heap trajectory. Assisted-by: Claude * jit: record integer zero-divisor raising arms Assisted-by: Claude * mapdict: harden builtin subclass carriers Guard the live Python class before native mapdict field access. Size map descriptors to the target word and exclude specialised tuple layouts. Mark private user layouts as GC objects without adding duplicate subclass-range peers. Re-root every mapdict carrier on class reassignment and allocate hasdict structseq values with tuple-user storage. Extend parity coverage for exact-value exits, descriptors, slots, GC inspection, and structseq extras. Assisted-by: Claude * jit: initialize inline allocation scalar fields Assisted-by: Claude * jit: record range zero-step raising arms Assisted-by: Claude * jit: record float zero-divisor raising arms Assisted-by: Claude * jit: record bigint zero-divisor raising arms Assisted-by: Claude * jit: record negative bigint shift raising arms Assisted-by: Claude * jit: scope FOR_ITER safety to escaping range loops Assisted-by: Claude * bench: re-record seven wasm jitstats baselines on the rebased base Five of them (`exception_traceback_loop_forms`, `gc_bug_bridge_flavor_traceback_names`, `loops_comprehension`, `newslice_step_hot`, `unpack_ex_hot`) return to the values already committed on the base; the rebase conflict resolution had kept this branch's older measurements over them. Their only remaining difference from the base is added counter keys. `range_ctor_in_loop` compiles and enters its loop for the first time, so loops_compiled 1 -> 5, bridges_compiled 0 -> 3 and guard_failures 0 -> 1009: a fixture that never entered compiled code reported zero guard failures trivially. An in-place revert of the FOR_ITER admission reproduced the old values. `closure_per_call` guard_failures moves 420 -> 417. This one is not attributed by a control arm. Assisted-by: Claude * jit: diagnose FOR_ITER gate opcode declines Assisted-by: Claude * jit: gate FOR_ITER decline census allocation Assisted-by: Claude * jit: gate FOR_ITER decline census collection Assisted-by: Claude * jit: guard numeric binary specialization classes Assisted-by: Claude * jit: retain context on specialized builtin raises Assisted-by: Claude * jit: skip redundant numeric class guards Assisted-by: Claude * test: drive the numeric subclass fixture through the specialized pc The fixture fed its subclass operand to a tail expression at a different BINARY_OP pc than the loop that went hot, so that site was never specialized and the check passed on a binary without the class guards. Iterate a list whose tail holds the subclass instead, so it arrives at the pc under test. Adds left-operand cases and a bool-driven case, which reaches the tagged and bool path where walker_numeric_builtin_class returns null and no class guard is emitted at all. Assisted-by: Claude * Grow FOR_ITER regions through handler rejoins Assisted-by: Claude * Tighten escaping range append recognition Assisted-by: Claude * Update range constructor loop jitstats Assisted-by: Claude * jit: admit LIST_EXTEND in FOR_ITER bodies Assisted-by: Claude * jit: admit call-bearing LIST_APPEND bodies in the FOR_ITER gate A LIST_APPEND body was admitted only when the body performed no call, because a mid-body abort after the append routed through fbw_foriter_inflight_take, which refuses delivery and dropped the iteration's item. range_ctor_in_loop goes from mc_entered=0 to 813. The surrounding comment previously cited blackhole.py as authority for the append being rolled back and replayed once. It is not: blackhole.py:1712 is setposition, which continues from the coordinate already reached, and upstream places the resume coordinate past a residual call so the effect is never re-executed. State instead what pyre actually relies on, and record that a non-committed walk exit still keeps the legacy entry replay whose delivery can be refused. Assisted-by: Claude * jit: census in-flight FOR_ITER delivery outcomes Assisted-by: Claude * bench: re-record nineteen jit-stats baselines Five fixtures enter compiled code where they previously did not, so their zero counters were zero trivially: exception_group_type loops_compiled 0 -> 1, guard_failures 0 -> 1 list_append_virtual_payload loops_compiled 0 -> 2, bridges 0 -> 8, guard_failures 0 -> 1603 minmax_key_rooting loops_compiled 1 -> 2, bridges 0 -> 2, guard_failures 5 -> 409 range_ctor_in_loop loops_compiled 1 -> 3, bridges 0 -> 3, guard_failures 0 -> 811 (812 on wasm) global_store_plain_dict_globals (wasm) loops_compiled 5 -> 6, loops_aborted 1 -> 2, guard_failures 1 -> 18 pickle_terminal_raise_resume (wasm) loops_compiled 67 -> 68, loops_aborted 13 -> 14, guard_failures 339 -> 356 mapdict_frozen_unboxing_fold takes guard_failures 2 -> 8 with loops_compiled unchanged at 3. An A/B across the call-bearing LIST_APPEND admission alone gives mc_entered 2 -> 8 on the same fixture, so the counter tracks compiled-code entries one for one: its `[C(i) for i in range(n)]` comprehension is a call-bearing LIST_APPEND body. gc_bug_bridge_flavor_traceback_names (wasm) improves guard_failures 2027 -> 1670. exception_reused_object_tb_not_doubled (wasm) loses fbw_blackhole_adopted_single_frame 3 -> 0. A binary built from 128590c with no branch commits applied reports 0 on the same fixture, so the fall is the base's; the baseline was last recorded at 779da08. Every other counter on that fixture is unchanged (loops 4, bridges 3, aborted 3, guard_failures 600) and its traceback-shape oracle passes. The recorder also writes the fbw_* and field_pos_* keys that were absent from baselines recorded before those counters existed. Assisted-by: Claude * Trace traceback escape marking in exception attribute fold Assisted-by: Claude * Trace fresh container allocations in FOR_ITER callees Admit replay-safe fresh tuple and list allocation helpers during nested callee tracing. Specialize len() for empty-list storage and add a cross-backend parity fixture for the admitted shape. Assisted-by: Claude * Admit tuple copies from exact lists during replay Assisted-by: Claude * Identify traceback walk bridge training The 603 guard failures comprise three 200-hit bridge thresholds and three one-off transition failures. The final bridge reconnects the traceback walk to its compiled inner-loop token, so no resume-semantics change is required. Assisted-by: Claude * bench: add a synthetic fixture for the subscript inline's index operand `Seq.__getitem__` reached as `p[len(EMPTY)]`, so the index arrives on the operand stack rather than from a constant or a local, with an `isinstance(index, slice)` branch in the body so the inline has a residual to abort on. Prints 276000 under cpython, pypy3 and pyre. The defect the shape covers — the FOR_ITER deferred admission reading `arg_class_guard.is_none()` as a proxy for "the entry opcode is a CALL", which admitted the BINARY_OP-entered subscript inline and let the flush resume one operand short — is fixed in #1082, which names the property directly and carries its own parity test. This holds the shape under the jit-stats gate too. Assisted-by: Claude * bench: re-record the wasm pickle terminal-raise baseline `loops_compiled` 66 -> 67, `loops_aborted` 14 -> 15 and `guard_failures` 339 -> 356 on the wasm leg of `synth/pickle_terminal_raise_resume`. The file already carried the 356 from an earlier recording; the two loop counters did not. `retraces_compiled=0` joins the recorded set. Bisected to `jit: admit call-bearing LIST_APPEND bodies in the FOR_ITER gate` by in-place whole-tree control arms at four points of this branch: the base and the trees at the three commits below it read 66 / 14 / 339, the tree at that commit reads 67 / 15 / 356, and every counter moves there together. A base control arm reproduces main's committed 66 / 14 / 339 on this host, so the move is this branch's and not the host's. The dynasm and cranelift baselines for the same fixture are byte-identical to main's (30 compiled, 1 aborted, 338 guard failures) and are unchanged here: the loop the widened gate admits is one only the guest reaches, which compiles 66 loops in this fixture where the native backends compile 30. The extra abort is one more attempt at a loop the gate now allows, recorded beside the compile it gained. Not the collection schedule: `PYPY_GC_MIN` at 256MB, 384MB and 512MB gives identical counters, and three repeats agree exactly. Assisted-by: Claude * jit: pair the vable static shadow write with a heap write-back `mirror_vable_static_to_boxes` wrote `virtualizable_boxes` without the `synchronize_virtualizable()` half `_opimpl_setfield_vable` performs (pyjitpl.py:1188-1199). `walker_capture_snapshot_for_last_guard_impl` publishes `last_instr = py_pc - 1` through it, and the walk never runs the interpreter's own `frame.last_instr = pc` store, so the live frame stayed one opcode behind the shadow and `check_synchronized_virtualizable` (pyjitpl.py:3463-3468) failed under `debug_assertions` in `gc_stress::module_dict_move_to_end_reentrant_survives_python_callbacks`. Add `TraceCtx::synchronize_virtualizable_static`, a single-static `write_boxes` that keeps `synchronize_virtualizable`'s guards and its `VableArrayStorage::RustVec` carve-out. The full `write_all_boxes` is not usable here: the shadow's array half holds NULL for the operand slots a mid-opcode guard resumes before, and writing it back would stamp those NULLs into the live frame. Call it from `mirror_vable_static_to_boxes`. `try_execute_residual_call_via_executor` saves the `last_instr` shadow entry before publishing the executing pc and restores it after the residual returns, matching `LiveLastInstrGuard`'s save/restore of the heap half. The restore is skipped when the callee forced the virtualizable. Assisted-by: Claude * docs: list the two FOR_ITER gate diagnostics in gate-triage `PYRE_FOR_ITER_GATE_DIAG` (pyre-jit-trace/src/jitcode_dispatch/mod.rs, pyre-jit/src/eval.rs) and `PYRE_FORITER_INFLIGHT_CENSUS` (pyre-jit-trace/src/jitcode_dispatch/mod.rs) are read through `env::var_os(..).is_some()`, so both are default-OFF diagnostics and belong in §6c. `pyre/pyrex/tests/gate_triage_complete.rs ::every_live_pyre_gate_has_a_gate_triage_entry` failed on their absence. Assisted-by: Claude * bench: re-record ten jit-stats baselines after the rebase Rebasing onto `a7eb493079f` moved these counters. Measured on the final tree with all three backends rebuilt from a full `extract-llbc.py`. synth/pypy_type_surface (all 3) bridges_compiled 102 -> 5, guard_failures 20497 -> 1011 synth/mapdict_frozen_unboxing_fold (all 3) guard_failures 8 -> 11 synth/ca_bridge_multiframe_resume_double_call (wasm) guard_failures 2581 -> 2592 synth/closure_per_call (wasm) guard_failures 418 -> 426 synth/wasm_ca_trampoline_decline (wasm) guard_failures 404 -> 601 synth/recursion_memo_branch (wasm) guard_failures 4724 -> 4704 `pypy_type_surface` returns to the values #999 committed. #1086 had rewritten the file to 102 / 20497 — the numbers the `Cls.__name__` metaclass fold produced while it guarded the raw `w_class` slot its oracle's `gettypefor` fallback never read — and #1086 landed before #1106 declined that fold, so the file has named a defect since. The fixed fold gives 5 / 1011 again. `pypy_type_surface`, `ca_bridge_multiframe_resume_double_call` and `wasm_ca_trampoline_decline` are also red on main's own CI at `b925c3e5ead` (run 31283765874, ubuntu leg), so those three do not originate here. An in-place control arm reverting only this branch's vable shadow write-back reproduces every one of these numbers, so none of them is that change. Assisted-by: Claude * jit: follow the InflightForiterBody field rename in the census `InflightForiterBody::Jit` carries `jitcode_index: i32` since #1111, which also made the identity negative when unresolvable. The census `code_ptr` resolution still destructured the former `outer_jitcode_index: u32` and cast it, so the crate stopped compiling once both sides met. `raw_code_for_jitcode_index` indexes with the value, so a negative index misses and the census keeps the live frame's code. Assisted-by: Claude * mapdict: split the layout predicate from the storage predicate `has_mapdict_layout` answers the physical question — the allocation carries the `MapdictStorageMixin` slots — and no longer consults `w_type_get_hasdict` for the generated int/str/tuple user layouts. `has_mapdict_storage` is that test plus the owning class's `hasdict` flag, and `mapdict_carrier` now asserts the layout predicate, so a `__slots__`-only native subclass no longer trips the assertion. `is_generated_user_layout_family` carries the specialised-tuple exclusion for the layout test, the storage test, and the carrier's `W_TupleObjectUser` arm. Assisted-by: Claude * _structseq: re-read the pinned class after the tuple allocation The array-backed tuple constructor can collect, so the class pointer read before it can be stale when it is stored into the new object's `w_class`. Re-read it from the shadow-stack slot after the allocation. Assisted-by: Claude * jit: gate the in-flight FOR_ITER census key lookup on the census `raw_code_for_jitcode_index` runs `ensure_finish_setup` and borrows `METAINTERP_SD`; `fbw_foriter_inflight_take` called it on every take even though `census_record_foriter_inflight` returns immediately unless `PYRE_FORITER_INFLIGHT_CENSUS` or `PYRE_FBW_DEBUG_ABORT` is set. The enable check moves into `foriter_inflight_census_enabled`, which both sites share. Assisted-by: Claude * jit: correct the exception descr group note on w_context `w_context` is written by the raise lowering, not left zeroed by GC pointer clearing. Assisted-by: Claude * test: scan the loop-region fixture in two passes `loop_region_includes_out_of_line_handler_rejoining_mid_body` compared each backward target against `outer_header` while still lowering it, so a jump seen before the smallest target was missed. The scan now runs twice over a shared target closure, each pass with its own `OpArgState`. Assisted-by: Claude * test: cover synchronize_virtualizable_static Five cases: the single-field write-back, absent virtualizable state, an out-of-range index, a RustVec-backed array field, and a shadow slot holding no concrete. Assisted-by: Claude * majit: exclude the identity slot from the static write-back bound `virtualizable_values`'s last slot holds the vable identity (`virtualizable_boxes[-1]`), not a field value. `synchronize_virtualizable_static` bounded `index` by the full vector length, so a shadow shorter than the declared static count would have written the identity ref into a static field. Bound by the data length. Assisted-by: Claude * jit: read PYRE_FOR_ITER_GATE_DIAG through one accessor The per-opcode decline and the whole-region decline each owned a function-local `OnceLock` for the same variable. Assisted-by: Claude * jit: admit builtin subclass carriers in the mapdict storage helpers The seven mapdict residual wrappers tested their receiver with `is_instance`, which is true only for an ordinary `W_ObjectObject`. The generated int/str/tuple user layouts failed that test, and the wrappers answer a value rather than declining: the unboxed reads returned 0 and 0.0, the boxed read returned PY_NULL, and all three writes returned without storing. `Flag.__or__` reads `other._value_`, so `Perm.R & Perm.R` computed `4 & 0`; `test.test_enum`'s `OldTestIntFlag` test_and/test_or/test_xor/ test_type failed on that. Measured on the release dynasm build, an unboxed int attribute read on an int/str/tuple subclass was wrong 1756/ 2411/2498 times per run and correct under PYRE_NO_JIT=1. The receiver test is now `has_mapdict_layout`, which is `mapdict_carrier`'s own precondition, shared through `is_mapdict_carrier`. The parity fixture gains loops that validate the loaded and stored values for the unboxed int and float slots; the existing ones discard what they load and so never observed this. Assisted-by: Claude * Revert "jit: admit call-bearing LIST_APPEND bodies in the FOR_ITER gate" This reverts commit 2a0f91f. The decline it removed is load-bearing. A comprehension whose body calls a user Python function drops an element: `[random.randrange(25) for i in range(size)]` returned 22 items for size=23, and PYRE_FORITER_INFLIGHT_CENSUS reported DELIVERED=0 REFUSED=1 for that body pc on the same run. `test.test_heapq`'s test_heapsort failed on the shortened list, raising IndexError from `heappop`. The reverted commit argued the append always sits past the resume coordinate; the census shows the refusal path is reachable, because the call commits body effects that `fbw_foriter_inflight_take` sees as a committed effect since the consume. The parity fixture records the shape. Assisted-by: Claude * majit: drop the narrowed virtualizable static synchronizer `mirror_vable_static_to_boxes` now calls `synchronize_virtualizable()`, the shape `_opimpl_setfield_vable` uses (`pyjitpl.py:1188-1199`), so the narrowed single-field variant and its tests have no caller. Assisted-by: Claude * bench: restore five jit-stats baselines the reverted gate had moved The call-bearing LIST_APPEND admission raised loops_compiled and bridges_compiled on exception_group_type, list_append_virtual_payload, minmax_key_rooting, range_ctor_in_loop and subscr_user_getitem_stack_index; reverting it returns them to what main records. mapdict_frozen_unboxing_fold's guard_failures returns to 2, the value main carries — the branch's 11 was recorded while the mapdict storage helpers answered zero. dynasm only; the cranelift and wasm baselines follow. Assisted-by: Claude * bench: restore the cranelift jit-stats baselines to match Same six benches as the dynasm pass, same direction and magnitude. Assisted-by: Claude * bench: restore the wasm jit-stats baselines to match The same six benches as the dynasm and cranelift passes, plus global_store_plain_dict_globals and pickle_terminal_raise_resume, whose observed loops_compiled / loops_aborted / guard_failures all return to the values main records. closure_per_call keeps main's guard_failures: its loops_compiled and bridges_compiled are unchanged, so the count drifts without a shape change. Assisted-by: Claude
Five commits on top of
origin/main. Four of them are one story: acoordinate is only meaningful in the frame, and at the operand offset, that
produced it. The fifth is the gate that surfaced all of them.
The gate
check.pygrows acpython-suitestage that runspyre/cpython_tests/run.pyagainst
baseline.json(103 modules, ~300s, dynasm). The suite alreadyexisted; nothing gated it, so a module going
PASS -> FAILwas invisibleoutside a manual run. Every defect below was found by turning it on.
Three wrong-frame reads
A full-body walk that descends into an inlined callee carries two sets of
metadata: the outer portal frame's, and the callee's. Interpreting a
callee-scoped quantity — a vable slot index, a JitCode offset, a Python pc —
with outer-frame metadata reads a real value from the wrong frame, which is
why these are miscompiles rather than aborts.
getarrayitem_vable_*/setarrayitem_vable_*slot split(
vable_ops.rs). The locals/stack split used the outer sym'snlocals,so a callee's
STORE_FASTwas classified as a stack push and neverrecorded; the blackhole then read NULL for that local.
test.test_datetimeshowed it as_find_ti'sbisect_right(self.lt[dt.fold], ts)raisingmissing 1 required positional argument.In-flight
FOR_ITERidentity (diag.rs). The stash paired thecallee's
op_pcwith the outer JitCode's index, soinflight_foriter_body_pcresolved a callee offset through the caller's pctables and answered with a Python pc belonging to neither loop. A callee
loop's item was stashed under an identity no resume could match, and a
caller loop resolving to the same pc had its own entry replaced.
Nested-inline
last_instr(inline_call.rs). Inf -> g -> h, thecoordinate published onto the outer frame while a residual runs came from
mapping the intermediate callee's
op.pcthrough the portal's tables.Observable as
sys._getframe(2).f_linenoreporting the driver'sdef-bodyline as well as the real call line — pinned by
parity_tests/nested_inline_caller_lineno.py.fbw_mode.inline_subwalkis not the discriminant for "am I in a calleeframe": it is also set for a canonical-helper descent and a recursive root
closure, neither of which resolves a callee. Frame layout keys off
ctx.callee_shadow; JitCode identity keys offctx.inline_callee_consts.jitcode_index— the same resolutionbuild_multi_frame_miframealready applies to its innermost frame.The wrong operand offset
reconstructed_all_ref_call_stackread the residual op's Ref var-list atoperand offset 1. That holds for the Ref-only shape
iRd>r, but themethod-form
CALLhelpers lower throughiIRd>r, whose leading Int list isvariable-width —
dispatch_residual_call_iIRd_kindreads its own Ref list at1 + i_width. At offset 1 the read landed on the Int list's length byteand resolved its register indices through the Ref bank.
The composed stack passed validation anyway, because the composition
self-corrects its height:
stack.len() == vstack_boxes.len()for anyfresh, so the flush'sdepth_at_py_pccheck could not fail. Forp[0]inside afor p in ...bodythe committed stack was
[iterator, p, iterator], and the interpreterre-executed the subscript with the loop's iterator as the index:
which failed
test.test_re. The offset now comes from the op's argcodes,walking the widths of
blackhole.py:112-157— the same walkdecode_op_atperforms. An op declaring no Ref list declines to the legacy replay.
Audited: the other 22 hard-coded offset-1 reads are reached only from
dispatch_residual_call_iRd_kind, where 1 is correct.reconstructed_all_ref_call_stackwas the single site reachable from bothdispatchers, because it hangs off
try_walker_inline_resolved_user_call.The iterator
__setstate__cursorParity coverage for the
ssize_tboundary of the cursor, continuing #1074.Testing
pyre/extra_tests/parity_tests/run.py— all pass, including the two newfixtures (
nested_inline_caller_lineno.py,foriter_body_call_abort_operand_stack.py) on cpython, dynasm and cranelift.cargo test -p pyre-jit-trace— 357 passed, 0 failed. Two new unit testspin the FOR_ITER identity and the argcode-derived offset; both were checked
green -> red -> green by negating their halves in place.
check.py—cpython-suite dynasm PASS, 103 modules.test.test_reandtest.test_datetimeboth pass.Known base red, not from this branch:
synth/pypy_type_surfaceregressesbridges_compiled 5 -> 102on all three backends at this merge-base. It isthe
Cls.__name__fold guarding a raw nullw_classongetset_descriptor,fixed by #1106, which is not in this base. Not re-recorded.
Branch note
This force-update drops
843de7569dc("sendpair[i]on the arity-2 tuplespecialisations back to the residual") from the branch. That commit removed the
subscript fold because it miscompiled
test.test_datetime; the first commithere fixes that miscompile at its root, and
test.test_datetimepasses withthe fold in place, so the workaround is no longer needed. The narrower
SpecialisedPairKind::Objectdecline that is already onmainis untouched.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--no-cpython-suiteto skip CPython suite validation when needed.Bug Fixes
Tests