diff --git a/CLAUDE.md b/CLAUDE.md index e70081e6a4..01bbdb8397 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1402 +**Current Version:** 0.5.1403 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 1c737cca79..abe3181c3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1402" +version = "0.5.1403" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1402" +version = "0.5.1403" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1402" +version = "0.5.1403" [[package]] name = "perry-ui-tvos" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1402" +version = "0.5.1403" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index a2903bb95d..c239494386 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1402" +version = "0.5.1403" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7701-element-shape-loop-applicability.md b/changelog.d/7701-element-shape-loop-applicability.md new file mode 100644 index 0000000000..be4e0a0f7a --- /dev/null +++ b/changelog.d/7701-element-shape-loop-applicability.md @@ -0,0 +1,106 @@ +### Performance + +- **The element-shape versioned loop clone now fires on the idiom people actually write** (#7480 step 4, repsel/#5093). + + #7612 landed the clone and #7669 taught it object-literal element types. Neither + moved `churn_read.ts` by a millisecond — it sat at 0.35 s across four measurement + rounds while everything around it improved 4–11×. The lowering was never the + problem. The **matcher** declined before reaching it, for two independent reasons, + each on its own fatal: + + 1. **`for (let j = 0; j < arr.length; j++)` failed the bound match.** The bound + admitted a literal or a loop-invariant local; `keep.length` is a `PropertyGet`, + so the match returned `None` at the condition, before the class resolver was + ever consulted. That is the bound form every one of #7480's own kernels — and + `churn_read`, `churn`, `retain` — is written in. + 2. **`type Node = { v: number; w: number }` shadowed the anon-shape resolver.** + `element_class_name` returns `receiver_class_name`'s answer when it has one, and + for an alias-typed array that answer is `"Node"` — a name no `ctx.classes` entry + owns, because the literals allocate an `__AnonShape_…`. The early return meant + the resolver #7669 added for exactly this case was never reached for any + alias-typed array. + + Both are fixed here. `element_class_name` only takes the named answer when it names + a real class; the anon-shape resolver follows `type` aliases (bounded, on both the + array and the element level, so `type Row = Node[]` resolves too). A new + `ElementShapeLoopBound::ArrayLength` admits `j < arr.length` **for the array the + body reads** — a foreign array's length is declined, because the preheader proves + nothing about how two lengths relate and the result would be an out-of-range read + rather than a slow clone. + + The `.length` trip count costs nothing to materialize: the guard's deref block + already loads `ArrayHeader.length`, so the usual `length >= bound` comparison + collapses and the only remaining obligation is that the `u32` fits a non-negative + `i32` (one `icmp`; the clone's counter is an i32 and its trip test is signed). + Hoisting `arr.length` out of the condition is not something JS licenses in general + — the property is re-read every iteration — and it is sound here for the same + reason the whole clone is: the matcher admits no store and no call in the body, and + every way to change an array's length is one or the other. The slow clone keeps + re-reading it. + +- **A guarded fast clone no longer emits the two calls that silently delete it.** + + `lower_element_shape_versioned_for` and `lower_class_field_versioned_for` both + build their fast clone first and prove it call-free second; when the proof + fails they terminate the guard with an *unconditional* branch to the slow + clone and leave the fast blocks as unreachable code. So a call emitted into + one of these clones does not make it slower — it removes it, silently, with + every block label still present for an IR census to find. Two lowerings were + doing exactly that to the element-shape clone: + + - **The GC back-edge poll.** #7690 restored moving-loop polls to ON by + default, putting a `js_gc_loop_safepoint()` in the clone's element-load + block. Measured on `churn_read.ts`: with polls off the clone runs (0.03 s + user); with polls on the guard emits `br label %element_shape.loop.slow.preheader` + and the program runs the generic body (0.54 s). Skipping the poll inside a + clone is not a new licence — a poll exists so an *allocating* loop can defer + a collection, which is why `loop_may_allocate` already gates it. That + predicate answers from the HIR body, before specialization, so it cannot see + that `arr[j].f` lowers to a bare load here. Inside a clone's fact scope it + can: the clone is call-free or it is not entered, and the slow clone — + lowered after the scope is popped — keeps its poll. + - **The loop-invariant `arr.length` hoist.** With a `.length` bound the clone + got a *second* `plen` diamond (`js_value_length_f64`) whose result nothing + reads, because the caller had already handed the trip count in as + `precomputed_i32_bound`. Only the load is skipped; the bounded-index and + buffer-width facts and the i32 counter slot are proofs and storage the + clone's other lowering may depend on. + + This is the failure mode `stmt/element_shape_loop.rs`'s module docs named in + advance — "a silent loss of the optimization … with no test failing" — and + every `perry-codegen` test was green throughout. `assert_fast_clone_is_entered` + now asserts the guard branches *into* the clone, and it is sabotage-tested: + with the suppression removed, the two `.length`-bound tests go red with + "the deref block ends in an unconditional branch to the slow clone", so a green + run is evidence rather than decoration. + + **The class-field clone is not affected today**, and that was measured rather + than assumed: with the suppression removed its three IR tests stay green, + because `loop_may_allocate` already proves an `obj.field`-only body inert and + emits no poll for it. The suppression covers it anyway — the two clones rest on + the identical argument — and its tests gained the same liveness assertion, plus + the same by-name slice fix, so the next body shape admitted to that matcher + cannot delete its clone unnoticed either. A third instance, + `lower_object_array_write_versioned_for`, has the same scan and no fact scope + to key off; it is a write-path loop, is not covered by the benchmarks measured + here, and is left for a follow-up rather than changed blind. + +### Fixed + +- **`fast_clone_slice` could attribute a neighbour's call to the fast clone** + (`stmt/element_shape_loop_tests.rs`). + + The IR census sliced "everything between the fast clone's cond block and the slow + clone's", which is the fast clone only while nothing else is emitted in between. An + `arr.length` bound makes the slow clone hoist its own length read, and those + `plen.*` blocks land in the gap — so the call-free census failed on a fast clone + that was, and still is, bare. It now selects blocks by name (`for.element_shape_fast.*` + and `element_shape.load`), keeping the anti-vacuity assertion that made it a gate. + This is the mirror image of the #7480 step 3 bug the function's own doc comment + records: a span that depends on what a neighbour emits can report wrong in either + direction, so ownership is asked of the block. + + New coverage: the `.length` bound fires and stays call-free; a foreign array's + `.length` is declined; an aliased element type resolves; an unresolvable `Named` + element type with no class and no alias is declined rather than guessed; and + `churn_read.ts`'s exact shape (alias **and** `.length` together) reaches the clone. diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index aa42ecf28f..cf6af16885 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -80,6 +80,29 @@ const ELEM_HEADER_MASK: &str = "402686207"; // 0x1800_80FF /// not forwarded, no per-object descriptors, typed layout intact. const ELEM_HEADER_EXPECT: &str = "268435458"; // 0x1000_0002 +/// Where the fast clone's trip count comes from. +/// +/// The two arms differ in *which* fact the preheader has to prove. A bound the +/// caller materialized is an independent number, so the preheader must show the +/// verified prefix reaches it. `arr.length` is not independent — it is the very +/// word this guard loads — so the comparison collapses and only the i32-range +/// obligation is left. +#[derive(Clone, Copy, Debug)] +pub(crate) enum ElementShapeLoopTripCount<'a> { + /// An i32 SSA value (or literal) materialized before the guard. Requires + /// `length >= bound`. + Bound(&'a str), + /// `for (j = …; j < arr.length; j++)` over the array the clone reads: the + /// length loaded inside the guard becomes the trip count. + /// + /// Hoisting `arr.length` out of the condition is a semantic change JS does + /// not license in general — the property is re-read every iteration. It is + /// sound *here* for the same reason the whole clone is: the matcher admits + /// no store and no call in the body, and every way to change an array's + /// length is one or the other. The slow clone keeps re-reading it. + ArrayLength, +} + /// Emit the once-per-loop element-shape guard into the current block chain. /// /// Leaves `ctx.current_block` on an UNTERMINATED block holding the accumulated @@ -104,15 +127,15 @@ const ELEM_HEADER_EXPECT: &str = "268435458"; // 0x1000_0002 /// an allocation can move the array, so a base derived before it could be a /// from-space address. /// -/// Returns `(elements_base, expected_keys, shape_ok)`. +/// Returns `(elements_base, expected_keys, shape_ok, bound_i32)`. pub(crate) fn emit_element_shape_loop_preheader_check( ctx: &mut FnCtx, array_local_id: u32, expected_class_id: &str, keys_global_name: &str, - bound_i32: &str, + trip_count: ElementShapeLoopTripCount<'_>, slow_label: &str, -) -> anyhow::Result<(String, String, String)> { +) -> anyhow::Result<(String, String, String, String)> { let brand_idx = ctx.new_block("element_shape.loop.preheader.brand"); let repair_idx = ctx.new_block("element_shape.loop.preheader.repair"); let query_idx = ctx.new_block("element_shape.loop.preheader.query"); @@ -255,7 +278,24 @@ pub(crate) fn emit_element_shape_loop_preheader_check( // loop reads". The matcher already pinned `start >= 0`. let len_ptr = blk.inttoptr(I64, &handle1); let length = blk.load(I32, &len_ptr); - let len_ok = blk.icmp_uge(I32, &length, bound_i32); + let (bound_i32, len_ok) = match trip_count { + ElementShapeLoopTripCount::Bound(bound) => { + (bound.to_string(), blk.icmp_uge(I32, &length, bound)) + } + // #7480 step 4: `for (j = 0; j < arr.length; j++)` — the trip count IS + // the length this block just read, so "the verified prefix covers every + // index" is true by construction and there is nothing to compare it + // against. What still has to be proven is that the u32 fits a + // non-negative i32: the clone's counter is an i32 and the emitted trip + // test is signed, so a length above `i32::MAX` would read as negative + // and run zero iterations while the slow clone ran billions. No such + // array is allocatable today (it would need 32 GB of element slots), + // which is exactly why the check is one `icmp` rather than a comment. + ElementShapeLoopTripCount::ArrayLength => { + let fits = blk.icmp_sgt(I32, &length, "-1"); + (length.clone(), fits) + } + }; // Elements base: `arr + size_of::()`. let base_addr = blk.add(I64, &handle1, "8"); @@ -275,7 +315,7 @@ pub(crate) fn emit_element_shape_loop_preheader_check( acc = blk.and(I1, &acc, &gate_ok); // No terminator: the caller branches after proving the clone call-free. - Ok((elements_base, expected_keys, acc)) + Ok((elements_base, expected_keys, acc, bound_i32)) } /// Emit one `arr[i].field` read inside the fast clone: bare element load, diff --git a/crates/perry-codegen/src/stmt/class_field_loop_tests.rs b/crates/perry-codegen/src/stmt/class_field_loop_tests.rs index 428c4f5e29..20fb823b7f 100644 --- a/crates/perry-codegen/src/stmt/class_field_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/class_field_loop_tests.rs @@ -223,6 +223,20 @@ fn assert_versioned_loop_lowered(ir: &str, what: &str) { "{what}: the versioned loop's SLOW clone is missing — a hoisted guard \ with no fallback arm is worse than no hoist at all" ); + // #7480 step 4: the clone must be ENTERED, not merely emitted. The lowering + // builds the fast clone first and proves it call-free second; on a failed + // proof it terminates the guard with an UNCONDITIONAL branch to the slow + // clone and leaves the fast blocks as unreachable code — a state in which + // every label assertion above still passes. The twin assertion on the + // element-shape clone caught exactly that: #7690's back-edge polls put a + // `js_gc_loop_safepoint()` inside the clone and silently deleted it. + assert!( + ir.contains("label %for.class_field_fast.cond") + || ir.contains("label %class_field.loop.fast.preheader"), + "{what}: the guard must branch INTO the fast clone. If it ends in an \ + unconditional branch to the slow clone, the call-free proof failed and \ + the clone is dead code that every label assertion above still accepts" + ); // The fast clone must be free of the per-access diamond it exists to // replace: no volatile gate load between the fast preheader and the store. let fast = fast_clone_slice(ir); diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index ae36c4bbad..7d0b4c7735 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -107,12 +107,20 @@ use super::loops::{ use crate::expr::{lower_expr, FnCtx}; use crate::types::{DOUBLE, I1, I32}; -/// Loop bound: a literal, or a loop-invariant local / module global that is -/// materialized to i32 once in the preheader. +/// 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 +/// `length`. #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum ElementShapeLoopBound { Constant(i64), Local(u32), + /// `j < arr.length` where `arr` is the array the body reads. The guard + /// already loads that word, so this arm costs nothing to materialize — + /// see [`crate::expr::element_shape_guard::ElementShapeLoopTripCount`]. + /// + /// Carries the receiver id because the bound is matched before the body + /// names the array; the caller cross-checks the two. + ArrayLength(u32), } #[derive(Debug)] @@ -246,11 +254,42 @@ fn element_class_name(ctx: &FnCtx<'_>, array_id: u32, counter_id: u32) -> Option index: Box::new(perry_hir::Expr::LocalGet(counter_id)), }, ) { - return Some(named); + // Only if it names a REAL class. `type Node = {v: number}` makes the + // element type `Named("Node")`, and the receiver resolver reports + // "Node" for it — a name no `ctx.classes` entry answers to, because the + // literals allocate an `__AnonShape_…`. Returning it unconditionally + // shadowed arm 2 for every alias-typed array, which is how the second + // half of `churn_read`'s miss survived #7669: the anon-shape resolver + // landed and was then never consulted for the shape it was written for. + if ctx.classes.contains_key(&named) { + return Some(named); + } } anon_shape_class_for_element_type(ctx, array_id) } +/// Follow `type A = B; type B = {…}` to the object type an alias spells. +/// +/// Bounded rather than cycle-detected: `type A = A` is not expressible in a +/// well-formed program, but codegen must not hang on a malformed one, and no +/// real alias chain is deep. Running out of budget declines the clone. +fn resolve_type_alias<'t>( + ctx: &'t FnCtx<'_>, + ty: &'t perry_hir::types::Type, +) -> &'t perry_hir::types::Type { + let mut current = ty; + for _ in 0..8 { + let perry_hir::types::Type::Named(name) = current else { + return current; + }; + let Some(next) = ctx.type_aliases.get(name) else { + return current; + }; + current = next; + } + current +} + /// Content-addressed synthetic class every closed-shape object literal lowers /// to (`perry-hir/src/lower/context.rs::mint_anon_shape_class`). const ANON_SHAPE_PREFIX: &str = "__AnonShape_"; @@ -286,7 +325,8 @@ const ANON_SHAPE_PREFIX: &str = "__AnonShape_"; fn anon_shape_class_for_element_type(ctx: &FnCtx<'_>, array_id: u32) -> Option { use perry_hir::types::Type as HirType; - let elem = match ctx.local_types.get(&array_id)? { + let array_ty = resolve_type_alias(ctx, ctx.local_types.get(&array_id)?); + let elem = match array_ty { HirType::Array(elem) => elem.as_ref(), // `new Array<{v: number}>(n)` locals carry the generic spelling. HirType::Generic { base, type_args } if base == "Array" && type_args.len() == 1 => { @@ -294,7 +334,9 @@ fn anon_shape_class_for_element_type(ctx: &FnCtx<'_>, array_id: u32) -> Option return None, }; - let HirType::Object(obj) = elem else { + // `type Node = {v: number; w: number}` — the annotation names the shape one + // indirection away. Both levels are resolved (`type Row = Node[]` too). + let HirType::Object(obj) = resolve_type_alias(ctx, elem) else { return None; }; // Only a CLOSED shape names a layout: an index signature, a method @@ -429,6 +471,20 @@ fn match_element_shape_versioned_loop( Expr::Integer(k) if (0..=i64::from(i32::MAX)).contains(k) => { ElementShapeLoopBound::Constant(*k) } + // `j < arr.length` — the idiom every one of #7480's own kernels is + // written in, and the reason the clone had not moved `churn_read` at + // all: `keep.length` is a `PropertyGet`, so the bound match declined + // before the class resolver was ever reached. The receiver is checked + // against the body's array below; an unrelated array's length would + // need its own invariance argument and is not admitted. + Expr::PropertyGet { + object, property, .. + } if property == "length" => match object.as_ref() { + Expr::LocalGet(recv_id) if *recv_id != counter_id => { + ElementShapeLoopBound::ArrayLength(*recv_id) + } + _ => return None, + }, Expr::LocalGet(bound_id) if *bound_id != counter_id => { if ctx.boxed_vars.contains(bound_id) { return None; @@ -488,10 +544,22 @@ fn match_element_shape_versioned_loop( if props.is_empty() || array_id == *acc_id || array_id == counter_id { return None; } - if let ElementShapeLoopBound::Local(bound_id) = bound { - if bound_id == array_id || bound_id == *acc_id { - return None; + match bound { + ElementShapeLoopBound::Local(bound_id) => { + if bound_id == array_id || bound_id == *acc_id { + return None; + } + } + // The guard's `length` load answers for the array the guard branded. + // `for (j = 0; j < other.length; j++) acc += keep[j].v` reads a + // DIFFERENT array's length, and the preheader proves nothing about how + // the two relate — that is an out-of-range read, not a slow clone. + ElementShapeLoopBound::ArrayLength(recv_id) => { + if recv_id != array_id { + return None; + } } + ElementShapeLoopBound::Constant(_) => {} } // The array must be loop-invariant and directly addressable — no boxing, @@ -608,10 +676,12 @@ pub(super) fn lower_element_shape_versioned_for( // One-time i32 materialization of the bound. A non-number / NaN / // fractional / out-of-range bound keeps full JS trip-count semantics in - // the slow clone. - let bound_i32: String = match matched.bound { - ElementShapeLoopBound::Constant(k) => k.to_string(), - ElementShapeLoopBound::Local(bound_id) => { + // the slow clone. `arr.length` materializes inside the guard instead — it + // is the word the guard already loads — so it contributes nothing here. + let materialized_bound: Option = match matched.bound { + ElementShapeLoopBound::ArrayLength(_) => None, + ElementShapeLoopBound::Constant(k) => Some(k.to_string()), + ElementShapeLoopBound::Local(bound_id) => Some({ let bound_d = lower_expr(ctx, &perry_hir::Expr::LocalGet(bound_id))?; let is_number = emit_js_value_is_number(ctx, &bound_d); let range_idx = ctx.new_block("element_shape.loop.bound.range"); @@ -642,17 +712,23 @@ pub(super) fn lower_element_shape_versioned_for( ctx.current_block = check_idx; bound_i32 - } + }), }; + let trip_count = match &materialized_bound { + Some(bound) => { + crate::expr::element_shape_guard::ElementShapeLoopTripCount::Bound(bound.as_str()) + } + None => crate::expr::element_shape_guard::ElementShapeLoopTripCount::ArrayLength, + }; let expected_class_id_str = matched.expected_class_id.to_string(); - let (elements_base, expected_keys, shape_ok) = + let (elements_base, expected_keys, shape_ok, bound_i32) = crate::expr::element_shape_guard::emit_element_shape_loop_preheader_check( ctx, matched.array_id, &expected_class_id_str, &matched.keys_global_name, - &bound_i32, + trip_count, &slow_pre_label, )?; // Deliberately unterminated: it branches into the fast clone only after 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 313e3d088b..9c65b8073c 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -266,6 +266,43 @@ fn emit(m: &Module) -> String { String::from_utf8(compile_module(m, ir_opts()).unwrap()).expect("LLVM IR should be UTF-8") } +/// [`emit`] with module-level `type X = …` aliases in scope. +fn emit_with_aliases(m: &Module, aliases: &[(&str, Type)]) -> String { + let mut opts = ir_opts(); + opts.type_aliases = aliases + .iter() + .map(|(name, ty)| ((*name).to_string(), ty.clone())) + .collect(); + String::from_utf8(compile_module(m, opts).unwrap()).expect("LLVM IR should be UTF-8") +} + +/// `.length` +fn length_of(receiver_id: u32) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::LocalGet(receiver_id)), + property: "length".to_string(), + byte_offset: 0, + } +} + +/// Replace the module's loop condition with `j < `. +/// +/// Mutates in place and panics if the statement is not where +/// [`element_shape_module`] puts it, for the same reason +/// [`object_element_module`] asserts its own shape: a silent skip would leave +/// the constant bound in place and every `.length` assertion below would be +/// re-testing the constant-bound path. +fn with_bound(m: &mut Module, bound: Expr) { + let Some(Stmt::For { condition, .. }) = m.init.get_mut(2) else { + panic!("element_shape_module's third init statement should be the `For`"); + }; + *condition = Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(COUNTER_ID)), + right: Box::new(bound), + }); +} + /// The blocks that exist only when the clone was really built AND entered. const CLONE_LABELS: [&str; 6] = [ "element_shape.loop.preheader.brand", @@ -302,9 +339,35 @@ fn block_def_offset(ir: &str, label: &str) -> Option { None } -/// The emitted text the fast clone owns: from the DEFINITION of its cond block -/// to the definition of the slow clone's. Covers `for.element_shape_fast.*` -/// and the `element_shape.load` block the field read branches into. +/// The clone must be **entered**, not merely emitted. +/// +/// `lower_element_shape_versioned_for` builds the fast clone first and proves +/// it call-free second. When the proof fails it terminates the deref block with +/// an *unconditional* branch to the slow clone and leaves the fast blocks as +/// unreachable code. Every [`CLONE_LABELS`] assertion still passes in that +/// state — all the labels are present — so the census alone cannot tell +/// "optimized" from "optimization silently deleted". +/// +/// That gap is not hypothetical. #7690 restored moving-loop back-edge polls to +/// ON by default, which put a `js_gc_loop_safepoint()` call in the clone's +/// element-load block; the whole clone became dead code, the benchmark it was +/// written for did not move, and not one test failed. The module docs of +/// `stmt/element_shape_loop.rs` named this failure mode in advance ("a silent +/// loss of the optimization … with no test failing"). This is the assertion +/// that makes it loud. +fn assert_fast_clone_is_entered(ir: &str) { + let deref = block_slice(ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("label %element_shape.loop.fast.preheader"), + "the guard must branch INTO the fast clone. The deref block ends in an \ + unconditional branch to the slow clone, which means the call-free \ + proof failed and the clone is dead code:\n{deref}" + ); +} + +/// The emitted text the fast clone owns: exactly the blocks named +/// `for.element_shape_fast.*` and the `element_shape.load` blocks its field +/// reads branch into. /// /// #7480 step 3 — ANTI-VACUITY. This used to slice from the first *substring* /// occurrence of `for.element_shape_fast.cond`, which is the @@ -316,19 +379,38 @@ fn block_def_offset(ir: &str, label: &str) -> Option { /// to prove the clone is really call-free had never been able to fail. The /// liveness assertion below is what makes it a gate — CLAUDE.md's "a gate must /// assert its subject was live". -fn fast_clone_slice(ir: &str) -> &str { - let start = block_def_offset(ir, "for.element_shape_fast.cond") - .expect("the fast clone's cond block should be DEFINED in the emitted IR"); - let end = block_def_offset(ir, "for.element_shape_slow.cond").unwrap_or(ir.len()); - assert!(end > start, "the fast clone must precede the slow clone"); - let slice = &ir[start..end]; +/// +/// #7480 step 4 — the OPPOSITE error, and the reason this now selects blocks by +/// name instead of slicing a span. "Everything between the fast cond and the +/// slow cond" is only the fast clone while nothing else is emitted in between. +/// An `arr.length` bound makes the slow clone hoist its own length read, and +/// those `plen.*` blocks land in the gap — so a call belonging to the SLOW +/// clone was attributed to the fast one and the census failed on a fast clone +/// that was, and still is, bare. A span that depends on what a neighbour emits +/// can report either way; ownership is a property of the block, so ask the +/// block. +fn fast_clone_slice(ir: &str) -> String { + let mut owned = String::new(); + let mut in_fast_block = false; + 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(':') { + in_fast_block = trimmed.starts_with("for.element_shape_fast.") + || trimmed.starts_with("element_shape.load"); + } + if in_fast_block { + owned.push_str(line); + } + } assert!( - slice.contains("for.element_shape_fast.body") && slice.contains("element_shape.load"), + owned.contains("for.element_shape_fast.body") && owned.contains("element_shape.load"), "the fast-clone slice must contain the cloned BODY and its element \ load, otherwise every negative assertion against it is vacuous; \ - sliced:\n{slice}" + sliced:\n{owned}" ); - slice + owned } #[test] @@ -359,6 +441,7 @@ fn element_shape_versioned_loop_fires_for_the_7480_access_shape() { // fallback is worse than no hoist. assert!(ir.contains("for.element_shape_slow.cond")); + assert_fast_clone_is_entered(&ir); let fast = fast_clone_slice(&ir); // The whole point: the fast clone contains NO call at all. That is the // revocation argument (call-free ⇒ no funnel can revoke the invariant and @@ -648,6 +731,7 @@ fn element_shape_versioned_loop_resolves_an_object_literal_element_type() { "the preheader must load the anon shape's keys global; emitted:\n{deref}" ); + assert_fast_clone_is_entered(&ir); let fast = fast_clone_slice(&ir); assert!( !fast.contains(" call "), @@ -836,6 +920,231 @@ fn object_literal_element_resolution_does_not_escape_the_clone() { ); } +// --------------------------------------------------------------------------- +// #7480 step 4: the two reasons `churn_read.ts` never reached the clone. +// +// #7612 landed the clone and #7669 taught it object-literal element types, and +// the benchmark the whole route was chosen for did not move by one millisecond +// across both. Not because the lowering was wrong — because the MATCHER never +// got to it. `for (let j = 0; j < keep.length; j++)` fails the bound match +// (`keep.length` is a `PropertyGet`), and `type Node = {v: number}` makes +// `element_class_name` answer "Node", a name no class owns, which returned +// early and skipped the anon-shape resolver #7669 had just added. +// +// Each is independently fatal, which is why they are tested independently and +// then together in the shape the benchmark actually has. +// --------------------------------------------------------------------------- + +const ANON_SHAPE_ALIASED: &str = "__AnonShape_0000000000000def"; + +#[test] +fn element_shape_versioned_loop_admits_an_array_length_bound() { + let mut m = object_element_module( + object_element_type(&[("v", Type::Number), ("w", Type::Number)], false), + vec![anon_shape_class( + 311, + ANON_SHAPE_VW, + &[("v", Type::Number), ("w", Type::Number)], + )], + ); + with_bound(&mut m, length_of(ARRAY_ID)); + let ir = emit(&m); + for label in CLONE_LABELS { + assert!( + ir.contains(label), + "`for (j = 0; j < arr.length; j++)` is the idiom every #7480 kernel \ + is written in and must reach the clone, but `{label}` is absent" + ); + } + + assert_fast_clone_is_entered(&ir); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains(" call "), + "the fast clone must stay call-free with a `.length` bound; found a \ + call in:\n{fast}" + ); + // The trip count is the word the guard already loaded. If the bound were + // re-derived per iteration the clone would still be correct and would win + // far less, and nothing else here would notice. + assert!( + !fast.contains("js_array_length"), + "the `.length` bound must be materialized once in the preheader, not \ + re-read inside the clone; emitted:\n{fast}" + ); + assert!( + fast.contains("fadd"), + "the accumulate must lower to an fadd inside the clone; emitted:\n{fast}" + ); +} + +/// SABOTAGE (bound provenance): the guard's `length` load answers for the +/// array the guard branded. Reading `keep[j]` while trip-counting on a +/// DIFFERENT array's length is an out-of-range read, not a slow clone. +#[test] +fn element_shape_versioned_loop_declines_a_foreign_arrays_length_bound() { + const OTHER_ARRAY_ID: u32 = 41; + let mut m = object_element_module( + object_element_type(&[("v", Type::Number), ("w", Type::Number)], false), + vec![anon_shape_class( + 311, + ANON_SHAPE_VW, + &[("v", Type::Number), ("w", Type::Number)], + )], + ); + m.init.insert( + 0, + Stmt::Let { + id: OTHER_ARRAY_ID, + name: "other".to_string(), + ty: Type::Array(Box::new(Type::Number)), + mutable: false, + init: Some(Expr::Array(Vec::new())), + }, + ); + // `element_shape_module` puts the `For` at index 2; the insert above + // shifted it to 3, so rebuild the condition by hand rather than through + // `with_bound`'s fixed index. + let Some(Stmt::For { condition, .. }) = m.init.get_mut(3) else { + panic!("the `For` should have shifted to index 3"); + }; + *condition = Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(COUNTER_ID)), + right: Box::new(length_of(OTHER_ARRAY_ID)), + }); + + let ir = emit(&m); + for label in CLONE_LABELS { + assert!( + !ir.contains(label), + "trip-counting on `other.length` while reading `keep[j]` must not \ + be cloned, but `{label}` was emitted — the preheader proves \ + nothing about how the two lengths relate" + ); + } +} + +#[test] +fn element_shape_versioned_loop_resolves_an_aliased_object_element_type() { + let mut m = object_element_module( + Type::Named("Node".to_string()), + vec![anon_shape_class( + 312, + ANON_SHAPE_ALIASED, + &[("v", Type::Number), ("w", Type::Number)], + )], + ); + // `type Node = { v: number; w: number }` — the alias is the ONLY thing + // standing between this module and the previous test's. + with_bound(&mut m, Expr::Integer(1_000_000)); + let ir = emit_with_aliases( + &m, + &[( + "Node", + object_element_type(&[("v", Type::Number), ("w", Type::Number)], false), + )], + ); + for label in CLONE_LABELS { + assert!( + ir.contains(label), + "an alias-typed element (`type Node = {{…}}`) must resolve to the \ + anon shape its literals allocate, but `{label}` is absent" + ); + } + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("AnonShape"), + "the guard must pin the anon shape the alias expands to; emitted:\n{deref}" + ); + // ANTI-VACUITY. `CLONE_LABELS` above is satisfied by a preheader that ends + // in an unconditional branch to the SLOW clone -- which is exactly what a + // failed call-free proof emits, and exactly the silent-deletion shape the + // rest of this file exists to catch. Asserting the labels alone would let + // alias resolution "work" while the clone it resolved was dead code. + // + // The sibling `.length` test carries these two assertions; this one, the + // only test covering alias resolution IN ISOLATION, did not. So a + // regression confined to the alias-without-a-`.length`-bound path -- the + // one shape no other test emits -- would have gone unseen. + assert_fast_clone_is_entered(&ir); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains(" call "), + "an alias-resolved fast clone must stay call-free -- a call is a GC \ + safepoint, and the element-shape guard is established BEFORE it; \ + emitted:\n{fast}" + ); +} + +/// SABOTAGE (alias resolution): an alias is not a licence to guess. With no +/// `type Node = …` in scope, `Node[]` names a class that does not exist and +/// the matcher must decline rather than fall through to whatever single anon +/// shape happens to be in the module. +#[test] +fn element_shape_versioned_loop_declines_an_unresolvable_named_element_type() { + let ir = emit(&object_element_module( + Type::Named("Node".to_string()), + vec![anon_shape_class( + 312, + ANON_SHAPE_ALIASED, + &[("v", Type::Number), ("w", Type::Number)], + )], + )); + for label in CLONE_LABELS { + assert!( + !ir.contains(label), + "`Node[]` with no class and no alias names no layout, but \ + `{label}` was emitted — the resolver guessed" + ); + } +} + +/// `churn_read.ts` itself: an aliased object element type AND an `arr.length` +/// bound, the combination the benchmark has had since it was written. Both +/// halves above pass individually; this is the one that was red for two +/// shipped PRs. +#[test] +fn element_shape_versioned_loop_fires_for_the_churn_read_shape() { + let mut m = object_element_module( + Type::Named("Node".to_string()), + vec![anon_shape_class( + 312, + ANON_SHAPE_ALIASED, + &[("v", Type::Number), ("w", Type::Number)], + )], + ); + with_bound(&mut m, length_of(ARRAY_ID)); + let ir = emit_with_aliases( + &m, + &[( + "Node", + object_element_type(&[("v", Type::Number), ("w", Type::Number)], false), + )], + ); + for label in CLONE_LABELS { + assert!( + ir.contains(label), + "`churn_read.ts`'s exact shape must reach the clone, but `{label}` \ + is absent" + ); + } + assert_fast_clone_is_entered(&ir); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains(" call "), + "the fast clone must be call-free; found a call in:\n{fast}" + ); + assert!( + !fast.contains("js_dynamic_string_or_number_add"), + "the accumulate must regain its numeric proof; emitted:\n{fast}" + ); + assert!( + fast.contains("fadd"), + "the accumulate must lower to an fadd; emitted:\n{fast}" + ); +} + #[test] fn the_repair_does_not_put_a_call_inside_the_fast_clone() { let ir = emit(&element_shape_module( @@ -850,6 +1159,7 @@ fn the_repair_does_not_put_a_call_inside_the_fast_clone() { // clone would void the revocation argument — and, because the lowering // then branches unconditionally to the slow clone, would silently delete // the optimization instead of failing. + assert_fast_clone_is_entered(&ir); let fast = fast_clone_slice(&ir); assert!( !fast.contains("call "), diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 551e47c6e0..22ee1a9624 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -4765,18 +4765,41 @@ pub(super) fn lower_for_after_init_with_i32_bound( // Whether THIS site allocated the counter's i32 slot (vs. the Let site or // repsel Phase 1 having done so). Only the inserter removes at loop exit. let mut hoist_counter_i32_was_fresh = false; + // #7480 step 4: inside a call-free-by-construction fast clone + // (`lower_element_shape_versioned_for`, `lower_class_field_versioned_for`) + // the caller has ALREADY materialized the trip count and passed it in + // `precomputed_i32_bound`, so the cond block never reads this slot. The + // hoist would emit a `js_value_length_f64` call whose result nothing + // consumes — and a call inside one of those clones does not make it slower, + // it DELETES it: the clone's own call-free scan fails and the guard branches + // unconditionally to the slow clone, leaving the fast blocks as unreachable + // code with every IR-census assertion still passing. That is precisely how + // `for (let j = 0; j < keep.length; j++) acc += keep[j].v` — #7480's own + // kernel, and the reason the clone exists — got a clone it never entered. + // + // Only the LOAD is skipped. The bounded-index / buffer-width facts and the + // i32 counter slot below are proofs and storage, not emitted work, and the + // clone's other lowering may depend on them; suppressing those too would + // trade one silent loss for another. + let in_call_free_clone = + !ctx.element_shape_loop_facts.is_empty() || !ctx.class_field_loop_facts.is_empty(); let hoisted_length_slot: Option = if let Some(hoist) = hoist_classification { - let arr_box_loaded = lower_expr( - ctx, - &perry_hir::Expr::PropertyGet { - byte_offset: 0, - object: Box::new(perry_hir::Expr::LocalGet(hoist.arr_id)), - property: "length".to_string(), - }, - )?; - let slot = ctx.func.alloca_entry(DOUBLE); - ctx.block().store(DOUBLE, &arr_box_loaded, &slot); - ctx.cached_lengths.insert(hoist.arr_id, slot.clone()); + let hoisted_slot = if in_call_free_clone { + None + } else { + let arr_box_loaded = lower_expr( + ctx, + &perry_hir::Expr::PropertyGet { + byte_offset: 0, + object: Box::new(perry_hir::Expr::LocalGet(hoist.arr_id)), + property: "length".to_string(), + }, + )?; + let slot = ctx.func.alloca_entry(DOUBLE); + ctx.block().store(DOUBLE, &arr_box_loaded, &slot); + ctx.cached_lengths.insert(hoist.arr_id, slot.clone()); + Some(slot) + }; // Also tell `lower_index_set_fast` (and similar sites) that // `arr[counter_id]` is statically inbounds for this body, so // it can skip the runtime length-load + bound check. @@ -4826,7 +4849,7 @@ pub(super) fn lower_for_after_init_with_i32_bound( } } - Some(slot) + hoisted_slot } else { None }; @@ -5283,6 +5306,47 @@ pub(crate) fn emit_gc_loop_safepoint( if !moving_safepoint_polls_enabled() || ctx.block().is_terminated() { return; } + // #7480 step 4: never inside a call-free-by-construction fast clone. + // + // `lower_class_field_versioned_for` and `lower_element_shape_versioned_for` + // hoist a guard into a preheader and clone the body against it, and both + // rest on the SAME safety argument: the clone makes no call, therefore + // allocates nothing, therefore cannot collect, therefore the pointer the + // preheader cached cannot move. Each verifies that by scanning its own + // emitted blocks afterwards, and a clone whose call-freeness is unproven is + // never entered — the deref block branches unconditionally to the slow + // clone and the fast blocks are left as unreachable code. + // + // A back-edge poll is a call. Emitting one into the clone therefore does + // not make the clone slower; it deletes the clone, silently, with the fast + // blocks still present in the IR for any census to find. #7690 turning + // these polls back on by default did exactly that to the ELEMENT-SHAPE + // clone: `churn_read.ts` went 0.03 s -> 0.54 s on the same compiler, every + // `perry-codegen` test stayed green, and the only symptom was a benchmark + // that did not move. That is the failure mode + // `stmt/element_shape_loop.rs`'s module docs predicted in as many words, + // and `assert_fast_clone_is_entered` is the assertion that now catches it. + // + // The class-field clone is NOT affected today, and that was checked rather + // than assumed: removing this suppression leaves its three IR tests green, + // because `loop_may_allocate` already proves an `obj.field`-only body inert + // and emits no poll for it. It is covered here anyway — the two clones rest + // on the identical argument, and the next body shape admitted to the + // class-field matcher that is not provably inert would delete that clone + // the same way. Its tests gained the same liveness assertion. + // + // Skipping the poll here is not a new licence — it is the rule the line + // below already applies. A poll exists so that an ALLOCATING loop can defer + // a collection to a safe point; a body that cannot allocate does not need + // one, which is precisely why `loop_may_allocate` gates the poll at all. + // `loop_may_allocate` answers from the HIR body, before specialization, so + // it cannot see that the clone's `arr[j].f` lowers to a bare load rather + // than to the generic diamond. Inside a fact scope, it can: the clone is + // call-free or it is not entered, and the slow clone — lowered after the + // scope is popped — keeps its poll either way. + if !ctx.element_shape_loop_facts.is_empty() || !ctx.class_field_loop_facts.is_empty() { + return; + } // Only an ALLOCATING loop body can defer a collection to this poll; skip the // poll for pure (non-allocating) bodies so numeric/vectorizable loops stay // call-free (a poll defeats LLVM auto-vectorization — measured ~2x on a tight