codewriter, jit-trace: re-land the goto_if_not fusion with the four walker sites it needs

[youknowone]

Re-lands 0dacccbfe2d, the optimize_goto_if_not fusion that #1664 measured,
found green in the shipped jitcode and then backed out because it broke the
JIT. The revert was right about the symptom and wrong about the cause: the
fusion is fine, and four walker sites did not know the opname it produces.

Why the revert was necessary, and what it actually was

The revert recorded pyre/check.py --backend dynasm at 478 passed / 60
failed with the fusion against 535 / 3 without, with this signature across
the 57 it added:

• loops_aborted 0 -> N, loops_compiled down,
  fbw_blackhole_adopted_single_frame 0 -> N
• pyre/bench/synth/listcomp_hot.py raising
  TypeError: 'int' object is not an iterator from for j in range(n)

That is the same signature as the int_is_zero walker gap fixed in #1664, and
the same cause. goto_if_not_int_is_true/iL is its own byte
(BC_GOTO_IF_NOT_INT_IS_TRUE = 15, against BC_GOTO_IF_NOT = 18), and pyre's
walker matches on the key string, not the byte. The fusion put 9284 of that
key into the shipped jitcodes; every site naming only the unfused spelling
stopped recognizing the branch.

Four sites — and two of them compare the bare opname

Searching for the full key "goto_if_not/iL" finds two. Two more compare
op.opname without argcodes, which is why the revert's own investigation
("body_branch_targets decodes any L argcode") cleared the scans wrongly:

site	what it does	what missing the key does
jitcode_dispatch/mod.rs dispatch	the arm	UnsupportedOpname, walk aborts mid-body
jitcode_dispatch/mod.rs reachability scan (op.key)	enqueues branch targets	the taken arm never reaches the worklist, which reads as an unreachable block
branch.rs decode_side_other_target (op.opname)	rebuilds a guard's not-taken arm from orgpc	Err("notgoto")
branch.rs branch_arm_reads_unrestorable_ref (op.opname)	straight-line boundary	the scan runs past the guard into a block belonging to another resume coordinate

The three non-dispatch sites now recognize the family by prefix and read the
label as the final operand, which every member spells the same way (iL,
iiL, rL, rrL, ffL), so the index is argcodes.len() - 1.

The dispatch arm is not an alias of the plain one

blackhole.py aliases them — bhimpl_goto_if_not_int_is_true = bhimpl_goto_if_not — and the walker's old comment cited exactly that alias as
the reason it needed no arm. But the walker is the metainterp side, and
pyjitpl.py opimpl_goto_if_not_int_is_true executes rop.INT_IS_TRUE and
hands the fresh condbox to opimpl_goto_if_not(..., replace=False). Routing
the fused key to the plain arm would record no int_is_true and let
replace rewrite the value box to a constant — and for the fused form that box
holds x, not the boolean the branch tested. So it uses
fused_goto_if_not_int_unary(.., OpCode::IntIsTrue), the exact sibling of the
existing int_is_zero arm.

Measured

Counting body.code[pos] for every pos in body.startpoints across the
shipped jit_metadata.json:

	before	after	delta
total instructions	152,888	142,164	−10,724 (−7.0%)
goto_if_not/iL	9,284	0	−9,284
goto_if_not_int_is_true/iL	0	9,284	+9,284
standalone int_is_true/i>i	9,348	80	−9,268
live/	31,726	30,052	−1,674
ptr_ne/rr>i → goto_if_not_ptr_ne/rrL	228 / 8	213 / 23	−15 / +15

9,268 standalone int_is_true ops disappear along with the 1,674 -live-
markers that preceded them, and each frees its destination register.
jit_metadata.json is 726 KB smaller.

Gates

• pyre/check.py --backend dynasm — 541 / 541 ALL PASSED
• pyre/check.py --backend cranelift — 541 / 541 ALL PASSED

Both runs report no jit-stats change at all: the counters the .jitstats
sidecars record did not move, so nothing needed --snapshot despite the 7%
instruction cut. The one within band (not gated) line
(synth/foriter_load_special_with) is present on the base too.

Runtime is not claimed here. The static reduction above is deterministic;
the local bench ratios were taken under a loaded machine on the "before" side
and are not a measurement. The CI pyre/check.py jobs on this PR against
main's own run at de7e1a70159 are the apples-to-apples comparison.

— authored by Claude

[coderabbitai]

Warning

Review limit reached

Next included review available in 19 minutes.

Check out review usage here.

View limit details

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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: ef16a48a-dff9-4095-886f-7d62bb940b2f

📥 Commits

Reviewing files that changed from the base of the PR and between de7e1a7 and 97a2753.

📒 Files selected for processing (4)

• majit/majit-translate/src/codewriter/flatten.rs
• majit/majit-translate/src/codewriter/jtransform.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/branch.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/mod.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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-03T13:02:49.146885Z	97a2753	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 97a2753).
Updated: 2026-09-03T13:08:44.818Z

Files in the reviewed diff

majit/majit-translate/src/codewriter/flatten.rs
majit/majit-translate/src/codewriter/jtransform.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/branch.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs

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

behaviour of the CPython in `lib-python/3` (version pinned in
`lib-python/stdlib-version.txt`), where PyPy and that CPython genuinely differ.
This is usually NOT a 3.11-vs-3.14 delta but a standing PyPy-vs-CPython
divergence; "CPython did not change in 3.14" is not grounds to refile it under
1 or 2. It qualifies only when the finding carries all four of:
(a) an observable difference — return value, exception type/message/attributes,
    identity, encoding-and-errors contract, or accepted argument shapes;
(b) a cited CPython artefact — a `lib-python/3/...:line` assertion, a measured
    run at the pinned version, or C read at that tag in a named checkout. Not
    docs, not a PEP, not a comment in pyre's own source;
(c) the PyPy `file:line` that decides, showing the two upstreams actually
    differ (if PyPy contradicts itself, pyre following PyPy's own declaration
    is section 4 as ordinary parity);
(d) no PyPy-side JIT/GC/annotator hint governing the value being changed —
    `@jit.*`, `_immutable_*`, `_attrs_`, `make_sure_not_resized`,
    `unrolling_iterable`, `rgc.*`, on the function, its helpers, or the class-
    and module-level bindings they read.
Missing any of (a)-(d), or leaving pyre matching NEITHER upstream on an
adjacent observable of the same decision, keep it in section 1 or 2 and say
which test it failed. Full rule: AGENTS.md "Spec follows CPython 3.14;
implementation follows PyPy".

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 0 generated `*.jitstats` baseline file(s)):
majit/majit-translate/src/codewriter/flatten.rs
majit/majit-translate/src/codewriter/jtransform.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/branch.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.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 Sep 7th, 2026 2:28 AM.
ERROR: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 7th, 2026 2:28 AM.

[codspeed-hq]

Merging this PR will not alter performance

✅ 10 untouched benchmarks
⏩ 6 skipped benchmarks¹

---

_{Comparing jitcode (97a2753) with main (de7e1a7)}

Footnotes

1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩