Skip to content

wasm JIT: bring nbody/fannkuch/fib to a few× of pypy (bridge chaining + CALL_ASSEMBLER default-on)#347

Merged
youknowone merged 12 commits into
mainfrom
wasm-jit
Jul 3, 2026
Merged

wasm JIT: bring nbody/fannkuch/fib to a few× of pypy (bridge chaining + CALL_ASSEMBLER default-on)#347
youknowone merged 12 commits into
mainfrom
wasm-jit

Conversation

@youknowone

@youknowone youknowone commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Refs #262.

Goal

Make the major benchmarks run at a sane speed on the wasm backend — per
#262, "at least better than naive and a few times slower than native".
Previously nbody / fannkuch / fib timed out or ran orders of magnitude
too slow on wasm.

Result

python3 pyre/check.py --backend wasm --no-syntheticwasm 10/10,
all within a few × of pypy:

bench pypy wasm ratio
fib_recursive 0.21s 3.02s 14.4x
nbody 0.12s 3.61s 30.1x
fannkuch 0.19s 2.12s 11.1x
spectral_norm 0.04s 0.21s 5.2x

(fannkuch was ~13.8s before bridge chaining; fib_recursive was ~24s.)

Changes

  • backend-wasm: bridge chaining + CALL_ASSEMBLER fast paths, default-on.
    The dominant levers for the loop/recursion-heavy benchmarks. Chained
    bridges use a global fail-index with nested chaining; self-recursive
    CALL_ASSEMBLER runs on GC JitFrames.
  • jit-trace / codewriter / optimizeopt correctness fixes that the
    above surfaced: word-ABI void helpers recorded through a sized descr;
    BhSizeSpec/BhDescr vtable widened to u64 (wasm32); runtime-minted
    mutable descrs treated as written by every non-elidable call
    (stale-global CSE fix); bridge walks excluded from
    terminate_no_replay store-commit.
  • check.py: wasm-only, nbody-only bounded float tolerance. nbody's
    **/pow output drifts ~1490 ULP (3e-13 rel) on wasm32 because libm's
    arch-specific pow building blocks (arch/aarch64.rs) match the platform
    libm on native but fall back to generic software on wasm32 — an
    unfixable target-ISA codegen gap, not a miscompile (FMA-fusion
    hypothesis was tested and refuted; host-libm callbacks rejected as
    host-dependent). Opt-in per bench via run_bench(..., wasm_float_tol=True);
    every other wasm bench and all other backends stay byte-exact.
  • builtins: hasattr/getattr attribute-name check through checkattrname
    (parity fix carried on this branch).
  • ci: save rust-cache on main only, restore-only on PRs — the
    still-applicable part of ci: save rust-cache on main only; finish LLBC artifact handoff #209 (its LLBC-artifact commit is already
    superseded on main); keeps per-PR target caches from multiplying the
    repo past GitHub's 10 GB cache budget.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added new WASM runtime toggles for bridges and inline allocation.
    • Added GC collection counters and richer JIT/runtime diagnostics.
    • Improved benchmark output handling for WASM with float-tolerant comparison on supported cases.
    • Added more detailed execution tracing and statistics in the WASM runner.
  • Bug Fixes

    • Improved bridge and allocation handling for WASM execution paths.
    • Tightened attribute-name and call/descriptor handling for more consistent behavior.
    • Fixed diagnostic reporting so more early-exit and fallback paths are now counted.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 22 minutes

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: 417b418b-728e-4658-99af-29b7e58c10b6

📥 Commits

Reviewing files that changed from the base of the PR and between 4b8a341 and 20ff264.

📒 Files selected for processing (29)
  • .github/workflows/pyre-ci.yml
  • majit/majit-backend-wasm/src/codegen.rs
  • majit/majit-backend-wasm/src/failguard.rs
  • majit/majit-backend-wasm/src/lib.rs
  • majit/majit-backend-wasm/tests/codegen_test.rs
  • majit/majit-gc/src/collector.rs
  • majit/majit-gc/src/lib.rs
  • majit/majit-metainterp/src/call_descr.rs
  • majit/majit-metainterp/src/history.rs
  • majit/majit-metainterp/src/jitcode/assembler.rs
  • majit/majit-metainterp/src/jitdriver.rs
  • majit/majit-metainterp/src/lib.rs
  • majit/majit-metainterp/src/optimizeopt/heap.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/pyjitpl/dispatch.rs
  • majit/majit-translate/src/codewriter/assembler.rs
  • majit/majit-translate/src/codewriter/jitcode.rs
  • pyre/check.py
  • pyre/pyre-interpreter/src/builtins.rs
  • pyre/pyre-jit-trace/src/descr.rs
  • pyre/pyre-jit-trace/src/helpers.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs
  • pyre/pyre-jit-trace/src/state.rs
  • pyre/pyre-jit-trace/src/trace.rs
  • pyre/pyre-jit/src/call_jit.rs
  • pyre/pyre-jit/src/eval.rs
  • pyre/pyre-jit/src/lib.rs
  • pyre/pyre-wasm-runner/src/main.rs
  • pyre/pyre-wasm/src/lib.rs

Walkthrough

This PR reworks the wasm JIT backend's bridge-chaining and guard-resolution machinery around global fail-descriptor and label-target registries, adds inline nursery-bump allocation and CA frame pooling, widens vtable identity to u64, introduces void-word-ABI call descriptors, expands diagnostics, adds wasm float-tolerant benchmark comparison, refactors attribute-name checks, and adjusts CI cache saving.

Changes

Wasm Backend Bridge, Allocation, and CA Machinery

Layer / File(s) Summary
Global fail-descriptor and label-target registries
majit/majit-backend-wasm/src/failguard.rs
Adds LabelTarget, FRAME_REF_HOME_FLOOR, global FAIL_DESCR_REGISTRY/LABEL_TARGETS registries, ChainedTraceMeta, reworked CompiledWasmLoop metadata, and a Drop impl retracting label targets.
GC diagnostics
majit/majit-gc/src/collector.rs, majit/majit-gc/src/lib.rs
Adds collection_counts() and type_alloc_is_plain() to GcAllocator with defaults, implemented in MiniMarkGC.
Write-barrier redesign
majit/majit-backend-wasm/src/codegen.rs
Redesigns emit_write_barrier with inline slow-path/trampoline fallback and updates field/array/interior-field write-barrier call sites.
Inline nursery allocation fast path
majit/majit-backend-wasm/src/codegen.rs
Adds NurseryAllocParams and inline bump-allocation fast paths for New/NewWithVtable/NewArray with eligibility checks and reload adjustments.
build_wasm_module signature and dispatch rework
majit/majit-backend-wasm/src/codegen.rs, majit/majit-backend-wasm/tests/codegen_test.rs
Extends module-building signature with nursery, is_loop, external_jump_key; reworks label-less dispatch and CA frame call_indirect paths; updates test call sites.
Loop compilation wiring
majit/majit-backend-wasm/src/lib.rs
Adds CA frame pooling, process-global toggles, nursery_alloc_params, and wires fail_index_base/label_descrs/label publishing into loop compilation.
compile_bridge rework
majit/majit-backend-wasm/src/lib.rs
Reworks guard resolution, loop-closing acceptance, livelock checks, CA gating, and bridge/fail-descr lookups via the new global registries.
Void-word-ABI descriptors and diagnostics
majit/majit-metainterp/src/call_descr.rs, majit/majit-metainterp/src/history.rs, pyre/pyre-jit-trace/src/helpers.rs, majit/majit-metainterp/src/lib.rs, majit/majit-backend-wasm/src/lib.rs, majit/majit-metainterp/src/jitdriver.rs, majit/majit-metainterp/src/pyjitpl.rs, pyre/pyre-jit/src/call_jit.rs, pyre/pyre-jit/src/lib.rs, pyre/pyre-wasm/src/lib.rs, pyre/pyre-wasm-runner/src/main.rs, pyre/pyre-jit-trace/src/trace.rs, pyre/pyre-jit-trace/src/jitcode_dispatch.rs
Adds void-word-ABI call descriptors and helper wiring, expands MC_DIAG/BRIDGE_DIAG counters, defaults bridges enabled, and adds GC/toggle exports and diagnostic printing.

Vtable Identity Width Unification

Layer / File(s) Summary
Widen vtable to u64
majit/majit-translate/src/codewriter/*, pyre/pyre-jit-trace/src/descr.rs, pyre/pyre-jit-trace/src/state.rs, pyre/pyre-jit/src/eval.rs, majit/majit-metainterp/src/pyjitpl/dispatch.rs, majit/majit-metainterp/src/jitcode/assembler.rs
Changes BhSizeSpec/BhDescr::Size vtable fields to u64, updating construction, keying, and accessor sites accordingly.

Heap Effect Invalidation

Layer / File(s) Summary
Conservative invalidation
majit/majit-metainterp/src/optimizeopt/heap.rs
Treats runtime-minted field/array descriptors with unset ei_index as written unless always pure.

Wasm Float-Tolerant Benchmarks

Layer / File(s) Summary
wasm_outputs_match plumbing
pyre/check.py
Adds float-tolerant token comparison and wasm_float_tol opt-in, enabled for nbody.

Builtin Attribute-Name Validation

Layer / File(s) Summary
Shared checkattrname usage
pyre/pyre-interpreter/src/builtins.rs
Consolidates hasattr/getattr attribute-name validation onto the shared helper.

CI Cache Gating

Layer / File(s) Summary
Restrict cache saves to main
.github/workflows/pyre-ci.yml
Adds save-if conditions so caches are saved only on the main branch across five jobs.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant compile_bridge
  participant chained_trace_meta
  participant LABEL_TARGETS
  participant FAIL_DESCR_REGISTRY
  compile_bridge->>FAIL_DESCR_REGISTRY: resolve failing guard via global_fail_descr
  compile_bridge->>chained_trace_meta: lookup cells_base and guard_fail_arg_advanced
  compile_bridge->>LABEL_TARGETS: label_target(descr_id) for loop-closing jump
  LABEL_TARGETS-->>compile_bridge: resume_safe, num_ref_homes, arity
  compile_bridge->>compile_bridge: livelock check via guard_fail_arg_advanced
Loading

Possibly related PRs

  • youknowone/pyre#41: Builds on the same global fail-descriptor registration/identity work now consumed by FAIL_DESCR_REGISTRY/global_fail_descr.
  • youknowone/pyre#312: Directly related through the same self-recursive CALL_ASSEMBLER/bridge-chaining machinery in codegen.rs, failguard.rs, and lib.rs.
  • youknowone/pyre#257: Shares the same wasm write-barrier and collecting-nursery Ref-home invariants extended by this PR's inline allocation fast paths.

Poem

A rabbit hops through fail-index lanes,
Chaining bridges where the guard remains,
Nursery bumps for babies born so quick,
Vtables widened, u64 the trick,
Diagnostics blink like carrot-lit stars — 🥕✨
Hop on, dear code, past all the guard bars!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main wasm JIT performance focus and highlights bridge chaining and CALL_ASSEMBLER defaults.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wasm-jit

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.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit d2eee85).

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)

None.

4. Structural adaptations

  • majit/majit-translate/src/codewriter/jitcode.rs:1087 ↔ rpython/jit/backend/llsupport/descr.py:117: Pyre stores serialized BhSizeSpec.vtable as u64; PyPy keeps SizeDescr(..., vtable=vtable) as an in-process lltype pointer. This is a Rust/wasm serialization adaptation, not a parity regression.

  • majit/majit-metainterp/src/call_descr.rs:569 ↔ rpython/jit/codewriter/effectinfo.py:271: Pyre adds make_call_descr_void_word_abi(... result_type=Void, result_size=8) for wasm signature-exact call_indirect; PyPy’s EffectInfo.MOST_GENERAL and calldescr path do not encode a dummy word return for void calls. This is an implementation-language/backend ABI adaptation.

  • majit/majit-metainterp/src/optimizeopt/heap.rs:1951 ↔ rpython/jit/codewriter/effectinfo.py:496: Pyre conservatively invalidates mutable runtime-minted field descrs with unset ei_index; PyPy assigns descr.ei_index = sys.maxint only during one closed compute_bitstrings(all_descrs) universe. This is a structural adaptation for Pyre descrs minted after setup.

  • majit/majit-metainterp/src/optimizeopt/heap.rs:1986 ↔ rpython/jit/codewriter/effectinfo.py:496: Same adaptation for array descrs: Pyre treats unset runtime-minted mutable array descrs as written by non-elidable calls, while PyPy’s descr universe is closed before bitstrings are used.

  • majit/majit-backend-wasm/src/codegen.rs:335 ↔ rpython/jit/backend/x86/assembler.py:2434: Pyre emits a wasm inline header-flag test plus call_indirect slow path for the write barrier; PyPy x86 emits TEST8(...jit_wb_if_flag...) and patches a backend slowpath. Same semantic shape, backend-specific lowering.

  • majit/majit-backend-wasm/src/codegen.rs:720 ↔ rpython/jit/backend/llsupport/gc.py:525: Pyre passes explicit wasm nursery addresses/threshold/type eligibility into NurseryAllocParams; PyPy exposes get_nursery_free_addr() / get_nursery_top_addr() through the backend GC descriptor. This is backend plumbing, not semantic divergence.

  • pyre/pyre-interpreter/src/builtins.rs:4974 ↔ pypy/module/__builtin__/operation.py:69: Pyre’s hasattr now calls shared checkattrname; PyPy does the same. This patch restores parity compared to main’s hasattr(): attribute name must be string special case.

@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

Here are some automated review suggestions for this pull request.

Reviewed commit: 75291c4adb

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

// as an advance.
let mutates_heap = ops.iter().any(|op| {
use majit_ir::OpCode::*;
op.opcode.is_call()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exclude pure calls from the no-advance escape hatch

When a loop-closing bridge has no advanced JUMP args but contains a CallPure* or CallLoopinvariant*, op.opcode.is_call() still makes mutates_heap true, so the no-advance livelock check accepts the bridge even though neither loop-carried state nor heap state changed. With wasm bridge chaining default-on, that side trace can tail-call the loop header with the same state indefinitely instead of declining to the blackhole fallback; restrict this escape hatch to calls that may actually mutate state.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 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 `@majit/majit-metainterp/src/optimizeopt/heap.rs`:
- Around line 1948-1951: The field-write check in the optimizeopt heap logic has
a redundant identity fallback: once the `effect_idx == u32::MAX &&
!descr.is_always_pure()` branch is in place, `writes_field_descr_by_identity()`
in the `writes_field` expression no longer affects the result. Update the
`writes_field` condition in the `heap.rs` optimization path to remove that dead
fallback, or if it must remain for clarity, revise the nearby comment to match
the broader invalidation behavior in the
`compute_bitstrings_has_run()`/JitDriver path. Use the
`check_write_descr_field`, `writes_field_descr_by_identity`, and
`compute_bitstrings_has_run` symbols to locate the logic.

In `@pyre/check.py`:
- Around line 399-406: The tolerance comparison in the float-matching logic is
using a manual absolute-difference threshold instead of the clearer standard
helper. Update the finite-number branch in the function that parses `out_tok`
and `exp_tok` to use `math.isclose(a, b, rel_tol=WASM_FLOAT_RTOL, abs_tol=0.0)`
after the existing `math.isfinite` guard, and remove the manual `abs(a - b)`
threshold check.

In `@pyre/pyre-wasm-runner/src/main.rs`:
- Around line 814-824: The PYRE_WASM_EXEC_TRACE lookup in the trace path is
being re-evaluated on every jit_execute round-trip, which adds avoidable
overhead. Cache this flag once during Host initialization alongside other
startup state like stdlib_root, then have the tracing block in main.rs use that
stored field instead of calling std::env::var_os each time. Update the
trace-gating logic around the exec_hist recording to read from the Host field so
the fast path stays free of repeated environment access.
🪄 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

Run ID: 183f35a1-b128-4e1a-a9f3-61c6be2d3ea5

📥 Commits

Reviewing files that changed from the base of the PR and between 827696c and 4b8a341.

📒 Files selected for processing (29)
  • .github/workflows/pyre-ci.yml
  • majit/majit-backend-wasm/src/codegen.rs
  • majit/majit-backend-wasm/src/failguard.rs
  • majit/majit-backend-wasm/src/lib.rs
  • majit/majit-backend-wasm/tests/codegen_test.rs
  • majit/majit-gc/src/collector.rs
  • majit/majit-gc/src/lib.rs
  • majit/majit-metainterp/src/call_descr.rs
  • majit/majit-metainterp/src/history.rs
  • majit/majit-metainterp/src/jitcode/assembler.rs
  • majit/majit-metainterp/src/jitdriver.rs
  • majit/majit-metainterp/src/lib.rs
  • majit/majit-metainterp/src/optimizeopt/heap.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/pyjitpl/dispatch.rs
  • majit/majit-translate/src/codewriter/assembler.rs
  • majit/majit-translate/src/codewriter/jitcode.rs
  • pyre/check.py
  • pyre/pyre-interpreter/src/builtins.rs
  • pyre/pyre-jit-trace/src/descr.rs
  • pyre/pyre-jit-trace/src/helpers.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs
  • pyre/pyre-jit-trace/src/state.rs
  • pyre/pyre-jit-trace/src/trace.rs
  • pyre/pyre-jit/src/call_jit.rs
  • pyre/pyre-jit/src/eval.rs
  • pyre/pyre-jit/src/lib.rs
  • pyre/pyre-wasm-runner/src/main.rs
  • pyre/pyre-wasm/src/lib.rs

Comment thread majit/majit-metainterp/src/optimizeopt/heap.rs
Comment thread pyre/check.py
Comment on lines +399 to +406
try:
a, b = float(out_tok), float(exp_tok)
except ValueError:
return False
if not (math.isfinite(a) and math.isfinite(b)):
return False
if abs(a - b) > WASM_FLOAT_RTOL * max(abs(a), abs(b)):
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Python math.isclose rel_tol abs_tol default semantics

💡 Result:

In Python, the math.isclose function determines whether two values are considered close based on provided relative and absolute tolerances [1][2]. The default semantics for these parameters are: rel_tol (relative tolerance): The default value is 1e-09 [3][1]. This parameter defines the maximum allowed difference between the two inputs, relative to the larger absolute value of the inputs [1][2]. It must be a non-negative value and less than 1.0 [3]. abs_tol (absolute tolerance): The default value is 0.0 [3][1]. This parameter specifies a minimum absolute tolerance, which is primarily useful for comparisons near zero where relative tolerance becomes ineffective [4][1]. It must be non-negative [3][1]. The mathematical condition for two values a and b to be considered close is: abs(a - b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol) [1][2][5] If the difference between the values is less than or equal to the larger of the two tolerance checks, the function returns True; otherwise, it returns False [1][2]. Special IEEE 754 values are handled such that NaN is not close to any value (including itself), and infinities are only considered close to themselves [1][2][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant region and surrounding imports in pyre/check.py
wc -l pyre/check.py
sed -n '1,120p' pyre/check.py
printf '\n---\n'
sed -n '360,430p' pyre/check.py

# Probe the exact equivalence of the manual check vs math.isclose on representative cases
python3 - <<'PY'
import math

WASM_FLOAT_RTOL = 1e-9

def manual(a, b):
    if not (math.isfinite(a) and math.isfinite(b)):
        return False
    return not (abs(a - b) > WASM_FLOAT_RTOL * max(abs(a), abs(b)))

def isclose(a, b):
    if not (math.isfinite(a) and math.isfinite(b)):
        return False
    return math.isclose(a, b, rel_tol=WASM_FLOAT_RTOL, abs_tol=0.0)

cases = [
    (0.0, 0.0),
    (0.0, 1e-12),
    (1.0, 1.0 + 1e-12),
    (1e20, 1e20 + 1e10),
    (-1e20, -1e20 * (1 + 1e-10)),
    (1e-300, 2e-300),
    (float("inf"), float("inf")),
    (float("nan"), float("nan")),
    (1.0, -1.0),
]
for a, b in cases:
    print(a, b, manual(a, b), isclose(a, b))
PY

Repository: youknowone/pyre

Length of output: 8735


Use math.isclose for the tolerance check. The finite-number guard already excludes inf/nan, so this can be expressed more clearly as math.isclose(a, b, rel_tol=WASM_FLOAT_RTOL, abs_tol=0.0) instead of a manual threshold comparison.

🤖 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/check.py` around lines 399 - 406, The tolerance comparison in the
float-matching logic is using a manual absolute-difference threshold instead of
the clearer standard helper. Update the finite-number branch in the function
that parses `out_tok` and `exp_tok` to use `math.isclose(a, b,
rel_tol=WASM_FLOAT_RTOL, abs_tol=0.0)` after the existing `math.isfinite` guard,
and remove the manual `abs(a - b)` threshold check.

Source: Linters/SAST tools

Comment on lines +814 to +824
// Diagnostic: record which (trace, guard-exit fail_index) round-tripped.
if std::env::var_os("PYRE_WASM_EXEC_TRACE").is_some() {
if let Some(mem) = caller.data().memory {
let fail_index = read_u32(&mem, &*caller, frame_ptr as usize);
*caller
.data_mut()
.exec_hist
.entry((func_id, fail_index))
.or_insert(0) += 1;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache the PYRE_WASM_EXEC_TRACE check instead of re-reading the env on every trace execution.

std::env::var_os(...) runs unconditionally on every jit_execute call — a path invoked on every host round-trip from a running trace, which this PR is specifically trying to minimize/speed up. Reading it once into a Host field at startup (like stdlib_root) avoids the per-call environment lookup.

♻️ Proposed fix
 struct Host {
     ...
+    /// Cached `PYRE_WASM_EXEC_TRACE` env check, computed once at startup.
+    exec_trace_enabled: bool,
     exec_hist: std::collections::BTreeMap<(u32, u32), u64>,
 }
     // Diagnostic: record which (trace, guard-exit fail_index) round-tripped.
-    if std::env::var_os("PYRE_WASM_EXEC_TRACE").is_some() {
+    if caller.data().exec_trace_enabled {
         if let Some(mem) = caller.data().memory {
             let fail_index = read_u32(&mem, &*caller, frame_ptr as usize);
🤖 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-wasm-runner/src/main.rs` around lines 814 - 824, The
PYRE_WASM_EXEC_TRACE lookup in the trace path is being re-evaluated on every
jit_execute round-trip, which adds avoidable overhead. Cache this flag once
during Host initialization alongside other startup state like stdlib_root, then
have the tracing block in main.rs use that stored field instead of calling
std::env::var_os each time. Update the trace-gating logic around the exec_hist
recording to read from the Host field so the fast path stays free of repeated
environment access.

@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

Here are some automated review suggestions for this pull request.

Reviewed commit: c8c8ed6e34

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

// and the header — the peeled iteration — which advances the state
// before the loop re-runs, so no advance is required of the bridge
// itself.
if bridge_is_loop_closing && resumes_at_loop_header {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Don't skip no-advance checks for earlier labels

When a loop-closing bridge targets an earlier label in a peeled loop, this condition bypasses the no-advance/livelock check entirely. That is only safe if the target-to-header segment is guaranteed to advance state, but LabelTarget records only resume_safe; an earlier label after the induction update, or whose suffix only copies/tests values, can re-enter the header with byte-identical args and heap, so bridge chaining can spin instead of falling back to blackhole. Either publish/prove an advances-to-header bit for earlier labels or keep the no-advance check for these targets.

Useful? React with 👍 / 👎.

youknowone added 12 commits July 3, 2026 21:10
`terminate_no_replay` keeps a Terminate walk's eager list stores
(journal-commit) so the loop-free function portal can return the walk's
result without replaying it. A bridge Terminate walk does not go through
that portal: its caller resumes the guarded region through the blackhole,
which re-applies the stores. Keeping them here as well double-applied
them. Gate the predicate on `!is_bridge_trace` so bridge walks roll their
stores back before the blackhole replay.

Assisted-by: Claude
…rname

hasattr/getattr validated the name inline with their own
"hasattr()/getattr(): attribute name must be string" messages; route both
through checkattrname (operation.py:41-45), which raises the unified
"attribute name must be string, not '<type>'" message setattr/delattr
already use.

Assisted-by: Claude
…very non-elidable call

A descr minted at runtime (ei_index unset, u32::MAX) is outside the
compute_bitstrings universe, so no call's write bitstring can ever name
it; a bitcheck miss proves nothing. Treating the miss as "not written"
let optheap keep a getfield cache across a store residual and re-read a
stale value (module-global cell fold: 'acc = acc + a; acc = acc + b'
dropped the first term). Field and array descr loops in
force_from_effectinfo now conservatively invalidate mutable
out-of-universe descrs on every non-elidable call.

Assisted-by: Claude
…ller-supplied effect

jit_store_name_to_namespace / jit_list_append are extern "C" helpers
whose C signature returns a dummy machine word (-> i64, value ignored)
but were recorded as plain void residuals. Add
make_call_descr_void_word_abi (result_type=Void, result_size=8, distinct
interning key) and TraceCtx::call_void_typed_word_abi so a
signature-exact backend lowering (wasm call_indirect) can select the
(i64xn) -> i64 type and drop the result. The new entry point takes a
caller-supplied EffectInfo — these helpers write the heap (namespace
cells, list storage), and the opcode-default empty write set let optheap
CSE a getfield across the call; both call sites record MOST_GENERAL
(graphanalyze.py:60 analyze_external_call for an unanalyzed external
writer). The call_*_typed record tail is factored into
record_call_with_descr (invalidate-before-record, pyjitpl.py:2683-2684).

Assisted-by: Claude
The vtable field is an ob_type pointer captured in the producing process
(the opcode_descrs.bin build script) and crosses the build->runtime
serialization boundary. Declared usize, a 64-bit host pointer failed
bincode's width check when deserialized on a wasm32 runtime. Store it as
u64 and cast at the consumers; the value is an opaque identity word
either way (real vtables are re-resolved via type_id -> gc_cache).

Assisted-by: Claude
…walk prints

MC_DIAG grows 8 -> 18 slots: compile_bridge entry/InvalidLoop/
retrace_requested/arity-giveup (8-11), start_bridge_tracing entry and its
early-return reasons (12-17), and stack_almost_full()==true (5, formerly
reserved). Under the existing fbw debug-abort flag, the full-body walker
also prints a bridge-walk epilogue line (commit state + outcome kind) and
a note when a bridge STORE_SUBSCR specialization declines to the generic
residual.

Assisted-by: Claude
…fault

Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges
resolve guard exits in-module through a global fail-index space with
nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined
bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to
default-true and the dormant-tracer comment block is dropped.

CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on):
- alloc/pop frame helpers lower through the residual (i64xn)->i64
  call_indirect type family instead of the jit_call trampoline
  (residual_max_arity forced >= 2 when the CA arm is emitted).
- CA callee frames are libc-allocated jitframes registered with the
  shadow stack and recycled through a LIFO pool, replacing per-call
  old-gen allocation that forced back-to-back major collections
  (gc_majors 1704 -> 0 on fib_recursive).
- Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are
  enabled, mirroring execute_token.
- CA on a loop that already has bridges (or further chaining on a loop
  whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing
  the two on one recursion produces wrong output on
  recursion_memo_branch/generator_tree_recursion (task #7).

Inline fast paths in generated code:
- write barrier: test the TRACK_YOUNG_PTRS header flag inline and only
  call_indirect the helper when set (genop_discard_cond_call_gc_wb).
- nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/
  NewWithVtable and constant-length NewArray/NewArrayClear bump
  nursery_free inline (header word + array length store) and only call
  the collecting helper on overflow, gated on the new
  type_alloc_is_plain trait (no destructor, not a weakref), size below
  max_nursery_object_size, and gc_stress off.

Diagnostics: collection_counts on the GC trait (MiniMarkGC override),
pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest
exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner
labels for the new MC_DIAG slots.

fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm
suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift).

Assisted-by: Claude
The wasm float tolerance is now opt-in per bench via
run_bench(..., wasm_float_tol=True) rather than applied to every wasm
bench. Only nbody is marked; all other wasm benches require
byte-identical output. The comparison gate becomes
`backend == "wasm" and wasm_float_tol`, threaded from run_bench through
_run_backend_bench. Corrects the wasm_outputs_match root-cause note: the
divergence is libm's arch-specific pow building blocks, not FMA fusion
(FMA hypothesis tested and refuted).

Assisted-by: Claude
Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five
Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check,
cpython-tests, wasm-build). PR jobs become restore-only and pull main's
default-branch cache via restore-keys, so each PR no longer persists its
own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB
cache budget.

This is the still-applicable part of #209; that PR's other commit
(LLBC handoff to wasm-build via artifact) is already superseded on main,
where wasm-build restores LLBC through download-artifact.

Assisted-by: Claude
DEFAULT_BACKENDS gains "wasm" whenever `rustup target list --installed`
reports wasm32-unknown-unknown, the sole extra prerequisite for building
the wasm backend (the wasmtime runtime is embedded in pyre-wasm-runner,
not an external tool). On a machine without that target the probe returns
false and the defaults stay dynasm,cranelift, so a bare check.py still
runs only the native backends there.

Assisted-by: Claude
…bitstrings_has_run

The runtime-minted-descr invalidation added in 6091829 fired on any
descr whose effect_idx is the u32::MAX sentinel. In the unit tests
compute_bitstrings is skipped, so every cached descr carries u32::MAX and
an unrelated CallMayForce write bit wrongly invalidated it, breaking
test_call_may_force_uses_effectinfo_to_keep_unaffected_cached_fields,
test_call_may_force_keeps_unaffected_variable_index_array_cache and
test_default_pipeline_escaping_call_arg_flush_is_selective.

Gate the clause on compute_bitstrings_has_run() in both the field and
array loops: after bitstrings run a u32::MAX effect_idx genuinely means an
out-of-universe runtime descr (invalidate conservatively); before that it
just means the fixture never stamped the slot, so the existing identity
fallback is the correct arbiter.

Assisted-by: Claude
check.py now adds wasm to its default backends when wasm32-unknown-unknown
is installed. The three pyre/check.py jobs share one steps anchor, so a
step guarded by runner.os == 'Linux' installs the target on the ubuntu leg
only — it builds and runs the wasm backend there, while macOS/Windows keep
no wasm32 target and stay on dynasm/cranelift. wasm output is
platform-independent, so exercising it on one OS is enough.

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

Here are some automated review suggestions for this pull request.

Reviewed commit: 20ff264ef5

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

// and no further chaining once the CA cell is live — the declined
// guard falls back to host round-trips, which handle it correctly.
let allow_ca = allow_ca && !source_has_bridges;
if !allow_ca && source_ca_active {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Decline CALL_ASSEMBLER when CA is disabled

When allow_ca was true for a self-recursive CallAssemblerR, wasm_unsupported_trace_reason(..., allow_ca) above already skipped the normal CALL_ASSEMBLER decline. If the later filters clear allow_ca because the source guard is nested or source_has_bridges is true, this branch only declines CA-active loops; otherwise the bridge continues into codegen with ca.emit_ca = false, where CallAssemblerR is lowered as a generic residual call using its first arg as a function pointer. In those recursive+nested or after-existing-bridge cases, decline here (or rerun the unsupported check) so the guard falls back to blackhole instead of compiling an invalid bridge.

Useful? React with 👍 / 👎.

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