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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions changelog.d/6925-repsel-p5a-proven-this.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Representation-selection Phase 5a β€” `Ptr<Shape>` proven `this` in methods (RFC Β§5.4/Β§5.6/Β§5.7):** two call sites already prove a receiver's exact shape and then throw the proof away by calling the guard-ridden PUBLIC method body, where every `this.field` re-enters the per-access guarded diamond (`expr/class_field_inline_guard.rs`) that LLVM provably cannot hoist (volatile gate + the fallback arm's opaque call block LICM). Phase 5a emits an additive `internal` proven-`this` clone of the method and routes both sites to it: `lower_call/method_override.rs`'s `method_direct.fast` arm (entered only after `js_method_direct_shape_guard` / `js_typed_feedback_method_direct_call_guard` matched class id **and** keys token) and `lower_call/property_get/dynamic_dispatch.rs`'s Phase-3b guard-free `Ptr<Shape>` receiver arm. Net new proof work, GC work and layout work: zero β€” the clone keeps the identical `(double this, double args…)` ABI and the identical shadow-bound tagged-at-rest receiver slot (`GC_TYPE_OBJECT` is movable, so the `TaPtr` no-shadow shortcut does **not** transfer), and field indexes come from the existing chain-global `class_field_global_index`.

Structural result (`test_gap_repsel_ptr_shape_locals.ts`, pre-opt β†’ clone; verified to persist post-`opt -O3`): `Vec5a.lenSq` 4 volatile gates / 4 `js_typed_feedback_class_field_get_guard` / 4 by-name gets (297 lines) β†’ **0 / 0 / 0** (97 lines); `Vec5a.normalizeTo` 5 / 7 / 5 (786 lines) β†’ **0 / 0 / 0** (489); `Counter5a.increment` and `.bump` 1 / 1 / 1 β†’ **0 / 0 / 0**. The generic bodies retain every diamond post-`-O3`, which is the direct evidence that the per-access cost is unhoistable and has to be removed structurally. Small clones are then fully inlined into their call sites by LLVM (they are `internal` with one caller).

Eligibility (class level): the Phase 3b `chain_admissible` walk (modeled, accessor-free, computed-free, statically-extended chain), `ModuleDispatchFacts::prototype_is_stable`, and the module-wide Β§5.2 shape-barrier kill. Method level: not async/generator/`was_plain_async`, no captures, no rest/default/`arguments` params, `this` never used as a value (reuses `ThisFlowAnalysis` β€” `Expr::This` in value position and closures mentioning `this` both reject), every `this.f = v` write to a declared chain field, and every internally-invoked `this.m()`/`super.m()` vetted transitively. **Subclass safety is enforced at the routing sites, not in the collector:** both only route when the receiver's proven exact class *declares* the method (`method_registry.rs` never inserts inherited entries), so the clone's `this` is exactly the class it was compiled for; inherited dispatch keeps today's lowering.

`numeric_fields` is deliberately **not** claimed for a proven `this`. A Phase 3b local can claim `JsNumber` under an exhaustive reachable-store proof because containment proves no alias exists; a proven `this` is caller-owned and aliased by construction, so an `obj.f = "s"` store elsewhere β€” which downgrades the slot's raw-f64 layout at runtime β€” is not enumerable. Instead the number context keeps a 2-instruction inline plain-finite check on the *loaded bits* with a cold `js_number_coerce` arm, which still retires the volatile gate, the 7-header-load shape check, the guard call and the by-name fallback lookup.

**`Object.freeze`/`seal`/`preventExtensions` (soundness item 1) β€” took option (a), the module-wide kill for write-containing clones.** These are not in the Β§5.2 barrier list; Phase 3b needs no rule there because containment proves no alias and a freeze of the local disqualifies it directly. A proven `this` has no containment, so `Object.freeze(c)` followed by `c.m()` would let a guard-free raw store silently succeed where strict-mode class code must throw a `TypeError`. A new `ModuleDispatchFacts::freeze_barrier_sites` fact therefore disables proven-`this` clones **that contain a `this.field` write** whenever the module contains any freeze-family site; read-only clones and all of Phase 3b are unaffected. Option (b) (reads-only first increment) was rejected because it would have excluded the single most common method shape (`this.value = this.value + 1`). Covered by a dedicated gap test (`test_gap_repsel_proven_this_frozen.ts` β€” separate file precisely because the kill is module-wide).

**Vtable reachability (soundness item 2):** the clone is `internal` and is never passed to `js_register_class_method`, so it has no indirect route via `js_native_call_method`, `emit_collapsed_instance_dispatch`, the own-property override probe, or `.call`/`.apply`/`.bind`. A **separate** ratchet test (`collectors::proven_this::tests::pshape_symbol_reachability`) pins a four-file allowlist for the symbol fragment rather than widening `spec_abi_symbol_reachability`'s allowlist, which would have un-ratcheted the Phase 2 guarantee.

Two deliberate deviations from the plan, both documented inline: (1) the per-field `js_typed_feedback_class_field_get_guard` loop at the guarded call site is **kept**, because it guards the `__typed_f64_recv` clone's bare `load double` field access and the whole-object shape guard does *not* subsume it β€” an external `obj.f = "s"` preserves class id and key set while downgrading the slot layout; the proven-`this` clone needs no such guard because it never claims `JsNumber`. (2) the typed-receiver arm keeps priority over the new routing at the guard-free site, to avoid regressing an already-tuned path.

Also included, the adjacent `type_analysis/predicates.rs` fix: the `Named(_)` exclusion that made `const o: SomeIface = new C()` lose its Phase 3b proof is narrowed to "declared `Named(n)` where `n` is a known class". A TS `Named` annotation is just as often an interface or alias, which names no layout and is strictly weaker than the provenance proof; explicit class annotations still win, and `Generic { .. }` keeps its blanket exclusion (#6040).

**Clone-symbol collision guard (#6927).** `pshape_method_name` appends a suffix to the public symbol, so a class declaring both `foo` and `foo__pshape` would give `foo`'s clone the same LLVM symbol as `foo__pshape`'s PUBLIC entry β€” two definitions of one name. Cross-class shapes collide identically (a class literally named `C__foo` with a method `pshape`), because the `__` separators are not escaped either, so the check compares composed **symbols** from the method registry rather than member names. Colliding clones stand down to today's guarded lowering, which is correct for any receiver. Pruning happens right after the registry is built and before `emit_module_artifacts` reads `cross_module.pshape_methods`, so emission and call-site routing stay in lockstep and a call site can never route to a clone that was declined. This is a local mitigation for this phase's suffix only β€” the hazard is pre-existing and shared by the whole generated-clone family (`foo` + `foo__generic` already collides today); the family-wide fix is tracked in **#6927**.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

**GC contract under a moving collector, verified in IR (relevant to #6910).** `GC_TYPE_OBJECT` is movable, so the `TaPtr` no-shadow shortcut does not transfer. Two properties were checked rather than assumed: (1) the clone keeps `js_shadow_slot_bind` on the receiver β€” the shadow-frame setup in `compile_method` is unconditional, and this change touches only `llvm_name`, `linkage`, and public-trampoline re-emission; (2) no raw receiver pointer outlives a safepoint. Pre-`opt`, all 9 raw-pointer derivations in `Vec5a.normalizeTo`'s clone are fed by a fresh load of the shadow-bound slot occurring after the last preceding call β€” including across the internal `this.lenSq()` call. Post-`opt -O3`, LLVM inlines `lenSq` (removing that safepoint outright) and reloads the receiver from the slot after the remaining `js_dynamic_div` call; a dominance-aware scan finds all 7 surviving derivations in the same basic block as their defining load with no intervening call. The escaped shadow-bound alloca is what defeats CSE.

The gap corpus additionally places allocation and call safepoints inside hot loops over proven receivers, and passes byte-exact with `PERRY_GC_FORCE_EVACUATE=1` and `PERRY_GC_VERIFY_EVACUATION=1` set. **That green arm is not evidence the proven-`this` receiver survives a live evacuation.** #6950 measured `FORCE_EVACUATE` to be inert on every reachable path (45 completed cycles, all manual full mark-sweeps, `moved_objects=0`), so what the arm establishes is byte-exact behavioural agreement *with the flags set* β€” nothing more. The actual soundness argument for a moving collector is the static/IR one above (retained `js_shadow_slot_bind`, no raw receiver pointer live across a safepoint), not the flag run.

Gated by `PERRY_PTR_SHAPE_THIS` (default on, `0`/`off`/`false` disables), keyed into the object cache.
46 changes: 46 additions & 0 deletions crates/perry-codegen/src/codegen/artifacts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,50 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cross_module
.typed_f64_receiver_methods
.contains_key(&(class.name.clone(), method.name.clone())),
None,
)
.with_context(|| format!("lowering method '{}::{}'", class.name, method.name))?;
// Representation-selection Phase 5a: the additive `internal`
// proven-`this` clone. Same HIR, same ABI, same shadow-bound
// tagged-at-rest receiver slot β€” only `this.field` lowering
// differs (bare fixed-offset access instead of the per-access
// guard diamond). Reached ONLY from the two call sites that
// already prove the receiver's exact shape; never registered into
// a runtime vtable.
if let Some(fact) = cross_module
.pshape_methods
.get(&(class.name.clone(), method.name.clone()))
{
compile_method(
llmod,
class,
method,
func_names,
strings,
class_table,
method_names,
module_globals,
module_global_types,
opts.import_function_prefixes,
enum_table,
static_field_globals,
class_ids,
func_signatures,
func_synthetic_arguments,
module_boxed_vars,
closure_rest_params,
cross_module,
None,
false,
Some(fact.clone()),
)
.with_context(|| {
format!(
"lowering proven-`this` clone of method '{}::{}'",
class.name, method.name
)
})?;
}
}
for member in class
.computed_members
Expand Down Expand Up @@ -444,6 +486,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cross_module,
None,
false,
None,
)
.with_context(|| {
format!(
Expand Down Expand Up @@ -508,6 +551,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cross_module,
None,
false,
None,
)
.with_context(|| format!("lowering getter '{}::{}'", class.name, prop))?;
}
Expand Down Expand Up @@ -560,6 +604,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cross_module,
None,
false,
None,
)
.with_context(|| format!("lowering setter '{}::{}'", class.name, prop))?;
}
Expand Down Expand Up @@ -654,6 +699,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
cross_module,
None,
false,
None,
)
.with_context(|| format!("lowering constructor for '{}'", class.name))?;
}
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,8 @@ pub(super) fn compile_closure(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down
4 changes: 4 additions & 0 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,8 @@ pub(super) fn compile_module_entry(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down Expand Up @@ -1437,6 +1439,8 @@ pub(super) fn compile_module_entry(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,8 @@ pub(super) fn compile_function(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down
30 changes: 24 additions & 6 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ pub(super) fn compile_method(
cross_module: &CrossModuleCtx,
typed_public_trampoline: Option<TypedFunctionTrampolineKind>,
force_generic_body: bool,
proven_this: Option<crate::collectors::PtrShapeLocal>,
) -> Result<()> {
let public_llvm_name = methods
.get(&(class.name.clone(), method.name.clone()))
Expand All @@ -272,7 +273,15 @@ pub(super) fn compile_method(
method.name
)
})?;
let llvm_name = if typed_public_trampoline.is_some() || force_generic_body {
// Representation-selection Phase 5a: the proven-`this` clone is a SECOND,
// additive body compiled from the same HIR through the same statement
// lowerer. It never replaces the public symbol and never participates in
// the typed-trampoline / generic-body split β€” those are emitted by the
// primary (`proven_this: None`) invocation for this same method.
let is_pshape_clone = proven_this.is_some();
let llvm_name = if is_pshape_clone {
crate::collectors::pshape_method_name(&public_llvm_name)
} else if typed_public_trampoline.is_some() || force_generic_body {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
generic_method_body_name(&public_llvm_name)
} else {
public_llvm_name.clone()
Expand All @@ -288,7 +297,7 @@ pub(super) fn compile_method(
let ic_base = llmod.ic_counter;
let buffer_alias_base = llmod.buffer_alias_counter;
let lf = llmod.define_function(&llvm_name, DOUBLE, params);
if typed_public_trampoline.is_some() || force_generic_body {
if is_pshape_clone || typed_public_trampoline.is_some() || force_generic_body {
lf.linkage = "internal".to_string();
}

Expand Down Expand Up @@ -551,6 +560,8 @@ pub(super) fn compile_method(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down Expand Up @@ -1025,10 +1036,15 @@ pub(super) fn compile_method(
for raw in &typed_parse_rodata {
llmod.add_raw_global(raw.clone());
}
if let Some(kind) = typed_public_trampoline {
emit_public_typed_method_trampoline(llmod, method, &public_llvm_name, &llvm_name, kind);
} else if force_generic_body {
emit_public_generic_method_forwarder(llmod, method, &public_llvm_name, &llvm_name);
// The Phase 5a clone is purely additive: the public symbol (and its
// trampoline/forwarder, if any) belongs to the primary invocation. Emitting
// it again here would define the same symbol twice.
if !is_pshape_clone {
if let Some(kind) = typed_public_trampoline {
emit_public_typed_method_trampoline(llmod, method, &public_llvm_name, &llvm_name, kind);
} else if force_generic_body {
emit_public_generic_method_forwarder(llmod, method, &public_llvm_name, &llvm_name);
}
}
Ok(())
}
Expand Down Expand Up @@ -1586,6 +1602,8 @@ pub(super) fn compile_static_method(
typed_i1_functions: &cross_module.typed_i1_functions,
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,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
typed_string_methods: &cross_module.typed_string_methods,
Expand Down
Loading
Loading