Skip to content

Assemble the two remaining jitstats-touching PRs onto one head (#1026, #1082) - #1101

Merged
youknowone merged 10 commits into
mainfrom
jitstats-combined
Aug 7, 2026
Merged

Assemble the two remaining jitstats-touching PRs onto one head (#1026, #1082)#1101
youknowone merged 10 commits into
mainfrom
jitstats-combined

Conversation

@youknowone

@youknowone youknowone commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Hand-assembled so the .jitstats baselines can be measured once, on the
combined head, instead of once per PR against two different bases.

Stacked on #1100. The six commits above it are:

PR source taken commits
#1026 published PR head 1
#1082 local perf-bridge (13 commits ahead of the PR head) 5

The set has shrunk twice while this was being assembled: #1088 and #1092
merged, then #1085 and #1095 merged. What is left is these two. #999 was
excluded deliberately from the start — 36 commits, 61 source files, and
already conflicting with main; folding it in would make this branch a
rewrite of #999 rather than an assembly. It has since merged into main, so
this branch now carries it as base rather than as content.

Why assemble at all

.jitstats values cannot be merged. Each PR's numbers were measured against
its own base, so landing them one at a time re-invalidates the next one's
baselines. One head means one measurement pass.

Every baseline on this branch still has to be regenerated here — the values
currently carried are whatever the cherry-picks brought along, and are not
claims about this tree.

Conflicts resolved

Assembly. builtins.rs#1026 adds is_builtin_ord_function at the same
point main has is_builtin_hash_function. Sibling wrapped-code identity
tests; both kept. Picking a side would silently kill one PR's fold.
.jitstats conflicts took the incoming side, since all of them are regenerated
here anyway.

Rebase onto d936eb4. #1097 landed Cls.__name__ folding at exactly the
insertion point of #1026's class-namespace-value fold, in two files:

  • baseobjspace.rstype_name_obj_fast_path against
    type_attr_value_fast_path
  • specialize.rstry_walker_specialize_load_type_name_attr against
    try_walker_specialize_load_type_attr

Both kept, in full, in both files. They are siblings, not rivals: __name__
is a GetSetProperty on the metatype and therefore a data descriptor, so
#1026's fast path declines it on its own is_data_descr check and the two can
never claim the same read. The residual_call.rs LOAD_ATTR dispatch chain
merged with both calls present and in that order — __name__ first, then the
property-getter inline, then the general type-attribute fold.

Audit

An eight-agent pass over the files these PRs touch in common
(residual_call.rs, specialize.rs, inline_call.rs, builtins.rs,
vstack_mirror.rs, mod.rs, check.py) looked for arms whose relative order
the merge changed, guards dropped, arms duplicated, and comments the merged
code no longer matches. It raised one finding, which survived adversarial
verification — and it turned out to be on main rather than in the assembly.
It named widened_method_foriter_admissible, which #1097 has since deleted
outright; the finding is retired rather than fixed.

Three load-bearing declines were checked by hand and survive here: the
BUILD_TUPLE dispatch order, try_walker_specialize_newtuple_object's arity-2
decline, and the specialised-pair _oo subscript decline.

Not yet done

This branch has not been gated. It needs three backend builds, a full
.jitstats regeneration, and the CI set.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 15 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 @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 Plus

Run ID: ba903239-ca63-49af-9a82-402a1d9e4de7

📥 Commits

Reviewing files that changed from the base of the PR and between d936eb4 and 973ffe7.

📒 Files selected for processing (44)
  • majit/majit-metainterp/src/jitdriver.rs
  • majit/majit-metainterp/src/optimizeopt/unroll.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/trace_ctx.rs
  • pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats
  • pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats
  • pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats
  • pyre/bench/synth/getframe_caller_locals_after_resume.cranelift.jitstats
  • pyre/bench/synth/getframe_caller_locals_after_resume.dynasm.jitstats
  • pyre/bench/synth/getframe_caller_locals_after_resume.py
  • pyre/bench/synth/getframe_caller_locals_after_resume.wasm.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats
  • pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats
  • pyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstats
  • pyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstats
  • pyre/bench/synth/retrace_accumulator_type_flip.py
  • pyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstats
  • pyre/bench/synth/str_search_index_bounds.cranelift.jitstats
  • pyre/bench/synth/str_search_index_bounds.dynasm.jitstats
  • pyre/bench/synth/type_dict_surrogate.py
  • pyre/check.py
  • pyre/extra_tests/parity_tests/builtin_new_argument_count.py
  • pyre/extra_tests/parity_tests/str_padding_fill_operand.py
  • pyre/gate-triage.md
  • pyre/pyre-interpreter/src/baseobjspace.rs
  • pyre/pyre-interpreter/src/builtins.rs
  • pyre/pyre-interpreter/src/module/_ast/moduledef.rs
  • pyre/pyre-interpreter/src/type_methods.rs
  • pyre/pyre-interpreter/src/typedef.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
  • pyre/pyre-jit-trace/src/state.rs
  • pyre/pyre-jit-trace/src/trace_opcode.rs
  • pyre/pyre-jit/src/call_jit.rs
  • pyre/pyre-wasm-runner/src/main.rs
  • pyre/pyre-wasm/src/lib.rs
  • pyre/pyrex/src/lib.rs

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

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

@youknowone youknowone changed the title Assemble the four remaining jitstats-touching PRs onto one head (#1026, #1082, #1085, #1095) Assemble the two remaining jitstats-touching PRs onto one head (#1026, #1082) Aug 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/youknowone/pyre/blob/c2ba29f81554e1593ca10be3bb4232e08f84c22b/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs#L2757-L2762
P1 Badge Require exact builtin operands before replaying str calls

When an inline full-body walk evaluates str(x) for a subclass of int, long, float, complex, or str, these layout predicates still return true because builtin subclasses retain the builtin ob_type; for example, an int subclass can override __str__ and mutate state. The call is therefore incorrectly classified as side-effect-free, allowing a later abort to replay the opcode and invoke the override twice instead of taking the nested-residual decline. Use an exact Python-type check for every accepted operand type (and likewise for the is_str iterator classification below).

AGENTS.md reference: AGENTS.md:L14-L19

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

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 973ffe7).
Updated: 2026-08-07T14:16:57.248Z

Files in the reviewed diff
majit/majit-metainterp/src/jitdriver.rs
majit/majit-metainterp/src/optimizeopt/unroll.rs
majit/majit-metainterp/src/pyjitpl.rs
majit/majit-metainterp/src/trace_ctx.rs
pyre/bench/synth/getframe_caller_locals_after_resume.py
pyre/bench/synth/retrace_accumulator_type_flip.py
pyre/bench/synth/type_dict_surrogate.py
pyre/check.py
pyre/extra_tests/parity_tests/builtin_new_argument_count.py
pyre/extra_tests/parity_tests/str_padding_fill_operand.py
pyre/gate-triage.md
pyre/pyre-interpreter/src/baseobjspace.rs
pyre/pyre-interpreter/src/builtins.rs
pyre/pyre-interpreter/src/module/_ast/moduledef.rs
pyre/pyre-interpreter/src/type_methods.rs
pyre/pyre-interpreter/src/typedef.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
pyre/pyre-jit-trace/src/state.rs
pyre/pyre-jit-trace/src/trace_opcode.rs
pyre/pyre-jit/src/call_jit.rs
pyre/pyre-wasm-runner/src/main.rs
pyre/pyre-wasm/src/lib.rs
pyre/pyrex/src/lib.rs

Codex did not produce a report (exit 1). Last log lines:

Scope discipline: before writing the report, run
`git diff upstream/main --name-only -- . ':(exclude)*.jitstats'` and treat that
file list as the authoritative definition of "this patch" (when an authoritative
changed-file list is appended below, use that instead of re-deriving it). The
excluded `*.jitstats` files are `pyre/check.py`'s recorded jit-stats baselines —
generated golden data with no RPython/PyPy counterpart, so no parity finding can
cite one, and a bulk re-record of them is not a change to review. Findings under
sections 1 and 2 MUST cite our-side files from that list; a divergence in any
file NOT in the list is by definition not introduced by this patch — report
it under section 3 instead, or omit it. Verify every section-1/2 citation
against the list before finalizing the report.

---

Output format requirements (so the report can be parsed mechanically and
posted/triaged automatically). Use these four headings VERBATIM, in this
order, and nothing else at heading level 2:

## 1. Regressions to PyPy parity introduced by this patch
## 2. Other mismatches introduced by this patch
## 3. Pre-existing mismatches (already present before this patch)
## 4. Structural adaptations

Under each heading, list every finding as a bullet. For each finding cite the
concrete `our_file.rs:line ↔ rpython_or_pypy_file.py:line` pair and quote the
divergence concisely. If a section has no findings, still emit the heading
followed by a single line `None.` so all four sections are always present.
Do not modify any files; produce the report only.

Authoritative changed-file list for this patch (git diff upstream/main --name-only,
minus 18 generated `*.jitstats` baseline file(s)):
majit/majit-metainterp/src/jitdriver.rs
majit/majit-metainterp/src/optimizeopt/unroll.rs
majit/majit-metainterp/src/pyjitpl.rs
majit/majit-metainterp/src/trace_ctx.rs
pyre/bench/synth/getframe_caller_locals_after_resume.py
pyre/bench/synth/retrace_accumulator_type_flip.py
pyre/bench/synth/type_dict_surrogate.py
pyre/check.py
pyre/extra_tests/parity_tests/builtin_new_argument_count.py
pyre/extra_tests/parity_tests/str_padding_fill_operand.py
pyre/gate-triage.md
pyre/pyre-interpreter/src/baseobjspace.rs
pyre/pyre-interpreter/src/builtins.rs
pyre/pyre-interpreter/src/module/_ast/moduledef.rs
pyre/pyre-interpreter/src/type_methods.rs
pyre/pyre-interpreter/src/typedef.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
pyre/pyre-jit-trace/src/state.rs
pyre/pyre-jit-trace/src/trace_opcode.rs
pyre/pyre-jit/src/call_jit.rs
pyre/pyre-wasm-runner/src/main.rs
pyre/pyre-wasm/src/lib.rs
pyre/pyrex/src/lib.rs
warning: Codex could not find bubblewrap on PATH. Install bubblewrap with your OS package manager. See the sandbox prerequisites: https://developers.openai.com/codex/concepts/sandboxing#prerequisites. Codex will use the bundled bubblewrap in the meantime.
ERROR: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 8th, 2026 3:32 AM.
ERROR: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 8th, 2026 3:32 AM.

…ding

`try_walker_specialize_newtuple` runs before
`try_walker_specialize_newtuple_object` again, and the two dispatch comments
in `residual_call` go back to describing that order.  With arity 2 declined
in the canonical fold the two arms cover disjoint arities, so the order no
longer decides which one claims a pair.

`try_walker_specialize_newtuple_object`'s header and the concrete-shadow
comment go back to naming arity 2 as declined.  A paragraph records what a
side exit does when the decline is lifted: the trace hands a real pair, with
inline `value0` / `value1` and no `wrappeditems` block, to a consumer picked
for the canonical layout, and `try_walker_specialize_subscr_specialised_pair`
reads a field that is not there.

Assisted-by: Claude
`walker_capture_inline_nonstandard_vable_guard` published the callee's own
resume coordinate only when the paused-caller chain covered the full inline
depth (`n_parents > 0 && n_parents == n_callees`).  A chain of one callee
frame and no paused caller is covered by the frame list just as directly, so
it takes the same publish; every other shape still falls through to the
single-frame sentinel.

`walker_capture_multi_frame_inline_snapshot` reaches
`publish_outermost_parent_vable_scalars` with an empty `parent_frames` on that
new shape, so the call is made only when there is a parent to publish.

Assisted-by: Claude
`type_descr_new` located `(name, bases, dict)` by scanning the positional
arguments for the first `str` with two more behind it, and took the metatype as
the last preceding type. `descr__new__` (typeobject.py:886-911) keys the
one-versus-three form on `len(__args__.arguments_w)` instead, runs
`_precheck_for_new` unconditionally after that decision, and only then reads the
name. The scan reached `type_descr_new_with_metaclass` before the branch that
prechecks, so a non-type metatype was dropped rather than refused and
`type.__new__(42, 'A', (), {})` built a class.

The scan's header cited a `[self, metatype, name, bases, dict]` shape from
`super()`. No receiver is prepended: `super().__new__(mcls)` inside a metaclass
raises `M.__new__() takes exactly 3 arguments (1 given)`, which is the one-element
`pos` arm, and `super().__new__ is type.__new__` holds with `__self__` the type.
`pos[0]` is always the metatype.

`pos` being empty is a shape upstream's gateway cannot produce, since it supplies
`w_typetype` from a declared parameter; it is refused here in `tp_new_wrapper`'s
words rather than through the three-argument message.

`_calculate_metaclass` (typeobject.py:945) now receives the bases as written.
`(object,)` is substituted for an empty tuple in `W_TypeObject.__init__`
(`bases_w or [space.w_object]`), after the winner is settled; supplying it earlier
weighed an explicit metatype against `type(object)` and reported a metaclass
conflict for `type.__new__(int, 'A', (), {})`. The winner then passes
`check_user_subclass` (typeobject.py:555-567), which `allocate_instance` runs on
the way in and which is what names `int is not a subtype of type`.

`_ast`'s heap-type creation passed `[name, bases, dict]` and relied on the scan
matching at index 0; it now passes the metatype. The `_ctypes` metaclasses
already forwarded `[metatype, name, bases, dict]`.

`bool.__new__`'s over-arity refusal reported `got more`, the only such wording in
the tree; the gateway counts the class argument along with the value.

Assisted-by: Claude
`pad_fillchar` refused every non-`str` fill, and its own header recorded the
gap: "The decode itself is not imported". `descr_ljust` and `descr_rjust`
(unicodeobject.py:1352,1371) convert the operand with
`convert_arg_to_w_unicode` (unicodeobject.py:175-184), which declines `bytes`
by name and hands everything else to `decode_object`
(unicodeobject.py:1727-1739); that reads the operand as a buffer and decodes it
strict UTF-8, so a `bytearray` or `memoryview` becomes a fill character.
`descr_center` (unicodeobject.py:1101) reads its operand with `space.utf8_w`
and is unchanged.

The `"decoding to str: ..."` arm now reports only an operand that exports no
buffer at all, which is what `decode_object` maps the buffer error to. A buffer
whose bytes are not valid UTF-8 raises from the decode. The single-character
check applies to the decoded operand, so a multi-byte fill is rejected by its
code-point count.

Assisted-by: Claude
…tion pre-seed it guarded

The gate was default-off, so the block it guarded never executed in any
production run, CI run, or recorded baseline. What is removed is an unattested
ON path for a slice that was never built.

On the single-frame leg the pre-seed was redundant. `trace_and_compile_from_bridge`
publishes `cpu.grab_exc_value`'s result into `BH_LAST_EXC_VALUE` before bridge
tracing starts, zeroes it when the guard carried no exception, and declines the
bridge in the third case; `seed_standing_exception_for_walk` then runs at walk
start, after `setup_bridge_sym`, and for an exception-guard bridge returns from
one of its first two arms in every case, overwriting all five exception slots or
clearing them. It has no `last_exc_box.is_none()` short-circuit, so it always
wins. Measured with a probe at both seed sites over 369 synth benches: 23
exception-guard bridges in 14 benches, 7 of which the gate would have seeded,
and in all 23 the value at the walk seed equalled the value at the setup seed,
pointer for pointer. `PYRE_CARRIER_EXC_RESUME=1` over the whole corpus was
dynasm 386/386, byte-identical to gate-off, with no jit-stats movement.

The redundancy does not extend to the multi-frame carrier leg, which never runs
the walk seed: `setup_bridge_sym` installs the inline carrier whenever
`resume_data.frames.len() > 1`, `trace_bytecode` returns through
`drive_bridge_carrier_walk` before the full-body-walk leg, and
`drive_bridge_frame_subwalk` seeds its sub-walk off `root_sym.last_exc_box()`.
That leg is unexercised by bench/synth — all 23 bridges took the single-frame
leg — which is why the gate was never validated. gate-triage.md §1b records it,
and records that a rebuilt slice must seed from `BH_LAST_EXC_VALUE`.

`TraceCtx::bridge_guard_exc` and the `guard_exc` parameter of
`start_bridge_tracing` go with it: the deleted line was their only reader.
`guard_exc` stays live in call_jit.rs for the `BH_LAST_EXC_VALUE` publish,
`pending_exc`, `GuardExcRoot::park` and the blackhole resume.

gate-triage.md: §1b rewritten to retire the gate, keeping §1e's 732,660/170
census and its seven named benches as history and correcting the
"reachable, not inert" inference — the census instruments `handle_fail`, one
layer above the seed site. The booked `class_of_last_exc_is_const` delta is
withdrawn as miscited: `prepare_resume_from_failure` calls
`handle_possible_exception` three lines later (pyjitpl.py:3169), which ends
`class_of_last_exc_is_const = True` (pyjitpl.py:3416), so upstream's
post-resumption steady state is `True` too. §3 gains a row, Dead 12 -> 13, and
the default-OFF experiment count goes 2 -> 1.

check.py dynasm 387/387, cranelift 387/387, both after re-extracting LLBC.
Local macOS wasm shows four benches off by one or two `guard_failures`; CI at
the parent commit reports wasm 383/383, so those are left for CI to arbitrate.

Assisted-by: Claude
`getattr(C, name)` and `C.attr` each residualized as one opaque CallMayForce.
Its descr is unregistered, so the optimizer assumed the most pessimistic effect
tier and mandated a ForceToken plus two SetfieldGc and a GuardNotForced on every
iteration -- even though all four call arguments were already Const.

Add `type_attr_value_fast_path`, which admits only the typeobject.py:811-828
shape whose result is a class-namespace value returned unchanged: a cacheable
type receiver whose metaclass is exactly `type`, no metatype data descriptor for
the name, a class-MRO hit, and a value whose type is a non-heap builtin defining
no `__get__`. The name is taken as `&Wtf8` and resolved through
`lookup_in_type_wtf8`, so a lone or embedded surrogate takes the same path.

Two walker folds consume it: `try_walker_specialize_builtin_type_getattr`
(PyreHelperKind::CallFn) and `try_walker_specialize_load_type_attr`
(PyreHelperKind::LoadAttr). Each pins the callable, the receiver, the name
object and the receiver's version tag before writing the value as a green
constant. The version pin is a quasi-immutable watcher, so it emits no
per-iteration op, and `mutated` recurses into subclasses, so it also covers a
write to any base class. `is_builtin_getattr_function` recognizes the callable
through the shared `is_builtin_code_function` identity test.

Marginal cost of one loop-invariant read, dynasm, against an in-place revert
control arm (empty-loop baseline 5.6ns folded, 6.1ns on the control arm):
`C.attr` 144.7 -> 6.0ns, `getattr(C, n)` 269.8 -> 5.8ns, an embedded surrogate
1034.5 -> 8.7ns, a lone surrogate 1213.4 -> 6.3ns. Every folded reading is at
the empty-loop floor. The control-arm readings were taken with sibling builds
on the box and are inflated in absolute terms; the separation is not. They were
measured one base earlier and were not re-run here.

bench/synth/type_dict_surrogate: max-pypy-ratio 350 -> 21. Twelve local readings
of the folded fixture across the three backends span 3.4x-7.0x, and the ceiling
is three times the slowest; the derivation is in the header.

bench/synth/pickle_terminal_raise_resume: re-record the wasm baseline,
loops_aborted 13 -> 14 and loops_compiled 67 -> 66 -- one loop in the fixture
aborts where it used to compile. The bench's output is unchanged, its perf gate
is unaffected, and dynasm and cranelift are unchanged.

Assisted-by: Claude
A guard-failure blackhole that resumes inside an inlined callee transfers the
callee's return value to its caller and releases the callee interpreter, but
releasing a BlackholeInterpreter does not restore `topframeref`. The completed
callee stayed the current frame, so any later read of the caller's locals
selected it instead -- and an inlined callee's frame carries only its
parameters.

JIT-only, and identical on all three backends; CPython, pypy3 and
`PYRE_NO_JIT=1` agree with each other:

    def inner(k):
        if k == 2000:
            sys._getframe(1)         # one bare force, reads nothing
        return k

    def outer(n):
        base = 11
        acc = 0
        for i in range(n):
            acc += inner(i) & 7
        return sorted(locals().keys())

reads `['k']` instead of `['acc', 'base', 'i', 'n']`, and the same shape makes
`sys._getframe(1).f_locals['base']` raise KeyError. One bare `sys._getframe(1)`
is enough -- no attribute of the frame has to be read, and the caller observes
it in its own `locals()` with no `sys._getframe` at the read.

`leave_resumed_blackhole_frame` performs the `executioncontext.py:91-107 leave`
frame transition before the innermost blackhole is released, guarded on
`topframeref` still resolving to that frame, and takes `got_exception` from the
call site: the two exception-propagation paths pass true and the return-value
transfer passes false.

It forces no vref. The guard already establishes that the outgoing vref
resolves to this frame, so `leave`'s own `frame_vref()` has nothing left to
materialize, and the caller is reached through `vref_referent` rather than
`get_f_back`. Forcing there panicked synth/exception_escape_inlined_midframe_tb_node
on wasm with `InvalidVirtualRef: frame-chain vref forced after its frame died`,
because the compiled frame is already gone by this point.

bench/synth/getframe_caller_locals_after_resume: new. It asserts the `f_locals`
read and the caller's own `locals()` together, since either can hold while the
other breaks, and it needs two triggering calls -- a single one passes with or
without the change. max-pypy-ratio is three times the slowest of nine local
readings across the three backends, which span 1.8x-5.5x.

check.py: dynasm 392/392, cranelift 392/392, wasm 388/388.

Assisted-by: Claude
…osses bytecode offsets

pyre compiled no retrace at all. A 3-backend MC_DIAG census over 397 synth
fixtures left one refusal cluster, identical on dynasm, cranelift and wasm and
confined to `synth/retrace_accumulator_type_flip`: five `cb_retrace_req`, five
`wct_declined`, five `ct_compile_bridge_false`, five `close_hdr_fallback` and
five `retrace_arity_giveup`. `retrace_limit` defaults to 0
(`rpython/rlib/jit.py:595`), so that fixture — which sets it to 5 — is the only
one in the corpus that reaches `compile_retrace`.

pypy3 on the same file compiles the retrace: under a "bridge out of Guard 0x..."
header it emits a float-specialised `label(p0, p11, p5, p6, f24, i26, i16)` and
closes onto it. `jit-summary` still reads "loops: 1 / bridges: 1" there, so the
summary does not report a retrace; the label inside the bridge artifact does.

`close_loop_args_at` publishes a stack depth for the duration of the JUMP-arg
derivation, and `merge_point_stack_depth_to_recover` supplied it only when the
merge point's static depth EXCEEDED the depth the frame still advertises. This
retrace resumes from a guard at one bytecode offset and closes onto the loop
header at another, where the static depth is shallower — 3 against the resumed
frame's 5 — so the helper declined and `flush_to_frame` pinned the stale 5 as a
constant. `valuestackdepth` is virtual-state index 4 of the
`[frame, ec, last_instr, pycode, valuestackdepth, ...]` closing-JUMP layout, so
the retrace then failed to match even the target token it had just created:

    [jit][jte] virtualstate mismatch index=4 box=ConstInt(5)
      expected=... Constant(Int(3)) ... incoming=... Constant(Int(5)) ...
    [jit][jte] target_token #2 generate_guards failed (force_boxes=false)

Only after that did `jump_to_preamble` run, and its arity give-up (MC_DIAG slot
57) refused the 13-arg body JUMP against the 8-arg preamble LABEL. That
give-up is the symptom, not the cause.

A bytecode offset has exactly one operand-stack depth, so the helper now takes
the target's depth in either direction. Narrowing loses nothing: JUMP args are
sized by `target_array_capacity` — the full virtualizable array
(`nlocals + ncells + co_stacksize`) — while the published depth only sets the
live prefix, and slots above it are null-padded capacity that
`materialize_fail_arg_slot` fills from `concrete_value_at`. The comment that
claimed narrowing "would lose a value the JUMP must carry" was wrong about its
own consumer, and is corrected along with the unit test, which now covers
deep-start/shallow-target as well as the reverse.

bench/synth/retrace_accumulator_type_flip: `loops_aborted` 5 -> 0 and
`guard_failures` 1202 -> 202 on all three backends; `loops_compiled` and
`bridges_compiled` stay 1 and 1, because the counters class the attached
retrace as that bridge. Its header is rewritten: the fixture used to document
the give-up it reached, and now documents the assembled retrace and the
cross-offset closure that produces it.

check.py: dynasm 392/392, cranelift 392/392, wasm 388/388.

Assisted-by: Claude
The retrace the previous commit made pyre assemble was observable in jit-stats
only as `loops_aborted` 5 -> 0 — an absence. A later change that stops pyre
REQUESTING the retrace leaves `loops_aborted` at 0 and LOWERS `guard_failures`,
and `_jit_stats_change` classes a `guard_failures` fall as IMPROVED, so the
corpus's only retrace coverage would be re-recorded away without anyone being
told. `retrace_limit` defaults to 0 (`rpython/rlib/jit.py:595`), so
`synth/retrace_accumulator_type_flip` is the only bench that reaches
`compile_retrace` at all and there is no second witness to notice.

`retraces_compiled` is bumped where a retrace is attached, next to the
`attached retrace to guard` log, so it counts an assembled artifact rather than
a request or a give-up. It is emitted by both `[jit-stats]` surfaces — `pyrex`
for the native backends and `pyre-wasm-runner` for wasm, the latter through a
`pyre_jit_retraces_compiled` guest export, since the host resolves each counter
by name and reports a missing one rather than reading it as zero.

check.py gates it on a FALL, the polarity `loops_compiled` already carries: the
regression is the artifact ceasing to exist. `_jit_stats_change` reads a field
absent from either side as "0", so the other 391 baselines gate on the new
counter without being re-recorded; only the one bench whose value is non-zero
is re-recorded, on all three backends.

Both directions of the counter were checked against the built binary rather
than assumed: `retrace_accumulator_type_flip` reports
`bridges_compiled=1 retraces_compiled=1`, while `bench/fannkuch` reports
`bridges_compiled=22 retraces_compiled=0` and `bench/nbody`
`bridges_compiled=5 retraces_compiled=0` — twenty-seven ordinary bridge
attachments move the counter not at all. Each backend's suite reported the
`retraces_compiled 0 -> 1` gate itself, wasm included, which is what shows the
export reaches the host.

check.py: dynasm 391 passed, cranelift 391 passed, wasm 387 passed, each with
the single expected `retraces_compiled 0 -> 1` re-record.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/youknowone/pyre/blob/973ffe7fead4d516c642c6b435dc33b789396904/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs#L2758-L2761
P1 Badge Require semantic exactness before replaying str()

is_int_or_long, is_float, is_complex, and is_str check the physical Rust layout, so they also accept user subclasses whose w_class defines __str__; builtin_str explicitly dispatches that override. In a nested full-body walk, such a call is therefore incorrectly classified as side-effect-free, bypassing fbw_abort_nested_unjournaled_residual; if the walk later aborts and replays, the user __str__ runs twice and can duplicate arbitrary mutations. Use semantic exact-type checks for these operands before granting replay safety.

AGENTS.md reference: AGENTS.md:L16-L18

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

@youknowone
youknowone merged commit 133c9d7 into main Aug 7, 2026
13 of 17 checks passed
@youknowone
youknowone deleted the jitstats-combined branch August 7, 2026 16:13
youknowone added a commit that referenced this pull request Aug 8, 2026
The folds themselves landed in #1101; what is left of this commit is the
cross-reference between them.

`type_name_obj_fast_path` / `try_walker_specialize_load_type_name_attr` cover
typeobject.py:814-819, the metatype-data-descriptor arm that `__name__`
resolves through. `type_attr_value_fast_path` /
`try_walker_specialize_load_type_attr` cover :820-822, the class-MRO-value
arm, and their oracle refuses any name the metatype answers with a data
descriptor. Each doc comment now names its own arm and points at the other, so
a reader landing on either knows the two are disjoint rather than redundant.

Assisted-by: Claude
youknowone added a commit that referenced this pull request Aug 8, 2026
…d-admit call boundary (#1082)

* jit: name which descr_getattribute arm each type-attribute fold covers

The folds themselves landed in #1101; what is left of this commit is the
cross-reference between them.

`type_name_obj_fast_path` / `try_walker_specialize_load_type_name_attr` cover
typeobject.py:814-819, the metatype-data-descriptor arm that `__name__`
resolves through. `type_attr_value_fast_path` /
`try_walker_specialize_load_type_attr` cover :820-822, the class-MRO-value
arm, and their oracle refuses any name the metatype answers with a data
descriptor. Each doc comment now names its own arm and points at the other, so
a reader landing on either knows the two are disjoint rather than redundant.

Assisted-by: Claude

* jit: relocate a bridge's target tokens, and refuse a cross-buffer JUMP to an unrelocated target

`assemble_bridge` never ran the `compiled_target_tokens` relocation that
`assemble_loop` inlined, so a LABEL assembled inside a bridge left its
`ll_loop_code` holding the in-buffer offset it was given at emission. A retrace
is attached to a guard, so it is assembled through the bridge path and carries
its own LABEL; a later trace closing onto that token missed
`target_tokens_currently_compiling`, read the field, and baked the offset as an
absolute branch target — `br x16` with 0x1fc, a deterministic SIGSEGV on a
nested loop whose inner accumulator changes type.

Extract the relocation into `fixup_target_tokens` and call it from both assemble
paths, matching `aarch64/assembler.py:1087` (called at `:98` and `:213`) and
`x86/assembler.py:990` (`:612`, `:706`). Both dynasm backends had the omission;
cranelift publishes an absolute address instead and is unaffected. The list is
consumed, so a token cannot be relocated twice.

A cross-buffer JUMP whose target is below the first page now fails the compile
rather than emitting the branch. Dynasm bakes the immediate at codegen, so
neither 0 (target never compiled) nor an unrelocated offset can be repaired
later; both are always wild branches.

Add `jit_retrace_nested_loop_bridge_label`, which SIGSEGVs before this change.

Assisted-by: Claude

* jit: name the FOR_ITER deferred-admit call-boundary property instead of inferring it

The `CalleeReplaySafety::DeferredCall` arm admitted a callee when
`arg_class_guard.is_none()`, standing in for "the entry is a CALL the abort
rewind can name". The arm's comment justified the proxy by asserting a binop
dunder dispatch was the only entry carrying an `arg_class_guard`. It is not:
`try_walker_inline_subscr_getitem` enters from `BINARY_OP` and passes no
`arg_class_guard`, so `obj[key]` was admitted and resumed one operand short —
the subscript index was replaced at runtime by an unrelated live Ref, failing
`test.test_re` in CI with

    TypeError: list indices must be integers or slices, not list_iterator

`try_walker_inline_resolved_user_call` now takes an explicit
`entry_is_call_boundary`, and the arm reads it directly. Audited per call site
against the old predicate's value: `try_walker_inline_user_binop` (BINARY_OP)
and `try_walker_inline_user_compareop` (COMPARE_OP) already evaluated to
`false` and keep it; `try_walker_inline_subscr_getitem` changes from `true` to
`false` and is the only behaviour change. `try_walker_inline_property_get`,
`try_walker_inline_property_set` and
`try_walker_specialize_seqiter_getitem_next` enter from LOAD_ATTR, STORE_ATTR
and FOR_ITER rather than a CALL but stay `true`, which is what they evaluated
to before; each says so at its call site.

What the gate asks is the entry opcode's stack effect, not the spelling: an
entry that only peeks re-executes from the stack the rewind already has. The
FOR_ITER call site carries that reason now rather than a reference to the
property routes — `opcode_for_iter` peeks its single iterator operand where
`opcode_binary_op` pops both of its own. The two attribute entries reach
their stack discipline through `load_attr_cached` / `store_attr_cached`, which
this audit did not follow, so they keep the standing they had and say so.

Add `jit_subscr_getitem_inline_keeps_its_index`, which raises that TypeError
before this change and is JIT-only.

Assisted-by: Claude
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant