perf: array read-modify-write in dense loops (#10718); native bitwise on unproven locals (#10511) - #11788
Merged
Conversation
added 3 commits
October 3, 2026 11:13
…e range tier (#10718) `a[i] = a[i] + 1; s += a[i]` fell off every loop tier. The classic range mode takes one statement, because a side exit after a store would replay the store, and the dense mode, which has no side exits, admitted only masked stores. The loop ran on the generic path at 206 instructions per element (node 39). The dense mode now admits counter-offset stores under the masked-store rule. The entry guard validates the whole counter window (in bounds, hole-free, raw-f64, plain, integrity-clean), and the stored value must be a statically genuine double, so the store has no value check and no side exit. The read and the write share that one guard, and the add is an unboxed fadd. The #10743 compound-assignment alias fold now runs over multi-statement bodies, and accumulator verification uses the lowering's own array set. arrRmw: 206 -> 14.6 instr/elem. Adds IR tests, including a sabotage-verified witness for the genuine-RHS rule, and test_gap_10718_array_rmw_dense.ts.
… loop clone (#10511) Unary `~` on an operand with no Number proof now takes the binary bitwise operators' inline guard (`|v| < 2^63`, with js_dynamic_bitnot on the cold arm) instead of calling the helper on every evaluation. New tier stmt/number_local_loop.rs, for the noble SHA-2 shape `let { A, B, C, D } = this; for (...) { ... ~B ... D = C; ... }`. In a receiver-free loop, a local is Number at every read when: - it holds a Number at entry (one inline test); - every write that can execute in the loop, in the body, the condition and the update clause, produces a Number given the admitted locals (a greatest fixed point); and - no closure, await, yield, boxed cell or module global can write it. The loop then runs in a clone whose 5L Number scope holds those locals, each in a plain F64 alloca that is written back on exit. When the entry test fails, the ordinary loop runs. destructure 212 -> 57 and annotated 213 -> 57 instr/iter (node 41). Adds IR tests; the clause-walk and fixed-point witnesses are sabotage-verified. Adds test_gap_10511_number_local_loop.ts.
proggeramlug
added
the
run-extended-tests
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (15)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds guarded unary bitwise-NOT lowering and a Number-local loop clone. It also extends dense range-loop matching to handle counter-offset stores, compound-assignment aliases, and multiple arrays. ChangesGuarded Bitwise Operations and Number-Local Loops
Dense Array Read-Modify-Write Loops
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change adds guarded bitwise lowering, a Number-local loop clone, and wider dense-array loop handling. No concrete defect was found, and the ordinary-loop and dynamic-helper fallbacks are retained. It is ready to merge, with the added IR and runtime tests as protection. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes retain runtime checks and fallback behavior around the expanded optimizations. No introduced security defect was established, but incomplete coverage prevents a minimal-risk assessment. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The PR also adds guarded unary bitwise lowering and a Number-versioned loop tier for unproven locals. Those changes address Full details: Docstring CoverageExplanation Docstring coverage is 57.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 76 functions across 13 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
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.
Closes #10718; partial progress on #10511. Two commits.
Instructions per op
a[i] = a[i] + 1; s += a[i])#10718 (array read-modify-write). A two-statement body fit no loop tier, so every element re-checked the array three times. The dense range tier now admits
a[i] = …anda[i ± c] = …when the stored value is provably a plain double. One entry guard covers bounds, holes and element kind for both the read and the write. The add is unboxed and never re-boxed, anda[i] += xtakes the same path.#10511 (bitwise on unproven locals).
~gets the same inline|v| < 2^63guard as the binary bitwise ops, withjs_dynamic_bitnotmoved to the cold arm.stmt/number_local_loop.rs. The loop is versioned on one Number test per local at entry. A fixed point covers every write in the loop, including the condition and update clauses, so it doesn't share the region bug fixed in fix(codegen): region Number proof judges loop condition and update writes #11782. Inside the clone those locals live in f64 slots.Tests
Verification
manifest_consistency(fixed on main by fix: main red — list net.Socket.writableCorked in the API manifest (#11757) #11785) and astatic_constfntest that fails only when run in parallel.bit|array|numeric|destruct|coerce: 151/151.Still open for #10511
i < a.lengthis still about 350 per element.Overlaps the #10741 lane in
stmt/loops.rs(dense mode).Summary by CodeRabbit
Performance Improvements
Bug Fixes