Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions changelog.d/inline-store-tier-covers-tagged-arrays.md
Original file line number Diff line number Diff line change
@@ -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`.
32 changes: 29 additions & 3 deletions crates/perry-codegen/src/expr/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -288,13 +300,27 @@ pub(crate) fn lower_index_set_fast(

let mut guard_ok = blk.and(I1, &is_array, &not_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
Expand Down
Loading