wasm: stamp w_class on NEW_WITH_VTABLE; drop the wasm-only LIST_APPEND residual decline - #770
Merged
Conversation
…effect guard A module/global-scope `for i in …` lowers the loop variable to a STORE_NAME / STORE_GLOBAL residual (not a STORE_FAST frame local). Executed on the authoritative walk while the FOR_ITER item is in flight, that store set `body_effect_since_consume` (residual_call.rs body_effect_candidate), so a later `VableEscapedDuringResidualCall` abort in the loop body made `fbw_foriter_inflight_take` refuse re-delivery and drop the iteration — resuming at the FOR_ITER header re-consumed the iterator and skipped the item, losing the body's effects (e.g. the `exc_info_inside` / `exc_info_after` counters in exception_metadata_hot). Re-delivery re-runs the body from the FOR_ITER continue-arm fallthrough, which re-binds the SAME re-delivered item to the SAME name — an idempotent write, not an accumulating double. Exempt it from the R1 body-effect accounting when the store's py_pc is the in-flight entry's body_pc, matching the existing `is_idempotent_gc_barrier` treatment. Add `fbw_foriter_inflight_top_body_pc` and a `exc_info_module_loop_hot` regression bench. Assisted-by: Claude
`fuse_boxing_alloc` (majit-translate model.rs) lowers a boxing constructor such
as `malloc_typed(W_IntObject { … })` to `NewWithVtable` + the payload store,
dropping the `ob_header` subtree on the contract that the backend stamps both
`ob_type` and `w_class` from the size descr — `w_class_obj()` (majit-ir
descr.rs) documents that a `new_with_vtable` inherits
`get_instantiate(vtable_type)` and that OptVirtualize folds `w_class` header
reads to that constant. The wasm backend wrote only the vtable, so a
materialized box carried whatever `w_class` bytes the allocation left behind.
On wasm those bytes are zero (`Nursery::reset` memsets on this target), so a
JIT-allocated `W_IntObject` had `w_class == null` while an interpreter-created
one carries the `int` type object. In a loop whose induction variable escapes
into the heap (`while i < N: lst.append(i); i = i + 1`) the promoted
`GuardValue(w_class, int_typeobj)` then failed on every trace entry; the
recovery bridge declined at setup and `declined_bridge_guards` short-circuited
every later failure to the interpreter.
Stamp `w_class` after the vtable write, mirroring dynasm
`genop_new_with_vtable`. On `append_only` (700k appends) trace entries drop
from 233267 to 13 and the declined-bridge short-circuit from 233065 to 0;
`synth/list_reverse` goes 52.8x -> 4.7x against CPython. wasm synthetic suite
289/289.
Assisted-by: Claude
#749 routed a LIST_APPEND fold decline (dominantly the realloc boundary) into the generic `jit_list_append` residual instead of aborting the trace, but kept a `cfg!(target_arch = "wasm32")` arm that still returned `UnfoldableListAppendResidualUnsupported`, because the wasm backend then miscompiled the resize/append bridge into an element drop. That arm aborts the bridge on every guard failure. On `nested_list_comprehension_hot` the wasm run enters `must_compile` 997002 times against 998000 trace entries, starts bridge tracing 4984 times, and reaches `compile_bridge` zero times; the abort census is `UnfoldableListAppendResidualUnsupported` 4992 (dynasm: 0, and it compiles six bridges). The trace records 25-35 body ops before aborting, so the zero-body-op structural-decline path never latches it and every attempt is retried. The drop no longer reproduces: with the arm removed, `check.py` is dynasm 309/309, cranelift 309/309, wasm 306/306, and the wasm times drop from 19.70s to 1.42s (`comprehension_object_append_hot`), 7.78s to 0.36s (`nested_list_comprehension_hot`), and 3.16s to 0.29s (`const_arg_call_resume`) — 34.0x/59.8x/21.1x against dynasm down to 2.3x/2.4x/1.9x. Five apparent regressions in the same table re-measure at their pre-change times. The element drop belongs to the wasm offset-0 silent-null class, whose effect is layout-dependent and benign on macOS aarch64, so this local run does not cover the Linux x86_64 manifestation; CI does. Assisted-by: Claude
… benches
check.py's regression floor (`_jit_stats_regression_floor`) fails a run when
`loops_aborted` or `internal_compile_panics` rises above the committed
`<bench>.<backend>.jitstats` baseline. Only `pyre/bench/*.py` carried such
baselines; `pyre/.gitignore` treated every `bench/synth/*.jitstats` as local
scratch, so no synthetic bench was gated on those counters.
Record and commit the baselines for the three benches the removed wasm-only
`ListAppendValue` residual decline covered, and add matching `!` exceptions to
`pyre/.gitignore`. Recorded values:
nested_list_comprehension_hot loops_aborted=2
comprehension_object_append_hot loops_aborted=2
const_arg_call_resume loops_aborted=1
dynasm and cranelift record byte-identical counters, and the residual aborts
are `LoopBearingCalleeInlineUnsupported { pc: 116 }` on the module-level trace
in all three benches, so the values come from the backend-independent tracer.
This is the only gate that covers wasm on these benches: `run_synthetic_bench`
passes `vs_pypy=None` for wasm, so its `max-pypy-ratio` header is not read, and
the per-bench timeout alone let the pre-fix 8.58s run pass. With the decline
arm in place the bridge aborted on every guard failure (4984 aborts recorded in
`PYRE_WASM_JIT_STATS`), which the floor now reports as
`jit-stats regression: loops_aborted 2 -> N`.
Verified by writing `loops_aborted=0` into the wasm baseline and running
`check.py --backend wasm`: `SNAPDIFF jit-stats regression: loops_aborted 0 -> 2`.
Assisted-by: Claude
WalkthroughWasm allocation now materializes ChangesWasm and JIT behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 c657d63). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats`:
- Line 4: Resolve or explicitly reclassify the known loops_aborted cases before
updating baselines, preserving pyre/check.py’s healthy zero expectation: adjust
pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats:4-4 and
comprehension_object_append_hot.dynasm.jitstats:4-4 after fixing their aborts,
keep pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats:2-2 from
weakening wasm’s sole JIT regression gate, and remove the aborts from
pyre/bench/synth/const_arg_call_resume.cranelift.jitstats:4-4 and
const_arg_call_resume.dynasm.jitstats:4-4.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs`:
- Around line 1505-1528: Regenerate the Charon .ullbc extraction after updating
residual_call.rs, including the related fbw_state.rs change, then run all eight
required JIT benchmarks and verify there is no regression; include the
regenerated artifacts and benchmark evidence with the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e564bba-d2e4-46ff-aaaa-7b36bee02fde
📒 Files selected for processing (14)
majit/majit-backend-wasm/src/codegen.rspyre/.gitignorepyre/bench/synth/comprehension_object_append_hot.cranelift.jitstatspyre/bench/synth/comprehension_object_append_hot.dynasm.jitstatspyre/bench/synth/comprehension_object_append_hot.wasm.jitstatspyre/bench/synth/const_arg_call_resume.cranelift.jitstatspyre/bench/synth/const_arg_call_resume.dynasm.jitstatspyre/bench/synth/const_arg_call_resume.wasm.jitstatspyre/bench/synth/exc_info_module_loop_hot.pypyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstatspyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstatspyre/bench/synth/nested_list_comprehension_hot.wasm.jitstatspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
| bridges_compiled=18 | ||
| guard_failures=3700 | ||
| internal_compile_panics=0 | ||
| loops_aborted=2 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The committed baselines weaken the loops_aborted regression gate.
pyre/check.py documents healthy baselines as zero, but these fixtures commit existing aborts. Fix or explicitly reclassify the underlying aborts before recording baselines:
pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats#L4-L4: change the nonzeroloops_abortedbaseline after resolving the abort.pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats#L4-L4: change the nonzeroloops_abortedbaseline after resolving the abort.pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats#L2-L2: avoid weakening wasm’s only stated JIT regression gate.pyre/bench/synth/const_arg_call_resume.cranelift.jitstats#L4-L4: remove the known abort from the healthy baseline.pyre/bench/synth/const_arg_call_resume.dynasm.jitstats#L4-L4: remove the known abort from the healthy baseline.
📍 Affects 5 files
pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats#L4-L4(this comment)pyre/bench/synth/comprehension_object_append_hot.dynasm.jitstats#L4-L4pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats#L2-L2pyre/bench/synth/const_arg_call_resume.cranelift.jitstats#L4-L4pyre/bench/synth/const_arg_call_resume.dynasm.jitstats#L4-L4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats` at line
4, Resolve or explicitly reclassify the known loops_aborted cases before
updating baselines, preserving pyre/check.py’s healthy zero expectation: adjust
pyre/bench/synth/comprehension_object_append_hot.cranelift.jitstats:4-4 and
comprehension_object_append_hot.dynasm.jitstats:4-4 after fixing their aborts,
keep pyre/bench/synth/comprehension_object_append_hot.wasm.jitstats:2-2 from
weakening wasm’s sole JIT regression gate, and remove the aborts from
pyre/bench/synth/const_arg_call_resume.cranelift.jitstats:4-4 and
const_arg_call_resume.dynasm.jitstats:4-4.
Comment on lines
+1505
to
1528
| // The loop-variable binding store is the op at the in-flight FOR_ITER's | ||
| // `body_pc` (the FOR_ITER continue-arm fallthrough), a STORE_NAME/ | ||
| // STORE_GLOBAL that writes the just-consumed item to the loop target (a | ||
| // module/global-scope `for i in …`; a function-scope loop var is a | ||
| // STORE_FAST frame local that never becomes a residual). Re-delivery | ||
| // re-runs the body from `body_pc`, re-storing the SAME re-delivered item to | ||
| // the SAME name — an idempotent write, never an accumulating double. Like | ||
| // the `is_idempotent_gc_barrier` write barrier it still EXECUTES concretely | ||
| // (the module dict must hold the binding for the walk's remaining reads) but | ||
| // it is not a body effect: keep it out of the R1 in-flight-FOR_ITER | ||
| // accounting so an escaping residual later in the same body does not | ||
| // refuse-drop the whole iteration. | ||
| // `vstack_cur_pypc` points one past the executing op (next-instr | ||
| // convention), while `body_pc` is the store's own py_pc, so the loop-var | ||
| // store satisfies `vstack_cur_pypc == body_pc + 1`. | ||
| let is_loop_var_binding_store = matches!( | ||
| helper, | ||
| majit_ir::PyreHelperKind::StoreName | majit_ir::PyreHelperKind::StoreGlobal | ||
| ) && fbw_foriter_inflight_top_body_pc() | ||
| .is_some_and(|body_pc| body_pc + 1 == ctx.vstack_cur_pypc as usize); | ||
| let body_effect_candidate = !provably_side_effect_free | ||
| && !is_idempotent_gc_barrier | ||
| && !is_loop_var_binding_store | ||
| && writes_live_heap |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)pyre/pyre-jit-trace/src/jitcode_dispatch/(residual_call|fbw_state)\.rs$|extract-llbc\.py|README|benchmark' || true
echo "== tracked changes/stat =="
git diff --stat || true
echo "== residual_call helper call context =="
if [ -f pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs ]; then
wc -l pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
sed -n '1470,1545p' pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs | cat -n
fi
echo "== fbw_state helper context =="
if [ -f pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs ]; then
wc -l pyre/pyre/jit-trace/src/jitcode_dispatch/fbw_state.rs
sed -n '660,710p' pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs | cat -n
fi
echo "== git diff around JIT files =="
git diff -- pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs | sed -n '1,220p' || true
echo "== search benchmark run / ullbc evidence =="
git diff -- pyre/pyre-jit-trace | rg -n '(benchmark|eight|ULLBC|ullbc|Charon|Charon|scripts/extract-llbc\.py|all eight|no-regression|regression)' || true
echo "== possible committed artifact changes in diff =="
git diff --name-only -- pyre | rg '\.ullbc$|pyre/pyre-jit-trace' || trueRepository: youknowone/pyre
Length of output: 7742
🌐 Web query:
RPython Charon ULLBC extraction scripts extract-llbc.py Pyre JIT changes Charon .ullbc
💡 Result:
Charon is a tool designed to interface between the Rust compiler (rustc) and program verification projects by converting Rust's Mid-level Intermediate Representation (MIR) into more stable, simplified formats [1][2]. These formats include ULLBC (Unstructured Low-Level Borrow Calculus) and LLBC (Low-Level Borrow Calculus), which are serialized as JSON files with .ullbc and .llbc extensions [1][3]. ULLBC represents a cleaned-up, slightly simplified version of MIR, while LLBC further restructures the control-flow to replace gotos with higher-level constructs like loops and if-then-else blocks [3]. In the context of the pyre project—a fast, compliant Rust-based Python interpreter that utilizes the MaJIT meta-tracing JIT framework—extract-llbc.py is a Python utility script used to automate the extraction of these artifacts [4][5]. Key aspects of this process include: - Deterministic Extraction: The script computes source fingerprints over Git-tracked inputs to manage extraction and caching [4]. It skips unnecessary re-extractions unless the inputs have changed or a --force flag is used [4]. - Environment Setup: It helps maintain stamp files per crate and handles environment configuration, such as MSVC link detection on Windows, replacing older, less robust shell scripts [4]. - Integration: The script is used within pyre's development workflow (e.g., python3 scripts/extract-llbc.py <target>) to ensure that the required .ullbc and .llbc artifacts are present for the JIT analysis and translation pipeline [4][6]. - JIT Context: By using Charon to extract MIR-derived artifacts, pyre can perform static analysis on the Rust source (via majit-analyze) to facilitate JIT compilation, serving a role similar to RPython's translation process in PyPy but tailored for a Rust-based architecture [5]. Documentation for the ULLBC structure itself can be found in the Charon library's internal definitions, specifically in modules like ullbc_ast.rs, which define the AST and serialization formats used [7][8].
Citations:
- 1: https://github.com/AeneasVerif/charon/blob/c3cfb06be901b9fcf33f52ddbf06029b6a8ad727/README.md
- 2: https://github.com/AeneasVerif/charon/blob/1103c5168ba1d9b8d3a7346863dc0fe7bb43379e/charon.opam
- 3: https://github.com/AeneasVerif/charon/blob/30cab88265206f4fa849736e704983e39a404d96/README.md
- 4: cache llbc #175
- 5: https://github.com/youknowone/pyre
- 6: https://github.com/youknowone/pyre/blob/main/memoryview.plan.md
- 7: https://aeneasverif.github.io/charon/charon_lib/ast/ullbc_ast/index.html
- 8: https://aeneasverif.github.io/charon/src/charon_lib/ast/ullbc_ast.rs.html
Run the required artifact regeneration and benchmark evidence for this JIT change.
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs now changes JIT source, so the corresponding Charon .ullbc extraction must be regenerated and all eight benchmarks must run with no regression before this change can be accepted.
📍 Affects 2 files
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs#L1505-L1528(this comment)pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs#L685-L695
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs` around lines 1505
- 1528, Regenerate the Charon .ullbc extraction after updating residual_call.rs,
including the related fbw_state.rs change, then run all eight required JIT
benchmarks and verify there is no regression; include the regenerated artifacts
and benchmark evidence with the change.
Source: Coding guidelines
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four commits on top of
main, all found by chasing the largest wasm-vs-dynasmruntime gaps in
pyre/check.py --synthetic-only.wasm: stamp w_class on NEW_WITH_VTABLEfuse_boxing_alloclowersmalloc_typed(W_IntObject { .. })toNewWithVtable+ a payload store, dropping the boxing ctor'sob_headersubtree on the contract that the backend stamps both
ob_type(the vtable)and
w_class(get_instantiate(vtable_type)) from the size descr. dynasm'sgenop_new_with_vtabledoes this; the wasm backend wrote only the vtable.wasm also zero-fills the nursery on reset, so the missing store left
w_class == 0, and the promotedGuardValue(w_class, int_typeobj)thatOptVirtualizefolds those header reads to failed on every iteration of anyloop that escapes a freshly boxed builtin.
synth/list_reverseon wasm:executes=233267,decl_shortcircuit=233065→executes=13,decl_shortcircuit=0; 52.8x → 4.7x vs CPython.cranelift's
NewWithVtablehas the same omission. It is latent there onlybecause that nursery is not zeroed on reset, so recycled memory usually still
holds a plausible
w_class; not fixed here.jit: drop the wasm-only LIST_APPEND residual-fallthrough safeguardThe top three wasm-vs-dynasm gaps were all one arm in
jitcode_dispatch/residual_call.rs. #749 let a declinedLIST_APPENDfold fallthrough to the generic
jit_list_appendresidual, but keptif cfg!(target_arch = "wasm32") { return Err(UnfoldableListAppendResidualUnsupported) }because wasm miscompiled the resulting resize/append bridge. That aborted the
bridge on every guard failure, forever:
FIRED=4984,sbt_entered=4984,cb_entered=0,bridge_diag entered=0— nothing ever compiled.Removing it (9 deleted lines, no replacement — the backend split disappears):
synth/nested_list_comprehension_hotsynth/comprehension_object_append_hotsynth/const_arg_call_resumesilent-null class: wasm32 linear-memory offset 0 is valid memory, so a null
Refis a layout-dependent wrong answer rather than a trap, and it does notreproduce on macOS aarch64 in principle. An ablation (safeguard off and the
w_classstamp above reverted) still passed locally, so thew_classfix isnot proven to be what cured it — #737's bridge-iter journal root is the likelier
fixer. Linux CI is the real gate for this commit. If it reddens, hunt the
null
Refin the append-residual bridge rather than restoring the arm.bench: commit jit-stats baselines for the three append-residual synth benchesNothing caught the above:
run_synthetic_benchpassesvs_pypy=Nonefor wasm,so the
max-pypy-ratioheader is not read there and the per-bench timeout alonelet the pre-fix 8.58s run pass.
check.py'sloops_aborted/internal_compile_panicsregression floor would have caught it, butpyre/.gitignoretreated everybench/synth/*.jitstatsas local scratch.Records and commits the baselines for these three benches (
loops_aborted=2 / 2 / 1, byte-identical on dynasm and cranelift; the residual aborts are
LoopBearingCalleeInlineUnsupported { pc: 116 }on the module-level trace) withmatching
!exceptions inpyre/.gitignore. Verified by writingloops_aborted=0into the wasm baseline:SNAPDIFF jit-stats regression: loops_aborted 0 -> 2.jit: exempt the FOR_ITER loop-variable store from the in-flight body-effect guardPre-existing commit carried on this branch; a module-level loop-variable
STORE_NAMEwas classified as a body effect and aborted the trace withVableEscaped.Verification
pyre/check.pylocally (macOS aarch64): dynasm 309/309, cranelift 309/309,wasm 306/306.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
sys.exc_info()behavior inside and after repeated exception handlers.