diff --git a/changelog.d/11857-regfix-store-constfn-values.md b/changelog.d/11857-regfix-store-constfn-values.md new file mode 100644 index 0000000000..d2a0745974 --- /dev/null +++ b/changelog.d/11857-regfix-store-constfn-values.md @@ -0,0 +1,7 @@ +A static-key store site whose value can never be a closure (a literal, an +operator's primitive result, an object or array literal) no longer emits the +ConstFn lane admission (#11798) on its hit and key-add paths: a flagged lane +takes the miss, exactly as a failed admission does. A store of `v + 1` or of a +string literal is back to its size before the ConstFn lanes (957 and 861 +bytes against 1,311 and 1,208), which most of TypeScript's enum and +initializer stores are. diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 8fa7b90cad..4dd5465b79 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -296,6 +296,7 @@ pub(crate) fn lower_put_value_property_set_by_name( assignment_strict: bool, ) -> Result { super::store_census::bump(ctx, super::store_census::BY_NAME_PUT_VALUE); + let value_may_be_closure = !super::put_value_store_ic::value_never_closure(value); rooting::with_operands_rooted_across( ctx, &[object], @@ -340,6 +341,7 @@ pub(crate) fn lower_put_value_property_set_by_name( // `[[Set]]` throws it (`js_put_value_set`: node's wording, and // before `Throw` is consulted), so no guard is emitted here. assignment_strict, + value_may_be_closure, ); Ok(result) }, diff --git a/crates/perry-codegen/src/expr/put_value_store_ic.rs b/crates/perry-codegen/src/expr/put_value_store_ic.rs index 31659ae3ae..3c9ac62ff1 100644 --- a/crates/perry-codegen/src/expr/put_value_store_ic.rs +++ b/crates/perry-codegen/src/expr/put_value_store_ic.rs @@ -211,6 +211,12 @@ fn straight_line_site_outlined(ctx: &FnCtx<'_>) -> bool { /// /// A nullish receiver fails the receiver test, and the miss entry's `[[Set]]` /// throws its TypeError, so the hit path pays nothing for it. +/// +/// `value_may_be_closure` is false when the stored value's expression can +/// never evaluate to a closure ([`value_never_closure`]): a ConstFn lane then +/// admits nothing it could store, so its admission arms are not emitted and a +/// flagged lane takes the miss, exactly as a failed admission would. +#[allow(clippy::too_many_arguments)] pub(crate) fn emit_static_store_ic( ctx: &mut FnCtx<'_>, obj_box: &str, @@ -218,6 +224,7 @@ pub(crate) fn emit_static_store_ic( value_double: &str, value_bits: &str, strict: bool, + value_may_be_closure: bool, ) -> String { let key_idx = ctx.strings.intern(property); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); @@ -485,15 +492,19 @@ pub(crate) fn emit_static_store_ic( let f64_slot = ctx.block().icmp_slt(I64, &word, "0"); let f64_idx = ctx.new_block(&format!("{STORE_IC_STEM}.hit.f64")); let f64_label = ctx.block_label(f64_idx); - let constfn_idx = ctx.new_block(&format!("{STORE_IC_STEM}.hit.constfn")); - let constfn_label = ctx.block_label(constfn_idx); - ctx.block().cond_br(&f64_slot, &f64_label, &constfn_label); + if value_may_be_closure { + let constfn_idx = ctx.new_block(&format!("{STORE_IC_STEM}.hit.constfn")); + let constfn_label = ctx.block_label(constfn_idx); + ctx.block().cond_br(&f64_slot, &f64_label, &constfn_label); + ctx.current_block = constfn_idx; + emit_constfn_value_check(ctx, &packed_ref, value_bits, &store_label, &miss_label); + } else { + ctx.block().cond_br(&f64_slot, &f64_label, &miss_label); + } ctx.current_block = f64_idx; let exponent = ctx.block().and(I64, value_bits, F64_EXP_MASK); let boxed = ctx.block().icmp_eq(I64, &exponent, F64_EXP_MASK); ctx.block().cond_br(&boxed, &miss_label, &store_label); - ctx.current_block = constfn_idx; - emit_constfn_value_check(ctx, &packed_ref, value_bits, &store_label, &miss_label); // The store, then the GC's obligations for the bits actually stored. ctx.current_block = store_idx; @@ -536,6 +547,7 @@ pub(crate) fn emit_static_store_ic( value_bits, &miss_label, &merge_label, + value_may_be_closure, ); ctx.current_block = miss_idx; @@ -612,6 +624,7 @@ fn emit_key_add_hit( value_bits: &str, miss_label: &str, merge_label: &str, + value_may_be_closure: bool, ) -> String { let rep_idx = ctx.new_block(&format!("{ADD_STEM}.rep")); let obj_idx = ctx.new_block(&format!("{ADD_STEM}.object")); @@ -652,15 +665,19 @@ fn emit_key_add_hit( let f64_slot = ctx.block().icmp_eq(I64, &flags, &ADD_F64_SLOT.to_string()); let f64_idx = ctx.new_block(&format!("{ADD_STEM}.f64")); let f64_label = ctx.block_label(f64_idx); - let constfn_idx = ctx.new_block(&format!("{ADD_STEM}.constfn")); - let constfn_label = ctx.block_label(constfn_idx); - ctx.block().cond_br(&f64_slot, &f64_label, &constfn_label); + if value_may_be_closure { + let constfn_idx = ctx.new_block(&format!("{ADD_STEM}.constfn")); + let constfn_label = ctx.block_label(constfn_idx); + ctx.block().cond_br(&f64_slot, &f64_label, &constfn_label); + ctx.current_block = constfn_idx; + emit_constfn_value_check(ctx, packed_ref, value_bits, &obj_label, miss_label); + } else { + ctx.block().cond_br(&f64_slot, &f64_label, miss_label); + } ctx.current_block = f64_idx; let exponent = ctx.block().and(I64, value_bits, F64_EXP_MASK); let boxed = ctx.block().icmp_eq(I64, &exponent, F64_EXP_MASK); ctx.block().cond_br(&boxed, miss_label, &obj_label); - ctx.current_block = constfn_idx; - emit_constfn_value_check(ctx, packed_ref, value_bits, &obj_label, miss_label); // The GcHeader's first word (obj_type | gc_flags << 8 | _reserved << 16). // No receiver-kind admission: the memo's pre-shape is an `Ordinary` shape @@ -983,3 +1000,40 @@ fn emit_key_handle(ctx: &mut FnCtx<'_>, key_handle_global: &str) -> String { let key_bits = blk.bitcast_double_to_i64(&key_box); blk.and(I64, &key_bits, crate::nanbox::POINTER_MASK_I64) } + +/// Can `value` never evaluate to a closure? Literals, operators whose result +/// is a primitive, fresh object and array literals, and a conditional or +/// logical expression all of whose possible results are such values. Anything +/// else (a read, a call, `new`, a function expression) may be one. +pub(crate) fn value_never_closure(value: &perry_hir::Expr) -> bool { + use perry_hir::Expr; + match value { + Expr::Undefined + | Expr::Null + | Expr::Bool(_) + | Expr::Number(_) + | Expr::Integer(_) + | Expr::BigInt(_) + | Expr::String(_) + | Expr::WtfString(_) + | Expr::Binary { .. } + | Expr::Unary { .. } + | Expr::Compare { .. } + | Expr::Update { .. } + | Expr::TypeOf(_) + | Expr::Void(_) + | Expr::InstanceOf { .. } + | Expr::In { .. } + | Expr::Object(_) + | Expr::Array(_) => true, + Expr::Logical { left, right, .. } => { + value_never_closure(left) && value_never_closure(right) + } + Expr::Conditional { + then_expr, + else_expr, + .. + } => value_never_closure(then_expr) && value_never_closure(else_expr), + _ => false, + } +} diff --git a/crates/perry-codegen/src/expr/write_pic_barrier_tests.rs b/crates/perry-codegen/src/expr/write_pic_barrier_tests.rs index 17fc8e49e9..7c834ebed1 100644 --- a/crates/perry-codegen/src/expr/write_pic_barrier_tests.rs +++ b/crates/perry-codegen/src/expr/write_pic_barrier_tests.rs @@ -737,3 +737,47 @@ fn dyn_ic_inline_store_keeps_its_semantic_fallback_for_reference_values() { "the receiver guard must still fall through to the outlined helper:\n{ir}" ); } + +/// A stored value that can never be a closure (a literal, an operator's +/// primitive result) has nothing a ConstFn lane could admit, so the site emits +/// no ConstFn admission: a flagged lane takes the miss, as a failed admission +/// would. A value that may be a closure keeps it. +#[test] +fn store_ic_emits_no_constfn_admission_for_a_value_that_cannot_be_a_closure() { + let constfn_blocks = [ + "put.pic.hit.constfn", + "put.add.constfn", + "put.pic.constfn.header", + ]; + for (name, value) in [ + ("store_ic_constfn_number", Expr::Number(1.0)), + ("store_ic_constfn_string", Expr::String("s".to_string())), + ( + "store_ic_constfn_sum", + Expr::Binary { + op: perry_hir::BinaryOp::Add, + left: Box::new(Expr::LocalGet(VALUE)), + right: Box::new(Expr::Number(1.0)), + }, + ), + ] { + let ir = write_pic_ir(name, value); + for stem in constfn_blocks { + assert!( + block(&ir, stem).is_none(), + "{name}: `{stem}` must not be emitted for a value that is never a closure:\n{ir}" + ); + } + assert!( + block(&ir, "put.pic.hit.f64").is_some(), + "{name}: the F64 lane check stays:\n{ir}" + ); + } + let ir = write_pic_ir("store_ic_constfn_any", Expr::LocalGet(VALUE)); + for stem in constfn_blocks { + assert!( + block(&ir, stem).is_some(), + "a value that may be a closure keeps `{stem}`:\n{ir}" + ); + } +}