diff --git a/changelog.d/10189-class-field-get-one-exit.md b/changelog.d/10189-class-field-get-one-exit.md new file mode 100644 index 0000000000..7eb9ba9a14 --- /dev/null +++ b/changelog.d/10189-class-field-get-one-exit.md @@ -0,0 +1,10 @@ +Collapse the monomorphic class-field GET tower to one runtime exit. The #5093 +inline precheck and the fast slot load are unchanged; the guard call, the +nullish-throw diamond and the by-name fallback arms are replaced by a single +call to the existing `js_class_field_get_ic`, which performs the same guard, +load, fallback and throw. Per site that is 88 → 47 IR instructions, 3 → 1 +runtime call sites and 6 → 1 RS4GC relocation reloads; the fast path no longer +crosses a statepoint, so it sheds the two relocation reloads it used to carry. +On @babel/parser `.text` shrinks 7.1 % and `.perry_gcmap` 2.0 %, with runtime +instructions and RSS unchanged and `interp.ts` 13 % faster; typed-feedback +guard counters are identical per site. diff --git a/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs b/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs index d2bd3ad63f..92d05f7a9b 100644 --- a/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs +++ b/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs @@ -210,13 +210,20 @@ fn guarded_call_routes_to_shadow_rooted_direct_field_clone() { clone.contains("inttoptr i64") && clone.contains("getelementptr double"), "the clone must use direct declared-field addressing:\n{clone}" ); + // One-exit class-field GET: the tower's miss arm is a single + // `js_class_field_get_ic` call whose body IS the guard, so the IC is now a + // witness of "a field IC diamond was rebuilt" and must be excluded here + // too -- otherwise this negative assertion silently stops covering the + // shape it was written for. assert!( !clone.contains("js_typed_feedback_class_field_get_guard") + && !clone.contains("js_class_field_get_ic") && !clone.contains("shape_descriptor_by_id"), "the clone fast body must not rebuild the field IC diamond:\n{clone}" ); assert!( - generic.contains("js_typed_feedback_class_field_get_guard") + generic.contains("js_class_field_get_ic") + || generic.contains("js_typed_feedback_class_field_get_guard") || generic.contains("js_object_get_field"), "the generic fallback must retain guarded field semantics:\n{generic}" ); @@ -264,6 +271,7 @@ fn unannotated_parameter_uses_the_runtime_validated_class_overlay() { ); assert!( !clone.contains("js_typed_feedback_class_field_get_guard") + && !clone.contains("js_class_field_get_ic") && !clone.contains("shape_descriptor_by_id"), "the unannotated clone must not rebuild the field IC diamond:\n{clone}" ); diff --git a/crates/perry-codegen/src/expr/array_callback_shape_tests.rs b/crates/perry-codegen/src/expr/array_callback_shape_tests.rs index 5258bb195b..13ad7f81ca 100644 --- a/crates/perry-codegen/src/expr/array_callback_shape_tests.rs +++ b/crates/perry-codegen/src/expr/array_callback_shape_tests.rs @@ -150,8 +150,13 @@ fn callback_body(ir: &str) -> &str { fn inline_array_callback_field_read_has_no_shape_guard() { let ir = emit(&module_with_callback(false)); let callback = callback_body(&ir); + // `js_class_field_get_ic` is the one-exit tower's miss arm (guard + slot + // load + by-name fallback in one call), so it is the current witness that a + // guarded read was emitted; without it this negative assertion would pass + // on a callback that is still fully guarded. assert!( !callback.contains("js_typed_feedback_class_field_get_guard") + && !callback.contains("js_class_field_get_ic") && !callback.contains("js_object_get_field_by_name_f64"), "the proven element parameter must use a direct fixed-offset load:\n{callback}" ); @@ -166,7 +171,8 @@ fn callback_source_array_alias_keeps_the_shape_guard() { let ir = emit(&module_with_callback(true)); let callback = callback_body(&ir); assert!( - callback.contains("js_typed_feedback_class_field_get_guard") + callback.contains("js_class_field_get_ic") + || callback.contains("js_typed_feedback_class_field_get_guard") || callback.contains("js_object_get_field_by_name_f64"), "declaring the source-array argument must deny the cross-boundary fact:\n{callback}" ); diff --git a/crates/perry-codegen/src/expr/class_field_get_shape_tests.rs b/crates/perry-codegen/src/expr/class_field_get_shape_tests.rs new file mode 100644 index 0000000000..d07a664bae --- /dev/null +++ b/crates/perry-codegen/src/expr/class_field_get_shape_tests.rs @@ -0,0 +1,310 @@ +//! The monomorphic class-field GET tower has ONE exit. +//! +//! The tower in [`super::property_get`] used to emit four runtime call sites +//! per `this.field` / `obj.field` read — `js_typed_feedback_class_field_get_ +//! guard` when the #5093 inline pre-check missed, `js_throw_type_error_ +//! property_access` for a nullish receiver, and one or two +//! `js_object_get_field_by_name_f64` by-name lookups — spread over +//! `class_field_get.{fast,fallback,merge,throw_nullish,fallback_lookup}`. At +//! ~74 pre-RS4GC instructions and 13,636 sites in @babel/parser that arm alone +//! was 27 % of the module's IR, and because every call is a statepoint the +//! `.perry_gcmap` grew with it. +//! +//! Everything behind the pre-check now lives in `js_class_field_get_ic`, the +//! runtime helper the #5391 path-2 full outline already called. What must hold, +//! and why a label-presence check cannot show it: +//! +//! 1. **The inline hit path is still there and still REACHED.** A one-exit +//! tower that also lost its pre-check would be a pure pessimisation, and +//! every assertion below about "one call" would still pass. So: a `cond_br` +//! out of `class_field_inline.deref` INTO `class_field_get.fast`, and a +//! `load double` inside that block. (CLAUDE.md, "a gate must assert its +//! subject was live".) +//! 2. **The miss arm is exactly one call.** Counted, not merely "the IC is +//! mentioned": a regression that re-adds the by-name fallback beside the IC +//! would keep every positive assertion true. +//! 3. **The retired symbols are not CALLED at this site.** Asserted as +//! `call @name(`, never as a bare substring — all three are still +//! declared in the module (and still emitted by other towers), so a +//! substring test is satisfied by the `declare` line alone and can never +//! fail. That exact trap is why the older assertions in +//! `tests/typed_feedback.rs` were rewritten rather than deleted. +//! 4. **Both representations.** A `number` field (`require_raw_f64 = 1`) and an +//! `any` field (boxed) take the same shape; the raw-f64 site must not grow a +//! second by-name call back. + +use crate::compile_module; +use perry_hir::types::Type; +use perry_hir::{Class, ClassField, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +const PARAM_ID: u32 = 3; + +/// Blocks the tower is allowed to create, in the order it creates them. +const TOWER_BLOCKS: [&str; 4] = [ + "class_field_inline.deref", + "class_field_inline.guardcall", + "class_field_get.fast", + "class_field_get.merge", +]; + +/// Blocks the four-exit tower created that must no longer exist. +const RETIRED_BLOCKS: [&str; 3] = [ + "class_field_get.fallback", + "class_field_get.throw_nullish", + "class_field_get.fallback_lookup", +]; + +/// Call FORMS (not bare names) for the runtime entries the tower stopped +/// emitting. Each is still declared in every module and still emitted by other +/// lowerings, so only the call form can distinguish "not called here". +const RETIRED_CALLS: [&str; 3] = [ + "call i32 @js_typed_feedback_class_field_get_guard(", + "call void @js_throw_type_error_property_access(", + "call double @js_object_get_field_by_name_f64(", +]; + +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 point_class(field_ty: Type) -> Class { + Class { + id: 101, + name: "Point".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![field("x", field_ty)], + constructor: None, + methods: Vec::new(), + 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, + specialized_from: None, + } +} + +/// `function probe(p: Point) { return p.x }` — a bare `Named` parameter, which +/// is what routes to the generic `class_field_get.*` tower (#8033: the +/// numeric-specific `class_field_get_number.*` path needs a +/// `stable_local_type_proof` a parameter has not got). +fn probe_module(field_ty: Type) -> Module { + let mut m = Module::new("class_field_get_shape.ts"); + m.classes = vec![point_class(field_ty)]; + m.functions = vec![Function { + id: 1, + name: "probe".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: PARAM_ID, + name: "p".to_string(), + ty: Type::Named("Point".to_string()), + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Any, + body: vec![Stmt::Return(Some(Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::LocalGet(PARAM_ID)), + property: "x".to_string(), + }))], + 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, + }]; + m.init_kind = ModuleInitKind::Eager; + m +} + +fn ir(field_ty: Type) -> String { + String::from_utf8( + compile_module( + &probe_module(field_ty), + super::class_field_barrier_tests::ir_opts(), + ) + .expect("module compiles"), + ) + .expect("LLVM IR should be UTF-8") +} + +/// Body of the first block whose label line starts with `prefix`. Block labels +/// carry per-function numeric suffixes (`.fast.6`), so callers pass the stable +/// prefix; a label line is line-initial and ends in `:`, while branches and +/// phis that merely mention it are indented. +fn block_body<'a>(ir: &'a str, prefix: &str) -> Option<&'a str> { + let needle = format!("\n{prefix}"); + let mut from = 0; + while let Some(rel) = ir[from..].find(&needle) { + let label_start = from + rel + 1; + let line_end = label_start + ir[label_start..].find('\n')?; + if ir[label_start..line_end].ends_with(':') { + let rest = &ir[line_end + 1..]; + let end = match (rest.find("\n\n"), rest.find("\n}")) { + (Some(a), Some(b)) => a.min(b), + (a, b) => a.or(b).unwrap_or(rest.len()), + }; + return Some(&rest[..end]); + } + from = line_end; + } + None +} + +/// The `prefix.N:` label as rendered, e.g. `class_field_get.fast.6`. +fn block_label(ir: &str, prefix: &str) -> Option { + let needle = format!("\n{prefix}"); + let mut from = 0; + while let Some(rel) = ir[from..].find(&needle) { + let label_start = from + rel + 1; + let line_end = label_start + ir[label_start..].find('\n')?; + let line = &ir[label_start..line_end]; + if line.ends_with(':') { + return Some(line.trim_end_matches(':').to_string()); + } + from = line_end; + } + None +} + +fn assert_one_exit(field_ty: Type, what: &str) { + let ir = ir(field_ty); + + // (1) the pre-check exists and BRANCHES INTO the fast slot load. + let deref = block_body(&ir, "class_field_inline.deref") + .unwrap_or_else(|| panic!("{what}: no class_field_inline.deref block:\n{ir}")); + let fast_label = block_label(&ir, "class_field_get.fast") + .unwrap_or_else(|| panic!("{what}: no class_field_get.fast block:\n{ir}")); + assert!( + deref.contains("br i1 ") && deref.contains(&format!("label %{fast_label}")), + "{what}: class_field_inline.deref does not branch into {fast_label}; \ + the inline hit path is dead:\n{deref}" + ); + let fast = block_body(&ir, "class_field_get.fast") + .unwrap_or_else(|| panic!("{what}: no class_field_get.fast body:\n{ir}")); + assert!( + fast.contains("load double"), + "{what}: the fast block no longer loads the slot:\n{fast}" + ); + assert!( + !fast.contains("call "), + "{what}: the fast block must stay call-free:\n{fast}" + ); + + // (2) the miss arm is EXACTLY one call. + let guardcall = block_body(&ir, "class_field_inline.guardcall") + .unwrap_or_else(|| panic!("{what}: no class_field_inline.guardcall block:\n{ir}")); + let calls = guardcall.matches("call ").count(); + assert_eq!( + calls, 1, + "{what}: the miss arm must be ONE call, found {calls}:\n{guardcall}" + ); + assert!( + guardcall.contains("call double @js_class_field_get_ic("), + "{what}: the miss arm's one call is not js_class_field_get_ic:\n{guardcall}" + ); + + // (3) the retired blocks and the retired CALL FORMS are gone. + for block in RETIRED_BLOCKS { + assert!( + block_body(&ir, block).is_none(), + "{what}: retired block {block} is still emitted:\n{ir}" + ); + } + for call in RETIRED_CALLS { + assert!( + !ir.contains(call), + "{what}: retired call form `{call}` is still emitted:\n{ir}" + ); + } + + // (4) the merge is a two-way phi (fast value, IC value) and nothing else. + let merge = block_body(&ir, "class_field_get.merge") + .unwrap_or_else(|| panic!("{what}: no class_field_get.merge block:\n{ir}")); + let phis: Vec<&str> = merge.lines().filter(|l| l.contains("phi double")).collect(); + assert_eq!(phis.len(), 1, "{what}: merge is not a single phi:\n{merge}"); + assert_eq!( + phis[0].matches('[').count(), + 2, + "{what}: merge phi does not have exactly two incoming edges (fast, IC):\n{}", + phis[0] + ); + + // (5) the tower creates exactly four blocks. + for block in TOWER_BLOCKS { + assert!( + block_body(&ir, block).is_some(), + "{what}: tower block {block} missing:\n{ir}" + ); + } +} + +#[test] +fn boxed_class_field_get_has_one_exit() { + assert_one_exit(Type::Any, "any-typed field"); +} + +#[test] +fn raw_f64_class_field_get_has_one_exit() { + // `number` makes `requires_raw_f64` true, which used to emit a SECOND + // by-name fallback record/call pair. The IC takes `require_raw_f64` as an + // argument instead, so the site stays at one call. + assert_one_exit(Type::Number, "number-typed field"); +} + +#[test] +fn the_ic_call_carries_the_guard_operands() { + // The helper runs the same guard the inline miss arm used to call, so it + // must receive the same seven operands in the same order — site id, + // receiver, expected class id, expected shape id, key, field index, + // require_raw_f64. A dropped operand compiles and silently guards on the + // wrong thing. + let ir = ir(Type::Number); + let guardcall = block_body(&ir, "class_field_inline.guardcall") + .expect("guardcall block") + .to_string(); + let line = guardcall + .lines() + .find(|l| l.contains("@js_class_field_get_ic(")) + .expect("the IC call line"); + let args = line + .rsplit_once("@js_class_field_get_ic(") + .expect("call args") + .1; + let args = args.rsplit_once(')').expect("closing paren").0; + let tys: Vec<&str> = args + .split(", ") + .map(|a| a.split_whitespace().next().unwrap_or("")) + .collect(); + assert_eq!( + tys, + vec!["i64", "double", "i32", "i32", "i64", "i32", "i32"], + "IC call signature drifted from the guard's operand list:\n{line}" + ); +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index bcc1eb6f32..84b166bb0f 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -156,6 +156,8 @@ mod array_push_guard_tests; mod barrier_stem_census_tests; #[cfg(test)] mod class_field_barrier_tests; +#[cfg(test)] +mod class_field_get_shape_tests; mod dispatch; #[cfg(test)] mod index_set_barrier_tests; diff --git a/crates/perry-codegen/src/expr/property_get.rs b/crates/perry-codegen/src/expr/property_get.rs index b3b5b32c1c..51fe3d1214 100644 --- a/crates/perry-codegen/src/expr/property_get.rs +++ b/crates/perry-codegen/src/expr/property_get.rs @@ -1660,29 +1660,24 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); return Ok(val); } - // #5093: build the guard operands once, up front, so both - // the inline shape pre-check and the guard-call fallback - // can reference them. - let (obj_bits, obj_handle, key_raw) = { + // #5093: build the guard operands once, up front, so + // both the inline shape pre-check and the fast slot + // load can reference them. + let (obj_bits, obj_handle) = { let blk = ctx.block(); let obj_bits = blk.bitcast_double_to_i64(&recv_box); let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); - (obj_bits, obj_handle, key_raw) + (obj_bits, obj_handle) }; let fast_idx = ctx.new_block("class_field_get.fast"); - let fallback_idx = ctx.new_block("class_field_get.fallback"); let merge_idx = ctx.new_block("class_field_get.merge"); let fast_label = ctx.block_label(fast_idx); - let fallback_label = ctx.block_label(fallback_idx); let merge_label = ctx.block_label(merge_idx); // #5093: inline shape pre-check. On a monomorphic hit it // branches straight to the fast slot load, skipping the // cross-crate guard call; on a miss it leaves the current - // block at the guard-call path below (unchanged). + // block at the single-exit IC call below. let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms( ctx, @@ -1703,9 +1698,48 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &fast_label, &subclass_arms, ); - let guard_ok = ctx.block().call( - I32, - "js_typed_feedback_class_field_get_guard", + // ONE EXIT. Everything the pre-check could not prove — + // the guard call, the guard-PASS slot load, the nullish + // TypeError, the fallback record and the by-name lookup — + // is the body of `js_class_field_get_ic`, the same helper + // the #5391 path-2 full outline a few lines above already + // calls. Emitting the four arms inline cost ~74 IR + // instructions and 4 runtime call sites per monomorphic + // `this.field` read; @babel/parser has 13,636 of them + // (27 % of its IR), and each call site is a statepoint, + // so `.perry_gcmap` scaled with them too. + // + // The pre-check and the fast slot load below are + // unchanged, so a pre-check HIT costs exactly what it + // cost before (no call at all) and a pre-check MISS costs + // exactly one call — what the guard call alone already + // cost. Arm for arm the helper reproduces the diamond: + // guard PASS -> reads the same slot at the same + // header-relative offset; + // nullish -> throws the same + // `js_throw_type_error_property_access` + // (#7153 added that check to the helper + // for exactly this equivalence); + // guard FAIL -> records the fallback, then + // `js_object_get_field_by_name_f64`. + // A plain number is self-boxing under NaN-boxing, so a + // `requires_raw_f64` site reads the helper's return the + // same way the old phi read the by-name fallback's return + // — neither arm converted, and neither does this one. + // + // The key handle is loaded HERE, in the cold miss block, + // instead of in the entry block: the call is its only + // consumer, so sinking it takes a load, a bitcast and a + // mask off the inline hit path. + let key_raw = { + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, &key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + blk.and(I64, &key_bits, POINTER_MASK_I64) + }; + let val_ic = ctx.block().call( + DOUBLE, + "js_class_field_get_ic", &[ (I64, &site_id), (DOUBLE, &recv_box), @@ -1716,9 +1750,57 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (I32, requires_raw_f64_str), ], ); - let guard_pass = ctx.block().icmp_ne(I32, &guard_ok, "0"); - ctx.block() - .cond_br(&guard_pass, &fast_label, &fallback_label); + let ic_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + if requires_raw_f64 { + // The miss arm's value now arrives from the outlined + // IC instead of the inline by-name call, so the + // dynamic-fallback record follows it. Same contract + // as before — a JS value, not a proven raw double, + // with the raw-f64 layout rejected and invalidated by + // a runtime API (`native_value/verify/raw_f64.rs` + // enforces all three on this consumer). + let fallback = LoweredValue { + semantic: SemanticKind::JsValue, + rep: NativeRep::JsValue, + llvm_ty: DOUBLE, + value: val_ic.clone(), + }; + ctx.record_lowered_value_with_access_mode_and_facts( + "ClassFieldGet", + None, + "js_class_field_get_ic", + &fallback, + Some(BoundsState::Unknown), + None, + Some(BufferAccessMode::DynamicFallback), + Some(MaterializationReason::RuntimeApi), + None, + None, + Vec::new(), + vec![ + raw_f64_layout_fact( + None, + "rejected", + "class_field_get_guard", + Some(MaterializationReason::RuntimeApi), + ), + raw_f64_layout_fact( + None, + "invalidated", + "runtime_api", + Some(MaterializationReason::RuntimeApi), + ), + ], + false, + false, + vec![ + format!("class={}", class_name), + format!("field={}", property), + format!("field_index={}", field_idx_str), + ], + ); + } ctx.current_block = fast_idx; // arm64_32 watchOS: the object fields region begins at @@ -1782,109 +1864,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); } - ctx.current_block = fallback_idx; - // #7153: the guard rejects a nullish receiver along with - // every other shape miss, but a nullish field read must - // throw TypeError per spec — the by-name lookup below - // answers `undefined` and the program keeps running on a - // silent wrong value. Mirror the generic path's check - // (generic_dispatch.rs); the cost sits on the cold - // fallback arm only. - let (is_null, is_nullish) = { - let blk = ctx.block(); - let is_undef = - blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64); - let is_null = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64); - let is_nullish = blk.or(I1, &is_undef, &is_null); - (is_null, is_nullish) - }; - let throw_idx = ctx.new_block("class_field_get.throw_nullish"); - let lookup_idx = ctx.new_block("class_field_get.fallback_lookup"); - let throw_label = ctx.block_label(throw_idx); - let lookup_label = ctx.block_label(lookup_idx); - ctx.block() - .cond_br(&is_nullish, &throw_label, &lookup_label); - - ctx.current_block = throw_idx; - let prop_entry = ctx.strings.entry(key_idx); - let prop_bytes_global = format!("@{}", prop_entry.bytes_global); - let prop_len_str = prop_entry.byte_len.to_string(); - let is_null_i32 = ctx.block().zext(I1, &is_null, I32); - ctx.block().call_void( - "js_throw_type_error_property_access", - &[ - (I32, &is_null_i32), - (PTR, &prop_bytes_global), - (I64, &prop_len_str), - ], - ); - ctx.block().unreachable(); - - ctx.current_block = lookup_idx; - let blk = ctx.block(); - crate::expr::emit_typed_feedback_record_call( - blk, - "js_typed_feedback_record_fallback_call", - &[(I64, &site_id)], - ); - let val_fallback_js = blk.call( - DOUBLE, - "js_object_get_field_by_name_f64", - &[(I64, &obj_bits), (I64, &key_raw)], - ); - let val_fallback = val_fallback_js.clone(); - let fallback_end_label = blk.label.clone(); - blk.br(&merge_label); - if requires_raw_f64 { - let fallback = LoweredValue { - semantic: SemanticKind::JsValue, - rep: NativeRep::JsValue, - llvm_ty: DOUBLE, - value: val_fallback_js.clone(), - }; - ctx.record_lowered_value_with_access_mode_and_facts( - "ClassFieldGet", - None, - "js_object_get_field_by_name_f64", - &fallback, - Some(BoundsState::Unknown), - None, - Some(BufferAccessMode::DynamicFallback), - Some(MaterializationReason::RuntimeApi), - None, - None, - Vec::new(), - vec![ - raw_f64_layout_fact( - None, - "rejected", - "class_field_get_guard", - Some(MaterializationReason::RuntimeApi), - ), - raw_f64_layout_fact( - None, - "invalidated", - "runtime_api", - Some(MaterializationReason::RuntimeApi), - ), - ], - false, - false, - vec![ - format!("class={}", class_name), - format!("field={}", property), - format!("field_index={}", field_idx_str), - ], - ); - } - ctx.current_block = merge_idx; return Ok(ctx.block().phi( DOUBLE, - &[ - (&val_fast, &fast_end_label), - (&val_fallback, &fallback_end_label), - ], + &[(&val_fast, &fast_end_label), (&val_ic, &ic_end_label)], )); } } diff --git a/crates/perry-codegen/src/native_value/verify/raw_f64.rs b/crates/perry-codegen/src/native_value/verify/raw_f64.rs index 51d2644550..0e15791933 100644 --- a/crates/perry-codegen/src/native_value/verify/raw_f64.rs +++ b/crates/perry-codegen/src/native_value/verify/raw_f64.rs @@ -96,6 +96,13 @@ pub(crate) fn raw_f64_dynamic_fallback_record(record: &NativeRepRecord) -> bool | ("PackedI32LoopGuard", "packed_i32_loop_fallback") | ("PackedU32LoopGuard", "packed_u32_loop_fallback") | ("ClassFieldGet", "js_object_get_field_by_name_f64") + // T3 one-exit class-field GET: the inline tower's miss arm is a + // single `js_class_field_get_ic` call instead of the nullish + // diamond + by-name lookup, so the dynamic-fallback record now + // names the IC. The by-name pair stays: it is still the contract + // asserted by `verify/tests.rs`, and dropping an entry rather + // than adding to it is exactly how #8858 silently lost one. + | ("ClassFieldGet", "js_class_field_get_ic") | ("ClassFieldSet", "js_object_set_field_by_name") ) } diff --git a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs index 3961e289d8..1c21de4031 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -1481,6 +1481,7 @@ fn assert_clone_fires_call_free(ir: &str, what: &str) { assert!( !fast.contains("js_object_get_field_by_name_f64") && !fast.contains("js_typed_feedback_class_field_get_guard") + && !fast.contains("js_class_field_get_ic") && !fast.contains("js_number_coerce"), "{what}: the by-name field diamond must be gone from the fast clone" ); diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index b14de19aa9..d4728dd188 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -12843,8 +12843,15 @@ fn typed_f64_receiver_method_clone_raw_loads_after_composed_guards() { // about which emission shape carried it. let inline_probe = caller_ir.find("@PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED"); let method_proof = inline_probe.map_or(method_guard, |p| p.min(method_guard)); + // One-exit class-field GET: at a field-GET site the runtime guard is no + // longer CALLED -- it is the body of `js_class_field_get_ic`, the inline + // pre-check's single miss edge. Other lowerings (the method-override + // receiver-clone gate) still call the guard directly, so accept either + // witness; what this assertion is about is that a field proof exists and + // dominates the typed call, not which symbol carries it. let field_guard = caller_ir - .find("call i32 @js_typed_feedback_class_field_get_guard") + .find("call double @js_class_field_get_ic") + .or_else(|| caller_ir.find("call i32 @js_typed_feedback_class_field_get_guard")) .unwrap_or_else(|| panic!("caller should guard raw-f64 receiver fields:\n{caller_ir}")); // Same for the raw-f64 FIELD proof: one inline class-field precheck // (the field-GET sites' form, first mentioned by its `deref` block in the diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index 3ee8f733eb..6dfb6d049e 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -646,20 +646,28 @@ fn typed_feedback_guards_direct_class_field_specialization() { )); assert!(ir.contains("class_field_set_guard")); - assert!(ir.contains("class_field_get_guard")); assert!(ir.contains("@perry_typed_shape_raw_f64_mask_")); assert!(ir.contains("js_typed_feedback_class_field_set_guard")); - assert!(ir.contains("js_typed_feedback_class_field_get_guard")); assert!(ir.contains("class_field_set.fast")); assert!(ir.contains("class_field_set.fallback")); // #8033: `receiver_class_name` now consults `stable_local_type_proof` // (runtime evidence), which a bare parameter lacks. The numeric-specific // `class_field_get_number.*` path is therefore not selected; the generic - // `class_field_get.*` diamond is emitted instead, with its own fast/fallback - // arms and the typed-feedback guard. The guard assertions above (lines - // 493-499) validate the test's core purpose. + // `class_field_get.*` tower is emitted instead. + // + // One-exit class-field GET: the tower keeps the #5093 inline pre-check and + // the fast slot load, and everything behind the pre-check — the guard call, + // the guard-PASS load, the nullish TypeError and the by-name lookup — is + // now the body of `js_class_field_get_ic`. The guard therefore still runs + // on every miss, one frame deeper; asserting `js_typed_feedback_class_ + // field_get_guard` as a bare substring here would prove nothing either way, + // because the `declare` line alone satisfies it (the same trap #7480's + // comment below describes). Assert the call FORMS instead. assert!(ir.contains("class_field_get.fast")); - assert!(ir.contains("class_field_get.fallback")); + assert!(ir.contains("class_field_inline.deref")); + assert!(ir.contains("call double @js_class_field_get_ic(")); + assert!(!ir.contains("call i32 @js_typed_feedback_class_field_get_guard(")); + assert!(!ir.contains("class_field_get.fallback")); assert!(ir.contains("store double")); assert!(!ir.contains("call void @js_gc_note_slot_layout")); // #5334 lever A: the SET fallback arm collapses to one outlined call; the @@ -670,9 +678,13 @@ fn typed_feedback_guards_direct_class_field_specialization() { // (from the class-field-GET fallback block, not the SET site — the SET copy // was folded into js_class_field_set_fallback by #5334). It is now emitted // only in a typed-feedback build, like the registration call beside it. - // The fallback ARM itself is unchanged and is asserted above/below. + // The fallback ARM itself is asserted above. assert!(!ir.contains("call void @js_typed_feedback_record_fallback_call")); - assert!(ir.contains("call double @js_object_get_field_by_name_f64")); + // The GET miss arm's by-name lookup moved inside `js_class_field_get_ic` + // (which records the fallback call itself, under the same runtime + // `typed_feedback_enabled()` gate the elided emission above respects), so + // the site no longer emits it. + assert!(!ir.contains("call double @js_object_get_field_by_name_f64(")); } /// Body of the first rendered block whose label starts with `label_prefix`, @@ -774,23 +786,27 @@ fn full_outline_ic_collapses_class_field_get_to_single_call() { let _lock = env_lock(); - // Forced ON: one outlined call, no inline get diamond. + // Forced ON: one outlined call and NO tower at all -- not even the #5093 + // inline pre-check, which is the whole point on an oversized module. { let _g = EnvVarGuard::set("PERRY_FULL_OUTLINE_IC", Some("1")); let ir = ir_for(build()); assert!(ir.contains("call double @js_class_field_get_ic")); assert!(!ir.contains("class_field_get.fast")); - assert!(!ir.contains("class_field_get.fallback")); - assert!(!ir.contains("call i32 @js_typed_feedback_class_field_get_guard")); + assert!(!ir.contains("class_field_inline.deref")); + assert!(!ir.contains("call i32 @js_typed_feedback_class_field_get_guard(")); } - // Forced OFF: the inline diamond, no full-outline call. + // Forced OFF: the inline tower. Since the one-exit change, BOTH arms call + // `js_class_field_get_ic`, so the discriminator is the TOWER, not the call: + // OFF keeps the pre-check and the fast slot load, ON has neither. { let _g = EnvVarGuard::set("PERRY_FULL_OUTLINE_IC", Some("0")); let ir = ir_for(build()); - assert!(!ir.contains("call double @js_class_field_get_ic")); + assert!(ir.contains("class_field_inline.deref")); assert!(ir.contains("class_field_get.fast")); - assert!(ir.contains("js_typed_feedback_class_field_get_guard")); + assert!(ir.contains("call double @js_class_field_get_ic")); + assert!(!ir.contains("call i32 @js_typed_feedback_class_field_get_guard(")); } } diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index c12ed2c0c5..4975aa0193 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -3038,3 +3038,148 @@ fn function_source_array_literal_keeps_the_array_index_fast_path_armed() { "the emitted guard must still admit plain arrays to the fast path" ); } + +// --------------------------------------------------------------------------- +// `js_class_field_get_ic` — the class-field GET tower's ONE exit. +// +// The helper predates these tests: it was written for the #5391 path-2 full +// outline, which only fires on oversized modules, so its four arms had no +// direct coverage. Every monomorphic `this.field` read whose inline pre-check +// misses now routes through it, so each arm is pinned here: +// +// guard PASS, boxed slot -> the same value the slot holds; +// guard PASS, raw-f64 slot -> the same double, with the intact bit live; +// nullish receiver -> TypeError, not `undefined` (#7153); +// guard FAIL -> one recorded fallback + the by-name answer. +// +// The pass cases assert the site counters too: a helper that silently took the +// fallback on every call would return the right value and pass a value-only +// test, which is exactly the "gate whose subject never ran" shape. +// --------------------------------------------------------------------------- + +// `guards` is a private submodule and its items are not re-exported through +// `super::*`, so name the helper explicitly. +use crate::typed_feedback::guards::js_class_field_get_ic; + +#[test] +fn class_field_get_ic_reads_the_boxed_slot_on_a_guard_pass() { + let _guard = typed_feedback_test_lock(); + reset_typed_feedback_for_tests(); + register(7401, TypedFeedbackSiteKind::PropertyGet, "obj.x"); + + let class_id = 0x7EED_7401; + let (obj, _, key_x, receiver) = class_instance(class_id, b"x"); + let expected_shape_id = shape_id(obj); + let payload = crate::string::js_string_from_bytes(b"boxed".as_ptr(), 5); + let stored = crate::JSValue::string_ptr(payload); + crate::object::js_object_set_field(obj, 0, stored); + + let got = js_class_field_get_ic(7401, receiver, class_id, expected_shape_id, key_x, 0, 0); + assert_eq!( + got.to_bits(), + stored.bits(), + "a guard PASS must answer with the slot's own NaN-box, bit for bit" + ); + + let site = &typed_feedback_snapshot().sites[0]; + assert_eq!(site.guard_passes, 1); + assert_eq!(site.guard_failures, 0); + assert_eq!( + site.fallback_calls, 0, + "the fast arm must not record a fallback; a helper that always took the \ + by-name path would return the same value" + ); +} + +#[test] +fn class_field_get_ic_reads_the_raw_f64_slot_on_a_guard_pass() { + let _guard = typed_feedback_test_lock(); + reset_typed_feedback_for_tests(); + register(7402, TypedFeedbackSiteKind::PropertyGet, "obj.x"); + + let class_id = 0x7EED_7402; + let (obj, _, key_x, receiver) = class_instance(class_id, b"x"); + let expected_shape_id = shape_id(obj); + crate::object::js_object_set_field(obj, 0, crate::JSValue::number(5.25)); + let raw_mask = [0b1u64]; + crate::gc::js_gc_init_typed_shape_layout( + obj as u64, + 1, + raw_mask.as_ptr(), + raw_mask.len() as u32, + std::ptr::null(), + 0, + ); + + let got = js_class_field_get_ic(7402, receiver, class_id, expected_shape_id, key_x, 0, 1); + assert_eq!( + got, 5.25, + "a `require_raw_f64` PASS must answer with the raw double the codegen \ + fast block would have loaded" + ); + + let site = &typed_feedback_snapshot().sites[0]; + assert_eq!(site.guard_passes, 1); + assert_eq!(site.fallback_calls, 0); +} + +#[test] +fn class_field_get_ic_records_the_fallback_and_answers_by_name_on_a_guard_fail() { + let _guard = typed_feedback_test_lock(); + reset_typed_feedback_for_tests(); + register(7403, TypedFeedbackSiteKind::PropertyGet, "obj.x"); + + let class_id = 0x7EED_7403; + let (obj, original_keys, key_x, receiver) = class_instance(class_id, b"x"); + let expected_shape_id = shape_id(obj); + crate::object::js_object_set_field(obj, 0, crate::JSValue::from_bits(5.0f64.to_bits())); + + // Add a key: the shape moves, so the cached (class id, shape id) pair the + // site was compiled against no longer describes this receiver. + let key_y = crate::string::js_string_from_bytes(b"y".as_ptr(), 1); + crate::object::js_object_set_field_by_name(obj, key_y, 10.0); + assert_ne!( + unsafe { crate::object::object_keys_array(obj) }, + original_keys, + "the fixture must actually transition the shape, or the FAIL arm never runs" + ); + + let got = js_class_field_get_ic(7403, receiver, class_id, expected_shape_id, key_x, 0, 0); + assert_eq!( + got.to_bits(), + 5.0f64.to_bits(), + "the FAIL arm must answer by name, not from the stale cached slot" + ); + + let site = &typed_feedback_snapshot().sites[0]; + assert_eq!(site.guard_passes, 0); + assert_eq!(site.guard_failures, 1); + assert_eq!( + site.fallback_calls, 1, + "the helper records the fallback itself — codegen no longer emits the \ + `js_typed_feedback_record_fallback_call` beside it" + ); +} + +#[test] +fn class_field_get_ic_throws_a_type_error_on_a_nullish_receiver() { + let _guard = typed_feedback_test_lock(); + reset_typed_feedback_for_tests(); + register(7404, TypedFeedbackSiteKind::PropertyGet, "obj.x"); + + let class_id = 0x7EED_7404; + let (obj, _, key_x, _) = class_instance(class_id, b"x"); + let expected_shape_id = shape_id(obj); + + for nullish in [crate::value::TAG_UNDEFINED, crate::value::TAG_NULL] { + let receiver = f64::from_bits(nullish); + let threw = catch_runtime_throw(|| { + js_class_field_get_ic(7404, receiver, class_id, expected_shape_id, key_x, 0, 0); + }); + assert!( + threw, + "a field read on a nullish receiver must throw TypeError, not \ + answer `undefined` through the by-name lookup (#7153); bits={nullish:#x}" + ); + } +} diff --git a/crates/perry/tests/issue_8774_argument_shape_clones.rs b/crates/perry/tests/issue_8774_argument_shape_clones.rs index 0c172b5f0c..d36b77b8bd 100644 --- a/crates/perry/tests/issue_8774_argument_shape_clones.rs +++ b/crates/perry/tests/issue_8774_argument_shape_clones.rs @@ -285,7 +285,8 @@ fn stable_argument_clones_are_direct_reported_and_moving_gc_safe() { ); assert!( !clone_body.contains("shape_descriptor_by_id") - && !clone_body.contains("js_typed_feedback_class_field_get_guard"), + && !clone_body.contains("js_typed_feedback_class_field_get_guard") + && !clone_body.contains("js_class_field_get_ic"), "{clone} rebuilt a field IC diamond:\n{clone_body}" ); assert!(ir.contains(&format!("call double @{clone}(")));