From efbf63cf0159739d6fc0ced5fe7cdd92b84caeaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:04:07 +0200 Subject: [PATCH 1/6] perf(codegen): route typed-clone fallbacks to the Phase 5a proven-`this` clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `{method}__pshape` clone emitted by representation-selection Phase 5a had zero call sites across the whole measured corpus: every clone was emitted, reachable from nothing, and dead-stripped by the linker. Root cause is arm ordering at both routing sites. emit_guarded_direct_method_call tried five typed-clone arms and consulted pshape_methods only in the final `else`; the Phase 3b guard-free site had the same shape, routing to the clone on its plain exit while its typed-receiver arm's generic fallback called the guard-ridden public body 25 lines above. A method can only admit a proven-`this` clone if it touches a declared field of its own chain, which is very nearly the definition of a typed-receiver-clone candidate — so the typed arm won whenever both were eligible. Both sites now resolve the clone once, up front, and use it for the generic fallback as well as the plain exit. Every rerouted block is dominated either by the class-id + keys-token guard or by Phase 3b containment, which is exactly what the existing `else` arm already relied on — no new proof obligation, unchanged ABI, unchanged shadow-bound receiver slot. --- crates/perry-codegen/src/collectors/mod.rs | 2 + .../src/collectors/proven_this.rs | 3 +- .../collectors/proven_this_routing_tests.rs | 395 ++++++++++++++++++ .../src/lower_call/method_override.rs | 61 ++- .../property_get/dynamic_dispatch.rs | 40 +- 5 files changed, 469 insertions(+), 32 deletions(-) create mode 100644 crates/perry-codegen/src/collectors/proven_this_routing_tests.rs diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 6ca59f5339..79183e2f4e 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -25,6 +25,8 @@ mod mutation; mod not_bigint_locals; mod pointer_locals; mod proven_this; +#[cfg(test)] +mod proven_this_routing_tests; mod ptr_numarray; mod ptr_shape; mod ptr_shape_report; diff --git a/crates/perry-codegen/src/collectors/proven_this.rs b/crates/perry-codegen/src/collectors/proven_this.rs index f817a52f88..fdcd3b3dc7 100644 --- a/crates/perry-codegen/src/collectors/proven_this.rs +++ b/crates/perry-codegen/src/collectors/proven_this.rs @@ -337,8 +337,9 @@ mod tests { // Naming + emission + the two proven call sites. `string_pool.rs` // (which emits `js_register_class_method`) is deliberately ABSENT: // the vtable must only ever hold the public symbol. - let allowed: [&str; 6] = [ + let allowed: [&str; 7] = [ "collectors/proven_this.rs", // this test + "collectors/proven_this_routing_tests.rs", // routing IR ratchet "codegen/typed_abi.rs", // name helper "codegen/method.rs", // clone emission "codegen/artifacts.rs", // emission driver diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs new file mode 100644 index 0000000000..88873042bf --- /dev/null +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -0,0 +1,395 @@ +//! Phase 5a proven-`this` **call-site** ratchets (#7128). +//! +//! `collectors/proven_this.rs` decides *whether* a `{public}__pshape` clone may +//! be emitted; `codegen/artifacts.rs` emits it. Neither of those is worth +//! anything unless a call site actually *targets* the clone — and for the whole +//! corpus measured in #7128, none did: every clone was emitted, reachable from +//! nothing, and dead-stripped by the linker. +//! +//! The failure was invisible to the two checks that were in place. An object +//! hash A/B scores the phase as working (`suite_09_method_calls`' object DOES +//! differ with the analysis off — by two dead clone bodies), and a promotion +//! counter scores it as working (the clone's `this` is a genuine `Ptr` +//! consumption, recorded at every `this.field` site *inside the dead body*). +//! Only reading the emitted IR for a `call` whose callee is the clone +//! distinguishes "routed" from "emitted and abandoned". +//! +//! So these tests assert on **call sites**, never on symbol presence alone: +//! every one of them requires `define`+`call` together, and +//! [`pshape_call_targets`] deliberately matches the callee position so a +//! `ptrtoint ptr @…__pshape` (the shape the typed-feedback guard passes a +//! function pointer in) can never be miscounted as a call. +//! +//! Both routing sites are covered: +//! +//! * `lower_call/method_override.rs` — the guarded `method_direct.fast` arm, +//! dominated by the class-id + keys-token guard. +//! * `lower_call/property_get/dynamic_dispatch.rs` — the Phase 3b guard-free +//! `Ptr` receiver arm. +//! +//! and in both, the case that regressed is the *typed-clone fallback*: when a +//! typed clone exists for the method, the typed arm ran first and its own +//! generic fallback called the guard-ridden public body, discarding a receiver +//! proof the enclosing block had already established. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{BinaryOp, Class, ClassField, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +fn ir_opts(is_entry: bool) -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: is_entry, + non_entry_module_prefixes: Vec::new(), + nextjs_path_init_modules: Vec::new(), + import_function_prefixes: std::collections::HashMap::new(), + import_function_ffi_aliases: std::collections::HashMap::new(), + import_function_origin_names: std::collections::HashMap::new(), + import_function_v8_specifiers: std::collections::HashMap::new(), + import_function_node_submodule: std::collections::HashMap::new(), + namespace_node_submodules: std::collections::HashMap::new(), + namespace_v8_specifiers: std::collections::HashMap::new(), + namespace_member_prefixes: std::collections::HashMap::new(), + namespace_member_origin_names: std::collections::HashMap::new(), + emit_ir_only: true, + verify_native_regions: false, + disable_buffer_fast_path: false, + namespace_imports: Vec::new(), + imported_classes: Vec::new(), + imported_enums: Vec::new(), + imported_async_funcs: std::collections::HashSet::new(), + type_aliases: std::collections::HashMap::new(), + imported_func_param_counts: std::collections::HashMap::new(), + imported_func_has_rest: std::collections::HashSet::new(), + imported_func_synthetic_arguments: std::collections::HashSet::new(), + imported_func_return_types: std::collections::HashMap::new(), + imported_vars: std::collections::HashSet::new(), + output_type: "executable".to_string(), + needs_stdlib: false, + needs_ui: false, + needs_geisterhand: false, + geisterhand_port: 7676, + enabled_features: Vec::new(), + native_module_init_names: Vec::new(), + js_module_specifiers: Vec::new(), + bundled_extensions: Vec::new(), + native_library_functions: Vec::new(), + i18n_table: None, + fast_math: false, + fp_contract_mode: crate::FpContractMode::Off, + app_metadata: AppMetadata::default(), + namespace_entries: Vec::new(), + dynamic_import_path_to_prefix: std::collections::HashMap::new(), + deferred_module_prefixes: std::collections::HashSet::new(), + module_init_deps: Vec::new(), + is_dynamic_import_target: false, + debug_locations: false, + module_source: None, + debug_source_line_offset: 0, + } +} + +fn field(name: &str, ty: Type) -> ClassField { + ClassField { + name: name.to_string(), + key_expr: None, + ty, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + } +} + +fn param(id: u32, name: &str, ty: Type) -> Param { + Param { + id, + name: name.to_string(), + ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +fn func(id: u32, name: &str, params: Vec, return_type: Type, body: Vec) -> Function { + Function { + id, + name: name.to_string(), + type_params: Vec::new(), + params, + return_type, + 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 class(id: u32, name: &str, fields: Vec, methods: Vec) -> Class { + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields, + constructor: None, + methods, + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + computed_members: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + } +} + +fn this_get(f: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::This), + property: f.to_string(), + byte_offset: 0, + } +} + +fn call(recv: Expr, method: &str, args: Vec) -> Expr { + Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(recv), + property: method.to_string(), + byte_offset: 0, + }), + args, + type_args: Vec::new(), + byte_offset: 0, + } +} + +/// Every LLVM callee named `…__pshape`. +/// +/// Matches the **callee position** of a `call` instruction specifically. A +/// `__pshape` symbol also appears in `define` lines and could appear in a +/// `ptrtoint ptr @… to i64` operand (how the typed-feedback guard receives a +/// function pointer); counting either as a call site is exactly the vacuous +/// pass this file exists to prevent. +fn pshape_call_targets(ir: &str) -> Vec { + let mut out = Vec::new(); + for line in ir.lines() { + let Some(call_at) = line.find("call ") else { + continue; + }; + let tail = &line[call_at..]; + // `call @name(` — the callee is the `@…` immediately before + // the argument list. + let Some(at) = tail.find('@') else { continue }; + let Some(paren) = tail[at..].find('(') else { + continue; + }; + let name = &tail[at + 1..at + paren]; + if name.ends_with("__pshape") { + out.push(name.to_string()); + } + } + out +} + +fn pshape_definitions(ir: &str) -> Vec { + ir.lines() + .filter(|l| l.starts_with("define") && l.contains("__pshape")) + .filter_map(|l| { + let at = l.find('@')?; + let paren = l[at..].find('(')?; + Some(l[at + 1..at + paren].to_string()) + }) + .collect() +} + +fn emit(m: &Module, is_entry: bool) -> String { + String::from_utf8(compile_module(m, ir_opts(is_entry)).unwrap()) + .expect("LLVM IR should be UTF-8") +} + +/// `bump(): void { this.value = this.value + 1 }` — a `void` return, so NO +/// typed clone of any tier is eligible (every tier requires a `number`/`i32`/ +/// `boolean`/`string` return). This is the arm that already worked. +fn bump_method() -> Function { + func( + 90, + "bump", + Vec::new(), + Type::Void, + vec![Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "value".to_string(), + value: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(this_get("value")), + right: Box::new(Expr::Number(1.0)), + }), + })], + ) +} + +/// `scale(f: number): number { return this.value * f }` — a `number` return +/// and an f64 parameter, so the typed-receiver / typed-f64 clones ARE eligible +/// and their arm runs before the proven-`this` arm. Its generic fallback is +/// what regressed. +fn scale_method() -> Function { + func( + 91, + "scale", + vec![param(70, "f", Type::Number)], + Type::Number, + vec![Stmt::Return(Some(Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(this_get("value")), + right: Box::new(Expr::LocalGet(70)), + }))], + ) +} + +fn counter_class() -> Class { + class( + 101, + "Counter", + vec![field("value", Type::Number)], + vec![bump_method(), scale_method()], + ) +} + +/// A module whose `probe(c: Counter)` calls both methods on a statically-typed +/// parameter. A typed parameter is not a Phase 3b shape-proven local (no +/// provenance, no containment), so both calls go through the *guarded* site: +/// `emit_guarded_direct_method_call`, behind the class-id + keys-token guard. +fn guarded_site_module() -> Module { + let mut m = Module::new("pshape_guarded.ts"); + m.classes = vec![counter_class()]; + m.functions = vec![func( + 1, + "probe", + vec![param(2, "c", Type::Named("Counter".to_string()))], + Type::Void, + vec![ + Stmt::Expr(call(Expr::LocalGet(2), "bump", Vec::new())), + Stmt::Expr(call(Expr::LocalGet(2), "scale", vec![Expr::Number(1.5)])), + ], + )]; + m.init_kind = ModuleInitKind::Eager; + m +} + +/// Regression: a method with NO eligible typed clone routes to its clone. +/// +/// This half was already true before #7128 and is kept as the control — if it +/// ever goes red, the routing site itself (not the arm ordering) has broken. +#[test] +fn untyped_method_routes_to_proven_this_clone() { + let ir = emit(&guarded_site_module(), false); + let defs = pshape_definitions(&ir); + let calls = pshape_call_targets(&ir); + let bump = defs + .iter() + .find(|d| d.contains("__bump__pshape")) + .unwrap_or_else(|| panic!("no proven-`this` clone emitted for `bump`: {defs:?}")); + assert!( + calls.iter().any(|c| c == bump), + "`bump`'s proven-`this` clone is emitted but nothing calls it — it will \ + be dead-stripped. defined={defs:?} called={calls:?}" + ); +} + +/// Regression (#7128): a method that ALSO has a typed clone must still route +/// its generic fallback to the proven-`this` clone. +/// +/// Before the fix, `emit_guarded_direct_method_call` tried the five typed arms +/// first and only the final `else` consulted `pshape_methods`. Every method +/// that admits a proven-`this` clone must touch a declared field of its own +/// chain — which is very nearly the definition of a typed-receiver-clone +/// candidate — so the typed arm won essentially whenever both were eligible, +/// and the clone was emitted with no caller at all. +/// +/// The typed arm's fast path is deliberately left alone (that is a +/// cost-model question, tracked separately): what must be true is that the +/// fallback it branches to no longer throws the receiver proof away. +#[test] +fn typed_clone_fallback_routes_to_proven_this_clone() { + let ir = emit(&guarded_site_module(), false); + let defs = pshape_definitions(&ir); + let calls = pshape_call_targets(&ir); + let scale = defs + .iter() + .find(|d| d.contains("__scale__pshape")) + .unwrap_or_else(|| panic!("no proven-`this` clone emitted for `scale`: defined={defs:?}")); + assert!( + calls.iter().any(|c| c == scale), + "`scale` has a typed clone, so the typed arm runs first; its generic \ + fallback must still route to the proven-`this` clone instead of the \ + guard-ridden public body. defined={defs:?} called={calls:?}" + ); + // The typed arm itself must survive — this fix reroutes the FALLBACK, it + // does not displace the typed clone. + assert!( + ir.contains("__typed_f64_recv") || ir.contains("__typed_f64"), + "the typed clone must still be emitted and preferred on the fast \ + path:\n{ir}" + ); +} + +/// The routed call must still hand the callee a shadow-bound receiver slot. +/// +/// #6925 kept the clone's `(double this, …)` ABI and its shadow-bound, +/// tagged-at-rest receiver slot precisely because `GC_TYPE_OBJECT` is MOVABLE +/// in the shipped configuration (#7019) — the `TaPtr` no-bind shortcut does not +/// transfer. Routing more call sites to the clone is only safe while that +/// remains true, so assert it at the callee rather than trusting the comment. +#[test] +fn proven_this_clone_binds_its_receiver_slot() { + let ir = emit(&guarded_site_module(), false); + for name in pshape_definitions(&ir) { + let start = ir + .find(&format!("@{name}(")) + .and_then(|i| ir[i..].find('{').map(|j| i + j)) + .unwrap_or_else(|| panic!("clone {name} has no body")); + let body_end = ir[start..] + .find("\n}") + .map(|e| start + e) + .unwrap_or(ir.len()); + let body = &ir[start..body_end]; + let store = body + .find("store double %this_arg") + .unwrap_or_else(|| panic!("{name}: receiver is never stored to a slot:\n{body}")); + let bind = body + .find("call void @js_shadow_slot_bind") + .unwrap_or_else(|| panic!("{name}: receiver slot is never shadow-bound:\n{body}")); + assert!( + store < bind, + "{name}: the receiver must be stored to its slot BEFORE the slot is \ + bound, with no safepoint between:\n{body}" + ); + } +} + +// 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 +// the binary cached — it would pass by skipping itself far more often than it +// checked anything, which is precisely the vacuous gate this file exists to +// avoid. The knob direction is exercised out-of-process instead, by the census +// (`benchmarks/repsel_census/README.md`), which re-runs the whole corpus under +// the knob on every CI job and asserts `ptr-shape-consumed` drops to zero. diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 25dba1a303..afb95b8c9e 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -180,6 +180,29 @@ pub(super) fn emit_guarded_direct_method_call( let expected_class_id = *ctx.class_ids.get(receiver_class_name)?; let keys_global_name = ctx.class_keys_globals.get(receiver_class_name)?.clone(); + // Representation-selection Phase 5a: the proven-`this` clone for this + // (class, method), when the emission loop produced one. + // + // Computed ONCE here rather than per-arm because the justification is the + // same for every block this helper emits below: they are all dominated by + // the `js_method_direct_shape_guard` / + // `js_typed_feedback_method_direct_call_guard` branch, which matched the + // exact class id AND the keys token. A `pshape_methods` hit additionally + // proves `receiver_class_name` DECLARES `property` (the map holds own + // declarations of module-local classes only), so the clone's `this` is + // exactly the class it was compiled for and can never be a subclass + // instance. + // + // The `perry_static_` exclusion is carried forward from the guard-free + // site (the #1787 static-receiver bug): those targets need + // `js_class_static_method_call`, not a plain `call double`, and no + // proven-`this` clone is ever emitted for them. + let pshape_fn: Option = (!direct_fn.starts_with("perry_static_") + && ctx + .pshape_methods + .contains_key(&(receiver_class_name.to_string(), property.to_string()))) + .then(|| crate::collectors::pshape_method_name(direct_fn)); + let expected_class_id_str = expected_class_id.to_string(); let expected_keys_slot = ctx.func.entry_init_load_global(&keys_global_name, I64); let expected_keys = ctx.block().load(I64, &expected_keys_slot); @@ -246,7 +269,9 @@ pub(super) fn emit_guarded_direct_method_call( ctx.current_block = fast_idx; let fast_value = { if let Some((typed_fn, typed_formal_count, receiver_info)) = typed_f64_receiver_direct_fn { - let generic_body_fn = crate::codegen::generic_method_body_name(direct_fn); + let generic_body_fn = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -374,7 +399,9 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_direct_fn { - let generic_body_fn = crate::codegen::generic_method_body_name(direct_fn); + let generic_body_fn = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -459,7 +486,9 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_i32_direct_fn { - let generic_body_fn = crate::codegen::generic_method_body_name(direct_fn); + let generic_body_fn = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -546,7 +575,9 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_i1_direct_fn { - let generic_body_fn = crate::codegen::generic_method_body_name(direct_fn); + let generic_body_fn = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -646,7 +677,9 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_string_direct_fn { - let generic_body_fn = crate::codegen::generic_method_body_name(direct_fn); + let generic_body_fn = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -758,18 +791,12 @@ pub(super) fn emit_guarded_direct_method_call( // needs no such guard because it never claims `JsNumber` — its // bare loads carry generic `JsValue` semantics (see // `collectors/proven_this.rs`). - // The `perry_static_` exclusion is carried forward from the - // guard-free site (the #1787 static-receiver bug): those targets - // need `js_class_static_method_call`, not a plain `call double`, - // and no proven-`this` clone is ever emitted for them. Belt and - // braces — a static's registry key is distinct from an instance - // method's, so the two maps cannot currently disagree. - let pshape_target = (!direct_fn.starts_with("perry_static_") - && ctx - .pshape_methods - .contains_key(&(receiver_class_name.to_string(), property.to_string()))) - .then(|| crate::collectors::pshape_method_name(direct_fn)); - let target = pshape_target.as_deref().unwrap_or(direct_fn); + // + // `pshape_fn` (computed once at the top of this function, where the + // `perry_static_` exclusion and the declaring-class argument are + // written out) is the same clone the typed arms above now route + // their generic fallbacks to. + let target = pshape_fn.as_deref().unwrap_or(direct_fn); ctx.block().call(DOUBLE, target, direct_arg_slices) } }; 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 9865a3b7c7..bb01846a85 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 @@ -870,6 +870,25 @@ pub(crate) fn try_lower_instance_method_call( .unwrap_or(false); if ptr_shape_receiver && !fallback_fn.starts_with("perry_static_") { ctx.note_ptr_shape_consumed(object, "ptr_shape_method"); + // Representation-selection Phase 5a: the proven-`this` + // clone, when one was emitted. Hoisted above the + // typed-receiver branch because BOTH exits of this block + // are guard-free under the same Phase 3b proof — the + // typed-receiver arm's own generic fallback used to call + // the guard-ridden public body while the plain exit 25 + // lines below routed to the clone, so a proven receiver + // whose ARGUMENTS happened to be non-plain-double silently + // lost the receiver proof too. + // + // A `pshape_methods` hit additionally proves `class_name` + // DECLARES `property` — so the clone's `this` is exactly + // the class it was compiled for and cannot be a subclass + // instance. + let pshape_target = ctx + .pshape_methods + .contains_key(&(class_name.clone(), property.to_string())) + .then(|| crate::collectors::pshape_method_name(&fallback_fn)); + let generic_target = pshape_target.as_deref().unwrap_or(fallback_fn.as_str()); // Prefer the typed-receiver clone (bare gep+load field // access inside the body) when one exists: the receiver // is proven, so only the ARGUMENT value classes need @@ -920,7 +939,7 @@ pub(crate) fn try_lower_instance_method_call( let typed_end = ctx.block().label.clone(); ctx.block().br(&merge_label); ctx.current_block = generic_idx; - let v_generic = ctx.block().call(DOUBLE, &fallback_fn, &arg_slices); + let v_generic = ctx.block().call(DOUBLE, generic_target, &arg_slices); let generic_end = ctx.block().label.clone(); ctx.block().br(&merge_label); ctx.current_block = merge_idx; @@ -934,19 +953,12 @@ pub(crate) fn try_lower_instance_method_call( return Ok(Some(merged)); } // Representation-selection Phase 5a: route to the - // proven-`this` clone when one exists. The receiver is - // already proven here (no guard is emitted on this path at - // all), and a `pshape_methods` hit additionally proves - // `class_name` DECLARES `property` — so the clone's `this` - // is exactly the class it was compiled for and cannot be a - // subclass instance. Same ABI, so the call is unchanged - // apart from the callee name. - let pshape_target = ctx - .pshape_methods - .contains_key(&(class_name.clone(), property.to_string())) - .then(|| crate::collectors::pshape_method_name(&fallback_fn)); - let target = pshape_target.as_deref().unwrap_or(fallback_fn.as_str()); - let direct = ctx.block().call(DOUBLE, target, &arg_slices); + // proven-`this` clone when one exists (`generic_target`, + // resolved at the top of this block). The receiver is + // already proven here — no guard is emitted on this path at + // all. Same ABI, so the call is unchanged apart from the + // callee name. + let direct = ctx.block().call(DOUBLE, generic_target, &arg_slices); return Ok(Some(direct)); } if let Some(guarded) = emit_guarded_direct_method_call( From 83a1dd46bdd63d5d0524a4e0dc348071ad372523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:04:53 +0200 Subject: [PATCH 2/6] docs(changelog): Phase 5a proven-`this` routing fragment (#7141) --- changelog.d/7141-pshape-routing.md | 49 ++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 changelog.d/7141-pshape-routing.md diff --git a/changelog.d/7141-pshape-routing.md b/changelog.d/7141-pshape-routing.md new file mode 100644 index 0000000000..955aed2af3 --- /dev/null +++ b/changelog.d/7141-pshape-routing.md @@ -0,0 +1,49 @@ +### Fixed + +- **repsel Phase 5a: proven-`this` clones had zero call sites and were dead-stripped (#7128).** + `collectors/proven_this.rs` emitted a `{method}__pshape` clone — a full second + body of the method whose `this` carries the `Ptr` proof, so every + `this.field` inside it is a bare fixed-offset `load double` instead of a + guarded diamond — but nothing in the corpus ever *called* one. Every clone was + emitted, reachable from nothing, and dropped by the linker. + + The two checks in place could not see it. An object-hash A/B scores the phase + as working, because `suite_09_method_calls`' object genuinely does differ with + the analysis off — by two dead clone bodies. And the promotion census scores + it as working, because the clone's `this` is a real `Ptr` consumption, + recorded at every `this.field` site *inside the body nobody calls*. That is + the reconciliation of the two contradictory reports: #7117's "7 receiver + consumptions corpus-wide" and #7128's "zero call sites" are both true and + describe the same dead code. + + Root cause is arm ordering at both routing sites. `emit_guarded_direct_method_call` + (`lower_call/method_override.rs`) tried five typed-clone arms and consulted + `pshape_methods` only in the final `else`; the Phase 3b guard-free site + (`lower_call/property_get/dynamic_dispatch.rs`) had the same shape, routing to + the clone on its plain exit but calling the guard-ridden public body from the + typed-receiver arm's own generic fallback 25 lines above. A method can only + admit a proven-`this` clone if it touches a declared field of its own chain, + which is very nearly the definition of a typed-receiver-clone candidate — so + the typed arm won essentially whenever both were eligible. + + Both sites now resolve the clone once, up front, and use it for the generic + fallback as well as the plain exit. The typed clone is still preferred on the + fast path; what changed is that falling off it no longer discards a receiver + proof the enclosing block had already established. Same `(double this, args…)` + ABI, same shadow-bound tagged-at-rest receiver slot, no new proof obligation: + every rerouted block is dominated either by the class-id + keys-token guard or + by Phase 3b containment, which is exactly what the existing `else` arm already + relied on. + + New `collectors/proven_this_routing_tests.rs` ratchets **call sites**, not + symbol presence — it matches the callee position of a `call` so a + `ptrtoint ptr @…__pshape` operand can never be miscounted — and asserts the + clone still stores-then-`js_shadow_slot_bind`s its receiver, since + `GC_TYPE_OBJECT` moves in the shipped configuration (#7019). + + Two findings are deliberately left unfixed and filed instead: routing the + class-id-switch dispatch tower (`idispatch.caseN`) would be **unsound** — + `delete inst.field` compacts packed slots while preserving `class_id`, which + is precisely what the keys token catches — and whether the typed arm should + yield to the clone on its *fast* path is a cost-model question for the + `collectors/repsel_benefit.rs` gate added in #7132. From 45eb94e4cdc96a9146c81f6a89f151fb7cc20b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:10:17 +0200 Subject: [PATCH 3/6] test(codegen): anchor the receiver-bind check on the clone's define line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit find("@name(") matches a routed CALL site first — which appears earlier in the module than the definition — so the slice could cover the caller's body and pass on some other function's shadow bind. --- .../collectors/proven_this_routing_tests.rs | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) 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 88873042bf..b6131484df 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -361,16 +361,27 @@ fn typed_clone_fallback_routes_to_proven_this_clone() { #[test] fn proven_this_clone_binds_its_receiver_slot() { let ir = emit(&guarded_site_module(), false); - for name in pshape_definitions(&ir) { - let start = ir - .find(&format!("@{name}(")) - .and_then(|i| ir[i..].find('{').map(|j| i + j)) - .unwrap_or_else(|| panic!("clone {name} has no body")); - let body_end = ir[start..] - .find("\n}") - .map(|e| start + e) - .unwrap_or(ir.len()); - let body = &ir[start..body_end]; + let names = pshape_definitions(&ir); + assert!( + !names.is_empty(), + "nothing to check — no proven-`this` clone was emitted at all" + ); + for name in names { + // Anchor on the DEFINITION line, not the first mention: a routed call + // site names the same symbol and appears earlier in the module, so + // `find("@{name}(")` alone would slice the caller's body and then + // "pass" by finding some other function's receiver bind. + let def_line = ir + .lines() + .position(|l| l.starts_with("define") && l.contains(&format!("@{name}("))) + .unwrap_or_else(|| panic!("clone {name} has no definition line")); + let body: String = ir + .lines() + .skip(def_line) + .take_while(|l| *l != "}") + .collect::>() + .join("\n"); + let body = body.as_str(); let store = body .find("store double %this_arg") .unwrap_or_else(|| panic!("{name}: receiver is never stored to a slot:\n{body}")); From eb455ede22fcd9cbcea53d98f5ed8384c3a770ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:30:07 +0200 Subject: [PATCH 4/6] docs(changelog): record the measured Phase 5a routing numbers (#7141) --- changelog.d/7141-pshape-routing.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/changelog.d/7141-pshape-routing.md b/changelog.d/7141-pshape-routing.md index 955aed2af3..8597b2a6e9 100644 --- a/changelog.d/7141-pshape-routing.md +++ b/changelog.d/7141-pshape-routing.md @@ -47,3 +47,13 @@ is precisely what the keys token catches — and whether the typed arm should yield to the clone on its *fast* path is a cost-model question for the `collectors/repsel_benefit.rs` gate added in #7132. + + Measured on a Raspberry Pi 5 (`perry-dev`, one `CARGO_TARGET_DIR` per arm, + identical package sets): `__pshape` call sites go 0 → 2 on + `fixture_ptr_shape.ts`, 0 → 2 on `fixture_ptr_shape_sites.ts` and 0 → 1 on + `09_method_calls.ts`. The body a routed call enters drops from 304 IR lines + with 4 `js_typed_feedback_class_field_*_guard` and 4 + `js_object_get_field_by_name*` calls to 104 lines with none of either + (`Point::norm2`, 19 → 7 opaque `js_*` calls). `cargo test -p perry-codegen + --lib` 409/409, `census --gate` OK with every floor held, and the three + `Ptr`/proven-`this` gap tests byte-identical against `main`. From 1bed0de295e5b91a412a620c8a036b2bcc103409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:31:23 +0200 Subject: [PATCH 5/6] style(codegen): rustfmt linker.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-existing on main since #7135 — `cargo fmt --all -- --check` is red at a3b31c0d8 with exactly this hunk and nothing else. Carried here in its own commit because the `lint` gate would otherwise fail this PR for it. --- crates/perry-codegen/src/linker.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/perry-codegen/src/linker.rs b/crates/perry-codegen/src/linker.rs index 4dad530820..38172041b4 100644 --- a/crates/perry-codegen/src/linker.rs +++ b/crates/perry-codegen/src/linker.rs @@ -96,9 +96,8 @@ fn write_ll_atomically(ll_path: &Path, ll_text: &str, counter: u64) -> Result<() return Ok(()); } } - fs::write(ll_path, ll_text.as_bytes()).with_context(|| { - format!("Failed to write temp .ll file at {}", ll_path.display()) - }) + fs::write(ll_path, ll_text.as_bytes()) + .with_context(|| format!("Failed to write temp .ll file at {}", ll_path.display())) } } } From aff97dac2e79e0ab0d1cb92e0a074c4002e1caff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 22:47:56 +0200 Subject: [PATCH 6/6] test(codegen): cover the Phase 3b guard-free routing site too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module doc claimed both routing sites were covered, but every test used a typed PARAMETER receiver, which only reaches emit_guarded_direct_method_call. The dynamic_dispatch.rs half of the fix had no unit test at all. Adds a shape-proven LOCAL fixture (single Let initialised by `new`, only field-accessed and method-called) whose method has a typed-receiver clone, so it takes the same shadowed fallback on the guard-free site. Fails against main's routing, passes with the fix; the receiver-bind ratchet now scans both modules. Note for the next person: returning `p.norm2()` directly disqualifies the local — the containment walk sees a LocalGet(p) inside the Return and does not distinguish a call receiver from an escape — which is why the result goes through an accumulator, exactly as fixture_ptr_shape.ts does. Also hoists generic_body_fn next to pshape_fn (CodeRabbit): both inputs are arm-invariant, so the five arms were recomputing one value. --- .../collectors/proven_this_routing_tests.rs | 161 +++++++++++++++++- .../src/lower_call/method_override.rs | 21 +-- 2 files changed, 166 insertions(+), 16 deletions(-) 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 b6131484df..e633a79e19 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -273,6 +273,131 @@ fn counter_class() -> Class { ) } +/// `class Point { x; y; constructor(x, y); norm2(): number }` — the shape of +/// `benchmarks/repsel_census/fixtures/fixture_ptr_shape.ts`, whose whole purpose +/// is to satisfy every rule in `collectors/ptr_shape.rs` so a local can actually +/// be shape-proven. `norm2` returns `number` and reads only declared fields, so +/// the typed-receiver clone is eligible and its arm runs first — the same +/// shadowing that hid the guarded site, on the other routing site. +fn point_class() -> Class { + let ctor = func( + 95, + "constructor", + vec![param(80, "x", Type::Number), param(81, "y", Type::Number)], + Type::Void, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "x".to_string(), + value: Box::new(Expr::LocalGet(80)), + }), + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "y".to_string(), + value: Box::new(Expr::LocalGet(81)), + }), + ], + ); + let norm2 = func( + 96, + "norm2", + Vec::new(), + Type::Number, + vec![Stmt::Return(Some(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(this_get("x")), + right: Box::new(this_get("x")), + }), + right: Box::new(Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(this_get("y")), + right: Box::new(this_get("y")), + }), + }))], + ); + let mut c = class( + 102, + "Point", + vec![field("x", Type::Number), field("y", Type::Number)], + vec![norm2], + ); + c.constructor = Some(ctor); + c +} + +/// A module whose `probe()` holds a Phase 3b **shape-proven local**: a single +/// `Let` initialised by `new` (provenance), only ever field-accessed and +/// method-called, never reassigned, captured, passed, returned or aliased +/// (containment). That combination is what routes through the guard-FREE site +/// in `lower_call/property_get/dynamic_dispatch.rs`, which is a different code +/// path from `guarded_site_module`'s typed parameter. +fn ptr_shape_local_module() -> Module { + let mut m = Module::new("pshape_local.ts"); + m.classes = vec![point_class()]; + m.functions = vec![func( + 1, + "probe", + Vec::new(), + Type::Number, + vec![ + Stmt::Let { + id: 3, + name: "total".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Number(0.0)), + }, + Stmt::Let { + id: 2, + name: "p".to_string(), + ty: Type::Named("Point".to_string()), + mutable: false, + init: Some(Expr::New { + class_name: "Point".to_string(), + args: vec![Expr::Number(3.0), Expr::Number(4.0)], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }), + }, + // `p.x = p.x + 1` — a declared-field read and write, which keeps the + // local inside the proof. + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(2)), + property: "x".to_string(), + value: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(2)), + property: "x".to_string(), + byte_offset: 0, + }), + right: Box::new(Expr::Number(1.0)), + }), + }), + // The method result is accumulated into a separate local, exactly as + // `fixture_ptr_shape.ts` does. Returning `p.norm2()` directly puts a + // `LocalGet(p)` inside the `Return` expression, which the + // containment walk treats as an escape — the receiver of a call is + // not distinguished there — and the local silently stops being + // shape-proven. + Stmt::Expr(Expr::LocalSet( + 3, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(3)), + right: Box::new(call(Expr::LocalGet(2), "norm2", Vec::new())), + }), + )), + Stmt::Return(Some(Expr::LocalGet(3))), + ], + )]; + m.init_kind = ModuleInitKind::Eager; + m +} + /// A module whose `probe(c: Counter)` calls both methods on a statically-typed /// parameter. A typed parameter is not a Phase 3b shape-proven local (no /// provenance, no containment), so both calls go through the *guarded* site: @@ -351,6 +476,38 @@ fn typed_clone_fallback_routes_to_proven_this_clone() { ); } +/// Regression (#7128), Phase 3b guard-free site: a shape-proven LOCAL whose +/// method also has a typed-receiver clone must still route to the proven-`this` +/// clone. +/// +/// `lower_call/property_get/dynamic_dispatch.rs` had the defect in its purest +/// form — the block routed to the clone on its plain exit, but the +/// typed-receiver arm 25 lines above called the guard-ridden public body from +/// its own generic fallback. Both exits are guard-free under the identical +/// Phase 3b proof, so a proven receiver whose ARGUMENTS happened to be +/// non-plain-double silently lost the receiver proof as well. +#[test] +fn ptr_shape_local_typed_fallback_routes_to_proven_this_clone() { + let ir = emit(&ptr_shape_local_module(), false); + let defs = pshape_definitions(&ir); + let calls = pshape_call_targets(&ir); + let norm2 = defs + .iter() + .find(|d| d.contains("__norm2__pshape")) + .unwrap_or_else(|| { + panic!( + "no proven-`this` clone emitted for `norm2` — the Phase 3b \ + fixture no longer satisfies the shape proof, so this test \ + would pass vacuously. defined={defs:?}" + ) + }); + assert!( + calls.iter().any(|c| c == norm2), + "a shape-proven local's method call must route to the proven-`this` \ + clone from the guard-free site too. defined={defs:?} called={calls:?}" + ); +} + /// The routed call must still hand the callee a shadow-bound receiver slot. /// /// #6925 kept the clone's `(double this, …)` ABI and its shadow-bound, @@ -360,7 +517,9 @@ fn typed_clone_fallback_routes_to_proven_this_clone() { /// remains true, so assert it at the callee rather than trusting the comment. #[test] fn proven_this_clone_binds_its_receiver_slot() { - let ir = emit(&guarded_site_module(), false); + let mut ir = emit(&guarded_site_module(), false); + ir.push('\n'); + ir.push_str(&emit(&ptr_shape_local_module(), false)); let names = pshape_definitions(&ir); assert!( !names.is_empty(), diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index afb95b8c9e..39f93ba0d8 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -203,6 +203,12 @@ pub(super) fn emit_guarded_direct_method_call( .contains_key(&(receiver_class_name.to_string(), property.to_string()))) .then(|| crate::collectors::pshape_method_name(direct_fn)); + // The body a failed typed guard falls back to. Arm-invariant (both inputs + // are), so it is resolved once here rather than five times below. + let generic_body_fn: String = pshape_fn + .clone() + .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); + let expected_class_id_str = expected_class_id.to_string(); let expected_keys_slot = ctx.func.entry_init_load_global(&keys_global_name, I64); let expected_keys = ctx.block().load(I64, &expected_keys_slot); @@ -269,9 +275,6 @@ pub(super) fn emit_guarded_direct_method_call( ctx.current_block = fast_idx; let fast_value = { if let Some((typed_fn, typed_formal_count, receiver_info)) = typed_f64_receiver_direct_fn { - let generic_body_fn = pshape_fn - .clone() - .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -399,9 +402,6 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_direct_fn { - let generic_body_fn = pshape_fn - .clone() - .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -486,9 +486,6 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_i32_direct_fn { - let generic_body_fn = pshape_fn - .clone() - .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -575,9 +572,6 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_i1_direct_fn { - let generic_body_fn = pshape_fn - .clone() - .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1) @@ -677,9 +671,6 @@ pub(super) fn emit_guarded_direct_method_call( ); result } else if let Some((typed_fn, typed_param_reps)) = typed_string_direct_fn { - let generic_body_fn = pshape_fn - .clone() - .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let formal_args: Vec<&str> = direct_arg_slices .iter() .skip(1)