diff --git a/changelog.d/10185-element-shape-fields-random.md b/changelog.d/10185-element-shape-fields-random.md new file mode 100644 index 0000000000..07b0533c87 --- /dev/null +++ b/changelog.d/10185-element-shape-fields-random.md @@ -0,0 +1,158 @@ +Serve the JSON access benchmark's `fields` and `random` shapes from the +element-shape versioned loop clone (#7480 / #5093 / #10171). Stacked on +#10171, which made the clone fire for `JSON.parse`'d record arrays at all. + +#10171 took the benchmark's `repeat` and `sequential` modes to parity. The two +remaining modes got nothing, for three separate reasons — and because a loop is +admitted as a whole, any one of them cost the entire clone: + +```ts +// random: a loop-carried index +cursor = (cursor * 17 + 7) % length; +const index = cursor; +sum += rows[index].id; + +// fields: three accumulator statements, one string read, one boolean ternary +const index = i % length; +sum += rows[index].id; +sum += rows[index].name.length; +sum += rows[index].active ? 1 : 0; +``` + +* **A loop-carried index** (`stmt/element_shape_carried.rs`). The recurrence is + folded to one affine pair `(a, b)` and evaluated as `srem i64` — the generic + `%` is an `frem`, which on aarch64 is a libm call, and a call inside this + clone does not slow it down, it DELETES it (#7690). Three obligations, all + discharged once in the preheader: the modulus materializes as an i32 in + `1..=i32::MAX` (so `m <= length` puts every index in bounds with no per-read + test), the carried binding's ENTRY value materializes as a non-negative + integral i32, and `a`/`b` are required non-negative so the dividend cannot go + negative — JS `%` returns a *negative* remainder for a negative dividend, + which is an out-of-bounds subscript rather than a slow path. + + The matcher also tracks the largest magnitude any sub-expression of the + recurrence can reach and declines above 2^53. "Fits i64" is necessary and NOT + sufficient: JavaScript evaluates the recurrence in doubles, and + `cursor * 1e9 + 1` fits i64 comfortably while losing its low bits in the f64 + the program actually runs. The bound is tracked per NODE rather than taken + from the folded pair, because folding cancels: `c * 1000 - c * 999` is + `1 * c`, but JavaScript still evaluates both thousand-fold products. + +* **Where the carried write-back goes is a correctness question, not a + scheduling one.** The residual side exit resumes the CURRENT iteration in the + slow clone, which re-runs the whole body — the recurrence included. So the + commit to the real binding is the LAST statement of the iteration: a + mid-iteration exit then leaves the binding holding that iteration's entry + value and the slow clone advances it exactly once. A write-back at the update + site double-applies it, and every later index is silently a different — still + in-bounds, still a valid record — one. + +* **K accumulator statements**, folded into one for the fast clone + (`acc = a; acc = acc + b` ≡ `acc = (a) + b`: the same operations on the same + operands in the same order, so the float result is bit-identical). Same + reason as above — the whole iteration must commit once, past every exit it + can take, or a tag test that fails on the third read leaves the first two + already applied when the slow clone re-runs it. Statements 2..K must read the + accumulator as the LEFT operand and must not read it in the addend, or the + substitution would change which value the addend sees. + +* **`arr[i].prop.length` and `arr[i].prop ? A : B`** + (`expr/element_shape_reads.rs`). The preheader proves which inline slot holds + a property; it proves nothing about what is in it. So each read tag-tests the + loaded word and side-exits when the answer is not the representation it is + about to assume: both string forms for `.length` (heap `utf16_len`, and the + SSO length byte — the same decode the runtime's own `.length` arms use), and + the two boolean singletons by exact NaN-box bit pattern for the ternary. + JS truthiness of `0` / `""` / `null` / an object is a runtime question the + clone does not guess, and a non-constant ternary arm is not admitted at all. + +* **One residual check per iteration, not one per read.** Three reads of + `rows[index]` were three element loads and three header/ShapeId checks. The + body's leading virtual binding now emits the deref and the residual once and + parks the masked handle in an entry alloca. That is also what makes the + multi-read side exit correct: every residual exit now precedes every store. + +* **The property denylist is arm-specific now.** The class-keyed arm keeps the + full list — its read bakes in a compile-time packed slot index while the + surrounding lowering may route the name elsewhere. The shape-keyed arm bakes + in nothing: it asks the runtime for that exact key's inline slot in that exact + ordinary ShapeId and declines on `-1`, and the residual pins + `obj_type == GC_TYPE_OBJECT` with no per-object descriptors — so every + receiver whose builtin branch could answer a name differently (a function's + `name`, an array's `length`, a Map's `size`) is already excluded, and for a + plain record an own data property SHADOWS the prototype name it collides + with. Only `__proto__` stays denied, and not because of JavaScript (node + gives `JSON.parse('{"__proto__":1}')` an own data property and reads `1` back + from it): Perry's generic property path may special-case the name ahead of + own-property lookup, and the clone must agree with the path it is a clone of + wherever the two differ. The full list would otherwise have cost the + benchmark its `fields` mode outright, for a field literally called `name`. + +**Measured** (ns/iter, 1M iterations, 5 interleaved rounds, best of 5, +`PERRY_NO_AUTO_OPTIMIZE=1`, on a loaded shared host; `new` and `base` are both +built from this worktree, `base` at #10171's head `2b77e7d4fe`): + +| cell | base (#10171) | new | node 26.5.1 | bun 1.3.14 | new / best | +|---|---:|---:|---:|---:|---:| +| 16k `repeat` | 4.21 | 4.17 | 3.56 | 5.23 | 1.171 | +| 16k `sequential` | 4.17 | 4.30 | 4.57 | 6.25 | 0.941 | +| 16k `random` | 15.48 | **4.97** | 7.16 | 9.96 | **0.694** | +| 16k `fields` | 25.59 | **6.05** | 5.88 | 9.77 | **1.029** | +| 1m `repeat` | 4.15 | 4.17 | 3.19 | 5.05 | 1.307 | +| 1m `sequential` | 4.26 | 4.20 | 9.42 | 7.96 | 0.528 | +| 1m `random` | 16.00 | **5.32** | 11.71 | 11.17 | **0.476** | +| 1m `fields` | 28.14 | **6.17** | 12.70 | 14.91 | **0.486** | +| 20m `repeat` | 4.12 | 4.11 | 3.50 | 5.24 | 1.174 | +| 20m `sequential` | 4.20 | 4.19 | 6.98 | 8.51 | 0.600 | +| 20m `random` | 27.53 | **5.63** | 9.31 | 10.56 | **0.605** | +| 20m `fields` | 27.05 | **6.06** | 10.98 | 14.81 | **0.552** | + +The two targeted shapes move 3.0x-4.9x and land below the better of node and +bun on five of their six cells. The sixth, 16k `fields`, is the one that does +not reach parity: 6.05 against node's 5.88 (1.029x; 6.08 vs 5.83 on a +separate 12-round re-measure of just that pair). +That is the smallest fixture, where the whole array is in L1 and the guard +overhead is the only thing left — node's own `fields` is just 1.29x its +`sequential` there, because its inline caches have type feedback proving `name` +is a string and `active` a boolean, while this clone tag-tests both on every +read. Paying that test is what lets it side-exit instead of deoptimize, so it +is a floor of the design rather than a missing peephole. Reported rather than +rounded off. + +`repeat` and `sequential`, which #10171 already served, are unchanged within +noise in both wall clock and instruction count — `sequential`'s single read has +nothing to share, and `repeat`'s constant index has no leading binding to hang +the prologue off. + +Instructions retired per iteration on the 16k fixture (2M iterations minus a +0-iteration run, `/usr/bin/time -l`): + +| shape | base | new | +|---|---:|---:| +| random | 207.6 | 29.1 | +| fields | 466.4 | 47.0 | +| sequential | 25.0 | 25.6 | +| repeat | 16.0 | 16.1 | + +`random` lands one recurrence above `sequential`'s 25, and `fields` two extra +guarded reads plus a string decode and a select above it — which is what the +shared residual buys, and the shape the numbers have to have if the clone is +really being entered rather than merely emitted (#10171's own lesson: a clone +can be `cond_br`-entered and never executed while every IR-census assertion +passes, and only the wall clock notices). + +Validation: `test-files/test_gap_json_record_loop_clone.ts` gains 40 cases — +the recurrence with and without its alias, the carried value read after the +loop, negative and fractional entry values, a zero and an over-long modulus, a +multiplier past the exact-double range, a mid-loop side exit whose sum AND +final cursor both observe the write-back protocol, SSO and heap `name`s, a +non-string and a null `name`, five non-boolean `active` values, a two-statement +fold whose second read side-exits, and records with own `name`/`length`/`size` +properties — all byte-identical to `node --experimental-strip-types` 26.5.1. +70 codegen IR-census tests (`element_shape_fields_random_tests.rs` and its +siblings), every positive paired with a sabotage case asserting the clone is +ABSENT. Seeded GC stress on the gap binary +(`PERRY_GC_SCHEDULE_SEED=1..4 PERRY_GC_SCHEDULE_RATE=0.2 +PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=32`): output +identical across all four seeds and to node, with 3–11 copying minors and +~14.4k moved objects per run, so the instrument's subject was live. diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index 04be6b3799..ebeac06c0f 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -504,6 +504,17 @@ fn reduction_add_is_reassociable(ctx: &FnCtx<'_>, left: &Expr, right: &Expr) -> } fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> { + // #10185: the element-shape fast clone's two NON-numeric reads — + // `arr[i].name.length` and `arr[i].active ? 1 : 0`. Both are numbers by the + // time they get here, each proven from the loaded word's own NaN-box tag + // with a side exit to the slow clone on a miss, so the `true` suppresses + // the residual `js_number_coerce` below: that call would not slow the clone + // down, it would delete it. Outside a clone the fact vector is empty and + // this is a no-op. See `expr::element_shape_reads`. + if let Some(value) = crate::expr::element_shape_reads::try_lower_cloned_read_operand(ctx, expr)? + { + return Ok((value, true)); + } // A stable-packed numeric clone has a stronger fact than the generic // untyped-local typed-array probe below: its preheader scanned the exact // range, and its emitted-IR gate proved that no call can invalidate that diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index d2ec6ce903..a8565b21b3 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -538,13 +538,137 @@ pub(crate) fn emit_element_shape_loop_preheader_check( }) } -/// Emit one `arr[i].field` read inside the fast clone: bare element load, -/// an optional residual per-element check, then a bare raw-f64 slot load. +/// Materialize this iteration's element index as an i32, or report that the +/// clone cannot index at all. /// -/// `idx_i32` must be an index the preheader discharged a bounds obligation for -/// (see [`ElementShapeIndexBound`]), and `fact` must be the fact the preheader -/// installed for this array. -pub(crate) fn emit_element_shape_field_load( +/// One arm per [`super::ElementShapeIndex`] spelling, and each reads exactly +/// the storage whose bounds obligation the preheader discharged +/// ([`ElementShapeIndexBound`]). `None` means the counter's canonical i32 slot +/// the matcher required has gone missing — that costs the read its fast +/// lowering, never correctness, and the matcher, the fact lookup and this must +/// all agree (`ElementShapeIndex::needs_counter_i32_slot`). +pub(crate) fn emit_element_shape_index( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, +) -> Option { + match &fact.index { + super::ElementShapeIndex::Counter => { + let slot = ctx.i32_counter_slots.get(&fact.index_local_id).cloned()?; + Some(ctx.block().load(I32, &slot)) + } + super::ElementShapeIndex::Constant(k) => Some(k.to_string()), + // The derived `const d = j % m` binding's own slot, written by the + // `Let` arm in `stmt/let_stmt.rs` earlier in this same iteration. + super::ElementShapeIndex::DerivedMod { slot, .. } => { + let slot = slot.clone(); + Some(ctx.block().load(I32, &slot)) + } + // #10185: the carried recurrence's private i32 slot, written by the + // body's first statement earlier in this same iteration. The REAL + // binding slot is deliberately NOT read here — it is one iteration + // behind until the trailing write-back commits, which is exactly what + // makes a mid-iteration side exit correct. + super::ElementShapeIndex::Carried(carried) => { + let slot = carried.slot.clone(); + Some(ctx.block().load(I32, &slot)) + } + } +} + +/// The masked heap handle of `arr[idx]` for THIS iteration, with the residual +/// per-element facts discharged. +/// +/// Two sources. When the matcher installed an [`super::ElementPrefetch`] the +/// body's leading virtual binding already did the deref and the residual check +/// for the whole iteration, so this is one load of an entry alloca; otherwise +/// (#10123's original shape) the deref and the check are emitted here, once per +/// read. +pub(crate) fn emit_element_shape_element_handle( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, +) -> String { + if let Some(prefetch) = &fact.elem_prefetch { + let slot = prefetch.handle_slot.clone(); + return ctx.block().load(I64, &slot); + } + emit_element_deref_with_residual(ctx, fact, idx_i32) +} + +/// Bare element load plus the residual per-OBJECT facts the array-level +/// invariant deliberately does not cover. Returns the masked handle. +/// +/// Emits nothing but loads, ALU and one `cond_br` — call-free by construction, +/// which is the whole revocation argument (see the module docs). +pub(crate) fn emit_element_deref_with_residual( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, +) -> String { + let blk = ctx.block(); + // The element-shape invariant proved every slot in the verified prefix + // is a POINTER_TAG object of the guarded identity, so the unbox needs + // no tag test and no handle-band test — the two checks that make up the + // element-read tier. + let idx64 = blk.sext(I32, idx_i32, I64); + let slot_ptr = blk.gep(I64, &fact.elements_base, &[(I64, &idx64)]); + let elem_bits = blk.load(I64, &slot_ptr); + let elem_handle = blk.and(I64, &elem_bits, crate::nanbox::POINTER_MASK_I64); + + if fact.statically_layout_proven { + return elem_handle; + } + let elem_ptr = blk.inttoptr(I64, &elem_handle); + let load_idx = ctx.new_block("element_shape.load"); + let load_label = ctx.block_label(load_idx); + let blk = ctx.block(); + + // Residual per-OBJECT facts (see the module docs for why they cannot come + // from the runtime array-level invariant). + let hdr_ptr = blk.gep(I8, &elem_ptr, &[(I64, "-8")]); + let hdr = blk.load(I32, &hdr_ptr); + let (mask, expect) = if fact.shape_keyed { + (ELEM_HEADER_SHAPE_MASK, ELEM_HEADER_SHAPE_EXPECT) + } else { + (ELEM_HEADER_MASK, ELEM_HEADER_EXPECT) + }; + let hdr_masked = blk.and(I32, &hdr, mask); + let hdr_ok = blk.icmp_eq(I32, &hdr_masked, expect); + + // #8113: the ShapeId moved from header offset 8 to 4. + let sid_ptr = blk.gep(I8, &elem_ptr, &[(I64, "4")]); + let shape_id = blk.load(I32, &sid_ptr); + let shape_ok = blk.icmp_eq(I32, &shape_id, &fact.expected_shape_id); + + let ok = blk.and(I1, &hdr_ok, &shape_ok); + // The side exit resumes the CURRENT iteration in the slow clone; no effect + // of this iteration has committed yet (#10185: with a prefetch this is the + // ONLY residual exit in the iteration, and it precedes every store). + blk.cond_br(&ok, &load_label, &fact.side_exit_label); + ctx.current_block = load_idx; + elem_handle +} + +/// #10185: the once-per-iteration element prologue. +/// +/// Emitted by the body's leading virtual binding (`stmt/let_stmt.rs` → +/// `stmt/element_shape_loop::lower_virtual_clone_binding`) once the index for +/// this iteration is in its slot. Parks the masked handle in an entry alloca so +/// every read in the iteration is a bare offset load. +pub(crate) fn emit_element_shape_prefetch( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, + handle_slot: &str, +) { + let handle = emit_element_deref_with_residual(ctx, fact, idx_i32); + ctx.block().store(I64, &handle, handle_slot); +} + +/// One tracked property's RAW slot word — a NaN-boxed `JSValue` in the +/// shape-keyed arm, a raw `double` in the class-keyed one. No tag test; the +/// caller adds the one its consumer needs. +pub(crate) fn emit_element_shape_slot_load( ctx: &mut FnCtx, fact: &super::ElementShapeLoopFact, idx_i32: &str, @@ -555,54 +679,27 @@ pub(crate) fn emit_element_shape_field_load( super::ElementShapeFieldSlot::Packed(index) => (I64, index.to_string()), super::ElementShapeFieldSlot::Runtime(reg) => (I64, reg.clone()), }; - - let elem_ptr = { - let blk = ctx.block(); - // The element-shape invariant proved every slot in the verified prefix - // is a POINTER_TAG object of the guarded identity, so the unbox needs - // no tag test and no handle-band test — the two checks that make up the - // element-read tier. - let idx64 = blk.sext(I32, idx_i32, I64); - let slot_ptr = blk.gep(I64, &fact.elements_base, &[(I64, &idx64)]); - let elem_bits = blk.load(I64, &slot_ptr); - let elem_handle = blk.and(I64, &elem_bits, crate::nanbox::POINTER_MASK_I64); - let elem_ptr = blk.inttoptr(I64, &elem_handle); - - if !fact.statically_layout_proven { - let load_idx = ctx.new_block("element_shape.load"); - let load_label = ctx.block_label(load_idx); - let blk = ctx.block(); - - // Residual per-OBJECT facts (see the module docs for why they - // cannot come from the runtime array-level invariant). - let hdr_ptr = blk.gep(I8, &elem_ptr, &[(I64, "-8")]); - let hdr = blk.load(I32, &hdr_ptr); - let (mask, expect) = if fact.shape_keyed { - (ELEM_HEADER_SHAPE_MASK, ELEM_HEADER_SHAPE_EXPECT) - } else { - (ELEM_HEADER_MASK, ELEM_HEADER_EXPECT) - }; - let hdr_masked = blk.and(I32, &hdr, mask); - let hdr_ok = blk.icmp_eq(I32, &hdr_masked, expect); - - // #8113: the ShapeId moved from header offset 8 to 4. - let sid_ptr = blk.gep(I8, &elem_ptr, &[(I64, "4")]); - let shape_id = blk.load(I32, &sid_ptr); - let shape_ok = blk.icmp_eq(I32, &shape_id, &fact.expected_shape_id); - - let ok = blk.and(I1, &hdr_ok, &shape_ok); - // One branch per access. The side exit resumes the CURRENT - // iteration in the slow clone; no effect has committed yet. - blk.cond_br(&ok, &load_label, &fact.side_exit_label); - ctx.current_block = load_idx; - } - elem_ptr - }; - + let elem_handle = emit_element_shape_element_handle(ctx, fact, idx_i32); let blk = ctx.block(); + let elem_ptr = blk.inttoptr(I64, &elem_handle); let fields_base = blk.gep(I8, &elem_ptr, &[(I64, &header_skip)]); let field_ptr = blk.gep(DOUBLE, &fields_base, &[(slot_ty, &slot_value)]); - let value = blk.load(DOUBLE, &field_ptr); + blk.load(DOUBLE, &field_ptr) +} + +/// Emit one `arr[i].field` read inside the fast clone: bare element load, +/// an optional residual per-element check, then a bare raw-f64 slot load. +/// +/// `idx_i32` must be an index the preheader discharged a bounds obligation for +/// (see [`ElementShapeIndexBound`]), and `fact` must be the fact the preheader +/// installed for this array. +pub(crate) fn emit_element_shape_field_load( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, + field_slot: &super::ElementShapeFieldSlot, +) -> String { + let value = emit_element_shape_slot_load(ctx, fact, idx_i32, field_slot); // #10123: the shape-keyed arm's representation check. // diff --git a/crates/perry-codegen/src/expr/element_shape_reads.rs b/crates/perry-codegen/src/expr/element_shape_reads.rs new file mode 100644 index 0000000000..9d78d32174 --- /dev/null +++ b/crates/perry-codegen/src/expr/element_shape_reads.rs @@ -0,0 +1,326 @@ +//! #10185: the two NON-numeric reads the element-shape fast clone admits — +//! `arr[i].prop.length` on a string field and `arr[i].prop ? A : B` on a +//! boolean one. +//! +//! ## Why they need their own lowering at all +//! +//! Both are ordinary JavaScript that the generic lowering handles perfectly +//! well — with a call. `.length` on an untyped receiver reaches the property +//! diamond and, on a miss, `js_object_get_field_by_name_f64`; a ternary on an +//! untyped condition reaches `js_is_truthy`. Inside this clone a call is not a +//! slow path, it is a DELETED clone (`contains_gc_unsafe_call`, #7690), so the +//! benchmark's `fields` shape — +//! +//! ```text +//! sum += rows[index].id; +//! sum += rows[index].name.length; +//! sum += rows[index].active ? 1 : 0; +//! ``` +//! +//! got no clone at all: one unadmitted read in the body costs the whole loop. +//! +//! ## What each one proves before it reads +//! +//! The element-shape preheader proves WHICH inline slot holds `name` and +//! `active`; it proves nothing about what is in them. So each read tag-tests +//! the loaded word and side-exits to the slow clone when the answer is not the +//! representation it is about to assume — exactly the discipline #10123 +//! established for the Number case. A `name` that is a number, a `null` or an +//! object, and an `active` that is anything but `true`/`false`, are re-run in +//! the slow clone, where full JS semantics apply. +//! +//! **`.length`.** JS `.length` is UTF-16 code units. A heap string keeps that +//! count in `StringHeader::utf16_len`, the leading `u32` — the identical load +//! the inline `.length` fast path in `property_get/generic_dispatch.rs` emits. +//! An SSO immediate (`SHORT_STRING_TAG`, up to five bytes packed into the +//! NaN-box) keeps its BYTE length in bits 40..=47, and this reads it the same +//! way the runtime's own SSO `.length` arms do +//! (`string/char_ops.rs::string_property_get_miss`, +//! `property_get/generic_dispatch.rs`). That convention is byte length, not +//! code-unit length, so a non-ASCII SSO string reports its UTF-8 size — a +//! PRE-EXISTING Perry-wide answer, reproduced here deliberately: the clone must +//! agree with the path it is a clone of, and diverging from it would be a +//! miscompile even where the shared answer is itself wrong. +//! +//! **The ternary.** JS truthiness of an arbitrary value is a runtime question +//! (`""`, `0`, `NaN`, `null`, every object). The clone does not guess it: only +//! the two boolean singletons are admitted, by exact NaN-box bit pattern, and +//! everything else side-exits. The result is one `select` between two +//! compile-time constants. + +use anyhow::Result; +use perry_hir::Expr; + +use super::FnCtx; +use crate::types::{DOUBLE, I1, I32, I64}; + +/// `STRING_TAG >> 48` — a heap `StringHeader` pointer in the low 48 bits. +const STRING_TAG_TOP16: &str = crate::nanbox::STRING_TAG_TOP16_I64; +/// `SHORT_STRING_TAG >> 48` — an SSO immediate. +const SHORT_STRING_TAG_TOP16: &str = crate::nanbox::SHORT_STRING_TAG_TOP16_I64; +/// `SHORT_STRING_LEN_SHIFT` — the length byte sits at bits 40..=47. +const SHORT_STRING_LEN_SHIFT: &str = "40"; + +/// `TAG_TRUE` (`0x7FFC_0000_0000_0004`) as a decimal i64 literal. +const TAG_TRUE_I64: &str = "9222246136947933188"; +/// `TAG_FALSE` (`0x7FFC_0000_0000_0003`) as a decimal i64 literal. +const TAG_FALSE_I64: &str = "9222246136947933187"; + +/// Is `expr` a tracked element read the fast clone has a fact for? +/// +/// Only meaningful INSIDE the clone — outside one the fact vector is empty and +/// every answer is `None`. +fn tracked_element_read(ctx: &FnCtx<'_>, expr: &Expr) -> bool { + let Expr::PropertyGet { + object, property, .. + } = expr + else { + return false; + }; + super::element_shape_loop_fact_for_property_get(ctx, object, property).is_some() +} + +/// `.length` — returns the inner read. +pub(crate) fn cloned_string_length_read<'e>(ctx: &FnCtx<'_>, expr: &'e Expr) -> Option<&'e Expr> { + let Expr::PropertyGet { + object, property, .. + } = expr + else { + return None; + }; + if property != "length" { + return None; + } + tracked_element_read(ctx, object.as_ref()).then(|| object.as_ref()) +} + +/// ` ? : ` — returns the +/// condition's inner read and the two constants. +pub(crate) fn cloned_bool_select_read<'e>( + ctx: &FnCtx<'_>, + expr: &'e Expr, +) -> Option<(&'e Expr, i64, i64)> { + let Expr::Conditional { + condition, + then_expr, + else_expr, + } = expr + else { + return None; + }; + if !tracked_element_read(ctx, condition.as_ref()) { + return None; + } + Some(( + condition.as_ref(), + integer_arm(then_expr)?, + integer_arm(else_expr)?, + )) +} + +/// A ternary arm the clone may bake in: an integer-valued literal. +/// +/// Deliberately narrow. A non-constant arm would have to be lowered on both +/// sides of a branch inside a clone whose whole admission argument is that its +/// body is one straight line of pure reads. +pub(crate) fn integer_arm(expr: &Expr) -> Option { + match expr { + Expr::Integer(k) => Some(*k), + Expr::Number(n) if n.is_finite() && n.fract() == 0.0 && *n >= -1e15 && *n <= 1e15 => { + Some(*n as i64) + } + _ => None, + } +} + +/// Resolve one tracked read to (fact, slot, index) and emit the raw slot word. +fn emit_tracked_slot_word(ctx: &mut FnCtx<'_>, read: &Expr) -> Option { + let Expr::PropertyGet { + object, property, .. + } = read + else { + return None; + }; + let (fact, slot) = super::element_shape_loop_fact_for_property_get(ctx, object, property) + .map(|(fact, slot)| (fact.clone(), slot.clone()))?; + // A class-keyed clone's slot holds a RAW double, not a NaN-boxed value — + // there is no tag to test and these two readers would be reading a + // `double`'s bit pattern as a box. Both forms are shape-keyed-only. + if !fact.shape_keyed { + return None; + } + let idx_i32 = super::element_shape_guard::emit_element_shape_index(ctx, &fact)?; + Some(super::element_shape_guard::emit_element_shape_slot_load( + ctx, &fact, &idx_i32, &slot, + )) +} + +/// The side exit the tracked read's own fact names. +fn side_exit_label(ctx: &FnCtx<'_>, read: &Expr) -> Option { + let Expr::PropertyGet { + object, property, .. + } = read + else { + return None; + }; + super::element_shape_loop_fact_for_property_get(ctx, object, property) + .map(|(fact, _)| fact.side_exit_label.clone()) +} + +/// Emit `.length` as an f64. +pub(crate) fn lower_cloned_string_length( + ctx: &mut FnCtx<'_>, + read: &Expr, +) -> Result> { + let Some(exit) = side_exit_label(ctx, read) else { + return Ok(None); + }; + let Some(value) = emit_tracked_slot_word(ctx, read) else { + return Ok(None); + }; + + let bits = ctx.block().bitcast_double_to_i64(&value); + let top16 = ctx.block().lshr(I64, &bits, "48"); + let is_heap = ctx.block().icmp_eq(I64, &top16, STRING_TAG_TOP16); + let is_sso = ctx.block().icmp_eq(I64, &top16, SHORT_STRING_TAG_TOP16); + let is_string = ctx.block().or(I1, &is_heap, &is_sso); + + let dispatch_idx = ctx.new_block("element_shape.strlen"); + let heap_idx = ctx.new_block("element_shape.strlen.heap"); + let sso_idx = ctx.new_block("element_shape.strlen.sso"); + let done_idx = ctx.new_block("element_shape.strlen.done"); + let dispatch_label = ctx.block_label(dispatch_idx); + let heap_label = ctx.block_label(heap_idx); + let sso_label = ctx.block_label(sso_idx); + let done_label = ctx.block_label(done_idx); + + // A non-string `name` re-runs the whole iteration in the slow clone rather + // than being decoded as one. + ctx.block().cond_br(&is_string, &dispatch_label, &exit); + + ctx.current_block = dispatch_idx; + ctx.block().cond_br(&is_heap, &heap_label, &sso_label); + + // `StringHeader::utf16_len` is the leading `u32`; `safe_load_i32_from_ptr` + // keeps a sub-page handle off the load, and is itself call-free (an `icmp`, + // a `select` onto a zero-valued global, and the load). + ctx.current_block = heap_idx; + let heap_handle = ctx.block().and(I64, &bits, crate::nanbox::POINTER_MASK_I64); + let heap_len_i32 = ctx.block().safe_load_i32_from_ptr(&heap_handle); + let heap_len = ctx.block().uitofp(I32, &heap_len_i32, DOUBLE); + let heap_end = ctx.block().label.clone(); + ctx.block().br(&done_label); + + ctx.current_block = sso_idx; + let sso_shifted = ctx.block().lshr(I64, &bits, SHORT_STRING_LEN_SHIFT); + let sso_len_byte = ctx.block().and(I64, &sso_shifted, "255"); + let sso_len = ctx.block().uitofp(I64, &sso_len_byte, DOUBLE); + let sso_end = ctx.block().label.clone(); + ctx.block().br(&done_label); + + ctx.current_block = done_idx; + let length = ctx + .block() + .phi(DOUBLE, &[(&heap_len, &heap_end), (&sso_len, &sso_end)]); + Ok(Some(length)) +} + +/// Emit ` ? A : B` as an f64 select over the two constants. +pub(crate) fn lower_cloned_bool_select( + ctx: &mut FnCtx<'_>, + read: &Expr, + on_true: i64, + on_false: i64, +) -> Result> { + let Some(exit) = side_exit_label(ctx, read) else { + return Ok(None); + }; + let Some(value) = emit_tracked_slot_word(ctx, read) else { + return Ok(None); + }; + + let bits = ctx.block().bitcast_double_to_i64(&value); + let is_true = ctx.block().icmp_eq(I64, &bits, TAG_TRUE_I64); + let is_false = ctx.block().icmp_eq(I64, &bits, TAG_FALSE_I64); + let is_bool = ctx.block().or(I1, &is_true, &is_false); + + let ok_idx = ctx.new_block("element_shape.bool"); + let ok_label = ctx.block_label(ok_idx); + // Everything that is not one of the two boolean singletons — `0`, `""`, + // `null`, an object — has a JS truthiness the clone is not allowed to + // guess, so it takes the side exit. + ctx.block().cond_br(&is_bool, &ok_label, &exit); + + ctx.current_block = ok_idx; + let then_literal = format!("{:.1}", on_true as f64); + let else_literal = format!("{:.1}", on_false as f64); + Ok(Some(ctx.block().select( + I1, + &is_true, + DOUBLE, + &then_literal, + &else_literal, + ))) +} + +/// The arithmetic-operand entry point (`expr::binary::lower_arithmetic_operand`). +/// +/// Returns the lowered f64, already representation-proven, so the caller must +/// NOT append a `js_number_coerce` — that call would delete the clone. +pub(crate) fn try_lower_cloned_read_operand( + ctx: &mut FnCtx<'_>, + expr: &Expr, +) -> Result> { + if let Some(read) = cloned_string_length_read(ctx, expr) { + let read = read.clone(); + return lower_cloned_string_length(ctx, &read); + } + if let Some((read, on_true, on_false)) = cloned_bool_select_read(ctx, expr) { + let read = read.clone(); + return lower_cloned_bool_select(ctx, &read, on_true, on_false); + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Anti-drift gate, the same one `element_shape_guard`'s constants carry: + /// every literal above reproduces a `perry-runtime::value::tags` constant, + /// and `perry-codegen` cannot share the definition. + #[test] + fn boolean_singleton_literals_match_the_runtime() { + assert_eq!(TAG_TRUE_I64, (0x7FFC_0000_0000_0004u64 as i64).to_string()); + assert_eq!(TAG_FALSE_I64, (0x7FFC_0000_0000_0003u64 as i64).to_string()); + // Neither may collide with any other singleton in the 0x7FFC namespace + // — a clone that read `undefined`/`null`/a hole as `false` would be a + // miscompile, not a slow path. + for other in [ + 0x7FFC_0000_0000_0001u64, // TAG_UNDEFINED + 0x7FFC_0000_0000_0002, // TAG_NULL + 0x7FFC_0000_0000_0010, // TAG_HOLE + 0x7FFC_0000_0000_0011, // TAG_TDZ + ] { + assert_ne!(TAG_TRUE_I64, (other as i64).to_string()); + assert_ne!(TAG_FALSE_I64, (other as i64).to_string()); + } + } + + #[test] + fn string_tag_literals_match_the_runtime() { + assert_eq!(STRING_TAG_TOP16, (0x7FFFu64).to_string()); + assert_eq!(SHORT_STRING_TAG_TOP16, (0x7FF9u64).to_string()); + assert_eq!(SHORT_STRING_LEN_SHIFT, "40"); + } + + #[test] + fn only_integral_ternary_arms_are_admitted() { + assert_eq!(integer_arm(&Expr::Integer(1)), Some(1)); + assert_eq!(integer_arm(&Expr::Number(0.0)), Some(0)); + assert_eq!(integer_arm(&Expr::Number(1.5)), None); + assert_eq!(integer_arm(&Expr::Number(f64::NAN)), None); + assert_eq!(integer_arm(&Expr::Number(f64::INFINITY)), None); + assert_eq!(integer_arm(&Expr::LocalGet(3)), None); + } +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index a0df121336..5af15820f4 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2134,6 +2134,66 @@ pub(crate) enum ElementShapeIndex { /// Entry-block i32 alloca the clone writes the derived index to. slot: String, }, + /// #10185: a LOOP-CARRIED index — `let c = …;` outside the loop, + /// `c = (a*c + b) % m;` as the body's first statement, then `arr[c]` (or + /// `const index = c; arr[index]`). The benchmark's `random` mode, and the + /// shape every pseudo-random walk over a record array is written in. + /// + /// Unlike [`Self::DerivedMod`]'s `d`, `c` is a real mutable binding that is + /// LIVE AFTER THE LOOP, so the clone owes a write-back. It pays it once per + /// iteration, at the END of the iteration (`Carried::commit_slot`), which is + /// also what makes the residual side exit correct: every exit from the + /// middle of an iteration leaves the real slot holding the value it had at + /// that iteration's ENTRY, so the slow clone re-runs the update exactly + /// once. A write-back at the update site would double-apply the recurrence. + Carried(Box), +} + +/// #10185: everything [`ElementShapeIndex::Carried`] needs, boxed to keep the +/// enum small. +#[derive(Debug, Clone)] +pub(crate) struct CarriedIndex { + /// The `let` binding the recurrence advances. + pub local_id: u32, + /// `const index = c` — an optional alias the body may spell the subscript + /// through. Also virtual: its `Let` emits nothing. + pub alias_id: Option, + /// `c' = (a*c + b) % m`, folded to one affine pair by the matcher. Both + /// non-negative, and `|a| * i32::MAX + |b|` proven below 2^53 so the i64 + /// evaluation agrees with the f64 one JavaScript performs. + pub coeff_a: i64, + pub coeff_b: i64, + /// i32 SSA value of the modulus, materialized in the preheader. + pub modulus_i32: String, + /// Entry-block i32 alloca holding the live carried value inside the clone. + pub slot: String, + /// The binding's REAL (NaN-boxed double) slot, written once per iteration + /// after every side exit for that iteration has been passed. + pub commit_slot: String, +} + +/// #10185: the per-iteration element prefetch. +/// +/// `rows[index].id + rows[index].name.length + (rows[index].active ? 1 : 0)` +/// reads ONE element three times. Without this each read repeats the element +/// load AND the residual header/ShapeId check, which is three branches and +/// three redundant loads per iteration. The prologue attached to the body's +/// leading virtual binding does both once and parks the masked element handle +/// in an entry alloca; every read is then a bare offset load plus the tag test +/// its own consumer needs. +/// +/// It is also what makes a MULTI-READ body's side exit correct: every residual +/// exit now happens in the prologue, before any accumulator store has +/// committed, so resuming the iteration in the slow clone cannot double-apply +/// an earlier read. +#[derive(Debug, Clone)] +pub(crate) struct ElementPrefetch { + /// The virtual binding whose lowering emits the prologue. Exactly one + /// statement owns it, so a body with both a carried update and an alias + /// emits it once. + pub site_local_id: u32, + /// Entry-block i64 alloca holding the masked element handle. + pub handle_slot: String, } impl ElementShapeIndex { @@ -2152,7 +2212,22 @@ impl ElementShapeIndex { /// then declined would hand `is_numeric_expr` a raw-double promise the /// generic path does not keep. pub(crate) fn needs_counter_i32_slot(&self) -> bool { - !matches!(self, ElementShapeIndex::Constant(_)) + !matches!( + self, + ElementShapeIndex::Constant(_) | ElementShapeIndex::Carried(_) + ) + } + + /// #10185: the locals whose `Let` / `LocalSet` the fast clone lowers + /// VIRTUALLY for this index form. Nothing may read one of them bare inside + /// the clone, and their shadow slots must not be cleared there. + pub(crate) fn virtual_locals(&self) -> impl Iterator + '_ { + let (a, b) = match self { + ElementShapeIndex::DerivedMod { local_id, .. } => (Some(*local_id), None), + ElementShapeIndex::Carried(carried) => (Some(carried.local_id), carried.alias_id), + _ => (None, None), + }; + a.into_iter().chain(b) } } @@ -2200,6 +2275,10 @@ pub(crate) struct ElementShapeLoopFact { pub index_local_id: u32, /// #10123: how the clone computes the element index. pub index: ElementShapeIndex, + /// #10185: the shared per-iteration element deref, when the body has a + /// leading virtual binding to hang it off. `None` keeps #10123's per-read + /// deref + residual check. + pub elem_prefetch: Option, /// #10123: true when the preheader proved an exact ordinary ShapeId rather /// than a class id. The per-element residual check then drops the /// typed-layout conjunct (a parsed record's layout is @@ -2229,6 +2308,15 @@ pub(crate) struct ElementShapeLoopFact { /// raw-f64 candidates; shape-keyed ones are the preheader's live query /// results (#10123). pub fields: std::collections::BTreeMap, + /// #10185: true when the fast clone lowers a body the matcher SYNTHESIZED + /// (the K-statement accumulator fold, or the carried index's trailing + /// write-back) rather than the source body. The function-wide + /// shadow-slot-clear map is keyed by statement INDEX, so a rewritten body + /// would attribute a clear to the wrong statement; the clone is call-free, + /// so no collection can observe a slot inside it and the clears are simply + /// suppressed there (`expr::emit_shadow_slot_clear`). The slow clone lowers + /// the original body, with its original indices and its original clears. + pub synthesized_body: bool, /// #7771: the body's `const r = arr[counter]` binding, when the matcher /// admitted the element-binding form. Inside the fast clone the `Let` /// itself emits nothing (`stmt/let_stmt.rs`) and every `r.field` read @@ -2304,6 +2392,12 @@ pub(crate) fn element_shape_loop_fact_for_property_get<'f>( (ElementShapeIndex::DerivedMod { local_id, .. }, Expr::LocalGet(id)) => { *id == *local_id } + // #10185: `arr[c]` and `const index = c; arr[index]` are + // the same subscript — the alias is virtual, so both + // spellings read the carried slot the preheader bounded. + (ElementShapeIndex::Carried(carried), Expr::LocalGet(id)) => { + *id == carried.local_id || carried.alias_id == Some(*id) + } _ => false, }; if !spelled { @@ -2923,6 +3017,7 @@ mod typed_array_rmw; pub(crate) use instance_misc1::builtin_parent_reserved_class_id; pub(crate) mod class_field_inline_guard; pub(crate) mod element_shape_guard; +pub(crate) mod element_shape_reads; mod js_runtime; mod literals_vars; mod logical_collections; diff --git a/crates/perry-codegen/src/expr/property_get/helpers.rs b/crates/perry-codegen/src/expr/property_get/helpers.rs index 51a8e8dbe6..302fedd4b8 100644 --- a/crates/perry-codegen/src/expr/property_get/helpers.rs +++ b/crates/perry-codegen/src/expr/property_get/helpers.rs @@ -368,29 +368,14 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context( // array; the report below must not re-derive it from the expression // shape, which the binding form does not carry. let arr_id = fact.array_local_id; - // The counter's canonical i32 slot is what the matcher required for - // every index form that reads the counter; without it there is nothing - // to index with. A constant index reads no counter and needs none. - let counter_slot = ctx.i32_counter_slots.get(&fact.index_local_id).cloned(); - if counter_slot.is_some() || !fact.index.needs_counter_i32_slot() { - // #10123: the index the preheader discharged a bounds obligation - // for. All three forms are i32 and in `[0, length)` by the time - // they reach the GEP, which is why the fast clone pays no per-read - // bounds test in any of them. - let idx_i32 = match &fact.index { - crate::expr::ElementShapeIndex::Counter => { - let slot = counter_slot.expect("checked above"); - ctx.block().load(I32, &slot) - } - crate::expr::ElementShapeIndex::Constant(k) => k.to_string(), - // The derived `const d = j % m` binding's own slot, written by - // the `Let` arm in `stmt/let_stmt.rs` earlier in this same - // iteration. - crate::expr::ElementShapeIndex::DerivedMod { slot, .. } => { - let slot = slot.clone(); - ctx.block().load(I32, &slot) - } - }; + // #10123: the index the preheader discharged a bounds obligation for. + // Every form is an i32 in `[0, length)` by the time it reaches the GEP, + // which is why the fast clone pays no per-read bounds test in any of + // them. `None` means the counter's canonical i32 slot the matcher + // required is missing, so there is nothing to index with. + if let Some(idx_i32) = + crate::expr::element_shape_guard::emit_element_shape_index(ctx, &fact) + { let value = crate::expr::element_shape_guard::emit_element_shape_field_load( ctx, &fact, diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 4f1ff8e459..2383e3890d 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -176,18 +176,32 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { // #10123's derived index (`const d = j % m`) is the same case for the same // reason: its `Let` emits one `srem` into a private i32 alloca, never a // shadow bind, so a lexical-death clear would be the clone's only call. + // + // #10185's carried index (`c = (a*c + b) % m`) and its optional + // `const index = c` alias are two more of the same. if ctx.element_shape_loop_facts.iter().any(|fact| { - let virtual_binding = match &fact.index { - crate::expr::ElementShapeIndex::DerivedMod { local_id, .. } => Some(*local_id), - _ => None, - }; fact.element_binding .into_iter() - .chain(virtual_binding) + .chain(fact.index.virtual_locals()) .any(|id| ctx.shadow_slot_map.get(&id) == Some(&slot_idx)) }) { return; } + // #10185: a SYNTHESIZED fast-clone body (the accumulator fold, the carried + // write-back) no longer has the statement indices the function-wide clear + // map is keyed by, so every clear reached from inside it would be + // attributed to some other statement's local. Suppress them for the fast + // clone only: it is call-free, so no collection can observe a shadow slot + // while it runs, an over-rooted stale value is rewritten like any root, and + // the slow clone — lowered from the ORIGINAL body after the facts are + // popped — keeps every clear it always had. + if ctx + .element_shape_loop_facts + .iter() + .any(|fact| fact.synthesized_body) + { + return; + } // Never-bound slot: it provably still holds its initial 0 (slots are only // written through bind/set, and every value-set site binds first), so the // clear would be a redundant `js_shadow_slot_set(idx, 0)` TLS hit. diff --git a/crates/perry-codegen/src/stmt/element_shape_carried.rs b/crates/perry-codegen/src/stmt/element_shape_carried.rs new file mode 100644 index 0000000000..3924d8e733 --- /dev/null +++ b/crates/perry-codegen/src/stmt/element_shape_carried.rs @@ -0,0 +1,416 @@ +//! #10185: the element-shape clone's LOOP-CARRIED index — the `random` access +//! shape. +//! +//! ```text +//! let cursor = 0; +//! for (let i = 0; i < count; i++) { +//! cursor = (cursor * 17 + 7) % length; // the recurrence +//! const index = cursor; // optional alias +//! sum += rows[index].id; +//! } +//! ``` +//! +//! ## What makes this different from `const d = i % m` +//! +//! `d` is dead the moment the iteration ends, so #10123 could keep it entirely +//! virtual — its `Let` writes a private i32 alloca and nothing ever reads the +//! real slot. `cursor` is a real `let` that OUTLIVES the loop, so the clone +//! owes a write-back. +//! +//! **Where that write-back goes is a correctness question, not a scheduling +//! one.** The clone's residual check side-exits by resuming the CURRENT +//! iteration in the slow clone, which re-runs the whole body — the recurrence +//! included. A write-back at the update site would therefore apply +//! `cursor = (cursor * 17 + 7) % length` twice for that iteration and every +//! subsequent index would be wrong (silently: still in bounds, still a valid +//! record, just not the one JavaScript names). So the commit is the LAST thing +//! the iteration does, after every side exit it can take: +//! +//! * mid-iteration exit ⇒ the real slot still holds the previous commit, which +//! is exactly this iteration's entry value, and the slow clone advances it +//! once; +//! * normal exit ⇒ the last iteration committed, so the real slot holds the +//! final value the code after the loop reads. +//! +//! ## Why the recurrence is evaluated in i64 +//! +//! JavaScript evaluates `(cursor * 17 + 7) % length` in doubles. `frem` on +//! aarch64 is a libm `fmod` call — inside this clone a call is not a slow path, +//! it DELETES the clone (#7690) — so the recurrence is folded to one affine +//! pair `(a, b)` and evaluated as `srem i64`. +//! +//! That is only equal to what JavaScript computes while every intermediate is +//! exactly representable as a double, so the matcher tracks the maximum +//! magnitude any sub-expression can reach over `carried ∈ [0, i32::MAX]` and +//! declines above 2^53. (Fitting i64, which the original design note asked for, +//! is necessary but NOT sufficient: `c * 1e10 + 1` fits i64 comfortably and +//! loses its low bits in the f64 JavaScript actually runs.) `a` and `b` are +//! also required non-negative, so the dividend is non-negative and `srem` +//! agrees with JS `%` — which returns a NEGATIVE remainder for a negative +//! dividend, i.e. an out-of-bounds subscript, not a slow path. + +use anyhow::Result; +use perry_hir::{BinaryOp, Expr, Stmt}; + +use super::loops::{local_bound_is_loop_invariant, local_has_readable_slot}; +use crate::expr::FnCtx; +use crate::types::{DOUBLE, I32, I64}; + +/// The largest integer magnitude an `f64` represents exactly. Above it the i64 +/// recurrence and the f64 one JavaScript runs can disagree. +const EXACT_F64_INTEGER_LIMIT: i64 = 9_007_199_254_740_992; // 2^53 + +/// The preheader materializes the entry value in `0..=i32::MAX`, so this bounds +/// every `carried` the recurrence can ever see. +const MAX_CARRIED: i64 = i32::MAX as i64; + +/// A matched `carried = % m` update. +#[derive(Clone, Copy, Debug)] +pub(super) struct MatchedCarried { + pub carried_id: u32, + pub modulus_id: u32, + pub coeff_a: i64, + pub coeff_b: i64, +} + +/// Fold `expr` to `a * carried + b`, tracking the largest magnitude ANY +/// sub-expression can reach for `carried ∈ [0, i32::MAX]`. +/// +/// Returns `(a, b, max_abs)`. `None` declines — an unknown node, a second +/// variable, a non-integral literal, or any overflow of the checked i64 +/// arithmetic used to compute the bounds. +/// +/// The magnitude is tracked per NODE rather than taken from the folded result +/// because folding can cancel: `c * 1000 - c * 999` is `1 * c`, but JavaScript +/// still evaluates both thousand-fold products. +fn affine_over_carried(expr: &Expr, carried_id: u32) -> Option<(i64, i64, i64)> { + let magnitude = |a: i64, b: i64| -> Option { + a.checked_abs()? + .checked_mul(MAX_CARRIED)? + .checked_add(b.checked_abs()?) + }; + match expr { + Expr::LocalGet(id) if *id == carried_id => Some((1, 0, MAX_CARRIED)), + Expr::Integer(k) => Some((0, *k, k.checked_abs()?)), + Expr::Number(n) if n.is_finite() && n.fract() == 0.0 => { + // `as i64` saturates rather than wrapping, so an out-of-range + // literal would fold to i64::MAX and read as a huge-but-valid + // coefficient. Range-check before converting. + if *n < -(EXACT_F64_INTEGER_LIMIT as f64) || *n > EXACT_F64_INTEGER_LIMIT as f64 { + return None; + } + let k = *n as i64; + Some((0, k, k.checked_abs()?)) + } + Expr::Binary { op, left, right } => { + let (la, lb, lmax) = affine_over_carried(left, carried_id)?; + let (ra, rb, rmax) = affine_over_carried(right, carried_id)?; + let (a, b) = match op { + BinaryOp::Add => (la.checked_add(ra)?, lb.checked_add(rb)?), + BinaryOp::Sub => (la.checked_sub(ra)?, lb.checked_sub(rb)?), + // A product is affine only when one side is a constant; `c * c` + // is not, and neither is `c * m` for a runtime `m`. + BinaryOp::Mul if la == 0 => (ra.checked_mul(lb)?, rb.checked_mul(lb)?), + BinaryOp::Mul if ra == 0 => (la.checked_mul(rb)?, lb.checked_mul(rb)?), + _ => return None, + }; + let here = magnitude(a, b)?; + Some((a, b, lmax.max(rmax).max(here))) + } + _ => None, + } +} + +/// Does `expr` read `id`? +/// +/// Conservative by construction: an expression node this does not enumerate +/// answers "yes". The alternative shape — a catch-all that returns `false` — +/// is the #6377 / `collect_local_refs_expr` failure mode CLAUDE.md names, and +/// here it would let an accumulator read be folded into a statement that no +/// longer sees the value it was written against. +pub(super) fn expr_reads_local(expr: &Expr, id: u32) -> bool { + match expr { + Expr::LocalGet(read) => *read == id, + Expr::Number(_) | Expr::Integer(_) => false, + Expr::Binary { left, right, .. } + | Expr::MathImul(left, right) + | Expr::MathPow(left, right) => expr_reads_local(left, id) || expr_reads_local(right, id), + Expr::NumberCoerce(operand) => expr_reads_local(operand, id), + Expr::MathMin(values) | Expr::MathMax(values) => { + values.iter().any(|e| expr_reads_local(e, id)) + } + Expr::MathAbs(value) + | Expr::MathSqrt(value) + | Expr::MathFloor(value) + | Expr::MathCeil(value) + | Expr::MathRound(value) + | Expr::MathTrunc(value) + | Expr::MathSign(value) + | Expr::MathF16round(value) => expr_reads_local(value, id), + Expr::PropertyGet { object, .. } => expr_reads_local(object, id), + Expr::IndexGet { object, index } => { + expr_reads_local(object, id) || expr_reads_local(index, id) + } + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + expr_reads_local(condition, id) + || expr_reads_local(then_expr, id) + || expr_reads_local(else_expr, id) + } + _ => true, + } +} + +/// Match the body's first statement as `carried = % m`. +#[allow(clippy::too_many_arguments)] +pub(super) fn match_carried_update( + ctx: &FnCtx<'_>, + stmt: &Stmt, + counter_id: u32, + condition: &Expr, + update: Option<&Expr>, + body: &[Stmt], +) -> Option { + let Stmt::Expr(Expr::LocalSet(carried_id, value)) = stmt else { + return None; + }; + let Expr::Binary { + op: BinaryOp::Mod, + left, + right, + } = value.as_ref() + else { + return None; + }; + let Expr::LocalGet(modulus_id) = right.as_ref() else { + return None; + }; + if *carried_id == counter_id || *modulus_id == counter_id || *modulus_id == *carried_id { + return None; + } + + let (coeff_a, coeff_b, max_abs) = affine_over_carried(left, *carried_id)?; + // A negative dividend makes JS `%` return a negative remainder — an + // out-of-bounds subscript the preheader's `m <= length` says nothing about. + // With `carried >= 0` (the preheader materializes it in `0..=i32::MAX`) and + // both coefficients non-negative, the dividend cannot be negative. + if coeff_a < 0 || coeff_b < 0 { + return None; + } + if max_abs > EXACT_F64_INTEGER_LIMIT { + return None; + } + + // The binding must be a plain, directly-addressable local the clone can + // write back to with one store. A module global would need the rooted-store + // path, a boxed or captured binding lives in a cell a plain store would + // leave stale for an observer outside the clone. + if !ctx.locals.contains_key(carried_id) + || ctx.module_globals.contains_key(carried_id) + || ctx.boxed_vars.contains(carried_id) + || ctx.closure_captures.contains_key(carried_id) + || !local_has_readable_slot(ctx, *carried_id) + || ctx.pod_records.contains_key(carried_id) + || ctx.scalar_replaced.contains_key(carried_id) + { + return None; + } + // `carried` is written exactly once per iteration — by the statement above. + // Every other statement the body matcher admits writes the accumulator, and + // it rejects a body whose accumulator IS the carried local, so the only way + // a second write could appear is a `for` update expression naming it. + if update.is_some_and(|u| expr_writes_local(u, *carried_id)) { + return None; + } + // The modulus is materialized ONCE in the preheader and every iteration's + // `srem` uses that value, so a body (or update, or condition) that could + // rewrite it would derive indices against a stale bound. + if ctx.boxed_vars.contains(modulus_id) + || ctx.closure_captures.contains_key(modulus_id) + || !(local_has_readable_slot(ctx, *modulus_id) + || ctx.module_globals.contains_key(modulus_id)) + || !local_bound_is_loop_invariant(condition, update, body, *modulus_id) + { + return None; + } + + Some(MatchedCarried { + carried_id: *carried_id, + modulus_id: *modulus_id, + coeff_a, + coeff_b, + }) +} + +/// Does `expr` assign to `id`? Conservative in the same direction as +/// [`expr_reads_local`] — an unenumerated node answers "yes". +fn expr_writes_local(expr: &Expr, id: u32) -> bool { + match expr { + Expr::LocalSet(target, _) => *target == id, + Expr::Update { id: target, .. } => *target == id, + Expr::LocalGet(_) | Expr::Number(_) | Expr::Integer(_) => false, + Expr::Binary { left, right, .. } => { + expr_writes_local(left, id) || expr_writes_local(right, id) + } + _ => true, + } +} + +/// Lower the fast clone's carried-index statements, or report that this +/// expression is not one. +/// +/// Two shapes, both synthesized or admitted by the matcher, and neither lowered +/// generically: +/// +/// * the UPDATE `carried = % m` — one `mul`/`add`/`srem` chain in i64 +/// writing the clone's private i32 slot, never the real binding; +/// * the COMMIT `carried = carried`, the trailing statement the matcher appends +/// to the synthesized fast body — one `sitofp` + `store` that publishes the +/// iteration's value to the real slot. +/// +/// The commit is spelled as a self-assignment because it has to be a statement +/// the generic lowering would also accept (the slow clone never sees it — the +/// facts are popped first — but nothing may depend on that). A user-written +/// `c = c` inside a body cannot reach here: the body matcher admits only the +/// carried update, one virtual binding and accumulator writes, and `c` is not +/// the accumulator. +pub(super) fn lower_virtual_carried_stmt(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { + let Expr::LocalSet(id, value) = expr else { + return Ok(false); + }; + let Some((carried, scope_id)) = ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + let crate::expr::ElementShapeIndex::Carried(carried) = &fact.index else { + return None; + }; + (carried.local_id == *id).then(|| (carried.clone(), fact.scope_id)) + }) else { + return Ok(false); + }; + + if matches!(value.as_ref(), Expr::LocalGet(read) if *read == *id) { + // COMMIT. The value published here is the one every read in this + // iteration already used, so the slot and the binding agree from the + // end of the iteration onward. + let slot = carried.slot.clone(); + let commit_slot = carried.commit_slot.clone(); + let blk = ctx.block(); + let current = blk.load(I32, &slot); + let boxed = blk.sitofp(I32, ¤t, DOUBLE); + blk.store(DOUBLE, &boxed, &commit_slot); + return Ok(true); + } + + // UPDATE. `0 <= carried <= i32::MAX` and `1 <= m <= i32::MAX` hold by the + // preheader's two materializations, and the matcher proved + // `a * i32::MAX + b < 2^53`, so the i64 chain cannot overflow and agrees + // with the f64 arithmetic JavaScript performs. + let slot = carried.slot.clone(); + let modulus_i32 = carried.modulus_i32.clone(); + let coeff_a = carried.coeff_a.to_string(); + let coeff_b = carried.coeff_b.to_string(); + { + let blk = ctx.block(); + let current = blk.load(I32, &slot); + let current64 = blk.sext(I32, ¤t, I64); + let scaled = blk.mul(I64, ¤t64, &coeff_a); + let shifted = blk.add(I64, &scaled, &coeff_b); + let modulus64 = blk.sext(I32, &modulus_i32, I64); + let wrapped = blk.srem(I64, &shifted, &modulus64); + let next = blk.trunc(I64, &wrapped, I32); + blk.store(I32, &next, &slot); + } + // The index for this iteration is now in its slot, so a body with no alias + // binding hangs the shared element prologue off the update itself. + super::element_shape_loop::emit_element_prefetch_for(ctx, scope_id, *id); + Ok(true) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn local(id: u32) -> Expr { + Expr::LocalGet(id) + } + fn mul(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(l), + right: Box::new(r), + } + } + fn add(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(l), + right: Box::new(r), + } + } + fn sub(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(l), + right: Box::new(r), + } + } + + #[test] + fn the_benchmark_recurrence_folds() { + // `(cursor * 17 + 7)` + let e = add(mul(local(9), Expr::Integer(17)), Expr::Integer(7)); + assert_eq!( + affine_over_carried(&e, 9), + Some((17, 7, 17 * MAX_CARRIED + 7)) + ); + } + + #[test] + fn a_constant_only_expression_folds_to_b() { + assert_eq!(affine_over_carried(&Expr::Integer(5), 9), Some((0, 5, 5))); + } + + #[test] + fn a_second_variable_declines() { + assert_eq!(affine_over_carried(&add(local(9), local(10)), 9), None); + assert_eq!(affine_over_carried(&mul(local(9), local(9)), 9), None); + } + + /// The magnitude bound is per NODE, not of the folded pair: cancellation + /// must not hide an intermediate JavaScript really computes in f64. + #[test] + fn cancelling_products_still_report_their_intermediates() { + // `c * 1000000000 - c * 999999999` folds to `1 * c`, but both products + // reach ~2e18 for a large `c` — far past 2^53, where the f64 JS runs + // and this i64 chain stop agreeing. + let e = sub( + mul(local(9), Expr::Integer(1_000_000_000)), + mul(local(9), Expr::Integer(999_999_999)), + ); + let (a, b, max_abs) = affine_over_carried(&e, 9).expect("affine"); + assert_eq!((a, b), (1, 0)); + assert!( + max_abs > EXACT_F64_INTEGER_LIMIT, + "the intermediate magnitude must be reported, not the folded one" + ); + } + + #[test] + fn a_non_integral_literal_declines() { + assert_eq!(affine_over_carried(&Expr::Number(1.5), 9), None); + assert_eq!(affine_over_carried(&Expr::Number(f64::NAN), 9), None); + assert_eq!(affine_over_carried(&Expr::Number(f64::INFINITY), 9), None); + // Saturating `as i64` would turn this into a plausible coefficient. + assert_eq!(affine_over_carried(&Expr::Number(1e300), 9), None); + } + + #[test] + fn reads_are_detected_conservatively() { + assert!(expr_reads_local(&add(local(9), Expr::Integer(1)), 9)); + assert!(!expr_reads_local(&add(local(8), Expr::Integer(1)), 9)); + // An unenumerated node answers "reads it". + assert!(expr_reads_local(&Expr::This, 9)); + } +} diff --git a/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs new file mode 100644 index 0000000000..429523b2b5 --- /dev/null +++ b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs @@ -0,0 +1,650 @@ +//! #10185: the `fields` and `random` access shapes — IR census for the +//! element-shape clone's loop-carried index, its multi-statement accumulator +//! fold, and its two non-numeric reads. +//! +//! A child module of `element_shape_shape_keyed_tests` (see the `mod` +//! declaration there), so `use super::*` brings both that file's shape-keyed +//! assertions and `element_shape_loop_tests`'s slicing helpers. +//! +//! Every positive here is paired with a sabotage case, and the two liveness +//! assertions #10171 was short — the preheader's EXACT comparison set, and that +//! the residual check really is shared rather than repeated — are the ones that +//! would have caught a clone that is emitted, branched into, and never entered +//! or never actually cheaper. + +use super::*; + +const ROWS2_ID: u32 = 40; +const COUNT2_ID: u32 = 41; +const MOD2_ID: u32 = 42; +const SUM2_ID: u32 = 43; +const COUNTER2_ID: u32 = 44; +const CURSOR_ID: u32 = 45; +const ALIAS_ID: u32 = 46; +const OTHER_ID: u32 = 47; + +/// `rows[].` +fn rows_field(index: Expr, prop: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ROWS2_ID)), + index: Box::new(index), + }), + property: prop.to_string(), + byte_offset: 0, + } +} + +/// `sum = sum + ` +fn add_to_sum(value: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + SUM2_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM2_ID)), + right: Box::new(value), + }), + )) +} + +fn binary(op: BinaryOp, left: Expr, right: Expr) -> Expr { + Expr::Binary { + op, + left: Box::new(left), + right: Box::new(right), + } +} + +/// `cursor = % n` +fn cursor_update(affine: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + CURSOR_ID, + Box::new(binary(BinaryOp::Mod, affine, Expr::LocalGet(MOD2_ID))), + )) +} + +/// The benchmark's own recurrence, `(cursor * 17 + 7)`. +fn benchmark_affine() -> Expr { + binary( + BinaryOp::Add, + binary(BinaryOp::Mul, Expr::LocalGet(CURSOR_ID), Expr::Integer(17)), + Expr::Integer(7), + ) +} + +/// `const index = cursor;` +fn cursor_alias() -> Stmt { + Stmt::Let { + id: ALIAS_ID, + name: "index".to_string(), + ty: Type::Number, + mutable: false, + init: Some(Expr::LocalGet(CURSOR_ID)), + } +} + +/// `const index = i % n;` +fn modulo_index() -> Stmt { + Stmt::Let { + id: ALIAS_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(binary( + BinaryOp::Mod, + Expr::LocalGet(COUNTER2_ID), + Expr::LocalGet(MOD2_ID), + )), + } +} + +/// ```text +/// function run(rows: any, count: number, n: number): number { +/// let sum = 0; +/// let cursor = 0; +/// for (let i = 0; i < count; i++) +/// return sum; +/// } +/// ``` +fn access_module(body: Vec) -> Module { + let mut m = Module::new("element_shape_loop.ts"); + let param = |id: u32, name: &str, ty: Type| perry_hir::Param { + id, + name: name.to_string(), + ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }; + m.functions = vec![perry_hir::Function { + id: 902, + name: "run".to_string(), + type_params: Vec::new(), + params: vec![ + param(ROWS2_ID, "rows", Type::Any), + param(COUNT2_ID, "count", Type::Number), + param(MOD2_ID, "n", Type::Number), + ], + return_type: Type::Number, + body: vec![ + Stmt::Let { + id: SUM2_ID, + name: "sum".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::Let { + id: CURSOR_ID, + name: "cursor".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: COUNTER2_ID, + name: "i".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + })), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(COUNTER2_ID)), + right: Box::new(Expr::LocalGet(COUNT2_ID)), + }), + update: Some(Expr::Update { + id: COUNTER2_ID, + op: UpdateOp::Increment, + prefix: false, + }), + body, + }, + Stmt::Return(Some(Expr::LocalGet(SUM2_ID))), + ], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }]; + m.init_kind = ModuleInitKind::Eager; + m +} + +/// The benchmark's `random` body, with and without the alias spelling. +fn random_body(with_alias: bool) -> Vec { + let index = if with_alias { ALIAS_ID } else { CURSOR_ID }; + let mut body = vec![cursor_update(benchmark_affine())]; + if with_alias { + body.push(cursor_alias()); + } + body.push(add_to_sum(rows_field(Expr::LocalGet(index), "id"))); + body +} + +/// The benchmark's `fields` body: three accumulator statements over ONE +/// element, one through a string and one through a boolean. +fn fields_body() -> Vec { + vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + add_to_sum(Expr::PropertyGet { + object: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "name")), + property: "length".to_string(), + byte_offset: 0, + }), + add_to_sum(Expr::Conditional { + condition: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "active")), + then_expr: Box::new(Expr::Integer(1)), + else_expr: Box::new(Expr::Integer(0)), + }), + ] +} + +/// How many side exits the clone can take per iteration — one branch per +/// residual / tag test that leaves for the slow preheader. +fn side_exit_count(fast: &str) -> usize { + fast.matches("label %element_shape.loop.slow.preheader") + .count() +} + +/// The LAST block in `fast` whose label starts with `prefix`, body only. +/// +/// Used to name the block a commit must live in. Reaching a block named after a +/// passed tag test IS the proof that every side exit of the iteration is behind +/// it — which is stronger than any textual before/after comparison, because the +/// emitted block ORDER is not the execution order (the loop's `update` block is +/// printed before the body blocks that branch into it). +fn last_block_with_prefix<'a>(fast: &'a str, prefix: &str) -> &'a str { + let start = fast + .rmatch_indices('\n') + .map(|(idx, _)| idx + 1) + .find(|&idx| { + fast[idx..].starts_with(prefix) + && fast[idx..] + .lines() + .next() + .unwrap_or("") + .trim_end() + .ends_with(':') + }) + .unwrap_or_else(|| panic!("no block named `{prefix}*` in:\n{fast}")); + let body = &fast[start..]; + let end = body.find("\n\n").unwrap_or(body.len()); + &body[..end] +} + +/// Count the `store` instructions one iteration of the clone executes. +fn store_count(fast: &str) -> usize { + fast.lines() + .filter(|l| l.trim_start().starts_with("store ")) + .count() +} + +// --------------------------------------------------------------------------- +// #10185 — the LOOP-CARRIED index (`random`). +// --------------------------------------------------------------------------- + +#[test] +fn a_carried_recurrence_gets_a_shape_keyed_clone() { + let ir = emit(&access_module(random_body(true))); + assert_shape_keyed_clone(&ir, "carried index with an alias"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("srem i64") && fast.contains("mul i64"), + "the recurrence must be the i64 chain, not an `frem` (a libm call on \ + aarch64, which would delete the clone); emitted:\n{fast}" + ); + assert!( + !fast.contains("frem"), + "an `frem` in the clone is the whole reason this form needed its own \ + lowering; emitted:\n{fast}" + ); + assert!( + ir.contains("element_shape.loop.carried.range"), + "the carried ENTRY value must be materialized as a validated \ + non-negative i32 — the i64 bound and the `srem` sign argument are \ + both stated over `0..=i32::MAX`" + ); + assert!( + ir.contains("element_shape.loop.modulus.range"), + "the modulus must be materialized: `srem` by zero is UB and `x % 0` \ + is NaN in JS" + ); +} + +#[test] +fn a_carried_recurrence_matches_without_the_alias() { + // `rows[cursor]` directly — the alias is sugar, not part of the form. + let ir = emit(&access_module(random_body(false))); + assert_shape_keyed_clone(&ir, "carried index, no alias"); + let fast = fast_clone_slice(&ir); + assert!(fast.contains("srem i64"), "emitted:\n{fast}"); +} + +/// THE correctness assertion for this form. +/// +/// The residual side exit resumes the CURRENT iteration in the slow clone, +/// which re-runs the whole body — the recurrence included. So the write-back to +/// the real binding must come after EVERY exit the iteration can take, or the +/// recurrence is applied twice and every later index is silently wrong (still +/// in bounds, still a valid record, just not the one JavaScript names). +#[test] +fn the_carried_write_back_follows_every_side_exit() { + let ir = emit(&access_module(random_body(true))); + let fast = fast_clone_slice(&ir); + // Every side exit this iteration can take is a tag test on a loaded word, + // and the LAST of them is the `id` read's Number test. Its admitted arm — + // `element_shape.number.*` — is therefore reachable only once the whole + // iteration is past every exit, so a commit inside it is provably last. + let committed = last_block_with_prefix(&fast, "element_shape.number"); + assert!( + committed.contains("sitofp i32") && committed.contains("store double"), + "the carried write-back must live past every side exit; a write-back \ + before one makes the slow clone apply the recurrence twice for that \ + iteration, and every later index is silently wrong. \ + final block:\n{committed}\nfull clone:\n{fast}" + ); + assert_eq!( + committed.matches("store double").count(), + 1, + "the carried value must be published exactly once per iteration; \ + final block:\n{committed}" + ); + assert_eq!( + store_count(&fast), + // the carried i32 slot, the prefetched element handle, the + // accumulator, the carried write-back, and the counter's increment + 5, + "a second write to the carried binding would double-apply the \ + recurrence on a side exit; emitted:\n{fast}" + ); +} + +/// The same liveness assertion #10123's derived index carries: the ONE length +/// comparison the preheader owes is `modulus <= length`, NOT the counter arm's +/// `length >= bound`. Demanding the latter makes the clone unenterable whenever +/// the loop runs more times than the array is long — which is every access +/// benchmark. +#[test] +fn a_carried_index_owes_exactly_the_modulus_obligation() { + let ir = emit(&access_module(random_body(true))); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert_eq!( + deref.matches("icmp uge i32").count(), + 1, + "a carried index owes exactly ONE length comparison (`modulus <= \ + length`); emitted:\n{deref}" + ); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains("icmp ult i32") && !fast.contains("icmp ugt i32"), + "the bounds obligation is discharged ONCE in the preheader; a per-read \ + test in the clone would mean it was not; emitted:\n{fast}" + ); +} + +#[test] +fn a_negative_coefficient_declines() { + // `(cursor * 17 - 7) % n` can produce a negative dividend, and JS `%` + // returns a NEGATIVE remainder for one — an out-of-bounds subscript the + // preheader's `m <= length` says nothing about. + let body = vec![ + cursor_update(binary( + BinaryOp::Sub, + binary(BinaryOp::Mul, Expr::LocalGet(CURSOR_ID), Expr::Integer(17)), + Expr::Integer(7), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence whose dividend can go negative must decline" + ); +} + +#[test] +fn a_non_affine_recurrence_declines() { + // `(cursor * cursor) % n` is not affine, so no (a, b) pair describes it and + // no magnitude bound can be proven. + let body = vec![ + cursor_update(binary( + BinaryOp::Mul, + Expr::LocalGet(CURSOR_ID), + Expr::LocalGet(CURSOR_ID), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a non-affine recurrence must decline" + ); +} + +#[test] +fn a_recurrence_that_leaves_exact_double_range_declines() { + // `cursor * 1e9 + 1` fits i64 comfortably and does NOT fit the f64 that + // JavaScript actually evaluates it in, so the i64 chain and the program + // would disagree. "Fits i64" was the original design note's obligation; it + // is necessary and not sufficient. + let body = vec![ + cursor_update(binary( + BinaryOp::Add, + binary( + BinaryOp::Mul, + Expr::LocalGet(CURSOR_ID), + Expr::Integer(1_000_000_000), + ), + Expr::Integer(1), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence whose intermediates pass 2^53 must decline: JS evaluates \ + it in doubles and the clone's i64 chain would not agree" + ); +} + +#[test] +fn a_bare_read_of_the_carried_local_declines() { + // Inside the clone the real slot is one iteration behind until the trailing + // commit, so a bare `cursor` in the accumulator would read a stale value. + let body = vec![ + cursor_update(benchmark_affine()), + cursor_alias(), + add_to_sum(binary( + BinaryOp::Add, + rows_field(Expr::LocalGet(ALIAS_ID), "id"), + Expr::LocalGet(CURSOR_ID), + )), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a bare read of the carried local must decline the clone" + ); +} + +#[test] +fn a_recurrence_over_a_second_variable_declines() { + // `(cursor + other) % n` is not a function of `cursor` alone; `other` is + // not materialized, bounded, or proven loop-invariant. + let body = vec![ + cursor_update(binary( + BinaryOp::Add, + Expr::LocalGet(CURSOR_ID), + Expr::LocalGet(OTHER_ID), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence reading a second variable must decline" + ); +} + +// --------------------------------------------------------------------------- +// #10185 — the MULTI-STATEMENT body, the string `.length` and the ternary +// (`fields`). +// --------------------------------------------------------------------------- + +#[test] +fn the_fields_body_gets_a_shape_keyed_clone() { + let ir = emit(&access_module(fields_body())); + assert_shape_keyed_clone(&ir, "three reads of one element"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("element_shape.strlen.heap") && fast.contains("element_shape.strlen.sso"), + "a string `.length` must decode BOTH representations inline — a call \ + to the runtime would delete the clone; emitted:\n{fast}" + ); + assert!( + fast.contains("select i1"), + "the ternary must become a select between the two constants; \ + emitted:\n{fast}" + ); + assert!( + fast.contains("9222246136947933188") && fast.contains("9222246136947933187"), + "the ternary must test the two boolean singletons by exact NaN-box bit \ + pattern; guessing JS truthiness in the clone would be a miscompile; \ + emitted:\n{fast}" + ); +} + +/// THE cost assertion. Three reads of `rows[index]` are three loads of the same +/// element and three repeats of the residual header/ShapeId check unless the +/// prologue shares them — and a clone that is entered but pays 3x the guard is +/// exactly the "emitted, branched into, and not actually cheaper" failure the +/// IR census exists to catch. +#[test] +fn three_reads_of_one_element_share_one_residual_check() { + let ir = emit(&access_module(fields_body())); + let fast = fast_clone_slice(&ir); + assert_eq!( + fast.matches("134250751").count(), + 1, + "the shape-keyed residual mask must appear ONCE per iteration, not \ + once per read; emitted:\n{fast}" + ); + assert_eq!( + fast.matches("element_shape.load").count(), + // one block definition plus the `br` that targets it + 2, + "there must be exactly one residual-check block in the iteration; \ + emitted:\n{fast}" + ); + // Four tag/residual tests, not twelve: the residual once, then one + // representation test per read. + assert_eq!( + side_exit_count(&fast), + 4, + "one residual check plus one tag test per read; emitted:\n{fast}" + ); +} + +/// THE correctness assertion for the multi-statement body. `sum += a; sum += b` +/// commits `sum` twice; if the second read's tag test side-exits, the slow +/// clone re-runs the iteration and applies `a` a second time. The fold makes +/// the whole iteration commit once, after every exit. +#[test] +fn the_accumulator_commits_once_after_every_side_exit() { + let ir = emit(&access_module(fields_body())); + let fast = fast_clone_slice(&ir); + assert_eq!( + fast.matches("fadd double").count(), + 3, + "the fold keeps all three additions in source order — it changes when \ + the result is STORED, never what is computed; emitted:\n{fast}" + ); + assert_eq!( + store_count(&fast), + // the derived index, the prefetched element handle, the accumulator, + // and the counter's own increment + 4, + "three accumulator statements must fold to ONE accumulator store; a \ + second one means a side exit from a later read can leave an earlier \ + read already applied when the slow clone re-runs the iteration; \ + emitted:\n{fast}" + ); + // The one store lives in the block the LAST tag test branches into, so it + // is behind every side exit of the iteration. + let committed = last_block_with_prefix(&fast, "element_shape.bool"); + assert!( + committed.contains("store "), + "the accumulator store must live past every side exit; final \ + block:\n{committed}\nfull clone:\n{fast}" + ); +} + +#[test] +fn an_addend_that_reads_the_accumulator_declines() { + // `sum += rows[i].id; sum += sum` — folding would substitute the PRE-first + // -statement value of `sum` into the second addend. + let body = vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + add_to_sum(Expr::LocalGet(SUM2_ID)), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an addend reading the accumulator must decline the fold, and with it \ + the clone" + ); +} + +#[test] +fn two_accumulators_decline() { + let body = vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + Stmt::Expr(Expr::LocalSet( + CURSOR_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(CURSOR_ID)), + right: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + }), + )), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "the body must write exactly one accumulator" + ); +} + +#[test] +fn a_ternary_with_a_non_constant_arm_declines() { + let body = vec![ + modulo_index(), + add_to_sum(Expr::Conditional { + condition: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "active")), + then_expr: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + else_expr: Box::new(Expr::Integer(0)), + }), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "only integer-constant ternary arms are admitted" + ); +} + +#[test] +fn a_ternary_on_something_other_than_an_element_read_declines() { + let body = vec![ + modulo_index(), + add_to_sum(Expr::Conditional { + condition: Box::new(Expr::LocalGet(COUNTER2_ID)), + then_expr: Box::new(Expr::Integer(1)), + else_expr: Box::new(Expr::Integer(0)), + }), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "the clone lowers a boolean-singleton test, not general JS truthiness" + ); +} + +/// The two non-numeric reads are SHAPE-keyed only: a class-keyed clone's slot +/// holds a raw `double`, so there is no NaN-box tag to test and both readers +/// would be decoding a double's bit pattern. +#[test] +fn a_class_typed_array_declines_the_string_length_read() { + let ir = emit(&element_shape_module( + vec![Stmt::Expr(Expr::LocalSet( + SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM_ID)), + right: Box::new(Expr::PropertyGet { + object: Box::new(elem_field(ARRAY_ID, Expr::LocalGet(COUNTER_ID))), + property: "length".to_string(), + byte_offset: 0, + }), + }), + ))], + None, + )); + assert!( + !ir.contains("element_shape.strlen"), + "the class-keyed arm reads raw doubles; there is no tag to test" + ); +} diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index 1ce4f0cd8d..47e54c7c46 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -152,7 +152,7 @@ use super::loops::{ CLASS_FIELD_LOOP_PROP_DENYLIST, }; use crate::expr::{lower_expr, FnCtx}; -use crate::types::{DOUBLE, I1, I32}; +use crate::types::{DOUBLE, I1, I32, I64}; /// Loop bound: a literal, a loop-invariant local / module global that is /// materialized to i32 once in the preheader, or the tracked array's own @@ -210,14 +210,28 @@ enum ElementShapeIdentity { enum MatchedIndex { Counter, Constant(i64), - DerivedMod { local_id: u32, modulus_id: u32 }, + DerivedMod { + local_id: u32, + modulus_id: u32, + }, + /// #10185: `c = (a*c + b) % m; … arr[c]`, optionally spelled through a + /// `const index = c` alias. The recurrence itself lives on + /// [`ElementShapeVersionedLoop::carried`] — this arm records only the + /// spellings the subscript may take. See `stmt/element_shape_carried.rs`. + Carried { + carried_id: u32, + alias_id: Option, + }, } impl MatchedIndex { /// Mirror of [`crate::expr::ElementShapeIndex::needs_counter_i32_slot`], /// asked before the fact exists. fn needs_counter_i32_slot(self) -> bool { - !matches!(self, MatchedIndex::Constant(_)) + !matches!( + self, + MatchedIndex::Constant(_) | MatchedIndex::Carried { .. } + ) } } @@ -235,6 +249,21 @@ struct ElementShapeVersionedLoop { /// form; `None` for the original single-statement accumulator body. element_binding: Option, accumulator_id: u32, + /// #10185: the matched recurrence, for the [`MatchedIndex::Carried`] form. + carried: Option, + /// #10185: the body the FAST clone lowers, when it is not the source body. + /// + /// Two rewrites, both of which exist to put every side exit an iteration + /// can take BEFORE any of its stores: + /// + /// * K accumulator statements fold into one, so a tag test that fails on + /// the third read cannot leave the first two already applied to `acc` + /// when the slow clone re-runs the iteration; + /// * a carried recurrence gains a trailing commit statement, so the real + /// binding is published once the iteration is past every exit. + /// + /// The SLOW clone always lowers the original `body`. + fast_body: Option>, } /// The locals the pure-expression walk reasons about. @@ -246,6 +275,11 @@ struct PureExprScope { /// #10123: `(d, m)` for a body whose first statement is /// `const d = counter % m`. derived: Option<(u32, u32)>, + /// #10185: `(carried, alias)` for a body whose first statement advances a + /// loop-carried index. Both are excluded from bare reads for the same + /// reason `derived` is — inside the clone the real slot is one iteration + /// behind until the trailing commit. + carried: Option<(u32, Option)>, } /// What the walk collects. @@ -311,8 +345,33 @@ fn element_shape_loop_pure_expr_collect( out.props.insert(property.clone()); true } + // #10185: `arr[].prop.length` — a STRING field's JS length. + // Admitted as its own form rather than as "a read of `prop` + // followed by a generic `.length`", because the generic one is a + // property diamond ending in a runtime call, and a call inside this + // clone deletes it. The emitted read tag-tests the loaded word for + // both string representations and side-exits otherwise + // (`expr::element_shape_reads`). + Expr::PropertyGet { .. } if property == "length" => { + element_shape_loop_pure_expr_collect(ctx, object, scope, out) + } _ => false, }, + // #10185: `arr[].prop ? A : B` with constant arms. JS truthiness + // of an arbitrary value is a runtime question the clone is not allowed + // to guess, so the emitted read admits ONLY the two boolean singletons + // and side-exits on everything else. Anything but a tracked element + // read in the condition, or a non-constant arm, declines. + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + matches!(condition.as_ref(), Expr::PropertyGet { .. }) + && crate::expr::element_shape_reads::integer_arm(then_expr).is_some() + && crate::expr::element_shape_reads::integer_arm(else_expr).is_some() + && element_shape_loop_pure_expr_collect(ctx, condition, scope, out) + } // A bare read of the array, the counter, the element binding or the // derived index as a VALUE could flow it into arbitrary lowering; only // scalar reads the analysis proves numeric are admitted. The element @@ -324,6 +383,10 @@ fn element_shape_loop_pure_expr_collect( Expr::LocalGet(id) => { scope.element_binding != Some(*id) && scope.derived.map(|(d, _)| d) != Some(*id) + // #10185: the carried index and its alias are virtual too — + // their real slots are one iteration behind inside the clone, + // so a bare read would hand out a stale value. + && scope.carried.is_none_or(|(c, alias)| *id != c && alias != Some(*id)) && out.array.is_none_or(|a| a != *id) && (*id == scope.accumulator_id || crate::type_analysis::is_numeric_expr(ctx, expr)) } @@ -381,7 +444,18 @@ fn match_index_form(index: &perry_hir::Expr, scope: &PureExprScope) -> Option None, + // #10185: `arr[c]` and `const index = c; arr[index]` are the same + // subscript; the alias is virtual and carries no obligation of its + // own. + _ => match scope.carried { + Some((carried_id, alias_id)) if carried_id == *id || alias_id == Some(*id) => { + Some(MatchedIndex::Carried { + carried_id, + alias_id, + }) + } + _ => None, + }, }, // `0..=i32::MAX`, so the preheader's `length > k` compare is an i32 // one and the emitted index needs no conversion. @@ -619,41 +693,100 @@ fn anon_shape_field_type_is_compatible( /// Answering `false` for want of the counter's i32 slot cannot happen — the /// matcher requires one for the derived-index form — and would only cost the /// clone, never correctness. -pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> bool { - if ctx - .element_shape_loop_facts - .iter() - .any(|fact| fact.element_binding == Some(id)) - { - return true; +pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> Result { + enum VirtualBinding { + /// #7771 `const r = arr[j]` and #10185 `const index = c`: pure aliases, + /// no instruction of their own. + Alias, + /// #10123 `const d = j % m`: one `srem i32`. + DerivedMod { + counter_id: u32, + modulus_i32: String, + slot: String, + }, } - let Some((counter_slot, modulus_i32, derived_slot)) = - ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { - let crate::expr::ElementShapeIndex::DerivedMod { + + let Some((scope_id, binding)) = ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + if fact.element_binding == Some(id) { + return Some((fact.scope_id, VirtualBinding::Alias)); + } + match &fact.index { + crate::expr::ElementShapeIndex::DerivedMod { local_id, modulus_i32, slot, - } = &fact.index - else { - return None; - }; - if *local_id != id { - return None; + } if *local_id == id => Some(( + fact.scope_id, + VirtualBinding::DerivedMod { + counter_id: fact.index_local_id, + modulus_i32: modulus_i32.clone(), + slot: slot.clone(), + }, + )), + crate::expr::ElementShapeIndex::Carried(carried) if carried.alias_id == Some(id) => { + Some((fact.scope_id, VirtualBinding::Alias)) } - Some(( - ctx.i32_counter_slots.get(&fact.index_local_id)?.clone(), - modulus_i32.clone(), - slot.clone(), - )) - }) + _ => None, + } + }) else { + return Ok(false); + }; + + match binding { + VirtualBinding::Alias => {} + VirtualBinding::DerivedMod { + counter_id, + modulus_i32, + slot, + } => { + let Some(counter_slot) = ctx.i32_counter_slots.get(&counter_id).cloned() else { + return Ok(false); + }; + let blk = ctx.block(); + let counter = blk.load(I32, &counter_slot); + let derived = blk.srem(I32, &counter, &modulus_i32); + blk.store(I32, &derived, &slot); + } + } + // #10185: this iteration's index is now in its slot, so this is where the + // shared element deref belongs — for the ONE binding that owns it. + emit_element_prefetch_for(ctx, scope_id, id); + Ok(true) +} + +/// #10185: emit the once-per-iteration element prologue, if `site_local` is the +/// statement the fact designated to own it. +/// +/// Every read in the clone loads the handle this parks, so the designated +/// statement MUST run before any of them — which is why the matcher only ever +/// designates the body's leading virtual statement, and why a fact carrying a +/// prefetch whose index cannot be materialized is a compiler bug rather than a +/// missed optimization (the reads would load an uninitialized alloca). The +/// matcher checks `needs_counter_i32_slot` before the fact is built, so the +/// index is always available here. +pub(super) fn emit_element_prefetch_for(ctx: &mut FnCtx<'_>, scope_id: u32, site_local: u32) { + let Some(fact) = ctx + .element_shape_loop_facts + .iter() + .find(|fact| fact.scope_id == scope_id) + .cloned() else { - return false; + return; + }; + let Some(prefetch) = fact.elem_prefetch.clone() else { + return; }; - let blk = ctx.block(); - let counter = blk.load(I32, &counter_slot); - let derived = blk.srem(I32, &counter, &modulus_i32); - blk.store(I32, &derived, &derived_slot); - true + if prefetch.site_local_id != site_local { + return; + } + let idx_i32 = crate::expr::element_shape_guard::emit_element_shape_index(ctx, &fact) + .expect("a prefetching fact always has a materializable index"); + crate::expr::element_shape_guard::emit_element_shape_prefetch( + ctx, + &fact, + &idx_i32, + &prefetch.handle_slot, + ); } /// Is `array_id` an untyped receiver — the shape-keyed arm's entry condition? @@ -799,7 +932,7 @@ fn match_element_shape_versioned_loop( return None; } - // Store-free body, in one of three admitted shapes (see the module docs): + // Store-free body, in one of four admitted shapes (see the module docs): // // 1. `acc = ].field>` — the original // single statement; @@ -807,89 +940,156 @@ fn match_element_shape_versioned_loop( // element-binding form, the shape real read loops are written in; // 3. `const d = j % m; acc = ` — // #10123's derived-index form, the shape every sequential pass over a - // parsed record array is written in. + // parsed record array is written in; + // 4. #10185's loop-carried form, `c = (a*c + b) % m;` optionally followed + // by `const index = c;`. // - // In 2 and 3 the binding is VIRTUAL inside the fast clone: its `Let` - // emits nothing (form 2) or one `srem i32` (form 3) rather than the - // generic lowering (`stmt/let_stmt.rs`), so the revocation argument (no - // store, no call in the clone) is unchanged. `const`-only, deliberately: a - // `var` binding is function-scoped and observable after the loop, where - // the skipped `Let` would leave the slot holding its pre-loop value. + // In 2, 3 and 4 the leading statement is VIRTUAL inside the fast clone: it + // emits nothing (form 2), one `srem i32` (form 3) or the i64 recurrence + // (form 4) rather than the generic lowering (`stmt/let_stmt.rs`, + // `stmt/element_shape_carried.rs`), so the revocation argument (no store, + // no call in the clone) is unchanged. The bindings are `const`-only, + // deliberately: a `var` binding is function-scoped and observable after the + // loop, where the skipped `Let` would leave the slot holding its pre-loop + // value. Form 4's `c` is the one mutable exception, and it pays for it with + // a write-back (see `element_shape_carried`). // + // #10185 also admits K >= 1 accumulator statements instead of exactly one, + // which is what `sum += rows[i].id; sum += rows[i].name.length; …` needs. // NOTHING else is admitted. let mut element_binding: Option<(u32, u32)> = None; let mut derived: Option<(u32, u32)> = None; - let (acc_id, value) = match body { - [Stmt::Expr(Expr::LocalSet(acc_id, value))] => (acc_id, value), - [Stmt::Let { - id, - mutable: false, - init: Some(binding_init), - .. - }, Stmt::Expr(Expr::LocalSet(acc_id, value))] => { - // The binding must be a plain, loop-owned const local in either - // form. A boxed or captured binding lives in a cell the clone's - // replacement `Let` would leave stale for an observer outside the - // clone; a module-global id is not a body-scoped binding at all. - if *id == counter_id - || ctx.boxed_vars.contains(id) - || ctx.module_globals.contains_key(id) - || ctx.closure_captures.contains_key(id) - { - return None; + let mut carried: Option = None; + let mut carried_alias: Option = None; + let mut rest: &[Stmt] = body; + + // (4) The recurrence, which must be the body's FIRST statement: the index + // it produces is read by every statement after it, and a read BEFORE it + // would see a value the preheader bounded but the commit protocol does not + // describe. + if let Some(first) = rest.first() { + if let Some(matched) = super::element_shape_carried::match_carried_update( + ctx, first, counter_id, condition?, update, body, + ) { + carried = Some(matched); + rest = &rest[1..]; + } + } + + // (2)/(3)/(4-alias) The leading virtual binding. + if let Some(Stmt::Let { + id, + mutable: false, + init: Some(binding_init), + .. + }) = rest.first() + { + // The binding must be a plain, loop-owned const local in every form. A + // boxed or captured binding lives in a cell the clone's replacement + // `Let` would leave stale for an observer outside the clone; a + // module-global id is not a body-scoped binding at all. + if *id == counter_id + || ctx.boxed_vars.contains(id) + || ctx.module_globals.contains_key(id) + || ctx.closure_captures.contains_key(id) + { + return None; + } + match binding_init { + Expr::IndexGet { object, index } if carried.is_none() => { + let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = + (object.as_ref(), index.as_ref()) + else { + return None; + }; + // Same receiver/index discipline as the walk's IndexGet + // arm: the fetch must be `arr[counter]` exactly. + if *idx_id != counter_id || *arr_id == counter_id || *id == *arr_id { + return None; + } + element_binding = Some((*id, *arr_id)); } - match binding_init { - Expr::IndexGet { object, index } => { - let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = - (object.as_ref(), index.as_ref()) - else { - return None; - }; - // Same receiver/index discipline as the walk's IndexGet - // arm: the fetch must be `arr[counter]` exactly. - if *idx_id != counter_id || *arr_id == counter_id || *id == *arr_id { - return None; - } - element_binding = Some((*id, *arr_id)); + // #10123. `%` on two locals, the left one the counter. The + // modulus is validated below (it must be readable and + // loop-invariant), and the RANGE obligation — `1 <= m` so the + // `srem` cannot divide by zero, `m <= length` so every derived + // index is in bounds — is discharged in the preheader, which + // is also where a non-number / fractional `m` sends the loop + // to the slow clone. + Expr::Binary { + op: BinaryOp::Mod, + left, + right, + } if carried.is_none() => { + let (Expr::LocalGet(num_id), Expr::LocalGet(modulus_id)) = + (left.as_ref(), right.as_ref()) + else { + return None; + }; + if *num_id != counter_id || *modulus_id == counter_id || *id == *modulus_id { + return None; } - // #10123. `%` on two locals, the left one the counter. The - // modulus is validated below (it must be readable and - // loop-invariant), and the RANGE obligation — `1 <= m` so the - // `srem` cannot divide by zero, `m <= length` so every derived - // index is in bounds — is discharged in the preheader, which - // is also where a non-number / fractional `m` sends the loop - // to the slow clone. - Expr::Binary { - op: BinaryOp::Mod, - left, - right, - } => { - let (Expr::LocalGet(num_id), Expr::LocalGet(modulus_id)) = - (left.as_ref(), right.as_ref()) - else { - return None; - }; - if *num_id != counter_id || *modulus_id == counter_id || *id == *modulus_id { - return None; - } - if !modulus_local_is_admissible(ctx, *modulus_id, condition?, update, body) { - return None; - } - derived = Some((*id, *modulus_id)); + if !modulus_local_is_admissible(ctx, *modulus_id, condition?, update, body) { + return None; } - _ => return None, + derived = Some((*id, *modulus_id)); } - (acc_id, value) + // #10185: `const index = c` after the recurrence. A pure alias, so + // it emits nothing at all — the subscript resolves to the carried + // slot either way. + Expr::LocalGet(aliased) => { + let matched = carried.as_ref()?; + if *aliased != matched.carried_id || *id == matched.modulus_id { + return None; + } + carried_alias = Some(*id); + } + _ => return None, } - _ => return None, - }; + rest = &rest[1..]; + } + + // (1) One or more accumulator statements over the SAME local. + let mut acc_id: Option<&u32> = None; + let mut values: Vec<&perry_hir::Expr> = Vec::new(); + for stmt in rest { + let Stmt::Expr(Expr::LocalSet(id, value)) = stmt else { + return None; + }; + match acc_id { + None => acc_id = Some(id), + Some(seen) if seen == id => {} + Some(_) => return None, + } + values.push(value.as_ref()); + } + let acc_id = acc_id?; if *acc_id == counter_id || !ctx.locals.contains_key(acc_id) || ctx.boxed_vars.contains(acc_id) || ctx.module_globals.contains_key(acc_id) + || carried.is_some_and(|c| c.carried_id == *acc_id || c.modulus_id == *acc_id) { return None; } + // #10185: statements 2..K are folded into statement 1 for the fast clone + // (`acc = a; acc = acc + b` ≡ `acc = (a) + b`), so that the WHOLE iteration + // commits once, after every side exit it can take. Each of them must + // therefore be `acc ` with `acc` as the left operand — the shape + // every compound assignment lowers to — and the right operand must not read + // `acc`, or substituting it would make it see the pre-statement value. + for value in &values[1..] { + let Expr::Binary { left, right, .. } = value else { + return None; + }; + if !matches!(left.as_ref(), Expr::LocalGet(id) if id == acc_id) { + return None; + } + if super::element_shape_carried::expr_reads_local(right, *acc_id) { + return None; + } + } + // The binding form pins the array before the walk runs, so a body mixing // `r.field` with `other[j].field` is declined by the walk's one-array rule. let scope = PureExprScope { @@ -897,15 +1097,22 @@ fn match_element_shape_versioned_loop( accumulator_id: *acc_id, element_binding: element_binding.map(|(id, _)| id), derived, + carried: carried.map(|c| (c.carried_id, carried_alias)), }; - if scope.element_binding == Some(*acc_id) || derived.map(|(d, _)| d) == Some(*acc_id) { + if scope.element_binding == Some(*acc_id) + || derived.map(|(d, _)| d) == Some(*acc_id) + || carried_alias == Some(*acc_id) + { return None; } let mut facts = PureExprFacts { array: element_binding.map(|(_, arr_id)| arr_id), ..PureExprFacts::default() }; - if !element_shape_loop_pure_expr_collect(ctx, value, &scope, &mut facts) { + if !values + .iter() + .all(|value| element_shape_loop_pure_expr_collect(ctx, value, &scope, &mut facts)) + { return None; } let array_id = facts.array?; @@ -927,12 +1134,35 @@ fn match_element_shape_versioned_loop( return None; } } + // #10185: a matched recurrence whose value nothing subscripts with would + // still have its `LocalSet` lowered generically — a `frem` libcall inside + // the clone, which deletes it. Decline instead, so the loop keeps whatever + // lowering it has today. + match (carried.as_ref(), index) { + (Some(matched), MatchedIndex::Carried { .. }) => { + if matched.carried_id == array_id + || matched.modulus_id == array_id + || carried_alias == Some(array_id) + { + return None; + } + } + // A recurrence the subscripts do not use, or a carried subscript with + // no recurrence (which the walk's scope makes unreachable). + (Some(_), _) | (None, MatchedIndex::Carried { .. }) => return None, + (None, _) => {} + } match bound { ElementShapeLoopBound::Local(bound_id) => { if bound_id == array_id || bound_id == *acc_id || Some(bound_id) == scope.element_binding || derived.map(|(d, _)| d) == Some(bound_id) + // The bound is materialized ONCE in the preheader, so a bound + // the recurrence rewrites every iteration would be stale. The + // carried local is also not readable bare inside the clone. + || carried.is_some_and(|c| c.carried_id == bound_id) + || carried_alias == Some(bound_id) { return None; } @@ -975,12 +1205,6 @@ fn match_element_shape_versioned_loop( return None; } - for prop in &facts.props { - if CLASS_FIELD_LOOP_PROP_DENYLIST.contains(&prop.as_str()) { - return None; - } - } - let identity = match element_class_name(ctx, array_id, counter_id) { Some(class_name) => { // The class arm keeps its ORIGINAL grammar. Its bounds argument is @@ -999,6 +1223,42 @@ fn match_element_shape_versioned_loop( None => return None, }; + // The property denylist, which is arm-specific (#10185). + // + // The CLASS arm's list exists because its read bakes in a compile-time + // packed slot index while the surrounding lowering may route the name + // somewhere else entirely (`length` header loads, `errors` runtime calls, + // accessor-ish names) — a name collision there costs the call-free + // guarantee. + // + // The SHAPE arm bakes in nothing: the preheader asks the runtime for the + // inline slot of that exact key in that exact ordinary ShapeId and declines + // on `-1`, and the per-element residual pins `obj_type == GC_TYPE_OBJECT` + // with no per-object descriptors. Every receiver kind whose builtin branch + // could answer a name differently — a function's `name`, an array's + // `length`, a Map's `size`, an AggregateError's `errors` — fails that + // residual, and for a plain record an own data property SHADOWS the + // prototype name it collides with, which is exactly what the inline slot + // holds. The whole list would otherwise have cost the benchmark its + // `fields` shape outright, for a field literally called `name`. + // + // `__proto__` stays denied, and not because of JavaScript: node gives + // `JSON.parse('{"__proto__":1}')` an OWN `__proto__` data property and + // reads `1` back from it, so the inline slot would be right. It is denied + // because Perry's generic property path may special-case the name ahead of + // own-property lookup, and the clone must agree with the clone-of, not with + // the spec, wherever those two differ. + const SHAPE_PROP_DENYLIST: &[&str] = &["__proto__"]; + let denylist = match identity { + ElementShapeIdentity::Class { .. } => CLASS_FIELD_LOOP_PROP_DENYLIST, + ElementShapeIdentity::Shape => SHAPE_PROP_DENYLIST, + }; + for prop in &facts.props { + if denylist.contains(&prop.as_str()) { + return None; + } + } + // The declared accumulator type is only a candidate: the lowering // validates the accumulator's current NaN-box tag in the preheader before // installing the numeric fact for the fast clone. The class arm keeps its @@ -1018,6 +1278,40 @@ fn match_element_shape_versioned_loop( return None; } + // #10185: the fast clone's body, when the two rewrites apply. `lead` is + // whatever statements the walk consumed ahead of the accumulator run (the + // recurrence and/or the virtual binding), kept verbatim. + let lead = &body[..body.len() - rest.len()]; + let fast_body = (values.len() > 1 || carried.is_some()).then(|| { + let mut stmts: Vec = lead.to_vec(); + // `acc = v1; acc = acc p2; acc = acc p3` + // ≡ `acc = ((v1) p2) p3` + // — the same operations on the same operands in the same order, which + // is what keeps the folded float arithmetic bit-identical. + let mut folded = values[0].clone(); + for value in &values[1..] { + let Expr::Binary { op, right, .. } = value else { + unreachable!("the accumulator fold validated every statement's shape"); + }; + folded = Expr::Binary { + op: *op, + left: Box::new(folded), + right: right.clone(), + }; + } + stmts.push(Stmt::Expr(Expr::LocalSet(*acc_id, Box::new(folded)))); + if let Some(matched) = carried.as_ref() { + // The commit marker (`stmt/element_shape_carried.rs`). It is the + // LAST statement, so it runs only once the iteration is past every + // exit it could take. + stmts.push(Stmt::Expr(Expr::LocalSet( + matched.carried_id, + Box::new(Expr::LocalGet(matched.carried_id)), + ))); + } + stmts + }); + Some(ElementShapeVersionedLoop { counter_id, bound, @@ -1027,6 +1321,8 @@ fn match_element_shape_versioned_loop( index, element_binding: scope.element_binding, accumulator_id: *acc_id, + carried, + fast_body, }) } @@ -1200,10 +1496,25 @@ pub(super) fn lower_element_shape_versioned_for( // floor of 1 — `srem` by zero is undefined behaviour, and `x % 0` is NaN // in JS, so a zero modulus is a slow-clone case rather than something the // clone may compute. - let modulus_i32: Option = match matched.index { - MatchedIndex::DerivedMod { modulus_id, .. } => Some(materialize_loop_i32( + // + // #10185's carried recurrence takes the same obligation for the same + // reason, and one more of its own: its ENTRY value must be a non-negative + // integral i32, because the whole i64 recurrence bound + // (`a * i32::MAX + b < 2^53`, and a non-negative dividend for `srem` to + // agree with JS `%`) is stated over that range. A fractional, negative or + // non-number `cursor` at loop entry is a slow-clone case. + let modulus_i32: Option = match (&matched.index, &matched.carried) { + (MatchedIndex::DerivedMod { modulus_id, .. }, _) => Some(materialize_loop_i32( + ctx, + *modulus_id, + 1, + i32::MAX, + &slow_pre_label, + "element_shape.loop.modulus", + )?), + (MatchedIndex::Carried { .. }, Some(carried)) => Some(materialize_loop_i32( ctx, - modulus_id, + carried.modulus_id, 1, i32::MAX, &slow_pre_label, @@ -1211,6 +1522,25 @@ pub(super) fn lower_element_shape_versioned_for( )?), _ => None, }; + let carried_slot: Option = match &matched.carried { + Some(carried) => { + let entry = materialize_loop_i32( + ctx, + carried.carried_id, + 0, + i32::MAX, + &slow_pre_label, + "element_shape.loop.carried", + )?; + // Entry-block alloca: LLVM lowers a non-entry `alloca` as a real + // stack bump with no restore, and this one is written every + // iteration. + let slot = ctx.func.alloca_entry(I32); + ctx.block().store(I32, &entry, &slot); + Some(slot) + } + None => None, + }; let trip_count = match &materialized_bound { Some(bound) => { @@ -1225,11 +1555,14 @@ pub(super) fn lower_element_shape_versioned_for( (MatchedIndex::Constant(k), _) => { crate::expr::element_shape_guard::ElementShapeIndexBound::Constant(*k) } - (MatchedIndex::DerivedMod { .. }, Some(modulus)) => { + // #10185: the recurrence's result is `srem` of a non-negative dividend + // by `m`, so it lands in `[0, m)` exactly like the derived index and + // takes exactly the same preheader obligation, `m <= length`. + (MatchedIndex::DerivedMod { .. } | MatchedIndex::Carried { .. }, Some(modulus)) => { crate::expr::element_shape_guard::ElementShapeIndexBound::Modulus(modulus.as_str()) } - (MatchedIndex::DerivedMod { .. }, None) => { - unreachable!("a derived index always materializes its modulus") + (MatchedIndex::DerivedMod { .. } | MatchedIndex::Carried { .. }, None) => { + unreachable!("a modulo-derived index always materializes its modulus") } }; let expected_class_id_str = match &matched.identity { @@ -1323,8 +1656,58 @@ pub(super) fn lower_element_shape_versioned_for( // iteration. slot: ctx.func.alloca_entry(I32), }, + MatchedIndex::Carried { + carried_id, + alias_id, + } => { + let carried = matched + .carried + .as_ref() + .expect("a carried index always matches a recurrence"); + crate::expr::ElementShapeIndex::Carried(Box::new(crate::expr::CarriedIndex { + local_id: carried_id, + alias_id, + coeff_a: carried.coeff_a, + coeff_b: carried.coeff_b, + modulus_i32: modulus_i32 + .clone() + .expect("a carried index always materializes its modulus"), + slot: carried_slot + .clone() + .expect("a carried index always materializes its entry value"), + commit_slot: ctx + .locals + .get(&carried_id) + .cloned() + .expect("the matcher required a directly-addressable local"), + })) + } }; + // #10185: the shared once-per-iteration element deref, hung off the body's + // leading virtual statement. Shape-keyed only: the class-keyed arm's reads + // are raw doubles behind a `GC_OBJ_TYPED_LAYOUT_INTACT` residual whose + // emitted IR #7480's census pins, and nothing in #10185 measures it. + let elem_prefetch = shape_keyed + .then(|| { + let site_local_id = match (&fact_index, matched.element_binding) { + (_, Some(binding)) => Some(binding), + (crate::expr::ElementShapeIndex::DerivedMod { local_id, .. }, _) => Some(*local_id), + // With an alias the prologue belongs to the alias `Let`, which + // runs after the recurrence; without one it belongs to the + // recurrence itself. Exactly one statement owns it either way. + (crate::expr::ElementShapeIndex::Carried(carried), _) => { + Some(carried.alias_id.unwrap_or(carried.local_id)) + } + _ => None, + }?; + Some(crate::expr::ElementPrefetch { + site_local_id, + handle_slot: ctx.func.alloca_entry(I64), + }) + }) + .flatten(); + let scope_id = ctx.next_loop_proof_scope_id(); let fast_scan_start = ctx.func.num_blocks(); ctx.current_block = fast_pre_idx; @@ -1333,6 +1716,7 @@ pub(super) fn lower_element_shape_versioned_for( array_local_id: matched.array_id, index_local_id: matched.counter_id, index: fact_index, + elem_prefetch, shape_keyed, scope_id, class_name: report_class, @@ -1341,6 +1725,7 @@ pub(super) fn lower_element_shape_versioned_for( side_exit_label: slow_pre_label.clone(), statically_layout_proven, fields, + synthesized_body: matched.fast_body.is_some(), element_binding: matched.element_binding, numeric_accumulator: matched.accumulator_id, }); @@ -1349,7 +1734,7 @@ pub(super) fn lower_element_shape_versioned_for( init, condition, update, - body, + matched.fast_body.as_deref().unwrap_or(body), "for.element_shape_fast", Some((matched.counter_id, guard.bound_i32)), ); diff --git a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs index c78cfc72e5..1757e1e2c7 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -402,14 +402,34 @@ fn assert_fast_clone_is_entered(ir: &str) { fn fast_clone_slice(ir: &str) -> String { let mut owned = String::new(); let mut in_fast_block = false; + // #10185: the emitted text carries the function TWICE (same block labels), + // so every block would be collected twice and any `matches().count()` + // assertion against the slice would read double. Stop at the first repeated + // label, which is where the second copy begins — `contains` assertions are + // unaffected, and counting one iteration of the clone is what makes "the + // residual check happens ONCE" expressible at all. + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); for line in ir.split_inclusive('\n') { let trimmed = line.trim_end(); // A block DEFINITION starts at column 0 and ends in `:`; anything else // belongs to whichever block was last opened. if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { + // #10185 added three more: the string-`.length` decode + // (`element_shape.strlen*`), the boolean ternary's admitted arm + // (`element_shape.bool`), and nothing for the shared prefetch, + // which reuses `element_shape.load`. Every block the clone can + // execute must be listed, or the negatives below go vacuous for it. in_fast_block = trimmed.starts_with("for.element_shape_fast.") || trimmed.starts_with("element_shape.load") - || trimmed.starts_with("element_shape.number"); + || trimmed.starts_with("element_shape.number") + || trimmed.starts_with("element_shape.strlen") + || trimmed.starts_with("element_shape.bool"); + // Only a repeated CLONE label means the second copy: unrelated + // functions share ordinary labels (`entry:`), and breaking on one + // of those would slice away the clone entirely. + if in_fast_block && !seen.insert(trimmed.to_string()) { + break; + } } if in_fast_block { owned.push_str(line); diff --git a/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs index 94ade02cc7..6d5d265332 100644 --- a/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs @@ -450,21 +450,78 @@ fn an_untracked_local_index_declines() { } #[test] -fn a_denylisted_property_declines_the_shape_keyed_arm() { - // `length`, `name`, `constructor`, … are answered by the runtime or by the - // prototype, not out of an inline slot, so a shape's key position for one - // would be the wrong answer even when it exists. +fn the_shape_keyed_arm_denies_only_proto() { + // #10185 narrowed this list. The CLASS arm still denies every name with a + // dedicated branch in the property dispatch, because its read bakes in a + // compile-time packed slot. The SHAPE arm bakes in nothing: the preheader + // asks the runtime for that exact key's inline slot in that exact ordinary + // ShapeId and declines on `-1`, and the residual pins + // `obj_type == GC_TYPE_OBJECT` with no per-object descriptors — so every + // receiver whose builtin branch could answer a name differently (a + // function's `name`, an array's `length`, a Map's `size`) is already + // excluded, and for a plain record an own data property shadows the + // prototype name it collides with. Denying the whole list cost the access + // benchmark its `fields` shape outright, for a field called `name`. + for property in ["length", "name", "constructor", "size", "message"] { + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + property, + ))], + )); + assert!( + ir.contains("element_shape.loop.fast.preheader"), + "`{property}` is an ordinary own inline slot on a parsed record; \ + the shape-keyed arm must serve it" + ); + } + // `__proto__` stays denied — not because of JavaScript (node gives + // `JSON.parse('{\"__proto__\":1}')` an own data property and reads `1` + // back), but because Perry's generic property path may special-case the + // name ahead of own-property lookup, and the clone must agree with the + // path it is a clone of. let ir = emit(&untyped_param_module( Type::Any, Type::Number, vec![untyped_accumulate(untyped_elem_field( Expr::LocalGet(U_COUNTER_ID), - "length", + "__proto__", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "`__proto__` must decline the shape-keyed clone" + ); +} + +#[test] +fn the_class_keyed_arm_keeps_the_full_property_denylist() { + // The narrowing above is shape-arm-only: a class-keyed read bakes in a + // packed slot index while the surrounding lowering may route the name + // somewhere else entirely, which is what the list has always protected. + let ir = emit(&element_shape_module( + vec![Stmt::Expr(Expr::LocalSet( + SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM_ID)), + right: Box::new(Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARRAY_ID)), + index: Box::new(Expr::LocalGet(COUNTER_ID)), + }), + property: "length".to_string(), + byte_offset: 0, + }), + }), ))], + None, )); assert!( !ir.contains("element_shape.loop.fast.preheader"), - "a denylisted property must decline the clone" + "a denylisted property must still decline the CLASS-keyed clone" ); } @@ -489,3 +546,9 @@ fn a_declared_but_unresolvable_element_type_still_declines() { "an unresolvable declared element type must decline both arms" ); } + +/// #10185's `fields` and `random` shapes — a child module so it inherits both +/// this file's shape-keyed assertions and `element_shape_loop_tests`'s slicing +/// helpers. +#[path = "element_shape_fields_random_tests.rs"] +mod fields_random; diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 28ec377674..288e799086 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -31,7 +31,7 @@ pub(crate) fn lower_let( // binding is VIRTUAL — the element binding emits nothing and the derived // index emits one `srem`. Both live with the clone, which is where their // soundness arguments and the preheader that validated them are. - if super::element_shape_loop::lower_virtual_clone_binding(ctx, id) { + if super::element_shape_loop::lower_virtual_clone_binding(ctx, id)? { return Ok(()); } // `let C = SomeClass` aliases the local `C` to the class diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index cc407d30c0..cc27d26763 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -20,6 +20,7 @@ mod cached_field_index_return; #[cfg(test)] mod class_field_loop_tests; mod counter_range; +mod element_shape_carried; mod element_shape_loop; #[cfg(test)] mod element_shape_loop_tests; @@ -281,6 +282,15 @@ fn lower_return_expr(ctx: &mut FnCtx<'_>, expr: &perry_hir::Expr) -> Result, stmt: &Stmt) -> Result<()> { match stmt { Stmt::Expr(e) => { + // #10185: the element-shape fast clone's carried-index statements + // (the recurrence and its trailing write-back) are lowered + // VIRTUALLY, exactly like the `Let` bindings in `let_stmt.rs` — + // the generic lowering of `c = (c * 17 + 7) % length` is an + // `frem` libcall, and a call inside the clone DELETES it (#7690). + // Outside a clone the fact vector is empty and this is a no-op. + if element_shape_carried::lower_virtual_carried_stmt(ctx, e)? { + return Ok(()); + } let prev_discard = ctx.discard_expr_value; ctx.discard_expr_value = true; // #7590: the non-leaking companion. `lower_expr` takes this at the diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index e3677034e2..338442e398 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -356,6 +356,18 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { Expr::Logical { left, right, .. } => { is_numeric_expr(ctx, left) && is_numeric_expr(ctx, right) } + // #10185: `arr[i]. ? 1 : 0` inside an element-shape fast + // clone. Deliberately NOT the general "both arms are numeric" rule — a + // general ternary's CONDITION is an arbitrary JS truthiness test, which + // is a runtime call; this arm is true only for the exact shape the + // clone lowers itself, where the condition is a tracked element read + // admitted as one of the two boolean singletons and everything else + // side-exits. + Expr::Conditional { .. } + if crate::expr::element_shape_reads::cloned_bool_select_read(ctx, e).is_some() => + { + true + } // `obj.field` where the field is declared as `number` on the // owning class. Without this, `this.value + 1` in a hot loop // wraps the field load in `js_number_coerce` which prevents @@ -374,6 +386,17 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { if property == "length" && expression_has_numeric_length(ctx, object) { return true; } + // #10185: `arr[i]..length` inside an element-shape + // fast clone. The receiver is an untyped element read, so the + // declared-type answer above cannot see a string there; the clone's + // own lowering tag-tests the loaded word for both string + // representations and side-exits when it is not one, which is a + // stronger proof than an annotation. Without this the `+` consuming + // it bails to `js_dynamic_string_or_number_add` and the call + // deletes the clone. + if crate::expr::element_shape_reads::cloned_string_length_read(ctx, e).is_some() { + return true; + } // repsel #7480 step 3: inside an element-shape fast clone a tracked // `arr[i].field` read is a GUARD-PROVEN raw double. The class-keyed // arm gets that from the residual check's diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 0626294170..df47d5af22 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -652,7 +652,13 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: "emit_proven_shape_recheck", "emit_class_field_inline_precheck", )), - (raw_element_guard, ("emit_element_shape_field_load",)), + # #10185: the element clone's residual moved out of + # `emit_element_shape_field_load` into the deref helper both the + # per-read path and the shared once-per-iteration prologue call, so the + # ShapeId read this asserts now lives there. Naming the function that + # actually emits the check is what keeps the assertion from going + # vacuous the next time the emitter is split. + (raw_element_guard, ("emit_element_deref_with_residual",)), ): for name in names: body = function_body(source, name) diff --git a/test-files/test_gap_json_record_loop_clone.ts b/test-files/test_gap_json_record_loop_clone.ts index 0a60a97359..363fb7a04b 100644 --- a/test-files/test_gap_json_record_loop_clone.ts +++ b/test-files/test_gap_json_record_loop_clone.ts @@ -270,3 +270,301 @@ function sumTwoFields(rows: any, count: number): number { return sum; } console.log("two-fields:", sumTwoFields(moduleRows, moduleLength)); + +// --------------------------------------------------------------------------- +// 9. #10185: the LOOP-CARRIED index (`random`), the K-statement accumulator +// fold, and the two non-numeric reads. +// +// Every case below is a MISCOMPILE if it is wrong, not a slow path: +// +// * the carried write-back is placed at the END of the iteration, so a +// mid-iteration side exit leaves the real binding holding that +// iteration's ENTRY value and the slow clone advances the recurrence +// exactly once. A write-back at the update site double-applies it, and +// every later index is silently a different (still valid) record; +// * K accumulator statements fold to one store for the same reason: a +// side exit from the third read must not leave the first two applied; +// * `.length` and the ternary read a NaN-boxed word and assume a +// representation, so each tag-tests it and side-exits otherwise. +// --------------------------------------------------------------------------- + +// The benchmark's `random` mode, with the `const index = cursor` alias. +function randomSum(rows: any, count: number, n: number): number { + let sum = 0; + let cursor = 0; + for (let i = 0; i < count; i++) { + cursor = (cursor * 17 + 7) % n; + const index = cursor; + sum += rows[index].id; + } + return sum; +} + +// The same recurrence subscripted DIRECTLY, with the carried value read after +// the loop — the clone owes a write-back, and this is what observes it. +function randomSumAndCursor( + rows: any, + count: number, + n: number, + start: number, +): string { + let sum: any = 0; + let cursor = start; + for (let i = 0; i < count; i++) { + cursor = (cursor * 17 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// A multiplier large enough that `a * cursor` leaves the range an `f64` +// represents exactly. JavaScript evaluates this in doubles, so an i64 +// recurrence would NOT agree — the matcher must decline and let the ordinary +// lowering run. +function randomSumWideMultiplier( + rows: any, + count: number, + n: number, +): string { + let sum = 0; + let cursor = 3; + for (let i = 0; i < count; i++) { + cursor = (cursor * 600000000 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// A recurrence with an even step, so a fractional carried value still lands +// on whole-number indices after the first update. +function randomSumEvenStep( + rows: any, + count: number, + n: number, + start: number, +): string { + let sum = 0; + let cursor = start; + for (let i = 0; i < count; i++) { + cursor = (cursor * 2 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// The benchmark's `fields` mode: three accumulator statements over ONE +// element, one through a string and one through a boolean. +function fieldsSum(rows: any, count: number, n: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % n; + sum += rows[index].id; + sum += rows[index].name.length; + sum += rows[index].active ? 1 : 0; + } + return sum; +} + +const randomRows: any = JSON.parse(buildRecords(40, "pad")); +console.log("random-sum:", randomSum(randomRows, 200, randomRows.length)); +console.log( + "random-sum-again:", + randomSum(randomRows, 200, randomRows.length), +); +console.log( + "random-cursor-live-after:", + randomSumAndCursor(randomRows, 200, randomRows.length, 0), +); +// An odd trip count observes a different point of the cycle than the run +// above, so the write-back is checked at two places rather than one. +console.log( + "random-cursor-live-after-odd:", + randomSumAndCursor(randomRows, 201, randomRows.length, 0), +); +// A modulus smaller than the array. 13 is chosen because `(17c + 7) % 13` +// walks six distinct indices from 0 — several plausible moduli make the +// recurrence a fixed point at 0, where every assertion below would hold of a +// loop that read one element forever. +console.log( + "random-small-modulus:", + randomSumAndCursor(randomRows, 50, 13, 0), +); +console.log( + "random-modulus-one:", + randomSumAndCursor(randomRows, 9, 1, 0), +); + +// Entry values the preheader must reject: JS `%` returns a NEGATIVE remainder +// for a negative dividend, and a fractional index is not an index at all. +// Both take the slow clone and keep ordinary JavaScript semantics. +try { + console.log( + "random-negative-start:", + randomSumAndCursor(randomRows, 4, 5, -3), + ); +} catch (err) { + console.log("random-negative-start-threw:", String(err).slice(0, 9)); +} +// A FRACTIONAL entry value. The step is even so every index the loop +// actually reads is a whole number — the divergence being checked is the +// preheader's, not the subscript's: if the clone materialized `0.5` as the +// i32 `0` it would start the recurrence one step early and every index would +// shift, which the sum and the final cursor both show. +console.log( + "random-fractional-start:", + randomSumEvenStep(randomRows, 6, 5, 0.5), +); +console.log( + "random-integral-start-same-step:", + randomSumEvenStep(randomRows, 6, 5, 0), +); +// `x % 0` is NaN, and `rows[NaN]` is `undefined`. +try { + console.log("random-modulus-zero:", randomSumAndCursor(randomRows, 3, 0, 0)); +} catch (err) { + console.log("random-modulus-zero-threw:", String(err).slice(0, 9)); +} +// A modulus past the array's length runs off the end. +try { + console.log( + "random-modulus-past-length:", + randomSumAndCursor(randomRows, 30, 500, 0), + ); +} catch (err) { + console.log("random-modulus-past-length-threw:", String(err).slice(0, 9)); +} +console.log( + "random-wide-multiplier:", + randomSumWideMultiplier(randomRows, 25, 11), +); + +// THE write-back test. The array is homogeneous in SHAPE, so the clone is +// entered — but one `id` is a string, so the per-read Number tag test side +// -exits mid-iteration and the slow clone re-runs it. If the recurrence had +// already been committed, the slow clone advances it a SECOND time and both +// the sum and the final cursor drift. +// `(17c + 7) % 5` walks 0 -> 2 -> 1 -> 4 -> 0, so index 2 — the string one — +// really is read. A modulus that skipped it would make this whole case +// vacuous while still printing a matching number. +const randomMixed: any = JSON.parse( + '[{"id":0},{"id":1},{"id":"two"},{"id":3},{"id":4}]', +); +console.log( + "random-side-exit:", + randomSumAndCursor(randomMixed, 30, randomMixed.length, 0), +); +console.log( + "random-side-exit-again:", + randomSumAndCursor(randomMixed, 30, randomMixed.length, 0), +); + +// The recurrence over a HETEROGENEOUS array declines at the array level. +console.log( + "random-hetero:", + randomSumAndCursor(hetero, 16, hetero.length, 0), +); + +// --------------------------------------------------------------------------- +// 9b. The `fields` shape. +// --------------------------------------------------------------------------- + +// SSO names (<= 5 bytes, packed into the NaN-box) and heap names in the SAME +// array — the shape is identical either way, and the `.length` decode has to +// handle both. +const mixedNames: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},' + + '{"id":2,"name":"user_222","active":false},' + + '{"id":3,"name":"","active":true},' + + '{"id":4,"name":"abcde","active":false},' + + '{"id":5,"name":"abcdef","active":true}]', +); +console.log("fields-sso-and-heap:", fieldsSum(mixedNames, 25, mixedNames.length)); +console.log( + "fields-sso-and-heap-again:", + fieldsSum(mixedNames, 25, mixedNames.length), +); + +// A NON-STRING `name`: `.length` of a number is `undefined`, so the sum goes +// NaN — and the clone must side-exit rather than decode a double as a string +// header. The fold is what keeps the earlier `id` from being applied twice. +const numberName: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":7,"active":true},' + + '{"id":3,"name":"cd","active":false}]', +); +console.log("fields-number-name:", fieldsSum(numberName, 9, numberName.length)); + +// A NULL `name`: `null.length` throws, exactly as JS says. +const nullName: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":null,"active":true}]', +); +try { + console.log("fields-null-name:", fieldsSum(nullName, 4, nullName.length)); +} catch (err) { + console.log("fields-null-name-threw:", String(err).slice(0, 9)); +} + +// NON-BOOLEAN `active` values. JS truthiness of `0`, `""`, `"no"` and `null` +// is a runtime question the clone is not allowed to guess: only the two +// boolean singletons are admitted and everything else side-exits. +const truthyActive: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":"cd","active":0},' + + '{"id":3,"name":"ef","active":"no"},{"id":4,"name":"gh","active":""},' + + '{"id":5,"name":"ij","active":null},{"id":6,"name":"kl","active":false}]', +); +console.log( + "fields-truthiness:", + fieldsSum(truthyActive, 24, truthyActive.length), +); +console.log( + "fields-truthiness-again:", + fieldsSum(truthyActive, 24, truthyActive.length), +); + +// Two accumulator statements where the SECOND read side-exits: the first must +// not be applied twice when the slow clone re-runs the iteration. +function twoStatementSum(rows: any, count: number, n: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % n; + sum += rows[index].id; + sum += rows[index].score; + } + return sum; +} +const secondFieldMixed: any = JSON.parse( + '[{"id":1,"score":10},{"id":2,"score":"x"},{"id":3,"score":30}]', +); +console.log( + "fold-side-exit:", + twoStatementSum(secondFieldMixed, 9, secondFieldMixed.length), +); + +// Heterogeneous shapes with the multi-read body. +console.log("fields-hetero:", fieldsSum(hetero, 12, hetero.length)); + +// --------------------------------------------------------------------------- +// 9c. Property names the CLASS-keyed arm denies. A parsed record's own data +// property SHADOWS the prototype name it collides with, and the +// shape-keyed preheader resolves it from the runtime shape table, so +// `name` and `length` are ordinary inline slots here. `name` in +// particular is what the access benchmark's own `fields` mode reads. +// --------------------------------------------------------------------------- +function ownNamedFields(rows: any, count: number, n: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % n; + sum += rows[index].length; + sum += rows[index].size; + sum += rows[index].name.length; + } + return sum; +} +const ownNames: any = JSON.parse( + '[{"length":3,"size":4,"name":"ab"},{"length":5,"size":6,"name":"cdef"},' + + '{"length":7,"size":8,"name":"ghijklm"}]', +); +console.log("own-named-fields:", ownNamedFields(ownNames, 12, ownNames.length)); +console.log( + "own-named-fields-again:", + ownNamedFields(ownNames, 12, ownNames.length), +);