[youknowone]

Rebased onto 51cf07eb732. The earlier set probe commit was dropped: #664 replaced the callback-visible table copy in w_set_{insert,contains,discard}_key_checked with a plain probe, so the O(n)-per-operation cost it removed no longer exists (synth/dict_set is 0.72s on this base, membership flat across set sizes).

What remains are two defects with one root cause: codewriter.rs had no representation for a local that is unbound on at least one predecessor block (the framestate union yields no SSA value for the slot).

1. DELETE_FAST — JIT-only compile panic (process dies)

def f():
    i = 0
    x = 0
    while i < 100000:
        if i < 50000:
            x = i
        else:
            del x
        i = i + 1
    return i

thread panicked at pyre/pyre-jit/src/jit/flatten.rs:1446:
make_return expects 1 or 2 args, got 3

Dropping the SSA value made DELETE_FAST emit an exitless permanent-abort block; flattening then misclassified its three incoming live values as return arguments and hit make_return's arity check. The lowering now reads the physical frame slot and keeps the bound/unbound branch.

Discriminators: del x unconditional in the loop is fine, and x = None in place of del x is fine — only the branch-arm del panicked. PYRE_NO_JIT=1 was always correct.

2. LOAD_FAST_CHECK — conditional assignment never compiled

if …: x = … is the ordinary Python shape, and the arm called emit_abort_permanent! whenever the slot had no SSA value, so every trace hit the abort marker and the loop was never compiled.

The arm's comment records the hazard it was avoiding: the residual + GuardNoException + push leaves a normal-return Finish continuation, and a later guard-failure bridge resolves into that return, silently swallowing the raise. The fix avoids raising from compiled code at all, reusing the CFG shape the FOR_ITER exhaustion lowering already uses in this file: read the slot from the vable, ptr_nonzero, continue on the bound arm, and side-exit at this opcode on null so the interpreter re-runs LOAD_FAST_CHECK and raises UnboundLocalError itself.

synth/load_fast_check	before	after
user time	1.46s	0.03s
loops_compiled / loops_aborted	0 / 5	1 / 0

Same numbers on both backends, and level with the control that hoists x = 0 before the loop so LOAD_FAST is emitted instead (0.04s). Output unchanged at 979999300000.

Verification

Rebuilt both backends at HEAD, then:

• the crash repro raises the correct UnboundLocalError on dynasm and cranelift, matching CPython;
• a 3-case script (hot unbound / never-bound / bound-every-iteration) is byte-identical to CPython on both backends;
• check.py: dynasm 241/241, cranelift 241/241.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b68c2d91-e2d1-48cf-8b3c-d31c26398c75

📥 Commits

Reviewing files that changed from the base of the PR and between 57b01d0 and 6e86efc.

📒 Files selected for processing (7)

• majit/majit-metainterp/src/history.rs
• pyre/bench/synth/set_reentrant_eq_mutation.py
• pyre/pyre-interpreter/src/jit_fnaddr.rs
• pyre/pyre-jit/src/jit/codewriter.rs
• pyre/pyre-object/src/dict_eq_hook.rs
• pyre/pyre-object/src/dictmultiobject.rs
• pyre/pyre-object/src/setobject.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf-loop

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/youknowone/pyre/blob/0b392060c38829ec22a0ec2b1fffc30c78d3647c/pyre-object/src/setobject.rs#L290
Register callback-free helpers as fnaddrs

When these set paths are traced by the JIT, begin_callback_free_probe/end_callback_free_probe and the other new #[dont_look_inside] TLS helpers have to be residualized through jit_trace_fnaddrs; I checked pyre-interpreter/src/jit_fnaddr.rs:1088-1168, and it still registers the existing dict_eq_hook helpers but not these new callback-free ones. In this repo, unregistered helper paths are represented as symbolic fnaddr placeholders, so compiled traces that execute set contains/add/discard can end up with an unpatchable residual call instead of the real TLS guard.

ℹ️ 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".

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 6e86efc).
Updated: 2026-07-21T22:26:29.035Z

Files in the reviewed diff

pyre/bench/synth/set_reentrant_eq_mutation.py
pyre/pyre-interpreter/src/jit_fnaddr.rs
pyre/pyre-jit/src/jit/codewriter.rs
pyre/pyre-object/src/dict_eq_hook.rs
pyre/pyre-object/src/dictmultiobject.rs
pyre/pyre-object/src/setobject.rs

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

• pyre/pyre-object/src/setobject.rs:334 ↔ rpython/rtyper/lltypesystem/rdict.py:593 — the new re-entrant scan returns immediately when __eq__ returns true, without validating that the callback left the candidate/table intact. RPython custom r_dict has paranoia=True and restarts if the callback changed the entries or candidate. Thus an equality method that clears/replaces the set then returns True makes add incorrectly skip insertion; membership can report true for a removed key; and discard can remove a replacement entry or report removal despite none. This affects the new fallback paths in w_set_insert_key_checked, w_set_contains_key_checked, and w_set_discard_key_checked.

3. Pre-existing mismatches (already present before this patch)

• pyre/pyre-object/src/setobject.rs:251 ↔ pypy/objspace/std/setobject.py:934 — w_set_add remains infallible: it calls object_key_for and discards w_set_insert_key_checked’s Result. PyPy’s object-set insertion propagates space.hash_w/space.eq_w exceptions through d[w_key] = None; Pyre can swallow them on this public path. This behavior was already present in upstream/main.
• pyre/pyre-object/src/dictmultiobject.rs:1750 ↔ pypy/objspace/std/setobject.py:167 — when no equality hook is installed, the fallback ladder only compares two frozensets (is_frozenset(a) && is_frozenset(b)), whereas PyPy’s set equality accepts either W_BaseSetObject subtype. The surrounding fallback also returns false for arbitrary user-defined keys without the hook. Both conditions predate this patch.

4. Structural adaptations

• pyre/pyre-object/src/setobject.rs:286 ↔ rpython/rtyper/lltypesystem/rdict.py:578 — the callback-free fast path plus a separately rooted scan is a Rust/IndexMap adaptation. RPython’s custom r_dict can invoke keyeq while walking its own low-level table; Rust must not retain an IndexMap borrow across user code. The adaptation is appropriate in principle, but must implement the source’s post-callback restart invariant (the section-2 defect).
• pyre/pyre-object/src/dict_eq_hook.rs:79 ↔ rpython/rtyper/lltypesystem/rdict.py:592 — the thread-local probe markers are temporary execution state used to avoid invoking Python equality while IndexMap is borrowed. They do not correspond to persistent PyPy object state and are a Rust/free-threading adaptation.
• pyre/pyre-jit/src/jit/codewriter.rs:11563 ↔ pypy/interpreter/pyopcode.py:536 — the explicit ptr_nonzero split and interpreter bailout model Python 3.14’s LOAD_FAST_CHECK while preserving PyPy’s “null local raises; non-null local pushes” behavior. The opcode/control-flow shape is a CPython-compiler and JIT-SSA adaptation.
• pyre/pyre-interpreter/src/jit_fnaddr.rs:1225 ↔ rpython/jit/codewriter/jtransform.py:456 — registering residual-call helper addresses is required by Pyre’s Rust source-translation/runtime-address model. RPython obtains direct-call function pointers from its low-level graph representation, so there is no one-to-one source declaration.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f540b31b8

ℹ️ 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".

[chatgpt-codex-connector]

Restart when equality mutates the matched set entry

When a same-hash key's __eq__ mutates this set and then returns True, this branch returns the pre-callback index without first verifying that the candidate entry is still present at that index. That makes the fallback path disagree with the RPython lookup it is porting, which rechecks the table/key before honoring found; otherwise contains can report a removed key as present, add can skip inserting the new key, and discard can remove whatever shifted into the stale index.

AGENTS.md reference: AGENTS.md:L194-L195

Useful? React with 👍 / 👎.