diff --git a/.github/workflows/gc-root-dominance.yml b/.github/workflows/gc-root-dominance.yml index 6b8f51c4cb..ebf89aef1d 100644 --- a/.github/workflows/gc-root-dominance.yml +++ b/.github/workflows/gc-root-dominance.yml @@ -157,6 +157,19 @@ jobs: - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable + # ★ The dependency-scale corpus needs the dependency. + # + # `zod` is this repo's own package.json devDependency, pinned by + # package-lock.json and governed by the same soak window as everything + # else in that file -- not a fixture invented for this job. + # `--ignore-scripts` because nothing here needs a lifecycle script to + # run, and a corpus generator is a bad place to execute one. + - uses: actions/setup-node@v6 + with: + node-version-file: .node-version + - name: Install the npm dependencies the dep corpus compiles + run: npm ci --ignore-scripts --no-audit --no-fund + - name: Cache cargo uses: actions/cache@v6 with: @@ -242,10 +255,95 @@ jobs: --allowlist scripts/gc_root_dominance_allowlist.json \ -v + # ★★ The DEPENDENCY-SCALE corpus (#7280). + # + # Everything above this line runs over ~124 hand-written `test-files/` + # sources. That corpus read ZERO in both gated modes while twenty lines of + # stock `zod` faulted deterministically under the from-space protector, + # and #7280 records the gap in one sentence: 25 curated files pass while + # 20 lines of stock zod fail. + # + # It is not a size problem, it is a distribution problem. Measured with + # `--stale-registers --moving-only` on the same compiler: + # + # curated (124 sources, 144 modules): 116 stale uses, and what + # dominates is property-GET helper windows and js_number_coerce + # dependency-scale (81 modules, 62 MB): 370 stale uses, and what + # dominates is js_object_assign_one (object spread, 137) and + # js_new_function_construct (102) -- populations the curated corpus + # produces 12 and 1 of + # + # Nothing is sampled away: all 81 modules and all 62 MB are checked. The + # cost is ~8s to emit and ~4s for the two gated arms below, because those + # arms are linear in instruction count. The `--stale-registers` ratchet is + # the expensive one (~5 min) and says so where it runs. + - name: Emit the dependency-scale IR corpus + run: ./scripts/gc_root_dominance_dep_corpus.sh ir-corpus-dep + + - name: Check root-store dominance (dependency-scale) + run: | + set -euo pipefail + # Floors from the corpus as of this commit (81 modules, ~12900 + # functions, ~7700 root stores), set below that with room for the + # dependency's own churn. `zod` growing is fine; `zod` no longer + # compiling natively is the finding, and these are what make it one. + python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ + --moving-only \ + --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --allowlist scripts/gc_root_dominance_allowlist.json \ + --seeded-violations 40 \ + -v + + - name: Check that every GC value in an alloca has a root store (dependency-scale) + run: | + set -euo pipefail + python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ + --unrooted-allocas \ + --moving-only \ + --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --allowlist scripts/gc_root_dominance_allowlist.json \ + -v + + # ★ The stale-register RATCHET, on both corpora. + # + # `--stale-registers` asks the third question: not "is the root store + # late" and not "is there a root store at all", but "is a register holding + # a rooted value used below a collection point". It is the mode that found + # #7206's two bugs and the mode #7280's zod fault lives in, and until now + # it ran only by hand -- so its number could move in either direction + # between one investigation and the next with nothing to say so. + # + # It is a BUDGET rather than an allowlist because the residual is a + # population, not a list of triaged sites: the remaining uses are the ones + # whose slot the program itself reassigns inside the window, which need a + # temp root rather than a re-read (see crate::root_reload). A budget can + # only be lowered, and lowering it is the ratchet. + # + # This step is minutes, not seconds -- the scan is superlinear in + # instruction count and the dependency corpus is 62 MB. That is the price + # of checking the population that actually breaks. + - name: Stale-register budget (curated) + run: | + set -euo pipefail + python3 scripts/gc_root_dominance_check.py ir-corpus \ + --stale-registers --moving-only \ + --min-files 90 --min-binds 1500 --min-funcs 1200 \ + --max-stale 39 + + - name: Stale-register budget (dependency-scale) + run: | + set -euo pipefail + python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ + --stale-registers --moving-only \ + --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --max-stale 118 + - name: Upload the IR corpus on failure if: failure() uses: actions/upload-artifact@v7 with: name: gc-root-dominance-ir - path: ir-corpus + path: | + ir-corpus + ir-corpus-dep retention-days: 7 diff --git a/changelog.d/7311-dep-scale-corpus-and-root-reload.md b/changelog.d/7311-dep-scale-corpus-and-root-reload.md new file mode 100644 index 0000000000..096345f76e --- /dev/null +++ b/changelog.d/7311-dep-scale-corpus-and-root-reload.md @@ -0,0 +1,166 @@ +### GC rooting: measure the right corpus, then fix the one rule that dominates it + +Two linked changes: the gate's corpus was measuring the wrong population, and +once it measured the right one, 73% of what it reported was a single rule. + +#### The corpus (#7280) + +`scripts/gc_root_dominance_corpus.sh` compiles ~124 hand-written `test-files/` +sources. It reads **zero** in both modes the CI gate runs, and it read zero +while twenty lines of stock `zod` faulted deterministically under the from-space +protector. #7280 puts the gap in one sentence: *25 curated files pass while 20 +lines of stock zod fail.* + +That is a distribution problem, not a size problem. Both corpora, same compiler, +`--stale-registers --moving-only`: + +| corpus | stale uses | dominant population | +|---|---|---| +| curated — 124 sources, 144 modules, 2378 functions | 116 | property-GET helper windows, `js_number_coerce`, `js_closure_callN` | +| dependency-scale — 81 modules, 62 MB, 12899 functions | 371 | `js_object_assign_one` 137, `js_new_function_construct` 102, `js_closure_call*` 30 | + +The curated corpus produces 12 of the first population and 1 of the second. A +hand-written test allocates a couple of objects and calls a couple of helpers; a +library spreads objects into objects, boxes every mutable capture because its +closures outlive their frames, and builds values field by field out of data. +The rooting hazards live in the *shapes*. + +So `scripts/gc_root_dominance_dep_corpus.sh` generates the second corpus from a +real npm dependency — `zod`, this repo's own `package.json` devDependency, +pinned by `package-lock.json` and governed by the same soak window as everything +else in that file — imported **by source path** so its modules compile natively +rather than falling back to V8 and emitting no IR to check. + +**Nothing is sampled away.** All 81 modules and all 62 MB are checked. Emitting +costs ~8s; the two gated arms cost ~3s each, because they are linear in +instruction count. The `--stale-registers` budget is the expensive one (~5 min) +and the workflow says so where it runs. + +`test-files/gc-dep-corpus/main.ts` is the only entry point and the rest of that +directory reaches the compiler by being imported from it, so the generator +**asserts that every `.ts` in the directory produced a module** — the check a +size floor could not be (#7278), since ~90 modules of `zod` swamp any count a +missing 40-line source would cross. + +#### The rule (`crate::root_reload`) + +> A load out of a shadow slot is a **copy** of a root. An evacuating minor +> rewrites the slot; it cannot rewrite the register. So every use a collection +> point can reach must re-read the slot — unless a store to that slot can also +> run on the way, in which case re-reading would observe an assignment the +> program made and the register is left alone. + +Verbatim from `zod`'s object-spread lowering, before: + +```llvm + call void @js_shadow_slot_bind(i32 0, ptr %r7) ; %r7 IS a root + %r8 = load double, ptr %r7 ; a COPY of the root + %r9 = call double @baseFields() ; evacuates; rewrites %r7 + %r10 = call double @js_object_assign_one(double %r8, double %r9) ; from-space + %r11 = load double, ptr %r7 ; the NEXT statement re-reads +``` + +Codegen was never unable to re-read the slot — the following statement does it, +because a fresh lowering emits a fresh load. The bug is that *within* one +lowering the load happens once, at the top, and the register is carried across +everything after it. That is why this is a pass and not another point fix: +`index_set.rs` alone lowers `object` before `value` at fifteen separate arms, +and the shape also appears in the object-literal spread, the inline class-field +store, `new`, property define, `instanceof` and the closure-call family. + +The soundness half is the half `expr/temp_root.rs` already documents: re-reading +a *local* is not unconditionally safe, because `new C(g, bump())` where `bump()` +assigns `g` must pass the pre-`bump()` value. At IR level that objection is +decidable — an assignment to a shadow-slotted local is a `store` to its alloca +in the same function, and a local captured *and* mutated by a closure is boxed +instead, so it has no plain shadow slot to reload. Both halves are path +questions over the real CFG, answered the way the checker answers them: a +back-edge round trip is not an intra-iteration path, because re-entering the +load's block re-executes the load. + +Where the reload was not needed — no call between the two points — LLVM's +EarlyCSE/GVN forwards it away, since nothing can clobber the alloca. Where it +was needed the intervening call is opaque and the load stays. The pass is close +to free exactly where it is redundant. + +Recording is at the choke point, not at the thirteen `js_shadow_slot_bind` emit +sites: `LlBlock::call_void` and `LlFunction::entry_setup_call_void` see every +bind form, including the slow arm of #7088's inline diamond (which emits the +same call), so a fourteenth site cannot be added without the set noticing. And +the pass runs in `compile_module` before **any** rendering path, so the text +renderer and the in-process constructor (#7301) see the same IR — a pass living +inside `to_ir` would silently not apply to the other. + +#### What it closes, on both corpora + +| arm | curated | dependency-scale | +|---|---|---| +| `--stale-registers --moving-only`, total | 116 → **39** | 371 → **118** | +| …of which `source=slotload` | 102 → **25** | 272 → **19** | +| bind-anchored `--moving-only` | 0 → 0 | 0 → 0 | +| `--unrooted-allocas --moving-only` | 0 → 0 | 0 → 0 | + +`js_object_assign_one` disappears from the dependency-scale report entirely +(137 → 0); `js_new_function_construct` goes 102 → 29. + +Both budgets are now ratchets in `gc-root-dominance.yml` (`--max-stale 39` and +`--max-stale 118`), because until now this mode ran only by hand — its number +could move in either direction between one investigation and the next with +nothing to say so. It is a budget rather than an allowlist because the residual +is a *population*, not a list of triaged sites: what is left is the uses whose +slot the program itself reassigns inside the window, which need a temp root +rather than a re-read. + +#### And one runtime-Rust rooting bug the corpus led to + +`js_regexp_new` takes `pattern` as a raw `StringHeader*` and then allocates +twice — `js_string_from_str` for the canonical flags, `gc_malloc` for the header +— before storing that pointer into `RegExpHeader::pattern_ptr`. Either +allocation can run an evacuating minor, after which the argument names retired +from-space and the header keeps a **permanently** dangling `pattern_ptr`; the +borrowed `pattern_str` had the same exposure and fed the owned `.source` copy. + +That is the runtime-Rust half of the invariant (#7249), which +`gc_root_dominance_check.py` is structurally blind to — it reads emitted IR and +cannot see a Rust local. Reproduction: + +``` +PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \ +PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 \ + ./ +``` + +Before: faults deterministically, `RETIRED FROM-SPACE`, `js_regexp_new + 3920` +← `js_regexp_construct` ← `perry_closure_…zod…regexes_ts__9`. The same fault +appears with the codegen pass alone, so it is not caused by it. After: clean. + +No unit-test witness ships with it, and that is a deliberate statement rather +than an omission: forcing a *copying* minor inside a runtime function needs a +collection at an allocation point, and `gc/zeal.rs` documents why that level +does not exist (an allocation-point collection takes `force_full_scan`, which +makes the copying minor ineligible, so it would move nothing). A test was +written, its liveness assert refused to pass, and it was deleted rather than +shipped as a test that cannot fail for the right reason. + +#### Measured after #7301 and #7305, not before + +The backend rewrite (typed `LlInst`, two consumers of one finalized-item +visitor) and the move from setjmp/longjmp to `invoke`/`landingpad` both landed +while this was in flight, so every number above is re-measured on `b50e857c2`, +both sides, over identical paths. The dependency-scale parent count moved by +exactly one (370 → 371); nothing else changed. + +Two consequences for the pass, both load-bearing: + +* It inserts a **typed** `LlInst::Load` and rewrites operands on typed variants + in place, never text, so `native_emit`'s `(typed, raw)` migration ratchet moves + in the intended direction. And it runs in `compile_module` before *any* + rendering path, so `to_ir` and the native C-API builder consume the same + stream — a pass inside `to_ir` would have applied to one consumer only. +* An `invoke` is modelled as **both** a call and a two-successor terminator. + Missing the call half would classify a throwing helper's window as + non-collecting and drop every reload inside a `try`; missing the successor half + would hide the unwind edge. The unwind edge is also why the rule is "reload at + the USE" and not "reload after the call": a load from the slot is valid + wherever it sits and reads whatever the collector last wrote, so it is correct + on both edges without a placement decision. diff --git a/crates/perry-codegen/src/block.rs b/crates/perry-codegen/src/block.rs index 90567b7df9..0d2dc10785 100644 --- a/crates/perry-codegen/src/block.rs +++ b/crates/perry-codegen/src/block.rs @@ -9,8 +9,8 @@ //! sorts out the registers. Explicit `phi` nodes are still emitted for //! control-flow merges (if/else value context, short-circuit logical ops). -use std::cell::{Cell, RefCell}; -use std::collections::HashMap; +use std::cell::{Cell, Ref, RefCell}; +use std::collections::{HashMap, HashSet}; use std::rc::Rc; use crate::codegen::FpContractMode; @@ -71,6 +71,19 @@ pub struct RegCounter { /// catch/finally bodies see the *enclosing* scope, which is exactly /// where a throw escaping them lands at runtime. eh_unwind_labels: RefCell>, + /// Every alloca this function has published to the precise-root collector + /// with `js_shadow_slot_bind(idx, ptr)`. + /// + /// Recorded at the choke points rather than at the thirteen emit sites, + /// because a fourteenth site is exactly the thing that gets added without + /// anyone remembering a register list somewhere else. + /// + /// A shadow slot is the one class of alloca whose contents the collector + /// writes behind generated code's back: an evacuating minor rewrites the + /// slot to the object's new address. [`crate::root_reload`] reads this set + /// to find the loads that rewrite makes stale. See + /// `docs/src/internals/gc-rooting-invariant.md`. + shadow_slot_allocas: RefCell>, } impl RegCounter { @@ -78,9 +91,39 @@ impl RegCounter { Self { value: Cell::new(0), eh_unwind_labels: RefCell::new(Vec::new()), + shadow_slot_allocas: RefCell::new(HashSet::new()), } } + /// Record `ptr` when `callee` is the precise-root bind. + /// + /// Called from the two choke points every bind form passes through: + /// [`LlBlock::call_void`] — which carries the direct call AND the slow arm + /// of #7088's inline diamond, since that arm emits the same call — and + /// `LlFunction::entry_setup_call_void`, which carries the persistent-slot + /// bind hoisted into the entry prelude. + pub(crate) fn note_shadow_slot_bind(&self, callee: &str, second_arg: Option<&str>) { + if callee != "js_shadow_slot_bind" { + return; + } + // `js_shadow_slot_bind(i32 idx, ptr %slot)`: the slot is argument two. + // A non-register operand means the bind was built some other way, and + // the conservative answer is to record nothing — a slot this set does + // not name is simply never reloaded. + if let Some(ptr) = second_arg { + if ptr.starts_with('%') { + self.shadow_slot_allocas + .borrow_mut() + .insert(ptr.to_string()); + } + } + } + + /// The shadow slots bound in this function. See [`crate::root_reload`]. + pub(crate) fn shadow_slot_allocas(&self) -> Ref<'_, HashSet> { + self.shadow_slot_allocas.borrow() + } + /// Enter an invoke-EH handler scope: calls emitted from here until the /// matching pop unwind to `lpad_label`. pub fn push_eh_scope(&self, lpad_label: String) { @@ -244,6 +287,14 @@ impl LlBlock { &self.instructions } + /// Mutable instruction list, for the whole-function passes that run after + /// lowering. See [`crate::root_reload`]. Not for emitters — they go through + /// [`LlBlock::push_inst`] / [`LlBlock::emit`], which keep the terminator + /// discipline and the try-region bookkeeping. + pub(crate) fn insts_mut(&mut self) -> &mut Vec { + &mut self.instructions + } + // -------- Arithmetic (double) -------- // // FP ops are emitted with no LLVM fast-math flags by default. Setting @@ -1136,6 +1187,8 @@ impl LlBlock { pub fn call_void(&mut self, func_name: &str, args: &[(LlvmType, &str)]) { // #835 + #846: same registry hook as `call` — see comment there. crate::ext_registry::record_ffi_call(func_name); + self.counter + .note_shadow_slot_bind(func_name, args.get(1).map(|(_, v)| *v)); if let Some((cont, lpad)) = self.eh_invoke_suffix(func_name) { let arg_str = format_args(args); self.emit(format!( diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index ca55bbdaa8..7eb4691c06 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -2566,6 +2566,15 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> .native_rep_records .extend(typed_clone_rejection_records); + // #7280: re-read every shadow slot below the collection points that can run + // under it. Whole-function, so it runs here rather than inside a lowering — + // the shape it fixes is spread over dozens of lowerings, fifteen arms of + // `index_set.rs` alone. It runs BEFORE any rendering path so the text + // renderer and the in-process constructor see the same IR; a pass living in + // one of them would silently not apply to the other. + // See `crate::root_reload`. + crate::root_reload::apply_to_module(&mut llmod); + let verify_native_regions = opts.verify_native_regions || std::env::var("PERRY_VERIFY_NATIVE_REGIONS").ok().as_deref() == Some("1"); if verify_native_regions { diff --git a/crates/perry-codegen/src/function.rs b/crates/perry-codegen/src/function.rs index 44f13e740f..af546a8f98 100644 --- a/crates/perry-codegen/src/function.rs +++ b/crates/perry-codegen/src/function.rs @@ -453,6 +453,8 @@ impl LlFunction { /// emitted at the top of the entry block with the other entry setup. pub fn entry_setup_call_void(&mut self, func_name: &str, args: &[(LlvmType, &str)]) { crate::ext_registry::record_ffi_call(func_name); + self.reg_counter + .note_shadow_slot_bind(func_name, args.get(1).map(|(_, v)| *v)); let arg_str = args .iter() .map(|(ty, value)| format!("{} {}", ty, value)) @@ -542,6 +544,58 @@ impl LlFunction { &self.blocks } + /// Mutable block list, for the whole-function passes that run after + /// lowering. See [`crate::root_reload`]. + pub(crate) fn blocks_mut(&mut self) -> &mut Vec { + &mut self.blocks + } + + pub(crate) fn reg_counter(&self) -> &RegCounter { + &self.reg_counter + } + + pub(crate) fn reg_counter_rc(&self) -> Rc { + self.reg_counter.clone() + } + + /// Index in block 0 where `entry_post_init_setup` is spliced, if this + /// function has an init prelude. See [`note_entry_block_insertions`]. + /// + /// [`note_entry_block_insertions`]: LlFunction::note_entry_block_insertions + pub(crate) fn entry_init_boundary(&self) -> Option { + self.entry_init_boundary + } + + /// Tell the function that `n` instructions were inserted into block 0 **at + /// or above** the splice point, so the splice still lands in the same place + /// relative to the prelude. + /// + /// ★ The count must be exactly the insertions at index ≤ the boundary. + /// Neither error is cosmetic: + /// + /// - **under-counting** leaves the splice too early, and the tail of the + /// init prelude ends up below it — a `keys_array` global read hoisted + /// above the `__perry_init_strings_*` call that populates it, i.e. a + /// zero, silently; + /// - **over-counting** — bumping by every insertion, including the ones + /// below the boundary — leaves the splice too LATE, and `to_ir` clamps an + /// out-of-range boundary with `.min(instruction_count())`, which moves the + /// whole post-init region to the END of the entry block. For a function + /// built by `enable_post_init_shadow_frame` that region contains the + /// `js_shadow_frame_enter` call itself, so every `js_shadow_slot_bind` + /// in the body then runs with no frame pushed and roots NOTHING. Measured: + /// the allocation-point acceptance arm went 30/30 → 0/30 with + /// `TypeError: value is not a function`, which reads exactly like the + /// rooting bug the pass was written to fix. + pub(crate) fn note_entry_block_insertions(&mut self, n: usize) { + if n == 0 { + return; + } + if let Some(b) = self.entry_init_boundary.as_mut() { + *b += n; + } + } + pub fn num_blocks(&self) -> usize { self.blocks.len() } diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index fbc57cd2fb..7d81aa5705 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -30,6 +30,7 @@ pub mod native_emit; pub(crate) mod native_value; pub(crate) mod nm_install; pub mod opt_report; +pub(crate) mod root_reload; pub mod runtime_decls; pub(crate) mod stmt; pub mod strings; diff --git a/crates/perry-codegen/src/module.rs b/crates/perry-codegen/src/module.rs index 9cdc603ef0..50b6a372bf 100644 --- a/crates/perry-codegen/src/module.rs +++ b/crates/perry-codegen/src/module.rs @@ -379,6 +379,17 @@ impl LlModule { self.functions.get_mut(idx) } + /// Every defined function, mutably — for the whole-module passes that run + /// after lowering and before any rendering path. See + /// [`crate::root_reload`], and note that "before ANY rendering path" is the + /// load-bearing part: the text renderer (`to_ir`, `render_codegen_units`) + /// and the in-process constructor (`for_each_final_line`) are separate + /// consumers, so a pass living inside one of them would silently not apply + /// to the other. + pub(crate) fn functions_mut(&mut self) -> impl Iterator { + self.functions.iter_mut() + } + /// Number of functions defined so far. Used to recover the index of a /// just-`define_function`ed function (whose `&mut` borrow must be released /// before the index can be read) when emitting a sequence of functions — diff --git a/crates/perry-codegen/src/root_reload.rs b/crates/perry-codegen/src/root_reload.rs new file mode 100644 index 0000000000..6597aeabe8 --- /dev/null +++ b/crates/perry-codegen/src/root_reload.rs @@ -0,0 +1,1171 @@ +//! Re-read a shadow slot below every collection point that can run under it +//! (#7280). +//! +//! # The rule +//! +//! `docs/src/internals/gc-rooting-invariant.md` states it, and the second half +//! is the half that keeps getting dropped: +//! +//! > Any GC-managed value that is live across a collection point must be +//! > reachable from a root before that point. **A value read out of a root and +//! > held in an SSA register across a call is not rooted.** It is a copy, and +//! > the collector cannot see copies. +//! +//! A shadow slot has property (2) — the collector *rewrites* it on evacuation — +//! but a register loaded from it beforehand has only property (1), liveness. +//! The object survives, at a new address, and the register names the old one: +//! +//! ```llvm +//! %r7 = alloca double +//! store double %arg1, ptr %r7 +//! call void @js_shadow_slot_bind(i32 0, ptr %r7) ; %r7 IS a root +//! %r8 = load double, ptr %r7 ; a COPY of the root +//! %r9 = call double @build_schema() ; evacuates; rewrites %r7 +//! %r10 = call double @js_object_assign_one(double %r8, ...) ; from-space +//! ``` +//! +//! That is verbatim from `zod`'s object-spread lowering, and the very next +//! statement in the same function re-loads `%r7` — because a *fresh* lowering +//! of the same local emits a fresh load. The bug is never that codegen cannot +//! re-read the slot; it is that within one lowering the load happens once, at +//! the top, and the register is carried across everything that follows. +//! +//! # Why this is a pass and not another fix +//! +//! Because the shape is not in one lowering. Measured over the two IR corpora +//! (`scripts/gc_root_dominance_corpus.sh`, `scripts/gc_root_dominance_dep_corpus.sh`), +//! `--stale-registers --moving-only` reports 116 and 370 stale uses; 102 and 271 +//! of them are this one shape, spread across the object-literal spread, the +//! inline class-field store, the numeric element store, `new`, property +//! define, `instanceof`, the closure-call family and more. `index_set.rs` alone +//! lowers `object` before `value` at fifteen separate arms. Fixing them one at +//! a time is what #7291 and #7299 did, correctly, without moving the acceptance +//! arm at all. +//! +//! So the fix is stated once, over the emitted CFG, where the property is +//! decidable: +//! +//! > For a load out of a shadow slot, every use that a collection point can +//! > reach re-reads the slot instead. +//! +//! # Why re-loading is sound, and when it is not +//! +//! Re-reading a slot is NOT unconditionally safe, and `expr/temp_root.rs`'s +//! [`operand_is_reloadable`] documents exactly why: re-lowering a local reads +//! its value *now*, and "now" is after the later arguments have run — any of +//! which may have reassigned it. `new C(g, bump())` where `bump()` assigns `g` +//! must pass the pre-`bump()` `g`; re-lowering hands it the post-`bump()` one, +//! which is a miscompile rather than a rooting fix. +//! +//! That objection is about the SOURCE program's assignments, and at this level +//! they are visible: an assignment to a shadow-slotted local is a `store` to +//! its alloca in this function. (A local captured *and mutated* by a closure is +//! boxed instead — `boxed_vars.rs` — so it has no plain shadow slot to reload.) +//! So the rule carries its own side condition: +//! +//! > …unless a store to that slot can also run on the way, in which case the +//! > register is left alone. +//! +//! Both halves are path questions over the real CFG, not line order, and both +//! are answered here the same way `scripts/gc_root_dominance_check.py` answers +//! them — a back-edge round trip is not an intra-iteration path, because +//! re-entering the load's block re-executes the load and produces a different +//! dynamic value. +//! +//! Left alone means left as it is today: the 17 curated and 39 dependency-scale +//! residuals where the slot is reassigned in the window are genuinely the +//! temp-root case, and they keep their existing behaviour rather than getting a +//! wrong one. +//! +//! # Cost +//! +//! A reload is one load from a stack slot. Where it was not needed — no call +//! between the two points — LLVM's EarlyCSE/GVN forwards it away, because with +//! no intervening call nothing can clobber the alloca. Where it *was* needed +//! the intervening call is opaque, LLVM must keep the load, and that is the +//! whole point. So the pass is close to free exactly where it is redundant. +//! +//! [`operand_is_reloadable`]: crate::expr::temp_root + +use std::collections::{HashMap, HashSet, VecDeque}; + +use crate::function::LlFunction; +use crate::inst::{LlInst, LoadFlavor}; +use crate::types::LlvmType; + +/// Runtime helpers that provably cannot allocate, run user code, or poll, and +/// which generated code emits often enough that treating them as collection +/// points would put a reload between every pair of instructions. +/// +/// **This list is deliberately a SUBSET of `NONCOLLECTING` in +/// `scripts/gc_root_dominance_check.py`**, which is the authority and names the +/// runtime line proving each entry. A subset is the safe direction: a helper +/// missing from here is treated as collecting, which inserts a reload the +/// checker would not have demanded — a load, not a bug. A helper that is here +/// and is NOT in the checker's set would be the unsafe direction, so the +/// invariant to preserve on edit is one-way containment. +const NON_COLLECTING: &[&str] = &[ + // shadow stack / roots + "js_shadow_slot_bind", + "js_shadow_slot_set", + "js_shadow_frame_enter", + "js_shadow_frame_push", + "js_shadow_frame_pop", + "js_shadow_state_addr", + "js_gc_temp_root_push", + "js_gc_temp_root_get", + "js_gc_temp_root_set", + "js_gc_temp_root_truncate", + // layout / barrier bookkeeping + "js_gc_init_typed_shape_layout", + "js_gc_layout_note_slot", + "js_write_barrier", + "js_write_barrier_root_nanbox", + "js_write_barrier_slot", + "js_runtime_write_barrier_slot", + "js_gc_register_global_root", + // pure value predicates / bit twiddling + "js_is_truthy", + "js_nanbox_get_pointer", + "js_value_is_object", + "js_value_is_string", + "js_typeof_tag", + // inline-cache guards: pure reads + "js_typed_feedback_closure_direct_call_guard", + "js_typed_feedback_shape_guard", + "js_typed_feedback_note", + // verified non-allocating bookkeeping stores/reads + "js_closure_set_capture_bits", + "js_closure_get_capture_bits", + "js_closure_set_capture_ptr", + "js_closure_get_capture_ptr", + "js_box_set_bits", + "js_box_get_bits", + "js_i32_box_set", + "js_bool_box_set", + "js_tdz_suppress_begin", + "js_tdz_suppress_end", + "js_array_note_numeric_write", + "js_array_length", + "js_object_mark_class", + "js_class_object_pin_parent", + "js_new_target_get", + "js_new_target_set", + "js_ctor_return_override", +]; + +/// Guard against a pathological function turning this pass into the compile's +/// bottleneck: the reachability walk is O(blocks) per slot load, so the product +/// is what matters. Above the cap the function keeps today's IR — the pass is +/// an improvement, not a correctness precondition, so declining is safe. +/// +/// Sized from the corpora: the largest function in the dependency-scale corpus +/// (`zod`'s parse core) is ~1400 blocks with ~90 slot loads, an order of +/// magnitude under this. +const MAX_BLOCK_LOAD_PRODUCT: usize = 8_000_000; + +fn is_collecting(callee: &str) -> bool { + if callee.starts_with("llvm.") { + return false; + } + !NON_COLLECTING.contains(&callee) +} + +/// One instruction, in the vocabulary this pass needs. +struct Facts { + /// Register this instruction defines, without the `%`. Kept even though + /// only `load_of` reads it today: `Facts` is the pass's whole vocabulary, + /// and a def map is the first thing a follow-up (a dead-load sweep, say) + /// needs. + #[allow(dead_code)] + result: Option, + /// Registers it reads, without the `%`. Only operands — never `result`. + uses: Vec, + /// `Some((dst, ty, slot))` for `dst = load ty, ptr %slot`. + load_of: Option<(String, LlvmType, String)>, + /// Alloca this instruction stores into, without the `%`. + stores_to: Option, + collecting: bool, + /// A `phi` cannot have an instruction inserted before it. Neither can an + /// inline block label (#7305's `invoke` continuation, emitted as a `Raw` + /// `:` line inside the same builder block): inserting there would put + /// an instruction between a terminator and its label. + is_phi: bool, + /// Successor block labels, for the CFG. + succs: Vec, +} + +/// Apply the reload rule to every function in `module`. Returns the number of +/// operands rewritten, which the unit tests assert on so a pass that silently +/// stops firing is a failure rather than a no-op. +pub(crate) fn apply_to_module(module: &mut crate::module::LlModule) -> usize { + let mut total = 0; + for f in module.functions_mut() { + total += apply_to_function(f); + } + total +} + +pub(crate) fn apply_to_function(func: &mut LlFunction) -> usize { + let slots: HashSet = { + let bound = func.reg_counter().shadow_slot_allocas(); + if bound.is_empty() { + return 0; + } + bound + .iter() + .map(|s| s.trim_start_matches('%').to_string()) + .collect() + }; + + let blocks = func.blocks_mut(); + if blocks.is_empty() { + return 0; + } + + let facts: Vec> = blocks + .iter() + .map(|b| b.insts().iter().map(|i| facts_of(i, &slots)).collect()) + .collect(); + + let label_index: HashMap<&str, usize> = blocks + .iter() + .enumerate() + .map(|(i, b)| (b.label.as_str(), i)) + .collect(); + + // Slot loads, and where they are. + let mut loads: Vec<(usize, usize)> = Vec::new(); + for (bi, fb) in facts.iter().enumerate() { + for (ii, f) in fb.iter().enumerate() { + if f.load_of.is_some() { + loads.push((bi, ii)); + } + } + } + if loads.is_empty() { + return 0; + } + if blocks.len().saturating_mul(loads.len()) > MAX_BLOCK_LOAD_PRODUCT { + return 0; + } + + let succs: Vec> = facts + .iter() + .map(|fb| { + let mut out: Vec = Vec::new(); + for f in fb { + for s in &f.succs { + if let Some(&i) = label_index.get(s.as_str()) { + if !out.contains(&i) { + out.push(i); + } + } + } + } + out + }) + .collect(); + + // Where a use must be rewritten: (block, insn, old register, new register, + // slot, type). Collected first so the instruction vectors are not mutated + // while `facts` still indexes them. + struct Rewrite { + blk: usize, + insn: usize, + from: String, + slot: String, + ty: LlvmType, + } + let mut rewrites: Vec = Vec::new(); + + for (lb, li) in loads { + let (dst, ty, slot) = facts[lb][li].load_of.clone().expect("load site"); + // Forward reachability from the load, never re-entering the load's own + // block: a back edge re-executes the load, so the value on the far side + // is a different dynamic instance and not this one. + // + // `collect_in[b]` / `store_in[b]`: can a collecting call / a store to + // `slot` have run on some path from the load to the TOP of block `b`? + let mut collect_in = vec![false; facts.len()]; + let mut store_in = vec![false; facts.len()]; + let mut seen = vec![false; facts.len()]; + let mut queue: VecDeque = VecDeque::new(); + + // Tail of the load's own block, from just after the load. + let mut c = false; + let mut s = false; + for f in facts[lb].iter().skip(li + 1) { + c |= f.collecting; + s |= f.stores_to.as_deref() == Some(slot.as_str()); + } + for &succ in &succs[lb] { + if succ == lb { + continue; + } + if !seen[succ] || (c && !collect_in[succ]) || (s && !store_in[succ]) { + collect_in[succ] |= c; + store_in[succ] |= s; + seen[succ] = true; + queue.push_back(succ); + } + } + while let Some(b) = queue.pop_front() { + let mut oc = collect_in[b]; + let mut os = store_in[b]; + for f in &facts[b] { + oc |= f.collecting; + os |= f.stores_to.as_deref() == Some(slot.as_str()); + } + for &succ in &succs[b] { + if succ == lb { + continue; + } + let grew = !seen[succ] || (oc && !collect_in[succ]) || (os && !store_in[succ]); + if grew { + collect_in[succ] |= oc; + store_in[succ] |= os; + seen[succ] = true; + queue.push_back(succ); + } + } + } + + for (ub, fb) in facts.iter().enumerate() { + // Only blocks the load can reach without re-entering its own block, + // plus the load's own block below the load. + if ub != lb && !seen[ub] { + continue; + } + let (mut c, mut s) = if ub == lb { + (false, false) + } else { + (collect_in[ub], store_in[ub]) + }; + let start = if ub == lb { li + 1 } else { 0 }; + for (ui, f) in fb.iter().enumerate().skip(start) { + if f.uses.iter().any(|u| *u == dst) && c && !s && !f.is_phi { + rewrites.push(Rewrite { + blk: ub, + insn: ui, + from: dst.clone(), + slot: slot.clone(), + ty, + }); + } + c |= f.collecting; + s |= f.stores_to.as_deref() == Some(slot.as_str()); + } + } + } + + if rewrites.is_empty() { + return 0; + } + + // Apply back-to-front within each block so earlier indices stay valid. + rewrites.sort_by(|a, b| (a.blk, a.insn).cmp(&(b.blk, b.insn)).reverse()); + let n = rewrites.len(); + let counter = func.reg_counter_rc(); + // ★ Only the insertions AT OR ABOVE the post-init splice point move it. + // Counting the ones below it too pushes the splice past the end of the + // entry block, `to_ir` clamps it, and the whole post-init region — which + // for a post-init-frame function contains `js_shadow_frame_enter` itself — + // lands after every `js_shadow_slot_bind` in the body. See + // `LlFunction::note_entry_block_insertions`; the symptom is indistinguishable + // from the bug this pass exists to fix, which is how it was found. + let boundary = func.entry_init_boundary(); + let entry_inserts = match boundary { + None => 0, + Some(b) => rewrites + .iter() + .filter(|r| r.blk == 0 && r.insn <= b) + .count(), + }; + { + let blocks = func.blocks_mut(); + for r in rewrites { + let fresh = format!("%r{}", counter.next()); + let reload = LlInst::Load { + dst: fresh.clone(), + ty: r.ty, + ptr: format!("%{}", r.slot), + flavor: LoadFlavor::Plain, + }; + let insts = blocks[r.blk].insts_mut(); + rename_operand(&mut insts[r.insn], &r.from, fresh.trim_start_matches('%')); + insts.insert(r.insn, reload); + } + } + func.note_entry_block_insertions(entry_inserts); + n +} + +fn facts_of(inst: &LlInst, slots: &HashSet) -> Facts { + let mut uses = Vec::new(); + let mut result = None; + let mut load_of = None; + let mut stores_to = None; + let mut collecting = false; + let mut is_phi = false; + let mut succs = Vec::new(); + + let reg = |s: &str| -> Option { + s.strip_prefix('%') + .map(|r| r.to_string()) + .filter(|r| !r.is_empty()) + }; + let use_op = |uses: &mut Vec, s: &str| { + if let Some(r) = reg(s) { + uses.push(r); + } + }; + + match inst { + LlInst::Raw(text) => return raw_facts(text, slots), + LlInst::Bin { dst, a, b, .. } => { + result = reg(dst); + use_op(&mut uses, a); + use_op(&mut uses, b); + } + LlInst::FNeg { dst, a, .. } => { + result = reg(dst); + use_op(&mut uses, a); + } + LlInst::FCmp { dst, a, b, .. } | LlInst::ICmp { dst, a, b, .. } => { + result = reg(dst); + use_op(&mut uses, a); + use_op(&mut uses, b); + } + LlInst::Alloca { dst, .. } => result = reg(dst), + LlInst::Load { dst, ty, ptr, .. } => { + result = reg(dst); + use_op(&mut uses, ptr); + if let (Some(d), Some(p)) = (reg(dst), reg(ptr)) { + if slots.contains(&p) { + load_of = Some((d, *ty, p)); + } + } + } + LlInst::Store { val, ptr, .. } => { + use_op(&mut uses, val); + use_op(&mut uses, ptr); + stores_to = reg(ptr); + } + LlInst::Cast { dst, v, .. } => { + result = reg(dst); + use_op(&mut uses, v); + } + LlInst::Select { + dst, cond, a, b, .. + } => { + result = reg(dst); + use_op(&mut uses, cond); + use_op(&mut uses, a); + use_op(&mut uses, b); + } + LlInst::Call { + dst, callee, args, .. + } => { + result = dst.as_deref().and_then(reg); + for (_, v) in args { + use_op(&mut uses, v); + } + collecting = is_collecting(callee); + } + LlInst::CallIndirect { + dst, fptr, args, .. + } => { + result = reg(dst); + use_op(&mut uses, fptr); + for (_, v) in args { + use_op(&mut uses, v); + } + collecting = true; + } + LlInst::AsmBarrier => {} + LlInst::Br { label } => succs.push(label.clone()), + LlInst::CondBr { cond, t, f } => { + use_op(&mut uses, cond); + succs.push(t.clone()); + succs.push(f.clone()); + } + LlInst::Ret { val, .. } => use_op(&mut uses, val), + LlInst::RetVoid | LlInst::Unreachable => {} + LlInst::Gep { dst, ptr, idxs, .. } => { + result = reg(dst); + use_op(&mut uses, ptr); + for (_, v) in idxs { + use_op(&mut uses, v); + } + } + LlInst::Phi { dst, pairs, .. } => { + result = reg(dst); + is_phi = true; + for (v, _) in pairs { + use_op(&mut uses, v); + } + } + } + + Facts { + result, + uses, + load_of, + stores_to, + collecting, + is_phi, + succs, + } +} + +/// `Raw` is the escape hatch `emit_raw` writes through, so its facts come from +/// the rendered line. Everything here is one-sided towards doing nothing: an +/// operand form this does not recognise contributes no rewrite, and a call it +/// cannot name is treated as collecting. +fn raw_facts(text: &str, slots: &HashSet) -> Facts { + let mut uses = Vec::new(); + let mut result = None; + let mut load_of = None; + let mut stores_to = None; + let mut succs = Vec::new(); + + let body = text.trim_start(); + let (lhs, rhs) = match body.split_once(" = ") { + Some((l, r)) if l.starts_with('%') && !l.contains(' ') => { + result = Some(l.trim_start_matches('%').to_string()); + (Some(l), r) + } + _ => (None, body), + }; + let _ = lhs; + + for tok in registers_in(rhs) { + uses.push(tok); + } + // A bare `