diff --git a/changelog.d/11872-dynarr-one-decline.md b/changelog.d/11872-dynarr-one-decline.md new file mode 100644 index 0000000000..5cf238fa96 --- /dev/null +++ b/changelog.d/11872-dynarr-one-decline.md @@ -0,0 +1,4 @@ +An untyped `obj[i] = v` store declines from both its typed-array tier and its +Array tier to one shared runtime block, so the admitted-`Uint8Array` byte arm +and the complete `[[Set]]` call are emitted once per site instead of once per +tier. The hit paths are unchanged; the compiled tsc workload drops 0.27 MB. diff --git a/changelog.d/11872-getter-arm-names.md b/changelog.d/11872-getter-arm-names.md new file mode 100644 index 0000000000..134e4b8eb7 --- /dev/null +++ b/changelog.d/11872-getter-arm-names.md @@ -0,0 +1,5 @@ +A static-key read site emits the class-getter arm (#10498) only for a property +name that some compiled class of the program declares as a getter, as the store +site already does for setters. The runtime admits an entry only for a declared +accessor, so every other read carried an arm it could never take. The compiled +tsc workload drops 7.8 MB (181.5 MB to 173.7 MB). diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index ccf51ce98e..bb55233bd8 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -123,12 +123,12 @@ pub fn namespace_member_func_key(namespace: &str, member: &str) -> String { /// The property names the program's compiled classes declare as accessors /// (`get k()` / `set k(v)`), across every module (#10498). /// -/// A store site's class-setter arm can only ever take an entry for a name some -/// compiled class declares as a setter: the runtime admits an entry only when -/// the receiver's class chain declares the accessor -/// (`class_chain_has_instance_accessor`). A store whose name no class declares -/// therefore emits no arm; it misses to the runtime as before the arm existed, -/// which still asks the same entry first. The getters are collected alongside. +/// A read site's class-getter arm and a store site's class-setter arm can only +/// ever take an entry for a name some compiled class declares as that kind of +/// accessor: the runtime admits an entry only when the receiver's class chain +/// declares the accessor (`class_chain_has_instance_accessor`). A site whose +/// name no class declares therefore emits no arm; it misses to the runtime as +/// before the arm existed, which still asks the same entry first. #[derive(Debug, Clone, Default)] pub struct ClassAccessorNames { getters: std::collections::HashSet, @@ -362,9 +362,9 @@ pub struct CompileOptions { pub object_literal_method_candidates: std::sync::Arc>>, /// The whole program's class accessor names ([`ClassAccessorNames`]), - /// which decide where the class-setter arms are emitted. `None` (a - /// standalone or test compile that did not collect them) emits the arm - /// at every store site. + /// which decide where the class-accessor arms are emitted. `None` (a + /// standalone or test compile that did not collect them) emits the arms + /// at every site. pub program_class_accessor_names: Option>, /// Imported enum member lists, keyed by the local name under which /// the enum is visible in this module. diff --git a/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs b/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs index 311b67e779..8cc0693704 100644 --- a/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs +++ b/crates/perry-codegen/src/expr/barrier_stem_census_tests.rs @@ -1148,3 +1148,38 @@ fn identical_labels_in_other_functions_cannot_validate_a_sabotaged_gate() { } } } + +/// The untyped store's two tiers (#5525 typed array, #10513 Array) decline to +/// ONE runtime block: the admitted-`Uint8Array` byte arm and the complete +/// `[[Set]]` are emitted once per site, not once per declining tier. +#[test] +fn the_untyped_store_carries_one_decline_for_both_tiers() { + let ir = dynarr_set_ir(); + let label_defs = |stem: &str| { + ir.lines() + .filter(|line| { + line.strip_prefix(stem) + .and_then(|rest| rest.strip_prefix('.')) + .and_then(|rest| rest.split(':').next()) + .is_some_and(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) + }) + .count() + }; + let sites = label_defs("dynarr.ta"); + assert!(sites > 0, "the probe must emit the untyped store:\n{ir}"); + assert_eq!(label_defs("dynarr.slow"), sites, "{ir}"); + assert_eq!( + label_defs("u8d.set.chk"), + sites, + "one byte arm per site, shared by both tiers:\n{ir}" + ); + assert_eq!( + ir.lines() + .filter( + |line| !line.starts_with("declare") && line.contains("@js_dyn_index_set_strict(") + ) + .count(), + sites, + "one complete [[Set]] per site:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/expr/index_set_typed_array.rs b/crates/perry-codegen/src/expr/index_set_typed_array.rs index e13a5734f5..a481135936 100644 --- a/crates/perry-codegen/src/expr/index_set_typed_array.rs +++ b/crates/perry-codegen/src/expr/index_set_typed_array.rs @@ -80,11 +80,17 @@ pub(super) fn lower_inline_dyn_typed_array_set( // #5525 kind cache (the only way that tier's fast arm is reachable) goes // there on one load and compare, so typed-array stores pay nothing for the // Array arm, and an Array pays only that compare for the typed-array one. + // + // Both tiers decline to ONE runtime block: the complete dynamic `[[Set]]` + // behind its admitted-`Uint8Array` byte arm is the same code whichever + // tier declined, so the site carries it once. let ta_idx = ctx.new_block("dynarr.ta"); let array_idx = ctx.new_block("dynarr.array"); + let slow_idx = ctx.new_block("dynarr.slow"); let done_idx = ctx.new_block("dynarr.done"); let ta_label = ctx.block_label(ta_idx); let array_label = ctx.block_label(array_idx); + let slow_label = ctx.block_label(slow_idx); let done_label = ctx.block_label(done_idx); { let blk = ctx.block(); @@ -105,7 +111,7 @@ pub(super) fn lower_inline_dyn_typed_array_set( blk.cond_br(&cached_typed_array, &ta_label, &array_label); } ctx.current_block = ta_idx; - let _ = emit_inline_ta_set_then_runtime(ctx, obj_box, idx_d, val_double, strict); + emit_inline_ta_set(ctx, obj_box, idx_d, val_double, Some(&slow_label)); ctx.block().br(&done_label); ctx.current_block = array_idx; @@ -119,11 +125,14 @@ pub(super) fn lower_inline_dyn_typed_array_set( facts.write_barrier_needed, facts.value_is_numeric, |ctx| { - emit_dyn_index_set_runtime(ctx, obj_box, idx_d, val_double, strict); + ctx.block().br(&slow_label); Ok(()) }, )?; ctx.block().br(&done_label); + ctx.current_block = slow_idx; + emit_dyn_index_set_runtime(ctx, obj_box, idx_d, val_double, strict); + ctx.block().br(&done_label); ctx.current_block = done_idx; Ok(val_double.to_string()) } @@ -184,17 +193,57 @@ fn emit_inline_ta_set_then_runtime( val_double: &str, strict: bool, ) -> String { + emit_inline_ta_set(ctx, obj_box, idx_d, val_double, None) + .expect("a typed-array store without a shared decline emits its own") + .emit(ctx, obj_box, idx_d, val_double, strict); + val_double.to_string() +} + +/// The decline block [`emit_inline_ta_set`] opened for itself, still to be +/// filled with the runtime store. +struct OwnDecline { + slow_idx: usize, + merge_idx: usize, +} + +impl OwnDecline { + fn emit(self, ctx: &mut FnCtx<'_>, obj_box: &str, idx_d: &str, val_double: &str, strict: bool) { + // ---- slow: preserve the source function's assignment strictness ---- + let merge_label = ctx.block_label(self.merge_idx); + ctx.current_block = self.slow_idx; + emit_dyn_index_set_runtime(ctx, obj_box, idx_d, val_double, strict); + ctx.block().br(&merge_label); + ctx.current_block = self.merge_idx; + } +} + +/// The #5525 guarded inline typed-array store. Every guard miss branches to +/// `decline`, or, with none given, to a decline block of its own that the +/// returned [`OwnDecline`] fills. Leaves the current block at the store's +/// merge (with a shared decline) where the assignment's value is +/// `val_double` on every path. +fn emit_inline_ta_set( + ctx: &mut FnCtx<'_>, + obj_box: &str, + idx_d: &str, + val_double: &str, + decline: Option<&str>, +) -> Option { let tag_mask = crate::nanbox::i64_literal(crate::nanbox::TAG_MASK); let pointer_tag = crate::nanbox::POINTER_TAG_I64; let pointer_mask = crate::nanbox::POINTER_MASK_I64; let fast_idx = ctx.new_block("tav.set.fast"); let store_idx = ctx.new_block("tav.set.store"); - let slow_idx = ctx.new_block("tav.set.slow"); + let own_slow_idx = decline.is_none().then(|| ctx.new_block("tav.set.slow")); let merge_idx = ctx.new_block("tav.set.merge"); let fast_label = ctx.block_label(fast_idx); let store_label = ctx.block_label(store_idx); - let slow_label = ctx.block_label(slow_idx); + let slow_label = match (own_slow_idx, decline) { + (Some(idx), _) => ctx.block_label(idx), + (None, Some(label)) => label.to_string(), + (None, None) => unreachable!("a decline block is opened when none is given"), + }; let merge_label = ctx.block_label(merge_idx); // ---- entry: combined cache/kind/range guard -> fast | slow ---- @@ -415,16 +464,14 @@ fn emit_inline_ta_set_then_runtime( blk.br(&merge_label); } - // ---- slow: preserve the source function's assignment strictness ---- - ctx.current_block = slow_idx; - emit_dyn_index_set_runtime(ctx, obj_box, idx_d, val_double, strict); - ctx.block().br(&merge_label); - // ---- merge: assignment yields the stored value on every path ---- - ctx.current_block = merge_idx; // All paths produce `val_double` as the expression result (matching // `js_dyn_index_set`'s `return value`), so no phi is needed. - val_double.to_string() + ctx.current_block = merge_idx; + own_slow_idx.map(|slow_idx| OwnDecline { + slow_idx, + merge_idx, + }) } /// Emit one per-kind integer typed-array element store block for diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index b08bc8b2a5..75b62a4306 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2680,6 +2680,13 @@ mod inline_cache_name_tests { } impl<'a> FnCtx<'a> { + /// May some compiled class of the program declare a getter named `name`? + /// Where this is false a read site emits no class-getter arm. + pub(crate) fn program_may_declare_getter(&self, name: &str) -> bool { + self.program_class_accessor_names + .is_none_or(|names| names.may_get(name)) + } + /// May some compiled class of the program declare a setter named `name`? /// Where this is false a store site emits no class-setter arm. pub(crate) fn program_may_declare_setter(&self, name: &str) -> bool { diff --git a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs index d6f2b7dfe5..53f1ea7b18 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -960,11 +960,14 @@ pub(crate) fn lower_generic_property_get( // #10498: a site whose reads inherit a compiled class getter answers them // inline, ahead of the front (`accessor_arm`). 64-bit targets only, as the // method site: the entry's words address 8-byte slots behind the header. + // Only for a name some compiled class of the program declares as a getter: + // no other site can ever take an entry. let accessor_entry = match fused_recv.as_ref() { Some(f) if array_length_arm.is_none() && front_idx.is_some() - && accessor_arm_target(ctx.target_triple) => + && accessor_arm_target(ctx.target_triple) + && ctx.program_may_declare_getter(property) => { Some((ctx.new_block("pic.acc.empty"), f.biased.clone())) } diff --git a/crates/perry-codegen/src/expr/property_get/tests.rs b/crates/perry-codegen/src/expr/property_get/tests.rs index 95806a76d9..c3a86e4c24 100644 --- a/crates/perry-codegen/src/expr/property_get/tests.rs +++ b/crates/perry-codegen/src/expr/property_get/tests.rs @@ -1834,13 +1834,12 @@ fn the_generic_slow_read_is_called_only_after_the_front_declines() { #[path = "array_length_tests.rs"] mod array_length; -/// The #10498 class-setter arm only where a compiled class of the program -/// declares a setter of the store's name: the runtime admits an entry only for -/// a declared accessor (`class_chain_has_instance_accessor`), so any other -/// site's arm is code that can never be taken and work on every miss. The -/// read site's getter arm is not gated. +/// The #10498 class-accessor arms only where a compiled class of the program +/// may declare the accessor: the runtime admits an entry only for a declared +/// accessor (`class_chain_has_instance_accessor`), so any other site's arm is +/// code that can never be taken and work on every miss. #[test] -fn class_setter_arms_are_emitted_only_for_declared_setter_names() { +fn class_accessor_arms_are_emitted_only_for_declared_accessor_names() { use crate::ClassAccessorNames; fn module_storing(property: &str) -> Module { let mut m = module_reading(property); @@ -1859,23 +1858,29 @@ fn class_setter_arms_are_emitted_only_for_declared_setter_names() { }; let read_arm = "pic.acc.empty"; let store_arm = "put.pic.acc"; - // Names not collected (a standalone compile): the store keeps its arm. + // Names not collected (a standalone compile): every site keeps its arms. let unknown = emit(None); + assert!(unknown.contains(read_arm), "{unknown}"); assert!(unknown.contains(store_arm), "{unknown}"); - // No class declares a setter `price` (a getter alone does not count). - let getter_only = emit(Some(ClassAccessorNames::from_names( - ["price".to_string()], + // No class declares `price`: neither arm. + let other = emit(Some(ClassAccessorNames::from_names( + ["total".to_string()], ["total".to_string()], ))); - assert!(!getter_only.contains(store_arm), "{getter_only}"); - assert!( - getter_only.contains(read_arm), - "the read arm is not gated:\n{getter_only}" - ); - // A declared setter keeps the store arm. + assert!(!other.contains(read_arm), "{other}"); + assert!(!other.contains(store_arm), "{other}"); + // A getter only: the read arm, not the store arm. + let getter = emit(Some(ClassAccessorNames::from_names( + ["price".to_string()], + Vec::new(), + ))); + assert!(getter.contains(read_arm), "{getter}"); + assert!(!getter.contains(store_arm), "{getter}"); + // A setter only: the store arm, not the read arm. let setter = emit(Some(ClassAccessorNames::from_names( Vec::new(), ["price".to_string()], ))); + assert!(!setter.contains(read_arm), "{setter}"); assert!(setter.contains(store_arm), "{setter}"); }