jit: store-loop header merge-point forward-select + list-subscript exact-type guards + int-subclass BigInt#477
Conversation
bh_store_subscr_fn stashed a raise from a user __setitem__ only in BH_LAST_EXC_VALUE, not the backend _store_exception cells that a compiled trace's trailing GUARD_NO_EXCEPTION reads. The guard then passed on a stale 0 and the raise was swallowed, so a for-loop with a raising subclass __setitem__ ran to completion under JIT. Also publish via jit_publish_exception, mirroring bh_store_attr_fn and jit_next. Assisted-by: Claude
The list subscript read (try_walker_specialize_subscr, generated_binary_subscr_value) and store (try_walker_specialize_store_subscr, generated_store_subscr_value) specializations gated on is_list and emitted GuardClass(&LIST_TYPE); both check ob_type. A list subclass shares ob_type == &LIST_TYPE but retags w_class and may override __getitem__/__setitem__, so the recorded direct-storage IR bypassed the override in compiled code. Gate on is_exact_list and add walker_guard_exact_w_class so a subclass side-exits to the generic residual that dispatches to the override, mirroring the tuple read path. Tighten the shared tuple gate to is_exact_tuple for consistency. Assisted-by: Claude
loop_header_merge_point_regs seeded the loop-header merge point's
pycode/frame/ec register colors from max{mp_pc <= entry}. For a second
sibling loop's header entry this selected the preceding loop's merge
point, leaving this loop's own pycode green color OpRef::NONE and
aborting the trace with JitMergePointGreenKeyUnresolved, so the loop
never compiled.
Header entries now select min{mp_pc >= entry}; body-guard bridge resume
(is_bridge_trace) keeps max{mp_pc <= entry}. Mirrors opimpl_jit_merge_point
(pyjitpl.py:1537) binding greenboxes from the merge-point op the frame is
about to execute. header_entry_is_nested excludes a loop nested inside a
still-active loop, which keeps the decline/abort path because the walker
miscompiles the bridges such an inner loop closes into.
Assisted-by: Claude
`generated_list_setitem_by_strategy` emitted only `guard_class(LIST_TYPE)` + strategy guard, so a trace recorded on an exact list could be re-entered by a list subclass (shared `ob_type == &LIST_TYPE`, retagged `w_class`, overriding `__setitem__`) and bypass the override via direct backing-store write. Add a default `WalkerFrameOps::guard_exact_w_class` (getfield `w_class` -> PtrEq canonical -> GuardTrue, skipped for null typeobj or unescaped virtuals) and call it between `guard_class` and `guard_list_strategy`, matching the walker-native store path ordering. Both `MIFrame` and `WalkContext` inherit it via the default method. The walker-native store/getitem paths already carry this guard; the live getitem helper `generated_list_getitem_by_strategy` is reached only by the retired executor-trait path, documented in place. Assisted-by: Claude
`int_descr_new` built every subclass instance via `w_int_get_value` + `w_int_new_unique`, truncating a value that overflows i64 (stored as a `W_LongObject`) to a garbage machine word. Branch on `is_long`: clone the `BigInt` into `w_long_new` for large magnitudes, keep the i64 fast path otherwise. `w_class = cls` is set on either representation. Assisted-by: Claude
Assisted-by: Claude
|
Warning Review limit reached
Next review available in: 37 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 (7)
✨ 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 0dd0caf). 1. 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
|
Six commits on top of
main(the FOR_ITER abort-free heap-store admission from this line already landed via #448 and was dropped during rebase).Commits
bh_store_subscr_fnpublishes a raise into bothBH_LAST_EXC_VALUEand the backend_store_exceptioncells, so a compiled trace's trailingGuardNoExceptionside-exits instead of reading a stale 0 and swallowing the raise. Mirrorsbh_store_attr_fn/jit_next.is_exact_listand emit a runtimewalker_guard_exact_w_class(PtrEq on the canonicalw_class) so a list subclass that sharesob_type == &LIST_TYPEbut overrides__getitem__/__setitem__side-exits to the generic residual rather than taking the direct-storage path.entry(min{mp_pc >= entry}) instead of the last one before it; the oldmax{<= entry}picked a preceding sibling loop's merge point, seeding that loop's green colors and leaving this loop's pycode colorOpRef::NONE(the "born-between-loops"JitMergePointGreenKeyUnresolvedabort). Body-guard bridge resumes keepmax{<= entry}.header_entry_is_nestedscopes the change out for a loop nested inside another still-active loop, keeping nested inner loops on the interpreted path (their bridges are separately miscompiled — see Split descr identity between metainterp and backend #36).WalkerFrameOps::guard_exact_w_classand calls it ingenerated_list_setitem_by_strategy, extending the exact-type guard to the residual/trait store helper (defense-in-depth; the live getitem helper is retired-path only and documented in place).int_descr_newbranches onis_long, cloning theBigIntintow_long_newfor magnitudes that overflow i64 (the i64 fast path truncated them to a garbage machine word).Verification
cargo build --release(dynasm) clean;python pyre/check.py --backend dynasm→ ALL PASSED: dynasm 161/161.PYRE_NO_JIT=1.Codex parity review
Ran against the true fork point: Section 1 & 2 = None (no new parity regressions or mismatches). Section 3/4 findings are all documented adaptations or accurately-documented unreachable pre-existing items (dispositions recorded in
.claude/codex-review-report.md). Follow-ups filed: #36 (nested-loop bridge exact-resume), #35 (rvmprof port).— commented by Claude