jit-trace: exactness gates on the int/float payload folds; dynasm cond-call predicate from arglocs

[youknowone]

Follow-ups to #1414, opened separately because that PR was squash-merged while
these were still local. Three independent fixes; the first two are defects the
#1414 review surfaced, the third is a duplication the same review flagged.

majit dynasm: read the cond-call predicate from its regalloc location

resolve_opref maps only constants and frame slots, so a predicate the
regalloc left register-resident has no mapping there and the emitter panics
compiling an otherwise valid trace. Three sites read it that way and now read
arglocs[0]: x86::genop_discard_cond_call, x86::genop_cond_call_value, and
aarch64::genop_cond_call_value. aarch64::genop_discard_cond_call was
already correct — the x86 twin's comment claims to mirror it but mirrored it
only for the callee and arguments.

On x86 the discard variant's test moves from rax to R11
(X86_64_SCRATCH_REG, outside ALL_CORE_REGS): consider_discard_nargs_j2
emits no before_call, so the op's other arglocs may be caller-saved
registers including rax. genop_cond_call_value keeps rax, which carries the
predicate to store_rax_to_result on the not-taken path.

Scope note: genop_cond_call_value still resolves its callee and arguments
through emit_call. That is the same exposure, but pre-existing rather than
introduced here, and the two helpers differ in their arg_types fallback
(emit_call infers per-argument types; emit_call_from_arglocs assumes
all-Int), so swapping it carries its own ABI risk and is left for a follow-up.

jit-trace: require an exact builtin before the int/float payload folds

walker_numeric_builtin_class answers with the canonical int/float class
and documents is_exact_builtin_instance in the operand gate as its
precondition. Three callers never established it, admitting a subclass through
is_int/is_float, which read ob_type:

• try_walker_specialize_truth_int folded the truth off the raw payload
  instead of running the subclass's __bool__, then pinned a w_class the
  recorded operand does not carry — a guard that fails on the very value that
  recorded it.
• try_walker_specialize_store_attr spelled mapdict's
  type(w_value) is space.IntObjectCls as is_bool || !is_int. Both arms now
  call the interpreter's own is_unboxable_int / is_unboxable_float rather
  than re-deriving the rule.
• try_walker_specialize_store_subscr gated the unboxed list strategies with
  is_int && !is_bool / is_float. Object storage keeps the value boxed and
  stays ungated.

The w_class pin rejects a subclass arriving after recording; it does not
cover one present at recording time.

The other walker_numeric_builtin_class callers are clean, and a single-file
census gets this wrong: compare_op_int / binary_op_int / binary_op_float
/ compare_op_float gate through walker_int_specialization_input_operands
and its float twin in mod.rs; newlist gates with is_plain_int1.

bench/synth/float_subclass_binop_dispatch.py only had warm-then-swap cases,
which pass either way — they meet the subclass after recording. Adds
cold-start cases that record on the subclass from the first iteration
(truth_cold_subclass differs 20000 vs 0 between the two answers) and
re-records the baselines: loops_compiled 17 → 24 across all three backends,
exactly the seven added functions.

interp: fold the math float coercion's int payload arm into int_payload_as_f64

try_get_double carried its own bool/int/long-to-f64 ladder including a second
copy of the "int too large to convert to float" message. The two orderings
differ only in which predicate answers a bool, and both read the same intval.

---

Verified: cargo check on pyre-interpreter, pyre-jit-trace,
majit-translate, majit-backend-dynasm (native and x86_64-apple-darwin);
cargo test -p pyre-interpreter 407 passed; cargo test -p majit-backend-dynasm 72 passed; check.py --synthetic-pattern float_subclass_binop_dispatch PASS on dynasm/cranelift/wasm.

Cold-start A/B, measured: same tree with only specialize.rs reverted to
0aa9ada1eb1^, two full release builds.

case	pre-fix	this PR	CPython
store_subscr_cold_subclass	LiarInt	LiarInt	LiarInt
store_attr_cold_subclass	LiarInt	LiarInt	LiarInt
store_attr_cold_subclass_float	LiarFloat	LiarFloat	LiarFloat
truth_cold_subclass	19952	20000	20000

So only truth_cold_subclass observes the defect: LiarBool(0) is falsy by
payload and true by __bool__, and 48 of 20000 iterations — the compiled
region's share — took the payload answer.

The other three are already correct before the fix, so they verify nothing.
The truth_int gate is confirmed by observation; the store_attr and
store_subscr gates rest on reading the gates, not on an observed failure.

🤖 Generated with Claude Code

https://claude.ai/code/session_019gCeUzbGWCK8S6vqnXh416

Summary by CodeRabbit

• Bug Fixes

  • Improved conditional calls and predicate handling across supported CPU architectures.
  • Prevented numeric subclasses and custom boolean behavior from being incorrectly treated as built-in values during JIT specialization.
  • Improved integer and boolean conversion behavior, including overflow handling.
  • Preserved subclass types when storing values through optimized list, attribute, and subscript paths.

• Tests

  • Added coverage for cold-start and warm-up scenarios involving numeric and boolean subclasses.
  • Updated benchmark JIT statistics to reflect expanded coverage.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 58 minutes

Limit details: You’ve used all 2 included reviews currently available.

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?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2b1fbb20-e840-4d70-9aae-c99f08ad69d4

📥 Commits

Reviewing files that changed from the base of the PR and between e2ec9e4 and c5b894c.

📒 Files selected for processing (3)

• majit/majit-backend-dynasm/src/aarch64/assembler.rs
• majit/majit-backend-dynasm/src/x86/assembler.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 74e641bb-d14d-41ad-8d24-2a86f6145694

📥 Commits

Reviewing files that changed from the base of the PR and between 9be6297 and e2ec9e4.

📒 Files selected for processing (9)

• majit/majit-backend-dynasm/src/aarch64/assembler.rs
• majit/majit-backend-dynasm/src/x86/assembler.rs
• pyre/bench/synth/float_subclass_binop_dispatch.cranelift.jitstats
• pyre/bench/synth/float_subclass_binop_dispatch.dynasm.jitstats
• pyre/bench/synth/float_subclass_binop_dispatch.py
• pyre/bench/synth/float_subclass_binop_dispatch.wasm.jitstats
• pyre/pyre-interpreter/src/module/math/interp_math.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

Walkthrough

Conditional-call emitters now use register-allocated predicate locations on x86 and AArch64. JIT specialization now rejects subclass values for selected unboxed operations. Benchmarks cover these subclass cases and update recorded statistics.

Changes

Conditional-call emitters

Layer / File(s)	Summary
Allocated predicate loading in conditional calls majit/majit-backend-dynasm/src/x86/assembler.rs, majit/majit-backend-dynasm/src/aarch64/assembler.rs	Conditional-call operations now load predicates from allocated locations. x86 uses R11 for scratch loads and preserves RAX for predicate and result handling. AArch64 passes allocation locations into genop_cond_call_value and loads the predicate into x0.

Exact-value specialization

Layer / File(s)	Summary
Exact builtin checks and shared conversion pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs, pyre/pyre-interpreter/src/objspace/std/mapdict.rs, pyre/pyre-interpreter/src/module/math/interp_math.rs	Truth specialization and non-object list stores now require exact builtin values. Integer attribute-store admission uses is_unboxable_int. Integer-to-float conversion uses int_payload_as_f64.
Subclass behavior benchmarks pyre/bench/synth/float_subclass_binop_dispatch.py, pyre/bench/synth/float_subclass_binop_dispatch.*.jitstats	The benchmark adds warm and cold subclass storage tests and a cold subclass truth test. Recorded guard failures and compiled loops increase from 17 to 24.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to e2ec9

This PR tightens subclass handling and corrects conditional-call predicate sourcing across supported backends, with targeted checks reported as passing; no actionable merge-blocking risk remains beyond normal review and checks.

Poem

I’m a rabbit with registers neat,
Loading predicates before the beat.
Subclasses stay boxed, truth stays true,
Fresh benchmark counters hop into view.
One careful nibble, then tests all pass!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 5 files. (4 skipped: 3 unsupported, 1 too large.)	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: exactness gates for payload folds and DynASM predicate loading from argument locations.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nbody

---

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.}

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit c5b894c).
Updated: 2026-08-23T06:05:18.002Z

Files in the reviewed diff

majit/majit-backend-dynasm/src/aarch64/assembler.rs
majit/majit-backend-dynasm/src/x86/assembler.rs
pyre/bench/synth/float_subclass_binop_dispatch.py
pyre/pyre-interpreter/src/module/math/interp_math.rs
pyre/pyre-interpreter/src/objspace/std/mapdict.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

None.

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

• majit/majit-backend-dynasm/src/x86/assembler.rs:8034 ↔ rpython/jit/backend/x86/assembler.py:2501 — COND_CALL_VALUE_* still invokes emit_call(op, 1), which re-resolves tail operands from slots/constants. The regallocator instead supplies the live call arguments in arglocs; PyPy’s CondCallSlowPath.generate_body remaps those locations into ABI registers. A tail argument that remains register-resident is therefore not faithfully passed. This was already present on upstream/main; the patch only corrects the predicate load.

• majit/majit-backend-dynasm/src/aarch64/assembler.rs:6665 ↔ rpython/jit/backend/aarch64/regalloc.py:723 — the AArch64 COND_CALL_VALUE_* path likewise calls emit_call(op, 1) instead of consuming the prepared arglocs. PyPy prepares each tail argument in an ABI register before _emit_op_cond_call; resolving only frame/constant operands can lose a register-resident argument. This also predates the patch.

4. Structural adaptations

• pyre/pyre-interpreter/src/module/math/interp_math.rs:24 ↔ pypy/module/math/interp_math.py:20 — Pyre intentionally treats a float subclass as a payload-bearing float and ignores an overridden __float__; PyPy’s exact-type test falls through to space.float(w_x). This is CPython-compatible: lib-python/3/test/test_capi/test_float.py:112-113 asserts that PyFloat_AsDouble(FloatSubclass2(4.25)) returns the stored 4.25, despite FloatSubclass2.__float__ returning 4.25 independently. The difference is observable for a subclass whose payload and override differ. _get_double has no governing PyPy @jit.*, GC, or annotator hint.