diff --git a/changelog.d/inline-store-tier-covers-tagged-arrays.md b/changelog.d/inline-store-tier-covers-tagged-arrays.md new file mode 100644 index 0000000000..ed8abda4e8 --- /dev/null +++ b/changelog.d/inline-store-tier-covers-tagged-arrays.md @@ -0,0 +1,49 @@ +The inline array-store guard now covers receivers that are not raw-f64. + +`lower_index_set_fast` builds a guard that tests, inline, everything the +out-of-line `js_typed_feedback_plain_array_index_set_guard` tests: array type, +not-forwarded, no element descriptors, the integrity flags, the prototype-chain +invalidation byte, and the length/capacity sanity bounds. It then jumps straight +to the store, skipping the call. + +That whole tier was gated on `require_numeric_layout`, so it was built only for +statically numeric receivers. A `boolean[]` — or any downgraded `any[]` — went to +the call on every store, forever, even though the in-bounds arm below already +knows how to store a tagged value into such a receiver (that arm is exactly what +the out-of-line guard fronts today). + +Only two of the guard's conditions belong to the raw-f64 store, and they are now +applied only when that store is the one being emitted: + +* the receiver's raw-f64 layout bits, because the raw arm writes an unboxed + double into the slot and that is valid only while the layout says the elements + are pointer-free — and a downgraded receiver has those bits clear by + definition, which is precisely why requiring them pinned `boolean[]` to the + call tier; +* the runtime numeric-tag test on the stored value, which exists because a + `number[]` slot can genuinely receive a non-number and the raw arm would write + its NaN-boxed tag verbatim. The tagged arm stores the box as a box. + +Measured on an idle Mac mini, all binaries built in one run, interleaved, min of +five, self-timed: + +| | main | with #9246 | this change | node | +|---|---:|---:|---:|---:| +| boolean-store loop | 207 ms | 138 ms | **64 ms** | 12 ms | +| `11_prime_sieve` | 27 ms | 20 ms | **11 ms** | 6 ms | + +`11_prime_sieve` moves from 4.5× Node to 1.8×. A nested-loop read benchmark is +unchanged, as expected. + +Verified against Node on a differential written for this change specifically — +the guard must reject exactly what the call rejects: a frozen array (stores +ignored), a sealed array and one under `preventExtensions` (in-bounds writes +allowed, growth refused), an element accessor descriptor (the setter must run), +extension past length, mixed types through one slot, and a store into an array +that was numeric. Byte-identical, plus five pre-existing differentials unchanged. + +One case in that differential diverges from Node — an `Array.prototype` index +setter installed with `Object.defineProperty` is bypassed — and it diverges +**identically on unmodified main, for numeric receivers too**, so it is neither +caused nor widened here. Filed separately; the flag the guards consult is only +set by an index *write* to the prototype, never by `defineProperty`. diff --git a/crates/perry-codegen/src/expr/index.rs b/crates/perry-codegen/src/expr/index.rs index 4b393b5f1d..2a94aed43b 100644 --- a/crates/perry-codegen/src/expr/index.rs +++ b/crates/perry-codegen/src/expr/index.rs @@ -209,7 +209,19 @@ pub(crate) fn lower_index_set_fast( // predicate (see `value_numeric` below) whenever it is not statically // known, so the tier covers those stores too and the flag only decides // whether those three instructions are emitted at all. - let inline_write_tier = require_numeric_layout && !super::typed_feedback_emission_enabled(); + // #9237: the tier is no longer restricted to statically numeric receivers. + // Every condition the out-of-line guard checks is already tested inline here + // — array type, not-forwarded, no element descriptors, integrity flags, the + // prototype-chain invalidation flag, and the length/capacity sanity bounds — + // and the in-bounds arm below already knows how to store a tagged value into + // a non-raw-f64 receiver (that arm is what the out-of-line guard fronts + // today). Only two of the conditions are specific to the raw-f64 store, and + // they are now applied only when it is the one being emitted, so a + // `boolean[]` store reaches the same inline guard instead of paying + // `js_typed_feedback_plain_array_index_set_guard` per element: ~41% of such + // a loop, and the reason `benchmarks/suite/11_prime_sieve.ts` sits on the + // call tier for its whole run. + let inline_write_tier = !super::typed_feedback_emission_enabled(); let cold_guard_idx = if inline_write_tier { Some(ctx.new_block("idxset.guard.cold")) } else { @@ -288,13 +300,27 @@ pub(crate) fn lower_index_set_fast( let mut guard_ok = blk.and(I1, &is_array, ¬_forwarded); guard_ok = blk.and(I1, &guard_ok, &integrity_clean); - guard_ok = blk.and(I1, &guard_ok, &is_dense); + if require_numeric_layout { + // Raw-f64 layout is a precondition of the RAW store only: that + // arm writes an unboxed double into the slot, which is valid + // only while the receiver's layout says its elements are + // pointer-free. A tagged store writes a NaN-boxed JSValue and + // is correct whatever the layout says — and for a downgraded + // receiver these bits are clear by definition, which is exactly + // why requiring them kept `boolean[]` on the call tier forever. + guard_ok = blk.and(I1, &guard_ok, &is_dense); + } guard_ok = blk.and(I1, &guard_ok, &default_prototype_chain); guard_ok = blk.and(I1, &guard_ok, &index_nonnegative); guard_ok = blk.and(I1, &guard_ok, &length_sane); guard_ok = blk.and(I1, &guard_ok, &capacity_sane); guard_ok = blk.and(I1, &guard_ok, &length_within_capacity); - if !value_is_canonical_raw_f64 { + // #9237: kept exactly where it is load-bearing. The comment below + // is about the RAW store — a `number[]` slot can genuinely receive a + // non-number at runtime, and writing its NaN-boxed tag verbatim as a + // double is the bug being prevented. The tagged arm stores the box + // as a box, so the test is dead work there and only there. + if require_numeric_layout && !value_is_canonical_raw_f64 { // #7396: the out-of-line guard's `is_numeric_value_bits(value)` // leg, inlined. It is load-bearing and NOT implied by // `require_numeric_layout`: that is a *static* TypeScript