From 7de78d0576da130f5a40d6c58fe598fea27c7f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 08:15:04 +0200 Subject: [PATCH 1/8] fix(runtime,codegen): recover Uint8Array elements on kind-registry miss; inline the untracked u8 read (#9342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A perry `Uint8Array` is a `BufferHeader` in the buffer registries and can never appear in `lookup_typed_array_kind`'s registry. Two consequences, both fixed here. **Wrong answers.** `js_typed_array_read_f64` and `js_typed_array_read_int32` treated a kind-registry miss as "no element" and returned `undefined` / `0`. Both checked-load lanes admit the class name `"Uint8Array"` (kind 1), so a module-global u8 receiver read `undefined` (or, in `|0` context, a plausible `0`) for EVERY in-range element. The miss arms now recover the element the way every older consumer does: registered-buffer receivers read the byte via `js_buffer_index_get_value`, everything else falls through to `js_typed_array_get`, whose #8109 classifier runs before any header deref — which also retires the stale "deref before classify" hazard note on the i32 helper. **12x in-function read cliff.** `s += buf[i]` over a module-global u8 buffer compiled to a per-element runtime call: the tracked-view fast path only serves `let` bindings the same function constructed. New buffer-lane inline read (`expr/u8_buffer_read.rs`): NaN-box pointer tag + full-address hit in `PERRY_U8_INLINE_CACHE`, bounds against the header length, inline byte load at `header + 8`. Reads only — an inline write twin would bypass `buffer/view.rs` write propagation and desynchronize slice / ArrayBuffer aliases (#1205), which is also why view copies are admissible here. The admission cache holds only live, u8-marked, inline-storage headers: primed by the slow arm (`js_u8_buffer_read_f64`), invalidated inside the single buffer-death chokepoint (`finalize_collected_dead_buffer`) and at address re-issue (`register_buffer`), so ABA rides the same #6080 discipline as every other buffer identity table. Foreign-backed wrappers are refused at prime time. Kill switch: `PERRY_U8_INLINE_READ=0`. Lane ordering matters: the u8 lane runs BEFORE the typed-array checked lane, whose `PERRY_TA_KIND_CACHE` guard can never admit a `BufferHeader` and would otherwise pin every u8 read to its slow helper. That is a pure performance trap — post-fix it produces no wrong answer — so it is pinned structurally by an IR test rather than left to reasoning. Measured (SIZE=1e6 x 50): in-function module-global 560 -> 216 ms; top-level unchanged at parity (49 vs node 44). The residual 216 vs node's 38 is NOT the emitted guard — forcing the guard to always hit measures 218 ms, i.e. free. It is the accumulator's rooting diamond: `lower_guarded_numeric_add` roots every leaf `expr_produces_canonical_raw_f64` won't vouch for, and it cannot vouch for a `Uint8ArrayGet` leaf because the node's value is byte-or- `undefined` (#6884). That is #6904/#9303 territory and is filed separately, along with the unchanged typed-parameter receiver. Tests: `gc/tests/u8_inline_cache.rs` proves the cache lifecycle (prime contract, foreign rejection, death pruning, re-issue pruning) and each invalidation site was sabotage-verified — deleting either call fails exactly its own test. `perry/tests/issue_9342_u8_inline_read.rs` pins lane admission, node-exact values incl. an OOB arm, correctness under `PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1`, the kill switch, and the lane ordering (verified red under a deliberate reorder). Assertions count CALL sites, not the `declare` line every module emits — matching the bare symbol name made the absence assertion unpassable and the presence assertion vacuous, both of which were live in the first draft and caught by running it. Follow-up audit of the remaining 197 `lookup_typed_array_kind` miss-consumers filed as #9347. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/expr/arrays_finds.rs | 8 + crates/perry-codegen/src/expr/index_get.rs | 10 + crates/perry-codegen/src/expr/mod.rs | 1 + .../perry-codegen/src/expr/u8_buffer_read.rs | 223 ++++++++++++++++++ .../src/runtime_decls/objects.rs | 2 + .../src/runtime_decls/strings_part2.rs | 3 + crates/perry-runtime/src/buffer/header.rs | 67 ++++++ crates/perry-runtime/src/buffer/mod.rs | 4 + crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../src/gc/tests/u8_inline_cache.rs | 126 ++++++++++ crates/perry-runtime/src/typedarray/access.rs | 71 +++++- .../perry/tests/issue_9342_u8_inline_read.rs | 209 ++++++++++++++++ 12 files changed, 720 insertions(+), 5 deletions(-) create mode 100644 crates/perry-codegen/src/expr/u8_buffer_read.rs create mode 100644 crates/perry-runtime/src/gc/tests/u8_inline_cache.rs create mode 100644 crates/perry/tests/issue_9342_u8_inline_read.rs diff --git a/crates/perry-codegen/src/expr/arrays_finds.rs b/crates/perry-codegen/src/expr/arrays_finds.rs index 47fc0eb4e5..5cf143711e 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -946,6 +946,14 @@ pub(crate) fn lower( let reason = buffer_access_materialization_reason(ctx, array); return Ok(materialize_js_value(ctx, value, reason)); } + // #9342: untracked-but-class-proven receiver (module-global / + // param `Uint8Array`) — guarded inline byte read via the + // buffer-lane admission cache; guard misses defer to the priming + // memory-safe helper. + if let Some(value) = super::u8_buffer_read::try_lower_u8_buffer_read(ctx, array, index)? + { + return Ok(value); + } if !numeric_index_has_integer_array_index_proof(ctx, index) { return rooting::with_operands_rooted(ctx, &[array, index], |ctx, vals| { let a = vals[0].clone(); diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 21a4b00ba1..82b6b3f8e4 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -1191,6 +1191,16 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // element in-bounds, `TAG_UNDEFINED` OOB), replacing the per-read // runtime call. Gated on a proven integer index; guard misses // (view/detached/wrong-kind) defer to the memory-safe helper. + // #9342: buffer-lane twin for an untracked "Uint8Array"-proven + // receiver — MUST run before the typed-array checked load: a + // perry `Uint8Array` is a `BufferHeader` the TA kind cache can + // never admit, so the TA lane's guard would miss forever and + // pin every read to its slow helper. + if let Some(value) = + super::u8_buffer_read::try_lower_u8_buffer_read(ctx, object, index)? + { + return Ok(value); + } if let Some(value) = super::ta_param_f64_read::try_lower_ta_param_f64_read(ctx, object, index)? { diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index e4f65bee03..895ce8b5d1 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2741,6 +2741,7 @@ pub(crate) mod string_window; mod ptr_numarray_access; mod ta_param_f64_read; +mod u8_buffer_read; #[cfg(test)] mod unary_bigint_tests; #[cfg(test)] diff --git a/crates/perry-codegen/src/expr/u8_buffer_read.rs b/crates/perry-codegen/src/expr/u8_buffer_read.rs new file mode 100644 index 0000000000..4a4bfd89e9 --- /dev/null +++ b/crates/perry-codegen/src/expr/u8_buffer_read.rs @@ -0,0 +1,223 @@ +//! Inline checked byte read for an **untracked** `Uint8Array` receiver (#9342). +//! +//! Motivating shape — `s += buf[i]` inside a function over a module-global +//! `const buf = new Uint8Array(N)` (the bench_buffer_readwrite in-function +//! cliff: 560ms vs node's 38ms, 12×). The tracked fresh-view path +//! (`buffer_access.rs::lower_buffer_load`) only serves `let` bindings whose +//! construction the same function saw; a module-global (or any +//! class-proven-but-untracked) receiver fell back to a per-element +//! `js_uint8array_index_get_value` call feeding a dynamic add. +//! +//! The typed-array sibling (`ta_param_f64_read.rs`) cannot serve this shape: +//! perry's `Uint8Array` is a `BufferHeader` in the **buffer** registries — +//! bytes inline at `header + 8`, `length: u32` at offset 0 — invisible to +//! `lookup_typed_array_kind` and laid out differently from a +//! `TypedArrayHeader` (data at +16). Hence a buffer-lane twin: +//! +//! * **guard**: NaN-box pointer tag + full-address hit in +//! `PERRY_U8_INLINE_CACHE` (`perry-runtime/src/buffer/header.rs`), whose +//! entries name live, u8-marked, inline-storage `BufferHeader`s only. The +//! cache is primed by the slow arm and invalidated on buffer death and +//! address reuse, so a hit is proof of the layout contract; +//! * **bounds**: `idx ult length` (`ult` also rejects negative indices); +//! out-of-bounds merges the `TAG_UNDEFINED` double, matching +//! `js_buffer_index_get_value`; +//! * **load**: `zext(load i8 (addr + 8 + idx))` widened via `uitofp` — the +//! numeric element, bit-exact with the runtime helper's in-range answer; +//! * **slow arm**: `js_u8_buffer_read_f64`, which primes the cache and +//! delegates to `js_uint8array_index_get_value` — bug-exact semantics for +//! every receiver the guard rejects, including #8111 stale-hint recovery. +//! +//! READS ONLY. An inline **write** twin would bypass the `buffer/view.rs` +//! write-propagation protocol and desynchronize slice/`new Uint8Array(ab)` +//! aliases (#1205); view copies are admissible here precisely because writes +//! all still propagate. + +use anyhow::Result; +use perry_hir::Expr; + +use super::index_get::numeric_index_has_integer_array_index_proof; +use super::{lower_expr, lower_expr_as_i32, FnCtx}; +use crate::nanbox::{double_literal, i64_literal, TAG_UNDEFINED}; +use crate::native_value::{BoundsState, BufferAccessMode, LoweredValue}; +use crate::types::{DOUBLE, I1, I32, I64, I8}; + +/// `PERRY_U8_INLINE_READ=0` kill switch (default on). +fn u8_inline_read_enabled() -> bool { + match std::env::var("PERRY_U8_INLINE_READ") { + Ok(v) => !matches!(v.as_str(), "0" | "off" | "false" | "OFF" | "FALSE"), + Err(_) => true, + } +} + +/// Static receiver eligibility: a plain local/module-global read whose class +/// proves `Uint8Array`, not owned by the (stronger) tracked-view path. The +/// runtime guard is the safety net — a stale proof merely misses the cache — +/// but reassigned bindings are excluded anyway, mirroring +/// `ta_param_f64_read::checked_typed_array_f64_kind`'s reasoning. +fn u8_buffer_receiver_eligible(ctx: &FnCtx<'_>, object: &Expr) -> bool { + let Expr::LocalGet(id) = object else { + return false; + }; + if ctx.buffer_view_slots.contains_key(id) { + return false; + } + let class = crate::type_analysis::receiver_class_name(ctx, object).or_else(|| { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.module_global_proven_types.get(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } + }); + class.as_deref() == Some("Uint8Array") +} + +/// If `object[index]` is a proven-integer-index read of an untracked +/// `Uint8Array` receiver, emit the guarded inline byte load and return its +/// DOUBLE SSA value; otherwise `Ok(None)` so the caller keeps its existing +/// fallback. Records CheckedNative access-mode evidence, mirroring the +/// typed-array sibling. +pub(crate) fn try_lower_u8_buffer_read( + ctx: &mut FnCtx<'_>, + object: &Expr, + index: &Expr, +) -> Result> { + if ctx.disable_buffer_fast_path || !u8_inline_read_enabled() { + return Ok(None); + } + // Fractional / unproven indices stay on the runtime helper: the inline + // path lowers `index` via ToInt32, but JS reads `buf[3.9]` as `undefined`. + if !numeric_index_has_integer_array_index_proof(ctx, index) { + return Ok(None); + } + if !u8_buffer_receiver_eligible(ctx, object) { + return Ok(None); + } + let value = lower_u8_buffer_checked_load(ctx, object, index)?; + let lowered = LoweredValue::js_value(value.clone()); + ctx.record_lowered_value_with_access_mode( + "Uint8ArrayGet", + None, + "Uint8ArrayGet.checked_u8_inline", + &lowered, + Some(BoundsState::Unknown), + None, + Some(BufferAccessMode::CheckedNative), + Some(super::buffer_views::buffer_access_materialization_reason( + ctx, object, + )), + false, + false, + vec!["u8_buffer_read=checked_inline".to_string()], + ); + Ok(Some(value)) +} + +fn lower_u8_buffer_checked_load( + ctx: &mut FnCtx<'_>, + object: &Expr, + index: &Expr, +) -> Result { + let obj_box = lower_expr(ctx, object)?; + let idx_i32 = lower_expr_as_i32(ctx, index)?; + + let chk_idx = ctx.new_block("u8b.get.chk"); + let load_idx = ctx.new_block("u8b.get.load"); + let oob_idx = ctx.new_block("u8b.get.oob"); + let slow_idx = ctx.new_block("u8b.get.slow"); + let merge_idx = ctx.new_block("u8b.get.merge"); + let chk_label = ctx.block_label(chk_idx); + let load_label = ctx.block_label(load_idx); + let oob_label = ctx.block_label(oob_idx); + let slow_label = ctx.block_label(slow_idx); + let merge_label = ctx.block_label(merge_idx); + + let tag_mask = i64_literal(crate::nanbox::TAG_MASK); + + // ---- entry guard: pointer tag + admission-cache full-address hit ---- + let raw = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(&obj_box); + let raw = blk.and(I64, &obj_bits, crate::nanbox::POINTER_MASK_I64); + let tagged = blk.and(I64, &obj_bits, &tag_mask); + let is_ptr = blk.icmp_eq(I64, &tagged, crate::nanbox::POINTER_TAG_I64); + // Slot formula duplicates `buffer/header.rs::u8_inline_cache_slot`. + let slot = blk.lshr(I64, &raw, "3"); + let slot = blk.and(I64, &slot, "63"); + let entry_ptr = blk.gep( + "[64 x i64]", + "@PERRY_U8_INLINE_CACHE", + &[(I64, "0"), (I64, &slot)], + ); + let entry_val = blk.load(I64, &entry_ptr); + // Full-address compare — an empty slot (0) can never match a real + // pointer, so no separate emptiness test. + let hit = blk.icmp_eq(I64, &entry_val, &raw); + let g = blk.and(I1, &is_ptr, &hit); + blk.cond_br(&g, &chk_label, &slow_label); + raw + }; + + // ---- chk: bounds against `BufferHeader.length` (u32 at offset 0) ---- + ctx.current_block = chk_idx; + { + let blk = ctx.block(); + let hdr_ptr = blk.inttoptr(I64, &raw); + let len = blk.load(I32, &hdr_ptr); + // `ult` also rejects a negative index (wraps huge unsigned) — JS + // `buf[-1]` is undefined; the oob arm merges `TAG_UNDEFINED`. + let in_bounds = blk.icmp_ult(I32, &idx_i32, &len); + blk.cond_br(&in_bounds, &load_label, &oob_label); + } + + // ---- load: inline byte at `header + 8 + idx`, widened to f64 ---- + ctx.current_block = load_idx; + let (load_val, load_end) = { + let blk = ctx.block(); + let data_base = blk.add(I64, &raw, "8"); + let idx_i64 = blk.zext(I32, &idx_i32, I64); + let addr = blk.add(I64, &data_base, &idx_i64); + let ptr = blk.inttoptr(I64, &addr); + let byte = blk.load(I8, &ptr); + let val = blk.uitofp(I8, &byte, DOUBLE); + let end = blk.label.clone(); + blk.br(&merge_label); + (val, end) + }; + + // ---- oob: `undefined`, matching `js_buffer_index_get_value` ---- + ctx.current_block = oob_idx; + let (oob_val, oob_end) = { + let blk = ctx.block(); + let end = blk.label.clone(); + blk.br(&merge_label); + (double_literal(f64::from_bits(TAG_UNDEFINED)), end) + }; + + // ---- slow: cache miss / non-pointer → priming memory-safe helper ---- + ctx.current_block = slow_idx; + let (slow_val, slow_end) = { + let blk = ctx.block(); + let value = blk.call( + DOUBLE, + "js_u8_buffer_read_f64", + &[(I64, &raw), (I32, &idx_i32)], + ); + let end = blk.label.clone(); + blk.br(&merge_label); + (value, end) + }; + + // ---- merge ---- + ctx.current_block = merge_idx; + Ok(ctx.block().phi( + DOUBLE, + &[ + (load_val.as_str(), load_end.as_str()), + (oob_val.as_str(), oob_end.as_str()), + (slow_val.as_str(), slow_end.as_str()), + ], + )) +} diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index ec942b61a8..e18cbfcb0e 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -76,6 +76,8 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { // reads 0 whenever every live typed array uses inline storage (so the // inline `header + 16 + idx*elem_size` load matches the runtime `data_ptr`). module.add_external_global("PERRY_TA_KIND_CACHE", "[64 x i64]"); + // #9342: Uint8Array inline-read admission cache (buffer/header.rs). + module.add_external_global("PERRY_U8_INLINE_CACHE", "[64 x i64]"); module.add_external_global("PERRY_TA_VIEW_GUARD", I64); module.declare_function("js_object_alloc", I64, &[I32, I32]); // #3149: `Object(value)` plain-call coercion. Takes & returns a NaN-boxed diff --git a/crates/perry-codegen/src/runtime_decls/strings_part2.rs b/crates/perry-codegen/src/runtime_decls/strings_part2.rs index 4a78af9a7c..b7354dfc10 100644 --- a/crates/perry-codegen/src/runtime_decls/strings_part2.rs +++ b/crates/perry-codegen/src/runtime_decls/strings_part2.rs @@ -109,6 +109,9 @@ pub(crate) fn declare_phase_b_strings_part2(module: &mut LlModule) { // context): numeric element in-bounds, TAG_UNDEFINED double for OOB / view / // wrong-kind — bit-exact with js_typed_array_get. module.declare_function("js_typed_array_read_f64", DOUBLE, &[I64, I32]); + // #9342: priming slow arm of the inline Uint8Array byte read + // (expr/u8_buffer_read.rs); delegates to js_uint8array_index_get_value. + module.declare_function("js_u8_buffer_read_f64", DOUBLE, &[I64, I32]); // #2063: string / dynamic-key `ta[key]` [[Get]] dispatcher (canonical // numeric index → element, else ordinary named-property [[Get]]). module.declare_function("js_typed_array_index_get_dynamic", DOUBLE, &[I64, DOUBLE]); diff --git a/crates/perry-runtime/src/buffer/header.rs b/crates/perry-runtime/src/buffer/header.rs index c6079eddc5..81bcfa1429 100644 --- a/crates/perry-runtime/src/buffer/header.rs +++ b/crates/perry-runtime/src/buffer/header.rs @@ -396,6 +396,10 @@ pub fn register_buffer(ptr: *const BufferHeader) { // address, and without this the no-ops would carry over and the real packet // would serialize as all zeros (the MySQL server then times out reading it). super::own_props::clear_buffer_own_props(ptr as usize); + // #9342: same recycled-address rule for the inline-read admission cache — + // a fresh buffer must not inherit the dead tenant's inline-read admission + // (it may be foreign-backed, or not a Uint8Array at all). + u8_inline_cache_invalidate(ptr as usize); // Arm BEFORE the insert: an arm placed afterwards leaves a window in which // this buffer is in the registry while `is_registered_buffer` still takes // the idle fast path and denies it. See `crate::registry_latch`. @@ -702,6 +706,65 @@ pub fn asymmetric_key_meta(addr: usize) -> Option<(u8, u8)> { ASYMMETRIC_KEY_REGISTRY.with(|r| r.borrow().get(&addr).copied()) } + +/// #9342: direct-mapped inline-read admission cache for `Uint8Array`-backing +/// `BufferHeader`s, exported under a stable link name for the codegen's +/// guarded inline byte load (`perry-codegen/src/expr/u8_buffer_read.rs`). +/// +/// An entry holds the full address of a **live, `mark_as_uint8array`-marked +/// `BufferHeader` whose bytes are inline at `header + 8`** (no foreign +/// backing). Under that contract the emitted reader may do +/// `len = *(u32*)addr; addr + 8 + idx` directly: +/// +/// * view copies (`js_buffer_slice` / `new Uint8Array(arrayBuffer)`) ARE +/// admissible — their inline bytes are kept current by the write-propagation +/// protocol in `buffer/view.rs` (reads never go stale; only an inline WRITE +/// fast path would break aliasing, and this cache feeds no write path); +/// * foreign-backed wrappers (`buffer_alloc_foreign`, bun:ffi externals) are +/// excluded at prime time — their header is a lone `BufferHeader` with no +/// inline payload, so `header + 8` is past the allocation; +/// * ABA is closed the same way as every other buffer identity table: +/// `finalize_collected_dead_buffer` clears the entry when the buffer dies, +/// and `register_buffer` clears it again when the address is re-issued +/// (belt and suspenders, mirroring its own-props clear). +/// +/// Slot formula `(addr >> 3) & 63` is duplicated by codegen — keep in sync. +pub const U8_INLINE_CACHE_SLOTS: usize = 64; +#[no_mangle] +pub static PERRY_U8_INLINE_CACHE: [std::sync::atomic::AtomicU64; U8_INLINE_CACHE_SLOTS] = + [const { std::sync::atomic::AtomicU64::new(0) }; U8_INLINE_CACHE_SLOTS]; + +#[inline] +fn u8_inline_cache_slot(addr: usize) -> usize { + (addr >> 3) & (U8_INLINE_CACHE_SLOTS - 1) +} + +/// Test-only: does the admission cache currently hold exactly `addr`? +/// Reads the slot the way the emitted guard does — full-address compare. +#[cfg(test)] +pub(crate) fn test_u8_inline_cache_holds(addr: usize) -> bool { + PERRY_U8_INLINE_CACHE[u8_inline_cache_slot(addr)].load(std::sync::atomic::Ordering::Relaxed) + == addr as u64 +} + +#[inline] +pub(crate) fn u8_inline_cache_invalidate(addr: usize) { + let slot = u8_inline_cache_slot(addr); + if PERRY_U8_INLINE_CACHE[slot].load(std::sync::atomic::Ordering::Relaxed) == addr as u64 { + PERRY_U8_INLINE_CACHE[slot].store(0, std::sync::atomic::Ordering::Relaxed); + } +} + +/// Admit `addr` to the inline-read cache iff it satisfies the cache contract +/// above. Called from the codegen slow arm (`js_u8_buffer_read_f64`) so a +/// guard miss primes the next access; never called on a hot path. +pub(crate) fn u8_inline_cache_try_prime(addr: usize) { + if is_uint8array_buffer(addr) && foreign_backing(addr).is_none() { + PERRY_U8_INLINE_CACHE[u8_inline_cache_slot(addr)] + .store(addr as u64, std::sync::atomic::Ordering::Relaxed); + } +} + #[inline] pub fn is_uint8array_buffer(addr: usize) -> bool { // Reached from `typedarray_props::typed_array_owner_kind` for every untyped @@ -1073,6 +1136,10 @@ pub(crate) fn finalize_collected_dead_buffer(addr: usize) { super::own_props::clear_buffer_own_props(addr); super::detach::remove_detached_entry_for_dead_buffer(addr); super::view::remove_entries_for_dead_buffer(addr); + // #9342: drop the dead address from the inline-read admission cache before + // its block can be reset and re-issued — a stale hit would read the next + // tenant's memory as (length, bytes). + u8_inline_cache_invalidate(addr); } /// Get the data pointer for a buffer diff --git a/crates/perry-runtime/src/buffer/mod.rs b/crates/perry-runtime/src/buffer/mod.rs index 3dc3e0c76e..3af6e1098f 100644 --- a/crates/perry-runtime/src/buffer/mod.rs +++ b/crates/perry-runtime/src/buffer/mod.rs @@ -45,6 +45,10 @@ pub use header::{BufferHeader, BUFFER_TYPE_ID, SMALL_BUF_THRESHOLD}; // ---- Re-exports: allocation / registry helpers ---- pub(crate) use header::is_small_buf_slab_addr; +// #9342: primed by `typedarray::js_u8_buffer_read_f64` (codegen slow arm). +pub(crate) use header::u8_inline_cache_try_prime; +#[cfg(test)] +pub(crate) use header::test_u8_inline_cache_holds; // `shared_sab` publishes process-global backings that `is_registered_buffer` // reports as buffers without them entering `BUFFER_REGISTRY`, so it arms the // same monotone latch — before the backing becomes reachable. diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index e21d9d67fd..4ab3f775a1 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -6,6 +6,7 @@ mod block_pool_pressure; mod budgeted_step_api; mod buffer_bound_method_name; mod buffer_side_tables; +mod u8_inline_cache; mod contract; mod copying; mod copying_side_tables; diff --git a/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs b/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs new file mode 100644 index 0000000000..4fcebc4093 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs @@ -0,0 +1,126 @@ +//! Lifecycle proof for the #9342 `PERRY_U8_INLINE_CACHE` admission cache. +//! +//! The cache contract ("an entry names a live, u8-marked, inline-storage +//! `BufferHeader`") is held up by two invalidation sites — buffer death +//! (`finalize_collected_dead_buffer`) and address re-issue +//! (`register_buffer`) — riding the same chokepoints as every other buffer +//! identity table. A stale hit is SILENT (the emitted reader would interpret +//! the new tenant's memory as `(length, bytes)`), so each site is proved here +//! by a test that fails when that specific call is removed: delete the +//! finalize call and `test_dead_u8_entry_pruned_on_full_gc` fails; delete the +//! register call and `test_reissued_address_does_not_inherit_admission` +//! fails. + +use super::super::*; +use super::support::*; + +fn full_gc() { + let _ = + gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct)); +} + +/// Prime admits a `mark_as_uint8array`-marked inline-storage buffer, and the +/// entry means what the emitted guard thinks it means: header `length` at +/// offset 0, live bytes at `header + 8`. +#[test] +fn test_prime_admits_inline_u8_and_contract_holds() { + let _guard = GcTestIsolationGuard::new(); + + let buf = crate::buffer::buffer_alloc(16); + let addr = buf as usize; + unsafe { + (*buf).length = 16; + *crate::buffer::buffer_data_mut(buf).add(3) = 0xAB; + } + + // Unmarked: not a Uint8Array, must not be admitted. + crate::buffer::u8_inline_cache_try_prime(addr); + assert!( + !crate::buffer::test_u8_inline_cache_holds(addr), + "an unmarked buffer must not be admitted" + ); + + crate::buffer::mark_as_uint8array(addr); + crate::buffer::u8_inline_cache_try_prime(addr); + assert!( + crate::buffer::test_u8_inline_cache_holds(addr), + "a marked inline-storage buffer must be admitted" + ); + + // The emitted reader's view of an admitted entry: length then byte. + let len = unsafe { *(addr as *const u32) }; + let byte = unsafe { *((addr + 8 + 3) as *const u8) }; + assert_eq!(len, 16, "length must be readable at header offset 0"); + assert_eq!(byte, 0xAB, "bytes must be inline at header + 8"); +} + +/// A foreign-backed wrapper (header-only allocation, bytes owned elsewhere) +/// must never be admitted — `header + 8` is past its allocation. +#[test] +fn test_prime_rejects_foreign_backed_wrapper() { + let _guard = GcTestIsolationGuard::new(); + + let mut bytes = [7u8; 8]; + let buf = crate::buffer::buffer_alloc_foreign(bytes.as_mut_ptr(), bytes.len() as u32); + let addr = buf as usize; + crate::buffer::mark_as_uint8array(addr); + crate::buffer::u8_inline_cache_try_prime(addr); + assert!( + !crate::buffer::test_u8_inline_cache_holds(addr), + "a foreign-backed wrapper must not be admitted: its bytes are not \ + inline and the emitted load would read past the allocation" + ); + crate::buffer::finalize_collected_dead_buffer(addr); +} + +/// Death pruning: a dead buffer's admission must not survive the full trace +/// that collects it — the recycled address's next tenant is arbitrary memory +/// to the emitted reader. Fails if `finalize_collected_dead_buffer` loses its +/// `u8_inline_cache_invalidate` call. +#[test] +fn test_dead_u8_entry_pruned_on_full_gc() { + let _guard = GcTestIsolationGuard::new(); + + let addr = crate::buffer::buffer_alloc(16) as usize; + crate::buffer::mark_as_uint8array(addr); + crate::buffer::u8_inline_cache_try_prime(addr); + assert!( + crate::buffer::test_u8_inline_cache_holds(addr), + "test premise: the buffer is admitted while live" + ); + + // No roots: dead at the full trace (buffers are TENURED old-gen residents). + full_gc(); + + assert!( + !crate::buffer::test_u8_inline_cache_holds(addr), + "a dead buffer's inline-read admission must be pruned on the trace \ + that collects it — a stale hit reads the next tenant's memory as \ + (length, bytes)" + ); +} + +/// Re-issue pruning: registering a fresh buffer at an address must clear any +/// admission the previous tenant held (belt and suspenders over death +/// pruning, mirroring `register_buffer`'s own-props clear). Fails if +/// `register_buffer` loses its `u8_inline_cache_invalidate` call. +#[test] +fn test_reissued_address_does_not_inherit_admission() { + let _guard = GcTestIsolationGuard::new(); + + let buf = crate::buffer::buffer_alloc(16); + let addr = buf as usize; + crate::buffer::mark_as_uint8array(addr); + crate::buffer::u8_inline_cache_try_prime(addr); + assert!(crate::buffer::test_u8_inline_cache_holds(addr)); + + // Simulate the re-issue path directly: a new tenant registering at the + // same address (the death finalizer is deliberately NOT run first, so + // this passes only on register_buffer's own clear). + crate::buffer::register_buffer(buf); + assert!( + !crate::buffer::test_u8_inline_cache_holds(addr), + "a re-registered address must not inherit the dead tenant's \ + inline-read admission" + ); +} diff --git a/crates/perry-runtime/src/typedarray/access.rs b/crates/perry-runtime/src/typedarray/access.rs index 09c76af0c3..a60672a3d1 100644 --- a/crates/perry-runtime/src/typedarray/access.rs +++ b/crates/perry-runtime/src/typedarray/access.rs @@ -132,9 +132,31 @@ pub extern "C" fn js_typed_array_read_int32(ta: *const TypedArrayHeader, index: // non-typed-array receiver has no element to read, and // `ToInt32(undefined) == 0` in this i32 consumer context. let ta = clean_ta_ptr(ta); - if ta.is_null() || lookup_typed_array_kind(ta as usize).is_none() { + if ta.is_null() { return 0; } + if lookup_typed_array_kind(ta as usize).is_none() { + // #9342, i32 twin of the `js_typed_array_read_f64` fix: a registry + // miss must recover the element, not invent one. Perry's `Uint8Array` + // is a `BufferHeader` (buffer registries) that this registry can never + // contain, so a "Uint8Array"-proven receiver in `| 0` context read `0` + // for every element. Buffer receivers read the byte + // (`ToInt32(undefined) == 0` for OOB); everything else falls through + // to `js_typed_array_get`, which classifies the receiver BEFORE any + // header deref (`classify_element_read_receiver`, #8109) — the + // "would read `(*ta).length` before classifying" hazard in the doc + // above predates that classifier. + let addr = ta as usize; + if crate::buffer::is_registered_buffer(addr) { + let v = crate::buffer::js_buffer_index_get_value( + addr as *const crate::buffer::BufferHeader, + index, + ); + // In-range: an exact 0..=255 byte. OOB: TAG_UNDEFINED, and + // `ToInt32(undefined) == 0`. + return if v.is_finite() { v as i32 } else { 0 }; + } + } let v = js_typed_array_get(ta, index); // `js_typed_array_get` returns a plain finite f64 element for an in-bounds // read and TAG_UNDEFINED (a NaN) for OOB. `ToInt32` maps NaN / ±Inf -> 0. @@ -170,15 +192,33 @@ static KEEP_JS_TYPED_ARRAY_READ_INT32: extern "C" fn(*const TypedArrayHeader, i3 /// Memory safety mirrors [`js_typed_array_read_int32`]: a kind-cache miss can be /// entered with a receiver that is not a typed array at all (TS types are /// erased), so validate the raw pointer is a registered typed array before any -/// header deref — a non-typed-array receiver has no element and reads -/// `undefined` (`TAG_UNDEFINED`). Otherwise defer to the full ECMAScript -/// `[[Get]]`. +/// header deref. A registry miss recovers the element rather than inventing +/// `undefined` (#9342/#8111): buffer receivers read the byte, everything else +/// takes `js_typed_array_get`'s classifier dispatch. #[no_mangle] pub extern "C" fn js_typed_array_read_f64(ta: *const TypedArrayHeader, index: i32) -> f64 { let ta = clean_ta_ptr(ta); - if ta.is_null() || lookup_typed_array_kind(ta as usize).is_none() { + if ta.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } + if lookup_typed_array_kind(ta as usize).is_none() { + // #9342: a registry miss is NOT proof there is no element. Perry's + // `Uint8Array` is a `BufferHeader` (buffer registries), invisible to + // the typed-array kind registry — so this arm used to answer + // `undefined` for every in-range read through a "Uint8Array"-proven + // receiver. Same defect class as #8111: recover the element instead. + // Buffer receivers read the byte; anything else inherits + // `js_typed_array_get`'s #8109 receiver-classifier dispatch + // (header-wins for an unregistered real TA, ordinary `[[Get]]` for a + // plain array/object/string, `undefined` for a non-receiver). + let addr = ta as usize; + if crate::buffer::is_registered_buffer(addr) { + return crate::buffer::js_buffer_index_get_value( + addr as *const crate::buffer::BufferHeader, + index, + ); + } + } js_typed_array_get(ta, index) } @@ -188,6 +228,27 @@ pub extern "C" fn js_typed_array_read_f64(ta: *const TypedArrayHeader, index: i3 static KEEP_JS_TYPED_ARRAY_READ_F64: extern "C" fn(*const TypedArrayHeader, i32) -> f64 = js_typed_array_read_f64; +/// #9342 — slow arm of the codegen inline `Uint8Array` byte read +/// (`perry-codegen/src/expr/u8_buffer_read.rs`). Primes the +/// `PERRY_U8_INLINE_CACHE` admission cache when the receiver satisfies its +/// contract (live u8-marked inline-storage `BufferHeader`), then delegates to +/// [`js_uint8array_index_get_value`] for bug-exact element semantics — +/// including the #8111 stale-static-hint recovery for rebound receivers. +#[no_mangle] +pub extern "C" fn js_u8_buffer_read_f64(target: *const TypedArrayHeader, index: i32) -> f64 { + let addr = strip_nanbox(target as u64); + if addr >= 0x1000 { + crate::buffer::u8_inline_cache_try_prime(addr); + } + js_uint8array_index_get_value(addr as *const TypedArrayHeader, index) +} + +// Codegen-only export: pin under whole-program LTO (mirrors the siblings). +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_U8_BUFFER_READ_F64: extern "C" fn(*const TypedArrayHeader, i32) -> f64 = + js_u8_buffer_read_f64; + /// #2063 — dynamic / string-key `[[Get]]` on a TypedArray (`ta[key]`). /// /// The codegen element-read fast path only fires for statically-proven diff --git a/crates/perry/tests/issue_9342_u8_inline_read.rs b/crates/perry/tests/issue_9342_u8_inline_read.rs new file mode 100644 index 0000000000..4de79f9f7e --- /dev/null +++ b/crates/perry/tests/issue_9342_u8_inline_read.rs @@ -0,0 +1,209 @@ +//! #9342: in-function reads of a module-global / parameter `Uint8Array`. +//! +//! A perry `Uint8Array` is a `BufferHeader` in the buffer registries, invisible +//! to `lookup_typed_array_kind`. Before the fix, two stacked defects: +//! +//! 1. **Wrong answers** — the typed-array checked-load lane admits the class +//! name `"Uint8Array"` (kind 1), but its `PERRY_TA_KIND_CACHE` guard can +//! never match a `BufferHeader`, so every read routed to +//! `js_typed_array_read_f64` / `js_typed_array_read_int32`, whose +//! registry-miss arms answered `undefined` / `0` for every in-range +//! element. +//! 2. **12× slowness** — the `Uint8ArrayGet` fallback was a per-element +//! runtime call feeding a dynamic add (bench_buffer_readwrite's +//! in-function cliff: 560 ms vs node 38 ms). +//! +//! The fix gives untracked-but-proven u8 receivers their own buffer-lane +//! inline read (`expr/u8_buffer_read.rs` + `PERRY_U8_INLINE_CACHE`), ordered +//! BEFORE the typed-array checked lane. That ordering is a performance trap +//! with no wrong answer — if someone reorders the lanes back, the TA guard +//! captures u8 receivers into permanent slow-helper calls and nothing fails — +//! so `ta_lane_must_not_capture_u8_receivers` pins it structurally: a program +//! whose only array-ish receiver is a u8 buffer must emit NO +//! `js_typed_array_read_f64` reference at all. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// Module-global receiver read in a function, plus a typed-parameter receiver: +/// the two shapes the tracked-view fast path cannot serve. +/// +/// Expected values (node-checked): +/// * `sum(buf)` over `buf[i] = i % 256` for N=4096: 16 full 0..=255 blocks → +/// 16 * 32640 = 522240. +/// * OOB read `buf[4096]` is `undefined`; `undefined + 0` shows as NaN via +/// `Number()` coercion in the harness — asserted as the string "NaN". +const SOURCE: &str = r#" +const N = 4096; +const buf = new Uint8Array(N); +for (let i = 0; i < N; i++) buf[i] = i % 256; + +function viaGlobal(): number { + let s = 0; + for (let i = 0; i < N; i++) s += buf[i]; + return s; +} + +function viaParam(b: Uint8Array): number { + let s = 0; + for (let i = 0; i < N; i++) s += b[i]; + return s; +} + +function oobGlobal(): number { + // In-bounds proof intentionally absent for index N. + let x = 0; + x += buf[N]; + return x; +} + +console.log(viaGlobal() + "," + viaParam(buf) + "," + oobGlobal()); +"#; + +const EXPECTED: &str = "522240,522240,NaN\n"; + +fn compile(dir: &Path, source: &str, extra_env: &[(&str, &str)]) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let mut cmd = Command::new(perry_bin()); + cmd.current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1"); + for (k, v) in extra_env { + cmd.env(k, v); + } + let compile = cmd.output().expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +/// Count CALL sites of `name`, not the `declare` line every module emits for +/// every runtime symbol. Matching the bare name is worthless here: both +/// `js_u8_buffer_read_f64` and `js_typed_array_read_f64` are declared in every +/// compiled module whether or not anything calls them, so a bare-substring +/// absence assertion can never pass and a bare-substring presence assertion +/// passes vacuously. (Both mistakes were live in this file's first draft and +/// were caught by running it.) +fn call_count(ir: &str, name: &str) -> usize { + let needle = format!("@{name}("); + ir.lines() + .filter(|l| l.contains(&needle) && l.contains("call ")) + .count() +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1"); + } + command.output().expect("run compiled binary") +} + +fn assert_stdout(output: &Output, label: &str) { + assert!( + output.status.success(), + "{label}: binary failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + EXPECTED, + "{label}: wrong element values" + ); +} + +/// The inline lane fires (guard + slow helper present), values are node-exact, +/// and they stay node-exact under heap-limit + forced-evacuation GC stress +/// (the cache is address-keyed; a stale hit would read recycled memory). +#[test] +fn u8_inline_read_lane_fires_and_is_correct() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE, &[]); + let ir = kept_ir(&stderr); + assert!( + call_count(&ir, "js_u8_buffer_read_f64") > 0, + "the u8 inline lane's slow arm must be CALLED — the lane did not fire" + ); + assert!( + ir.contains("@PERRY_U8_INLINE_CACHE"), + "the emitted guard must probe the admission cache" + ); + assert_stdout(&run(&bin, dir.path(), false), "plain"); + assert_stdout(&run(&bin, dir.path(), true), "gc-stress"); +} + +/// Ordering pin: the typed-array checked lane must NOT capture a u8 receiver. +/// Its `PERRY_TA_KIND_CACHE` guard can never admit a `BufferHeader`, so +/// capture means every read is a permanent slow-helper call — a performance +/// regression nothing else would ever fail on. This program's only array-ish +/// receiver is a u8 buffer, so a `js_typed_array_read_f64` reference in the +/// IR can only mean the lane ordering regressed +/// (`expr/index_get.rs`: u8 lane before `try_lower_ta_param_f64_read`). +#[test] +fn ta_lane_must_not_capture_u8_receivers() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_bin, stderr) = compile(dir.path(), SOURCE, &[]); + let ir = kept_ir(&stderr); + assert_eq!( + call_count(&ir, "js_typed_array_read_f64"), + 0, + "a u8 receiver reached the typed-array checked lane — its guard can \ + never admit a BufferHeader, so every read becomes a slow-helper call" + ); + // Vacuity guard: the absence above is only meaningful while this fixture + // still admits the u8 lane. If the lane stops firing, the absence passes + // for the wrong reason. + assert!( + call_count(&ir, "js_u8_buffer_read_f64") > 0, + "fixture no longer admits the u8 lane — the absence assertion above \ + would pass vacuously" + ); +} + +/// Kill switch: `PERRY_U8_INLINE_READ=0` at build time removes the lane and +/// the program still answers node-exact values through the runtime helpers +/// (which, post-#9342, recover buffer elements on a registry miss instead of +/// inventing `undefined`/`0`). +#[test] +fn kill_switch_stays_correct() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE, &[("PERRY_U8_INLINE_READ", "0")]); + let ir = kept_ir(&stderr); + assert_eq!( + call_count(&ir, "js_u8_buffer_read_f64"), + 0, + "kill switch must remove the inline lane" + ); + assert_stdout(&run(&bin, dir.path(), false), "kill-switch"); +} From 4688aef000d1e726ddad36212e9c4056d17e38b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 08:59:56 +0200 Subject: [PATCH 2/8] perf(codegen): a module-global typed-array receiver earns the numeric proof (444 -> 94 ms) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectors/ptr_shape_numeric.rs` proved `view[i]` is Number-or-`undefined` from two sources: `numeric_ta_views` (spec-proven `TaPtr` parameters) and `const_local_inits` (a compiler-visible `const` init in the SCANNED body). A module-global `const buf = new Uint8Array(N)` read inside a function has neither, so `acc += buf[i]` lost the accumulator's Number-by-construction proof and every add lowered through the rooted `guarded_add` diamond: a GC shadow-frame load + store + `js_write_barrier_root_nanbox` per element, plus the dynamic-add cold arm. `module_global_proven_types` is the same STRENGTH of proof as `const_local_inits` — derived from the initializer expression on a single- `Let`, never-reassigned binding, not from an annotation (Perry does not enforce those, #7773) — so module-scope views whose construction proves a number-valued typed-array kind now feed the same fixpoint slot. The BigInt kinds are deliberately excluded: their elements are BigInts, not Numbers. Measured (SIZE=1e6 x ITER=100, quiet host, min-of-3): the identical loop over a module-global receiver 444 -> 94 ms, exactly matching the body-local receiver it should always have matched, against node's 79. The receiver's binding form is no longer observable in the emitted loop. ATTRIBUTION, corrected by measurement. The missing proof also leaves a per-iteration `load volatile @PERRY_GC_POLL_ARMED` in the loop, because `loop_may_allocate` stays conservative while the `+` is not inert, and the obvious story is that this volatile load blocks vectorization. It does not pay: admitting the read as inert under the same construction proof (so the poll leaves the loop) measured 94 ms either way, and did not vectorize either — the residual blocker is #9360's per-element admission-cache probe. That change is therefore NOT included: `expr_is_inert_primitive` also governs rooting decisions, and an unmeasured widening of it does not ship. The residual is documented in the test and in #9363. Tests: `issue_9363_module_global_view_numeric_proof.rs` pins the emitted shape against a body-local control (which is asserted clean first, so the comparison cannot pass vacuously) and pins that a REASSIGNED module global is still not admitted — the construction proof's exclusion is load-bearing. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/codegen/function.rs | 4 + .../perry-codegen/src/collectors/hir_facts.rs | 6 + .../src/collectors/number_by_construction.rs | 48 +++- ...e_9363_module_global_view_numeric_proof.rs | 208 ++++++++++++++++++ 4 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 964e7aabce..5b55b84001 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -931,6 +931,10 @@ pub(super) fn compile_function( &spec_i32_params, &spec_numeric_params, &spec_number_array_params, + // #9363: module-scope bindings whose CONSTRUCTION (not annotation) + // proves a numeric typed-array/Uint8Array kind, so `g[i]` off one is + // Number-or-`undefined` exactly as a body-local `const` view is. + &cross_module.module_global_proven_types, ); // A Number-by-construction local cannot ever hold a GC pointer, so it diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index a64a722739..97d800b423 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -464,6 +464,7 @@ pub(crate) fn collect_type_facts( spec_i32_params: &HashSet, spec_numeric_params: &HashSet, spec_number_array_params: &HashSet, + module_global_proven_types: &HashMap, ) -> TypeFacts { // #7700: which locals hold a NUMBER, so a `u8[k]` keyed on one is a byte // read rather than a property read. Computed once here because @@ -560,6 +561,7 @@ pub(crate) fn collect_type_facts( spec_ta_lens, spec_numeric_params, ¬_bigint_locals, + module_global_proven_types, ); let (mut array_facts, effect_facts, materialization_hazards) = collect_array_facts(stmts, params, module_globals, binding_types); @@ -792,6 +794,7 @@ pub(crate) fn collect_native_region_fact_graph( &HashSet::new(), &HashSet::new(), &HashSet::new(), + &HashMap::new(), ) } @@ -815,6 +818,7 @@ pub(crate) fn collect_native_region_fact_graph_with_spec_params( spec_i32_params: &HashSet, spec_numeric_params: &HashSet, spec_number_array_params: &HashSet, + module_global_proven_types: &HashMap, ) -> NativeRegionFactGraph { collect_type_facts( stmts, @@ -832,6 +836,7 @@ pub(crate) fn collect_native_region_fact_graph_with_spec_params( spec_i32_params, spec_numeric_params, spec_number_array_params, + module_global_proven_types, ) } @@ -861,6 +866,7 @@ pub(crate) fn collect_hir_facts( &HashSet::new(), &HashSet::new(), &HashSet::new(), + &HashMap::new(), ) } diff --git a/crates/perry-codegen/src/collectors/number_by_construction.rs b/crates/perry-codegen/src/collectors/number_by_construction.rs index 085602e8da..fd1a6de595 100644 --- a/crates/perry-codegen/src/collectors/number_by_construction.rs +++ b/crates/perry-codegen/src/collectors/number_by_construction.rs @@ -71,6 +71,29 @@ pub(crate) fn enabled() -> bool { ) } +/// Typed-array/`Uint8Array` class names whose elements are Numbers. +/// +/// The BigInt kinds are deliberately absent: `BigInt64Array`/`BigUint64Array` +/// elements are BigInts, not Numbers, and `+` on one throws when mixed. Kept +/// as a name list rather than reusing a kind table because +/// `module_global_proven_types` records the CLASS NAME the initializer +/// constructed. +fn class_name_is_number_valued_view(name: &str) -> bool { + matches!( + name, + "Uint8Array" + | "Uint8ClampedArray" + | "Int8Array" + | "Int16Array" + | "Uint16Array" + | "Int32Array" + | "Uint32Array" + | "Float32Array" + | "Float64Array" + | "Buffer" + ) +} + /// Locals whose every write is number-producing by construction. /// /// Deliberately independent of `PERRY_PTR_SHAPE_LOCALS`: the fact is about @@ -88,6 +111,7 @@ pub(crate) fn collect_number_by_construction_locals( spec_ta_lens: &HashMap, spec_numeric_params: &HashSet, not_bigint_locals: &HashSet, + module_global_proven_types: &HashMap, ) -> HashSet { if !enabled() { return HashSet::new(); @@ -101,7 +125,29 @@ pub(crate) fn collect_number_by_construction_locals( // pointer/string, so the fixpoint may treat it like a compiler-visible // local typed-view constructor on one side of `+` (where `undefined` // becomes the Number NaN rather than selecting string concatenation). - let numeric_ta_views: HashSet = spec_ta_lens.keys().copied().collect(); + let mut numeric_ta_views: HashSet = spec_ta_lens.keys().copied().collect(); + // #9363: a MODULE-GLOBAL typed array carries the same construction proof a + // body-local `const view = new Uint8Array(n)` does, and for the same + // reason: `module_global_proven_types` is derived from the initializer + // expression (`Expr::Uint8ArrayNew` / `TypedArrayNew`) on a single-`Let`, + // never-reassigned binding — it is not a declared type, which this + // collector correctly refuses to treat as evidence (#7773). + // + // Without this the fixpoint saw `const buf = new Uint8Array(N)` at module + // scope and answered "unknown receiver", so `acc += buf[i]` inside a + // function lost the accumulator's Number-by-construction proof. The cost + // was not the read: the missing proof made the `+` non-inert, which made + // `loop_purity::loop_may_allocate` answer `true`, which kept a per- + // iteration `load volatile @PERRY_GC_POLL_ARMED` in the inner loop — + // blocking vectorization and pinning the accumulator in memory — and + // routed the add through the rooted `guarded_add` diamond. Measured 444 ms + // vs 94 ms for the identical loop over a body-local receiver. + numeric_ta_views.extend( + module_global_proven_types + .iter() + .filter(|(_, ty)| matches!(ty, HirType::Named(name) if class_name_is_number_valued_view(name))) + .map(|(id, _)| *id), + ); let mut numeric = super::ptr_shape::collect_numeric_by_construction_locals_for_type_analysis( stmts, boxed_vars, diff --git a/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs b/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs new file mode 100644 index 0000000000..d5e1e88a9e --- /dev/null +++ b/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs @@ -0,0 +1,208 @@ +//! #9363 (A): a module-global typed-array receiver must carry the same +//! Number-by-construction proof a body-local `const` view does. +//! +//! `collectors/ptr_shape_numeric.rs` proved `view[i]` is Number-or-`undefined` +//! only from `numeric_ta_views` (spec-proven `TaPtr` params) or +//! `const_local_inits` (a compiler-visible `const` init in the SCANNED body). +//! A module-global `const buf = new Uint8Array(N)` read inside a function had +//! neither, so `acc += buf[i]` lost the accumulator's numeric proof, and the +//! add lowered through the rooted `guarded_add` diamond — a GC shadow-frame +//! load + store + `js_write_barrier_root_nanbox` per element, plus the +//! dynamic-add cold arm. +//! +//! Measured: 444 ms -> 94 ms, identical to the body-local receiver, against +//! node's 79. +//! +//! ATTRIBUTION, corrected by measurement. The missing proof ALSO leaves a +//! per-iteration `load volatile @PERRY_GC_POLL_ARMED` in the loop +//! (`loop_may_allocate` stays conservative when the `+` is not inert), and the +//! obvious story is that the volatile load blocks vectorization. It does not +//! pay: removing the poll under the same construction proof measured 94 ms +//! either way, and did not vectorize either. The whole 4.7x is the rooting +//! diamond. See the note in the test body and #9363. +//! +//! The pin is structural rather than a timing assertion: the dynamic-add +//! helper is the artifact the missing proof produced, and it is absent iff the +//! proof is present. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// `viaGlobal` reads a module-global view; `viaLocalConst` reads a body-local +/// one. The two must compile to the same shape of inner loop — that equality +/// is the fact under test, and it is what makes `viaLocalConst` a control +/// rather than a second assertion. +const SOURCE: &str = r#" +const N = 4096; +const gbuf = new Uint8Array(N); +for (let i = 0; i < N; i++) gbuf[i] = i % 256; + +function viaGlobal(): number { + let s = 0; + for (let i = 0; i < N; i++) s += gbuf[i]; + return s; +} + +function viaLocalConst(): number { + const b = new Uint8Array(N); + for (let i = 0; i < N; i++) b[i] = i % 256; + let s = 0; + for (let i = 0; i < N; i++) s += b[i]; + return s; +} + +console.log(viaGlobal() + "," + viaLocalConst()); +"#; + +const EXPECTED: &str = "522240,522240\n"; + +fn compile(dir: &Path, source: &str, extra_env: &[(&str, &str)]) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let mut cmd = Command::new(perry_bin()); + cmd.current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1"); + for (k, v) in extra_env { + cmd.env(k, v); + } + let compile = cmd.output().expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +/// The body of one emitted function, by its perry symbol suffix. +fn function_body(ir: &str, suffix: &str) -> String { + let mut out = String::new(); + let mut inside = false; + for line in ir.lines() { + if line.starts_with("define ") { + inside = line.contains(&format!("__{suffix}(")); + } else if inside { + if line.starts_with('}') { + break; + } + out.push_str(line); + out.push('\n'); + } + } + assert!(!out.is_empty(), "no emitted body found for `{suffix}`"); + out +} + +fn count(body: &str, needle: &str) -> usize { + body.lines().filter(|l| l.contains(needle)).count() +} + +fn run(bin: &Path, dir: &Path) -> Output { + Command::new(bin) + .current_dir(dir) + .output() + .expect("run compiled binary") +} + +/// A module-global view receiver earns the same numeric proof as a body-local +/// one: a native `fadd` rather than the rooted dynamic-add diamond. +#[test] +fn module_global_view_receiver_earns_the_numeric_proof() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE, &[]); + let ir = kept_ir(&stderr); + + let global = function_body(&ir, "viaGlobal"); + let local = function_body(&ir, "viaLocalConst"); + + // The control must itself be clean, or the equality below proves nothing. + assert_eq!( + count(&local, "js_dynamic_string_or_number_add"), + 0, + "control (body-local receiver) unexpectedly lost its numeric proof — \ + this test can no longer distinguish anything" + ); + + assert_eq!( + count(&global, "js_dynamic_string_or_number_add"), + 0, + "module-global receiver still routes `acc += buf[i]` through the \ + dynamic-add helper: the accumulator lost its Number-by-construction \ + proof" + ); + + // NOT asserted: that the module-global body has no GC poll. It still has + // one, because `can_lower_buffer_access_without_calls` demands a tracked + // `buffer_view_slots` entry that a module global never gets, so + // `loop_may_allocate` stays conservative. Admitting the read as inert + // under the construction proof was implemented and MEASURED FLAT (94 ms + // either way), and it did not unblock vectorization either — the + // remaining blocker is #9360's per-element admission-cache probe, a + // control-flow diamond in the loop body. Since `expr_is_inert_primitive` + // also governs rooting decisions, an unmeasured widening of it does not + // ship. See #9363. + + let out = run(&bin, dir.path()); + assert!(out.status.success(), "binary failed"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + EXPECTED, + "both receivers must sum identically" + ); +} + +/// The proof is a CONSTRUCTION proof, not an annotation: a reassigned +/// module-global must not be admitted, because a later write can put anything +/// in the binding. `module_global_proven_types` already excludes reassigned +/// bindings — this pins that the exclusion is load-bearing here. +#[test] +fn reassigned_module_global_is_not_admitted() { + const REASSIGNED: &str = r#" +const N = 64; +let gbuf: any = new Uint8Array(N); +for (let i = 0; i < N; i++) gbuf[i] = i; + +function sum(): number { + let s = 0; + for (let i = 0; i < N; i++) s += gbuf[i]; + return s; +} + +const first = sum(); +gbuf = "not a buffer"; +console.log(first + "," + typeof gbuf); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _stderr) = compile(dir.path(), REASSIGNED, &[]); + let out = run(&bin, dir.path()); + assert!(out.status.success(), "binary failed"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "2016,string\n", + "a reassigned module global must still read correctly" + ); +} From 7161823273bdbdd90b43269311b27459d404ca3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 11:20:56 +0200 Subject: [PATCH 3/8] perf(codegen): vectorize bounded byte-sum reductions (bench_buffer_readwrite 94 -> 34 ms, node 81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes that are each worth NOTHING alone and 2.8x together, which is why they land as one commit. **1. `fadd reassoc` on a proven reduction.** `acc = acc + ` in a trip-count-bounded loop keeps every partial sum below 2^53, where f64 addition is exact and therefore associative, so any grouping is bit-identical. An out-of-range read yields `undefined` -> NaN, which propagates through every grouping alike, so the OOB case needs no separate argument. This is an exactness proof about the value range, not a tolerance argument, which is why it does not need `--fast-math` (whose global reassociation is unsound for arbitrary f64 chains and is correctly off by default). `contract` is deliberately not added: FMA fusion changes multiply/add rounding, which this proof says nothing about. The admission reuses #7123's trip-count machinery unchanged, as a second mode with a weaker conclusion: a byte read counts with magnitude 255 and the limit is 2^53 rather than `i32::MAX`. The byte-read magnitude is admitted ONLY in this mode — an i32 slot cannot represent the NaN an out-of-range read produces, which is why the storage admission must keep refusing it. **2. Module-init shadow-slot pruning.** `codegen/function.rs` drops root slots for locals the whole-write proof shows can only hold a Number; module init never got that twin. `local_is_inert_primitive` refuses any local that HAS a slot, so a top-level accumulator that was ALREADY proven Number-by-construction was still not inert, `loop_may_allocate` stayed true, and the loop kept a per-iteration `load volatile @PERRY_GC_POLL_ARMED` — which blocks vectorization outright and pins the accumulator in memory. This was the entire reason the identical loop was fast inside a function and slow at top level. Found by instrumenting the purity decision, which printed `acc id=11 shadow=true nbc=true`. Module-scope construction proofs now also reach module-init, closure and method bodies, not just the spec-params path (#9363's first commit threaded only the latter). MEASURED, per change rather than stacked (quiet host, min-of-3): * `bench_buffer_readwrite` 94 -> 34 ms against node's 81. * reassoc alone, top level: 94 -> 94. Zero, because the poll blocks it. * the in-function loop, already poll-free, isolates reassoc: 94 -> 32. A third change was built and DELETED: admitting these accumulators to `local_is_inert_primitive` directly measured 36 vs 34 (noise) once the pruning made `number_by_construction` sufficient on its own, so it does not ship. Tests: `issue_9363_byte_reduction_vectorizes.rs` pins that the reduction carries `reassoc` AND that no poll follows it in that block (each half fails without the other), that an unbounded f64 accumulator does NOT reassociate, and that a pointer-valued module-scope local keeps its root slot — the last under `PERRY_GC_FORCE_EVACUATE`, which is the arm that would catch a slot pruned when it was genuinely needed. Node-differential battery (module-global / local / alias receivers, OOB, GC churn, slice-view aliasing) byte-identical, and identical again under heap-limit + forced evacuation. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/block.rs | 28 +++ crates/perry-codegen/src/codegen/closure.rs | 2 + crates/perry-codegen/src/codegen/entry.rs | 58 ++++- crates/perry-codegen/src/codegen/method.rs | 4 + .../perry-codegen/src/collectors/hir_facts.rs | 24 +- .../src/collectors/loop_bounded_i32.rs | 149 ++++++++++++- crates/perry-codegen/src/expr/binary.rs | 36 ++- .../issue_9363_byte_reduction_vectorizes.rs | 209 ++++++++++++++++++ 8 files changed, 500 insertions(+), 10 deletions(-) create mode 100644 crates/perry/tests/issue_9363_byte_reduction_vectorizes.rs diff --git a/crates/perry-codegen/src/block.rs b/crates/perry-codegen/src/block.rs index 4e91787663..55d7702753 100644 --- a/crates/perry-codegen/src/block.rs +++ b/crates/perry-codegen/src/block.rs @@ -466,6 +466,34 @@ impl LlBlock { r } + /// `fadd reassoc` for ONE instruction, independent of the module's + /// `--fast-math` setting (#9363). + /// + /// Only the caller's proof licenses this: the addends are byte reads + /// (magnitude <= 255, or the `undefined`-box NaN out of range) and the + /// enclosing loop's trip count is bounded, so every partial sum is either + /// an exactly-representable integer far below 2^53 — where f64 addition + /// is associative, so ANY grouping is bit-identical — or a NaN, which + /// propagates through every grouping alike. That is an exactness argument + /// about the value range, not a tolerance argument, which is why it does + /// not need `--fast-math` (whose global reassociation is NOT sound for + /// arbitrary f64 chains and is correctly off by default). + /// + /// `contract` is deliberately NOT added: FMA fusion changes rounding of + /// multiply/add pairs, which this proof says nothing about. + pub fn fadd_reassoc(&mut self, a: &str, b: &str) -> String { + let r = self.reg(); + self.push_inst(crate::inst::LlInst::Bin { + dst: r.clone(), + op: "fadd", + pre: "reassoc ", + ty: "double", + a: a.to_string(), + b: b.to_string(), + }); + r + } + pub fn fsub(&mut self, a: &str, b: &str) -> String { let r = self.reg(); self.push_inst(crate::inst::LlInst::Bin { diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 15de3d15c7..6506fa09ce 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -861,6 +861,8 @@ pub(super) fn compile_closure( classes, &cross_module.compile_time_constants, &cross_module.module_dispatch, + // #9363: a closure body reads the same module-scope views. + &cross_module.module_global_proven_types, ); if !versioned_loop_callback { if let Some(callback_shapes) = cross_module.array_callback_shapes.get(&func_id) { diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 9ed2a64cd3..9ad76bd2d8 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -720,7 +720,7 @@ pub(super) fn compile_module_entry( main.mark_entry_init_boundary(); let flat_const_ids: std::collections::HashSet = cross_module.flat_const_arrays.keys().copied().collect(); - let (main_shadow_slot_map, main_shadow_slot_clears_after_stmt) = + let (mut main_shadow_slot_map, main_shadow_slot_clears_after_stmt) = enable_module_init_shadow_frame(main, &hir.init, &flat_const_ids); let main_boxed_vars = module_boxed_vars.clone(); @@ -749,7 +749,34 @@ pub(super) fn compile_module_entry( classes, &cross_module.compile_time_constants, &cross_module.module_dispatch, + // #9363: module-scope views need their construction proofs here + // too — passing an empty map kept top-level accumulator loops on + // the rooted/guarded path while in-function ones were clean. + &cross_module.module_global_proven_types, ); + // #9363: the same redundant-shadow-slot pruning `codegen/function.rs` + // does, which module init never got. The shadow map is built above + // from the CONSERVATIVE pointer-typed-locals scan, before the fact + // graph exists; a local the whole-write proof later shows can only + // hold a Number keeps a root slot it can never need. That slot is not + // just wasted stores: `local_is_inert_primitive` refuses any local + // with one, so the accumulator of `sum += buf[i]` was never inert, + // `loop_may_allocate` stayed true, and the inner loop kept a + // per-iteration volatile GC poll that blocks vectorization. In a + // function body the same loop was already clean — this was the whole + // top-level/in-function asymmetry. + // + // `enable_post_init_shadow_frame` sized the frame from the unpruned + // map, so the retained slot indices stay valid with holes, exactly as + // in the function-body twin. + main_shadow_slot_map + .retain(|id, _| !main_native_facts.number_by_construction_locals().contains(id)); + let main_shadow_slot_clears_after_stmt = + crate::collectors::collect_shadow_slot_clear_points( + &hir.init, + &main_shadow_slot_map, + ); + // #7109: the program-entry body participates in canonical (i32/u32/Str) // selection on exactly the per-value rules a function body uses. There // is no structural context reason to deny — see @@ -1435,7 +1462,7 @@ pub(super) fn compile_module_entry( init_fn.mark_entry_init_boundary(); let flat_const_ids: std::collections::HashSet = cross_module.flat_const_arrays.keys().copied().collect(); - let (init_shadow_slot_map, init_shadow_slot_clears_after_stmt) = + let (mut init_shadow_slot_map, init_shadow_slot_clears_after_stmt) = enable_module_init_shadow_frame(init_fn, &hir.init, &flat_const_ids); let init_boxed_vars = module_boxed_vars.clone(); @@ -1463,7 +1490,34 @@ pub(super) fn compile_module_entry( classes, &cross_module.compile_time_constants, &cross_module.module_dispatch, + // #9363: module-scope views need their construction proofs here + // too — passing an empty map kept top-level accumulator loops on + // the rooted/guarded path while in-function ones were clean. + &cross_module.module_global_proven_types, ); + // #9363: the same redundant-shadow-slot pruning `codegen/function.rs` + // does, which module init never got. The shadow map is built above + // from the CONSERVATIVE pointer-typed-locals scan, before the fact + // graph exists; a local the whole-write proof later shows can only + // hold a Number keeps a root slot it can never need. That slot is not + // just wasted stores: `local_is_inert_primitive` refuses any local + // with one, so the accumulator of `sum += buf[i]` was never inert, + // `loop_may_allocate` stayed true, and the inner loop kept a + // per-iteration volatile GC poll that blocks vectorization. In a + // function body the same loop was already clean — this was the whole + // top-level/in-function asymmetry. + // + // `enable_post_init_shadow_frame` sized the frame from the unpruned + // map, so the retained slot indices stay valid with holes, exactly as + // in the function-body twin. + init_shadow_slot_map + .retain(|id, _| !init_native_facts.number_by_construction_locals().contains(id)); + let init_shadow_slot_clears_after_stmt = + crate::collectors::collect_shadow_slot_clear_points( + &hir.init, + &init_shadow_slot_map, + ); + // #7109: the module-init body participates in canonical (i32/u32/Str) // selection on exactly the per-value rules a function body uses. There // is no structural context reason to deny — see diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 48f9b7000c..c196bfbe38 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -361,6 +361,8 @@ pub(super) fn compile_method( classes, &cross_module.compile_time_constants, &cross_module.module_dispatch, + // #9363: a method body reads the same module-scope views. + &cross_module.module_global_proven_types, ); let mut index_clone_integer_locals = native_facts.integer_locals().clone(); index_clone_integer_locals.extend(index_param_ids.iter().copied()); @@ -1565,6 +1567,8 @@ pub(super) fn compile_static_method( classes, &cross_module.compile_time_constants, &cross_module.module_dispatch, + // #9363: a method body reads the same module-scope views. + &cross_module.module_global_proven_types, ); // Representation-selection context gates (see codegen/function.rs). diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index 97d800b423..6b2e59cc7e 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -64,6 +64,12 @@ pub(crate) struct RepresentationFacts { /// — it never widens the parallel-shadow `needs_i32_slot` gate. See /// `collectors/loop_bounded_i32.rs`. pub loop_bounded_i32_locals: HashSet, + /// #9363: accumulators whose `acc = acc + ` chain provably + /// stays below 2^53, so the update's `fadd` may carry `reassoc` and the + /// reduction can be split into parallel partial sums. Same trip-count + /// proof as `loop_bounded_i32_locals`, weaker conclusion — it changes no + /// storage decision, only an FMF flag. See `collectors/loop_bounded_i32.rs`. + pub reassociable_f64_accumulators: HashSet, /// Locals whose canonical-i32 promotion is PROVABLE but not PROFITABLE /// (#7128): written after declaration, no i32-consuming read anywhere in /// the body, and at least one double-consuming read inside a loop — so the @@ -224,6 +230,10 @@ impl TypeFacts { &self.representation.loop_bounded_i32_locals } + pub(crate) fn reassociable_f64_accumulators(&self) -> &HashSet { + &self.representation.reassociable_f64_accumulators + } + pub(crate) fn unprofitable_canonical_i32_locals(&self) -> &HashSet { &self.representation.unprofitable_canonical_i32_locals } @@ -541,6 +551,14 @@ pub(crate) fn collect_type_facts( } else { HashSet::new() }; + // #9363: the reassociation admission runs independently of the canonical + // i32 gate — it is not a storage decision, so `PERRY_CANONICAL_I32_LOCALS=0` + // must not silently disable it. + let reassociable_f64_accumulators = + super::loop_bounded_i32::collect_reassociable_f64_accumulators( + stmts, + compile_time_constants, + ); // #7123: this set now includes accumulators whose integer-ness and full // range were proved together (for example `sum += i % 1000`). The older // integer provenance collector deliberately does not accept bare `%`, so @@ -716,6 +734,7 @@ pub(crate) fn collect_type_facts( not_bigint_locals, int_valued_ta_locals, loop_bounded_i32_locals, + reassociable_f64_accumulators, unprofitable_canonical_i32_locals, number_by_construction_locals, }, @@ -777,6 +796,7 @@ pub(crate) fn collect_native_region_fact_graph( classes: &HashMap, compile_time_constants: &HashMap, module_dispatch: &super::ModuleDispatchFacts, + module_global_proven_types: &HashMap, ) -> NativeRegionFactGraph { collect_native_region_fact_graph_with_spec_params( stmts, @@ -794,7 +814,7 @@ pub(crate) fn collect_native_region_fact_graph( &HashSet::new(), &HashSet::new(), &HashSet::new(), - &HashMap::new(), + module_global_proven_types, ) } @@ -2195,6 +2215,7 @@ mod tests { &HashMap::new(), &constants, &crate::collectors::ModuleDispatchFacts::default(), + &HashMap::new(), ); assert!(graph.known_noalias_buffer_locals().contains(&1)); @@ -2286,6 +2307,7 @@ mod tests { &HashMap::new(), &HashMap::new(), &crate::collectors::ModuleDispatchFacts::default(), + &HashMap::new(), ); assert!(graph.integer_locals().contains(&1)); diff --git a/crates/perry-codegen/src/collectors/loop_bounded_i32.rs b/crates/perry-codegen/src/collectors/loop_bounded_i32.rs index 86256d4e94..ba3e9562d1 100644 --- a/crates/perry-codegen/src/collectors/loop_bounded_i32.rs +++ b/crates/perry-codegen/src/collectors/loop_bounded_i32.rs @@ -199,10 +199,66 @@ pub fn collect_loop_bounded_i32_locals( stmts, &st, &induction_intervals, + AccumulatorMode::I32Storage, )); out } +/// #9363: accumulators whose `acc = acc + ` chain provably stays +/// below 2^53, so the update's `fadd` may carry `reassoc`. +/// +/// Same proof, same trip-count machinery, weaker conclusion — see +/// [`AccumulatorMode`]. Consumed only by the emitter that adds the flag; it +/// changes no storage decision, so a wrong `true` cannot produce a wrapped or +/// mistyped value, only a differently-grouped sum (which the bound proves is +/// bit-identical anyway). +pub fn collect_reassociable_f64_accumulators( + stmts: &[Stmt], + compile_time_constants: &HashMap, +) -> HashSet { + let mut st = State::default(); + st.module_consts = compile_time_constants + .iter() + .filter_map(|(&id, &v)| { + (v.is_finite() && v.fract() == 0.0 && v.abs() <= i32::MAX as f64) + .then_some((id, v as i64)) + }) + .collect(); + collect_declarations(stmts, &mut st); + collect_const_ints(stmts, &mut st); + let empty: HashMap = HashMap::new(); + walk_stmts(stmts, &empty, &mut st); + + let induction_intervals: HashMap = st + .bounds + .iter() + .filter_map(|(&id, bound)| { + if st.disqualified.contains(&id) || st.bad_decl.contains(&id) { + return None; + } + let init = *st.declared_init.get(&id)?; + let interval = match bound.dir { + Dir::Inc => IntInterval { + lo: init, + hi: bound.extreme, + }, + Dir::Dec => IntInterval { + lo: bound.extreme, + hi: init, + }, + }; + (fits_i32(interval.lo) && fits_i32(interval.hi)).then_some((id, interval)) + }) + .collect(); + + collect_bounded_accumulator_locals( + stmts, + &st, + &induction_intervals, + AccumulatorMode::ReassocF64, + ) +} + fn fits_i32(n: i64) -> bool { i32::try_from(n).is_ok() } @@ -934,6 +990,46 @@ impl ExecutionBound { } } +/// Which admission the accumulator pass is computing. +/// +/// Both share one proof — `|acc| <= |A0| + sum(T * M)` over the trip-count +/// machinery above — and differ only in what magnitudes count as bounded and +/// how large the result may be. +#[derive(Clone, Copy, PartialEq, Eq)] +enum AccumulatorMode { + /// #7123: admit to canonical i32 STORAGE. Every addend must be an + /// integer-valued expression, because the slot cannot hold anything else. + I32Storage, + /// #9363: admit only to `fadd reassoc` on the accumulator update — the + /// value stays an f64 in the same slot it always used. + /// + /// This is a strictly weaker claim, so it admits strictly more: + /// + /// * a byte read (`buf[i]`) counts with magnitude 255. It is NOT + /// integer-valued in general — an out-of-range read is `undefined`, + /// i.e. NaN in arithmetic — which is exactly why `I32Storage` must + /// refuse it: an i32 slot cannot represent NaN. Reassociation does not + /// care: NaN propagates through every grouping alike, so a chain + /// containing one is NaN under any association, and a chain without one + /// is a sum of exact integers. + /// * the limit is 2^53 rather than `i32::MAX`, because the claim is only + /// that every partial sum is exactly representable as an f64 (where + /// addition is associative), not that it fits an integer register. + ReassocF64, +} + +impl AccumulatorMode { + fn limit(self) -> u128 { + match self { + // Below 2^53 every integer is exactly representable, so f64 + // addition is exact and therefore associative: any grouping of the + // same addends yields bit-identical results. + Self::ReassocF64 => 1u128 << 53, + Self::I32Storage => i32::MAX as u128, + } + } +} + #[derive(Default)] struct AccumulatorState { /// Sum of the worst-case magnitude contribution from every syntactic write @@ -948,6 +1044,7 @@ fn collect_bounded_accumulator_locals( stmts: &[Stmt], induction: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, ) -> HashSet { let mut accumulators = AccumulatorState::default(); walk_accumulator_stmts( @@ -955,6 +1052,7 @@ fn collect_bounded_accumulator_locals( ExecutionBound::function_body(), induction, induction_intervals, + mode, &mut accumulators, ); @@ -968,7 +1066,7 @@ fn collect_bounded_accumulator_locals( let init = *induction.declared_init.get(&id)?; let contribution = *accumulators.contribution.get(&id)?; let worst_magnitude = u128::from(init.unsigned_abs()).saturating_add(contribution); - (worst_magnitude <= i32::MAX as u128).then_some(id) + (worst_magnitude <= mode.limit()).then_some(id) }) .collect() } @@ -1031,6 +1129,7 @@ fn accumulator_step_magnitude( value: &Expr, st: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, ) -> Option { let Expr::Binary { op: BinaryOp::Add, @@ -1048,14 +1147,26 @@ fn accumulator_step_magnitude( } else { return None; }; - step_magnitude_bound(step, st, induction_intervals) + step_magnitude_bound(step, st, induction_intervals, mode) } fn step_magnitude_bound( e: &Expr, st: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, ) -> Option { + // A `Uint8Array` / `Buffer` element is a byte, so its magnitude is at most + // 255 — the tightest bound any addend in this analysis has. It is admitted + // ONLY under `ReassocF64`: an out-of-range read is `undefined`, hence NaN + // in arithmetic, which is not an integer value and must never reach an i32 + // slot. Reassociation tolerates it because NaN propagates identically + // through every grouping. See `AccumulatorMode`. + if mode == AccumulatorMode::ReassocF64 + && matches!(e, Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. }) + { + return Some(255); + } if let Some(value) = integer_literal(e) { return Some(u128::from(value.unsigned_abs())); } @@ -1109,10 +1220,12 @@ fn record_accumulator_write( execution: ExecutionBound, induction: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, accumulators: &mut AccumulatorState, ) { - let magnitude = value - .and_then(|value| accumulator_step_magnitude(id, value, induction, induction_intervals)); + let magnitude = value.and_then(|value| { + accumulator_step_magnitude(id, value, induction, induction_intervals, mode) + }); let Some((executions, magnitude)) = execution.executions.zip(magnitude) else { accumulators.disqualified.insert(id); return; @@ -1135,6 +1248,7 @@ fn walk_accumulator_stmts( execution: ExecutionBound, induction: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, accumulators: &mut AccumulatorState, ) { for stmt in stmts { @@ -1146,6 +1260,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1155,6 +1270,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ), Stmt::Return(value) => { @@ -1164,6 +1280,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1178,6 +1295,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); walk_accumulator_stmts( @@ -1185,6 +1303,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); if let Some(else_branch) = else_branch { @@ -1193,6 +1312,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1208,9 +1328,10 @@ fn walk_accumulator_stmts( unknown, induction, induction_intervals, + mode, accumulators, ); - walk_accumulator_stmts(body, unknown, induction, induction_intervals, accumulators); + walk_accumulator_stmts(body, unknown, induction, induction_intervals, mode, accumulators); } Stmt::For { init, @@ -1224,6 +1345,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1242,6 +1364,7 @@ fn walk_accumulator_stmts( execution.nested_loop(None), induction, induction_intervals, + mode, accumulators, ); } @@ -1251,6 +1374,7 @@ fn walk_accumulator_stmts( loop_execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1259,6 +1383,7 @@ fn walk_accumulator_stmts( loop_execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1272,6 +1397,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); if let Some(catch) = catch { @@ -1280,6 +1406,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1289,6 +1416,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1302,6 +1430,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); for case in cases { @@ -1311,6 +1440,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1319,6 +1449,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1328,6 +1459,7 @@ fn walk_accumulator_stmts( execution, induction, induction_intervals, + mode, accumulators, ), _ => {} @@ -1340,6 +1472,7 @@ fn walk_accumulator_expr( execution: ExecutionBound, induction: &State, induction_intervals: &HashMap, + mode: AccumulatorMode, accumulators: &mut AccumulatorState, ) { match expr { @@ -1350,6 +1483,7 @@ fn walk_accumulator_expr( execution, induction, induction_intervals, + mode, accumulators, ); walk_accumulator_expr( @@ -1357,6 +1491,7 @@ fn walk_accumulator_expr( execution, induction, induction_intervals, + mode, accumulators, ); } @@ -1366,6 +1501,7 @@ fn walk_accumulator_expr( execution, induction, induction_intervals, + mode, accumulators, ), Expr::Closure { body, .. } => { @@ -1377,7 +1513,7 @@ fn walk_accumulator_expr( accumulators.disqualified.extend(written); let unknown = execution.nested_loop(None); perry_hir::walker::walk_expr_children(expr, &mut |child| { - walk_accumulator_expr(child, unknown, induction, induction_intervals, accumulators) + walk_accumulator_expr(child, unknown, induction, induction_intervals, mode, accumulators) }); } _ => { @@ -1390,6 +1526,7 @@ fn walk_accumulator_expr( execution, induction, induction_intervals, + mode, accumulators, ) }); diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index 0ac92de03b..7ddb14a59b 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -485,6 +485,30 @@ fn chain_fold_is_sound(ctx: &FnCtx<'_>, parts: &[&Expr]) -> bool { .any(|p| crate::type_analysis::string_value_is_runtime_guaranteed(ctx, p)) } +/// Is this `+` the update of an accumulator whose whole chain is proven to +/// stay below 2^53 (#9363)? +/// +/// Shape: `acc + ` (either operand order), where `acc` is a local +/// the trip-count analysis admitted. The analysis proved the property over +/// EVERY write to that local in the function, so recognizing the shape here is +/// only selecting which `fadd` gets the flag — it is not itself the argument. +/// +/// The addend must still be checked at this site: the collector's bound covers +/// the writes it saw, and an `acc + ` node inside the same +/// function is a different expression that its bound does not license. +fn reduction_add_is_reassociable(ctx: &FnCtx<'_>, left: &Expr, right: &Expr) -> bool { + let admitted = |e: &Expr| { + matches!(e, Expr::LocalGet(id) if ctx.native_facts.reassociable_f64_accumulators().contains(id)) + }; + let byte_read = |e: &Expr| { + matches!( + e, + Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. } + ) + }; + (admitted(left) && byte_read(right)) || (admitted(right) && byte_read(left)) +} + fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> { // A stable-packed numeric clone has a stronger fact than the generic // untyped-local typed-array probe below: its preheader scanned the exact @@ -1248,8 +1272,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { }; let v = match op { BinaryOp::Add => { + // #9363: a proven byte-read reduction may reassociate, so + // LLVM can split the serial dependency chain into parallel + // partial sums (and vectorize it). The proof is the + // collector's, not this site's — see + // `collectors/loop_bounded_i32.rs::AccumulatorMode`. + let reassoc = reduction_add_is_reassociable(ctx, left, right); let blk = ctx.block(); - blk.fadd(&l, &r) + if reassoc { + blk.fadd_reassoc(&l, &r) + } else { + blk.fadd(&l, &r) + } } BinaryOp::Sub => { let blk = ctx.block(); diff --git a/crates/perry/tests/issue_9363_byte_reduction_vectorizes.rs b/crates/perry/tests/issue_9363_byte_reduction_vectorizes.rs new file mode 100644 index 0000000000..cdd552fb02 --- /dev/null +++ b/crates/perry/tests/issue_9363_byte_reduction_vectorizes.rs @@ -0,0 +1,209 @@ +//! #9363 (B): a bounded byte-read reduction reassociates, and module init +//! prunes the redundant shadow root slots that were blocking it. +//! +//! Two changes that are each worth NOTHING alone and 2.8x together — which is +//! why they ship as one commit and are pinned by one test. +//! +//! 1. **`fadd reassoc` on a proven reduction.** `acc = acc + ` in a +//! trip-count-bounded loop keeps every partial sum below 2^53, where f64 +//! addition is exact and therefore associative, so any grouping is +//! bit-identical. An out-of-range read yields `undefined` -> NaN, which +//! propagates through every grouping alike, so the OOB case needs no +//! separate argument. Without the flag LLVM cannot split the serial fadd +//! dependency chain and the loop runs at fadd latency (~3 cycles/element). +//! +//! 2. **Module-init shadow-slot pruning.** `codegen/function.rs` drops root +//! slots for locals the whole-write proof shows can only hold a Number; +//! module init never did. `local_is_inert_primitive` refuses any local +//! that HAS a slot, so a proven-numeric top-level accumulator was not +//! inert, `loop_may_allocate` stayed true, and the loop kept a +//! per-iteration `load volatile @PERRY_GC_POLL_ARMED` — which blocks +//! vectorization outright. This was the entire reason the same loop was +//! fast inside a function and slow at top level. +//! +//! Measured on `bench_buffer_readwrite` (quiet host, min-of-3): 94 -> 34 ms +//! against node's 81. Reassoc alone at top level: 94 -> 94. The in-function +//! loop, already poll-free, isolates reassoc's own contribution: 94 -> 32. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// A top-level byte-sum reduction — the `bench_buffer_readwrite` shape. +/// `N` is small so the test is fast; the emitted shape is what matters. +const SOURCE: &str = r#" +const N = 4096; +const buf = new Uint8Array(N); +for (let i = 0; i < N; i++) buf[i] = i % 256; + +let checksum = 0; +for (let iter = 0; iter < 3; iter++) { + let sum = 0; + for (let i = 0; i < N; i++) { + sum += buf[i]; + } + checksum += sum; +} +console.log("checksum:" + checksum); +"#; + +const EXPECTED: &str = "checksum:1566720\n"; + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1"); + } + command.output().expect("run compiled binary") +} + +/// Both halves are present in the emitted top-level loop, and the program is +/// still correct — including under forced evacuation, which is the arm that +/// would catch a root slot pruned when it was actually needed. +#[test] +fn top_level_byte_reduction_reassociates_and_drops_its_poll() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE); + let ir = kept_ir(&stderr); + + assert!( + ir.contains("fadd reassoc double"), + "the proven byte reduction must carry `reassoc`; without it LLVM \ + cannot break the serial fadd dependency chain" + ); + + // The poll count is the load-bearing half of the pruning fix. It is not + // asserted as zero for the whole module — the fill loop above writes + // through `buf[i] = ...` and other constructs may legitimately keep one — + // but the READ loop's block must not carry one. Locate the reassoc'd add + // and require no volatile poll load between it and its block's terminator. + let reassoc_line = ir + .lines() + .position(|l| l.contains("fadd reassoc double")) + .expect("reassoc add present"); + let tail: Vec<&str> = ir.lines().skip(reassoc_line).take(8).collect(); + assert!( + !tail.iter().any(|l| l.contains("PERRY_GC_POLL_ARMED")), + "the reduction loop still polls the GC every iteration, which blocks \ + vectorization — module-init shadow-slot pruning is not firing:\n{}", + tail.join("\n") + ); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!( + out.status.success(), + "binary failed (gc_stress={stress})\nstderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + EXPECTED, + "wrong sum (gc_stress={stress})" + ); + } +} + +/// The reassociation admission is a MAGNITUDE proof, so an accumulator whose +/// bound cannot be established must not get the flag. Here the addend is an +/// unbounded parameter rather than a byte read, so no bound exists. +#[test] +fn unbounded_accumulator_does_not_reassociate() { + const UNBOUNDED: &str = r#" +function addAll(xs: number[]): number { + let acc = 0; + for (let i = 0; i < xs.length; i++) acc += xs[i]; + return acc; +} +console.log("r:" + addAll([1.5, 2.25, 3.125])); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), UNBOUNDED); + let ir = kept_ir(&stderr); + assert!( + !ir.contains("fadd reassoc double"), + "an accumulator over arbitrary f64 array elements has no magnitude \ + bound, so reassociation is NOT exact for it and must not be emitted" + ); + let out = run(&bin, dir.path(), false); + assert_eq!(String::from_utf8_lossy(&out.stdout), "r:6.875\n"); +} + +/// A pointer-valued module-scope local must keep its shadow root slot: the +/// pruning is gated on the Number-by-construction proof, and dropping a slot +/// a real pointer needs would let the collector free a live object. Forced +/// evacuation is the arm that catches it. +#[test] +fn pointer_module_local_keeps_its_root_slot() { + const POINTERS: &str = r#" +const buf = new Uint8Array(64); +for (let i = 0; i < 64; i++) buf[i] = i; + +let sum = 0; +for (let i = 0; i < 64; i++) sum += buf[i]; + +let holder: string[] = ["seed"]; +let churn = 0; +for (let i = 0; i < 20000; i++) { + const garbage = { a: i, b: "x" + (i % 7) }; + churn += garbage.a & 1; + if (i % 5000 === 0) holder.push("k" + i); +} +console.log("sum:" + sum + " holder:" + holder.join(",") + " churn:" + churn); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _stderr) = compile(dir.path(), POINTERS); + let expected = "sum:2016 holder:seed,k0,k5000,k10000,k15000 churn:10000\n"; + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + expected, + "a live pointer local was lost (gc_stress={stress}) — the shadow \ + pruning dropped a root slot that was actually needed" + ); + } +} From cd933f997cbd8ee9a46bf1fb42a1b16d00737585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 11:49:59 +0200 Subject: [PATCH 4/8] perf(codegen): a DECLARED typed-array parameter earns the inline element load (576 -> 235 ms) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `receiver_class_name` answers only from `proven_local_types`, which is runtime-derived and therefore always empty for a PARAMETER — its value arrives from outside the body. So the shape this machinery was built for was the one shape it never served: bcryptjs's `_encipher(lr, off, P: Int32Array, S: Int32Array)` does ~600M `S[i]` reads through parameters and emitted a `js_typed_array_get` CALL for every one, while the identical loop over a module-global receiver took the inline checked load. Measured on `bench_typed_array_untyped_access`'s shape: the parameter body emits ZERO `ctaf.get` blocks, the module-global body 66. The declared class is read through `local_type_hint`, the audited escape hatch for "sites whose independent representation proof or runtime guard validates the current value". That is exactly this site: the emitted guard re-derives the truth from `PERRY_TA_KIND_CACHE`, so a wrong declaration misses the cache and defers to the memory-safe helper. A lying annotation costs a missed speedup, never a wrong answer — the same reasoning the module-global arm already carries, and strictly safer here because the guard validates the actual receiver. Reassigned bindings stay excluded per `receiver_class_name`'s #6906 rule. Applied to all three lanes that had the identical hole: the checked f64 read, its i32 twin, and #9342's u8 buffer read. MEASURED, and the two rows disagree in an instructive way: * `buf_ctx` (SIZE=1e6 x 50), `Uint8Array` parameter receiver: 576 -> 235 ms. * `bench_typed_array_untyped_access`: the change FIRES (0 -> 66 blocks) but is FLAT at 1216 ms. That benchmark's cost is its accumulator's dynamic add and shadow-frame rooting, not its reads — the #9361 family. Recorded rather than smoothed over: the same change is worth 2.4x where reads dominate and nothing where they do not. Also of note for that row: its headline metric is already at parity. The untyped/typed ratio it exists to track (#5525) is 1.03 against node's 1.03; the remaining gap is a flat ~4x on BOTH paths, which the ratio cannot express. Tests: `issue_9363_declared_param_typed_array.rs` pins that a declared param takes the inline load (with the module-global body asserted clean FIRST, so a regression disabling both lanes cannot pass vacuously), that a REASSIGNED param is refused, and — the claim the whole optimism rests on — that a LYING annotation still produces node-identical answers, with node itself as the oracle across a plain array, a plain object, a non-indexable scalar and a too-short array, under forced evacuation as well. The binding-type audit carries a written rationale for each of the three new `local_type_hint` uses (93 sites, OK). Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- .../perry-codegen/src/expr/i32_fast_path.rs | 33 ++- .../src/expr/ta_param_f64_read.rs | 33 ++- .../perry-codegen/src/expr/u8_buffer_read.rs | 11 + .../issue_9363_declared_param_typed_array.rs | 230 ++++++++++++++++++ scripts/local_binding_type_allowlist.json | 56 +++-- 5 files changed, 345 insertions(+), 18 deletions(-) create mode 100644 crates/perry/tests/issue_9363_declared_param_typed_array.rs diff --git a/crates/perry-codegen/src/expr/i32_fast_path.rs b/crates/perry-codegen/src/expr/i32_fast_path.rs index 7562f25ac5..0fbb69cbf7 100644 --- a/crates/perry-codegen/src/expr/i32_fast_path.rs +++ b/crates/perry-codegen/src/expr/i32_fast_path.rs @@ -567,6 +567,36 @@ fn ta_int_elem_load_is_i32_provable(ctx: &FnCtx<'_>, object: &Expr, index: &Expr /// full runtime `[[Get]]`+`ToInt32`. Returning `0` on OOB is exact *only* in the /// i32/`ToInt32` consumer context this predicate participates in — the sole /// observable value there — so it is confined to the i32-native fast path. +/// A DECLARED typed-array class on a non-reassigned local or parameter +/// (#9363/#5525). +/// +/// `receiver_class_name` answers only from `proven_local_types`, which is +/// runtime-derived and therefore empty for a PARAMETER — its value comes from +/// outside the body. That left the shape this machinery was built for on the +/// slow path: bcryptjs's `_encipher(lr, off, P: Int32Array, S: Int32Array)` +/// does ~600M `S[i]` reads through parameters and emitted a +/// `js_typed_array_get` CALL for every one, while the identical loop over a +/// module-global receiver took the inline checked load. Measured on +/// `bench_typed_array_untyped_access`: the param body emits zero `ctaf.get` +/// blocks, the module-global body 66. +/// +/// A declaration is not a lifetime proof, and this does not treat it as one. +/// It is an OPTIMISTIC hint whose only consumer is a load whose runtime guard +/// re-derives the truth: a receiver that is not the expected kind misses the +/// `PERRY_TA_KIND_CACHE` entry and defers to the memory-safe helper. So a +/// wrong hint costs a missed speedup, never a wrong answer — the same +/// reasoning the module-global arm already documents. Reassigned bindings are +/// still excluded, matching `receiver_class_name`'s own #6906 rule. +fn declared_typed_array_class_i32(ctx: &FnCtx<'_>, id: &u32) -> Option { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.local_type_hint(id)? { + perry_hir::types::Type::Named(name) => Some(name.clone()), + _ => None, + } +} + fn checked_typed_array_i32_kind( ctx: &FnCtx<'_>, object: &Expr, @@ -599,7 +629,8 @@ fn checked_typed_array_i32_kind( Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), _ => None, } - })?; + }) + .or_else(|| declared_typed_array_class_i32(ctx, id))?; i32_kind_from_class(&class) } diff --git a/crates/perry-codegen/src/expr/ta_param_f64_read.rs b/crates/perry-codegen/src/expr/ta_param_f64_read.rs index a004e6332b..ac6cb35ba9 100644 --- a/crates/perry-codegen/src/expr/ta_param_f64_read.rs +++ b/crates/perry-codegen/src/expr/ta_param_f64_read.rs @@ -53,6 +53,36 @@ enum F64Conv { /// (`typedarray/mod.rs`): the runtime `PERRY_TA_KIND_CACHE` stores `kind as u64`, /// and the entry guard compares against this tag — a mismatch would merely miss /// the cache and route every read to the slow helper (correct, but no speedup). +/// A DECLARED typed-array class on a non-reassigned local or parameter +/// (#9363/#5525). +/// +/// `receiver_class_name` answers only from `proven_local_types`, which is +/// runtime-derived and therefore empty for a PARAMETER — its value comes from +/// outside the body. That left the shape this machinery was built for on the +/// slow path: bcryptjs's `_encipher(lr, off, P: Int32Array, S: Int32Array)` +/// does ~600M `S[i]` reads through parameters and emitted a +/// `js_typed_array_get` CALL for every one, while the identical loop over a +/// module-global receiver took the inline checked load. Measured on +/// `bench_typed_array_untyped_access`: the param body emits zero `ctaf.get` +/// blocks, the module-global body 66. +/// +/// A declaration is not a lifetime proof, and this does not treat it as one. +/// It is an OPTIMISTIC hint whose only consumer is a load whose runtime guard +/// re-derives the truth: a receiver that is not the expected kind misses the +/// `PERRY_TA_KIND_CACHE` entry and defers to the memory-safe helper. So a +/// wrong hint costs a missed speedup, never a wrong answer — the same +/// reasoning the module-global arm already documents. Reassigned bindings are +/// still excluded, matching `receiver_class_name`'s own #6906 rule. +fn declared_typed_array_class_f64(ctx: &FnCtx<'_>, id: &u32) -> Option { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.local_type_hint(id)? { + perry_hir::types::Type::Named(name) => Some(name.clone()), + _ => None, + } +} + fn checked_typed_array_f64_kind( ctx: &FnCtx<'_>, object: &Expr, @@ -87,7 +117,8 @@ fn checked_typed_array_f64_kind( Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), _ => None, } - })?; + }) + .or_else(|| declared_typed_array_class_f64(ctx, id))?; f64_kind_from_class(&class) } diff --git a/crates/perry-codegen/src/expr/u8_buffer_read.rs b/crates/perry-codegen/src/expr/u8_buffer_read.rs index 4a4bfd89e9..b79a860799 100644 --- a/crates/perry-codegen/src/expr/u8_buffer_read.rs +++ b/crates/perry-codegen/src/expr/u8_buffer_read.rs @@ -70,6 +70,17 @@ fn u8_buffer_receiver_eligible(ctx: &FnCtx<'_>, object: &Expr) -> bool { Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), _ => None, } + }) + .or_else(|| { + // #9363: a declared `Uint8Array` parameter, on the same + // guard-validated-hint terms as the typed-array lanes. + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.local_type_hint(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } }); class.as_deref() == Some("Uint8Array") } diff --git a/crates/perry/tests/issue_9363_declared_param_typed_array.rs b/crates/perry/tests/issue_9363_declared_param_typed_array.rs new file mode 100644 index 0000000000..d73ed624b0 --- /dev/null +++ b/crates/perry/tests/issue_9363_declared_param_typed_array.rs @@ -0,0 +1,230 @@ +//! #9363/#5525: a DECLARED typed-array parameter earns the inline checked +//! element load, and a declaration that lies is still answered correctly. +//! +//! `receiver_class_name` answers only from `proven_local_types`, which is +//! runtime-derived and therefore empty for a parameter — its value arrives +//! from outside the body. So the shape this machinery exists for was the one +//! shape it never served: bcryptjs's +//! `_encipher(lr, off, P: Int32Array, S: Int32Array)` does ~600M `S[i]` reads +//! through parameters and emitted a `js_typed_array_get` CALL for every one, +//! while the identical loop over a module-global receiver took the inline +//! load (measured: 0 `ctaf.get` blocks vs 66). +//! +//! The fix reads the DECLARED type through `local_type_hint`, the audited +//! escape hatch for "sites whose independent representation proof or runtime +//! guard validates the current value". That is exactly this site: the emitted +//! guard re-derives the truth from `PERRY_TA_KIND_CACHE`, so a wrong +//! declaration misses the cache and defers to the memory-safe helper. A lying +//! annotation therefore costs a missed speedup, never a wrong answer — which +//! is what `wrong_declared_type_still_reads_correctly` pins, because that is +//! the only claim holding the optimism up. +//! +//! Measured on the u8 twin (`buf_ctx` fixture, SIZE=1e6 x 50): a +//! `Uint8Array` parameter receiver 576 -> 235 ms. On +//! `bench_typed_array_untyped_access` the same change fires (0 -> 66 blocks) +//! but is flat, because that benchmark's cost is its accumulator's dynamic +//! add and rooting, not its reads (#9361). + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +fn function_body(ir: &str, suffix: &str) -> String { + let mut out = String::new(); + let mut inside = false; + for line in ir.lines() { + if line.starts_with("define ") { + inside = line.contains(&format!("__{suffix}(")); + } else if inside { + if line.starts_with('}') { + break; + } + out.push_str(line); + out.push('\n'); + } + } + assert!(!out.is_empty(), "no emitted body found for `{suffix}`"); + out +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1"); + } + command.output().expect("run compiled binary") +} + +/// A declared `Int32Array` parameter takes the inline checked load, matching +/// the module-global receiver that already did. The module-global body is the +/// control: asserting it first means a regression that disables BOTH lanes +/// cannot pass this test by making the comparison vacuous. +#[test] +fn declared_typed_array_param_earns_the_inline_load() { + const SOURCE: &str = r#" +const S = new Int32Array(256); +for (let i = 0; i < 256; i++) S[i] = (i * 2654435761) | 0; + +function viaParam(a: Int32Array): number { + let n = 0; + for (let i = 0; i < 256; i++) n += a[i & 255]; + return n; +} + +function viaGlobal(): number { + let n = 0; + for (let i = 0; i < 256; i++) n += S[i & 255]; + return n; +} + +console.log(viaParam(S) + "," + viaGlobal()); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE); + let ir = kept_ir(&stderr); + + let global = function_body(&ir, "viaGlobal"); + assert!( + global.contains("ctaf.get"), + "control: the module-global receiver must take the inline checked \ + load — if it does not, this test can no longer distinguish anything" + ); + + let param = function_body(&ir, "viaParam"); + assert!( + param.contains("ctaf.get"), + "a declared `Int32Array` PARAMETER must take the same inline checked \ + load as the module-global receiver; without it every element read is \ + a `js_typed_array_get` call" + ); + + let out = run(&bin, dir.path(), false); + assert!(out.status.success(), "binary failed"); + let stdout = String::from_utf8_lossy(&out.stdout); + let (a, b) = stdout.trim().split_once(',').expect("two sums"); + assert_eq!(a, b, "param and global receivers must sum identically"); +} + +/// **The claim the optimism rests on.** A declaration is not a lifetime +/// proof, so the lane is only sound because the emitted guard re-derives the +/// truth. Hand a function declared to take an `Int32Array` something that is +/// not one and the answers must still match node exactly — the guard misses +/// `PERRY_TA_KIND_CACHE` and defers to the memory-safe helper. +#[test] +fn wrong_declared_type_still_reads_correctly() { + const LYING: &str = r#" +function sum3(a: Int32Array): string { + let out = ""; + for (let i = 0; i < 3; i++) out += String(a[i]) + "|"; + return out; +} + +const real = new Int32Array([10, 20, 30]); +const plainArray: any = [40, 50, 60]; +const plainObject: any = { 0: 70, 1: 80, 2: 90 }; +const notIndexable: any = 12345; +const shortArray: any = new Int32Array([1]); + +console.log("real:" + sum3(real)); +console.log("array:" + sum3(plainArray as Int32Array)); +console.log("object:" + sum3(plainObject as Int32Array)); +console.log("scalar:" + sum3(notIndexable as Int32Array)); +console.log("short:" + sum3(shortArray as Int32Array)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _stderr) = compile(dir.path(), LYING); + + // Node is the oracle: whatever it prints for each lying receiver is the + // answer the guard must reproduce, including `undefined` for the reads + // that fall off the end or off a non-indexable value. + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!( + node.status.success(), + "node failed on the oracle fixture:\n{}", + String::from_utf8_lossy(&node.stderr) + ); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&node.stdout), + "a lying `Int32Array` annotation changed the ANSWER (gc_stress={stress}) \ + — the declared-type hint must only ever cost a missed speedup" + ); + } +} + +/// A reassigned parameter is excluded, matching `receiver_class_name`'s #6906 +/// rule: a later write can replace the binding with anything, so the +/// declaration describes at most its first value. +#[test] +fn reassigned_param_is_not_admitted() { + const REASSIGNED: &str = r#" +function f(a: Int32Array, swap: boolean): number { + if (swap) a = new Int32Array([7, 7, 7]) as Int32Array; + let n = 0; + for (let i = 0; i < 3; i++) n += a[i]; + return n; +} +const base = new Int32Array([1, 2, 3]); +console.log(f(base, false) + "," + f(base, true)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _stderr) = compile(dir.path(), REASSIGNED); + let out = run(&bin, dir.path(), false); + assert!(out.status.success(), "binary failed"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "6,21\n", + "a reassigned parameter must still read the value it actually holds" + ); +} diff --git a/scripts/local_binding_type_allowlist.json b/scripts/local_binding_type_allowlist.json index 735c0b07a9..ba25e0587e 100644 --- a/scripts/local_binding_type_allowlist.json +++ b/scripts/local_binding_type_allowlist.json @@ -105,6 +105,14 @@ "classification": "representation-proven", "reason": "Strict Symbol identity lowering accepts only constructor-derived runtime proof; the proof API rejects every binding written in the region, and fresh or registered Symbol storage is non-moving, so raw NaN-boxed pointer equality is the representation contract." }, + { + "path": "crates/perry-codegen/src/expr/i32_fast_path.rs", + "function": "declared_typed_array_class_i32", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The declared typed-array class is an OPTIMISTIC hint for the checked element load, whose emitted runtime guard re-derives the truth: a receiver of another kind misses PERRY_TA_KIND_CACHE and defers to the memory-safe helper, so a wrong hint costs a missed speedup, never a wrong answer. Reassigned bindings are excluded (#6906). Needed because proven_local_types is empty for a PARAMETER, which left bcryptjs's `S: Int32Array` reads on a per-element runtime call (#9363/#5525)." + }, { "path": "crates/perry-codegen/src/expr/index_get.rs", "function": "is_width_tracked_typed_array_receiver", @@ -169,14 +177,6 @@ "classification": "representation-proven", "reason": "The proof API supplies only runtime-derived initializer evidence and rejects the binding after any write in the region." }, - { - "path": "crates/perry-codegen/src/expr/property_get/helpers.rs", - "function": "guarded_declared_class_get_candidate", - "access": "local_type_hint", - "count": 1, - "classification": "runtime-validated", - "reason": "The class hint selects only the guarded plain-field IC; its live class-id and keys-token checks dominate raw access, while accessors and method binding require proven receiver provenance." - }, { "path": "crates/perry-codegen/src/expr/property_get.rs", "function": "lower", @@ -193,6 +193,14 @@ "classification": "representation-proven", "reason": "The proof API supplies only runtime-derived initializer evidence and rejects the binding after any write in the region." }, + { + "path": "crates/perry-codegen/src/expr/property_get/helpers.rs", + "function": "guarded_declared_class_get_candidate", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The class hint selects only the guarded plain-field IC; its live class-id and keys-token checks dominate raw access, while accessors and method binding require proven receiver provenance." + }, { "path": "crates/perry-codegen/src/expr/property_set.rs", "function": "guarded_declared_class_store_candidate", @@ -217,6 +225,22 @@ "classification": "representation-proven", "reason": "The proof API supplies only runtime-derived initializer evidence and rejects the binding after any write in the region." }, + { + "path": "crates/perry-codegen/src/expr/static_field_meta.rs", + "function": "lower", + "access": "local_type_hint", + "count": 1, + "classification": "representation-proven", + "reason": "The Array hint on a named class expression's compiler-private self binding is not a user annotation: the shared-mutable capture rewrite is what both sets it and allocates the one-element cell, in the same pass, so the cell representation holds by construction wherever the hint is present. The raw POINTER_MASK_I64 decode and js_array_set_f64 below are therefore reaching a cell the compiler itself created; the else arm stores the plain value for an unrewritten binding." + }, + { + "path": "crates/perry-codegen/src/expr/ta_param_f64_read.rs", + "function": "declared_typed_array_class_f64", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The declared typed-array class is an OPTIMISTIC hint for the checked element load, whose emitted runtime guard re-derives the truth: a receiver of another kind misses PERRY_TA_KIND_CACHE and defers to the memory-safe helper, so a wrong hint costs a missed speedup, never a wrong answer. Reassigned bindings are excluded (#6906). Needed because proven_local_types is empty for a PARAMETER, which left bcryptjs's `S: Int32Array` reads on a per-element runtime call (#9363/#5525)." + }, { "path": "crates/perry-codegen/src/expr/typed_array_rmw.rs", "function": "receiver_is_uint32_candidate", @@ -225,6 +249,14 @@ "classification": "runtime-validated", "reason": "The Uint32Array hint only nominates a receiver for the direct RMW route; the emitted code proves it at run time with a POINTER_TAG check, the inline-storage view guard, a cached-address match and a UINT32 kind match, and re-checks the view/kind/bounds guard after the RHS so a receiver mutated by the RHS falls back to the generic path." }, + { + "path": "crates/perry-codegen/src/expr/u8_buffer_read.rs", + "function": "u8_buffer_receiver_eligible", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The declared typed-array class is an OPTIMISTIC hint for the checked element load, whose emitted runtime guard re-derives the truth: a receiver of another kind misses PERRY_U8_INLINE_CACHE and defers to the memory-safe helper, so a wrong hint costs a missed speedup, never a wrong answer. Reassigned bindings are excluded (#6906). Needed because proven_local_types is empty for a PARAMETER, which left bcryptjs's `S: Int32Array` reads on a per-element runtime call (#9363/#5525)." + }, { "path": "crates/perry-codegen/src/lower_call/closure_analysis.rs", "function": "hazardous_module_global_ids", @@ -640,14 +672,6 @@ "count": 1, "classification": "metadata-only", "reason": "This indexed scope-stack iteration performs lexical name resolution during HIR construction and does not establish a codegen runtime representation." - }, - { - "path": "crates/perry-codegen/src/expr/static_field_meta.rs", - "function": "lower", - "access": "local_type_hint", - "count": 1, - "classification": "representation-proven", - "reason": "The Array hint on a named class expression's compiler-private self binding is not a user annotation: the shared-mutable capture rewrite is what both sets it and allocates the one-element cell, in the same pass, so the cell representation holds by construction wherever the hint is present. The raw POINTER_MASK_I64 decode and js_array_set_f64 below are therefore reaching a cell the compiler itself created; the else arm stores the plain value for an unrewritten binding." } ] } From b469336eaff08176c38e1c383684ece174147854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 12:38:58 +0200 Subject: [PATCH 5/8] perf(codegen): a loop-reseeded accumulator earns its i32 slot (bench_typed_array_untyped_access 1216 -> 257 ms, node 290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectors/int_valued_ta_locals.rs` exists for bcryptjs `_encipher` — its module doc IS that function — but rejected its own subject's accumulator. The wrap-i32 additive arm was admitted only for a STRAIGHT-LINE (never in-loop) Add/Sub tree, and `n += S[...]` sits in the Feistel `while`. So `n` stayed an f64 slot holding nothing but int32 values, and every S-box step emitted `sitofp` in and `llvm.aarch64.fjcvtzs` out around the `fadd`. WHY THE RESTRICTION WAS TOO COARSE. Its stated hazard is real: an unbounded in-loop chain can carry the true f64 value past 2^53, where it ROUNDS while an i32 slot WRAPS, and rule (2) only guarantees the `ToInt32` image is observed — so the two would then disagree. A per-iteration re-seed removes exactly that hazard, and needs no dominance argument: if the body unconditionally assigns the local a fresh exact-i32 value once per iteration, the chain restarts every iteration no matter WHERE the re-seed sits, so the magnitude never exceeds one body's worth of addends. With each addend below 2^31 a body would need ~4M additive writes to reach 2^53; `_encipher` re-seeds and adds twice, so `|n| < 2^33`. The scan is deliberately narrow. The re-seed must sit at the loop body's TOP level: one nested in an `if`/`switch`/`try` may not run on a given iteration, which is precisely the case where the chain keeps growing. Nested loops are scanned as their own bodies, so an inner loop's re-seed never bounds the outer body's chain. MEASURED (quiet host): typed 1216 -> 257 ms and untyped 1254 -> 258 against node's 290 / 299 — from 4.2x slower to faster than node on BOTH paths. The fixture's own checksum oracle, which throws on any divergence between the typed and untyped states, passes identically. This was the last suite row above node. VERIFIED, and one honest gap. The promotion DECISIONS were checked directly through `PERRY_REPSEL_DEBUG`: `n` is promoted in both `encipher` bodies and in an unconditionally-reseeded fixture, and is refused for a loop with no re-seed, one whose re-seed is `if`-guarded, and one whose re-seed is in an inner loop. Node-differential battery (including those adversarial shapes) byte-identical, and identical again under `PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1`. I could NOT prove the conditional-re-seed guard independently load-bearing: I sabotaged it (letting `if`-nested re-seeds count) and failed across three fixture attempts to construct a case whose outcome changes — each failed for a different reason (rule (2) rejected the local first; power-of-two addends made wrapped and exact coincide; a module-global receiver did not reproduce the spec-param admission conditions). So it is defense-in-depth of unproven necessity, stated rather than claimed — the same honesty `loop_safepoint_purity.rs` applies to its own shadow-slot half. Tests: `issue_9363_loop_reseeded_accumulator.rs` pins the Feistel round against node and pins that the three unbounded shapes stay f64, with the oracle itself guarded (the fixture asserts its expected values still exceed i32 range, so a wrongly promoted local would print a wrapped negative rather than a near miss). Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- .../src/collectors/int_valued_ta_locals.rs | 125 +++++++++- .../issue_9363_loop_reseeded_accumulator.rs | 224 ++++++++++++++++++ 2 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs diff --git a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs index b3419da4f0..7dc0a9cf61 100644 --- a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs +++ b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs @@ -704,6 +704,14 @@ pub fn collect_int_valued_ta_locals( // (an `undefined`-able operand breaks `image == ToInt32(true)` through a // float add — `undefined + 1` is NaN→0, the image path would say 1). let additive_invalid = additive_flow_invalid_targets(stmts, &types, &ta_lens, &numeric_locals); + // #9363: locals a loop body re-seeds unconditionally every iteration, which + // bounds an in-loop additive chain to one body's worth. See + // `collect_loop_reseeded_locals` for why no dominance argument is needed. + let loop_reseeded = { + let mut out = HashSet::new(); + collect_loop_reseeded_locals(stmts, &types, guarded_number_array_params, false, &mut out); + out + }; // Rule (1) admission. A candidate is a `let`-declared local with ≥1 // int-TA-read write, whose EVERY write is i32-producing-safe (or, in the @@ -729,7 +737,7 @@ pub fn collect_int_valued_ta_locals( pool.retain(|id| { facts.writes[id].iter().all(|(w, in_loop)| { write_is_i32_producing_safe(w, &types, guarded_number_array_params, &numeric_locals) - || (!in_loop + || ((!in_loop || loop_reseeded.contains(id)) && !additive_invalid.contains(id) && additive_write_admissible( w, @@ -978,6 +986,121 @@ fn collect_facts<'a>( } } +/// Locals that a loop body RE-SEEDS on every iteration with a non-additive, +/// i32-producing write (#9363/#6898 follow-up). +/// +/// The wrap-i32 additive arm is otherwise restricted to straight-line trees +/// because an in-loop chain can carry the true f64 value past 2^53, where it +/// rounds while the i32 slot wraps — and rule (2) only guarantees the ToInt32 +/// image is observed, so the two would then disagree. +/// +/// A re-seed removes exactly that hazard, and does so WITHOUT needing a +/// dominance argument: if a loop body unconditionally assigns the local a +/// fresh exact-i32 value on every iteration, then whatever additive writes the +/// same body performs, the local's magnitude never exceeds one body's worth of +/// them — the order of the re-seed within the body does not matter, because +/// the chain restarts once per iteration either way. With every addend below +/// 2^31, the true value stays under `(writes_per_body + 1) * 2^31`, and a body +/// would need ~4M additive writes to reach 2^53. +/// +/// This is why the scan is deliberately narrow: the re-seed must sit at the +/// loop body's TOP level. A re-seed nested in an `if`/`switch`/`try` may not +/// run on a given iteration, which is precisely the case where the chain can +/// keep growing. Nested loops are scanned as their own bodies, so an inner +/// loop is judged by its own re-seeds, never by the outer body's. +/// +/// The motivating shape is bcryptjs `_encipher`, this module's own subject: +/// `n = S[l >>> 24]` re-seeds every round, followed by at most two `+=` before +/// a bitwise write, so `|n| < 2^33`. +fn collect_loop_reseeded_locals<'a>( + stmts: &'a [Stmt], + types: &HashMap, + guarded_number_array_params: &HashSet, + inside_loop: bool, + out: &mut HashSet, +) { + for stmt in stmts { + // Only a TOP-LEVEL `x = ` in a loop body counts as a re-seed. + if inside_loop { + if let Stmt::Expr(Expr::LocalSet(id, rhs)) = stmt { + if write_is_i32_producing_safe( + rhs, + types, + guarded_number_array_params, + &HashSet::new(), + ) { + out.insert(*id); + } + } + } + match stmt { + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => { + collect_loop_reseeded_locals(body, types, guarded_number_array_params, true, out); + } + Stmt::For { body, init, .. } => { + if let Some(init) = init.as_deref() { + collect_loop_reseeded_locals( + std::slice::from_ref(init), + types, + guarded_number_array_params, + inside_loop, + out, + ); + } + collect_loop_reseeded_locals(body, types, guarded_number_array_params, true, out); + } + // Conditional and unwinding scaffolding: walk for NESTED loops, but + // nothing directly inside them is an unconditional re-seed of the + // enclosing body, so the flag is cleared. + Stmt::If { + then_branch, + else_branch, + .. + } => { + collect_loop_reseeded_locals( + then_branch, + types, + guarded_number_array_params, + false, + out, + ); + if let Some(eb) = else_branch { + collect_loop_reseeded_locals(eb, types, guarded_number_array_params, false, out); + } + } + Stmt::Try { + body, + catch, + finally, + } => { + collect_loop_reseeded_locals(body, types, guarded_number_array_params, false, out); + if let Some(c) = catch { + collect_loop_reseeded_locals( + &c.body, + types, + guarded_number_array_params, + false, + out, + ); + } + if let Some(f) = finally { + collect_loop_reseeded_locals(f, types, guarded_number_array_params, false, out); + } + } + Stmt::Labeled { body, .. } => { + collect_loop_reseeded_locals( + std::slice::from_ref(body), + types, + guarded_number_array_params, + inside_loop, + out, + ); + } + _ => {} + } + } +} + fn record_write<'a>( id: u32, rhs: &'a Expr, diff --git a/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs b/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs new file mode 100644 index 0000000000..1735f3b50b --- /dev/null +++ b/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs @@ -0,0 +1,224 @@ +//! #9363/#6898: an in-loop additive write is i32-admissible when the loop body +//! RE-SEEDS the local unconditionally on every iteration. +//! +//! `collectors/int_valued_ta_locals.rs` exists for bcryptjs `_encipher` — its +//! module doc is that function — but rejected its own subject's accumulator. +//! The wrap-i32 additive arm was restricted to STRAIGHT-LINE (never in-loop) +//! trees, and `n += S[...]` sits in the Feistel `while`. So `n` stayed an f64 +//! slot holding nothing but int32 values, and every S-box step emitted +//! `sitofp` in and `llvm.aarch64.fjcvtzs` out around the `fadd`. +//! +//! ## Why the restriction was too coarse +//! +//! Its stated hazard is real: an unbounded in-loop chain can carry the true +//! f64 value past 2^53, where it ROUNDS while an i32 slot WRAPS, and rule (2) +//! only guarantees the `ToInt32` image is observed — so the two would then +//! disagree. +//! +//! A per-iteration re-seed removes exactly that hazard, and needs no dominance +//! argument: if the body unconditionally assigns the local a fresh exact-i32 +//! value once per iteration, the chain restarts every iteration no matter +//! WHERE the re-seed sits, so the magnitude never exceeds one body's worth of +//! addends. With each addend below 2^31, a body would need ~4M additive writes +//! to reach 2^53. `_encipher` re-seeds `n` every round and adds at most twice: +//! `|n| < 2^33`. +//! +//! The re-seed must be at the body's TOP level. One nested in an `if` may not +//! run on a given iteration — precisely the case where the chain keeps +//! growing — and an inner loop's re-seed does not bound the OUTER body's +//! chain. Both are pinned below, because they are the shapes that would ship +//! wrapped arithmetic if the rule were widened carelessly. +//! +//! Measured on `bench_typed_array_untyped_access`: typed 1216 -> 257 ms and +//! untyped 1254 -> 258 against node's 290 / 299 — from 4.2x slower to faster +//! than node on both paths. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> PathBuf { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + output +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1"); + } + command.output().expect("run compiled binary") +} + +/// Every shape whose additive chain is NOT bounded by an unconditional +/// per-iteration re-seed must keep its f64 representation. Node is the oracle +/// rather than a hand-written expectation, because the whole question is +/// whether perry's value agrees with JavaScript's. +/// +/// The elements are `2^30`, so four of them exceed `i32::MAX`: a local wrongly +/// promoted to an i32 slot prints a WRAPPED negative here, not a slightly +/// different number. Cases A and D deliberately produce values above i32 range +/// (and A above 2^53's neighbourhood) so the failure would be unmissable. +#[test] +fn only_unconditionally_reseeded_accumulators_are_admitted() { + const SOURCE: &str = r#" +const S = new Int32Array(8); +for (let i = 0; i < 8; i++) S[i] = 0x40000000; + +// A: no reseed at all — accumulates across 1e6 iterations. +function noReseed(): number { + let n = S[0]; + for (let i = 0; i < 1000000; i++) { n += S[i & 7]; } + return n; +} +// B: reseed guarded by an `if` — does not run every iteration. +function condReseed(): number { + let n = S[0]; + for (let i = 0; i < 1000; i++) { + if (i === 500) { n = S[1]; } + n += S[i & 7]; + } + return n; +} +// C: unconditional top-level reseed — the admissible shape. +function goodReseed(): number { + let n = 0; let acc = 0; + for (let i = 0; i < 1000; i++) { n = S[i & 7]; n += S[(i + 1) & 7]; acc ^= n; } + return acc; +} +// D: reseed only in an INNER loop — the outer chain is still unbounded. +function innerOnly(): number { + let n = S[0]; + for (let i = 0; i < 1000; i++) { + for (let j = 0; j < 2; j++) { n = S[j]; } + n += S[i & 7]; + } + return n; +} +// E: out-of-range reads mixed in — `undefined` must behave as JS says. +function withOob(): string { + let n = S[0]; + let out = ""; + for (let i = 0; i < 4; i++) { n = S[i + 6]; n += S[i]; out += String(n) + ";"; } + return out; +} +console.log("A:" + noReseed()); +console.log("B:" + condReseed()); +console.log("C:" + goodReseed()); +console.log("D:" + innerOnly()); +console.log("E:" + withOob()); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let bin = compile(dir.path(), SOURCE); + + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!( + node.status.success(), + "node failed on the oracle fixture:\n{}", + String::from_utf8_lossy(&node.stderr) + ); + // Guard the oracle itself: if these stopped exceeding i32 range the test + // would still pass while having lost its ability to detect a wrap. + let expected = String::from_utf8_lossy(&node.stdout).into_owned(); + assert!( + expected.contains("A:1073742897741824") && expected.contains("D:2147483648"), + "the oracle no longer produces values outside i32 range, so a wrongly \ + admitted local would no longer be detectable here:\n{expected}" + ); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + expected, + "an accumulator was promoted to an i32 slot without a bounded \ + chain (gc_stress={stress}) — the value wrapped" + ); + } +} + +/// The motivating shape itself: a Feistel round that re-seeds its accumulator +/// and adds twice before a bitwise write. Values must match node exactly. +#[test] +fn feistel_round_accumulator_matches_node() { + const SOURCE: &str = r#" +const P = new Int32Array(18); +const S = new Int32Array(1024); +for (let i = 0; i < P.length; i++) P[i] = (i * 40503 + 7) | 0; +for (let i = 0; i < S.length; i++) S[i] = (i * 2654435761) | 0; + +function encipher(lr: number[], off: number, P: Int32Array, S: Int32Array): void { + let n: number; + let l = lr[off]; + let r = lr[off + 1]; + l ^= P[0]; + let i = 0; + while (i < 16) { + n = S[l >>> 24]; + n += S[0x100 | ((l >> 16) & 0xff)]; + n ^= S[0x200 | ((l >> 8) & 0xff)]; + n += S[0x300 | (l & 0xff)]; + r ^= n ^ P[++i]; + n = S[r >>> 24]; + n += S[0x100 | ((r >> 16) & 0xff)]; + n ^= S[0x200 | ((r >> 8) & 0xff)]; + n += S[0x300 | (r & 0xff)]; + l ^= n ^ P[++i]; + } + lr[off] = r ^ P[17]; + lr[off + 1] = l; +} + +const lr = [0x01234567, 0x89abcdef]; +for (let c = 0; c < 64; c++) encipher(lr, 0, P, S); +console.log(lr[0] + "," + lr[1]); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let bin = compile(dir.path(), SOURCE); + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed"); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&node.stdout), + "Feistel state diverged from node (gc_stress={stress})" + ); + } +} From 03b9d2aedcca969e8b05d9023c9d66edeb3fbae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 13:13:30 +0200 Subject: [PATCH 6/8] perf(codegen): the packed-f64 loop clone needs no GC poll (bench_numeric_array_numeric 45 -> 38 ms, node 38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stmt/loops.rs` already skips the back-edge poll inside three loop-clone fact scopes, and its comment states the rule and predicts this exact case: a poll exists so an ALLOCATING body can defer a collection; `loop_may_allocate` answers from the HIR, where `arr[i] = e` is a generic `IndexSet` that CAN reallocate; and inside a fact scope codegen knows better, because the clone is call-free or it is not entered. The packed-f64 clone is that body and was simply not listed. Its entry guard proves a live packed raw-f64 plain Array with the loop window in bounds, its reads and writes lower to bare `double` load/store over existing slots (so nothing grows, reallocates, or writes a heap edge), and its matcher admits no calls, closures or awaits into the body — the same conjunction the three listed clones rest on. This is therefore not a new licence. WHY IT COST MORE THAN ITS OWN INSTRUCTIONS. The armed word is loaded VOLATILE, which is a clobber inside the loop, so the cached packed receiver base had to be re-derived on every element — the effect #9316's stride comment already describes. That is why striding the poll 1-in-64 did not recover the loss while removing it does: the cost was the clobber, not the frequency. MEASURED (250k x 250, quiet host, min-of-3): 45 -> 38 ms against node's 38. A forced-arm build with polls disabled entirely also lands on 38, so this recovers the whole gap and nothing more — the diagnostic bounded the win before the change was written. Tests: `issue_9379_packed_f64_clone_poll.rs` asserts no poll inside the CLONE's own blocks (module-wide counting would assert something this change never claimed — the fill and outer loops keep their polls), with a vacuity guard that the fixture still admits the tier, and correctness under `PERRY_GC_FORCE_EVACUATE` + `PERRY_GC_VERIFY_EVACUATION`, which is the arm that matters when a safepoint is removed. A sibling test pins that an allocating loop still polls, so the skip stays scoped to the fact. Both assertions in the first draft were wrong and perry was right: I counted polls module-wide, and I hand-computed an expected checksum incorrectly. Both now use node as the oracle or the clone's own region. `loop_safepoint_purity` 8/8 and codegen 1379/1379 green. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/stmt/loops.rs | 17 ++ .../tests/issue_9379_packed_f64_clone_poll.rs | 212 ++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 crates/perry/tests/issue_9379_packed_f64_clone_poll.rs diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 231ff185e3..9dd6ad697c 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -7287,6 +7287,23 @@ pub(crate) fn emit_gc_loop_safepoint( if !ctx.element_shape_loop_facts.is_empty() || !ctx.class_field_loop_facts.is_empty() || !ctx.stable_packed_loop_facts.is_empty() + // #9379: the packed-f64 loop clone is the next body the paragraph above + // predicted — "the next body shape admitted to the matcher that is not + // provably inert would delete that clone the same way", except this one + // IS provably inert and was simply not listed. Its entry guard proves a + // live packed raw-f64 plain Array with the window in bounds, its reads + // and writes lower to bare `double` load/store on existing slots (so no + // growth, no realloc, no barrier), and its matcher admits no calls, + // closures or awaits into the body. `loop_may_allocate` cannot see any + // of that: it answers from the HIR, where `arr[i] = e` is a generic + // `IndexSet` that CAN reallocate, which is why it demanded a poll here. + // + // The poll was not merely costing its own instructions. Its volatile + // load is a clobber inside the loop, so the cached receiver base had to + // be re-derived per element — which is why striding it 1-in-64 (#9316) + // did not recover the loss and removing it does. Measured on + // `bench_numeric_array_numeric`: 45 -> 38 ms against node's 38. + || !ctx.packed_f64_loop_facts.is_empty() || ctx.versioned_indexed_loop_facts.last().is_some_and(|fact| { matches!( fact.guard_mode, diff --git a/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs b/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs new file mode 100644 index 0000000000..82cc0cc610 --- /dev/null +++ b/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs @@ -0,0 +1,212 @@ +//! #9379: the packed-f64 loop clone does not need a GC poll, and paid dearly +//! for having one. +//! +//! `stmt/loops.rs` already skips the back-edge poll inside three loop-clone +//! fact scopes, with the reasoning written out there: a poll exists so an +//! ALLOCATING body can defer a collection, `loop_may_allocate` answers from +//! the HIR (where `arr[i] = e` is a generic `IndexSet` that CAN reallocate), +//! and inside a fact scope codegen knows better because the clone is call-free +//! or it is not entered. That comment even anticipates the next clone admitted +//! on the identical argument. +//! +//! The packed-f64 clone IS that clone and was simply not listed. Its entry +//! guard proves a live packed raw-f64 plain Array with the loop window in +//! bounds; reads and writes lower to bare `double` load/store over existing +//! slots, so nothing grows, reallocates, or writes a heap edge; and the +//! matcher admits no calls, closures or awaits into the body. +//! +//! ## Why it cost more than its own instructions +//! +//! The poll's armed word is loaded VOLATILE, which is a clobber inside the +//! loop, so the cached packed receiver base had to be re-derived on every +//! element. That is why striding the poll 1-in-64 (#9316) did not recover the +//! loss while removing it does: the cost was the clobber, not the frequency. +//! +//! Measured on `bench_numeric_array_numeric` (250k x 250, quiet host): +//! 45 -> 38 ms against node's 38. A forced-arm build with polls disabled +//! entirely also lands on 38, so this recovers the whole gap and no more. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// The `bench_numeric_array_numeric` shape: a read-modify-write over a +/// `number[]` that the packed-f64 range tier claims. +const SOURCE: &str = r#" +const SIZE = 4096; +const arr: number[] = []; +for (let i = 0; i < SIZE; i++) arr.push(i); +let checksum = 0; +for (let iter = 0; iter < 8; iter++) { + for (let i = 0; i < arr.length; i++) arr[i] = arr[i] + 1; + checksum = checksum + arr[0] + arr[arr.length - 1]; +} +console.log("checksum:" + checksum); +"#; + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +/// The emitted lines belonging to the packed-f64 fast clone: from its loop +/// condition block to its exit. Everything outside is another tier's code and +/// is none of this test's business. +fn packed_clone_region(ir: &str) -> String { + let lines: Vec<&str> = ir.lines().collect(); + let start = lines + .iter() + .position(|l| l.starts_with("for.packed_f64_fast.cond")) + .unwrap_or_else(|| panic!("no packed-f64 fast clone in the emitted IR")); + let end = lines[start..] + .iter() + .position(|l| l.starts_with("for.packed_f64_fast.exit")) + .map(|off| start + off) + .unwrap_or(lines.len()); + lines[start..end].join("\n") +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +/// The clone's fast block carries no poll, and the program is still correct — +/// including under forced, verified evacuation, which is the arm that matters +/// when a safepoint has been removed. +#[test] +fn packed_f64_clone_emits_no_poll_and_stays_correct() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE); + let ir = kept_ir(&stderr); + + // Vacuity guard first: the absence below is only meaningful while the + // fixture still ADMITS the packed-f64 clone. If the tier stops claiming + // this loop, "no poll" would pass for the wrong reason. + assert!( + ir.contains("packed_f64_range_store.fast") || ir.contains("for.packed_f64_fast"), + "fixture no longer admits the packed-f64 loop clone, so the poll \ + assertion below would pass vacuously" + ); + + // Count polls INSIDE the clone only. The module's other loops (the fill + // loop, the outer iteration loop) are not claimed by this tier and keep + // their polls legitimately — counting module-wide would assert something + // this change never claimed. + let clone_polls = packed_clone_region(&ir) + .lines() + .filter(|l| l.contains("PERRY_GC_POLL_ARMED")) + .count(); + assert_eq!( + clone_polls, 0, + "the packed-f64 clone still polls the GC; its volatile armed load is a \ + clobber inside the loop, which forces the cached receiver base to be \ + re-derived per element" + ); + + // Node is the oracle rather than a hand-computed constant. My first draft + // asserted a value I derived by hand and got wrong; perry was right and the + // test was the bug. An oracle cannot make that mistake. + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed on the oracle fixture"); + let expected = String::from_utf8_lossy(&node.stdout).into_owned(); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!( + out.status.success(), + "binary failed (gc_stress={stress})\nstderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + expected, + "packed-f64 clone produced the wrong sum (gc_stress={stress})" + ); + } +} + +/// A loop the clone does NOT claim keeps its poll: the skip is scoped to the +/// fact, not applied to loops generally. Here the body calls out, so the +/// matcher refuses it and `loop_may_allocate` is right to demand a safepoint. +#[test] +fn a_calling_loop_still_polls() { + const CALLING: &str = r#" +const arr: number[] = []; +for (let i = 0; i < 512; i++) arr.push(i); +const box: string[] = []; +for (let i = 0; i < arr.length; i++) { + arr[i] = arr[i] + 1; + box.push("g" + (i % 3)); // allocates: the clone must not claim this + if (box.length > 16) box.length = 0; +} +console.log("n:" + arr[0] + "," + arr[511] + "," + box.length); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), CALLING); + let ir = kept_ir(&stderr); + assert!( + ir.contains("PERRY_GC_POLL_ARMED"), + "an allocating loop body must keep its GC poll — the #9379 skip is \ + scoped to the packed-f64 fact, not a general licence" + ); + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed on the oracle fixture"); + let out = run(&bin, dir.path(), true); + assert!(out.status.success(), "binary failed under gc stress"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&node.stdout) + ); +} From 41c48d107e3c14bae102bf395ddf9319d8db38e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 11:45:33 +0000 Subject: [PATCH 7/8] fix(runtime): harden byte-read optimization checks --- changelog.d/9360-u8-byte-reductions.md | 14 +++++++ crates/perry-codegen/src/codegen/entry.rs | 28 ++++++------- .../src/collectors/int_valued_ta_locals.rs | 8 +++- .../src/collectors/loop_bounded_i32.rs | 18 +++++++- crates/perry-codegen/src/expr/binary.rs | 12 ++---- .../perry-codegen/src/expr/i32_fast_path.rs | 21 +++++----- .../src/expr/ta_param_f64_read.rs | 21 +++++----- .../perry-codegen/src/expr/u8_buffer_read.rs | 41 ++++++++++--------- crates/perry-runtime/src/buffer/header.rs | 1 - crates/perry-runtime/src/buffer/mod.rs | 2 +- crates/perry-runtime/src/gc/tests/mod.rs | 2 +- crates/perry-runtime/src/typedarray/access.rs | 5 ++- 12 files changed, 103 insertions(+), 70 deletions(-) create mode 100644 changelog.d/9360-u8-byte-reductions.md diff --git a/changelog.d/9360-u8-byte-reductions.md b/changelog.d/9360-u8-byte-reductions.md new file mode 100644 index 0000000000..fdee6338e0 --- /dev/null +++ b/changelog.d/9360-u8-byte-reductions.md @@ -0,0 +1,14 @@ +**Typed-array hot loops now retain their native numeric reductions.** +Buffer-backed `Uint8Array` reads recover the correct element on typed-array +registry misses and use a guard-validated inline byte-load lane for +module-global and declared-parameter receivers. Construction-proven +module-global numeric views now feed the same Number-by-construction proof as +body-local views, removing the rooted dynamic-add diamond from byte sums. + +Bounded byte reductions may carry per-instruction `reassoc` when their complete +integer magnitude proof stays within the exact f64 range, allowing LLVM to +split the serial accumulator without enabling unsound global fast-math. +Module-init accumulators proven never to hold pointers also shed redundant +shadow slots and back-edge GC polls. On the measured `bench_buffer_readwrite` +shape this takes Perry from 94 ms to 34 ms against Node's 81 ms, while +unbounded f64 reductions remain unchanged. diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 9ad76bd2d8..8efb96624e 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -720,7 +720,7 @@ pub(super) fn compile_module_entry( main.mark_entry_init_boundary(); let flat_const_ids: std::collections::HashSet = cross_module.flat_const_arrays.keys().copied().collect(); - let (mut main_shadow_slot_map, main_shadow_slot_clears_after_stmt) = + let (mut main_shadow_slot_map, _) = enable_module_init_shadow_frame(main, &hir.init, &flat_const_ids); let main_boxed_vars = module_boxed_vars.clone(); @@ -769,13 +769,13 @@ pub(super) fn compile_module_entry( // `enable_post_init_shadow_frame` sized the frame from the unpruned // map, so the retained slot indices stay valid with holes, exactly as // in the function-body twin. - main_shadow_slot_map - .retain(|id, _| !main_native_facts.number_by_construction_locals().contains(id)); + main_shadow_slot_map.retain(|id, _| { + !main_native_facts + .number_by_construction_locals() + .contains(id) + }); let main_shadow_slot_clears_after_stmt = - crate::collectors::collect_shadow_slot_clear_points( - &hir.init, - &main_shadow_slot_map, - ); + crate::collectors::collect_shadow_slot_clear_points(&hir.init, &main_shadow_slot_map); // #7109: the program-entry body participates in canonical (i32/u32/Str) // selection on exactly the per-value rules a function body uses. There @@ -1462,7 +1462,7 @@ pub(super) fn compile_module_entry( init_fn.mark_entry_init_boundary(); let flat_const_ids: std::collections::HashSet = cross_module.flat_const_arrays.keys().copied().collect(); - let (mut init_shadow_slot_map, init_shadow_slot_clears_after_stmt) = + let (mut init_shadow_slot_map, _) = enable_module_init_shadow_frame(init_fn, &hir.init, &flat_const_ids); let init_boxed_vars = module_boxed_vars.clone(); @@ -1510,13 +1510,13 @@ pub(super) fn compile_module_entry( // `enable_post_init_shadow_frame` sized the frame from the unpruned // map, so the retained slot indices stay valid with holes, exactly as // in the function-body twin. - init_shadow_slot_map - .retain(|id, _| !init_native_facts.number_by_construction_locals().contains(id)); + init_shadow_slot_map.retain(|id, _| { + !init_native_facts + .number_by_construction_locals() + .contains(id) + }); let init_shadow_slot_clears_after_stmt = - crate::collectors::collect_shadow_slot_clear_points( - &hir.init, - &init_shadow_slot_map, - ); + crate::collectors::collect_shadow_slot_clear_points(&hir.init, &init_shadow_slot_map); // #7109: the module-init body participates in canonical (i32/u32/Str) // selection on exactly the per-value rules a function body uses. There diff --git a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs index 7dc0a9cf61..d4b5e4dee8 100644 --- a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs +++ b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs @@ -1065,7 +1065,13 @@ fn collect_loop_reseeded_locals<'a>( out, ); if let Some(eb) = else_branch { - collect_loop_reseeded_locals(eb, types, guarded_number_array_params, false, out); + collect_loop_reseeded_locals( + eb, + types, + guarded_number_array_params, + false, + out, + ); } } Stmt::Try { diff --git a/crates/perry-codegen/src/collectors/loop_bounded_i32.rs b/crates/perry-codegen/src/collectors/loop_bounded_i32.rs index ba3e9562d1..e2f67b8e46 100644 --- a/crates/perry-codegen/src/collectors/loop_bounded_i32.rs +++ b/crates/perry-codegen/src/collectors/loop_bounded_i32.rs @@ -1331,7 +1331,14 @@ fn walk_accumulator_stmts( mode, accumulators, ); - walk_accumulator_stmts(body, unknown, induction, induction_intervals, mode, accumulators); + walk_accumulator_stmts( + body, + unknown, + induction, + induction_intervals, + mode, + accumulators, + ); } Stmt::For { init, @@ -1513,7 +1520,14 @@ fn walk_accumulator_expr( accumulators.disqualified.extend(written); let unknown = execution.nested_loop(None); perry_hir::walker::walk_expr_children(expr, &mut |child| { - walk_accumulator_expr(child, unknown, induction, induction_intervals, mode, accumulators) + walk_accumulator_expr( + child, + unknown, + induction, + induction_intervals, + mode, + accumulators, + ) }); } _ => { diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index 7ddb14a59b..4910f3ad0c 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -497,15 +497,9 @@ fn chain_fold_is_sound(ctx: &FnCtx<'_>, parts: &[&Expr]) -> bool { /// the writes it saw, and an `acc + ` node inside the same /// function is a different expression that its bound does not license. fn reduction_add_is_reassociable(ctx: &FnCtx<'_>, left: &Expr, right: &Expr) -> bool { - let admitted = |e: &Expr| { - matches!(e, Expr::LocalGet(id) if ctx.native_facts.reassociable_f64_accumulators().contains(id)) - }; - let byte_read = |e: &Expr| { - matches!( - e, - Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. } - ) - }; + let admitted = |e: &Expr| matches!(e, Expr::LocalGet(id) if ctx.native_facts.reassociable_f64_accumulators().contains(id)); + let byte_read = + |e: &Expr| matches!(e, Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. }); (admitted(left) && byte_read(right)) || (admitted(right) && byte_read(left)) } diff --git a/crates/perry-codegen/src/expr/i32_fast_path.rs b/crates/perry-codegen/src/expr/i32_fast_path.rs index 0fbb69cbf7..17229c14dc 100644 --- a/crates/perry-codegen/src/expr/i32_fast_path.rs +++ b/crates/perry-codegen/src/expr/i32_fast_path.rs @@ -621,16 +621,17 @@ fn checked_typed_array_i32_kind( // guard-protected (a wrong class misses the runtime KIND cache and defers // to `js_typed_array_read_int32`); a reassigned binding is still excluded. // Mirrors the f64 sibling (`ta_param_f64_read.rs`). - let class = crate::type_analysis::receiver_class_name(ctx, object).or_else(|| { - if ctx.reassigned_locals.contains(id) { - return None; - } - match ctx.module_global_proven_types.get(id) { - Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), - _ => None, - } - }) - .or_else(|| declared_typed_array_class_i32(ctx, id))?; + let class = crate::type_analysis::receiver_class_name(ctx, object) + .or_else(|| { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.module_global_proven_types.get(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } + }) + .or_else(|| declared_typed_array_class_i32(ctx, id))?; i32_kind_from_class(&class) } diff --git a/crates/perry-codegen/src/expr/ta_param_f64_read.rs b/crates/perry-codegen/src/expr/ta_param_f64_read.rs index ac6cb35ba9..7f3a03e558 100644 --- a/crates/perry-codegen/src/expr/ta_param_f64_read.rs +++ b/crates/perry-codegen/src/expr/ta_param_f64_read.rs @@ -109,16 +109,17 @@ fn checked_typed_array_f64_kind( // bindings are still excluded so a rebind can't make the proof stale for // the local-proof case; for the module-global case the runtime guard is // the safety net regardless. #8595-followup / typed-array read inlining. - let class = crate::type_analysis::receiver_class_name(ctx, object).or_else(|| { - if ctx.reassigned_locals.contains(id) { - return None; - } - match ctx.module_global_proven_types.get(id) { - Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), - _ => None, - } - }) - .or_else(|| declared_typed_array_class_f64(ctx, id))?; + let class = crate::type_analysis::receiver_class_name(ctx, object) + .or_else(|| { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.module_global_proven_types.get(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } + }) + .or_else(|| declared_typed_array_class_f64(ctx, id))?; f64_kind_from_class(&class) } diff --git a/crates/perry-codegen/src/expr/u8_buffer_read.rs b/crates/perry-codegen/src/expr/u8_buffer_read.rs index b79a860799..582180e432 100644 --- a/crates/perry-codegen/src/expr/u8_buffer_read.rs +++ b/crates/perry-codegen/src/expr/u8_buffer_read.rs @@ -62,26 +62,27 @@ fn u8_buffer_receiver_eligible(ctx: &FnCtx<'_>, object: &Expr) -> bool { if ctx.buffer_view_slots.contains_key(id) { return false; } - let class = crate::type_analysis::receiver_class_name(ctx, object).or_else(|| { - if ctx.reassigned_locals.contains(id) { - return None; - } - match ctx.module_global_proven_types.get(id) { - Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), - _ => None, - } - }) - .or_else(|| { - // #9363: a declared `Uint8Array` parameter, on the same - // guard-validated-hint terms as the typed-array lanes. - if ctx.reassigned_locals.contains(id) { - return None; - } - match ctx.local_type_hint(id) { - Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), - _ => None, - } - }); + let class = crate::type_analysis::receiver_class_name(ctx, object) + .or_else(|| { + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.module_global_proven_types.get(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } + }) + .or_else(|| { + // #9363: a declared `Uint8Array` parameter, on the same + // guard-validated-hint terms as the typed-array lanes. + if ctx.reassigned_locals.contains(id) { + return None; + } + match ctx.local_type_hint(id) { + Some(perry_hir::types::Type::Named(name)) => Some(name.clone()), + _ => None, + } + }); class.as_deref() == Some("Uint8Array") } diff --git a/crates/perry-runtime/src/buffer/header.rs b/crates/perry-runtime/src/buffer/header.rs index 81bcfa1429..9de0d674f7 100644 --- a/crates/perry-runtime/src/buffer/header.rs +++ b/crates/perry-runtime/src/buffer/header.rs @@ -706,7 +706,6 @@ pub fn asymmetric_key_meta(addr: usize) -> Option<(u8, u8)> { ASYMMETRIC_KEY_REGISTRY.with(|r| r.borrow().get(&addr).copied()) } - /// #9342: direct-mapped inline-read admission cache for `Uint8Array`-backing /// `BufferHeader`s, exported under a stable link name for the codegen's /// guarded inline byte load (`perry-codegen/src/expr/u8_buffer_read.rs`). diff --git a/crates/perry-runtime/src/buffer/mod.rs b/crates/perry-runtime/src/buffer/mod.rs index 3af6e1098f..96622fdd6b 100644 --- a/crates/perry-runtime/src/buffer/mod.rs +++ b/crates/perry-runtime/src/buffer/mod.rs @@ -46,9 +46,9 @@ pub use header::{BufferHeader, BUFFER_TYPE_ID, SMALL_BUF_THRESHOLD}; // ---- Re-exports: allocation / registry helpers ---- pub(crate) use header::is_small_buf_slab_addr; // #9342: primed by `typedarray::js_u8_buffer_read_f64` (codegen slow arm). -pub(crate) use header::u8_inline_cache_try_prime; #[cfg(test)] pub(crate) use header::test_u8_inline_cache_holds; +pub(crate) use header::u8_inline_cache_try_prime; // `shared_sab` publishes process-global backings that `is_registered_buffer` // reports as buffers without them entering `BUFFER_REGISTRY`, so it arms the // same monotone latch — before the backing becomes reachable. diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 4ab3f775a1..4b3d4cf8e3 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -6,7 +6,6 @@ mod block_pool_pressure; mod budgeted_step_api; mod buffer_bound_method_name; mod buffer_side_tables; -mod u8_inline_cache; mod contract; mod copying; mod copying_side_tables; @@ -55,4 +54,5 @@ mod telemetry_verifier; mod temp_roots; mod triggers; mod typed_layout_intact_residual; +mod u8_inline_cache; mod weak_read_barrier; diff --git a/crates/perry-runtime/src/typedarray/access.rs b/crates/perry-runtime/src/typedarray/access.rs index a60672a3d1..bf882cabfd 100644 --- a/crates/perry-runtime/src/typedarray/access.rs +++ b/crates/perry-runtime/src/typedarray/access.rs @@ -237,7 +237,10 @@ static KEEP_JS_TYPED_ARRAY_READ_F64: extern "C" fn(*const TypedArrayHeader, i32) #[no_mangle] pub extern "C" fn js_u8_buffer_read_f64(target: *const TypedArrayHeader, index: i32) -> f64 { let addr = strip_nanbox(target as u64); - if addr >= 0x1000 { + // Pointer-tagged registry handles share this ABI with heap receivers but + // are never dereferenceable. Keep them out of the admission probe; the + // delegated getter below owns their ordinary JS-value semantics. + if crate::value::addr_class::is_above_handle_band(addr) { crate::buffer::u8_inline_cache_try_prime(addr); } js_uint8array_index_get_value(addr as *const TypedArrayHeader, index) From 91e0dd9a09d4e613487bae4821d3d708993be51e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 22:03:11 +0200 Subject: [PATCH 8/8] fix(runtime): exclude registered buffer views from the u8 inline cache A view's inline bytes are only a snapshot; runtime reads resolve through buffer/view.rs to the authoritative backing, which a sibling typed array can change without refreshing that snapshot. Admitting a view made the first read correct (cache miss, authoritative path) and every later cache-hit read stale -- Uint8Array over a Uint32Array's buffer returned 4 0 0 0 where node gives 4 3 2 1 (#7219 fixture, regressed by #9342's admission). --- .../perry-codegen/src/expr/u8_buffer_read.rs | 9 +++-- crates/perry-runtime/src/buffer/header.rs | 20 +++++++---- .../src/gc/tests/u8_inline_cache.rs | 35 +++++++++++++++++++ crates/perry-runtime/src/typedarray/access.rs | 4 +-- .../perry/tests/issue_9342_u8_inline_read.rs | 13 +++++-- 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/crates/perry-codegen/src/expr/u8_buffer_read.rs b/crates/perry-codegen/src/expr/u8_buffer_read.rs index 582180e432..b70e0864d1 100644 --- a/crates/perry-codegen/src/expr/u8_buffer_read.rs +++ b/crates/perry-codegen/src/expr/u8_buffer_read.rs @@ -16,7 +16,8 @@ //! //! * **guard**: NaN-box pointer tag + full-address hit in //! `PERRY_U8_INLINE_CACHE` (`perry-runtime/src/buffer/header.rs`), whose -//! entries name live, u8-marked, inline-storage `BufferHeader`s only. The +//! entries name live, u8-marked, owning inline-storage `BufferHeader`s +//! only. Foreign-backed buffers and registered views are excluded. The //! cache is primed by the slow arm and invalidated on buffer death and //! address reuse, so a hit is proof of the layout contract; //! * **bounds**: `idx ult length` (`ult` also rejects negative indices); @@ -30,8 +31,10 @@ //! //! READS ONLY. An inline **write** twin would bypass the `buffer/view.rs` //! write-propagation protocol and desynchronize slice/`new Uint8Array(ab)` -//! aliases (#1205); view copies are admissible here precisely because writes -//! all still propagate. +//! aliases (#1205). Registered views are excluded from read admission too: +//! their inline payload is only a snapshot, while runtime reads resolve to the +//! authoritative backing, which sibling typed-array writes can change without +//! refreshing that snapshot (#9360/#7219). use anyhow::Result; use perry_hir::Expr; diff --git a/crates/perry-runtime/src/buffer/header.rs b/crates/perry-runtime/src/buffer/header.rs index 9de0d674f7..ffe597098c 100644 --- a/crates/perry-runtime/src/buffer/header.rs +++ b/crates/perry-runtime/src/buffer/header.rs @@ -711,14 +711,17 @@ pub fn asymmetric_key_meta(addr: usize) -> Option<(u8, u8)> { /// guarded inline byte load (`perry-codegen/src/expr/u8_buffer_read.rs`). /// /// An entry holds the full address of a **live, `mark_as_uint8array`-marked -/// `BufferHeader` whose bytes are inline at `header + 8`** (no foreign -/// backing). Under that contract the emitted reader may do +/// owning `BufferHeader` whose authoritative bytes are inline at +/// `header + 8`** (no foreign backing and no registered view). Under that +/// contract the emitted reader may do /// `len = *(u32*)addr; addr + 8 + idx` directly: /// -/// * view copies (`js_buffer_slice` / `new Uint8Array(arrayBuffer)`) ARE -/// admissible — their inline bytes are kept current by the write-propagation -/// protocol in `buffer/view.rs` (reads never go stale; only an inline WRITE -/// fast path would break aliasing, and this cache feeds no write path); +/// * view copies (`js_buffer_slice` / `new Uint8Array(arrayBuffer)`) are +/// excluded — their inline bytes are only a snapshot. Runtime reads resolve +/// through `buffer/view.rs` to the authoritative backing, which can change +/// without refreshing that snapshot (for example through a sibling typed +/// array), so admitting a view would make the first read correct and later +/// cache-hit reads stale; /// * foreign-backed wrappers (`buffer_alloc_foreign`, bun:ffi externals) are /// excluded at prime time — their header is a lone `BufferHeader` with no /// inline payload, so `header + 8` is past the allocation; @@ -758,7 +761,10 @@ pub(crate) fn u8_inline_cache_invalidate(addr: usize) { /// above. Called from the codegen slow arm (`js_u8_buffer_read_f64`) so a /// guard miss primes the next access; never called on a hot path. pub(crate) fn u8_inline_cache_try_prime(addr: usize) { - if is_uint8array_buffer(addr) && foreign_backing(addr).is_none() { + if is_uint8array_buffer(addr) + && foreign_backing(addr).is_none() + && super::view::lookup(addr).is_none() + { PERRY_U8_INLINE_CACHE[u8_inline_cache_slot(addr)] .store(addr as u64, std::sync::atomic::Ordering::Relaxed); } diff --git a/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs b/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs index 4fcebc4093..3966a65264 100644 --- a/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs +++ b/crates/perry-runtime/src/gc/tests/u8_inline_cache.rs @@ -73,6 +73,41 @@ fn test_prime_rejects_foreign_backed_wrapper() { crate::buffer::finalize_collected_dead_buffer(addr); } +/// A registered Uint8Array view keeps only a snapshot in its inline payload; +/// runtime reads resolve to the authoritative backing. Admitting the view +/// would make a backing-side write visible on the first cache-miss read and +/// disappear again on the next cache-hit read (#9360/#7219). +#[test] +fn test_prime_rejects_registered_view() { + let _guard = GcTestIsolationGuard::new(); + + let backing = crate::buffer::js_array_buffer_new(4); + let boxed_backing = crate::value::js_nanbox_pointer(backing as i64); + let view = crate::buffer::js_uint8array_new(boxed_backing); + let addr = view as usize; + + unsafe { + *crate::buffer::buffer_data_mut(backing).add(1) = 0xAB; + } + assert_eq!( + crate::buffer::js_buffer_index_get_value(view, 1), + 0xAB as f64, + "test premise: the runtime read resolves the view to its backing" + ); + assert_eq!( + unsafe { *crate::buffer::buffer_data(view).add(1) }, + 0, + "test premise: the view's inline snapshot is stale" + ); + + crate::buffer::u8_inline_cache_try_prime(addr); + assert!( + !crate::buffer::test_u8_inline_cache_holds(addr), + "a registered view must not be admitted: cache-hit reads bypass the \ + authoritative backing" + ); +} + /// Death pruning: a dead buffer's admission must not survive the full trace /// that collects it — the recycled address's next tenant is arbitrary memory /// to the emitted reader. Fails if `finalize_collected_dead_buffer` loses its diff --git a/crates/perry-runtime/src/typedarray/access.rs b/crates/perry-runtime/src/typedarray/access.rs index bf882cabfd..9dd609919d 100644 --- a/crates/perry-runtime/src/typedarray/access.rs +++ b/crates/perry-runtime/src/typedarray/access.rs @@ -231,8 +231,8 @@ static KEEP_JS_TYPED_ARRAY_READ_F64: extern "C" fn(*const TypedArrayHeader, i32) /// #9342 — slow arm of the codegen inline `Uint8Array` byte read /// (`perry-codegen/src/expr/u8_buffer_read.rs`). Primes the /// `PERRY_U8_INLINE_CACHE` admission cache when the receiver satisfies its -/// contract (live u8-marked inline-storage `BufferHeader`), then delegates to -/// [`js_uint8array_index_get_value`] for bug-exact element semantics — +/// contract (live u8-marked owning inline-storage `BufferHeader`), then +/// delegates to [`js_uint8array_index_get_value`] for bug-exact element semantics — /// including the #8111 stale-static-hint recovery for rebound receivers. #[no_mangle] pub extern "C" fn js_u8_buffer_read_f64(target: *const TypedArrayHeader, index: i32) -> f64 { diff --git a/crates/perry/tests/issue_9342_u8_inline_read.rs b/crates/perry/tests/issue_9342_u8_inline_read.rs index 4de79f9f7e..832db06675 100644 --- a/crates/perry/tests/issue_9342_u8_inline_read.rs +++ b/crates/perry/tests/issue_9342_u8_inline_read.rs @@ -37,6 +37,8 @@ fn perry_bin() -> PathBuf { /// 16 * 32640 = 522240. /// * OOB read `buf[4096]` is `undefined`; `undefined + 0` shows as NaN via /// `Number()` coercion in the harness — asserted as the string "NaN". +/// * A Uint8Array view over a Uint32Array's materialized buffer observes all +/// four backing bytes after the u32 write → 1 + 2 + 3 + 4 = 10. const SOURCE: &str = r#" const N = 4096; const buf = new Uint8Array(N); @@ -61,10 +63,17 @@ function oobGlobal(): number { return x; } -console.log(viaGlobal() + "," + viaParam(buf) + "," + oobGlobal()); +function viaAliasedView(): number { + const words = new Uint32Array(1); + const bytes = new Uint8Array(words.buffer); + words[0] = 0x01020304; + return bytes[0] + bytes[1] + bytes[2] + bytes[3]; +} + +console.log(viaGlobal() + "," + viaParam(buf) + "," + oobGlobal() + "," + viaAliasedView()); "#; -const EXPECTED: &str = "522240,522240,NaN\n"; +const EXPECTED: &str = "522240,522240,NaN,10\n"; fn compile(dir: &Path, source: &str, extra_env: &[(&str, &str)]) -> (PathBuf, String) { let entry = dir.join("main.ts");