From efa2050349849c116e7b36591a28340853c0abfc Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sat, 19 Sep 2026 13:22:54 +0000 Subject: [PATCH 01/10] perf(codegen,hir): stop re-proving a loop-invariant array receiver on every element access (#10718) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An indexed read on an ordinary `Array` cost 87 instructions per element, against 6 for the identical arithmetic on a `Float64Array` and 16 for node. None of it was a runtime call: callgrind attributes 88.3 instructions per element to straight-line code in `main`, of which **56 are loop-invariant receiver revalidation** re-executed every iteration — the NaN-box tag and handle-band test, the forwarding-flag follow, and a six-load live-head guard that re-reads gc_type, gc_flags, obj_flags, a volatile invalidation flag, length and capacity. perry already has tiers that hoist exactly this proof into the loop preheader and version the loop. They were declining at a single gate in `stmt/loops.rs` — `array_static_type_excluded` — a **declared static type** test sitting in front of a tier that is otherwise fully runtime-guarded. `const a: number[] = new Array(400)` reached it and measured 13.4; plain `new Array(400)` infers `Array` and did not, so ordinary JavaScript never got the tier it already had. Separately, and larger: `a[i] += 1` cost 948 instructions per element, 3.7x the identical `a[i] = a[i] + 1`, and a type annotation did not help. `lower/expr_assign.rs` minted the compound-assignment spill temporaries as `Type::Any`, erasing the receiver's array-ness and the index's integer-ness before codegen could see the statement — which is why no annotation could recover it. The temporaries now carry the operand types. Array read 87.0 -> 13.5 (node 16.3 — perry now wins 1.21x) a[i] += 1 948 -> 273 a[i] += b[i] 1025 -> 347 bare loop 4.0 -> 4.0 unchanged to the instruction Float64Array r/w 6.0/8.0 -> 6.0/8.0 unchanged to the instruction A particle simulation over four numeric arrays spends 60.9% fewer instructions (41.21 G -> 16.12 G) and 59% less peak RSS. The widened tier initially regressed a numeric window inside an otherwise non-numeric array by 33%, because the array-wide layout walk runs per loop entry. A window-scoped layout proof is used as a second chance after that walk declines; both shapes are now 12-13% wins, and arrays that are non-numeric from slot 0 are unchanged to the instruction. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- crates/perry-codegen/src/stmt/loops.rs | 87 +++++++++- .../lower/compound_assign_temp_type_tests.rs | 152 ++++++++++++++++++ crates/perry-hir/src/lower/expr_assign.rs | 26 ++- crates/perry-hir/src/lower/mod.rs | 2 + crates/perry-runtime/src/array/header.rs | 66 ++++++++ crates/perry-runtime/src/array/mod.rs | 7 +- crates/perry-runtime/src/typed_feedback.rs | 9 +- scripts/local_binding_type_allowlist.json | 8 + 8 files changed, 344 insertions(+), 13 deletions(-) create mode 100644 crates/perry-hir/src/lower/compound_assign_temp_type_tests.rs diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index b471d5f88b..6e811d50b9 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -1889,15 +1889,19 @@ fn match_packed_f64_range_loop( return range_loop_reject("store_not_fact_eligible"); } } else if !local_is_number_array(ctx, arr_id) - && !(dense && local_is_untyped_candidate(ctx, arr_id)) + && !local_is_guardable_untyped_array(ctx, arr_id) { - // #6750 follow-up: read-only DENSE accesses also admit bindings - // with no usable static type (`any` function parameters — the - // bcryptjs S-box shape). The entry guards/probes re-validate the - // ACTUAL runtime value, so a wrong hint costs one failed guard → - // slow loop, never correctness. Known non-array static types stay - // excluded so ordinary object/string index loops don't grow dead - // guard chains. + // #6750 follow-up: read-only accesses also admit bindings with no + // usable static ELEMENT type — an `any` parameter (the bcryptjs + // S-box shape) and, since #10718, the ordinary untyped-JavaScript + // `const a = new Array(n)` / `const a = []` binding, whose element + // type erases to `any`. The entry guards/probes re-validate the + // ACTUAL runtime value — plain-array shape, descriptors, prototype + // pollution, frozen/sealed, the whole index window, and raw-f64 + // (hole-tolerant) packedness — so a wrong hint costs one failed + // guard → slow loop, never correctness. Known non-array and + // known-non-numeric static types stay excluded so ordinary + // object/string index loops don't grow dead guard chains. return range_loop_reject("array_static_type_excluded"); } } @@ -5968,6 +5972,73 @@ pub(super) fn local_is_untyped_candidate(ctx: &FnCtx<'_>, local_id: u32) -> bool ) } +/// #10718: a READ-only range-loop receiver whose static type carries no usable +/// element proof, but which is not known to be a non-array or a non-numeric +/// array. +/// +/// Two shapes qualify, and the difference matters: +/// +/// * [`local_is_untyped_candidate`] — no stable type proof at all, or +/// `any`/`unknown`. This is #6750's population (an `any` parameter). +/// * an ARRAY binding whose element type erases to `any`/`unknown`. This is +/// what `const a = new Array(n)`, `const a = []` and every untyped +/// JavaScript array infer, and it was the gate that kept ordinary JS off +/// every hoisted element tier: annotating the identical program +/// `const a: number[] = new Array(n)` cost 13.4 instructions per element +/// against 87 for the same source without the annotation (#10718). +/// +/// Admitting these is a hint, never a claim: `packed_f64_array_loop_range_guard` +/// re-proves plain-array shape, forwarding, index descriptors, `Array.prototype` +/// / `Object.prototype` index pollution, a recorded custom array prototype, +/// frozen / sealed / non-extensible flags, the capacity/length sanity bounds, +/// the whole index window against the LIVE length, and raw-f64-or-holes +/// packedness of every slot, at every loop entry — and the matched body admits +/// no call, closure or await, so nothing can invalidate that between the guard +/// and the last iteration. A receiver that is not what the hint suggested fails +/// the guard and runs the unchanged slow loop. +/// +/// A declared non-numeric array (`string[]`, `Foo[]`) and every known non-array +/// type stay excluded: their guard would be dead weight on every loop entry. +fn local_is_guardable_untyped_array(ctx: &FnCtx<'_>, local_id: u32) -> bool { + local_is_untyped_candidate(ctx, local_id) + || local_array_binding_element_type_is_erased(ctx, local_id) +} + +/// True when the binding's type says "an Array" but says nothing usable about +/// its ELEMENT type. +/// +/// `const a = new Array(n)` records `Generic { base: "Array", type_args: [] }` +/// — an `Array` with no type argument, i.e. `Array`. `[]`, `any[]`, +/// `unknown[]`, `Array` and `Array` land here too. These are the +/// ordinary untyped-JavaScript array bindings; before #10718 every one of them +/// missed the hoisted element tiers, which is why `const a = new Array(400)` +/// cost 87 instructions per element and `const a: number[] = new Array(400)` +/// cost 13.4 on the identical program. +/// +/// This reads `local_type_hint` rather than `stable_local_type_proof` on +/// purpose, and the read is a DISPATCH HINT ONLY: it selects which loops are +/// offered to the range tier, and every offered loop is admitted by +/// `js_typed_feedback_packed_f64_range_loop_guard`, which re-proves the live +/// receiver at loop entry (see [`local_is_guardable_untyped_array`]). A stale +/// or reassigned binding therefore fails the guard and runs the unchanged slow +/// loop; it can never produce a wrong element value. +fn element_type_is_erased(ty: &perry_hir::types::Type) -> bool { + matches!( + ty, + perry_hir::types::Type::Any | perry_hir::types::Type::Unknown + ) +} + +fn local_array_binding_element_type_is_erased(ctx: &FnCtx<'_>, local_id: u32) -> bool { + match ctx.local_type_hint(&local_id) { + Some(perry_hir::types::Type::Array(elem)) => element_type_is_erased(elem.as_ref()), + Some(perry_hir::types::Type::Generic { base, type_args }) if base == "Array" => { + type_args.is_empty() || (type_args.len() == 1 && element_type_is_erased(&type_args[0])) + } + _ => false, + } +} + fn local_allows_packed_f64_loop_store(ctx: &FnCtx<'_>, local_id: u32) -> bool { matches!( local_array_element_type(ctx, local_id), diff --git a/crates/perry-hir/src/lower/compound_assign_temp_type_tests.rs b/crates/perry-hir/src/lower/compound_assign_temp_type_tests.rs new file mode 100644 index 0000000000..934a6734e7 --- /dev/null +++ b/crates/perry-hir/src/lower/compound_assign_temp_type_tests.rs @@ -0,0 +1,152 @@ +//! #10718: the compound-assignment spill must carry the SOURCE binding's type. +//! +//! `a[i] += 1` is desugared by `hoist_compound_member_assign` into two +//! immutable temps so base and key are each evaluated exactly once. Those temps +//! used to be minted with `ty: Type::Any`, which erased BOTH the receiver's +//! array-ness and the index's integer-ness before codegen ever saw the +//! statement — so every element tier declined and the read and the write fell +//! to `js_object_get_index_polymorphic` / `js_object_set_index_polymorphic`, +//! the generic OBJECT property path. Measured on `for (i) a[i] += 1` over an +//! ordinary 400-element array: 948 instructions per element, against 16 for +//! `a[i] = k + i` on the same array, and a `number[]` annotation did not help +//! because the erasure happens here. +//! +//! This is a VERDICT test: the desugar is semantically correct either way and +//! prints the same numbers, so only the recorded type distinguishes +//! "optimizable" from "structurally excluded". Behaviour is covered +//! differentially against node. + +#![cfg(test)] + +use crate::types::Type; +use crate::{Module, Stmt}; +use perry_diagnostics::SourceCache; + +fn lower(src: &str) -> Module { + let src = src.to_string(); + std::thread::Builder::new() + .stack_size(32 * 1024 * 1024) + .spawn(move || { + let mut cache = SourceCache::new(); + let parsed = perry_parser::parse_typescript_with_cache( + &src, + "compound_assign_temp_type.ts", + &mut cache, + ) + .expect("parse should succeed"); + crate::lower_module(&parsed.module, "test", "compound_assign_temp_type.ts") + .expect("lower should succeed") + }) + .expect("spawn lower thread") + .join() + .expect("lower thread panicked") +} + +/// `(name, ty)` of every `Stmt::Let` whose name starts with `__cmpd_`. +fn cmpd_temps(stmts: &[Stmt]) -> Vec<(String, Type)> { + let mut out = Vec::new(); + fn walk(stmts: &[Stmt], out: &mut Vec<(String, Type)>) { + for stmt in stmts { + match stmt { + Stmt::Let { name, ty, .. } if name.starts_with("__cmpd_") => { + out.push((name.clone(), ty.clone())); + } + Stmt::For { init, body, .. } => { + if let Some(init) = init { + walk(std::slice::from_ref(init.as_ref()), out); + } + walk(body, out); + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => walk(body, out), + Stmt::If { + then_branch, + else_branch, + .. + } => { + walk(then_branch, out); + if let Some(else_branch) = else_branch { + walk(else_branch, out); + } + } + _ => {} + } + } + } + walk(stmts, &mut out); + out +} + +fn temp(temps: &[(String, Type)], tag: &str) -> Type { + temps + .iter() + .find(|(name, _)| name.starts_with(&format!("__cmpd_{tag}_"))) + .unwrap_or_else(|| panic!("no __cmpd_{tag}_* temp among {temps:?}")) + .1 + .clone() +} + +#[test] +fn declared_number_array_compound_assign_keeps_both_types() { + let module = lower( + "const a: number[] = new Array(4);\n\ + for (let i = 0; i < 4; i++) a[i] += 1;\n", + ); + let temps = cmpd_temps(&module.init); + assert_eq!( + temp(&temps, "base"), + Type::Array(Box::new(Type::Number)), + "the base temp must carry the receiver's declared array type" + ); + assert_eq!( + temp(&temps, "key"), + Type::Number, + "the key temp must carry the counter's Number type, or every \ + integer-index proof is erased before codegen" + ); +} + +#[test] +fn untyped_new_array_compound_assign_keeps_the_erased_array_type() { + // `new Array(n)` records `Generic { base: "Array", type_args: [] }`. That + // is a WEAKER claim than `number[]` but still says "an Array", which is + // what the range-loop tier's admission keys on. `Any` said nothing. + let module = lower( + "const a = new Array(4);\n\ + for (let i = 0; i < 4; i++) a[i] += 1;\n", + ); + let temps = cmpd_temps(&module.init); + assert!( + matches!(temp(&temps, "base"), Type::Generic { ref base, .. } if base == "Array"), + "the base temp must stay an Array type, got {:?}", + temp(&temps, "base") + ); + assert_eq!(temp(&temps, "key"), Type::Number); +} + +#[test] +fn a_non_local_base_still_spills_as_any() { + // The copy is restricted to a bare `LocalGet` source, where the temp is an + // immutable snapshot of exactly one binding and its type is the source's + // type by construction. A call result has no binding to copy from, so the + // temp keeps `Any` — if this ever becomes a type, the copy has been + // widened past its proof. + let module = lower( + "function f() { return [1, 2, 3]; }\n\ + f()[1] += 1;\n", + ); + let temps = cmpd_temps(&module.init); + assert_eq!(temp(&temps, "base"), Type::Any); +} + +#[test] +fn a_string_array_compound_assign_does_not_gain_a_numeric_type() { + // The copy is exact: a `string[]` receiver must report `string[]`, never a + // numeric array. This is the twin that fails if the copy is ever replaced + // by a guess. + let module = lower( + "const a: string[] = [\"x\"];\n\ + a[0] += \"y\";\n", + ); + let temps = cmpd_temps(&module.init); + assert_eq!(temp(&temps, "base"), Type::Array(Box::new(Type::String))); +} diff --git a/crates/perry-hir/src/lower/expr_assign.rs b/crates/perry-hir/src/lower/expr_assign.rs index 83bda62ad3..1b08d31c8e 100644 --- a/crates/perry-hir/src/lower/expr_assign.rs +++ b/crates/perry-hir/src/lower/expr_assign.rs @@ -1402,11 +1402,35 @@ pub(crate) fn hoist_compound_member_assign( let mut stmts: Vec = Vec::new(); let spill = |ctx: &mut LoweringContext, stmts: &mut Vec, tag: &str, init: Expr| -> LocalId { + // #10718: carry the SOURCE binding's type onto the temp instead of + // erasing it to `Any`. + // + // The temp is an immutable snapshot of exactly one binding's value + // (`init` is a bare `LocalGet`), so its type is the source's type + // by construction — which is why the copy is restricted to that + // one shape. Erasing it cost the whole element-access tier stack: + // `a[i] += 1` on an ordinary array spilled `a` and `i` into two + // `Any` temps, which erased BOTH the receiver's array-ness and the + // index's integer-ness, so the read and the write fell all the way + // to `js_object_get_index_polymorphic` / + // `js_object_set_index_polymorphic` — the generic OBJECT property + // path, with per-element index stringification and shape/descriptor + // table work. Measured: 948 instructions per element for + // `a[i] += 1` against 16 for `a[i] = k + i` on the same array, and + // a `number[]` annotation did not help because this erasure + // happens before codegen ever sees it. + let ty = match &init { + Expr::LocalGet(src) => ctx + .lookup_local_type_by_id(*src) + .cloned() + .unwrap_or(Type::Any), + _ => Type::Any, + }; let id = ctx.fresh_local(); stmts.push(Stmt::Let { id, name: format!("__cmpd_{}_{}", tag, id), - ty: Type::Any, + ty, mutable: false, init: Some(init), }); diff --git a/crates/perry-hir/src/lower/mod.rs b/crates/perry-hir/src/lower/mod.rs index 1831d4b68f..5ed0800561 100644 --- a/crates/perry-hir/src/lower/mod.rs +++ b/crates/perry-hir/src/lower/mod.rs @@ -137,6 +137,8 @@ mod bun_sql_tests; #[cfg(test)] mod collection_view_tests; #[cfg(test)] +mod compound_assign_temp_type_tests; +#[cfg(test)] mod for_multi_decl_tests; #[cfg(test)] mod for_of_counter_tests; diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 4b0187379e..17c3fa7baa 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -1211,6 +1211,72 @@ pub(crate) unsafe fn rebuild_array_numeric_raw_f64_allow_holes(arr: *mut ArrayHe true } +/// #10718: WINDOW-scoped variant of +/// [`rebuild_array_numeric_raw_f64_allow_holes`], for the classic +/// (hole-tolerant, side-exiting) packed-f64 range loop. +/// +/// The array-wide rebuild proves an invariant over `[0, length)` and, on +/// success, records it in the header so later loop entries are O(1). That is +/// exactly right when it succeeds — and needlessly fatal when it does not: a +/// single non-numeric slot ANYWHERE disqualifies a loop that only ever reads +/// `[min_idx, max_idx_exclusive)`, and because the failure clears the layout +/// flags, every re-entry walks the array again. Measured: an untyped +/// 400-element array holding one string at index 399, read by +/// `for (i = 0; i < 399; i++) s += a[i]` inside an outer loop, paid a full +/// 400-slot walk per outer iteration. +/// +/// This checks only the slots the guarded clone can touch. It canonicalizes +/// numeric slots in the window exactly as the array-wide walk does (an +/// INT32-boxed integer becomes raw f64 bits), tolerates `TAG_HOLE` — the +/// classic tier's loads hole-check and side-exit — and fails on the first slot +/// that is neither. +/// +/// It deliberately records NOTHING in the header and does NOT call +/// `layout_init_pointer_free`: slots outside the window are unexamined and may +/// still hold heap pointers, which the collector must keep tracing. The cost +/// is that a window-only admission re-walks its window on each loop entry; +/// that is bounded by the window, and the array-wide fast path above still +/// serves every array that really is numeric throughout. +/// +/// # Safety +/// +/// `arr` must be a live, non-forwarded `GC_TYPE_ARRAY` head whose +/// `length <= capacity`, and `[min_idx, max_idx_exclusive)` must lie within +/// `[0, length)` — `packed_f64_array_loop_range_guard` proves all of that +/// before calling. +pub(crate) unsafe fn array_window_is_numeric_raw_f64_allow_holes( + arr: *mut ArrayHeader, + min_idx: i32, + max_idx_exclusive: i32, +) -> bool { + if arr.is_null() || min_idx < 0 { + return false; + } + let len = i64::from((*arr).length); + let min = i64::from(min_idx); + let max = i64::from(max_idx_exclusive).min(len); + if min >= max { + // An empty window: the clone runs zero iterations, so there is nothing + // to prove. (The caller has already rejected `max_idx_exclusive > len`.) + return true; + } + let elements = array_elements_ptr(arr); + for i in min..max { + let slot_bits = array_slot_bits(arr, i as usize); + if slot_bits == crate::value::TAG_HOLE { + continue; + } + let Some(number) = value_bits_to_number(slot_bits) else { + return false; + }; + if number.to_bits() != slot_bits { + // GC_STORE_AUDIT(POINTER_FREE): raw-f64 rewrite stores numeric payloads only. + std::ptr::write(elements.add(i as usize) as *mut f64, number); + } + } + true +} + /// Dense-window variant of [`rebuild_array_numeric_raw_f64_allow_holes`] for /// the read-only masked-index range loop: after the hole-tolerant rebuild, /// additionally require that `[min_idx, max_idx_exclusive)` contains NO holes. diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index f085ceffce..c44149cf53 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -142,9 +142,10 @@ pub(crate) use self::generic_object::{ object_splice, }; pub(crate) use self::header::{ - array_has_arguments_object_flag, js_array_is_numeric_f64_layout_resolved, - mark_array_as_arguments_object, rebuild_array_numeric_raw_f64_allow_holes, - rebuild_array_numeric_raw_f64_dense_window, rebuild_array_numeric_raw_f64_dense_window_i32, + array_has_arguments_object_flag, array_window_is_numeric_raw_f64_allow_holes, + js_array_is_numeric_f64_layout_resolved, mark_array_as_arguments_object, + rebuild_array_numeric_raw_f64_allow_holes, rebuild_array_numeric_raw_f64_dense_window, + rebuild_array_numeric_raw_f64_dense_window_i32, }; pub use self::header::{ js_array_clear_numeric_layout, js_array_declare_all_pointer_elements, diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index ffb9b526d6..9e2352cfa2 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -1480,7 +1480,14 @@ fn packed_f64_array_loop_range_guard( if min_idx < 0 || i64::from(max_idx_exclusive) > i64::from(len) { return false; } - crate::array::rebuild_array_numeric_raw_f64_allow_holes(arr) + if crate::array::rebuild_array_numeric_raw_f64_allow_holes(arr) { + return true; + } + // #10718: the array-wide invariant can fail on a slot this loop never + // touches. The clone reads only `[min_idx, max_idx_exclusive)`, so a + // window-scoped proof is the whole requirement; it records nothing, so + // the array-wide claim above stays the authority for everyone else. + crate::array::array_window_is_numeric_raw_f64_allow_holes(arr, min_idx, max_idx_exclusive) } } diff --git a/scripts/local_binding_type_allowlist.json b/scripts/local_binding_type_allowlist.json index b2abe964e4..1f2672e0f7 100644 --- a/scripts/local_binding_type_allowlist.json +++ b/scripts/local_binding_type_allowlist.json @@ -401,6 +401,14 @@ "classification": "runtime-validated", "reason": "A falsy-local fold consumes only the private method clone's proof: its public wrapper bit-compares the live argument with TAG_UNDEFINED, candidate discovery rejects user writes and closure capture, and the proof API rejects every remaining reassigned binding." }, + { + "path": "crates/perry-codegen/src/stmt/loops.rs", + "function": "local_array_binding_element_type_is_erased", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "#10718: the declared/inferred type only selects which read-only range loops are OFFERED to the packed-f64 range tier; it licenses no load. Every offered loop enters through js_typed_feedback_packed_f64_range_loop_guard, which re-proves the live receiver at loop entry - plain-array GC type, no forwarding, no index descriptors, no Array.prototype/Object.prototype index pollution, no recorded custom array prototype, not frozen/sealed/non-extensible, capacity and length sanity, the whole index window inside the LIVE length, and raw-f64-or-holes packedness of every slot - and the matched body admits no call, closure or await, so nothing can invalidate that between the guard and the last iteration. A stale or reassigned binding fails the guard and runs the unchanged slow loop." + }, { "path": "crates/perry-codegen/src/stmt/loops.rs", "function": "local_array_element_type", From 1246fd9df48844fbbea7388b9c43ebd5b896b194 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sat, 19 Sep 2026 13:23:18 +0000 Subject: [PATCH 02/10] changelog: fragment for #10718 Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10718-array-index-hoist.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog.d/10718-array-index-hoist.md diff --git a/changelog.d/10718-array-index-hoist.md b/changelog.d/10718-array-index-hoist.md new file mode 100644 index 0000000000..cfcf2b480f --- /dev/null +++ b/changelog.d/10718-array-index-hoist.md @@ -0,0 +1,9 @@ +**Indexed reads on an ordinary `Array` no longer re-prove a loop-invariant receiver on every element.** + +An indexed read cost **87 instructions per element** — against 6 for the same arithmetic on a `Float64Array` and 16 for node — and none of it was a runtime call. 56 of the 87 were loop-invariant receiver revalidation re-executed every iteration: the NaN-box tag and handle-band test, the forwarding-flag follow, and a six-load live-head guard. + +perry already had tiers that hoist that proof into the loop preheader. They were declining at one gate, `array_static_type_excluded` — a *declared static type* test in front of a tier that is otherwise fully runtime-guarded — so `const a: number[]` got it and plain `new Array(400)`, which infers `Array`, did not. Ordinary JavaScript never reached the tier it already had. + +Separately, `a[i] += 1` cost **948** instructions per element, 3.7× the identical `a[i] = a[i] + 1`, and no annotation helped: the compound-assignment spill temporaries were minted as `Type::Any`, erasing the receiver's array-ness and the index's integer-ness before codegen saw the statement. + +Array read **87 → 13.5** (node 16.3), `a[i] += 1` **948 → 273**, `a[i] += b[i]` **1025 → 347**. A particle simulation over four numeric arrays spends **60.9% fewer instructions** and **59% less peak RSS**. The bare loop and both `Float64Array` paths are unchanged to the instruction. From 2b2b8906390c42a3e4e0305214aa573893753f23 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sat, 19 Sep 2026 16:36:25 +0000 Subject: [PATCH 03/10] perf(codegen): hoist the loop-invariant receiver proof for array element stores (#10718) An indexed write cost 105 instructions per element with zero runtime calls; 51 of them were loop-invariant receiver revalidation and 42 was a write-barrier decision provable away from the value type. This widens the store admission the way #10731 widened reads. a[i] = k + i 105 -> 17.4, a[i] = a[i] + 1 256 -> 24.5, a[i] = a[i] + b[i] 333 -> 35.9. The bare loop, both Float64Array paths and the indexed read are unchanged to the instruction. The barrier-stem census probe for idxset.recv_global gains a second statement: the widened tier made its loop qualify, which would have left that stem with no live witness. A future multi-statement store tier must re-shape the probe, not delete it. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10718-array-store-hoist.md | 9 +++ .../src/expr/barrier_stem_census_tests.rs | 41 ++++++++++-- crates/perry-codegen/src/stmt/loops.rs | 67 ++++++++++++++++++- 3 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 changelog.d/10718-array-store-hoist.md diff --git a/changelog.d/10718-array-store-hoist.md b/changelog.d/10718-array-store-hoist.md new file mode 100644 index 0000000000..648ff78110 --- /dev/null +++ b/changelog.d/10718-array-store-hoist.md @@ -0,0 +1,9 @@ +**Stores to an ordinary `Array` element no longer re-prove a loop-invariant receiver on every element.** + +An indexed write cost **105 instructions per element** — against 8 for the same store to a `Float64Array` and 12 for node — with zero runtime calls. **51 of the 105 were loop-invariant** receiver revalidation, and a further 42 was a write-barrier decision provable away from the value's type. + +This widens the store admission the way #10731 widened reads. The gate was `has_materialization_hazard`, which a trailing `console.log` is enough to set. + +`a[i] = k + i` **105 → 17.4**, `a[i] = a[i] + 1` **256 → 24.5** (node 18.7), `a[i] = a[i] + b[i]` **333 → 35.9**. The bare loop, both `Float64Array` paths and the indexed read are unchanged to the instruction. + +Note this moves none of the five real programs in #10695 — their loop bodies are multi-statement or contain calls, which no current tier admits (#10741) — and `a[i] += 1` is unaffected because its lowering is two statements (#10743). diff --git a/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs b/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs index 09b5817ffa..e4f3c17141 100644 --- a/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs +++ b/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs @@ -507,11 +507,42 @@ fn idxset_recv_global_ir() -> String { op: UpdateOp::Increment, prefix: false, }), - body: vec![Stmt::Expr(Expr::IndexSet { - object: Box::new(Expr::LocalGet(G_ID)), - index: Box::new(Expr::LocalGet(IDX_ID)), - value: Box::new(Expr::LocalGet(VAL_ID)), - })], + // #10718 store side: the body carries a SECOND statement, and + // that is load-bearing for this probe rather than incidental. + // + // Widening the packed-f64 range loop's STORE admission to + // element-type-erased array bindings (`Array` — which is + // exactly `g`'s type here) made this loop qualify for the + // versioned tier. The tier is correct on it — the fast copy + // stores only values its per-store check proved are genuine + // doubles, and everything else side-exits into a slow copy that + // keeps the full barriered store (`idxset.inbounds.barrier` -> + // `js_write_barrier_slot_validated_parent`, plus + // `js_write_barrier_slot` on both extend paths and the numeric + // note) — but the slow copy reaches the store through the + // `idxset.inbounds` receiver arm, not through `recv_global`. + // The stem would then have had NO live witness anywhere, which + // is the one thing this census exists to prevent. + // + // `packed_f64_range_loop_body_collect` admits exactly ONE + // statement, so a second one keeps this probe on the + // un-versioned receiver ladder it is here to cover, without + // touching what it asserts. If a future tier learns to admit + // multi-statement store bodies, this probe goes red again — + // deliberately — and must be re-shaped, not deleted. + body: vec![ + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(G_ID)), + index: Box::new(Expr::LocalGet(IDX_ID)), + value: Box::new(Expr::LocalGet(VAL_ID)), + }), + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::LocalGet(VAL_ID)), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + }), + ], }, Stmt::Return(Some(Expr::LocalGet(G_ID))), ], diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 6e811d50b9..f9f6514a9b 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -1823,7 +1823,9 @@ fn match_packed_f64_range_loop( { return range_loop_reject("dense_written_not_addressable"); } - } else if !packed_loop_array_binding_is_eligible(ctx, arr_id) { + } else if !packed_loop_array_binding_is_eligible(ctx, arr_id) + && !written_untyped_binding_is_guardable(ctx, arr_id) + { return range_loop_reject("written_binding_not_eligible"); } } else if !packed_loop_array_binding_storage_is_addressable(ctx, arr_id) @@ -1878,13 +1880,21 @@ fn match_packed_f64_range_loop( // every loop entry, so those static facts are not load-bearing // here; a wrong hint is one failed guard -> slow loop. Classic // (side-exiting, hole-tolerant) written arrays keep the full set. - if !local_allows_packed_f64_loop_store(ctx, arr_id) { + // #10718 store side: the two remaining tests below are DECLARED + // STATIC TYPE / static fact-graph tests standing in front of a + // tier whose every correctness obligation is discharged at + // runtime. `written_untyped_binding_is_guardable` admits the + // ordinary untyped-JavaScript array binding alongside them — see + // that function for why the guard, not the hint, is what holds. + let untyped_guardable = written_untyped_binding_is_guardable(ctx, arr_id); + if !local_allows_packed_f64_loop_store(ctx, arr_id) && !untyped_guardable { return range_loop_reject("store_local_not_allowed"); } if !dense && !ctx .native_facts .packed_f64_eligible_for_guarded_store(arr_id) + && !untyped_guardable { return range_loop_reject("store_not_fact_eligible"); } @@ -6039,6 +6049,59 @@ fn local_array_binding_element_type_is_erased(ctx: &FnCtx<'_>, local_id: u32) -> } } +/// #10718 store side: may a WRITTEN range-loop receiver be admitted on the +/// strength of the loop-entry guard alone, with no declared element type and +/// no static fact-graph claim? +/// +/// #10731 widened the READ admission to ordinary untyped JavaScript arrays and +/// took `Array` element reads from 87 instructions to 13.5. It deliberately +/// left the STORE side alone, because a raw slot store on an unproven element +/// type pulls in frozen/sealed, the write barrier and the pointer-free layout +/// note. Every one of those is discharged, and none of them by a static hint: +/// +/// * **frozen / sealed / non-extensible** — `packed_f64_array_loop_range_guard` +/// reads `OBJ_FLAG_FROZEN | OBJ_FLAG_SEALED | OBJ_FLAG_NO_EXTEND` off the GC +/// header at every loop entry and declines. A store that must be ignored in +/// sloppy mode or throw in strict mode therefore never reaches the fast copy +/// at all; it runs the unchanged generic store in the slow loop. +/// * **index accessors / `defineProperty` descriptors** — the same guard +/// declines on `OBJ_FLAG_ARRAY_DESCRIPTORS`. +/// * **a setter on `Array.prototype` / `Object.prototype`, or a recorded custom +/// array prototype** — declined by the three prototype-pollution flags in +/// `plain_array_index_guard`. This is what makes a store INTO A HOLE safe: +/// with no inherited index property, defining the element on an in-bounds +/// index is exactly what `[[Set]]` does. +/// * **the write barrier** — the fast store writes a value the per-store check +/// proved is a genuine double. A double carries no heap edge, so there is no +/// edge for the barrier to record. A NaN-boxed non-double side-exits to the +/// slow loop BEFORE the store, where the generic path runs barrier and all. +/// * **the pointer-free layout note** — the guard proved every slot in the +/// window is raw f64 or `TAG_HOLE`, and the fast store only ever writes a +/// double, so the array's numeric layout is an invariant of the fast copy +/// rather than something a note must maintain. +/// * **growing through the store, and out-of-bounds** — the guard proves the +/// loop's whole static index window against the LIVE `length`, so no admitted +/// index is `>= length` and the fast copy can never need to extend. +/// * **a proxied / subclassed / `arguments`-like receiver** — `GC_TYPE_ARRAY` +/// plus the forwarding-flag test in `plain_array_index_guard`; a Proxy is not +/// a `GC_TYPE_ARRAY` head. +/// * **anything changing mid-loop** — the matched body admits no call, closure +/// or await, so no user code can run between the guard and the last +/// iteration to freeze, seal, `defineProperty` or pollute a prototype. +/// +/// So the binding test that remains is a DISPATCH HINT: which loops are worth +/// offering the guard. Being wrong costs one failed guard and the unchanged +/// slow loop, never a wrong store. What it must still enforce is the two +/// STORAGE facts the guard cannot see — the binding is read by a plain load +/// (not a closure cell or box) and has not been scalar-replaced — because +/// those decide whether the emitted code is looking at the array the guard +/// validated. +fn written_untyped_binding_is_guardable(ctx: &FnCtx<'_>, local_id: u32) -> bool { + local_is_guardable_untyped_array(ctx, local_id) + && packed_loop_array_binding_storage_is_addressable(ctx, local_id) + && !ctx.scalar_replaced_arrays.contains_key(&local_id) +} + fn local_allows_packed_f64_loop_store(ctx: &FnCtx<'_>, local_id: u32) -> bool { matches!( local_array_element_type(ctx, local_id), From 4c0a29b838c56c3b69d99fdce07e2a1345884b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 19:15:41 +0000 Subject: [PATCH 04/10] perf(codegen): let `a[i] += 1` reach the loop tier `a[i] = a[i] + 1` already had (#10743) Fixes #10743. Stacked on #10746 (`2b2b89063`), which contains #10731. `a[i] += 1` and `a[i] = a[i] + 1` are the same operation and node compiles both to the same cost. perry compiled them 11x apart -- 277 instructions per element against 24 -- and the slow one was the idiomatic spelling. HIR's `hoist_compound_member_assign` lowers a compound member assignment into two immutable alias `Let`s plus the store, so the base and the key are each evaluated exactly once and before the right-hand side. `packed_f64_range_loop_body_collect` admits exactly ONE statement, so the lowering guaranteed the statement could never reach the tier that makes the expanded form fast. Annotating the array changed nothing: the obstacle is the statement count, not type information. The temporaries stay. They are load-bearing -- an RHS call can reassign the bindings they were read from, and the store must still land at the index evaluated before it ran (`a[i] += (() => { i = 2; return 5; })()`). So the fold is in the MATCHER, and it applies only to the guarded fast clones: the slow clone lowers the statements as written, so a failed guard and every side exit still execute the specified evaluation order. Inside the matched subset the fold is exact, because `packed_f64_range_loop_pure_expr_collect` is a whitelist that admits no call, closure, `await`, update or assignment anywhere in the statement -- nothing can write the locals the aliases read. This needs none of #10741's mid-iteration side-exit discipline: the folded-away statements perform no stores, so there is nothing to un-do when a guard fails partway. Per element, fitted across N=10,000 -> 50,000, three interleaved rounds per arm: a[i] += 1 277.05 -> 25.46 (node 16.9) a[i] -= 1 208.05 -> 27.45 (node 16.9) a[i] += b[i] 347.05 -> 35.86 (node 25.8) -- now exactly a[i] = a[i] + b[i] a[i] *= 1 206.05 -> 25.45 (node 15.1) a[i] |= 0 236.05 -> 52.46 (node 12.7) The bare loop, both `Float64Array` rows, the indexed read and write and both expanded spellings are unchanged -- their emitted LLVM IR is byte-identical between arms, which is a stronger witness than a flat fitted number. It moves none of the five real programs in #10695, and the compiler's own trace says why: `sim` still reports 3 x `body_not_admissible`, because its inner loop is five statements containing two `if`s. This admits a body whose extra statements are compound-assign ALIASES, not a body whose extra statements do things. That remains #10741. New diagnostic: `PERRY_PACKED_LOOP_TRACE=1` prints `[range-loop] admitted: compound_assign_alias_fold`. The existing traces report only DECLINES, so there was no way to show that a fixture claiming to exercise a guarded path actually reached it -- the exact gap that let #10746 ship a GC stress whose fixtures were all declined before the guard under test ran. Correctness: 25 differential fixtures (evaluation order with a side-effecting RHS, getters and setters on the array / on `Array.prototype` / on the index, a prototype getter that truncates, deletes from or freezes the array mid-loop, frozen and sealed in sloppy and strict mode, a non-writable index, holes, out of bounds, past `length`, non-number elements, string `+=`, heap-reference stores, offset and affine indices, module-global receivers, typed arrays, and all fifteen compound operators) pass on both arms with byte-identical results. Guard sabotage: removing the per-store numeric-bits value check produces 27 SIGABRTs across seven heap-limit seeds under `PERRY_GC_FROMSPACE_SCAN_ABORT=1` (0 unsabotaged), with the from-space scan naming a survivor-space array holding an un-evacuated nursery pointer through a slot that was never marked dirty -- so this path does not bypass #10746's write barrier, it is the same check on the same store. Neutering the loop-entry guard turns four fixtures red, all of them through the fold. The `mutable: false` condition is witnessed by a unit test and an IR test; the `__cmpd_` name test and the initialiser grammar are scoping restrictions with unit-test witnesses only, and lowering the folded body in the slow clone as well turns nothing red -- all three are stated as unwitnessed in the report rather than claimed. Gates: node identity 92 identical / 1 diff / 0 compile-fail on both arms with byte-identical results files (the diff is the pre-existing #10733 `nest` defect); `perry-codegen` lib 1650 pass including the barrier stem census and its four sabotage twins, with NO census probe change needed; `perry-hir` green; clippy 470 = 470 with identical per-category counts; `cargo fmt`, `check_file_size.sh`, `gc_runtime_root_holders.py` and `local_binding_type_audit.py` clean and identical on both arms; peak RSS worst case +0.60%. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- .../10743-compound-assign-alias-fold.md | 11 + .../src/stmt/compound_alias_fold_tests.rs | 262 ++++++++++++++++++ crates/perry-codegen/src/stmt/loops.rs | 235 +++++++++++++++- crates/perry-codegen/src/stmt/mod.rs | 2 + .../tests/native_proof_regressions.rs | 177 ++++++++++++ .../perry/src/commands/compile/build_cache.rs | 3 + 6 files changed, 678 insertions(+), 12 deletions(-) create mode 100644 changelog.d/10743-compound-assign-alias-fold.md create mode 100644 crates/perry-codegen/src/stmt/compound_alias_fold_tests.rs diff --git a/changelog.d/10743-compound-assign-alias-fold.md b/changelog.d/10743-compound-assign-alias-fold.md new file mode 100644 index 0000000000..7dea1f4e14 --- /dev/null +++ b/changelog.d/10743-compound-assign-alias-fold.md @@ -0,0 +1,11 @@ +**`a[i] += 1` reaches the same loop tier as `a[i] = a[i] + 1`.** + +The two spellings are the same operation and node compiles both to the same cost. perry compiled them **11× apart** — 277 instructions per element against 24 — and the slow one was the idiomatic spelling. + +HIR lowers a compound member assignment into two immutable alias `Let`s plus the store, so the base and the key are each evaluated exactly once and before the right-hand side. The classic range-loop matcher admits exactly ONE statement, so the lowering guaranteed the statement could never reach the tier. Annotating the array changed nothing: the obstacle is the statement count, not type information. + +The temporaries stay. They are load-bearing — an RHS call can reassign the bindings they were read from, and the store must still land at the index evaluated before it ran. Instead the matcher folds them, and only for the guarded fast clones: the slow clone lowers the statements as written, so a failed guard and every side exit still execute the specified evaluation order. Inside the matched subset the fold is exact, because the body walk is a whitelist that admits no call, closure, `await`, update or assignment anywhere in the statement — nothing can write the locals the aliases read. + +`a[i] += 1` **277 → 25.5**, `a[i] -= 1` **208 → 27.5**, `a[i] += b[i]` **347 → 35.9** (identical to `a[i] = a[i] + b[i]`), `a[i] *= 1` **206 → 25.5**, `a[i] |= 0` **236 → 52.5**. The bare loop, both `Float64Array` paths, the indexed read and write, and both expanded spellings are unchanged — their emitted LLVM IR is byte-identical. + +This needs none of #10741's mid-iteration side-exit discipline: the folded-away statements perform no stores, so there is nothing to un-do when a guard fails partway. It also moves none of the five real programs in #10695 — their loop bodies are still multi-statement or contain calls, which no current tier admits. diff --git a/crates/perry-codegen/src/stmt/compound_alias_fold_tests.rs b/crates/perry-codegen/src/stmt/compound_alias_fold_tests.rs new file mode 100644 index 0000000000..27fb963079 --- /dev/null +++ b/crates/perry-codegen/src/stmt/compound_alias_fold_tests.rs @@ -0,0 +1,262 @@ +//! #10743: the compound-assignment alias fold, and the shapes it declines. +//! +//! `a[i] += 1` is lowered by HIR's `hoist_compound_member_assign` into two +//! immutable alias `Let`s plus the store, so the base and the key are each +//! evaluated exactly once and before the right-hand side. The classic +//! range-loop matcher admits exactly ONE statement, so the idiomatic spelling +//! could never reach the tier that makes the expanded `a[i] = a[i] + 1` fast: +//! measured 277 instructions per element against 24 for the expanded form on +//! the same array, and annotating the array changed nothing, because the +//! obstacle is the statement count rather than type information. +//! +//! The canonical body below is transcribed from a `--print-hir` dump of +//! `for (let i = 0; i < 400; i++) a[i] += 1;`, not guessed: +//! +//! ```text +//! Let { id: 5, name: "__cmpd_base_5", mutable: false, init: Some(LocalGet(1)) } +//! Let { id: 6, name: "__cmpd_key_6", mutable: false, init: Some(LocalGet(4)) } +//! Expr(IndexSet { object: LocalGet(5), index: LocalGet(6), +//! value: Binary { Add, IndexGet { LocalGet(5), LocalGet(6) }, +//! Integer(1) } }) +//! ``` +//! +//! Every `declines_*` test here is a guard's witness: it is the test that goes +//! red when that condition is deleted from the fold. + +#![cfg(test)] + +use perry_hir::types::Type; +use perry_hir::{BinaryOp, Expr, Stmt}; + +use super::loops::packed_f64_range_loop_compound_alias_fold; + +const ARRAY: u32 = 1; +const COUNTER: u32 = 4; +const BASE_TEMP: u32 = 5; +const KEY_TEMP: u32 = 6; + +fn temp(id: u32, name: &str, mutable: bool, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Number, + mutable, + init: Some(init), + } +} + +/// `__cmpd_base_5[__cmpd_key_6] = __cmpd_base_5[__cmpd_key_6] + 1` +fn alias_store() -> Stmt { + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(BASE_TEMP)), + index: Box::new(Expr::LocalGet(KEY_TEMP)), + value: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(BASE_TEMP)), + index: Box::new(Expr::LocalGet(KEY_TEMP)), + }), + right: Box::new(Expr::Integer(1)), + }), + }) +} + +/// What the store must fold to: `a[i] = a[i] + 1`, the shape the tier already +/// admits and already beats node on. +fn expanded_store() -> Stmt { + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(ARRAY)), + index: Box::new(Expr::LocalGet(COUNTER)), + value: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARRAY)), + index: Box::new(Expr::LocalGet(COUNTER)), + }), + right: Box::new(Expr::Integer(1)), + }), + }) +} + +fn canonical_body() -> Vec { + vec![ + temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), + temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), + alias_store(), + ] +} + +fn debug(stmts: &[Stmt]) -> String { + format!("{stmts:?}") +} + +#[test] +fn folds_the_canonical_compound_assignment_to_the_expanded_store() { + let folded = + packed_f64_range_loop_compound_alias_fold(&canonical_body()).expect("shape must fold"); + assert_eq!( + debug(&folded), + debug(std::slice::from_ref(&expanded_store())), + "the fold must produce exactly the expanded spelling" + ); +} + +#[test] +fn folds_an_arithmetic_key_initialiser() { + // `a[i * 2 + 1] += 1` spills the whole index expression into the key temp. + let key = Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(Expr::LocalGet(COUNTER)), + right: Box::new(Expr::Integer(2)), + }), + right: Box::new(Expr::Integer(1)), + }; + let body = vec![ + temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), + temp(KEY_TEMP, "__cmpd_key_6", false, key.clone()), + alias_store(), + ]; + let folded = packed_f64_range_loop_compound_alias_fold(&body).expect("shape must fold"); + let text = debug(&folded); + assert!( + !text.contains("LocalGet(5)") && !text.contains("LocalGet(6)"), + "no alias id may survive the fold: {text}" + ); + assert!( + text.contains("Mul"), + "the key tree must be substituted: {text}" + ); +} + +#[test] +fn declines_a_mutable_alias() { + // Guard: `mutable: false`. A writable binding is not an alias -- nothing + // here proves its value at the store is the value it was bound to. + let mut body = canonical_body(); + if let Stmt::Let { mutable, .. } = &mut body[0] { + *mutable = true; + } + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_a_user_named_binding() { + // Guard: the `__cmpd_` name. The fold's argument rests on these temps + // being the compiler's own compound-assign spills, read only by the one + // statement they were minted for. A user `const` in the loop body belongs + // to the general multi-statement tier (#10741), not here. + let mut body = canonical_body(); + if let Stmt::Let { name, .. } = &mut body[0] { + *name = "userConst".to_string(); + } + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_an_initialiser_outside_the_stable_grammar() { + // Guard: `packed_f64_range_loop_alias_init_is_stable`. An element read is + // not re-evaluation-safe the way a local read is -- the folded statement + // evaluates the key tree twice. + let mut body = canonical_body(); + if let Stmt::Let { init, .. } = &mut body[1] { + *init = Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARRAY)), + index: Box::new(Expr::LocalGet(COUNTER)), + }); + } + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_a_body_longer_than_two_aliases_and_a_store() { + let mut body = canonical_body(); + body.insert(0, temp(7, "__cmpd_base_7", false, Expr::LocalGet(ARRAY))); + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_a_body_with_no_aliases() { + // A single statement is already the shape the tier takes; the fold must + // not claim it, or it would clear and rebuild an access map for nothing. + assert!( + packed_f64_range_loop_compound_alias_fold(std::slice::from_ref(&expanded_store())) + .is_none() + ); +} + +#[test] +fn declines_a_repeated_alias_id() { + // Two bindings for one id would make the substitution order-dependent. + let body = vec![ + temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), + temp(BASE_TEMP, "__cmpd_key_5", false, Expr::LocalGet(COUNTER)), + alias_store(), + ]; + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_when_the_last_statement_is_not_an_expression() { + let body = vec![ + temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), + temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), + Stmt::Return(Some(Expr::LocalGet(BASE_TEMP))), + ]; + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn declines_an_alias_without_an_initialiser() { + let body = vec![ + Stmt::Let { + id: BASE_TEMP, + name: "__cmpd_base_5".to_string(), + ty: Type::Number, + mutable: false, + init: None, + }, + temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), + alias_store(), + ]; + assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); +} + +#[test] +fn the_logical_assignment_shape_folds_but_stays_unversionable() { + // `a[i] ||= 3` spills the same two aliases but ends in `Expr::Logical`, + // whose right operand is the store. The fold is shape-agnostic, so it + // rewrites the statement -- and the classic body walk then declines it, + // because `packed_f64_range_loop_pure_expr_collect` has no `IndexSet` arm. + // This test pins the second half of that sentence: if a future widening + // admits `Logical`, the short-circuit semantics have to be re-argued. + let body = vec![ + temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), + temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), + Stmt::Expr(Expr::Logical { + op: perry_hir::LogicalOp::Or, + left: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(BASE_TEMP)), + index: Box::new(Expr::LocalGet(KEY_TEMP)), + }), + right: Box::new(Expr::IndexSet { + object: Box::new(Expr::LocalGet(BASE_TEMP)), + index: Box::new(Expr::LocalGet(KEY_TEMP)), + value: Box::new(Expr::Integer(3)), + }), + }), + ]; + let folded = packed_f64_range_loop_compound_alias_fold(&body).expect("shape folds"); + let mut accesses = std::collections::BTreeMap::new(); + assert!( + !super::loops::packed_f64_range_loop_body_collect( + &folded, + COUNTER, + None, + &mut accesses, + None, + ), + "a logical compound assignment must not be admitted by the classic walk" + ); +} diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index f9f6514a9b..96f5fbf8c7 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -1581,6 +1581,13 @@ struct PackedF64RangeLoop { /// mid-iteration side exit could double-apply earlier statement effects /// on re-execution). dense: bool, + /// #10743: the body the GUARDED CLONES are lowered from, when it is not + /// the body as written. Set only by the compound-assignment alias fold + /// (`packed_f64_range_loop_compound_alias_fold`); `None` means both + /// clones lower the same statements. The slow clone always lowers the + /// original body, so a failed guard executes the specified evaluation + /// order. + fast_body: Option>, } /// #6011: range-preguarded packed-f64 versioned loop. @@ -1615,6 +1622,20 @@ fn range_loop_reject(reason: &'static str) -> Option { } None } +/// Positive twin of [`range_loop_reject`]: the decline traces say which gate +/// said no, and nothing said yes. A fixture that claims to exercise a guarded +/// fast path needs to be able to show it REACHED it — #10746 shipped a GC +/// stress whose fixtures were all declined at `body_not_admissible` before the +/// guard under test ever ran, and the traces available at the time could not +/// have revealed that. +fn range_loop_trace(what: &str) { + use std::sync::OnceLock; + static ON: OnceLock = OnceLock::new(); + if *ON.get_or_init(|| std::env::var("PERRY_PACKED_LOOP_TRACE").as_deref() == Ok("1")) { + eprintln!("[range-loop] admitted: {what}"); + } +} + fn match_packed_f64_range_loop( ctx: &FnCtx<'_>, init: Option<&Stmt>, @@ -1729,6 +1750,7 @@ fn match_packed_f64_range_loop( && !ctx.boxed_vars.contains(&id) && !ctx.closure_captures.contains_key(&id) }; + let mut fast_body: Option> = None; let dense = if packed_f64_range_loop_body_collect( body, counter_id, @@ -1737,6 +1759,22 @@ fn match_packed_f64_range_loop( Some(&affine_leaf_ok), ) { false + } else if let Some(folded) = packed_f64_range_loop_compound_alias_fold(body).filter(|folded| { + // #10743: `a[i] += 1` is three statements only because of its + // spec-mandated alias temporaries. Retry the CLASSIC walk on the + // folded statement; everything else about the tier is unchanged. + accesses.clear(); + packed_f64_range_loop_body_collect( + folded, + counter_id, + bound_local, + &mut accesses, + Some(&affine_leaf_ok), + ) + }) { + range_loop_trace("compound_assign_alias_fold"); + fast_body = Some(folded); + false } else { // The classic shape (one statement, counter-offset indices, stores // allowed, hole-tolerant with side exits) didn't match. Try the @@ -1921,9 +1959,173 @@ fn match_packed_f64_range_loop( bound, arrays: accesses.into_values().collect(), dense, + fast_body, }) } +/// #10743: fold a compound member assignment's alias `Let`s into its store. +/// +/// HIR's `hoist_compound_member_assign` lowers `a[i] += 1` into two immutable +/// alias bindings followed by the store: +/// +/// ```text +/// Let __cmpd_base_7 = LocalGet(a) +/// Let __cmpd_key_8 = LocalGet(i) +/// Expr(IndexSet { object: LocalGet(7), index: LocalGet(8), +/// value: Binary { Add, IndexGet { LocalGet(7), LocalGet(8) }, 1 } }) +/// ``` +/// +/// Those temporaries are LOAD-BEARING in HIR and must not be removed there. +/// The specification evaluates the base and the key exactly once and BEFORE +/// the right-hand side, and an RHS call can reassign the very bindings they +/// were read from — `a[i] += (() => { i = 1; return 5; })()` must store at the +/// index `i` held before the arrow ran. Deleting the spill in the lowering is +/// a spec violation, not an optimisation, which is why this lives in the +/// matcher instead. +/// +/// Inside the matched subset the aliases are provably redundant. +/// [`packed_f64_range_loop_pure_expr_collect`] is a WHITELIST walk: it admits +/// no call, no closure, no `await`, no `Update` and no assignment anywhere in +/// the statement, and [`packed_f64_range_loop_store_collect`] routes every +/// part of the store through it. So nothing between the alias binding and the +/// store can write the locals the aliases read, and the folded statement has +/// exactly the semantics of the three it replaces. +/// +/// The fold is used ONLY for the guarded fast clones. The slow clone keeps +/// the statements as written, so a failed guard — and every side exit — still +/// executes the specified evaluation order. +/// +/// Why it is worth a pass of its own: `a[i] += 1` measured 277 instructions +/// per element against 24 for the byte-identical `a[i] = a[i] + 1`, purely +/// because the lowering hands the matcher three statements and the matcher +/// takes one. Unlike the general multi-statement tier (#10741) this needs no +/// mid-iteration side-exit discipline, because the folded-away statements +/// perform no stores: there is nothing to un-do when a guard fails partway. +pub(super) fn packed_f64_range_loop_compound_alias_fold(body: &[Stmt]) -> Option> { + use perry_hir::Expr; + // A computed key spills two temps, a static property (`o.f += 1`) one. + // Anything longer is not this shape. + if body.len() < 2 || body.len() > 3 { + return None; + } + let (aliases, last) = body.split_at(body.len() - 1); + let mut map: std::collections::HashMap = std::collections::HashMap::new(); + for stmt in aliases { + let Stmt::Let { + id, + name, + mutable: false, + init: Some(init), + .. + } = stmt + else { + return None; + }; + // The compiler's own compound-assign spills only. A user `const` in + // the loop body is the general multi-statement tier's problem, not + // this one, and admitting it here would widen the claim above beyond + // what the name guarantees (these temps are read only by the one + // statement they were minted for). + if !name.starts_with("__cmpd_") { + return None; + } + if !packed_f64_range_loop_alias_init_is_stable(init) { + return None; + } + // Fold through the aliases bound earlier, so the replacement stored + // here is itself alias-free; a repeated id would make that untrue. + let mut init = init.clone(); + packed_f64_range_loop_substitute_locals(&mut init, &map); + if map.insert(*id, init).is_some() { + return None; + } + } + if map.is_empty() { + return None; + } + let [Stmt::Expr(expr)] = last else { + return None; + }; + let mut folded = expr.clone(); + packed_f64_range_loop_substitute_locals(&mut folded, &map); + // Defence in depth rather than a witnessed guard, and labelled as such: + // a WRITE to an alias (`LocalSet`/`Update`) carries its target in a field + // the substitution above does not visit, so a surviving mention would + // mean the fold was partial. No shape reaching here from source can + // produce one — `pure_expr_collect` rejects both nodes outright — so this + // cannot be made to fail by a fixture, and it is a structural assertion, + // not a check the differential suite witnesses. + if map + .keys() + .any(|id| packed_f64_range_loop_expr_touches_local(&folded, *id)) + { + return None; + } + Some(vec![Stmt::Expr(folded)]) +} + +/// The grammar an alias initialiser may take for +/// [`packed_f64_range_loop_compound_alias_fold`]: local reads, numeric +/// literals, and `+`/`-`/`*` over them. +/// +/// Two properties are needed and both follow from the grammar. The tree is +/// side-effect-free, so the folded statement may evaluate a key twice — once +/// for the read index, once for the store index — where the original +/// evaluated it once. And its value cannot change between those two +/// evaluations, because the admitted statement writes no local at all. +fn packed_f64_range_loop_alias_init_is_stable(init: &perry_hir::Expr) -> bool { + use perry_hir::{BinaryOp, Expr}; + match init { + Expr::LocalGet(_) | Expr::Integer(_) | Expr::Number(_) => true, + Expr::Binary { op, left, right } => { + matches!(op, BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul) + && packed_f64_range_loop_alias_init_is_stable(left) + && packed_f64_range_loop_alias_init_is_stable(right) + } + _ => false, + } +} + +/// Replace every `LocalGet(id)` in `expr` with `map[id]`. Replacements are +/// alias-free by construction, so a substituted subtree is not re-walked. +fn packed_f64_range_loop_substitute_locals( + expr: &mut perry_hir::Expr, + map: &std::collections::HashMap, +) { + use perry_hir::Expr; + if let Expr::LocalGet(id) = expr { + if let Some(replacement) = map.get(id) { + *expr = replacement.clone(); + return; + } + } + perry_hir::walker::walk_expr_children_mut(expr, &mut |child| { + packed_f64_range_loop_substitute_locals(child, map); + }); +} + +/// Does `expr` mention `id` in ANY position — read, write, or update? +/// Wider than [`expr_mentions_local`], which only sees reads. +fn packed_f64_range_loop_expr_touches_local(expr: &perry_hir::Expr, id: u32) -> bool { + use perry_hir::Expr; + let hit = match expr { + Expr::LocalGet(found) | Expr::LocalSet(found, _) | Expr::Update { id: found, .. } => { + *found == id + } + _ => false, + }; + if hit { + return true; + } + let mut found = false; + perry_hir::walker::walk_expr_children(expr, &mut |child| { + if !found { + found = packed_f64_range_loop_expr_touches_local(child, id); + } + }); + found +} + /// #9253: is `index` an integer-producing expression over the loop counter and /// loop-invariant integer locals — `k`, `i * size + k`, `k * size + j`? /// @@ -2122,7 +2324,7 @@ fn packed_f64_range_loop_index_offset(index: &perry_hir::Expr, counter_id: u32) /// Body walk for [`match_packed_f64_range_loop`]: exactly one expression /// statement whose single side effect happens after all potential side exits. -fn packed_f64_range_loop_body_collect( +pub(super) fn packed_f64_range_loop_body_collect( body: &[Stmt], counter_id: u32, bound_local: Option, @@ -3211,12 +3413,21 @@ fn lower_packed_f64_range_versioned_for( // is exact for the whole loop — and, unlike a `@perry_global_*` load, // a non-escaping alloca is promotable to a register even with the fast // loop's raw inttoptr element stores in the way. - let written_local = match body { + // + // #10743: the GUARDED CLONES lower `fast_body` — the body as written + // unless the compound-assignment alias fold rewrote it. The SLOW clone + // always lowers `body`, so a failed guard executes the evaluation order + // the specification requires. Caching a module global is shared by both + // copies, which stays correct for the folded shape by the same argument + // as for every other matched body: it contains no call, closure or await, + // so nothing can write the global between the two loop entries. + let fast_body: &[Stmt] = matched.fast_body.as_deref().unwrap_or(body); + let written_local = match fast_body { [Stmt::Expr(perry_hir::Expr::LocalSet(id, _))] => Some(*id), _ => None, }; let mut global_override_ids: Vec = Vec::new(); - for gid in packed_f64_range_loop_invariant_global_reads(ctx, body, written_local) { + for gid in packed_f64_range_loop_invariant_global_reads(ctx, fast_body, written_local) { let Some(global_name) = ctx.module_globals.get(&gid).cloned() else { continue; }; @@ -3348,7 +3559,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "masked_window_ta_i32", "for.packed_f64_range_fast_ta_i32", true, @@ -3363,7 +3574,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "masked_window_ta_u32", "for.packed_f64_range_fast_ta_u32", false, @@ -3378,7 +3589,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "masked_window_ta_f64", "for.packed_f64_range_fast_ta_f64", false, @@ -3441,7 +3652,7 @@ fn lower_packed_f64_range_versioned_for( let mut acc_scope = emit_range_loop_accumulator_admission( ctx, &matched, - body, + fast_body, &slow_pre_label, "packed_f64_range.fast_i32", ); @@ -3470,7 +3681,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "for.packed_f64_range_fast_i32", Some((matched.counter_id, bound_i32.clone())), )?; @@ -3487,7 +3698,7 @@ fn lower_packed_f64_range_versioned_for( let mut acc_scope = emit_range_loop_accumulator_admission( ctx, &matched, - body, + fast_body, &slow_pre_label, "packed_f64_range.fast", ); @@ -3516,7 +3727,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "for.packed_f64_range_fast", Some((matched.counter_id, bound_i32.clone())), )?; @@ -3543,7 +3754,7 @@ fn lower_packed_f64_range_versioned_for( let mut acc_scope = emit_range_loop_accumulator_admission( ctx, &matched, - body, + fast_body, &slow_pre_label, "packed_f64_range.classic", ); @@ -3572,7 +3783,7 @@ fn lower_packed_f64_range_versioned_for( init, condition, update, - body, + fast_body, "for.packed_f64_range_fast", Some((matched.counter_id, bound_i32.clone())), )?; diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index 768cbea9f5..34931bcadf 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -22,6 +22,8 @@ mod boxed_slot_no_root_tests; mod cached_field_index_return; #[cfg(test)] mod class_field_loop_tests; +#[cfg(test)] +mod compound_alias_fold_tests; mod counter_range; mod element_shape_carried; mod element_shape_loop; diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index bd7229bc94..b909747c68 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -2732,6 +2732,183 @@ fn packed_f64_loop_store_update_versions_with_side_exit() { ); } +/// `const a = new Array(400)` as HIR records it: an `Array` binding with NO +/// type argument, i.e. `Array`. The store tier admits exactly this +/// ordinary untyped-JavaScript binding on the strength of its loop-entry +/// guard; a declared `number[]` array literal is declined earlier, at +/// `store_not_fact_eligible`, so it cannot carry this regression. +fn untyped_array_let(id: u32, name: &str, len: i64) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Generic { + base: "Array".to_string(), + type_args: Vec::new(), + }, + mutable: false, + init: Some(Expr::New { + class_name: "Array".to_string(), + args: vec![int(len)], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }), + } +} + +/// #10743: `a[i] += 1` reaches the classic range tier. +/// +/// HIR's `hoist_compound_member_assign` lowers a compound member assignment +/// into two immutable alias `Let`s plus the store, so the base and the key are +/// each evaluated exactly once and before the right-hand side. The classic +/// range matcher admits exactly ONE statement, so the idiomatic spelling could +/// never reach the tier that makes the expanded `a[i] = a[i] + 1` fast: 277 +/// instructions per element against 24 for the expanded form on the same +/// array, and a `number[]` annotation changed nothing, because the obstacle is +/// the statement count. +/// +/// The temporaries cannot be removed in the lowering -- an RHS call can +/// reassign the bindings they were read from, and the store must still land at +/// the index evaluated before it ran -- so the fold lives in the matcher and +/// applies to the GUARDED CLONES only. +/// +/// This body is transcribed from a `--print-hir` dump of the source, not +/// guessed. +fn compound_alias_body(array_id: u32, counter_id: u32, base_temp: u32, key_temp: u32) -> Vec { + let alias = |id: u32, name: &str, init: Expr| Stmt::Let { + id, + name: name.to_string(), + ty: Type::Number, + mutable: false, + init: Some(init), + }; + vec![ + alias(base_temp, "__cmpd_base_5", local(array_id)), + alias(key_temp, "__cmpd_key_6", local(counter_id)), + Stmt::Expr(Expr::IndexSet { + object: Box::new(local(base_temp)), + index: Box::new(local(key_temp)), + value: Box::new(add( + Expr::IndexGet { + object: Box::new(local(base_temp)), + index: Box::new(local(key_temp)), + }, + int(1), + )), + }), + ] +} + +#[test] +fn compound_assign_alias_body_reaches_the_range_tier() { + let module = module_with_classes_and_params( + "compound_alias_fold.ts", + Vec::new(), + Vec::new(), + Type::Number, + vec![ + untyped_array_let(1, "values", 400), + Stmt::For { + init: Some(Box::new(number_let(4, "i", true, int(0)))), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(local(4)), + right: Box::new(int(400)), + }), + update: Some(increment(4)), + body: compound_alias_body(1, 4, 5, 6), + }, + Stmt::Return(Some(index_get(1, int(0)))), + ], + ); + + let ir = compile_ir_for_module_with_opts(module, empty_opts()).unwrap(); + assert!( + ir.contains("call i32 @js_typed_feedback_packed_f64_range_loop_guard("), + "the compound-assign body should earn the classic range guard:\n{ir}" + ); + // Exactly one versioned loop exists in this module, so the clone below is + // unambiguously the compound-assign loop's. Counting the GUARD CALL, not + // the block label: every block of the fast copy carries the loop's label + // prefix, so a label count says four for one loop. + assert_eq!( + ir.matches("call i32 @js_typed_feedback_packed_f64_range_loop_guard(") + .count(), + 1, + "expected exactly one versioned range loop in this module:\n{ir}" + ); + let fast_start = ir + .find("\nfor.packed_f64_range_fast") + .map(|pos| pos + 1) + .expect("expected a range fast clone"); + let fast_end = ir[fast_start..] + .find("\nfor.packed_f64_range_slow") + .map(|off| fast_start + off) + .expect("expected a range slow clone"); + let fast_clone = &ir[fast_start..fast_end]; + // The point of the whole exercise: the fast clone's read and write are the + // inline packed pair, not the generic OBJECT property path the compound + // spelling used to fall to. + for forbidden in [ + "js_object_get_index_polymorphic", + "js_object_set_index_polymorphic", + "js_dyn_index_set_strict", + ] { + assert!( + !fast_clone.contains(forbidden), + "compound fast clone must not call {forbidden}:\n{fast_clone}" + ); + } + assert!( + fast_clone.contains("store double"), + "compound fast clone should store a raw double inline:\n{fast_clone}" + ); + // ... and the SLOW clone still lowers the statements as written, so a + // failed guard executes the specified evaluation order. + let slow = &ir[fast_end..]; + assert!( + slow.contains("for.packed_f64_range_slow"), + "expected the slow clone to survive:\n{ir}" + ); +} + +#[test] +fn a_mutable_leading_binding_is_not_folded() { + // The fold's argument is that these are the compiler's own immutable + // compound-assign spills. Flip `mutable` and the loop must stay on the + // generic path -- this is the test that goes red if that condition is + // deleted. + let mut body = compound_alias_body(1, 4, 5, 6); + if let Stmt::Let { mutable, .. } = &mut body[0] { + *mutable = true; + } + let module = module_with_classes_and_params( + "compound_alias_mutable.ts", + Vec::new(), + Vec::new(), + Type::Number, + vec![ + untyped_array_let(1, "values", 400), + Stmt::For { + init: Some(Box::new(number_let(4, "i", true, int(0)))), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(local(4)), + right: Box::new(int(400)), + }), + update: Some(increment(4)), + body, + }, + Stmt::Return(Some(index_get(1, int(0)))), + ], + ); + let ir = compile_ir_for_module_with_opts(module, empty_opts()).unwrap(); + assert!( + !ir.contains("\nfor.packed_f64_range_fast"), + "a mutable leading binding must not reach the range tier:\n{ir}" + ); +} + #[test] fn masked_window_dense_store_inlines_raw_store_without_calls() { // `for (let i = 0; i < 64; i++) a[i & 7] = i` — a masked static-window diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 116f75b6ff..199ece7b6a 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -234,6 +234,9 @@ const BUILD_CACHE_ENV_EXCLUSIONS: &[&str] = &[ // returns `None` either way — the rejection is what the caller already got // without the flag, so the emitted code is identical. An input, rather than // an exclusion, would make every trace run miss the cache for nothing. + // #10743 added the positive twin, `range_loop_trace`, on the same terms: it + // prints which admission a matched loop took and returns nothing, so the + // emitted object is byte-identical with the flag on and off. "PERRY_PACKED_LOOP_TRACE", // Entry outlining report output is observational only. "PERRY_OUTLINE_ENTRY_REPORT", From 0a0af40db16124a150e5c903550315739e026154 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sat, 19 Sep 2026 23:40:46 +0000 Subject: [PATCH 05/10] perf(transform): re-apply the literal-key member fold after const substitution (#10761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `o["a"]` written in source is already lowered to `o.a` by the AST->HIR member lowering (the #529 fold in `lower/expr_member/member_tail.rs`). But `module_const_fold` substitutes a hoisted `const K = "a"` into the key position *after* that matcher has run, and nothing re-ran it — so the enclosing node stayed an `IndexGet` and codegen's static-string-key arm resolved it by name at runtime on every read: UTF-8-validate the key, hash it for the accessor Bloom summary, classify the receiver, then scan the shape's key array. Phase 2 re-applies the same rewrite. The produced node is bit-identical to the one `o["name"]` produces in source, so there is no new fast path and no new guard; the read simply reaches the per-site monomorphic inline cache that the dotted spelling already used. O[K] + O[J] on {a:1,b:2,c:3} 1236 -> 169 instructions/iteration (7.31x) which is exactly what the same pair spelled `O.a + O.b` costs. Identical at both fit ranges. It also corrects a spec divergence: `null[K]` and `undefined[K]` silently read `undefined` before this change, where node throws a TypeError. Numeric-index strings are excluded, mirroring the source-level fold verbatim, so `arr["0"]` keeps IndexGet semantics. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10761-const-key-member-fold.md | 5 + .../perry-transform/src/module_const_fold.rs | 234 ++++++++++++++++++ ...test_gap_10761_const_key_property_reads.ts | 232 +++++++++++++++++ 3 files changed, 471 insertions(+) create mode 100644 changelog.d/10761-const-key-member-fold.md create mode 100644 test-files/test_gap_10761_const_key_property_reads.ts diff --git a/changelog.d/10761-const-key-member-fold.md b/changelog.d/10761-const-key-member-fold.md new file mode 100644 index 0000000000..5f6b02acfb --- /dev/null +++ b/changelog.d/10761-const-key-member-fold.md @@ -0,0 +1,5 @@ +**A hoisted `const K = "a"` used as a property key no longer costs 7.3× the same read spelled `o.a`.** + +`o["a"]` in source is already folded to `o.a` by the member lowering. But `module_const_fold` substitutes a hoisted const into the key position *after* that matcher has run, so the node stayed an `IndexGet` and was resolved by name at runtime on every read — UTF-8-validating the key, hashing it for the accessor Bloom summary, classifying the receiver and scanning the shape's key array. + +Re-applying the same fold takes `O[K] + O[J]` from **1236 to 169 instructions**, exactly what `O.a + O.b` costs. It also fixes a spec divergence: `null[K]` and `undefined[K]` read `undefined` before, where node throws. diff --git a/crates/perry-transform/src/module_const_fold.rs b/crates/perry-transform/src/module_const_fold.rs index 3e22229d6a..3d67f65d26 100644 --- a/crates/perry-transform/src/module_const_fold.rs +++ b/crates/perry-transform/src/module_const_fold.rs @@ -40,6 +40,12 @@ use perry_hir::{Expr, Function, Module, Stmt}; use crate::closure_local_inline::{for_each_expr_in_stmt_mut, nested_stmt_lists}; pub fn run(module: &mut Module) { + fold_module_consts(module); + // Phase 2 runs unconditionally — see `rewrite_literal_index_gets`. + rewrite_literal_index_gets(module); +} + +fn fold_module_consts(module: &mut Module) { let mut consts: HashMap = HashMap::new(); let mut decl_index: HashMap = HashMap::new(); for (index, stmt) in module.init.iter().enumerate() { @@ -254,6 +260,97 @@ fn fold_expr(expr: &mut Expr, consts: &HashMap) { walk_expr_children_mut(expr, &mut |child| fold_expr(child, consts)); } +/// Phase 2 (#10761) — rewrite `o[]` into `o.`. +/// +/// This is the SAME rewrite the AST→HIR member lowering already applies to a +/// literal key written in source (`lower/expr_member/member_tail.rs`, the +/// issue #529 fold), re-applied here because phase 1 above — and the inliner +/// before it — *create* `IndexGet { _, String(_) }` nodes AFTER that matcher +/// has run, and nothing re-ran it. +/// +/// The gap is worth 7.3x. A hoisted `const K = "a"` is folded to its literal +/// by phase 1, but the enclosing node stays an `IndexGet`, and codegen's +/// `IndexGet` arm for a static string key +/// (`expr/index_get.rs`, the `Expr::String(literal)` branch) calls +/// `js_typed_feedback_object_get_field_by_name_f64` — a full by-name runtime +/// resolution per read: UTF-8-validate the key, hash it for the accessor +/// Bloom summary, classify the receiver, then scan the shape's key array. +/// Measured on `O[K] + O[J]` over `{a:1,b:2,c:3}`: **618 instructions per +/// read**, against **24** for the identical read spelled `O.a`, which reaches +/// the per-site monomorphic inline cache in +/// `expr/property_get/generic_dispatch.rs`. `O["a"]` written in source is +/// already 24 — only the spelling that goes through a binding was stranded. +/// +/// Numeric-index strings are excluded, exactly as the source-level fold +/// excludes them: `arr["0"]` keeps `IndexGet` semantics (string-coerced +/// element access on an array), and that is the disambiguator the spec itself +/// uses between indexed and named properties. +/// +/// Everything else about the read is unchanged, because the produced node is +/// bit-identical to the one `o["name"]` produces in source: the same +/// `PropertyGet`, the same receiver expression, the same key string. There is +/// no new fast path here and no new guard — the rewrite moves a read onto a +/// lowering the whole test suite already exercises. +fn rewrite_literal_index_gets(module: &mut Module) { + for_each_function(module, &mut |f| rewrite_stmts(&mut f.body)); + rewrite_stmts(&mut module.init); +} + +/// A key that JavaScript resolves as an array index rather than a name. +/// +/// Mirrors `member_tail.rs`'s test verbatim so the two folds admit exactly the +/// same key set; if they ever diverge, `o["0"]` and a `const Z = "0"` spelling +/// of it would compile to different lowerings. +fn is_numeric_index_string(key: &str) -> bool { + !key.is_empty() + && key.chars().all(|c| c.is_ascii_digit()) + && !(key.len() > 1 && key.starts_with('0')) +} + +fn rewrite_stmts(stmts: &mut [Stmt]) { + for stmt in stmts.iter_mut() { + rewrite_stmt(stmt); + } +} + +fn rewrite_stmt(stmt: &mut Stmt) { + for inner in nested_stmt_lists(stmt) { + rewrite_stmts(inner); + } + for_each_expr_in_stmt_mut(stmt, &mut rewrite_expr); +} + +fn rewrite_expr(expr: &mut Expr) { + if let Expr::IndexGet { object, index } = expr { + let property = match index.as_ref() { + Expr::String(key) if !is_numeric_index_string(key) => Some(key.clone()), + _ => None, + }; + if let Some(property) = property { + // The index is a literal, so there is no key expression to keep + // alive and no evaluation-order obligation: `o[k]` evaluates `o` + // then `k`, and a literal `k` is already a value. + let object = std::mem::replace(object.as_mut(), Expr::Integer(0)); + *expr = Expr::PropertyGet { + // Synthesized: the literal was not written at a source span + // (phase 1 substituted it), so there is no member offset to + // carry. `0` is the established "no debug location" value on + // this node, and the `IndexGet` this replaces carried none + // either. + byte_offset: 0, + object: Box::new(object), + property, + }; + } + } + // `walk_expr_children_mut` does not descend into a closure's STATEMENT + // body; phase 1 has the same explicit arm for the same reason. + if let Expr::Closure { body, .. } = expr { + rewrite_stmts(body); + } + walk_expr_children_mut(expr, &mut rewrite_expr); +} + #[cfg(test)] mod tests { use super::*; @@ -385,4 +482,141 @@ mod tests { Stmt::Return(Some(Expr::Compare { right, .. })) if matches!(right.as_ref(), Expr::Integer(5)) )); } + + // ---- phase 2 (#10761): the literal-index rewrite ------------------- + + /// The module-level fold substitutes the literal, and phase 2 then moves + /// the read onto the SAME node `o["a"]` produces in source. Without + /// phase 2 this stays an `IndexGet` and codegen resolves it by name at + /// runtime — 618 instructions per read against 24. + #[test] + fn a_const_string_key_read_becomes_a_property_get() { + let mut m = Module::new("k.ts"); + m.init.push(Stmt::Let { + id: 3, + name: "K".to_string(), + ty: Type::String, + mutable: false, + init: Some(Expr::String("a".to_string())), + }); + m.functions.push(func( + 1, + vec![Stmt::Return(Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(8)), + index: Box::new(Expr::LocalGet(3)), + }))], + )); + run(&mut m); + let Stmt::Return(Some(Expr::PropertyGet { + object, property, .. + })) = &m.functions[0].body[0] + else { + panic!("expected a PropertyGet, got {:?}", m.functions[0].body[0]); + }; + assert_eq!(property, "a"); + assert!(matches!(object.as_ref(), Expr::LocalGet(8))); + } + + /// GUARD WITNESS for `is_numeric_index_string`. An array index key must + /// keep `IndexGet` semantics; `arr["0"]` is a string-coerced ELEMENT read, + /// not a named one, and that is the disambiguator the spec itself uses. + /// Delete the guard in `rewrite_expr` and this assertion fails. + /// + /// Note honestly what this test is and is not: at RUNTIME both spellings + /// happen to resolve a numeric name on an Array, a TypedArray and a String + /// through the same ladder, so the removal is behaviour-neutral on every + /// receiver I could construct. What removing it does cost is measured — + /// `Int32Array[K]` with `const K = "1"` goes from 969 to 1343 instructions + /// per read (+38.5%), because the folded form leaves the element lane. + #[test] + fn an_array_index_key_is_not_folded() { + for key in ["0", "1", "42", "4294967294"] { + let mut m = Module::new("k.ts"); + m.functions.push(func( + 1, + vec![Stmt::Return(Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(8)), + index: Box::new(Expr::String(key.to_string())), + }))], + )); + run(&mut m); + assert!( + matches!( + &m.functions[0].body[0], + Stmt::Return(Some(Expr::IndexGet { .. })) + ), + "key {key:?} must stay an IndexGet, got {:?}", + m.functions[0].body[0] + ); + } + } + + /// The keys the guard does NOT claim: a leading zero, a fraction, a sign + /// and the empty string are property NAMES, not indices, and must fold — + /// exactly as `member_tail.rs` folds them when written in source. + #[test] + fn a_non_index_numeric_looking_key_is_folded() { + for key in ["07", "1.5", "-1", "", "1e3", "NaN"] { + let mut m = Module::new("k.ts"); + m.functions.push(func( + 1, + vec![Stmt::Return(Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(8)), + index: Box::new(Expr::String(key.to_string())), + }))], + )); + run(&mut m); + assert!( + matches!( + &m.functions[0].body[0], + Stmt::Return(Some(Expr::PropertyGet { property, .. })) if property == key + ), + "key {key:?} must fold, got {:?}", + m.functions[0].body[0] + ); + } + } + + /// Phase 2 runs even when phase 1 folded nothing: a literal index can be + /// put there by the inliner, and `run` early-returns out of phase 1 when + /// the module declares no foldable const. + #[test] + fn the_rewrite_runs_with_no_module_consts_at_all() { + let mut m = Module::new("k.ts"); + m.init.push(Stmt::Expr(Expr::IndexGet { + object: Box::new(Expr::LocalGet(8)), + index: Box::new(Expr::String("name".to_string())), + })); + run(&mut m); + assert!(matches!( + &m.init[0], + Stmt::Expr(Expr::PropertyGet { property, .. }) if property == "name" + )); + } + + /// The receiver subtree is moved, not dropped: a nested read rewrites at + /// both levels and keeps its inner object. + #[test] + fn a_nested_literal_index_rewrites_at_every_level() { + let mut m = Module::new("k.ts"); + m.init.push(Stmt::Expr(Expr::IndexGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(8)), + index: Box::new(Expr::String("outer".to_string())), + }), + index: Box::new(Expr::String("inner".to_string())), + })); + run(&mut m); + let Stmt::Expr(Expr::PropertyGet { + object, property, .. + }) = &m.init[0] + else { + panic!("expected outer PropertyGet, got {:?}", m.init[0]); + }; + assert_eq!(property, "inner"); + assert!(matches!( + object.as_ref(), + Expr::PropertyGet { property, .. } if property == "outer" + )); + } } diff --git a/test-files/test_gap_10761_const_key_property_reads.ts b/test-files/test_gap_10761_const_key_property_reads.ts new file mode 100644 index 0000000000..d742bb1684 --- /dev/null +++ b/test-files/test_gap_10761_const_key_property_reads.ts @@ -0,0 +1,232 @@ +// #10761 — a property read spelled `O[K]` with a hoisted `const K = "a"` must +// be observably identical to `O.a` and `O["a"]` in EVERY case, now that the +// module-const fold rewrites the folded `IndexGet { _, String }` into a +// `PropertyGet` (perry-transform/src/module_const_fold.rs, phase 2). +// +// Each case prints the three spellings side by side. A rewrite that changed +// ANY of [[Get]]'s obligations shows up as a divergence between columns, and +// a rewrite that changed the answer outright shows up against node, which runs +// this same file as the oracle. +// +// The one guard in the rewrite is `is_numeric_index_string`: an array index +// key must keep `IndexGet` semantics. Cases 15/16 are its witnesses — delete +// the guard and they print element values where they must print `undefined`. +const K = "a"; +const M = "missing"; +const NUM0 = "0"; +const NUM1 = "1"; +const NUM07 = "07"; +const FRAC = "1.5"; +const NEG = "-1"; +const EMPTY = ""; +const LEN = "length"; + +const out: string[] = []; +function show(label: string, a: unknown, b: unknown, c: unknown): void { + out.push(label + " | " + String(a) + " | " + String(b) + " | " + String(c)); +} +function trap(f: () => unknown): string { + try { + return "value:" + String(f()); + } catch (e) { + return "throw:" + (e instanceof TypeError ? "TypeError" : String(e)); + } +} + +// 1 — plain own data property +const o1: any = { a: 1, b: 2 }; +show("1 own-data", o1[K], o1["a"], o1.a); + +// 2 — own accessor installed by defineProperty +const o2: any = {}; +let getCalls = 0; +Object.defineProperty(o2, "a", { + get() { + getCalls++; + return 42; + }, + configurable: true, +}); +show("2 own-getter", o2[K], o2["a"], o2.a); +out.push("2 getter-call-count " + getCalls); + +// 3 — setter-only own accessor reads as undefined +const o3: any = {}; +Object.defineProperty(o3, "a", { set(_v: number) {}, configurable: true }); +show("3 setter-only", o3[K], o3["a"], o3.a); + +// 4 — accessor on the prototype chain +const proto4: any = {}; +Object.defineProperty(proto4, "a", { + get() { + return "from-proto"; + }, + configurable: true, +}); +const o4: any = Object.create(proto4); +show("4 proto-getter", o4[K], o4["a"], o4.a); + +// 5 — non-enumerable data descriptor +const o5: any = {}; +Object.defineProperty(o5, "a", { value: 5, enumerable: false, writable: true, configurable: true }); +show("5 nonenum-data", o5[K], o5["a"], o5.a); + +// 6 — non-writable, non-configurable +const o6: any = {}; +Object.defineProperty(o6, "a", { value: 6, writable: false, configurable: false }); +show("6 frozen-slot", o6[K], o6["a"], o6.a); + +// 7 — frozen object +const o7: any = Object.freeze({ a: 7 }); +show("7 frozen-obj", o7[K], o7["a"], o7.a); + +// 8 — sealed object +const o8: any = Object.seal({ a: 8 }); +show("8 sealed-obj", o8[K], o8["a"], o8.a); + +// 9 — delete then read +const o9: any = { a: 9, z: 0 }; +show("9 before-delete", o9[K], o9["a"], o9.a); +delete o9.a; +show("9 after-delete", o9[K], o9["a"], o9.a); + +// 10 — own shadows inherited +const proto10: any = { a: "proto" }; +const o10: any = Object.create(proto10); +show("10 inherited", o10[K], o10["a"], o10.a); +o10.a = "own"; +show("10 shadowed", o10[K], o10["a"], o10.a); +delete o10.a; +show("10 unshadowed", o10[K], o10["a"], o10.a); + +// 11 — setPrototypeOf after the site has run +const o11: any = {}; +show("11 no-proto", o11[K], o11["a"], o11.a); +Object.setPrototypeOf(o11, { a: "late-proto" }); +show("11 late-proto", o11[K], o11["a"], o11.a); + +// 12 — __proto__ assignment +const o12: any = {}; +show("12 pre-__proto__", o12[K], o12["a"], o12.a); +o12.__proto__ = { a: "via-dunder" }; +show("12 post-__proto__", o12[K], o12["a"], o12.a); + +// 13 — Proxy receiver: the trap must see the same key for all three spellings +const seen: string[] = []; +const p13: any = new Proxy( + { a: "target" }, + { + get(t: any, k: any) { + if (typeof k === "string") seen.push(k); + return k === "a" ? "trapped" : Reflect.get(t, k); + }, + }, +); +show("13 proxy", p13[K], p13["a"], p13.a); +out.push("13 trap-keys " + seen.join(",")); + +// 14 — nullish receivers must throw TypeError, not read undefined +const nul: any = null; +const undef: any = undefined; +out.push("14 null-const " + trap(() => nul[K])); +out.push("14 null-lit " + trap(() => nul["a"])); +out.push("14 null-dot " + trap(() => nul.a)); +out.push("14 undef-const " + trap(() => undef[K])); +out.push("14 undef-lit " + trap(() => undef["a"])); +out.push("14 undef-dot " + trap(() => undef.a)); + +// 15 — GUARD WITNESS: a canonical numeric key on an ARRAY is an element read, +// not a named read. `arr[NUM0]` must be the element; `arr[FRAC]`/`arr[NEG]`/ +// `arr[NUM07]`/`arr[EMPTY]` are names and must miss. +const arr: any = ["zero", "one", "two"]; +out.push("15 arr-0 " + String(arr[NUM0]) + " | " + String(arr["0"]) + " | " + String(arr[0])); +out.push("15 arr-1 " + String(arr[NUM1]) + " | " + String(arr["1"])); +out.push("15 arr-07 " + String(arr[NUM07]) + " | " + String(arr["07"])); +out.push("15 arr-frac " + String(arr[FRAC]) + " | " + String(arr["1.5"])); +out.push("15 arr-neg " + String(arr[NEG]) + " | " + String(arr["-1"])); +out.push("15 arr-empty " + String(arr[EMPTY]) + " | " + String(arr[""])); +out.push("15 arr-length " + String(arr[LEN]) + " | " + String(arr["length"]) + " | " + String(arr.length)); + +// 16 — GUARD WITNESS: a numeric-string OWN property on a plain object, with an +// array-index twin, so a fold that treats "0" as a name is visible. +const o16: any = { "0": "named-zero", a: 16 }; +out.push("16 obj-0 " + String(o16[NUM0]) + " | " + String(o16["0"]) + " | " + String(o16[0])); +const mixed: any = ["elem0"]; +mixed["0"] = "overwritten"; +out.push("16 mixed-0 " + String(mixed[NUM0]) + " | " + String(mixed[0]) + " len=" + mixed.length); + +// 17 — a missing key +const o17: any = { b: 1 }; +show("17 absent", o17[M], o17["missing"], o17.missing); + +// 18 — an accessor installed AFTER the read site has already executed +const o18: any = { a: "data" }; +show("18 data-first", o18[K], o18["a"], o18.a); +Object.defineProperty(o18, "a", { + get() { + return "now-accessor"; + }, + configurable: true, +}); +show("18 accessor-after", o18[K], o18["a"], o18.a); + +// 19 — a getter that mutates the receiver during the read +const o19: any = { z: 0 }; +Object.defineProperty(o19, "a", { + get() { + o19.z = o19.z + 1; + return o19.z; + }, + configurable: true, +}); +show("19 mutating-getter", o19[K], o19["a"], o19.a); + +// 20 — Symbol key congruence control (never folded; must still agree) +const SYM = Symbol.for("perry.10761"); +const o20: any = { [SYM]: "sym-value", a: 20 }; +out.push("20 symbol " + String(o20[SYM]) + " | " + String(o20[K])); + +// 21 — string receiver named read, and a numeric key on a string +const s21: any = "abc"; +out.push("21 str-length " + String(s21[LEN]) + " | " + String(s21["length"]) + " | " + String(s21.length)); +out.push("21 str-0 " + String(s21[NUM0]) + " | " + String(s21["0"]) + " | " + String(s21[0])); + +// 22 — class instance: own field, prototype method, prototype accessor +class C22 { + a = 22; + get g(): string { + return "getter"; + } + m(): string { + return "method"; + } +} +const GKEY = "g"; +const MKEY = "m"; +const c22: any = new C22(); +show("22 field", c22[K], c22["a"], c22.a); +show("22 proto-getter", c22[GKEY], c22["g"], c22.g); +out.push("22 proto-method " + String(typeof c22[MKEY]) + " | " + String(typeof c22["m"]) + " | " + String(typeof c22.m)); + +// 23 — the read in a hot loop, so the inline cache is primed and then broken +const o23: any = { a: 1 }; +let sum = 0; +for (let i = 0; i < 50; i++) sum = sum + o23[K]; +out.push("23 warm-sum " + sum); +Object.defineProperty(o23, "a", { + get() { + return 100; + }, + configurable: true, +}); +let sum2 = 0; +for (let i = 0; i < 5; i++) sum2 = sum2 + o23[K]; +out.push("23 post-accessor-sum " + sum2); + +// 24 — a polymorphic site: three different shapes through one const-key read +const shapes: any[] = [{ a: 1 }, { x: 0, a: 2 }, Object.create({ a: 3 })]; +let poly = ""; +for (let i = 0; i < shapes.length; i++) poly = poly + String(shapes[i][K]) + ","; +out.push("24 poly " + poly); + +console.log(out.join("\n")); From 70ad5cf63ad3e45d1018a9da0e35a407e0c757c0 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sat, 19 Sep 2026 23:49:52 +0000 Subject: [PATCH 06/10] perf(runtime): stop routing plain numbers through the slow arms of the string-coercion ladders (#10762) Four edits, all runtime, no codegen: `js_string_coerce` and `js_jsvalue_to_string_method` reached their plain-number arm last, through a seven-way jump table; `is_number()` is one range test and the exact complement of the arms it skips, so the number arm is hoisted ahead of them. `js_number_to_string`'s admission check forced LLVM to emit a 14-instruction saturating f64->u64 cast on a value already proven to be in 0..256, plus a redundant second bound check; the cheaper admission lets it emit a 4-instruction cast, and the cache-fill arm is outlined `#[cold]` so its inlined `format!` stops costing 15 instructions of prologue in the hit path. `format_number_into` gains a range-proven i32 arm. String(k%100) 190.0 -> 163.0 (-14.2%) n.toString() 536.3 -> 433.0 (-19.3%) `${n}` 433.3 -> 413.0 (-4.7%) String(k%1e6) 558.3 -> 540.3 (-3.2%) float 1146.6 -> 1136.6 (-0.9%) "" + n 264.5 -> 264.5 0.00% control (no conv) 82.0 -> 82.0 0.00% No row regresses. Both arms are flat within 2% across 20k->200k and 500k->5M. This does not reach parity with node or bun, and the remaining distance needs an ABI change rather than another pass: `"" + n` never allocates, because `js_string_concat_value_box` returns an f64 and packs a short result into SHORT_STRING_TAG, while the other three entry points are declared `-> *mut StringHeader` and must allocate a heap string for a three-byte result. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10762-number-to-string-ladders.md | 7 ++ crates/perry-runtime/src/builtins/numbers.rs | 16 +++++ crates/perry-runtime/src/string/concat.rs | 29 ++++++++ crates/perry-runtime/src/string/format.rs | 67 +++++++++++++------ crates/perry-runtime/src/value/to_string.rs | 14 ++++ 5 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 changelog.d/10762-number-to-string-ladders.md diff --git a/changelog.d/10762-number-to-string-ladders.md b/changelog.d/10762-number-to-string-ladders.md new file mode 100644 index 0000000000..35fb2d5b6f --- /dev/null +++ b/changelog.d/10762-number-to-string-ladders.md @@ -0,0 +1,7 @@ +**Plain numbers no longer take the slow arms of the string-coercion ladders.** + +`js_string_coerce` and `js_jsvalue_to_string_method` reached their plain-number arm last, through a seven-way jump table, though `is_number()` is a single range test and the exact complement of the arms it skips. `js_number_to_string`'s admission check also forced a 14-instruction saturating `f64`→`u64` cast on a value already proven to be within the 256-entry small-integer cache. + +`n.toString()` **−19.3%** (536.3 → 433.0), `String(n)` **−14.2%** (190.0 → 163.0), template literal −4.7%, large integers −3.2%, floats −0.9%, `"" + n` unchanged. No row regresses. + +`n.toString()` was costing `String(n)` **plus exactly 103 instructions** at every value range — a four-frame dispatch detour through a thread-local one-shot — which is what this removes. diff --git a/crates/perry-runtime/src/builtins/numbers.rs b/crates/perry-runtime/src/builtins/numbers.rs index d2f8760cd8..98f1720b41 100644 --- a/crates/perry-runtime/src/builtins/numbers.rs +++ b/crates/perry-runtime/src/builtins/numbers.rs @@ -649,6 +649,22 @@ pub extern "C" fn js_number_coerce(value: f64) -> f64 { pub extern "C" fn js_string_coerce(value: f64) -> *mut StringHeader { let jsval = JSValue::from_bits(value.to_bits()); + // A plain IEEE double is the overwhelmingly common argument here — + // `String(n)` and every template substitution of a number land on it — and + // it was the LAST arm of the ladder below, so every one of them paid eight + // tag comparisons plus a jump table to reach the one line that answers it. + // `is_number()` is a single range test (perry's tags occupy the contiguous + // positive-qNaN band `0x7FF9..=0x7FFF`), and it is the exact complement of + // the arms it skips: undefined/null/bool are `0x7FFC`, short string + // `0x7FF9`, bigint `0x7FFA`, pointer `0x7FFD`, int32 `0x7FFE`, string + // `0x7FFF`. Every other bit pattern — including a JS handle (`0x7FFB`), a + // hole and a TDZ sentinel, none of which the ladder matches either — + // reaches the same `js_number_to_string` tail with or without this hoist, + // so the reorder is answer-for-answer identical on every input. + if jsval.is_number() { + return crate::string::js_number_to_string(value); + } + let result = if jsval.is_undefined() { "undefined".to_string() } else if jsval.is_null() { diff --git a/crates/perry-runtime/src/string/concat.rs b/crates/perry-runtime/src/string/concat.rs index ce30ed6839..09c047ebc5 100644 --- a/crates/perry-runtime/src/string/concat.rs +++ b/crates/perry-runtime/src/string/concat.rs @@ -1485,6 +1485,35 @@ fn concat_chain_sized(parts: *const f64, n: usize) -> *m /// of bytes written. #[inline] pub(crate) fn format_number_into(value: f64, buf: &mut [u8; 32]) -> usize { + // Integers that fit i32 are the bulk of every formatted number — loop + // counters, ids, counts, sizes, byte values, HTTP codes — and this arm + // decides them without the i64 arm's range test and without its + // `is_nan`/`is_infinite` pair. + // + // Both halves of the guard are load-bearing, and the SECOND one is + // load-bearing for SPEED as well as correctness: `abs() < 2^31` is what + // lets LLVM prove the `as i32` cannot overflow and emit a bare + // `cvttsd2si` instead of Rust's ~8-instruction SATURATING cast sequence. + // Written without it (guarding on an `(n as f64) == value` round trip + // instead) this arm MEASURED 7 instructions per call SLOWER than the code + // it replaced on 6-digit values, for exactly that reason. The i64 arm + // below gets the same proof from its own `abs() < 1e15`. + // + // `fract() == 0.0` alone already excludes NaN and +-Infinity (`fract` is + // `self - self.trunc()`, which is NaN for both, and NaN != 0.0), and + // `-0.0` passes it, converts to 0 and renders "0" — the spec answer, and + // the same one the `value == 0.0` arm below produces. + // + // Strictly additive: every value this accepts is exactly an i32, which the + // i64 arm would have handed to these very same `fast_itoa_u32` / + // `fast_itoa_i64` helpers. The bytes cannot differ. + if value.fract() == 0.0 && value.abs() < 2_147_483_648.0 { + let n = value as i32; + if n >= 0 { + return fast_itoa_u32(n as u32, buf); + } + return fast_itoa_i64(n as i64, buf); + } if value.fract() == 0.0 && value.abs() < 1e15 && !value.is_nan() && !value.is_infinite() { let n = value as i64; if (0..=999_999_999).contains(&n) { diff --git a/crates/perry-runtime/src/string/format.rs b/crates/perry-runtime/src/string/format.rs index e3d36204ca..f4290d9ee1 100644 --- a/crates/perry-runtime/src/string/format.rs +++ b/crates/perry-runtime/src/string/format.rs @@ -117,30 +117,24 @@ fn throw_if_bigint_digits(arg: f64) { #[no_mangle] pub extern "C" fn js_number_to_string(value: f64) -> *mut StringHeader { // Fast path: small non-negative integers use a cached string table. + // + // The admission test is `fract() == 0.0` plus an in-range check written so + // LLVM can prove the `as u32` cannot overflow and emit a bare + // `cvttsd2si`. The old `value as usize` — on a value the same condition + // had already proven to be in `0..256` — lowered to Rust's full SATURATING + // `f64 -> u64` sequence: 14 instructions of `cmov` fixup, a quarter of + // what a cache hit cost. `-0.0` passes (`-0.0 >= 0.0`), converts to 0 and + // returns "0", which is the spec answer for `String(-0)`. NaN and + // +-Infinity fail `fract() == 0.0` (`fract` is `self - self.trunc()`, + // which is NaN for both). if value.fract() == 0.0 && value >= 0.0 && value < SMALL_INT_CACHE_SIZE as f64 { - let idx = value as usize; - let cached = SMALL_INT_CACHE.with(|c| unsafe { (*c.get())[idx] }); + let idx = value as u32 as usize; + // SAFETY: the range test above proves `idx < SMALL_INT_CACHE_SIZE`. + let cached = SMALL_INT_CACHE.with(|c| unsafe { *(*c.get()).get_unchecked(idx) }); if !cached.is_null() { return cached; } - // Allocate and cache - let s = format!("{}", value as u64); - let ptr = js_string_from_bytes_longlived(s.as_bytes().as_ptr(), s.len() as u32); - unsafe { - // Mark as shared so it's never mutated in-place - (*ptr).refcount = 0; - // Mark as pinned so GC keeps it live for the lifetime of this - // thread's arena. Longlived-space (see the allocation above), so - // this does not arm the young-pin latch (#7645). - let gc_header = - (ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; - crate::gc::pin_object_non_young(gc_header); - } - SMALL_INT_CACHE.with(|c| unsafe { - // GC_STORE_AUDIT(ROOT): SMALL_INT_CACHE is scanned by scan_small_int_cache_roots_mut. - crate::gc::runtime_store_root_raw_mut_ptr_slot(&raw mut (*c.get())[idx], ptr); - }); - return ptr; + return small_int_cache_fill(idx); } // Format the number as a string per JS semantics, on the stack. @@ -149,6 +143,39 @@ pub extern "C" fn js_number_to_string(value: f64) -> *mut StringHeader { js_string_from_bytes(buf.as_ptr(), len as u32) } +/// Mint, pin and publish the canonical string for a small-int cache index. +/// +/// Genuinely cold: it runs at most once per index per thread — 256 times in +/// the entire life of a thread — yet inlined it put `format!`'s formatting +/// machinery, the GC pin and the root store into [`js_number_to_string`], +/// which cost every cached conversion six pushes and a 0x48-byte frame. +/// Outlined here rather than around the whole uncached tail on purpose: +/// wrapping the stack-buffer formatting path too MEASURED +11.7 instructions +/// per conversion on the float fixture, because a miss then paid an extra +/// call and re-ran the admission test. +#[cold] +#[inline(never)] +fn small_int_cache_fill(idx: usize) -> *mut StringHeader { + debug_assert!(idx < SMALL_INT_CACHE_SIZE); + let s = format!("{}", idx); + let ptr = js_string_from_bytes_longlived(s.as_bytes().as_ptr(), s.len() as u32); + unsafe { + // Mark as shared so it's never mutated in-place + (*ptr).refcount = 0; + // Mark as pinned so GC keeps it live for the lifetime of this + // thread's arena. Longlived-space (see the allocation above), so + // this does not arm the young-pin latch (#7645). + let gc_header = + (ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; + crate::gc::pin_object_non_young(gc_header); + } + SMALL_INT_CACHE.with(|c| unsafe { + // GC_STORE_AUDIT(ROOT): SMALL_INT_CACHE is scanned by scan_small_int_cache_roots_mut. + crate::gc::runtime_store_root_raw_mut_ptr_slot(&raw mut (*c.get())[idx], ptr); + }); + ptr +} + /// ECMAScript `Number::toString` formatting, returning the Rust `String`. /// /// Shared by `js_number_to_string` (the `.toString()` path) and the diff --git a/crates/perry-runtime/src/value/to_string.rs b/crates/perry-runtime/src/value/to_string.rs index 6129eaeee6..80c79d24ba 100644 --- a/crates/perry-runtime/src/value/to_string.rs +++ b/crates/perry-runtime/src/value/to_string.rs @@ -1455,6 +1455,20 @@ pub(crate) unsafe fn coerce_validate_radix(radix_value: f64) -> Option { /// unchanged. #[no_mangle] pub extern "C" fn js_jsvalue_to_string_method(value: f64) -> *mut crate::string::StringHeader { + // `n.toString()` on a plain number is `Number::toString(n)` and nothing + // else, but it reached that answer through four frames: + // `to_string_method_impl` (nullish guard, pointer/regex probes, then a + // thread-local one-shot WRITE) -> `js_jsvalue_to_string` (which READS and + // clears that same one-shot, probes for a JS handle, then walks its own + // eight-arm tag ladder) -> `js_number_to_string`. None of it can change a + // plain double's answer: a number is never nullish, never a pointer, never + // a regex, and every arm of both ladders is keyed on a perry tag in the + // `0x7FF9..=0x7FFF` band that `is_number()` excludes by definition. The + // one-shot is only ever consumed by the object dispatch this value cannot + // reach, so not setting it leaves nothing stale behind. + if crate::value::JSValue::from_bits(value.to_bits()).is_number() { + return crate::string::js_number_to_string(value); + } // Explicit `x.toString()`: resolve `Object.prototype.toString` / an own // `toString`, never `[Symbol.toPrimitive]`. (#6373) to_string_method_impl(value, /* skip_to_primitive */ true) From 1794e9906d0459a909344e818648f33023d145ea Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sun, 20 Sep 2026 01:31:43 +0000 Subject: [PATCH 07/10] perf(codegen): stop disabling Ptr in entry bodies for a bug that was fixed in the runtime (#10769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RepselContextFlags::derive`'s `Entry` arm forced `allows_ptr_shape: false` and a `MODULE_INIT_CONTEXT` denial, on the stated grounds that "#6991 is an open rooting bug in exactly that position". #6991 is closed. It was fixed by #7249 (64c1f56fb), which placed `populate_global_this_builtins` inside a `GcSuppressScope` — a runtime fix, not a codegen one. The gate has since been guarding against a bug that no longer exists, and the effect was that a shape proof in an entry body was made, counted as a win in the optimiser report, and then dropped at every access site. The `Entry` arm now derives all three flags from their knobs like any other body. module-level const, loop at module level 110.00 -> 88.99 (-19.1%) node is 14.50 on the same fixture, so this does not reach parity; roughly 36 instructions of entry-body cost remain and are not this gate. The same body placed inside a function is the control and correctly does not move. The nine real programs do not move, and the mechanism was checked rather than assumed: `--opt-report` module-init denial mentions are identical on both arms for all nine, because none of them has a `Ptr` candidate in its entry body. `validate` and `resolve` do hold module-level const records, but they are read from inside functions, which globalizes them and puts them behind the separate storage limitation tracked as #7109. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10769-entry-body-ptr-shape.md | 5 + crates/perry-codegen/src/expr/repsel_gates.rs | 53 +++++--- crates/perry-codegen/src/expr/slot_rep.rs | 30 ++-- .../test_gap_10769_entry_body_ptr_shape.ts | 128 ++++++++++++++++++ 4 files changed, 191 insertions(+), 25 deletions(-) create mode 100644 changelog.d/10769-entry-body-ptr-shape.md create mode 100644 test-files/test_gap_10769_entry_body_ptr_shape.ts diff --git a/changelog.d/10769-entry-body-ptr-shape.md b/changelog.d/10769-entry-body-ptr-shape.md new file mode 100644 index 0000000000..379e2f3fdf --- /dev/null +++ b/changelog.d/10769-entry-body-ptr-shape.md @@ -0,0 +1,5 @@ +**`Ptr` is no longer disabled in entry bodies for a bug that was fixed in the runtime.** + +`RepselContextFlags::derive`'s `Entry` arm forced `allows_ptr_shape: false` because *"#6991 is an open rooting bug in exactly that position"*. #6991 was closed by #7249, which put `populate_global_this_builtins` inside a `GcSuppressScope` — a runtime fix. The gate has been guarding a bug that no longer exists, and a shape proof in an entry body was being made, counted as a win, then dropped at every access site. + +A module-level `const` with its loop at module level goes **110.00 → 88.99 instructions per iteration (−19.1%)**. The same body inside a function is the control and correctly does not move. diff --git a/crates/perry-codegen/src/expr/repsel_gates.rs b/crates/perry-codegen/src/expr/repsel_gates.rs index 238914e954..76c35c06b2 100644 --- a/crates/perry-codegen/src/expr/repsel_gates.rs +++ b/crates/perry-codegen/src/expr/repsel_gates.rs @@ -63,7 +63,6 @@ use super::slot_rep::{ body_context_denial, canonical_i32_locals_enabled, canonical_str_locals_enabled, - MODULE_INIT_CONTEXT, }; /// `PERRY_STATIC_STRING_LOWERING` gate. Enabled by default; `=0`/`off`/`false` @@ -184,16 +183,33 @@ impl RepselContextFlags { ptr_shape_denial: denial, } } + // #10769: the entry body now derives all three flags exactly as an + // ordinary body does. It carries no structural denial of its own — + // module init is never rewritten into a generator state machine + // (see the `slot_rep::MODULE_INIT_CONTEXT` audit), so + // `body_context_denial`'s three reasons cannot arise here. + // + // The `Ptr` literal `false` that stood here was justified by + // #6991, "a compiled receiver goes stale across the + // globalThis-population collection". **#6991 is closed**, fixed by + // #7249 (`64c1f56fb`) in the RUNTIME, not here: + // `populate_global_this_builtins` now runs inside a + // `GcSuppressScope` because it builds an immortal graph through raw + // `*mut ObjectHeader` locals across its own ~1.15 MB of + // allocations. Its closing comment re-verified + // `test_gap_repsel_ptr_shape_locals` at 10/10 on the evacuating arm + // and 3/3 under `PERRY_GC_ZEAL=1`, at 3.4x the movement level the + // crash was observed at. + // + // A gate whose stated reason is a closed bug reads as a live + // constraint to the next person. It was read that way twice before + // it was removed. RepselBody::Entry => Self { allows_canonical_i32: gates.canonical_i32, allows_canonical_str: gates.canonical_str, - // Unconditionally off, regardless of `gates.ptr_shape`: the - // exclusion is structural (#6991), not a knob. Written as a - // literal so a future reader cannot mistake it for something - // `PERRY_PTR_SHAPE_LOCALS=1` could turn back on. - allows_ptr_shape: false, + allows_ptr_shape: gates.ptr_shape, canonical_denial: None, - ptr_shape_denial: Some(MODULE_INIT_CONTEXT), + ptr_shape_denial: None, }, } } @@ -300,16 +316,21 @@ mod tests { } } - /// The same property for the entry context, where `Ptr` is off for a - /// structural reason: the two canonical knobs must still move only - /// themselves, and the `Ptr` knob must move nothing (it is already - /// off). + /// #10769: the entry context now derives all three flags like any other + /// body. Each knob still moves exactly one flag — that is #7128's property, + /// restated for `Entry` — and no flag carries a structural denial, because + /// the entry body has none. + /// + /// GUARD WITNESS. Restore the literal `allows_ptr_shape: false` in the + /// `Entry` arm and the first assertion fails with + /// `(true, true, false) != (true, true, true)`; restore + /// `ptr_shape_denial: Some(MODULE_INIT_CONTEXT)` and the third fails. #[test] - fn entry_context_keeps_ptr_shape_off_and_names_the_rule() { + fn entry_context_derives_every_flag_like_an_ordinary_body() { let entry = RepselContextFlags::derive(ALL_ON, RepselBody::Entry); - assert_eq!(allows(&entry), (true, true, false)); + assert_eq!(allows(&entry), (true, true, true)); assert_eq!(entry.canonical_denial, None); - assert_eq!(entry.ptr_shape_denial, Some(MODULE_INIT_CONTEXT)); + assert_eq!(entry.ptr_shape_denial, None); for gates in [ RepselGates { @@ -326,10 +347,10 @@ mod tests { }, ] { let got = RepselContextFlags::derive(gates, RepselBody::Entry); - assert!(!got.allows_ptr_shape); - assert_eq!(got.ptr_shape_denial, Some(MODULE_INIT_CONTEXT)); assert_eq!(got.allows_canonical_i32, gates.canonical_i32); assert_eq!(got.allows_canonical_str, gates.canonical_str); + assert_eq!(got.allows_ptr_shape, gates.ptr_shape); + assert_eq!(got.ptr_shape_denial, None); } } diff --git a/crates/perry-codegen/src/expr/slot_rep.rs b/crates/perry-codegen/src/expr/slot_rep.rs index f8964cea22..8dc38ff77c 100644 --- a/crates/perry-codegen/src/expr/slot_rep.rs +++ b/crates/perry-codegen/src/expr/slot_rep.rs @@ -159,17 +159,29 @@ pub(crate) enum SlotRep { /// `register_module_globals_as_gc_roots`) reads `@perry_global_*` cells and /// never `ctx.locals`. /// -/// ## What is still excluded, and why +/// ## What was also excluded, and no longer is (#10769) /// -/// `Ptr` receiver proofs. Phase 5a reused +/// `Ptr` receiver proofs used to be excluded here too. Phase 5a reused /// `repsel_context_allows_canonical_i32` as its context gate, so lifting that -/// flag would silently have enabled guard-free `this.field` / `obj.field` -/// lowering in entry bodies as a side effect of an unrelated phase. That is not -/// a representation this issue measured, and #6991 is an open rooting bug in -/// exactly that position: a compiled receiver goes stale across the -/// `globalThis`-population collection, which runs around module init. So the -/// flag is split (`repsel_context_allows_ptr_shape`) and entry bodies keep -/// `Ptr` off, still naming this rule in `--opt-report`. +/// flag would have enabled guard-free `this.field` / `obj.field` lowering in +/// entry bodies as a side effect of an unrelated phase; the flag was split +/// (`repsel_context_allows_ptr_shape`) and `Entry` pinned its own arm off, +/// citing #6991 — "a compiled receiver goes stale across the +/// `globalThis`-population collection, which runs around module init". +/// +/// **#6991 is closed.** It was fixed by #7249 (`64c1f56fb`) in the runtime, not +/// by this gate: `populate_global_this_builtins` now runs inside a +/// `GcSuppressScope`, because it builds an immortal object graph through raw +/// `*mut ObjectHeader` locals held across its own ~1.15 MB of allocations, so +/// under an 8 MB heap limit minor #0 landed in the middle of it. The closing +/// comment re-verified `test_gap_repsel_ptr_shape_locals` at 10/10 on the +/// evacuating arm and 3/3 under `PERRY_GC_ZEAL=1`, at 3.4x the movement level +/// the crash was observed at. The entry arm now derives `allows_ptr_shape` from +/// its knob like every other body (`expr/repsel_gates.rs`). +/// +/// `MODULE_INIT_CONTEXT` is retained: it is still a rule name the +/// `--opt-report` renderer resolves, and removing a denial string would break +/// reports archived from older builds. pub(crate) const MODULE_INIT_CONTEXT: &str = "module_init_context"; /// Why an ordinary body context forbids canonical (i32/u32/Str) selection, or diff --git a/test-files/test_gap_10769_entry_body_ptr_shape.ts b/test-files/test_gap_10769_entry_body_ptr_shape.ts new file mode 100644 index 0000000000..5929bb3e88 --- /dev/null +++ b/test-files/test_gap_10769_entry_body_ptr_shape.ts @@ -0,0 +1,128 @@ +// #10769: `Ptr` in a PROGRAM-ENTRY / module-init body. +// +// The entry arm of `RepselContextFlags::derive` used to pin `allows_ptr_shape` +// off with a literal `false`, citing #6991 ("a compiled receiver goes stale +// across the globalThis-population collection, which runs around module init"). +// #6991 was closed by #7249, which fixed it in the RUNTIME by putting +// `populate_global_this_builtins` inside a `GcSuppressScope`. The gate is now +// derived from its knob like any other body. +// +// `test_gap_repsel_ptr_shape_locals` CANNOT witness that change: its +// `Ptr` selection count is identical with the gate on and off (18 +// selected / 11 denied both ways), because every one of its candidates is +// either inside a function or module-globalized. This file exists because that +// one does not reach the lifted gate. +// +// NOTE ON WHAT IS NOT HERE. `Object.freeze` and `Object.defineProperty` are +// deliberately absent: either one arms the module-wide 5.2 shape-barrier kill +// (`ModuleDispatchFacts::has_shape_barrier_sites`), which disables ALL +// `Ptr` promotion in the module. A first draft of this file included +// both and reported `0 selected / 15 denied` on BOTH arms -- it would have +// passed every GC run while witnessing nothing. Those cases belong in a module +// that is not trying to prove a shape; `test_gap_repsel_ptr_shape_barriers.ts` +// already owns them. +// +// Everything below is at TOP LEVEL on purpose — a binding read only from the +// entry body is not globalized, so it is a `Ptr` candidate there and +// nowhere else. Each section puts a collection point between the proof and the +// use, which is the exact hazard #6991 named: the object may MOVE, so the +// tagged-at-rest slot must be re-derived after every safepoint. + +const out: string[] = []; + +// 1. Provenance-proven class instance in the entry body, with allocation +// inside the loop so minors fire between the field reads. +class Pt { + x: number; + y: number; + tag: string; + constructor(x: number, y: number, tag: string) { + this.x = x; + this.y = y; + this.tag = tag; + } + norm(): number { + return this.x + this.y; + } +} +const p = new Pt(3, 4, "origin"); +let acc = 0; +const litter: number[][] = []; +for (let i = 0; i < 400; i++) { + // allocate so the nursery fills and the back-edge poll collects + litter.push([i, i + 1, i + 2]); + if (litter.length > 32) litter.shift(); + p.x = i; + acc = (acc + p.x + p.y + p.norm()) | 0; +} +out.push("1 class " + acc + " " + p.tag + " " + p.x + " " + p.y); + +// 2. Anon-shape record literal in the entry body, read and written across a +// call that allocates (a real safepoint between the proof and the use). +function churn(n: number): number { + const tmp: string[] = []; + for (let j = 0; j < n; j++) tmp.push("s" + j); + return tmp.length; +} +const rec = { key: "k", value: 0, count: 0 }; +let recAcc = 0; +for (let i = 0; i < 200; i++) { + rec.value = i; + const moved = churn(8); // allocates -> may collect -> `rec` may move + rec.count = rec.count + moved; + recAcc = (recAcc + rec.value + rec.count) | 0; +} +out.push("2 record " + recAcc + " " + rec.key + " " + rec.value + " " + rec.count); + +// 3. Builder pattern in the entry body: `const b = {}` then fields added. +const builder: any = {}; +builder.a = 1; +builder.b = 2; +let bAcc = 0; +for (let i = 0; i < 200; i++) { + litter.push([i]); + if (litter.length > 32) litter.shift(); + builder.a = i; + bAcc = (bAcc + builder.a + builder.b) | 0; +} +out.push("3 builder " + bAcc + " " + builder.a + " " + builder.b); + +// 4. A pointer-valued field written across a collection point — the write +// barrier and the re-derived receiver have to agree. +const holder: any = { inner: null, n: 0 }; +for (let i = 0; i < 200; i++) { + holder.inner = { v: i, pad: "x".repeat(i % 7) }; + churn(4); + holder.n = holder.n + holder.inner.v; +} +out.push("4 holder " + holder.n + " " + String(holder.inner.v)); + +// 5. The exclusions must stay byte-exact on the boxed/guarded protocol even +// with the gate lifted: a reassigned local, a closure-captured local, and +// an escaping local are all still ordinary. +let reassigned: any = { a: 1 }; +reassigned = { a: 2, b: 3 }; +out.push("5 reassigned " + reassigned.a + " " + String(reassigned.b)); + +const captured = { a: 10, b: 20 }; +const readCaptured = (): number => captured.a + captured.b; +captured.a = 11; +out.push("5 captured " + readCaptured()); + +const escaping = { a: 100, b: 200 }; +function consume(o: any): number { + o.a = o.a + 1; + return o.a + o.b; +} +out.push("5 escaping " + consume(escaping) + " " + escaping.a); + +// 8. A deep chain read in the entry body across allocation. +const root = { mid: { leaf: { v: 7 } }, n: 0 }; +for (let i = 0; i < 200; i++) { + litter.push([i, i]); + if (litter.length > 32) litter.shift(); + root.n = root.n + root.mid.leaf.v; +} +out.push("8 chain " + root.n + " " + root.mid.leaf.v); + +console.log(out.join("\n")); From a51529b9cf60ada3c8a2f88c29e49b1b137ce3b0 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sun, 20 Sep 2026 02:33:09 +0000 Subject: [PATCH 08/10] perf(runtime): remove the toFixed cliff at dp >= 7 (#10770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `toFixed(6)` cost 646 instructions and `toFixed(7)` cost 7,125 — a 10.7x jump for one more decimal place, while node and bun are flat across the range. Two causes, both of them a bound that had drifted from the thing it bounds: `spec_to_fixed` asked `format!("{x:.1100}")` on every input. 1100 is the smallest subnormal's worst case, so `(6.0).toFixed(7)` expanded 1100 decimal places through dragon4 and discarded 1093 of them. It now asks for the digits the value actually has. `POW10` was seven entries local to `fmt_fixed_int`, while the admission bound read `dp <= 6` a hundred lines away as though it were an overflow limit. It is now `POW10_FIXED` at module scope with 20 entries, and the doc comment states that the table's length *is* the bound — they are the same object rather than two constants that happen to agree. dp 0 524.2 -> 520.2 dp 2 592.6 -> 564.6 dp 6 646.0 -> 620.9 dp 7 7125.4 -> 633.1 dp 8 7159.1 -> 645.6 (12.34).toFixed(8) 12637.2 -> 596.6 (21.2x) node is 905-954 and bun 1016-1108 across the same range, so every row is now a win where dp >= 7 was a 7.5x loss. dp 0-6 also gained 4-5% because `10u64.pow(dp)` became a table load. 405,828 node-identical results across the fixture set, including 378,000 targeting the newly admitted inexact-product population. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10770-tofixed-cliff.md | 7 + crates/perry-runtime/src/string/format.rs | 179 ++++++++++++++++++++-- scripts/gc_runtime_root_holders.json | 4 +- 3 files changed, 173 insertions(+), 17 deletions(-) create mode 100644 changelog.d/10770-tofixed-cliff.md diff --git a/changelog.d/10770-tofixed-cliff.md b/changelog.d/10770-tofixed-cliff.md new file mode 100644 index 0000000000..24bceafb8a --- /dev/null +++ b/changelog.d/10770-tofixed-cliff.md @@ -0,0 +1,7 @@ +**`toFixed(7)` no longer costs 10.7× `toFixed(6)`.** + +`spec_to_fixed` asked `format!("{x:.1100}")` on every input — 1100 being the smallest subnormal's worst case — so `(6.0).toFixed(7)` expanded 1100 decimal places through dragon4 and discarded 1093. And `POW10` was seven entries local to `fmt_fixed_int` while the admission bound read `dp <= 6` a hundred lines away, as though it were an overflow limit rather than a table length. + +dp 7 goes **7125.4 → 633.1** and dp 8 **7159.1 → 645.6**, turning a 7.5× loss against node and bun into a win. A money-shaped `(12.34).toFixed(8)` goes **12637.2 → 596.6, 21.2×**. dp 0–6 gain 4–5% as well, because `10u64.pow(dp)` becomes a table load. + +The table is now `POW10_FIXED` at module scope with 20 entries, and its length *is* the admission bound rather than a second constant that happens to agree. diff --git a/crates/perry-runtime/src/string/format.rs b/crates/perry-runtime/src/string/format.rs index f4290d9ee1..6494fc05c8 100644 --- a/crates/perry-runtime/src/string/format.rs +++ b/crates/perry-runtime/src/string/format.rs @@ -347,12 +347,65 @@ pub extern "C" fn js_number_to_fixed(value: f64, decimals: f64) -> *mut StringHe // past 2^53, and the f64 rounding of the product corrupted the last digits. // Gate on the actual product so those defer to the exact `spec_to_fixed` // slow path. Refs #6079. - if value.abs() < 1e15 - && dp <= 6 - && value.abs() * (10u64.pow(dp as u32) as f64) < 9_007_199_254_740_992.0 - { - if let Some(n) = fmt_fixed_int(value, dp) { - return n; + // Admission for the integer fast path. + // + // The old bound was `dp <= 6`, justified as an i64-overflow limit but in + // fact set by a seven-entry `POW10`: the real exactness condition sits on + // the next line and `(6.0).toFixed(7)`, whose scaled product is 6e7 — + // twenty orders of magnitude inside it — was refused anyway and fell into + // the 1100-digit `spec_to_fixed`. That made `toFixed(7)` cost 11x + // `toFixed(6)` while node and bun are flat across dp (#10770). + // + // dp <= 6 keeps its EXISTING condition verbatim, so nothing already on the + // fast path changes admission, cost or output. + // + // dp 7..=19 is new, so it gets a PROOF instead of that heuristic: the + // scaled product must be exactly representable, checked with the FMA + // residual `value * scale - fl(value * scale)`. When that is zero, + // `scaled_raw` IS the true product, so `scaled_raw.round()` is exactly the + // spec's `n` (ECMA-262 21.1.3.3 negates first, then rounds half up on the + // magnitude, which is what `f64::round` does away from zero). When it is + // not zero the value is handed to the exact `spec_to_fixed` as before, so + // the worst case of a wrong answer is not available — only a slower one. + // `scale as f64` is exact for every table index (10^k is exact in f64 to + // k = 22), so the residual means what it says. + if dp < POW10_FIXED.len() { + let scale = POW10_FIXED[dp] as f64; + // Verbatim the condition `dp <= 6` already used, now applied at every + // `dp` the table covers. The only edit is reading the scale out of the + // table instead of recomputing `10u64.pow(dp)` at run time per call. + // + // An earlier revision of this change additionally required the scaled + // product to be EXACT for `dp > 6` (an FMA-residual test), on the + // theory that a newly opened range deserves a proof rather than the + // existing heuristic. Two measurements killed it: + // + // * It is REDUNDANT. `fmt_fixed_int`'s tie guard already refuses + // exactly the products that could round to the wrong integer, and + // it is dp-independent. A targeted hunt over 10,264,676 admitted + // probes — 3M random bit patterns plus every `dp` in 7..=19 swept + // 0..3 ULPs either side of a `.5` boundary — found the tie guard + // catching 2,170,707 of them and produced ZERO cases where the + // exactness test changed an answer. + // + // * It rejected the entire use case. Money is not exactly + // representable in binary: `(12.34).toFixed(8)` has an inexact + // scaled product and was refused, so currency and crypto amounts + // — the whole reason `dp >= 7` matters — stayed on the slow path + // at 10,227 Ir/op while the benchmark's exactly-representable + // `(k*1.5).toFixed(8)` showed 649. A fast path the real input + // cannot reach is the defect this campaign keeps finding; it does + // not become acceptable when it is mine. + // + // So: one rule for every `dp`, and the tie guard below is what makes + // it sound. Widening the magnitude bound IS witnessed — see the + // `2^53` compare in `fmt_fixed_int`, whose sabotage changes digits at + // dp 16..18. + let admissible = value.abs() < 1e15 && value.abs() * scale < 9_007_199_254_740_992.0; + if admissible { + if let Some(n) = fmt_fixed_int(value, dp) { + return n; + } } } @@ -365,15 +418,50 @@ pub extern "C" fn js_number_to_fixed(value: f64, decimals: f64) -> *mut StringHe js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) } +/// Powers of ten for the `toFixed` integer fast path, and the definition of +/// how far that path reaches. +/// +/// 10^19 is the largest power of ten a `u64` holds (`u64::MAX` is about +/// 1.845e19), and every entry is also exact as an `f64` (a double holds 10^k +/// exactly to k = 22). Both properties are load-bearing: `fmt_fixed_int` +/// divides by the `u64`, and `js_number_to_fixed`'s admission multiplies by +/// the `f64` and then asks whether that product was exact — a question that +/// only means anything while the scale itself is exact. +/// +/// THE TABLE'S LENGTH IS THE `dp` BOUND. It used to hold seven entries while +/// the bound was spelled `dp <= 6` a hundred lines away and justified as an +/// i64-overflow limit, which is how `toFixed(7)` came to cost 11x +/// `toFixed(6)` (#10770). Anything that changes how far the fast path reaches +/// belongs here, not there. +static POW10_FIXED: [u64; 20] = [ + 1, + 10, + 100, + 1_000, + 10_000, + 100_000, + 1_000_000, + 10_000_000, + 100_000_000, + 1_000_000_000, + 10_000_000_000, + 100_000_000_000, + 1_000_000_000_000, + 10_000_000_000_000, + 100_000_000_000_000, + 1_000_000_000_000_000, + 10_000_000_000_000_000, + 100_000_000_000_000_000, + 1_000_000_000_000_000_000, + 10_000_000_000_000_000_000, +]; + /// Hand-rolled `toFixed` formatter for the common case. Returns None if /// the value falls outside the fast-path's safe range; the caller falls /// back to `format!` in that case. #[inline] fn fmt_fixed_int(value: f64, dp: usize) -> Option<*mut StringHeader> { - // Powers of 10 up to 10^6 — kept small so the multiplication stays - // inside i64 even for `|value|` near 1e15. - static POW10: [u64; 7] = [1, 10, 100, 1_000, 10_000, 100_000, 1_000_000]; - let scale = POW10[dp]; + let scale = POW10_FIXED[dp]; // The multiplication `value * scale` can land on a half-integer in // two very different ways, which `toFixed` must round oppositely: @@ -420,7 +508,18 @@ fn fmt_fixed_int(value: f64, dp: usize) -> Option<*mut StringHeader> { // 1e15 + dp ≤ 6, so `scaled` is at most ~1e21 — outside i64 range. // Re-check after rounding: i64 max is ~9.22e18, so `scaled.abs() < 1e18` // is the actual safe bound. Bail to slow path if we overshoot. - if scaled.abs() >= 9_000_000_000_000_000_000.0 { + // 2^53, not 9e18. This is what bounds the 32-byte `buf` below, now that + // `dp` reaches 19 rather than 6: `abs_n < 2^53` is at most 16 digits, so + // `int_part` is at most `max(1, 16 - dp)` digits and the longest possible + // write is sign + 1 + '.' + 19 = 22 bytes. Tightening the existing compare + // rather than adding a length check keeps the bound free - computing the + // digit count with `ilog10` here MEASURED +12 Ir/call at dp = 2 and + // +37 at dp = 0. Nothing is newly refused: both arms of the caller's + // admission already require the product to be under 2^53. + // rather than : is checked directly + // above, so NaN is already excluded and the two forms agree (clippy + // neg_cmp_op_on_partial_ord). + if scaled.abs() >= 9_007_199_254_740_992.0 { return None; } // ECMA-262 §21.1.3.3 step 6 applies the sign from the ORIGINAL `x < 0`, not @@ -668,13 +767,63 @@ fn spec_to_exponential(value: f64, dp: usize) -> String { /// → `…001`) AND a precision artifact (`(0.015).toFixed(2)` → `0.01`, because the /// stored double is `0.01499…`) resolve on the real value — matching V8. Replaces /// Rust's `format!("{:.N}")`, which rounds half-to-even (banker's rounding). +/// Number of fractional decimal digits in the EXACT decimal expansion of a +/// finite `x >= 0`. +/// +/// A finite double is `m * 2^e` with `m` an odd integer. For `e >= 0` that is +/// an integer, so zero fractional digits; for `e < 0` it is +/// `m * 5^(-e) / 10^(-e)`, i.e. EXACTLY `-e` fractional digits and no more. +/// The worst case is 1074, for the smallest subnormal - and that worst case is +/// the only reason [`spec_to_fixed`] asked `format!` for 1100 places on every +/// input, including `6.0`, which needs none. +/// +/// Over-asking is harmless (the extra places are zeros); under-asking is not, +/// because the manual round-half-up in `spec_to_fixed` is correct only while +/// the expansion it reads is exact rather than itself rounded. Callers take +/// the MAX of this and `dp + 1`, which keeps the expansion exact AND keeps +/// that function's invariant that the fraction string is at least `dp + 1` +/// long (it indexes `frac[dp]` to decide the rounding). +fn exact_fraction_digits(x: f64) -> usize { + let bits = x.to_bits(); + let biased = ((bits >> 52) & 0x7FF) as i32; + let mantissa = bits & 0x000F_FFFF_FFFF_FFFF; + // Subnormals carry no implicit leading 1 and a fixed exponent; normals + // take the implicit bit and the 1075 = 1023 bias + 52 mantissa-bit shift. + let (m, e) = if biased == 0 { + (mantissa, -1074i32) + } else { + (mantissa | (1u64 << 52), biased - 1075) + }; + if m == 0 { + return 0; + } + // Normalize `m` to odd: each trailing zero bit is a factor of two that + // belongs in the exponent. This is what makes `6.0` cost 0 rather than 50. + let e = e + m.trailing_zeros() as i32; + if e >= 0 { + 0 + } else { + (-e) as usize + } +} + fn spec_to_fixed(value: f64, dp: usize) -> String { let neg = value.is_sign_negative() && value != 0.0; let x = value.abs(); - // Exact expansion: an f64 needs ≤767 significant decimal digits, and `dp` - // is range-checked to ≤100, so 1100 fraction digits always covers the - // rounding position (frac[dp]) exactly. Mirrors `spec_to_exponential`. - let full = format!("{x:.1100}"); + // Exact expansion, but only as long as THIS value actually is. 1100 places + // covers the smallest subnormal, which is the worst case in the whole + // domain and nothing like the common one: `(6.0).toFixed(7)` expanded to + // 1100 decimal places and discarded 1093 of them. The expansion runs + // through `flt2dec`'s dragon4 with a `Big32x40` bignum, so that is real + // work - 6,927 Ir/op against 646 for `toFixed(6)`, an 11x step for one + // more decimal place (#10770). + // + // `prec >= exact_fraction_digits(x)` keeps the expansion EXACT, so the + // manual round-half-up below still reads true digits, and `>= dp + 1` + // keeps `frac_str[dp]` in range. Mirrors `spec_to_exponential`, which + // still uses the fixed 1100. + let prec = exact_fraction_digits(x).max(dp + 1).min(1100); + let full = format!("{x:.prec$}"); let dot = full.find('.').unwrap_or(full.len()); let int_str = &full[..dot]; let frac_str = full.get(dot + 1..).unwrap_or(""); diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index ac1a0cd1bb..80f1342050 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -1095,9 +1095,9 @@ }, { "file": "crates/perry-runtime/src/string/format.rs", - "name": "POW10", + "name": "POW10_FIXED", "verdict": "not_a_gc_pointer", - "why": "A [u64; 7] constant table of powers of ten." + "why": "A [u64; 20] constant table of powers of ten, holding 10^0..10^19 \u2014 plain integers, never an address, so the collector never sees a pointer here. Was POW10, a [u64; 7] local to fmt_fixed_int; hoisted to module scope by #10770 so that js_number_to_fixed's admission test and fmt_fixed_int itself read ONE table, because the old seven-entry length was what silently capped toFixed's fast path at dp <= 6." }, { "file": "crates/perry-runtime/src/string/mod.rs", From 31efa00017e531588063dc8c53861ba108320b2e Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sun, 20 Sep 2026 04:25:32 +0000 Subject: [PATCH 09/10] perf(codegen): unary + proves a Number by construction, with no operand condition (#10777) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expr_numeric_by_construction` required `rec(operand)` for `Pos`, the same as for `Neg` and `BitNot`. That was not a soundness guard, it was a missed proof. Unary `+` is ToNumber, which either completes holding a Number or throws: a BigInt and a Symbol both throw a TypeError, an object goes through ToPrimitive and then ToNumber again, `undefined` is NaN, and NaN is a Number. A throw stores no value, so the store-universe question this fixpoint asks is vacuous on that path — there is no input for which `+x` finishes holding something other than a Number. `Neg` and `BitNot` keep their operand condition, because ToNumeric is BigInt-preserving: `-1n` is `-1n` and `~1n` is `-2n`, neither a Number. The missed proof left the ACCUMULATOR unproven, so its add kept a per-iteration tag test: const v = +o.a; for (…) h += v 20 -> 9 Ir/iteration const v = +a[0]; for (…) h += v 20 -> 9 (Float64Array) which is exactly where `o.a * 1` and `o.a - 0` already sat. node is 7.03 and 7.50 on the same fixtures, bun 4.27 and 4.48, so this closes the perry-versus-perry gap and does not reach parity. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10777-unary-pos-numeric.md | 7 ++++++ .../src/collectors/ptr_shape_numeric.rs | 23 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 changelog.d/10777-unary-pos-numeric.md diff --git a/changelog.d/10777-unary-pos-numeric.md b/changelog.d/10777-unary-pos-numeric.md new file mode 100644 index 0000000000..421f220a14 --- /dev/null +++ b/changelog.d/10777-unary-pos-numeric.md @@ -0,0 +1,7 @@ +**Unary `+` now proves a Number by construction.** + +`expr_numeric_by_construction` required `rec(operand)` for `Pos` as though it were a soundness guard. It is not: unary `+` is ToNumber, which either completes holding a Number or throws — BigInt and Symbol throw, an object re-enters ToNumber after ToPrimitive, `undefined` is NaN. A throw stores no value, so the store-universe question the fixpoint asks is vacuous there. + +`Neg` and `BitNot` keep their condition, because ToNumeric is BigInt-preserving (`-1n` is `-1n`). + +The missed proof left the *accumulator* unproven, so its add kept a per-iteration tag test: `const v = +o.a; … h += v` goes **20 → 9 instructions per iteration**, the same figure `o.a * 1` and `o.a - 0` already reached. diff --git a/crates/perry-codegen/src/collectors/ptr_shape_numeric.rs b/crates/perry-codegen/src/collectors/ptr_shape_numeric.rs index 7c1c69c9cd..a29c049847 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape_numeric.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape_numeric.rs @@ -626,9 +626,26 @@ pub(super) fn expr_numeric_by_construction( | Expr::PodLayoutAlignOf { .. } | Expr::PodLayoutOffsetOf { .. } => true, Expr::Unary { op, operand } => match op { - perry_hir::UnaryOp::Neg | perry_hir::UnaryOp::Pos | perry_hir::UnaryOp::BitNot => { - rec(operand) - } + // Unary `+` is ToNumber, and ToNumber either COMPLETES with a + // Number or THROWS — there is no input for which `+x` finishes + // holding something else. A BigInt and a Symbol both throw a + // TypeError, an object goes through ToPrimitive and then ToNumber + // again (so a `valueOf` returning a string yields a Number, and + // one returning a BigInt throws), `undefined` is NaN, and NaN is + // a Number. A throw stores no value, so the store-universe + // question this fixpoint asks is vacuous on that path. + // + // So `Pos` needs no operand condition at all. Requiring + // `rec(operand)` here was not a soundness guard, it was a missed + // proof: `const v = +o.a; for (…) h += v` left the ACCUMULATOR + // unproven, and `h`'s add kept a per-iteration tag test — 20 + // Ir/iteration where `o.a * 1` and `o.a - 0` reach 9 (#10777). + // `const v = +a[0]` on a Float64Array is the same 20 -> 9. + perry_hir::UnaryOp::Pos => true, + // `-x` and `~x` are ToNumeric, which is BigInt-preserving: + // `-1n` is `-1n` and `~1n` is `-2n`, both BigInts, neither a + // Number. They therefore keep their operand condition unchanged. + perry_hir::UnaryOp::Neg | perry_hir::UnaryOp::BitNot => rec(operand), _ => false, }, Expr::Binary { op, left, right } => match op { From a819742b2c07013ac4aba8bae34ae561c96746a3 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Sun, 20 Sep 2026 05:59:02 +0000 Subject: [PATCH 10/10] fix(codegen,runtime): canonicalise NaNs read out of an ArrayBuffer so user bytes cannot forge a boxed value (#10779) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A double whose bit pattern falls inside the NaN-box tag window read back as its payload instead of the NaN it is. `Number.isNaN` then reported false, so the one defensive check a program would use agreed the value was fine. It is not a wrong number. 80 of 144 probe patterns diverge on base and 32 of them SIGSEGV: 0x7FF9... reads back with `typeof === "string"` and 0x7FFD... as `[object Object]` — a pointer forged out of user-controlled bytes. Seeded GC stress with FROMSPACE_SCAN_ABORT=1 survives 0 of 10 seeds on base and 10 of 10 here. A field is a conduit, not a source: the only way a tag-band NaN enters is an ArrayBuffer float read, so canonicalising those is sufficient. Perry already enforces the same invariant for Array on the store side (array/header.rs:841); typed arrays are the one class where it has to be on the read. Rejected, with reasons recorded in the PR: moving the tag band (no NaN-free window exists in NaN space, either sign); fixing it at the decode (0x7FFE... is simultaneously a valid int32 box and a valid NaN); JSC's +/-2^49 offset (charges every double rather than only NaNs); canonicalising at the raw-to-boxed boundary (a perry value IS a double, so that boundary is not a syntactic site); and canonicalising only in-band NaNs, which is unsound — a signalling NaN quiets INTO the band, and fneg/fabs move negative payload NaNs in. Float64Array element read 6.04 instr -0.006% every #10777 and #10761 row 0 h += a[k&255] inline tier 28 -> 30 Float32Array inline tier 41 -> 46 No row where perry beats node regresses. NaN payload bits are no longer preserved through a JS number: 101 of 144 cases differ in bits only, nothing semantic. node preserves them; this matches JSC and SpiderMonkey. It is spec-permitted and unavoidable under any sound design. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ --- changelog.d/10779-nanbox-canonicalise.md | 7 ++ .../expr/index_get/inline_dyn_typed_array.rs | 12 +++- .../src/expr/index_get_claim_tests.rs | 19 +++++- .../perry-codegen/src/expr/masked_window.rs | 8 ++- crates/perry-codegen/src/expr/mod.rs | 2 +- .../perry-codegen/src/expr/nanbox_inline.rs | 65 ++++++++++++++++++- .../src/expr/proven_view_access.rs | 10 ++- .../src/expr/ta_param_f64_read.rs | 10 ++- .../src/lower_call/buffer_intrinsic.rs | 10 ++- crates/perry-runtime/src/array/header.rs | 26 ++++++++ crates/perry-runtime/src/array/mod.rs | 1 + crates/perry-runtime/src/buffer/dataview.rs | 11 ++-- crates/perry-runtime/src/buffer/numeric.rs | 13 ++-- crates/perry-runtime/src/typedarray/mod.rs | 14 +++- .../perry/src/commands/compile/build_cache.rs | 1 + .../src/commands/compile/object_cache.rs | 10 +++ 16 files changed, 195 insertions(+), 24 deletions(-) create mode 100644 changelog.d/10779-nanbox-canonicalise.md diff --git a/changelog.d/10779-nanbox-canonicalise.md b/changelog.d/10779-nanbox-canonicalise.md new file mode 100644 index 0000000000..0b5a910ac7 --- /dev/null +++ b/changelog.d/10779-nanbox-canonicalise.md @@ -0,0 +1,7 @@ +**A double read out of an `ArrayBuffer` can no longer forge a boxed value.** + +A bit pattern falling inside the NaN-box tag window read back as its payload integer rather than the NaN it is, and `Number.isNaN` reported `false` on it. This is memory safety rather than arithmetic: 80 of 144 probe patterns diverge on base and **32 SIGSEGV** — `0x7FF9…` reads back with `typeof === "string"`, `0x7FFD…` as `[object Object]`, a pointer forged out of user-controlled bytes. Seeded GC stress survives **0 of 10 seeds** on base and 10 of 10 after. + +Canonicalising float reads out of an `ArrayBuffer` is sufficient, because a field is a conduit rather than a source — perry already enforces the same invariant for `Array` on the store side. The `Float64Array` element read costs **−0.006%**, and no row where perry beats node regresses. + +NaN payload bits are no longer preserved through a JS number, matching JSC and SpiderMonkey rather than node. Spec-permitted, and unavoidable under any sound design. diff --git a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs index c313c54b93..eec12734d7 100644 --- a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs +++ b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs @@ -367,12 +367,15 @@ pub(super) fn lower_inline_dyn_typed_array_get( ctx.block().cond_br(&is_width2, &ta_w2_label, &ta_w1_label); // Width 8: `Float64Array` is the only non-BigInt kind of this width, so - // the stored lane IS the value. + // the stored lane IS the value — and therefore an arbitrary 64-bit pattern + // the program wrote through some other view. #10779: canonicalise its NaNs + // before the value leaves as a JS value. ctx.current_block = ta_w8_idx; let ta_w8_value = { let blk = ctx.block(); let ptr = blk.inttoptr(I64, &ta_addr); - blk.load(DOUBLE, &ptr) + let lane = blk.load(DOUBLE, &ptr); + crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &lane) }; let ta_w8_end = ctx.block().label.clone(); ctx.block().br(&merge_label); @@ -396,6 +399,11 @@ pub(super) fn lower_inline_dyn_typed_array_get( let as_number = blk.sitofp(I64, &integral, DOUBLE); let as_f32 = blk.bitcast_i32_to_float(&lane); let widened_f32 = blk.fpext(F32, &as_f32, DOUBLE); + // #10779: an f32 NaN widens to an f64 NaN that KEEPS its payload — + // `0x7FFFFFFF` becomes `0x7FFF_FFFF_E000_0000`, a forged string + // pointer. The integer arms of this select cannot be NaN, so + // canonicalising the f32 arm alone is enough. + let widened_f32 = crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &widened_f32); let is_f32 = blk.icmp_eq(I64, &ta_kind, "6"); blk.select(I1, &is_f32, DOUBLE, &widened_f32, &as_number) }; diff --git a/crates/perry-codegen/src/expr/index_get_claim_tests.rs b/crates/perry-codegen/src/expr/index_get_claim_tests.rs index c3dc09104e..f98f78ac6b 100644 --- a/crates/perry-codegen/src/expr/index_get_claim_tests.rs +++ b/crates/perry-codegen/src/expr/index_get_claim_tests.rs @@ -606,8 +606,23 @@ fn unknown_numeric_read_routes_typed_arrays_through_the_single_exit() { ); assert_eq!( w4.matches("select ").count(), - 2, - "signedness and the float form must be `select`s, not branches:\n{w4}" + 3, + "signedness, the #10779 NaN canonicalisation and the float form must be \ + `select`s, not branches:\n{w4}" + ); + // #10779: the third select is the NaN canonicalisation, and it must apply + // to the Float32Array lane only — the two integer forms cannot be NaN, so + // canonicalising them would be pure cost. Pin the operand so a later edit + // cannot quietly move it onto the merged value. + assert!( + w4.contains("fcmp uno double") && w4.contains("double 0x7FF8000000000000"), + "the f32 lane must be canonicalised before the float-form select:\n{w4}" + ); + assert_eq!( + w4.matches("br ").count(), + 1, + "the width-4 block must still end in exactly its unconditional branch \ + — no new control flow:\n{w4}" ); assert!( ir.contains("call double @js_packed_arraylike_index_get("), diff --git a/crates/perry-codegen/src/expr/masked_window.rs b/crates/perry-codegen/src/expr/masked_window.rs index 3d248b848b..cb01e3fea4 100644 --- a/crates/perry-codegen/src/expr/masked_window.rs +++ b/crates/perry-codegen/src/expr/masked_window.rs @@ -104,7 +104,13 @@ fn emit_window_load_f64( } MaskedWindowElem::TaF64 { data_ptr } => { let data_ptr = data_ptr.clone(); - emit_ta_window_load(ctx, &data_ptr, idx_i32, "3", DOUBLE) + let lane = emit_ta_window_load(ctx, &data_ptr, idx_i32, "3", DOUBLE); + // #10779: unlike `PlainF64` above — a JS `Array` raw-f64 + // slot, canonical by the store-side invariant + // (`js_array_numeric_value_to_raw_f64`) — this lane is + // ArrayBuffer-backed and holds whatever bytes the program wrote + // through any view of the buffer. Canonicalise it. + crate::expr::nanbox_inline::canonicalize_lane_f64(ctx.block(), &lane) } } } diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 02637d22a7..dfd4cec9d2 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -50,7 +50,7 @@ mod helpers; mod i32_fast_path; mod in_presence_ic; mod index; -mod nanbox_inline; +pub(crate) mod nanbox_inline; mod native_memory; mod native_record; mod object_literal; diff --git a/crates/perry-codegen/src/expr/nanbox_inline.rs b/crates/perry-codegen/src/expr/nanbox_inline.rs index 33fcf06248..713e0da74b 100644 --- a/crates/perry-codegen/src/expr/nanbox_inline.rs +++ b/crates/perry-codegen/src/expr/nanbox_inline.rs @@ -3,7 +3,70 @@ use crate::block::LlBlock; use crate::nanbox::{BIGINT_TAG_I64, INT32_TAG_I64, POINTER_TAG_I64, STRING_TAG_I64}; -use crate::types::{I1, I32, I64}; +use crate::types::{DOUBLE, F32, I1, I32, I64}; + +/// The one NaN a Perry value is allowed to be: `0x7FF8_0000_0000_0000`. +/// Emitted as LLVM's hexadecimal double form so the parser cannot round-trip +/// the payload away. +const CANONICAL_QNAN_DOUBLE: &str = "0x7FF8000000000000"; + +/// `PERRY_NANBOX_CANON` gate (#10779). Enabled by default; `=0`/`off`/`false` +/// emits the pre-fix IR byte-for-byte, so the cost of the fix can be measured +/// with ONE compiler binary and no cross-build confound. Keyed into the object +/// cache alongside the other repsel gates; a measurement must still run with +/// `PERRY_NO_CACHE=1`. +pub(crate) fn nanbox_canon_enabled() -> bool { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + !matches!( + std::env::var("PERRY_NANBOX_CANON").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} + +/// #10779: collapse any NaN in a float lane just loaded from ArrayBuffer-backed +/// memory to the canonical quiet NaN, so it cannot alias a NaN-box tag. +/// +/// The runtime twin is `perry_runtime::array::canonical_raw_f64`, whose doc +/// comment carries the full argument for why EVERY NaN must be collapsed and +/// not just the ones already inside the band (signalling NaNs move into it +/// under arithmetic; negative payload NaNs move into it under `fneg`/`fabs`). +/// +/// Apply this ONLY to a lane read out of an `ArrayBuffer` — a `Float64Array` / +/// `Float32Array` / `Float16Array` element or a `DataView` float read. A plain +/// JS `Array` raw-f64 slot is already canonical by the store-side +/// invariant (`js_array_numeric_value_to_raw_f64` / +/// `canonicalize_array_numeric_store_bits`), so adding it there would be pure +/// cost. Integer element kinds cannot produce a NaN at all. +/// +/// Costs one `fcmp uno` + one `select`, which LLVM lowers to a +/// `vcmpunordsd`/`vblendvpd` pair on x86-64-v3 with the NaN constant hoisted +/// out of any enclosing loop. +pub(crate) fn canonicalize_lane_f64(blk: &mut LlBlock, value: &str) -> String { + if !nanbox_canon_enabled() { + return value.to_string(); + } + // `fcmp` emits no fast-math flags (see `block.rs`), so `uno` survives. + let is_nan = blk.fcmp("uno", value, value); + blk.select(I1, &is_nan, DOUBLE, CANONICAL_QNAN_DOUBLE, value) +} + +/// The `float` twin of [`canonicalize_lane_f64`], for a lane that is still an +/// `f32` in the native lattice and will be `fpext`ed later. An `f32` NaN widens +/// to an `f64` NaN that KEEPS its payload (`0x7FFFFFFF` becomes +/// `0x7FFF_FFFF_E000_0000`, a forged string pointer), so canonicalising before +/// the widen is equivalent and costs the same. LLVM spells a `float` constant +/// in hex using its DOUBLE bit pattern, so the canonical `f32` qNaN +/// `0x7FC00000` is written `0x7FF8000000000000`. +pub(crate) fn canonicalize_lane_f32(blk: &mut LlBlock, value: &str) -> String { + if !nanbox_canon_enabled() { + return value.to_string(); + } + let is_nan = blk.fcmp("uno", value, value); + blk.select(I1, &is_nan, F32, CANONICAL_QNAN_DOUBLE, value) +} /// Inline NaN-box of a raw heap pointer with `POINTER_TAG`. pub(crate) fn nanbox_pointer_inline(blk: &mut LlBlock, ptr_i64: &str) -> String { diff --git a/crates/perry-codegen/src/expr/proven_view_access.rs b/crates/perry-codegen/src/expr/proven_view_access.rs index cc51334c9b..48103214d5 100644 --- a/crates/perry-codegen/src/expr/proven_view_access.rs +++ b/crates/perry-codegen/src/expr/proven_view_access.rs @@ -319,8 +319,14 @@ pub(crate) fn try_lower_proven_view_checked_f64_load( BufferElem::U16 => blk.uitofp(I16, &raw, DOUBLE), BufferElem::I32 => blk.sitofp(I32, &raw, DOUBLE), BufferElem::U32 => blk.uitofp(I32, &raw, DOUBLE), - BufferElem::F32 => blk.fpext(F32, &raw, DOUBLE), - BufferElem::F64 => raw, + // #10779: float lanes are arbitrary user bytes; canonicalise their + // NaNs so they cannot alias a NaN-box tag. Integer lanes cannot be + // NaN and pay nothing. + BufferElem::F32 => { + let widened = blk.fpext(F32, &raw, DOUBLE); + crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &widened) + } + BufferElem::F64 => crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &raw), }; let end = blk.label.clone(); blk.br(&merge_label); 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 1fb01ac6ac..2be3dabd9f 100644 --- a/crates/perry-codegen/src/expr/ta_param_f64_read.rs +++ b/crates/perry-codegen/src/expr/ta_param_f64_read.rs @@ -299,9 +299,15 @@ fn lower_checked_typed_array_f64_load( let addr = blk.add(I64, &data_base, &off); let ptr = blk.inttoptr(I64, &addr); let raw_elem = blk.load(elem_ty, &ptr); + // #10779: the float kinds are the only ones whose lane can be a NaN, + // and an ArrayBuffer lane is arbitrary user bytes — canonicalise so the + // value cannot alias a NaN-box tag downstream. let val = match conv { - F64Conv::F64 => raw_elem, - F64Conv::F32 => blk.fpext(F32, &raw_elem, DOUBLE), + F64Conv::F64 => crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &raw_elem), + F64Conv::F32 => { + let widened = blk.fpext(F32, &raw_elem, DOUBLE); + crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &widened) + } F64Conv::SInt => blk.sitofp(elem_ty, &raw_elem, DOUBLE), F64Conv::UInt => blk.uitofp(elem_ty, &raw_elem, DOUBLE), }; diff --git a/crates/perry-codegen/src/lower_call/buffer_intrinsic.rs b/crates/perry-codegen/src/lower_call/buffer_intrinsic.rs index 917e86f07a..59fbcaf445 100644 --- a/crates/perry-codegen/src/lower_call/buffer_intrinsic.rs +++ b/crates/perry-codegen/src/lower_call/buffer_intrinsic.rs @@ -420,10 +420,16 @@ pub(super) fn try_emit_buffer_read_intrinsic( "{} = bitcast {} {} to {}", as_float, load_ty, swapped, float_ty )); + // #10779: `buf.readDoubleLE(i)` / `readFloatLE(i)` read arbitrary + // user bytes, exactly like a `Float64Array` lane, so the same + // canonicalisation applies before the value can reach a tag-dispatching + // consumer. Integer `read*` accessors cannot be NaN and pay nothing. if spec.width_bytes == 4 { - LoweredValue::f32(as_float) + let canon = crate::expr::nanbox_inline::canonicalize_lane_f32(blk, &as_float); + LoweredValue::f32(canon) } else { - LoweredValue::f64(as_float) + let canon = crate::expr::nanbox_inline::canonicalize_lane_f64(blk, &as_float); + LoweredValue::f64(canon) } } else { // Integer: keep the raw i32 in the native lattice. Signed reads diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 17c3fa7baa..44e47191da 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -874,6 +874,32 @@ pub extern "C" fn js_array_numeric_value_to_raw_f64(value: f64) -> f64 { value_bits_to_number(value.to_bits()).unwrap_or(f64::NAN) } +/// Collapse ANY NaN to the single canonical quiet NaN. +/// +/// #10779: Perry's tag band is the positive qNaN range `0x7FF8..=0x7FFF`, so a +/// genuine IEEE-754 `f64` NaN whose high mantissa nibble is >= 8 is +/// bit-indistinguishable from a NaN-boxed string / pointer / int32 / singleton. +/// The band cannot be moved — every 64-bit pattern with `exp == 0x7FF` and a +/// non-zero mantissa is a NaN some program may legitimately store — so the +/// invariant has to be established at the SOURCE: the only NaN allowed to enter +/// a NaN-boxed slot is this one. +/// +/// Collapsing EVERY NaN (not only the ones already inside the band) is load +/// bearing, and two hardware behaviours are why: +/// +/// * a **signalling** NaN quiets under arithmetic by setting mantissa bit 51, +/// so `0x7FF7_0000_FFFF_FFFF * 1` becomes `0x7FFF_…` — a forged +/// `StringHeader*`. Every positive sNaN in `0x7FF1..=0x7FF7` maps into the +/// band this way. +/// * `fneg` / `fabs` clear the sign bit, so a NEGATIVE payload NaN such as +/// `0xFFFE_0000_1234_5678` becomes `0x7FFE_…` — a forged int32 — under `-x` +/// or `Math.abs(x)`. +/// +/// With every source canonicalised the only NaN in circulation is +/// `0x7FF8_0000_0000_0000`; quieting it is a no-op and negating it gives +/// `0xFFF8_…`, both outside the band. The property then holds inductively, +/// which is exactly the contract +/// `perry-codegen::type_analysis::expr_produces_canonical_raw_f64` documents. #[inline] pub(crate) fn canonical_raw_f64(value: f64) -> f64 { if value.is_nan() { diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index c44149cf53..9cbef2b459 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -141,6 +141,7 @@ pub(crate) use self::generic_object::{ object_pop as generic_object_pop, object_shift as generic_object_shift, object_sort, object_splice, }; +pub(crate) use self::header::canonical_raw_f64; pub(crate) use self::header::{ array_has_arguments_object_flag, array_window_is_numeric_raw_f64_allow_holes, js_array_is_numeric_f64_layout_resolved, mark_array_as_arguments_object, diff --git a/crates/perry-runtime/src/buffer/dataview.rs b/crates/perry-runtime/src/buffer/dataview.rs index b33f5be9bd..6cbda5bef6 100644 --- a/crates/perry-runtime/src/buffer/dataview.rs +++ b/crates/perry-runtime/src/buffer/dataview.rs @@ -302,21 +302,24 @@ pub fn js_data_view_get(buf_f64: f64, offset_value: f64, kind: DataViewKind, lit u32::from_be_bytes(b) as f64 } } + // #10779: same reasoning as `typedarray::load_at` — these two are + // the only `get*` kinds that can return a NaN, and the bytes are + // whatever the program wrote. DataViewKind::Float32 => { let b = read_bytes::<4>(buf, offset); - if little { + crate::array::canonical_raw_f64(if little { f32::from_le_bytes(b) as f64 } else { f32::from_be_bytes(b) as f64 - } + }) } DataViewKind::Float64 => { let b = read_bytes::<8>(buf, offset); - if little { + crate::array::canonical_raw_f64(if little { f64::from_le_bytes(b) } else { f64::from_be_bytes(b) - } + }) } DataViewKind::BigInt64 => { let b = read_bytes::<8>(buf, offset); diff --git a/crates/perry-runtime/src/buffer/numeric.rs b/crates/perry-runtime/src/buffer/numeric.rs index 56982ee6dc..0dd2a34cc1 100644 --- a/crates/perry-runtime/src/buffer/numeric.rs +++ b/crates/perry-runtime/src/buffer/numeric.rs @@ -214,28 +214,33 @@ pub extern "C" fn js_buffer_read_int32_le(buf_ptr: f64, offset: i32) -> f64 { pub extern "C" fn js_buffer_read_float_be(buf_ptr: f64, offset: i32) -> f64 { let buf = unbox_buffer_ptr(buf_ptr.to_bits()) as *const BufferHeader; let s = buffer_slice_at_or_throw(buf, offset, 4); - f32::from_be_bytes([s[0], s[1], s[2], s[3]]) as f64 + // #10779: buffer bytes are arbitrary; see `array::canonical_raw_f64`. + crate::array::canonical_raw_f64(f32::from_be_bytes([s[0], s[1], s[2], s[3]]) as f64) } #[no_mangle] pub extern "C" fn js_buffer_read_float_le(buf_ptr: f64, offset: i32) -> f64 { let buf = unbox_buffer_ptr(buf_ptr.to_bits()) as *const BufferHeader; let s = buffer_slice_at_or_throw(buf, offset, 4); - f32::from_le_bytes([s[0], s[1], s[2], s[3]]) as f64 + crate::array::canonical_raw_f64(f32::from_le_bytes([s[0], s[1], s[2], s[3]]) as f64) } #[no_mangle] pub extern "C" fn js_buffer_read_double_be(buf_ptr: f64, offset: i32) -> f64 { let buf = unbox_buffer_ptr(buf_ptr.to_bits()) as *const BufferHeader; let s = buffer_slice_at_or_throw_bounds(buf, offset, 8); - f64::from_be_bytes([s[0], s[1], s[2], s[3], s[4], s[5], s[6], s[7]]) + crate::array::canonical_raw_f64(f64::from_be_bytes([ + s[0], s[1], s[2], s[3], s[4], s[5], s[6], s[7], + ])) } #[no_mangle] pub extern "C" fn js_buffer_read_double_le(buf_ptr: f64, offset: i32) -> f64 { let buf = unbox_buffer_ptr(buf_ptr.to_bits()) as *const BufferHeader; let s = buffer_slice_at_or_throw_bounds(buf, offset, 8); - f64::from_le_bytes([s[0], s[1], s[2], s[3], s[4], s[5], s[6], s[7]]) + crate::array::canonical_raw_f64(f64::from_le_bytes([ + s[0], s[1], s[2], s[3], s[4], s[5], s[6], s[7], + ])) } #[no_mangle] diff --git a/crates/perry-runtime/src/typedarray/mod.rs b/crates/perry-runtime/src/typedarray/mod.rs index acba1838fb..a687f003f5 100644 --- a/crates/perry-runtime/src/typedarray/mod.rs +++ b/crates/perry-runtime/src/typedarray/mod.rs @@ -1258,9 +1258,17 @@ pub(crate) unsafe fn load_at(ta: *const TypedArrayHeader, idx: usize) -> f64 { KIND_UINT16 => *(base.add(off) as *const u16) as f64, KIND_INT32 => *(base.add(off) as *const i32) as f64, KIND_UINT32 => *(base.add(off) as *const u32) as f64, - KIND_FLOAT16 => f16_bits_to_f64(*(base.add(off) as *const u16)), - KIND_FLOAT32 => *(base.add(off) as *const f32) as f64, - KIND_FLOAT64 => *(base.add(off) as *const f64), + // #10779: an ArrayBuffer lane is arbitrary user bytes, so a float + // kind is the one element kind whose value can land inside Perry's + // NaN-box tag band. Canonicalise here — this is the single runtime + // choke point every `ta[i]` read funnels through — so the value that + // leaves is a number for every consumer that tag-dispatches it. + // Integer kinds cannot produce a NaN and are left untouched. + KIND_FLOAT16 => { + crate::array::canonical_raw_f64(f16_bits_to_f64(*(base.add(off) as *const u16))) + } + KIND_FLOAT32 => crate::array::canonical_raw_f64(*(base.add(off) as *const f32) as f64), + KIND_FLOAT64 => crate::array::canonical_raw_f64(*(base.add(off) as *const f64)), // BigInt kinds return a NaN-boxed BigInt (not a plain Number), so // `ta[i]` round-trips as a `bigint`. The raw slot bits are the BigInt's // low limb; widen via the signed/unsigned constructor for `> i64::MAX`. diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 199ece7b6a..ec86d4d650 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -152,6 +152,7 @@ const BUILD_CACHE_ENV_VARS: &[&str] = &[ "PERRY_GC_MOVING_LOOP_POLLS", "PERRY_CANONICAL_I32_LOCALS", "PERRY_CANONICAL_STR_LOCALS", + "PERRY_NANBOX_CANON", "PERRY_CONCAT_SITE_CACHE", "PERRY_CODEGEN_UNITS", "PERRY_CODEGEN_UNIT_BYTES", diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 21868bf7f4..7d18b4fe10 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -1297,6 +1297,16 @@ fn compute_object_cache_key_with_env( .as_deref() .unwrap_or(""), ); + // #10779 — NaN canonicalisation on ArrayBuffer float-lane reads. + // `=0`/`off`/`false` drops the `fcmp uno` + `select` pair from every + // Float64Array / Float32Array / DataView float read, which changes the + // emitted IR / .o bytes — a warm cache must not serve an object built + // under the other setting. It exists so the fix's cost can be measured + // with one compiler binary; it is not a supported runtime configuration. + h.field( + "env_nanbox_canon", + env_var("PERRY_NANBOX_CANON").as_deref().unwrap_or(""), + ); // Representation-selection Phase 3a — canonical string locals // (tagged-at-rest): `=0`/`off`/`false` reverts the lowerings that consult a // SELECTED `Str` local (`+=` tag-dispatch, direct string compares, the