perf(codegen): lower Math.imul and typed-array-param reads to native ops#6850
Merged
Conversation
added 2 commits
July 26, 2026 16:37
Two integer-math JS primitives were compiled as runtime calls and cascaded a whole hot integer loop into slow f64 ToInt32 towers. - Math.imul(a,b) -> native `mul i32` when both operands are provable i32 (exact: the low 32 bits of the product are identical for signed/unsigned operands). Non-finite / fractional / >2^32 operands keep the js_math_imul helper so JS ToUint32/ToInt32 semantics are preserved. Because `Math.imul` is defined as ToInt32(ToUint32*ToUint32 mod 2^32), an integer-literal operand is exact under low-32 truncation even above i32::MAX, so the imul operand check now accepts 32-bit-representable literals -- fixing the accumulator case `a = Math.imul(a, 0x9e3779b1)` (golden-ratio constant 2654435761 > i32::MAX). This relaxation is confined to Math.imul operands: a plain `x * BIGLIT | 0` evaluates its product in f64 (precision loss above 2^53), so it must stay ToInt32(f64_product), never an exact `mul i32`. - Typed-array element read through a parameter (S[i] where S: Int32Array etc. is a function parameter, in an i32/ToInt32 context) -> checked inline native load: a runtime guard (pointer + inline-storage PERRY_TA_VIEW_GUARD + kind-cache) and a header-length bounds check gate a bare width-correct load; an in-kind out-of-bounds read yields 0 (== ToInt32(undefined)); every rejected shape (view/detached/resizable backing, wrong runtime kind) defers to the new js_typed_array_read_int32 fallback. Extends the #6750 typed-array-local bare load to parameters, whose length/storage are unknown at compile time. Plain-value parameter reads still observe undefined out of bounds. On the 40M-iteration Int32Array-parameter bit-mixer both runtime-call families (js_math_imul x3, js_typed_array_get x3) reach zero and the read/multiply chain stays in native i32. Adds gap tests test_gap_math_imul_native.ts and test_gap_typedarray_param_read.ts (byte-identical to node --experimental-strip-types).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe compiler adds native i32 lowering for eligible ChangesNative i32 lowering
Checked typed-array parameter loads
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AOTLowering
participant TypedArrayHeader
participant LLVMIR
participant RuntimeFallback
AOTLowering->>TypedArrayHeader: Check view guard, kind cache, and length
AOTLowering->>LLVMIR: Emit typed load or zero for out-of-bounds
AOTLowering->>RuntimeFallback: Call js_typed_array_read_int32 on guard miss
RuntimeFallback->>TypedArrayHeader: Read and apply ToInt32 semantics
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@crates/perry-codegen/src/expr/i32_fast_path.rs`:
- Around line 771-775: Update the Expr::IndexGet eligibility logic so
checked_typed_array_i32_kind(ctx, object) is used only when index has an
integral or int-range proof. Preserve the existing
ta_int_elem_load_is_i32_provable and masked_window_i32_load_is_provable paths,
while preventing fractional indices such as 3.9 from reaching
lower_checked_typed_array_i32_load.
In `@crates/perry-runtime/src/typedarray/access.rs`:
- Around line 60-70: The js_typed_array_read_int32 path can dereference an
invalid TypedArrayHeader before validating the pointer. Update
js_typed_array_get, or the entry path it uses, to validate the raw pointer with
lookup_typed_array_kind or strict_typed_array_from_raw before any
TypedArrayHeader field access, while preserving the existing fallback behavior
for invalid or out-of-bounds reads.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0e682cd-1fc8-4386-9be1-a1952668ad83
📒 Files selected for processing (8)
changelog.d/6850-native-imul-typed-array-param.mdcrates/perry-codegen/src/expr/i32_fast_path.rscrates/perry-codegen/src/expr/math_simple.rscrates/perry-codegen/src/expr/mod.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-runtime/src/typedarray/access.rstest-files/test_gap_math_imul_native.tstest-files/test_gap_typedarray_param_read.ts
…rray-param read Two Major findings on #6850: 1. Correctness — the checked typed-array-param i32 fast path gated on the receiver kind but NOT the index: `S[3.9]` in an i32/ToInt32 context lowered the index through `fptosi` (=3) and read element 3, but JS reads a fractional typed-array index as `undefined` (-> 0 in that consumer). Gate the arm on `numeric_index_has_integer_array_index_proof` (made pub(crate)) — the same integer-index proof the sibling typed-array read paths in index_get.rs use. Integer literals, `i & mask`, and i32 loop counters keep the native path. 2. Memory safety — `js_typed_array_read_int32` (cold fallback) called `js_typed_array_get`, which reads `(*ta).length` (a TypedArrayHeader field) before classifying the pointer. That fallback is reached on a wrong-kind / kind-cache miss, which includes a receiver that is NOT a typed array (a `f(S: Int32Array)` called with an arbitrary value — TS types are erased), so a non-typed-array pointer type-confused the first GC-header read. Validate the raw pointer with `lookup_typed_array_kind` (the same gate `strict_typed_array_from_raw` uses; it covers native/inline views) before any header access; a non-array receiver returns 0 (= ToInt32(undefined)). Gap test extended with the fractional-index-in-i32-context regression cases, byte-identical to Node.
Contributor
Author
|
Both addressed in a292848: 1. Fractional index on the checked TA-param path (i32_fast_path.rs). Confirmed real — 2. Unvalidated |
proggeramlug
added a commit
that referenced
this pull request
Jul 26, 2026
…6855) * perf(codegen): inline small hot (in-loop) functions via inlinehint After native Math.imul + typed-array-param reads (#6850), the only residual gap of a tight integer-math kernel vs V8 is function-call overhead: V8 inlines a small hot function into its loop; Perry left it out-of-line. A NaN-boxed bit-mixer (`mix`, ~10 statements) costs ~800 in LLVM's inline model once GC shadow-frame calls + typed-array reads + double<->i32 marshaling are counted — well above the base -O3 threshold — so -O3 kept it as a call. Bias, don't force. Perry keeps `alwaysinline` only for the genuinely tiny (<= 8 statements). This adds a distinct `inlinehint` path for functions that are: - small (9..=SIZE_CAP statements, default cap 20), - called from inside a loop (a whole-module HIR pre-pass collects callee ids with an in-loop call site — approximates "hot" for an AOT compiler), AND - called from few total sites (default <= 4). The linker raises `-inlinehint-threshold` (default 850) so the hinted kernels actually inline; this lifts LLVM's ceiling ONLY for functions Perry stamped `inlinehint` — every other function keeps the base -O3 threshold, so cold code is untouched. Anti-bloat is the whole point, and it rests on two backstops: 1. The loop gate: a function only ever called from cold/straight-line code is never hinted, so cold utility functions can't bloat the binary. 2. The call-site cap: `-inlinehint-threshold` raises the ceiling at EVERY call site of a hinted callee (LLVM can't tell hot from cold per-site through a function attribute), so a hot kernel also called from many cold sites would be duplicated at all of them. Capping total call sites bounds the duplication. Measured binary-size delta (flag OFF vs ON), byte-precise __text section: - real programs (5 test-files): 0 hints, 0.000% delta; - broadly-called helper (300 fns x 40 cold sites): excluded by cap, 0%; - large programs (>6MB IR compile at -Os): flag inert, 0%; - realistic hot-kernel density: +0.34% (25 kernels), +0.79% (50), +1.57% (100); an all-kernels synthetic (250) reaches +3.86% — bounded by the cap (without it the same program's optimized IR grew 5.7x). `mix` 40M-call microbench (min-of-many, contended box load ~27): OFF 1024ms -> ON 904ms. Structural proof (load-independent): the caller loop no longer emits a `bl` to `mix` (0 vs 2 BR26 relocations), and `mix` carries `inlinehint`. Gated behind PERRY_INLINE_HOT_SMALL (default on); PERRY_INLINE_HOT_SMALL_CAP / _THRESHOLD / _MAX_SITES tune the window, hint threshold, and call-site cap. All four are folded into the object cache key. Existing `noinline` cases (try/setjmp/volatile) are respected via to_ir's has_try-first attribute precedence. Adds test_gap_inline_hot_small.ts (byte-identical to Node ON/OFF and under PERRY_GC_FORCE_EVACUATE=1). Stacks on #6850 (merged). Claude-Session: https://claude.ai/code/session_01UGDwjukzhowLDJYsMPFwMv * docs(changelog): add 6855 inline-hot-small fragment Claude-Session: https://claude.ai/code/session_01UGDwjukzhowLDJYsMPFwMv * perf(codegen): scan all class-member bodies in hot-callee call-site census CodeRabbit (#6855): collect_hot_loop_callees only walked hir.init, free functions, class constructors, and instance methods. It missed static methods, getters/setters, computed-key members (body + key expr), and instance/static field initializers. The miss matters for the anti-bloat cap, not just hot-callee discovery: `max_call_sites` bounds inlinehint duplication, so any call site the census doesn't see is a call site the cap can't count. A small function whose extra call sites hide in a getter or a field initializer could slip under the cap and get hinted despite being widely used, defeating the size backstop. Field/computed-key exprs are walked with in_loop=false so they feed the count without spuriously marking a callee hot. Verified byte-exact vs Node 26.5.0 on a class kernel whose getter, static method, and field initializer each carry an in-loop call (flag on and off). --------- 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.
Summary
Two JS integer-math primitives that should be single machine instructions were
compiled as runtime function calls, and they cascaded through the i32
type-flow to poison a whole hot loop into slow f64
ToInt32arithmetic. Thislands both native lowerings (they reinforce each other — fixing one alone leaves
the cascade intact):
A.
Math.imul(a, b)-> nativemul i32when both operands are provablyin-range i32. Multiplication mod 2^32 has identical low 32 bits for signed and
unsigned operands, so this is exact. Non-finite / fractional /
>2^32operandskeep the
js_math_imulruntime helper (a barefptosiwould break JSToUint32/ToInt32:NaN->0,Inf->0, truncation).The accumulator case
a = Math.imul(a, 0x9e3779b1)did not lower before:the mixer constant
0x9e3779b1= 2654435761 exceedsi32::MAX, so the i32fast-path leaf check rejected it. Because
Math.imulis defined asToInt32(ToUint32·ToUint32 mod 2^32), an integer-literal operand is exact underlow-32 truncation even above
i32::MAX, so the imul operand check nowaccepts 32-bit-representable literals. The relaxation is deliberately confined
to
Math.imuloperands: a plainx * BIGLIT | 0evaluates its product in f64(precision loss above 2^53) and must stay
ToInt32(f64_product), never an exactmul i32.B. Reading a typed-array element through a parameter (
S[i]whereS: Int32Arrayetc. is a function parameter, in an i32/ToInt32context) ->checked inline native load. Perry already emitted bare loads for typed-array
locals with proven bounds (#6750); a parameter's length and inline-vs-view
storage are unknown at compile time, so it fell back to
js_typed_array_get.The new lowering guards a bare width-correct load at runtime (pointer +
inline-storage
PERRY_TA_VIEW_GUARD+ kind-cache + header-length bounds check);an in-kind out-of-bounds read yields
0(== ToInt32(undefined), the onlyobservable value in that context); and every rejected shape
(view/detached/resizable backing, wrong runtime kind) defers to a new
js_typed_array_read_int32runtime fallback. Plain-value parameter reads stillobserve
undefinedout of bounds.Measurements (
/tmp/mix/mix.ts, 40M-iteration Int32Array-parameter bit-mixer)Structural (load-independent), in
perry_fn_mix:js_math_imulcallsjs_typed_array_getcallsjs_dyn_index_getcallssitofp/fptosi/selectmul i32Wall clock (min-of-many; machine was heavily contended, 1-min load 7–18, so
treat as directional):
mix40MThe residual ~2.1x is not the new guard: a typed-array-local variant
(unchecked native load, no per-read guard) measures ~967 ms — within noise of
the parameter/checked version — so the checked lowering matches the "ideal"
local path; the remaining gap is Perry's general per-call/slot overhead for this
micro-kernel on a saturated box, unrelated to either fix.
Correctness
New byte-for-byte gap tests (vs
node --experimental-strip-types):test_gap_math_imul_native.ts—Math.imuledge cases (NaN/±Infinity->0,fractional,
0x7fffffff/0xffffffffboundary,2**32+3), i32-accumulatorchains, nested imul, plus scoping guards asserting
x * BIGLIT | 0andfriends stay f64-exact.
test_gap_typedarray_param_read.ts— every typed-array kind read as aparameter in a loop; OOB (i32-context -> 0, plain-value ->
undefined),negative & fractional index,
Float64Array(width != 4), anArrayBuffer-backed view (slow-path storage), and a detached buffer.Also verified against a targeted set of existing gap tests covering the affected
paths (
toint32_large_finite,i32_arith_chain_copy,ushr_uint32_signed_slot,typed_arrays,arraybuffer_transfer,number_math, buffers, float16, …) —all byte-identical to Node.
Summary by CodeRabbit
Math.imulfast path for provably in-range 32-bit integers (including representable large 32-bit literals), with automatic fallback to preserve JS semantics.imulinputs.0for out-of-bounds/detached/invalid cases; plain reads continue to returnundefined.imulboundaries/accumulator chains, parameter typed-array reads (including fractional index regression), and detached-buffer behavior.