From 1a88e92902048c9d0b51b91fdfdaf4c2a40819e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 08:26:29 +0200 Subject: [PATCH 1/8] wip(codegen): route class-id dispatch tower to proven-this clone behind inline keys check (#7142) --- crates/perry-codegen/src/codegen/closure.rs | 1 + crates/perry-codegen/src/codegen/entry.rs | 2 + crates/perry-codegen/src/codegen/function.rs | 1 + crates/perry-codegen/src/codegen/method.rs | 2 + crates/perry-codegen/src/codegen/mod.rs | 15 ++ crates/perry-codegen/src/codegen/opts.rs | 7 + crates/perry-codegen/src/collectors/mod.rs | 5 +- .../src/collectors/proven_this.rs | 24 +++ .../src/collectors/repsel_benefit.rs | 76 +++++++++- .../src/expr/class_field_inline_guard.rs | 100 +++++++++++++ crates/perry-codegen/src/expr/mod.rs | 6 + .../property_get/dynamic_dispatch.rs | 140 +++++++++++++++++- 12 files changed, 372 insertions(+), 7 deletions(-) diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 773327fe4f..ce4fa29486 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -1000,6 +1000,7 @@ pub(super) fn compile_closure( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this: None, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 146213b95a..be3605438f 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -862,6 +862,7 @@ pub(super) fn compile_module_entry( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this: None, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, @@ -1523,6 +1524,7 @@ pub(super) fn compile_module_entry( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this: None, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 4fdc676d53..a48c35b494 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -835,6 +835,7 @@ pub(super) fn compile_function( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this: None, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index b974296389..c56f1a2ab8 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -577,6 +577,7 @@ pub(super) fn compile_method( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, @@ -1632,6 +1633,7 @@ pub(super) fn compile_static_method( typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps, typed_f64_methods: &cross_module.typed_f64_methods, pshape_methods: &cross_module.pshape_methods, + pshape_tower_routable: &cross_module.pshape_tower_routable, proven_this: None, typed_i32_methods: &cross_module.typed_i32_methods, typed_i1_methods: &cross_module.typed_i1_methods, diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index be26ac34fb..e9cb91d3f9 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1388,6 +1388,9 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> (String, String), crate::collectors::PtrShapeLocal, > = std::collections::HashMap::new(); + // #7142: the profitability subset the class-id dispatch tower may route to. + let mut pshape_tower_routable: std::collections::HashSet<(String, String)> = + std::collections::HashSet::new(); // Phase 3b typed-receiver widening: chain-global field indexes need the // full class table — and it must be the SAME table dynamic dispatch's // call-site gating consults (`class_table`, incl. class-expression @@ -1410,6 +1413,17 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> receiver_class_table, &module_dispatch_facts, ) { + // #7142: the tower routing site emits its own inline shape + // re-check, so it only takes the clone where the clone deletes + // strictly more guarded field sites than that check costs. The + // other two sites are guard-dominated and route unconditionally. + if crate::collectors::pshape_tower_route_profitable( + class, + method, + receiver_class_table, + ) { + pshape_tower_routable.insert((class.name.clone(), method.name.clone())); + } pshape_methods.insert((class.name.clone(), method.name.clone()), fact); } match typed_abi::typed_f64_method_rejection_reason(method) { @@ -1708,6 +1722,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> typed_i1_method_param_reps, typed_f64_receiver_methods, pshape_methods, + pshape_tower_routable, typed_f64_closures: std::collections::HashSet::new(), typed_i32_closures: std::collections::HashSet::new(), typed_i1_closures: std::collections::HashSet::new(), diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index 7528b1cf9c..39435a42cc 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -806,6 +806,13 @@ pub(crate) struct CrossModuleCtx { /// instance with a different chain. pub pshape_methods: std::collections::HashMap<(String, String), crate::collectors::PtrShapeLocal>, + /// #7142: the subset of [`Self::pshape_methods`] whose clone the class-id + /// dispatch tower may route to. The other two routing sites are dominated by + /// a shape guard they pay regardless, so the clone is free for them; the + /// tower has to emit its own inline shape re-check, so it only routes where + /// the clone deletes strictly more work than that check costs + /// (`collectors/repsel_benefit.rs`). + pub pshape_tower_routable: std::collections::HashSet<(String, String)>, /// Inline closure bodies that have a generated internal typed-f64 clone. /// Only statically-known local closure calls may select these clones after /// closure identity/arity and numeric argument guards pass. diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 7d9abe9970..99f4fa865d 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -71,7 +71,10 @@ pub(crate) use integer_locals::{ pub(crate) use local_refs::{expr_contains_local_get, mark_all_candidate_refs_in_expr}; pub(crate) use mutation::has_any_mutation; pub(crate) use pointer_locals::collect_pointer_typed_locals; -pub(crate) use proven_this::{method_proven_this, prune_colliding_clones, pshape_method_name}; +pub(crate) use proven_this::{ + method_proven_this, prune_colliding_clones, pshape_method_name, + tower_route_profitable as pshape_tower_route_profitable, +}; pub(crate) use ptr_numarray::{NumArrayDensity, NumArrayLocal}; pub(crate) use ptr_shape::{ptr_shape_locals_enabled, PtrShapeLocal}; pub(crate) use refs::{ diff --git a/crates/perry-codegen/src/collectors/proven_this.rs b/crates/perry-codegen/src/collectors/proven_this.rs index fdcd3b3dc7..8509d1d03f 100644 --- a/crates/perry-codegen/src/collectors/proven_this.rs +++ b/crates/perry-codegen/src/collectors/proven_this.rs @@ -228,6 +228,30 @@ pub(crate) fn method_proven_this( }) } +/// #7142 profitability: should a class-id dispatch-tower case route to +/// `method`'s `{public}__pshape` clone? +/// +/// Only meaningful once [`method_proven_this`] has admitted a clone — this adds +/// the "should we?" half that the admission test (a pure "may we?" conjunction) +/// deliberately does not carry. Refusing is always sound: the tower case keeps +/// calling the public body. +/// +/// The two other routing sites do NOT consult this. They are dominated by a +/// shape guard that is paid whether or not the clone is taken, so for them the +/// clone is free; only the tower pays for its own proof. +pub(crate) fn tower_route_profitable( + class: &Class, + method: &Function, + classes: &HashMap, +) -> bool { + let chain = chain_classes(classes, &class.name); + if chain.is_empty() { + return false; + } + let fields = chain_field_names(&chain); + super::repsel_benefit::tower_route_profitable(method, &fields) +} + /// Does the method body READ `this.` anywhere? fn method_reads_chain_field(method: &Function, fields: &HashSet) -> bool { let mut found = false; diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index 17bd605381..251350730d 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -109,7 +109,27 @@ use std::collections::HashSet; -use perry_hir::{BinaryOp, Expr, Stmt, UnaryOp}; +use perry_hir::{BinaryOp, Expr, Function, Stmt, UnaryOp}; + +/// #7142: how many guarded `this.` sites a proven-receiver +/// clone must delete before routing a class-id dispatch-tower case to it pays +/// for the inline shape re-check the route adds. +/// +/// The re-check (`expr::class_field_inline_guard::emit_proven_shape_recheck`) +/// is one instance of the same header check the per-access inline guard emits +/// at every `this.field` site inside the public body. So the trade at a tower +/// case is *N in-body checks for 1 at the call site*, and the break-even is +/// exactly `N == 1`: at one field site the route swaps a check for a check, +/// adds a second call site (code size) and a branch, and is strictly worse +/// whenever that single site sits behind a conditional the call does not always +/// reach. At two it deletes one whole check plus its cold guard-call block, and +/// the margin grows with every further site. +/// +/// This is a *count*, not a cost model: it has no target-specific term, so it +/// says the same thing on AArch64 and x86-64 (contrast the fusion term in +/// #7146, which does not). It deliberately under-counts — see +/// [`proven_receiver_clone_field_sites`]. +const TOWER_ROUTE_MIN_FIELD_SITES: u32 = 2; /// What the *consumer* of a read wants the value to be. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -460,6 +480,60 @@ pub(crate) fn collect_unprofitable_canonical_i32_locals( .collect() } +/// #7142: how many guarded `this.` sites the +/// proven-receiver clone of `method` deletes. +/// +/// Counts the HIR forms the clone lowers guard-free — `PropertyGet`, +/// `PropertySet`, `PropertyUpdate` and `PutValueSet` on `this` — restricted to +/// fields the class chain actually declares, because only those get a fixed +/// slot; anything else keeps its by-name lowering in the clone too. +/// +/// Deliberately an UNDER-count in two directions, both of which push toward +/// refusing a route rather than taking one: +/// +/// * a site inside a loop is counted once, though it is paid per iteration; +/// * `this.other()` calls that the clone also lowers guard-free (the +/// `ThisFlowAnalysis` walk vets them transitively) are not followed. +pub(crate) fn proven_receiver_clone_field_sites( + method: &Function, + chain_fields: &HashSet, +) -> u32 { + let mut sites = 0u32; + super::scalar_method_dispatch::for_each_expr_in_stmts(&method.body, &mut |e| { + let named = match e { + Expr::PropertyGet { + object, property, .. + } + | Expr::PropertySet { + object, property, .. + } + | Expr::PropertyUpdate { + object, property, .. + } => matches!(object.as_ref(), Expr::This) && chain_fields.contains(property.as_str()), + Expr::PutValueSet { target, key, .. } => { + matches!(target.as_ref(), Expr::This) + && matches!(key.as_ref(), Expr::String(k) if chain_fields.contains(k.as_str())) + } + _ => false, + }; + if named { + sites += 1; + } + }); + sites +} + +/// #7142: may a class-id dispatch-tower case route to `method`'s +/// proven-receiver clone? +/// +/// A pure PROFITABILITY verdict — refusing is always sound (the case keeps +/// calling the public body, which is correct for any receiver), and the +/// soundness half lives entirely in the emitted keys check. See +/// [`TOWER_ROUTE_MIN_FIELD_SITES`] for the break-even argument. +pub(crate) fn tower_route_profitable(method: &Function, chain_fields: &HashSet) -> bool { + proven_receiver_clone_field_sites(method, chain_fields) >= TOWER_ROUTE_MIN_FIELD_SITES +} + #[cfg(test)] #[path = "repsel_benefit/tests.rs"] mod tests; diff --git a/crates/perry-codegen/src/expr/class_field_inline_guard.rs b/crates/perry-codegen/src/expr/class_field_inline_guard.rs index 0281ac6d92..0faacbbc9c 100644 --- a/crates/perry-codegen/src/expr/class_field_inline_guard.rs +++ b/crates/perry-codegen/src/expr/class_field_inline_guard.rs @@ -49,6 +49,9 @@ const OBJECT_TYPE_REGULAR: &str = "1"; const TYPED_LAYOUT_INTACT_BIT: &str = "4096"; // GC_OBJ_TYPED_LAYOUT_INTACT (0x1000) const OBJ_FLAG_FROZEN_BIT: &str = "1"; // OBJ_FLAG_FROZEN (0x01) const OBJ_FLAG_HAS_DESCRIPTORS_BIT: &str = "2048"; // OBJ_FLAG_HAS_DESCRIPTORS (0x800) +/// `OBJ_FLAG_FROZEN | OBJ_FLAG_HAS_DESCRIPTORS` — both live in the same +/// `GcHeader::_reserved` i16, so one mask tests both. +const OBJ_FLAG_FROZEN_OR_DESCRIPTORS: &str = "2049"; const F64_EXP_MASK: &str = "9218868437227405312"; // 0x7FF0_0000_0000_0000 /// Emit the `i1` "plain finite number" predicate on a value's raw bits: true @@ -196,6 +199,103 @@ pub(crate) fn emit_class_field_loop_preheader_check( } } +/// #7142: the inline shape re-check that licenses routing a class-id dispatch +/// tower case to a proven-receiver method clone. +/// +/// ## What the caller has already established +/// +/// The caller emits this INTO a dispatch-tower case block, i.e. a block reached +/// only when `js_object_get_class_id(obj_handle)` returned a specific non-zero +/// user class id. That call already rejects the handle band, the built-in +/// Set/Map/RegExp registries, out-of-heap-range addresses, and any allocation +/// whose `GcHeader.obj_type` is not `GC_TYPE_OBJECT` +/// (`object/field_get_set/field_ops.rs`). So every predicate +/// [`emit_class_field_inline_precheck`] evaluates *before* its dereference is +/// already discharged, and the loads below need no gate/deref split — they all +/// fit in the caller's single basic block. +/// +/// ## What is left, and why each one +/// +/// * **`keys_array` identity** — the load-bearing one. `delete inst.f` compacts +/// the packed inline slots while PRESERVING `class_id`, so a class-id match +/// alone does not prove the layout: on `class C { a; b; c }`, `delete inst.b` +/// moves `c` from slot 2 to slot 1. The compaction installs a freshly CLONED +/// keys array (`object/delete_rest.rs` — `js_array_alloc` + +/// `set_object_keys_array`, cloned precisely because the old array is shared +/// between every instance of the shape), so a pointer compare against the +/// class's `@perry_class_keys_*` global catches it. The check is deliberately +/// DYNAMIC: the `delete` shape barrier that stands the analysis down is +/// module-scoped while receivers alias across modules (#7143), so no static +/// proof is available at this site. +/// * **The sticky `@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch** — flipped +/// the moment a descriptor / accessor lands on a class prototype or on +/// `Object.prototype`, or typed-feedback tracing turns on. This is the very +/// latch the per-access inline guard *inside the body being replaced* reads, +/// so a routed call is never weaker than the lowering it displaces. +/// * **Per-object `OBJ_FLAG_HAS_DESCRIPTORS`** — instance-level descriptor +/// installs deliberately do NOT flip the process-global latch (#5654), so +/// they are vetted per receiver, exactly as the per-access check does. +/// * **`OBJ_FLAG_FROZEN`** — a proven-receiver clone may contain field WRITES, +/// and a guard-free raw store into a frozen receiver would silently succeed +/// where the spec requires a strict-mode `TypeError`. The clone's own +/// admission rules this out only through a MODULE-scoped freeze-barrier kill, +/// so the receiver is vetted here as well. +/// * **Not-forwarded** and **`object_type == OBJECT_TYPE_REGULAR`** — the two +/// header predicates `js_object_get_class_id` does not itself check. +/// +/// Cost: one volatile `i8` load of the latch, three loads off the receiver (two +/// of them from the `GcHeader` word the tower's class-id read already pulled +/// in), nine ALU ops and one conditional branch. `expected_keys` is expected to +/// come from an entry-hoisted slot (`LlFunction::entry_init_load_global`), so +/// the global itself is read once per function, not per call. +/// +/// Emits into the CURRENT block and terminates it; the caller supplies both +/// successor labels and sets `ctx.current_block` afterwards. +pub(crate) fn emit_proven_shape_recheck( + ctx: &mut FnCtx, + obj_handle: &str, + expected_keys: &str, + proven_label: &str, + generic_label: &str, +) { + let blk = ctx.block(); + + // Policy latch first — volatile for the same reason the per-access check + // loads it volatile: the runtime flips it sticky 0 -> 1 mid-execution and + // LLVM must not hoist a stale 0 across the flip. + let flag = blk.load_volatile(I8, "@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED"); + let flag_ok = blk.icmp_eq(I8, &flag, "0"); + + let obj_ptr = blk.inttoptr(I64, obj_handle); + + // GcHeader (precedes the object by 8 bytes): gc_flags @-7 (i8), + // _reserved @-6 (i16). + let gflags_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-7")]); + let gflags = blk.load(I8, &gflags_ptr); + let fwd = blk.and(I8, &gflags, GC_FLAG_FORWARDED_I8); + let not_fwd = blk.icmp_eq(I8, &fwd, "0"); + + let res_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-6")]); + let reserved = blk.load(I16, &res_ptr); + let latched = blk.and(I16, &reserved, OBJ_FLAG_FROZEN_OR_DESCRIPTORS); + let unlatched = blk.icmp_eq(I16, &latched, "0"); + + // ObjectHeader: object_type @0 (i32), keys_array @16 (i64). `class_id` @4 + // was already matched by the tower's `js_object_get_class_id` compare. + let object_type = blk.load(I32, &obj_ptr); + let ot_ok = blk.icmp_eq(I32, &object_type, OBJECT_TYPE_REGULAR); + + let ka_ptr = blk.gep(I8, &obj_ptr, &[(I64, "16")]); + let keys_array = blk.load(I64, &ka_ptr); + let ka_ok = blk.icmp_eq(I64, &keys_array, expected_keys); + + let mut acc = blk.and(I1, &flag_ok, ¬_fwd); + acc = blk.and(I1, &acc, &unlatched); + acc = blk.and(I1, &acc, &ot_ok); + acc = blk.and(I1, &acc, &ka_ok); + blk.cond_br(&acc, proven_label, generic_label); +} + /// Emit the inline class-field shape pre-check. /// /// Before calling, the caller must have already created `fast_label` (the slot diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 279f205ccd..4b5055341b 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -876,6 +876,12 @@ pub(crate) struct FnCtx<'a> { pub pshape_methods: &'a std::collections::HashMap<(String, String), crate::collectors::PtrShapeLocal>, + /// #7142: the subset of [`Self::pshape_methods`] the class-id dispatch + /// tower may route to. A profitability filter only — see + /// `collectors::pshape_tower_route_profitable`. Soundness at that site comes + /// entirely from the emitted inline keys check, never from this set. + pub pshape_tower_routable: &'a std::collections::HashSet<(String, String)>, + /// Locals referenced anywhere inside a nested closure body (including /// explicit capture lists). Excluded from canonical-i32 selection — the /// capture machinery stays on the boxed protocol. Empty when diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index bb01846a85..b27301671e 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -18,6 +18,114 @@ use crate::lower_call::method_override::{ emit_guarded_direct_method_call, emit_own_method_override_check, }; +/// #7142: the proven-`this` clone a class-id dispatch-tower case may route to, +/// plus the keys token the routed path must re-check inline. +struct TowerPshapeRoute { + /// The `{public}__pshape` clone symbol (same `(double this, args…)` ABI as + /// the public one — only the body's `this.field` lowering differs). + clone_fn: String, + /// `@perry_class_keys___`, holding the class's canonical + /// keys-array pointer. A receiver still carrying it has the declared packed + /// layout; `delete` swaps in a freshly cloned array, which is exactly what + /// the inline compare catches. + keys_global: String, +} + +/// May this dispatch-tower case take the proven-`this` clone? +/// +/// Three conditions, and only the FIRST two are about correctness of the +/// *target*; the layout proof itself is emitted, not decided here. +/// +/// 1. `owner` is `Some` — the receiver class of this case declares `property` +/// itself, so the clone's `this` is exactly the class it was compiled for. +/// 2. a clone was actually emitted for that pair (`pshape_methods`, already +/// pruned of symbol collisions by `prune_colliding_clones`, #6927). +/// 3. routing pays (`pshape_tower_routable`) — this site, unlike the two +/// guard-dominated ones, emits its own shape re-check and must earn it. +fn tower_pshape_route( + ctx: &FnCtx<'_>, + owner: Option<&str>, + property: &str, + fname: &str, +) -> Option { + let owner = owner?; + // Carried forward from both existing routing sites (the #1787 + // static-receiver bug): a `perry_static_*` target needs + // `js_class_static_method_call`, not a plain `call double`, and no + // proven-`this` clone is ever emitted for one. + if fname.starts_with("perry_static_") { + return None; + } + let key = (owner.to_string(), property.to_string()); + if !ctx.pshape_methods.contains_key(&key) || !ctx.pshape_tower_routable.contains(&key) { + return None; + } + let keys_global = ctx.class_keys_globals.get(owner)?.clone(); + Some(TowerPshapeRoute { + clone_fn: crate::collectors::pshape_method_name(fname), + keys_global, + }) +} + +/// Emit the keys-guarded diamond for one dispatch-tower case: inline shape +/// re-check → `{public}__pshape` on a match, the unchanged public body +/// otherwise. +/// +/// The tower's case block proves `class_id`, which is NOT enough for the +/// clone's bare fixed-slot accesses — `delete inst.f` compacts the packed slots +/// while preserving `class_id`. The keys token is what closes that gap, and it +/// has to be checked *dynamically*: the `delete` shape barrier that stands the +/// whole analysis down is module-scoped while a receiver can be deleted from +/// through an alias in another module (#7143), so a static proof would be +/// exactly the wrong instrument here. +fn emit_tower_pshape_call( + ctx: &mut FnCtx<'_>, + case_no: usize, + route: &TowerPshapeRoute, + recv_handle: &str, + fname: &str, + case_arg_slices: &[(crate::types::LlvmType, &str)], +) -> String { + // The global is read ONCE per function (entry-hoisted); the case block only + // reloads it from the stack slot, which mem2reg folds away. + let keys_slot = ctx.func.entry_init_load_global(&route.keys_global, I64); + let expected_keys = ctx.block().load(I64, &keys_slot); + + let proven_idx = ctx.new_block(&format!("idispatch.case{}.pshape", case_no)); + let generic_idx = ctx.new_block(&format!("idispatch.case{}.generic", case_no)); + let join_idx = ctx.new_block(&format!("idispatch.case{}.join", case_no)); + let proven_label = ctx.block_label(proven_idx); + let generic_label = ctx.block_label(generic_idx); + let join_label = ctx.block_label(join_idx); + + crate::expr::class_field_inline_guard::emit_proven_shape_recheck( + ctx, + recv_handle, + &expected_keys, + &proven_label, + &generic_label, + ); + + ctx.current_block = proven_idx; + let v_proven = ctx.block().call(DOUBLE, &route.clone_fn, case_arg_slices); + let proven_end = ctx.block().label.clone(); + ctx.block().br(&join_label); + + ctx.current_block = generic_idx; + let v_generic = ctx.block().call(DOUBLE, fname, case_arg_slices); + let generic_end = ctx.block().label.clone(); + ctx.block().br(&join_label); + + ctx.current_block = join_idx; + ctx.block().phi( + DOUBLE, + &[ + (v_proven.as_str(), proven_end.as_str()), + (v_generic.as_str(), generic_end.as_str()), + ], + ) +} + /// Interface / dynamic dispatch fallback: when the static class is unknown OR /// resolves to an interface name not in the class registry, BUT the property /// name corresponds to a method defined on at least one class in the registry, @@ -106,6 +214,14 @@ pub(crate) fn try_lower_instance_method_call( // `implementors`, so each case block can build its own per-arity args // without rescanning `ctx.methods`. let mut impl_meta: Vec<(bool, usize)> = Vec::new(); + // #7142: aligned 1:1 with `implementors` — `Some(class)` exactly when + // the receiver class of this case DECLARES `property` itself (the walk + // stopped at its own entry). That is the condition a proven-`this` + // clone needs: the class the clone was compiled for is then the + // receiver's exact dynamic class, so `this` cannot be a subclass + // instance with a longer chain. Inherited dispatch gets `None` and + // keeps today's lowering. + let mut impl_owner: Vec> = Vec::new(); let mut seen_pairs: std::collections::HashSet<(u32, String)> = std::collections::HashSet::new(); for (start_cls, &start_cid) in ctx.class_ids.iter() { @@ -118,6 +234,7 @@ pub(crate) fn try_lower_instance_method_call( // method resolved, so its arity metadata is available now. let has_rest = matches!(ctx.method_has_rest.get(&key), Some(&true)); let decl = ctx.method_param_counts.get(&key).copied().unwrap_or(0); + impl_owner.push((c == *start_cls).then(|| start_cls.clone())); implementors.push((start_cid, fname)); impl_meta.push((has_rest, decl)); } @@ -343,10 +460,13 @@ pub(crate) fn try_lower_instance_method_call( } let mut phi_inputs: Vec<(String, String)> = Vec::new(); - for (((_, fname), &case_idx), &(impl_has_rest, impl_decl_count)) in implementors - .iter() - .zip(case_idxs.iter()) - .zip(impl_meta.iter()) + for (case_no, ((((_, fname), &case_idx), &(impl_has_rest, impl_decl_count)), owner)) in + implementors + .iter() + .zip(case_idxs.iter()) + .zip(impl_meta.iter()) + .zip(impl_owner.iter()) + .enumerate() { ctx.current_block = case_idx; // #1758: a `perry_static_*` implementor is a STATIC method on a @@ -432,7 +552,17 @@ pub(crate) fn try_lower_instance_method_call( } let case_arg_slices: Vec<(crate::types::LlvmType, &str)> = case_args.iter().map(|s| (DOUBLE, s.as_str())).collect(); - ctx.block().call(DOUBLE, fname, &case_arg_slices) + match tower_pshape_route(ctx, owner.as_deref(), property, fname) { + Some(route) => emit_tower_pshape_call( + ctx, + case_no, + &route, + &recv_handle, + fname, + &case_arg_slices, + ), + None => ctx.block().call(DOUBLE, fname, &case_arg_slices), + } }; let after_label = ctx.block().label.clone(); if !ctx.block().is_terminated() { From 14e49c148c2bd5dadf812f137b7877ddda41ebdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 08:30:15 +0200 Subject: [PATCH 2/8] test(codegen): tower routing + keys-guard + profitability ratchets (#7142) --- .../collectors/proven_this_routing_tests.rs | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index e633a79e19..8ce388c5ed 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -555,6 +555,242 @@ fn proven_this_clone_binds_its_receiver_slot() { } } +// --------------------------------------------------------------------------- +// #7142: the class-id dispatch tower, routed under an INLINE keys check. +// --------------------------------------------------------------------------- + +/// `class Row { id; weight; score }` with the two methods the tower ratchets +/// need: `rescore` (four declared-field sites — the shape of +/// `benchmarks/app-patterns/kernels/batch.ts`'s hot method) and `tag` (exactly +/// ONE, which is the profitability model's break-even and must be refused). +fn row_class() -> Class { + let rescore = func( + 97, + "rescore", + vec![param(82, "factor", Type::Number)], + Type::Number, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "score".to_string(), + value: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(this_get("weight")), + right: Box::new(Expr::LocalGet(82)), + }), + right: Box::new(this_get("id")), + }), + }), + Stmt::Return(Some(this_get("score"))), + ], + ); + let tag = func( + 98, + "tag", + Vec::new(), + Type::Number, + vec![Stmt::Return(Some(this_get("id")))], + ); + class( + 103, + "Row", + vec![ + field("id", Type::Number), + field("weight", Type::Number), + field("score", Type::Number), + ], + vec![rescore, tag], + ) +} + +/// A module whose `probe(r: Shaped)` calls both `Row` methods on a receiver +/// typed as an INTERFACE. `Shaped` is not in the class registry, which is +/// exactly what `needs_dynamic_dispatch` keys on — so both calls lower to the +/// `idispatch.*` class-id switch tower rather than to either of the two +/// guard-dominated routing sites. This is `batch.ts`'s `rows.map((r) => …)` +/// shape reduced to its essentials. +fn tower_site_module() -> Module { + let mut m = Module::new("pshape_tower.ts"); + m.classes = vec![row_class()]; + m.functions = vec![func( + 1, + "probe", + vec![param(2, "r", Type::Named("Shaped".to_string()))], + Type::Number, + vec![Stmt::Return(Some(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(call( + Expr::LocalGet(2), + "rescore", + vec![Expr::Number(1.5)], + )), + right: Box::new(call(Expr::LocalGet(2), "tag", Vec::new())), + }))], + )]; + m.init_kind = ModuleInitKind::Eager; + m +} + +/// Split rendered IR into `(label, body_lines)` — block labels render +/// unindented and colon-terminated, every instruction is indented. +fn blocks(ir: &str) -> Vec<(String, Vec<&str>)> { + let mut out: Vec<(String, Vec<&str>)> = Vec::new(); + for line in ir.lines() { + if !line.starts_with(char::is_whitespace) + && line.ends_with(':') + && !line.starts_with("define") + { + out.push((line.trim_end_matches(':').to_string(), Vec::new())); + } else if let Some(last) = out.last_mut() { + last.1.push(line); + } + } + out +} + +/// The block that contains a `call` to `name`. +fn block_calling<'a>(bs: &'a [(String, Vec<&'a str>)], name: &str) -> Option<&'a (String, Vec<&'a str>)> { + let needle = format!("@{}(", name); + bs.iter() + .find(|(_, body)| body.iter().any(|l| l.contains("call ") && l.contains(&needle))) +} + +/// Regression (#7142): the class-id dispatch tower routes its case to the +/// proven-`this` clone. +/// +/// This is the site #7141 deliberately left alone: `batch.ts`'s receiver has no +/// static class, so neither existing routing site is ever reached and the clone +/// stayed dead on the one workload `Ptr` exists for. +#[test] +fn tower_case_routes_to_proven_this_clone() { + let ir = emit(&tower_site_module(), false); + // Anti-vacuity: this must really be the class-id tower, not one of the two + // guard-dominated sites that already routed before this change. + assert!( + ir.contains("idispatch.case"), + "the fixture no longer lowers through the class-id dispatch tower, so \ + this test would pass for the wrong reason:\n{ir}" + ); + let defs = pshape_definitions(&ir); + let calls = pshape_call_targets(&ir); + let rescore = defs + .iter() + .find(|d| d.contains("__rescore__pshape")) + .unwrap_or_else(|| panic!("no proven-`this` clone emitted for `rescore`: {defs:?}")); + assert!( + calls.iter().any(|c| c == rescore), + "the dispatch tower's case proves the receiver's class_id; under an \ + inline keys check it must route to the proven-`this` clone instead of \ + the guard-ridden public body. defined={defs:?} called={calls:?}" + ); +} + +/// Soundness ratchet (#7142): the routed call is dominated by a compare of the +/// receiver's live `keys_array` against the class's `@perry_class_keys_*` token. +/// +/// A `class_id` match alone is NOT a layout proof — `delete inst.f` compacts +/// the packed slots while preserving `class_id` +/// (`object/delete_rest.rs`), which is why #7141 refused to route this site at +/// all. The compare below is the entire difference between sound and unsound, +/// so it is traced end to end: global → entry-hoisted slot → reload in the +/// guard block → `icmp eq i64` → the branch that enters the clone's block. +#[test] +fn tower_route_is_guarded_by_the_class_keys_token() { + let ir = emit(&tower_site_module(), false); + let bs = blocks(&ir); + let clone = pshape_definitions(&ir) + .into_iter() + .find(|d| d.contains("__rescore__pshape")) + .expect("no proven-`this` clone for `rescore`"); + let (clone_block, _) = block_calling(&bs, &clone) + .unwrap_or_else(|| panic!("the clone is never called — nothing to guard:\n{ir}")); + + // The block whose terminator enters the clone's block. + let (_, guard_body) = bs + .iter() + .find(|(_, body)| { + body.iter() + .any(|l| l.starts_with(" br i1 ") && l.contains(&format!("label %{}", clone_block))) + }) + .unwrap_or_else(|| { + panic!("nothing conditionally branches to {clone_block} — the clone is reached unguarded:\n{ir}") + }); + + // 1. the class keys global is loaded once at function entry … + let global_load = ir + .lines() + .find(|l| l.contains("= load i64, ptr @perry_class_keys_")) + .unwrap_or_else(|| panic!("the class keys token is never read:\n{ir}")); + let global_reg = global_load.trim().split(' ').next().expect("ssa name"); + // 2. … and parked in an entry slot … + let store = ir + .lines() + .find(|l| l.contains(&format!("store i64 {}, ptr ", global_reg))) + .unwrap_or_else(|| panic!("the hoisted keys token is never stored:\n{ir}")); + let slot = store.rsplit(' ').next().expect("slot name"); + // 3. … which the guard block reloads … + let expected = guard_body + .iter() + .find_map(|l| { + let l = l.trim(); + l.ends_with(&format!("load i64, ptr {}", slot)) + .then(|| l.split(' ').next().expect("ssa name").to_string()) + }) + .unwrap_or_else(|| { + panic!("the guard block never reads the hoisted keys token:\n{guard_body:#?}") + }); + // 4. … and compares against the receiver's live keys_array. + assert!( + guard_body + .iter() + .any(|l| l.contains("icmp eq i64") && l.contains(&expected)), + "the routed call is not dominated by a keys-token compare — a class_id \ + match alone does not prove the packed layout (`delete` compacts slots \ + while preserving class_id):\n{guard_body:#?}" + ); + // The sticky prototype-descriptor / tracing latch the per-access inline + // guard reads must be honoured too, or the route would be weaker than the + // lowering it replaces. + assert!( + guard_body + .iter() + .any(|l| l.contains("@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED")), + "the routed call must also honour the sticky inline-guard latch:\n{guard_body:#?}" + ); +} + +/// Profitability ratchet (#7142): a clone that deletes exactly ONE guarded +/// field site does not earn the tower's inline re-check, so the tower keeps +/// calling the public body. +/// +/// The re-check IS one instance of the same header check the body would have +/// run at that single site, so routing there swaps a check for a check and adds +/// a second call site for nothing. The clone is still emitted — the two +/// guard-dominated routing sites pay no extra proof and take it happily. +#[test] +fn tower_route_refused_when_clone_deletes_one_field_site() { + let ir = emit(&tower_site_module(), false); + let defs = pshape_definitions(&ir); + let calls = pshape_call_targets(&ir); + let tag = defs + .iter() + .find(|d| d.contains("__tag__pshape")) + .unwrap_or_else(|| { + panic!( + "`tag` admits a proven-`this` clone (it reads a declared field), \ + so the refusal below would be vacuous without one: {defs:?}" + ) + }); + assert!( + !calls.iter().any(|c| c == tag), + "`tag` has a single guarded field site, so routing the tower to its \ + clone trades one shape check for one shape check plus a branch and a \ + second call site. It must be refused. defined={defs:?} called={calls:?}" + ); +} + // NOTE on the `PERRY_PTR_SHAPE_LOCALS=0` direction: it is deliberately NOT a // test in this file. `ptr_shape_locals_enabled` memoises in a `OnceLock`, so a // test that sets the variable in-process observes whatever the first reader in From 5c05334bf2c0a39ebff1383ec26acbc1a1ae71e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 08:32:52 +0200 Subject: [PATCH 3/8] test(gap): cross-module delete reproducer for the tower keys guard (#7142) --- .../collectors/proven_this_routing_tests.rs | 17 +++---- .../_helpers/pshape_tower_delete_rows.ts | 38 ++++++++++++++++ ...test_gap_pshape_tower_delete_keys_guard.ts | 44 +++++++++++++++++++ 3 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 test-files/_helpers/pshape_tower_delete_rows.ts create mode 100644 test-files/test_gap_pshape_tower_delete_keys_guard.ts diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index 8ce388c5ed..ac802598d8 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -621,11 +621,7 @@ fn tower_site_module() -> Module { Type::Number, vec![Stmt::Return(Some(Expr::Binary { op: BinaryOp::Add, - left: Box::new(call( - Expr::LocalGet(2), - "rescore", - vec![Expr::Number(1.5)], - )), + left: Box::new(call(Expr::LocalGet(2), "rescore", vec![Expr::Number(1.5)])), right: Box::new(call(Expr::LocalGet(2), "tag", Vec::new())), }))], )]; @@ -651,10 +647,15 @@ fn blocks(ir: &str) -> Vec<(String, Vec<&str>)> { } /// The block that contains a `call` to `name`. -fn block_calling<'a>(bs: &'a [(String, Vec<&'a str>)], name: &str) -> Option<&'a (String, Vec<&'a str>)> { +fn block_calling<'a>( + bs: &'a [(String, Vec<&'a str>)], + name: &str, +) -> Option<&'a (String, Vec<&'a str>)> { let needle = format!("@{}(", name); - bs.iter() - .find(|(_, body)| body.iter().any(|l| l.contains("call ") && l.contains(&needle))) + bs.iter().find(|(_, body)| { + body.iter() + .any(|l| l.contains("call ") && l.contains(&needle)) + }) } /// Regression (#7142): the class-id dispatch tower routes its case to the diff --git a/test-files/_helpers/pshape_tower_delete_rows.ts b/test-files/_helpers/pshape_tower_delete_rows.ts new file mode 100644 index 0000000000..f90898822d --- /dev/null +++ b/test-files/_helpers/pshape_tower_delete_rows.ts @@ -0,0 +1,38 @@ +// Helper for `test_gap_pshape_tower_delete_keys_guard.ts` (#7142). +// +// This module deliberately contains NO `delete` (and no other §5.2 shape +// barrier). That is the point: the barrier that stands representation-selection +// down is collected per MODULE (`collect_module_dispatch_facts`), so the +// routing decision made here is taken with no knowledge of the `delete` the +// importing module performs on these very instances (#7143). Only the inline +// keys check emitted at the call site can catch it. +// +// Keep `pick()` reading slot 0 and slot 2 with slot 1 skipped: deleting `b` +// compacts `c` from slot 2 down into slot 1, so a routing decision made on +// `class_id` alone reads a slot that is now `undefined`. + +export class Row { + a: number; + b: number; + c: number; + + constructor(a: number, b: number, c: number) { + this.a = a; + this.b = b; + this.c = c; + } + + // Two declared-field sites, which is what makes routing the dispatch tower + // to the proven-`this` clone profitable at all. + pick(): number { + return this.a * 100 + this.c; + } +} + +// `r` inside the `map` callback has no static class, so `r.pick()` lowers to +// the class-id switch tower (`idispatch.*`) rather than to either of the two +// guard-dominated routing sites. Same shape as +// `benchmarks/app-patterns/kernels/batch.ts`'s `rows.map((r) => r.rescore(1.5))`. +export function pickAll(rows: Row[]): number[] { + return rows.map((r) => r.pick()); +} diff --git a/test-files/test_gap_pshape_tower_delete_keys_guard.ts b/test-files/test_gap_pshape_tower_delete_keys_guard.ts new file mode 100644 index 0000000000..369766dfd3 --- /dev/null +++ b/test-files/test_gap_pshape_tower_delete_keys_guard.ts @@ -0,0 +1,44 @@ +// Issue #7142: routing the class-id dispatch tower to a proven-`this` clone is +// sound ONLY behind an inline keys check. +// +// `js_object_delete_field` compacts an object's packed inline slots while +// PRESERVING `class_id` (`object/delete_rest.rs`): on `class Row { a; b; c }`, +// `delete row.b` moves `c` from slot 2 into slot 1 and shortens `field_count`. +// So a dispatch-tower case that matched `class_id` has NOT proved the layout, +// and the clone's bare fixed-slot loads would read the wrong slot. The +// compaction installs a freshly CLONED keys array, which is what the inline +// pointer compare against `@perry_class_keys_*` catches — the receiver falls to +// the generic by-name path and reads the right values. +// +// The `delete` lives in THIS module while the class and its dispatcher live in +// `_helpers/pshape_tower_delete_rows.ts`, on purpose: the `delete` shape +// barrier that stands the analysis down is module-scoped while the receivers +// alias across modules (#7143). A same-module `delete` would disable the clone +// entirely and this test would pass vacuously. +// +// With a class-id-only route this prints `103,NaN,309,412`. Compared +// byte-for-byte against `node --experimental-strip-types`. + +import { Row, pickAll } from "./_helpers/pshape_tower_delete_rows.ts"; + +const rows: Row[] = []; +for (let i = 0; i < 4; i++) { + rows.push(new Row(i + 1, (i + 1) * 10, (i + 1) * 3)); +} + +// Baseline: every row still carries the class's canonical keys array. +console.log("before:", pickAll(rows).join(",")); + +// The compaction `class_id` cannot see. +delete (rows[1] as any).b; + +console.log("after:", pickAll(rows).join(",")); +console.log("b:", (rows[1] as any).b); +console.log("c:", rows[1].c); +console.log("keys:", Object.keys(rows[1] as any).join("|")); + +// A second delete on a different row, to show the check is per-receiver rather +// than a process-wide latch: rows 0, 2 and 3 keep answering from the clone. +delete (rows[3] as any).a; +console.log("after2:", pickAll(rows).join(",")); +console.log("keys3:", Object.keys(rows[3] as any).join("|")); From d03a78ba1b8bc31c0b6f7832f1855fe9351dc2c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:12:21 +0200 Subject: [PATCH 4/8] test(gap): register the tower keys-guard reproducer in the GC x repsel corpus (#7142) --- ...te_rows.ts => repsel_pshape_tower_rows.ts} | 2 +- ...=> test_gap_repsel_pshape_tower_delete.ts} | 28 ++++++++++++++++--- test-parity/gc_repsel_corpus.txt | 11 ++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) rename test-files/_helpers/{pshape_tower_delete_rows.ts => repsel_pshape_tower_rows.ts} (95%) rename test-files/{test_gap_pshape_tower_delete_keys_guard.ts => test_gap_repsel_pshape_tower_delete.ts} (63%) diff --git a/test-files/_helpers/pshape_tower_delete_rows.ts b/test-files/_helpers/repsel_pshape_tower_rows.ts similarity index 95% rename from test-files/_helpers/pshape_tower_delete_rows.ts rename to test-files/_helpers/repsel_pshape_tower_rows.ts index f90898822d..4490892cfb 100644 --- a/test-files/_helpers/pshape_tower_delete_rows.ts +++ b/test-files/_helpers/repsel_pshape_tower_rows.ts @@ -1,4 +1,4 @@ -// Helper for `test_gap_pshape_tower_delete_keys_guard.ts` (#7142). +// Helper for `test_gap_repsel_pshape_tower_delete.ts` (#7142). // // This module deliberately contains NO `delete` (and no other §5.2 shape // barrier). That is the point: the barrier that stands representation-selection diff --git a/test-files/test_gap_pshape_tower_delete_keys_guard.ts b/test-files/test_gap_repsel_pshape_tower_delete.ts similarity index 63% rename from test-files/test_gap_pshape_tower_delete_keys_guard.ts rename to test-files/test_gap_repsel_pshape_tower_delete.ts index 369766dfd3..3c28caabf2 100644 --- a/test-files/test_gap_pshape_tower_delete_keys_guard.ts +++ b/test-files/test_gap_repsel_pshape_tower_delete.ts @@ -11,15 +11,32 @@ // the generic by-name path and reads the right values. // // The `delete` lives in THIS module while the class and its dispatcher live in -// `_helpers/pshape_tower_delete_rows.ts`, on purpose: the `delete` shape +// `_helpers/repsel_pshape_tower_rows.ts`, on purpose: the `delete` shape // barrier that stands the analysis down is module-scoped while the receivers // alias across modules (#7143). A same-module `delete` would disable the clone // entirely and this test would pass vacuously. // -// With a class-id-only route this prints `103,NaN,309,412`. Compared +// With a class-id-only route this prints `after: 103,NaN,309,412`. Compared // byte-for-byte against `node --experimental-strip-types`. +// +// GC arms: `churn()` allocates well past the matrix's `--pressure 8` +// (`PERRY_GC_HEAP_LIMIT=8`) heap limit while `rows` stays live across it, so an +// evacuating arm has both a reason to collect and a receiver to move. The +// routed call re-derives its receiver from the NaN-boxed argument and the clone +// shadow-binds its own receiver slot (#6925/#6990), which is what has to +// survive the move. + +import { Row, pickAll } from "./_helpers/repsel_pshape_tower_rows.ts"; -import { Row, pickAll } from "./_helpers/pshape_tower_delete_rows.ts"; +function churn(n: number): number { + // Escaping allocations: each object is pushed, so nothing is scalar + // replaced and the nursery genuinely fills. + const sink: { x: number }[] = []; + for (let i = 0; i < n; i++) { + sink.push({ x: i }); + } + return sink.length; +} const rows: Row[] = []; for (let i = 0; i < 4; i++) { @@ -29,6 +46,8 @@ for (let i = 0; i < 4; i++) { // Baseline: every row still carries the class's canonical keys array. console.log("before:", pickAll(rows).join(",")); +console.log("churn:", churn(200_000)); + // The compaction `class_id` cannot see. delete (rows[1] as any).b; @@ -38,7 +57,8 @@ console.log("c:", rows[1].c); console.log("keys:", Object.keys(rows[1] as any).join("|")); // A second delete on a different row, to show the check is per-receiver rather -// than a process-wide latch: rows 0, 2 and 3 keep answering from the clone. +// than a process-wide latch: rows 0 and 2 keep answering from the clone. delete (rows[3] as any).a; +console.log("churn2:", churn(200_000)); console.log("after2:", pickAll(rows).join(",")); console.log("keys3:", Object.keys(rows[3] as any).join("|")); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 3b754996fb..9625f297f1 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -75,6 +75,17 @@ test_gap_repsel_p4b_field_store_elision # was failing on main for every PR until this line landed. test_gap_repsel_proven_this_frozen +# --- Phase 5a routing: class-id dispatch tower -> proven `this` (#7142) ------ +# The tower case proves `class_id`, which `delete` preserves while compacting +# the packed slots, so the route carries an INLINE keys-token check. Registered +# because the routed path has its own GC contract: the receiver is re-derived +# from the NaN-boxed argument at the call site and the clone shadow-binds its +# own receiver slot (#6925/#6990 -- `GC_TYPE_OBJECT` is movable, the `TaPtr` +# no-bind shortcut does not transfer). `churn()` allocates past the matrix's +# `--pressure 8` heap limit with `rows` live across it, so the evacuating arms +# have a receiver to move rather than reporting the file inert. +test_gap_repsel_pshape_tower_delete + # --- Typed-array constructor source rooting (#6981) -------------------------- # Not a representation file, so the UNREGISTERED gate does not auto-detect it: # registered explicitly per the header rule above. Gates the runtime-side half From eaed4d8547f1f0dc6bf5c3082e1dfd73ad0c5abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:14:04 +0200 Subject: [PATCH 5/8] docs(changelog): tower->pshape keys-guarded routing fragment (#7169) --- changelog.d/7169-tower-pshape-keys-guard.md | 80 +++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 changelog.d/7169-tower-pshape-keys-guard.md diff --git a/changelog.d/7169-tower-pshape-keys-guard.md b/changelog.d/7169-tower-pshape-keys-guard.md new file mode 100644 index 0000000000..b0047c807b --- /dev/null +++ b/changelog.d/7169-tower-pshape-keys-guard.md @@ -0,0 +1,80 @@ +### perf(codegen): route the class-id dispatch tower to the proven-`this` clone behind an inline keys check (#7142) + +`benchmarks/app-patterns/kernels/batch.ts` — the workload `Ptr` exists +for — consumed nothing from representation-selection Phase 5a. Its hot call is +`r.rescore(1.5)` inside `rows.map((r) => …)`, whose closure parameter has no +static class, so `receiver_class_name()` returns `None`, neither of the two +routing sites #7141 fixed is ever reached, and the call lowers to the class-id +switch tower in `lower_call/property_get/dynamic_dispatch.rs`. + +#7141 refused to route that tower because a `class_id` match is not a layout +proof: `delete inst.f` compacts the packed inline slots while **preserving** +`class_id` (`object/delete_rest.rs`), so on `class Row { a; b; c }`, +`delete row.b` moves `c` from slot 2 into slot 1 and the clone's bare +fixed-offset loads would read the wrong slot. + +The compaction installs a freshly **cloned** keys array, so an inline pointer +compare against the class's `@perry_class_keys_*` token catches it exactly. +That check is now emitted at `idispatch.caseN` — one basic block, 21 IR +instructions, no calls: four loads off the receiver (three from header words the +tower's own `js_object_get_class_id` already touched), one entry-hoisted token +reload, nine ALU ops, one branch. The dereference needs no gate/deref split +because a non-zero class-id match already rejects the handle band, the +Set/Map/RegExp registries, out-of-heap addresses and non-`GC_TYPE_OBJECT` +allocations. + +The check is **dynamic on purpose**. The `delete` shape barrier that stands the +whole analysis down is collected per module while receivers alias across modules +(#7143), so at this site a static proof would be the wrong instrument — which is +also why the two pre-existing routing sites were safe and this one was not. + +Beyond the keys token the check carries the sticky +`@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch (prototype-level descriptors, +tracing mode), the per-object `OBJ_FLAG_HAS_DESCRIPTORS` bit (instance-level +installs deliberately do not flip the process-global latch, #5654), +`OBJ_FLAG_FROZEN` (a proven-`this` clone may contain field writes, and Phase 5a +rules a frozen receiver out only through a module-scoped kill — this makes the +tower route strictly stronger than `js_method_direct_shape_guard`), plus +not-forwarded and `object_type == OBJECT_TYPE_REGULAR`. Routing is restricted to +cases whose receiver class **declares** the method, so the clone's `this` is +exactly the class it was compiled for. + +Unlike the two guard-dominated sites, this one pays for its own proof, so it +consults a profitability gate (`collectors/repsel_benefit.rs`, the module #7132 +added) instead of routing unconditionally: the re-check is one instance of the +same header check the public body runs at every `this.field`, so the trade is +*N in-body checks for 1 at the call site* and the break-even is exactly `N == 1`. +It routes at `N >= 2`. The rule is a count with no target-specific term. + +Measured on `batch.ts` (perry-dev, darwin-arm64), the two bodies the tower now +chooses between: + +| body | IR lines | class-field guards | by-name field ops | total `js_*` call sites | +|---|---|---|---|---| +| `…__Row__rescore` (public) | 352 | 3 | 3 | 15 | +| `…__Row__rescore__pshape` | 189 | 0 | 0 | 6 | + +`…__rescore__pshape` goes from **0 call sites to 1** — the module-wide static +totals are deliberately unchanged, because the public body survives as the miss +arm and as the registered vtable symbol, which is why the A/B is reported at the +call site rather than as a module count. + +`test-files/test_gap_repsel_pshape_tower_delete.ts` is the soundness test, built +red-first: construct → `delete` a field through a cross-module alias → call the +method. Against a class-id-only route it prints `after: 103,NaN,309,412` where +Node 26.5.1 prints `103,206,309,412`; with the keys check the whole file is +byte-identical to the oracle. Registered in `test-parity/gc_repsel_corpus.txt` +and GC-live by construction (`copied_objects=5971` at `--pressure 8`), so the +evacuating arms have a receiver to move rather than reporting the file inert. + +Three call-site ratchets in `collectors/proven_this_routing_tests.rs` with +disjoint red sets, verified by building each sabotage arm: reverting the routing +reddens only the routing test, removing the keys compare reddens only the +keys-guard test (which traces the token global → entry slot → guard-block reload +→ `icmp eq i64` → the branch into the clone's block), and removing the +profitability gate reddens only the refusal test. + +Unchanged `(double this, args…)` ABI; the clone still stores its receiver to a +slot and `js_shadow_slot_bind`s it with no safepoint between (#6925/#6990 — +`GC_TYPE_OBJECT` is movable in the shipped configuration, so the `TaPtr` no-bind +shortcut does not transfer). From 574be38598843a6f7dc56300391642aa3a25d041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:24:52 +0200 Subject: [PATCH 6/8] test(codegen): unit-test the tower routing profitability threshold (#7142) --- .../src/collectors/repsel_benefit.rs | 5 +- .../src/collectors/repsel_benefit/tests.rs | 108 ++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index 251350730d..b7b6ef1c4b 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -494,10 +494,7 @@ pub(crate) fn collect_unprofitable_canonical_i32_locals( /// * a site inside a loop is counted once, though it is paid per iteration; /// * `this.other()` calls that the clone also lowers guard-free (the /// `ThisFlowAnalysis` walk vets them transitively) are not followed. -pub(crate) fn proven_receiver_clone_field_sites( - method: &Function, - chain_fields: &HashSet, -) -> u32 { +fn proven_receiver_clone_field_sites(method: &Function, chain_fields: &HashSet) -> u32 { let mut sites = 0u32; super::scalar_method_dispatch::for_each_expr_in_stmts(&method.body, &mut |e| { let named = match e { diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index 8cc9975a1d..6e9cd0877a 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -578,3 +578,111 @@ fn unmodelled_forms_are_neutral() { let out = run(&stmts, &HashSet::new()); assert!(!out.contains(&1), "typeof-read counter refused: {out:?}"); } + +// --------------------------------------------------------------------------- +// #7142: the dispatch-tower routing threshold. +// --------------------------------------------------------------------------- + +fn this_get(field: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::This), + property: field.to_string(), + byte_offset: 0, + } +} + +fn method(body: Vec) -> perry_hir::Function { + perry_hir::Function { + id: 1, + name: "m".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: HirType::Number, + body, + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn declared(names: &[&str]) -> HashSet { + names.iter().map(|s| (*s).to_string()).collect() +} + +/// One declared-field site is the break-even, not a win: the route swaps the +/// body's single inline shape check for one at the call site and adds a branch +/// plus a second call site on top. Two is the first count that deletes work. +#[test] +fn tower_route_break_even_is_one_field_site() { + let fields = declared(&["a", "c"]); + + let one = method(vec![Stmt::Return(Some(this_get("a")))]); + assert_eq!(proven_receiver_clone_field_sites(&one, &fields), 1); + assert!(!tower_route_profitable(&one, &fields)); + + let two = method(vec![Stmt::Return(Some(bin( + BinaryOp::Add, + this_get("a"), + this_get("c"), + )))]); + assert_eq!(proven_receiver_clone_field_sites(&two, &fields), 2); + assert!(tower_route_profitable(&two, &fields)); +} + +/// Only DECLARED chain fields get a fixed slot; anything else keeps its by-name +/// lowering inside the clone too, so it is not work the route deletes and must +/// not be counted toward the threshold. +#[test] +fn tower_route_counts_only_declared_fields() { + let fields = declared(&["a"]); + let m = method(vec![ + Stmt::Expr(this_get("a")), + Stmt::Expr(this_get("expando")), + Stmt::Expr(this_get("alsoNotDeclared")), + ]); + assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 1); + assert!(!tower_route_profitable(&m, &fields)); +} + +/// A field access on something that is not `this` is somebody else's receiver +/// and is not affected by the clone at all. +#[test] +fn tower_route_ignores_non_this_receivers() { + let fields = declared(&["a"]); + let m = method(vec![ + Stmt::Expr(this_get("a")), + Stmt::Expr(Expr::PropertyGet { + object: Box::new(get(9)), + property: "a".to_string(), + byte_offset: 0, + }), + ]); + assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 1); +} + +/// Writes count as well as reads — a `this.f = …` site pays the same inline +/// check (plus the frozen/plain-value conjuncts) in the public body. +#[test] +fn tower_route_counts_writes() { + let fields = declared(&["a", "b"]); + let m = method(vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "a".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + Stmt::Expr(Expr::PropertyUpdate { + object: Box::new(Expr::This), + property: "b".to_string(), + op: BinaryOp::Add, + prefix: false, + }), + ]); + assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 2); + assert!(tower_route_profitable(&m, &fields)); +} From 5aae37a7c627924ccfcd9459e4294eb50eea5f33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:26:44 +0200 Subject: [PATCH 7/8] docs(codegen): record the tower route's GC ordering invariant (#7142) --- .../lower_call/property_get/dynamic_dispatch.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index b27301671e..35197f21d4 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -78,6 +78,20 @@ fn tower_pshape_route( /// whole analysis down is module-scoped while a receiver can be deleted from /// through an alias in another module (#7143), so a static proof would be /// exactly the wrong instrument here. +/// +/// GC ordering invariant: `recv_handle` is the raw pointer masked out of the +/// receiver in `idispatch.tower`, and the header loads below dereference it. +/// That is only safe because **nothing between the mask and this point is a +/// safepoint**: the only allocating thing a case block can emit before the call +/// is the rest-array bundling (`js_array_alloc` / `js_array_push_f64`), and a +/// rest-bearing method can never reach here — `collectors/proven_this.rs` +/// rejects any method with a rest or synthesized-`arguments` parameter, so no +/// clone exists for one and `tower_pshape_route` returns `None`. The non-rest +/// preamble emits no instructions at all (already-lowered SSA values plus +/// `undefined` literals). If a future change makes the case preamble allocate, +/// this dereference must move above it — the `GC_FLAG_FORWARDED` conjunct would +/// degrade a moved receiver to the generic path rather than misread it, but +/// relying on that instead of on the ordering would be luck, not a proof. fn emit_tower_pshape_call( ctx: &mut FnCtx<'_>, case_no: usize, From 5b3571aee5d550dbee92b2e8dd5a65a12c5ee325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:31:23 +0200 Subject: [PATCH 8/8] docs(changelog): record the measured instruction delta (#7169) --- changelog.d/7169-tower-pshape-keys-guard.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/changelog.d/7169-tower-pshape-keys-guard.md b/changelog.d/7169-tower-pshape-keys-guard.md index b0047c807b..605fea59b2 100644 --- a/changelog.d/7169-tower-pshape-keys-guard.md +++ b/changelog.d/7169-tower-pshape-keys-guard.md @@ -59,6 +59,18 @@ totals are deliberately unchanged, because the public body survives as the miss arm and as the registered vtable symbol, which is why the A/B is reported at the call site rather than as a module count. +Timed on a quiet Raspberry Pi 5 with `perf stat`, two compilers from the same +tree (arm `before` has the tower route forced off, nothing else), ASLR disabled, +pinned to one core, interleaved, 12 reps each. The workload is bimodal at ~1% +independently of the arm (3/12 runs per arm in the low mode; zero GC collections +in both, so it is not a GC-schedule effect), so the delta is reported per mode: +**−0.156%** in the low mode and **−0.168%** in the high mode, overall median +−0.17%. That is ≈6.0M instructions over 40,000 `rescore` calls, ≈150 +instructions per call; the whole-program figure is small because `rescore` is a +small share of `batch.ts`, which is dominated by allocation, `sort` and +`reduce`. The fixture is there because its *receiver shape* is the hard case, +not because its method is the hot spot. + `test-files/test_gap_repsel_pshape_tower_delete.ts` is the soundness test, built red-first: construct → `delete` a field through a cross-module alias → call the method. Against a class-id-only route it prints `after: 103,NaN,309,412` where