perf(codegen): masked-window dense loops admit provably-numeric stores - #9063
Merged
proggeramlug merged 2 commits intoAug 29, 2026
Merged
Conversation
proggeramlug
force-pushed
the
perf/masked-window-stores
branch
from
August 29, 2026 15:39
8c66da6 to
3d2c7ec
Compare
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe dense packed-f64 range loop now recognizes eligible masked-window stores. Proven genuine ChangesDense masked-window store lowering
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds guarded dense f64 stores while preserving fallback behavior for unsupported cases; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant DenseRangeLoopMatcher
participant IndexSetLowering
participant RawF64Slot
DenseRangeLoopMatcher->>IndexSetLowering: admit masked store with PlainF64 fact
IndexSetLowering->>IndexSetLowering: validate genuine f64 RHS
IndexSetLowering->>RawF64Slot: emit inline store double
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is detailed and relevant. It explains the motivation, implementation, tier restrictions, performance results, correctness coverage, and issue follow-up. It does not use the exact template headings or provide completed checklist results, but it contains the required information in substance. Full details: Docstring CoverageExplanation Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. (2 skipped: 1 unsupported, 1 too large.)
✨ Finishing Touches🧪 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 |
The dense packed-f64 range loop (PerryTS#6750) was read-only: its guard proves a whole static index window in bounds and hole-free on a plain raw-f64 array at loop entry, and the fast clone has no side exits — so loads are bare in-window reads, but any store in the body rejected the loop entirely, sending masked fill/copy loops (`a[i & K] = v`, the ring/LUT write shape) to per-store guarded lanes. Admit stores whose RHS provably materializes a GENUINE (unboxed) double by construction — number/integer literals, a local with a shared i32 shadow slot (a versioned-loop counter reads as `sitofp i32 -> double`), or an in-window masked load (the guard proved the slot holds a raw-f64 number). Dense mode has no side exits, so the proof must be static; no arithmetic is admitted in this first cut because an `Any`-typed operand can route `+` through a boxed-result helper. The store then lowers to a bare in-window `store double`: a number carries no heap edge, so no barrier and no layout note (GC_STORE_AUDIT(POINTER_FREE)), and the array pointer is re-derived from the receiver box per store exactly like the plain-tier loads, so a loop-poll GC move cannot strand it. Store-admitting loops use ONLY the f64 dense guard tier: a genuine-double store could break the i32 tier's all-slots-i32 loading proof mid-loop, and the TA tiers' hoisted data pointers serve reads only (`MaskedWindowArrayFact.allows_stores` is set solely in the f64 dense-with-stores scope, and `masked_window_store_fact_for_index` requires it plus the PlainF64 elem). Written arrays in dense mode take the READ-side eligibility (addressable, not scalar-replaced) instead of the full static fact set: the materialization-hazard bit trips on the very `new Array(n).fill(0)` construction calls that build these buffers, while the dense guard re-validates the ACTUAL runtime array — shape, raw-f64 packedness, integrity flags, the whole hole-free window — at every loop entry, and the matched body admits no call/closure/await. A wrong static hint costs one failed guard -> slow loop, never correctness. Isolated masked store loops (dev box; node 26.5 in parens): receiver = parameter 3.4 -> 0.4 ns (0.7), module global 3.1 -> 0.4 (0.7), function local 3.7 -> 0.5 (0.7) — every plain-array store shape in the matrix now ahead of node. Ten-probe node differential is byte-identical: wrap-around fill, holey array at entry (guard fails -> slow loop), window wider than the array (slow loop extends), frozen receiver, mixed-type window, read-write aliasing through the same window, two-array copy with scalar statements, NaN / -0 fidelity, reduce-observed results, and the module-global receiver. Two native-proof pins: the fast clone contains the f64 dense guard, no i32-tier guard, no per-store runtime call and no barrier, with a raw `store double`; and an RHS that is numeric only by TYPE (a number parameter — potentially INT32-boxed at runtime) never reaches the raw store.
proggeramlug
force-pushed
the
perf/masked-window-stores
branch
from
August 29, 2026 15:43
3d2c7ec to
2e6c0c3
Compare
Contributor
Author
|
Head is now Full host gate on this head: all real lint steps pass (formatting ✓, file-size ✓, GC store-site inventory ✓ — no new barrier stems, the masked store is POINTER_FREE), ratchets clean, suites green. One parallel-only runtime flake ( |
Contributor
Author
|
Merged. Admitting stores into a clone that has no side exits means the proof has to be fully static, so that's what I went at. The admission set is appropriately narrow — literals, a local with a shared i32 shadow slot, and an in-window masked load — and I think declining arithmetic in this cut is the right call rather than a limitation: an Probed the ways a store could smuggle a non-double into the dense tier, or the tier could be entered when it shouldn't: literal fill, counter RHS through the Two details I checked because they are the ones that bite quietly:
The tier discipline note in the description is worth keeping visible — restricting store-admitting loops to the f64 dense guard tier because a genuine-double store would break the i32 tier's all-slots-i32 proof mid-loop is the kind of interaction that only shows up as a wrong answer much later. Validation: |
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Aug 29, 2026
…umulator proofs, hazard relaxation Follow-up to PerryTS#9060/PerryTS#9063/PerryTS#9070: the packed-loop admission residuals behind the compare-only and reduce shapes. 1. If-conditions were invisible to the stable-packed matcher (PerryTS#9060's documented residual): stmt_flags / leading_read_requires_numeric matched only Let/Expr/Throw/Return, so `if (arr[i] < 0) count++` never admitted — and later reads inside If branches were invisible to the replay-safety check. Both now descend; `Compare` joins `Binary` as a numeric-consumption context (a wrong hint fails the require_numeric guard into the generic loop, never a wrong answer). 2. Numeric accumulators for the plain packed clones (versioned + range): `s += a[i]` inside a packed fast clone lowered its `+` through js_dynamic_string_or_number_add on EVERY iteration plus two root barriers — the by-construction collector runs before clone facts exist, so `s` had no proof. The packed fast preheaders now run PerryTS#9060's collect_numeric_accumulators with one Number tag test each (a non-Number accumulator takes the slow clone before anything ran), and the ids ride PackedF64LoopFact.numeric_accumulators, consulted by is_numeric_expr — the same mechanism and kill switch as the stable clone. 3. Guarded reads are numeric inside clones: has_numeric_index_fact and the boxed-fallback hazard predicate now recognize packed versioned/range facts and masked-window facts — the clone's read either produces a genuine raw double or side-exits BEFORE the value is consumed, so there is no boxed edge. This turns `if (a[i] < 0)` into a bare fcmp (was js_rel_lt per iteration) and feeds the accumulator walk. 4. Versioned-loop READ bodies take the store arm's relaxed eligibility: a call-free read body cannot invalidate what the entry guard re-proves, so the whole-function materialization hazard (tripped by the very `new Array(n).fill()` construction calls that build these buffers) no longer blocks versioning — locally-built arrays version at all. Same argument, word for word, as the existing store-arm comment; the two invalidation tests that pinned the read-side conservatism now pin the versioned-behind-guard contract their store twin already used. A stable-tier literal-bound arm was built and WITHDRAWN: plain-array literal bounds already version through the range loop, and the arm re-claimed five-field object-write bodies that nested_same_shape_object_writes deliberately keeps outside any clone. Isolated (dev box; node 26.5 in parens): count loop `if (a[i]<0) c++` 4.58 -> 1.60 ns/el (0.62) literal reduce `i<8192, s+=a[i]` 4.32 -> 4.16 (1.01) len-bound reduce (local array) 5.30 -> 4.14 (0.99) The count loop's residual is the per-iteration length IC, which PerryTS#9070's hoist removes at merge. The reduce rows are now call-free (census: fadd plus the loop poll only) and latency-bound on the accumulator's GC-root slot — true parity there needs unboxed accumulator slots in clones, scoped as the follow-on. Nine-probe differential vs node byte-identical, incl. a string accumulator (tag test -> slow clone -> concat), a literal bound past the array length (guard fail -> undefined += NaN), holey/mixed arrays, and an accumulator reassigned to a string mid-loop through a branch. perry-codegen suites 1823/0.
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Aug 29, 2026
…umulator proofs, hazard relaxation Follow-up to PerryTS#9060/PerryTS#9063/PerryTS#9070: the packed-loop admission residuals behind the compare-only and reduce shapes. 1. If-conditions were invisible to the stable-packed matcher (PerryTS#9060's documented residual): stmt_flags / leading_read_requires_numeric matched only Let/Expr/Throw/Return, so `if (arr[i] < 0) count++` never admitted — and later reads inside If branches were invisible to the replay-safety check. Both now descend; `Compare` joins `Binary` as a numeric-consumption context (a wrong hint fails the require_numeric guard into the generic loop, never a wrong answer). 2. Numeric accumulators for the plain packed clones (versioned + range): `s += a[i]` inside a packed fast clone lowered its `+` through js_dynamic_string_or_number_add on EVERY iteration plus two root barriers — the by-construction collector runs before clone facts exist, so `s` had no proof. The packed fast preheaders now run PerryTS#9060's collect_numeric_accumulators with one Number tag test each (a non-Number accumulator takes the slow clone before anything ran), and the ids ride PackedF64LoopFact.numeric_accumulators, consulted by is_numeric_expr — the same mechanism and kill switch as the stable clone. 3. Guarded reads are numeric inside clones: has_numeric_index_fact and the boxed-fallback hazard predicate now recognize packed versioned/range facts and masked-window facts — the clone's read either produces a genuine raw double or side-exits BEFORE the value is consumed, so there is no boxed edge. This turns `if (a[i] < 0)` into a bare fcmp (was js_rel_lt per iteration) and feeds the accumulator walk. 4. Versioned-loop READ bodies take the store arm's relaxed eligibility: a call-free read body cannot invalidate what the entry guard re-proves, so the whole-function materialization hazard (tripped by the very `new Array(n).fill()` construction calls that build these buffers) no longer blocks versioning — locally-built arrays version at all. Same argument, word for word, as the existing store-arm comment; the two invalidation tests that pinned the read-side conservatism now pin the versioned-behind-guard contract their store twin already used. A stable-tier literal-bound arm was built and WITHDRAWN: plain-array literal bounds already version through the range loop, and the arm re-claimed five-field object-write bodies that nested_same_shape_object_writes deliberately keeps outside any clone. Isolated (dev box; node 26.5 in parens): count loop `if (a[i]<0) c++` 4.58 -> 1.60 ns/el (0.62) literal reduce `i<8192, s+=a[i]` 4.32 -> 4.16 (1.01) len-bound reduce (local array) 5.30 -> 4.14 (0.99) The count loop's residual is the per-iteration length IC, which PerryTS#9070's hoist removes at merge. The reduce rows are now call-free (census: fadd plus the loop poll only) and latency-bound on the accumulator's GC-root slot — true parity there needs unboxed accumulator slots in clones, scoped as the follow-on. Nine-probe differential vs node byte-identical, incl. a string accumulator (tag test -> slow clone -> concat), a literal bound past the array length (guard fail -> undefined += NaN), holey/mixed arrays, and an accumulator reassigned to a string mid-loop through a branch. perry-codegen suites 1823/0.
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Aug 29, 2026
…rithmetic in masked-window stores Two follow-ups to the packed-loop store work (PerryTS#9041, PerryTS#9063): 1. The versioned packed loop's fast clone re-evaluated `i < arr.length` per iteration — ~20 inline instructions of handle decode + GC-header checks + the length load, which LLVM cannot hoist past the body's raw element stores. The entry guard just proved a live, non-forwarded plain array whose length the matched body cannot change (in-bounds stores only, no calls), so hoist the length ONCE in the fast preheader and hand it to `lower_for_after_init_with_i32_bound`, exactly like the range-versioned fast copy (PerryTS#6011). A mid-loop GC move changes the array's address, never its length, so the hoisted VALUE stays correct. Length-bound store loops: 1.3 -> 0.70 ns/store (node 0.59); constant-bound and hoisted-local-bound loops also gain (3.3 -> 2.2, 3.6 -> 2.8). 2. Masked-window dense stores (PerryTS#9063) admitted only literal / counter / in-window-load RHS values. Extend both predicates with float arithmetic (`+ - * /`, unary negation) over admitted operands: both sides being genuine doubles pins the numeric lowering to a bare float instruction — the boxed-`+`-helper hazard needs a non-numeric operand — and a float op over canonical operands cannot fabricate a NaN-box pattern (the default quiet NaN 0x7FF8 is a genuine double). `%` and `**` stay excluded (runtime-helper lowerings). `a[i & K] = b[i & K] + 1.5`: 15.9 -> 0.50 ns/store, ahead of node's 1.33. IR census of the fast clone: one raw load, one fadd, one raw store, the loop poll — no calls. Correctness: seven-probe arithmetic differential vs node byte-identical — NaN propagation, x/0 -> ±Infinity, 0/0 -> NaN, -0 quotients and products, overflow to Infinity, denormals, read-modify-write of the same slot, and a mixed-type source array whose guard failure routes the loop through the slow clone where JS `+` string concatenation applies. New pin: the arith fast clone is call-free with `fadd double` + `store double`, and never routes through `js_add`.
proggeramlug
added a commit
that referenced
this pull request
Aug 29, 2026
…rithmetic in masked-window stores (#9070) Two follow-ups to the packed-loop store work (#9041, #9063): 1. The versioned packed loop's fast clone re-evaluated `i < arr.length` per iteration — ~20 inline instructions of handle decode + GC-header checks + the length load, which LLVM cannot hoist past the body's raw element stores. The entry guard just proved a live, non-forwarded plain array whose length the matched body cannot change (in-bounds stores only, no calls), so hoist the length ONCE in the fast preheader and hand it to `lower_for_after_init_with_i32_bound`, exactly like the range-versioned fast copy (#6011). A mid-loop GC move changes the array's address, never its length, so the hoisted VALUE stays correct. Length-bound store loops: 1.3 -> 0.70 ns/store (node 0.59); constant-bound and hoisted-local-bound loops also gain (3.3 -> 2.2, 3.6 -> 2.8). 2. Masked-window dense stores (#9063) admitted only literal / counter / in-window-load RHS values. Extend both predicates with float arithmetic (`+ - * /`, unary negation) over admitted operands: both sides being genuine doubles pins the numeric lowering to a bare float instruction — the boxed-`+`-helper hazard needs a non-numeric operand — and a float op over canonical operands cannot fabricate a NaN-box pattern (the default quiet NaN 0x7FF8 is a genuine double). `%` and `**` stay excluded (runtime-helper lowerings). `a[i & K] = b[i & K] + 1.5`: 15.9 -> 0.50 ns/store, ahead of node's 1.33. IR census of the fast clone: one raw load, one fadd, one raw store, the loop poll — no calls. Correctness: seven-probe arithmetic differential vs node byte-identical — NaN propagation, x/0 -> ±Infinity, 0/0 -> NaN, -0 quotients and products, overflow to Infinity, denormals, read-modify-write of the same slot, and a mixed-type source array whose guard failure routes the loop through the slow clone where JS `+` string concatenation applies. New pin: the arith fast clone is call-free with `fadd double` + `store double`, and never routes through `js_add`. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 29, 2026
…umulator proofs, hazard relaxation (#9084) Follow-up to #9060/#9063/#9070: the packed-loop admission residuals behind the compare-only and reduce shapes. 1. If-conditions were invisible to the stable-packed matcher (#9060's documented residual): stmt_flags / leading_read_requires_numeric matched only Let/Expr/Throw/Return, so `if (arr[i] < 0) count++` never admitted — and later reads inside If branches were invisible to the replay-safety check. Both now descend; `Compare` joins `Binary` as a numeric-consumption context (a wrong hint fails the require_numeric guard into the generic loop, never a wrong answer). 2. Numeric accumulators for the plain packed clones (versioned + range): `s += a[i]` inside a packed fast clone lowered its `+` through js_dynamic_string_or_number_add on EVERY iteration plus two root barriers — the by-construction collector runs before clone facts exist, so `s` had no proof. The packed fast preheaders now run #9060's collect_numeric_accumulators with one Number tag test each (a non-Number accumulator takes the slow clone before anything ran), and the ids ride PackedF64LoopFact.numeric_accumulators, consulted by is_numeric_expr — the same mechanism and kill switch as the stable clone. 3. Guarded reads are numeric inside clones: has_numeric_index_fact and the boxed-fallback hazard predicate now recognize packed versioned/range facts and masked-window facts — the clone's read either produces a genuine raw double or side-exits BEFORE the value is consumed, so there is no boxed edge. This turns `if (a[i] < 0)` into a bare fcmp (was js_rel_lt per iteration) and feeds the accumulator walk. 4. Versioned-loop READ bodies take the store arm's relaxed eligibility: a call-free read body cannot invalidate what the entry guard re-proves, so the whole-function materialization hazard (tripped by the very `new Array(n).fill()` construction calls that build these buffers) no longer blocks versioning — locally-built arrays version at all. Same argument, word for word, as the existing store-arm comment; the two invalidation tests that pinned the read-side conservatism now pin the versioned-behind-guard contract their store twin already used. A stable-tier literal-bound arm was built and WITHDRAWN: plain-array literal bounds already version through the range loop, and the arm re-claimed five-field object-write bodies that nested_same_shape_object_writes deliberately keeps outside any clone. Isolated (dev box; node 26.5 in parens): count loop `if (a[i]<0) c++` 4.58 -> 1.60 ns/el (0.62) literal reduce `i<8192, s+=a[i]` 4.32 -> 4.16 (1.01) len-bound reduce (local array) 5.30 -> 4.14 (0.99) The count loop's residual is the per-iteration length IC, which #9070's hoist removes at merge. The reduce rows are now call-free (census: fadd plus the loop poll only) and latency-bound on the accumulator's GC-root slot — true parity there needs unboxed accumulator slots in clones, scoped as the follow-on. Nine-probe differential vs node byte-identical, incl. a string accumulator (tag test -> slow clone -> concat), a literal bound past the array length (guard fail -> undefined += NaN), holey/mixed arrays, and an accumulator reassigned to a string mid-loop through a branch. perry-codegen suites 1823/0. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
Follow-up to #9041 — the last plain-array store shape that trailed node.
What
The dense packed-f64 range loop (#6750) was read-only: its entry guard proves a whole static index window in bounds and hole-free on a plain raw-f64 array, and the fast clone has no side exits — so loads are bare in-window reads, but any store in the body rejected the loop entirely, sending masked fill/copy loops (
a[i & K] = v, the ring-buffer/LUT write shape) to per-store guarded lanes.This admits stores whose RHS provably materializes a genuine (unboxed) double by construction — number/integer literals, a local with a shared i32 shadow slot (a versioned-loop counter reads as
sitofp i32 → double), or an in-window masked load (the guard proved the slot holds a raw-f64 number). Dense mode has no side exits, so the proof must be static; no arithmetic is admitted in this cut (anAny-typed operand can route+through a boxed-result helper). The store lowers to a bare in-windowstore double: a number carries no heap edge → no barrier, no layout note (GC_STORE_AUDIT(POINTER_FREE)— no new barrier stem, nothing for the census), and the array pointer is re-derived from the receiver box per store exactly like the plain-tier loads, so a loop-poll GC move cannot strand it.Tier discipline
MaskedWindowArrayFact.allows_storesis set solely in the f64 dense-with-stores scope;masked_window_store_fact_for_indexrequires it plus the PlainF64 elem.new Array(n).fill(0)construction calls that build these buffers, while the dense guard re-validates the ACTUAL runtime array at every loop entry and the matched body admits no call/closure/await. A wrong static hint costs one failed guard → slow loop, never correctness. Classic (side-exiting, hole-tolerant) written arrays keep the full set.Numbers (isolated masked store loops, dev box; node 26.5 in parens)
With #9036 + #9041 + this, every plain-array store shape in the ops matrix is ahead of node.
Correctness
store double; an RHS numeric only by TYPE (a number parameter — potentially INT32-boxed at runtime) never reaches the raw store.Adjacency
stable_packed_loop.rs/loops.rs textual adjacency with perf/packed-loop-accumulator-v2 (numeric accumulators) — semantically independent per coordination with that branch's author; whichever lands second rebases trivially.Summary by CodeRabbit
Performance Improvements
Bug Fixes
Tests