From a528bcbbdb7cfb160a81141538a68a666c9344a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 18:35:24 +0200 Subject: [PATCH 1/5] perf(codegen): gate the class-field write barrier on the parent's generation, and let a hot recursive function inline its bump allocator --- crates/perry-codegen/src/codegen/function.rs | 4 + crates/perry-codegen/src/codegen/mod.rs | 4 + crates/perry-codegen/src/codegen/opts.rs | 6 + .../src/collectors/hot_callees.rs | 84 ++++ crates/perry-codegen/src/collectors/mod.rs | 2 +- .../src/expr/class_field_barrier_tests.rs | 362 ++++++++++++++++++ crates/perry-codegen/src/expr/mod.rs | 2 + .../perry-codegen/src/expr/write_barrier.rs | 54 ++- crates/perry-codegen/src/function.rs | 7 + .../perry-codegen/src/lower_call/new_alloc.rs | 23 +- 10 files changed, 542 insertions(+), 6 deletions(-) create mode 100644 crates/perry-codegen/src/expr/class_field_barrier_tests.rs diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 839555b1a0..f92aa67787 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -434,6 +434,10 @@ pub(super) fn compile_function( // threshold move" — an `alwaysinline` callee is excluded from the latter // and is the hottest possible case for the former. lf.hot_loop_callee = cross_module.hot_loop_callees.contains(&f.id); + // #7864: the allocator's hotness set. Set from the same well-ordered point + // as `hot_loop_callee` (before the entry block exists and before any + // expression is lowered), for the same reason. + lf.alloc_hot = cross_module.alloc_hot_functions.contains(&f.id); if !lf.force_inline && inline_hot_small_enabled() && (INLINE_HOT_SMALL_MIN..=inline_hot_small_size_cap()).contains(&f.body.len()) diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 08f012d09b..190b74d063 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1706,6 +1706,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> hir, crate::codegen::helpers::inline_hot_small_max_call_sites(), ), + // #7864: the allocator's own "is this hot" set — same in-loop proxy, + // no call-site cap (the cap prices `inlinehint`'s duplication, which + // the inline bump allocator does not incur), plus direct recursion. + alloc_hot_functions: crate::collectors::collect_alloc_hot_functions(hir), clamp3_functions: hir .functions .iter() diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index 46784af7cd..1190479ae9 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -976,4 +976,10 @@ pub(crate) struct CrossModuleCtx { /// `PERRY_INLINE_HOT_SMALL` is off (the flag is checked at the decision /// site, so the set is still populated but simply not consulted). pub hot_loop_callees: std::collections::HashSet, + /// #7864: `FuncId`s in THIS module whose `new` sites earn the inline bump + /// allocator — `collectors::collect_alloc_hot_functions`. Deliberately a + /// SECOND set rather than a widening of `hot_loop_callees`: that one gates + /// `inlinehint`, whose cost scales with call sites, and the two must not + /// share an admission rule. See the collector's doc comment. + pub alloc_hot_functions: std::collections::HashSet, } diff --git a/crates/perry-codegen/src/collectors/hot_callees.rs b/crates/perry-codegen/src/collectors/hot_callees.rs index 7bbc4f63bc..9bec3970cd 100644 --- a/crates/perry-codegen/src/collectors/hot_callees.rs +++ b/crates/perry-codegen/src/collectors/hot_callees.rs @@ -99,6 +99,90 @@ pub fn collect_hot_loop_callees(hir: &Module, max_call_sites: u32) -> HashSet HashSet { + let mut scan = HotCalleeScan::default(); + walk_stmts(&hir.init, false, &mut scan); + for f in &hir.functions { + walk_stmts(&f.body, false, &mut scan); + } + for c in &hir.classes { + if let Some(ctor) = &c.constructor { + walk_stmts(&ctor.body, false, &mut scan); + } + for m in c.methods.iter().chain(c.static_methods.iter()) { + walk_stmts(&m.body, false, &mut scan); + } + for (_, g) in &c.getters { + walk_stmts(&g.body, false, &mut scan); + } + for (_, s) in &c.setters { + walk_stmts(&s.body, false, &mut scan); + } + for cm in &c.computed_members { + walk_expr(&cm.key_expr, false, &mut scan); + walk_stmts(&cm.function.body, false, &mut scan); + } + for field in c.fields.iter().chain(c.static_fields.iter()) { + if let Some(key) = &field.key_expr { + walk_expr(key, false, &mut scan); + } + if let Some(init) = &field.init { + walk_expr(init, false, &mut scan); + } + } + } + let mut hot = scan.in_loop; + // Rule 2: a direct self-call. Scanned per function so the recursion is + // attributed to the caller it actually appears in, which a whole-module + // call-count table cannot express. + for f in &hir.functions { + let mut self_scan = HotCalleeScan::default(); + walk_stmts(&f.body, false, &mut self_scan); + if self_scan.call_counts.contains_key(&f.id) { + hot.insert(f.id); + } + } + hot +} + fn record_callee(callee: &Expr, in_loop: bool, scan: &mut HotCalleeScan) { if let Expr::FuncRef(id) = callee { *scan.call_counts.entry(*id).or_insert(0) += 1; diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index a370c03c50..4390749942 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -63,7 +63,7 @@ pub(crate) use hir_facts::{ collect_native_region_fact_graph, collect_native_region_fact_graph_with_spec_lens, NativeRegionFactGraph, }; -pub(crate) use hot_callees::collect_hot_loop_callees; +pub(crate) use hot_callees::{collect_alloc_hot_functions, collect_hot_loop_callees}; pub(crate) use i32_locals::{ collect_integer_let_ids, collect_localset_ids_in_stmts, is_strictly_i32_bounded_expr, is_ushr_zero, diff --git a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs new file mode 100644 index 0000000000..136519d2b7 --- /dev/null +++ b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs @@ -0,0 +1,362 @@ +//! #7864: the class-field store's remembered-set write barrier sits behind a +//! LIVE test of the parent's generation. +//! +//! The subject is the tail of +//! [`super::write_barrier::emit_jsvalue_slot_store_pointer_tested`]. #7511 put +//! that store's three bookkeeping calls behind one live test of the stored +//! VALUE ("does this publish a heap pointer at all"); this adds the other half +//! of the question the barrier itself asks ("is the parent old enough for +//! anyone to care"), which is where every object-literal constructor lives — +//! the instance was allocated in the nursery a few instructions earlier. +//! +//! ## What these tests have to pin, and why each direction +//! +//! A predicate that silently never fires still compiles and still prints the +//! right answer; the program just stays slow. A predicate wired the WRONG way +//! round also compiles and prints the right answer *until a minor GC lands in +//! the window*, and then strands a live child. So the census asserts three +//! things a label-presence check cannot separate: +//! +//! 1. **The gate is REACHED** — there is a `cond_br` INTO +//! `class_field_set.barrier`, not merely a block with that name. (CLAUDE.md, +//! "a gate must assert its subject was live"; #7690 is the precedent where +//! an optimization was silently deleted while every label survived.) +//! 2. **The condition is the real one** — the block that branches loads +//! `gc_flags` and masks `GC_FLAG_TENURED`, and reads +//! `@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT`. Hard-wiring the claim +//! (`false`, or a constant, or dropping the incremental disjunct) fails +//! here. That is the sabotage this file is verified against. +//! 3. **The barrier is on the RIGHT edge and still REACHABLE** — the `true` +//! successor is the barrier block, the `false` successor is the join, and +//! `js_write_barrier_slot` is still emitted inside. This is a guard, never +//! an elision: a receiver that IS tenured takes the call exactly as before, +//! which is what makes the change a scheduling decision rather than a +//! semantic one. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{ + Class, ClassField, Expr, Function, Module, ModuleInitKind, Param, Stmt, +}; + +/// The block that exists only when the #7864 gate was emitted. +const BARRIER_BLOCK: &str = "class_field_set.barrier"; +/// `GC_FLAG_TENURED` as the emitted `and i8` mask. +const TENURED_MASK: &str = "and i8"; +const TENURED_VALUE: &str = ", 32"; +const INCREMENTAL_GLOBAL: &str = "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT"; +const BARRIER_CALL: &str = "call void @js_write_barrier_slot"; + +/// These tests describe DEFAULT barrier emission. `PERRY_WRITE_BARRIERS=0` +/// removes every barrier and would make all of them vacuously "pass" the +/// negative half while the positive half fails for the wrong reason. +fn assert_default_barrier_env_not_disabled() { + assert!( + !matches!( + std::env::var("PERRY_WRITE_BARRIERS").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ), + "these tests describe DEFAULT barrier emission; PERRY_WRITE_BARRIERS must be unset or on" + ); +} + +fn ir_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: true, + 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(), + namespace_member_nested: 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, + } +} + +const PARAM_ID: u32 = 7; + +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(), + } +} + +/// `constructor(v) { this.v = v }` — the shape HIR synthesizes for every +/// closed-shape object literal (`lower/context.rs::mint_anon_shape_class`), so +/// this is `{ kind: "num", num: n }` after lowering, not a contrived fixture. +/// The parameter is `Any`, which is what makes the value's pointer-ness +/// undecidable statically and puts the store on the #7511 live-test tier at +/// all. +fn param_prologue_ctor() -> Function { + Function { + id: 90, + name: "constructor".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: PARAM_ID, + name: "v".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Void, + body: vec![Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "v".to_string(), + value: Box::new(Expr::LocalGet(PARAM_ID)), + })], + 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 boxed_class() -> Class { + Class { + id: 2, + name: "Boxed".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![field("v", Type::Any)], + constructor: Some(param_prologue_ctor()), + 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, + } +} + +fn probe_module() -> Module { + let mut m = Module::new("class_field_barrier.ts"); + m.classes = vec![boxed_class()]; + m.functions = vec![Function { + id: 1, + name: "probe".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Named("Boxed".to_string()), + body: vec![Stmt::Return(Some(Expr::New { + class_name: "Boxed".to_string(), + args: vec![Expr::Number(1.0)], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }))], + 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 = vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(1)), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + })]; + m.init_kind = ModuleInitKind::Eager; + m +} + +fn ir() -> String { + String::from_utf8(compile_module(&probe_module(), ir_opts()).expect("module compiles")) + .expect("LLVM IR should be UTF-8") +} + +/// The `br i1 %cond, label %class_field_set.barrier.N, label %...` line, plus +/// the body of the block that contains it. +/// +/// Selected by following the LABEL DEFINITION of the branching block, not by +/// slicing text around the first mention of the barrier name — the first +/// mention IS the branch, and a slice starting there is empty. +fn branch_into_barrier(ir: &str) -> Option<(String, String)> { + let mut current_body: Vec<&str> = Vec::new(); + for line in ir.lines() { + let trimmed = line.trim_end(); + if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { + current_body.clear(); + continue; + } + let t = trimmed.trim_start(); + if t.starts_with("br i1 ") && t.contains(&format!("label %{BARRIER_BLOCK}")) { + return Some((trimmed.to_string(), current_body.join("\n"))); + } + current_body.push(line); + } + None +} + +/// Body of the named block (label definition to its terminator, inclusive). +fn block_body(ir: &str, label_prefix: &str) -> Option { + let mut inside = false; + let mut out: Vec<&str> = Vec::new(); + for line in ir.lines() { + let trimmed = line.trim_end(); + if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { + if inside { + break; + } + inside = trimmed.trim_end_matches(':').starts_with(label_prefix); + continue; + } + if inside { + out.push(line); + if trimmed.trim_start().starts_with("br ") || trimmed.trim_start().starts_with("ret ") { + break; + } + } + } + (inside && !out.is_empty()).then(|| out.join("\n")) +} + +/// (1) + (2): the gate exists, is BRANCHED INTO, and its condition is the live +/// header test rather than a constant. +/// +/// This is the assertion the sabotage targets: replace +/// `emit_parent_may_need_remembering_check(...)` with a literal `"true"` and +/// the branch stops being a `br i1 %reg`; replace it with `"false"` and the +/// TENURED mask / incremental-count loads vanish from the branching block. +#[test] +fn the_class_field_barrier_sits_behind_a_live_parent_generation_test() { + assert_default_barrier_env_not_disabled(); + let ir = ir(); + let (branch, body) = branch_into_barrier(&ir).unwrap_or_else(|| { + panic!( + "no `br i1 ..., label %{BARRIER_BLOCK}` — the #7864 gate was never \ + REACHED, so every object-literal field store still pays the \ + remembered-set call from the nursery:\n{ir}" + ) + }); + assert!( + body.contains(TENURED_MASK) && body.contains(TENURED_VALUE), + "the branching block does not mask GC_FLAG_TENURED (0x20) out of \ + gc_flags, so the branch rests on something other than the parent's \ + generation:\n{body}" + ); + assert!( + body.contains(INCREMENTAL_GLOBAL), + "the predicate dropped its incremental-cycle disjunct; skipping the \ + barrier also skips barrier_child_prologue's SATB shading, which is \ + NOT a generational question:\n{body}" + ); + // The barrier must be on the TAKEN edge. A swapped `cond_br` compiles, + // prints the right answer, and strands a child on the next minor GC. + let successors: Vec<&str> = branch + .split("label %") + .skip(1) + .map(|part| part.split([',', ' ']).next().unwrap()) + .collect(); + assert_eq!(successors.len(), 2, "expected a two-way branch: {branch}"); + assert!( + successors[0].starts_with(BARRIER_BLOCK), + "the barrier is on the FALSE edge — an untenured parent would take the \ + call and a tenured one would skip it, which is the failure this gate \ + exists to avoid: {branch}" + ); + assert!( + successors[1].starts_with("class_field_set.gc_bookkeeping.done"), + "the not-needed edge must join at the bookkeeping continuation: {branch}" + ); +} + +/// (3) The boundary: a guard, not an elision. +/// +/// `js_write_barrier_slot` must still be emitted, inside the barrier block. +/// A tenured receiver — an object promoted between its allocation and this +/// store, or one written long after construction — reaches it exactly as +/// before. A test that asserted the call ABSENT would be pinning a stranded +/// child. +#[test] +fn the_gated_arm_still_reaches_the_barrier_call() { + assert_default_barrier_env_not_disabled(); + let ir = ir(); + assert!( + ir.contains(BARRIER_CALL), + "js_write_barrier_slot was ELIDED rather than gated — a tenured parent \ + would publish an old->young edge nobody records:\n{ir}" + ); + let barrier_body = block_body(&ir, BARRIER_BLOCK) + .unwrap_or_else(|| panic!("no `{BARRIER_BLOCK}` block body:\n{ir}")); + assert!( + barrier_body.contains(BARRIER_CALL), + "the barrier call left the block the gate branches into, so it is \ + reached under some OTHER condition than the parent's generation:\n\ + {barrier_body}" + ); + assert!( + ir.contains("call void @js_gc_write_barriers_emitted(i32 1)"), + "the module must still declare to the runtime that generated barriers \ + exist — the remembered set's arming protocol reads this:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index cab44dd775..453fa529c1 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -137,6 +137,8 @@ pub(crate) use write_barrier::{ // under 2000 lines. Inherent methods (`record_value`) need no re-export. #[cfg(test)] mod array_push_guard_tests; +#[cfg(test)] +mod class_field_barrier_tests; mod dispatch; mod record_value; mod repsel_gates; diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index dcd314231e..f60e06d7a1 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -497,6 +497,33 @@ pub(crate) fn emit_may_carry_heap_pointer_check(blk: &mut LlBlock, value_bits: & /// Callers that already proved the value statically pass all three flags /// `false`; then no test and no blocks are emitted at all, and lever D's /// existing elision is unchanged. +/// +/// ## The parent's half of the same question (#7864) +/// +/// The value test answers "does this store publish a heap pointer at all". It +/// does not answer "does anyone need to know" — and for the shape this emitter +/// exists to serve, the answer is almost always no. HIR rewrites every +/// closed-shape object literal into a `new` of a synthesized anon-shape class +/// (`lower/context.rs::mint_anon_shape_class`), so `{ kind: "num", num: n }` +/// reaches the shared `_constructor` and writes its fields into an +/// instance allocated a few instructions earlier **in the nursery**. A nursery +/// parent is fully retraced by every minor GC, so the edge it publishes is +/// rediscovered and the remembered-set record is pure cost. +/// +/// The pointer-bearing arm is therefore itself gated on +/// [`emit_parent_may_need_remembering_check`] — the identical predicate +/// `expr/array_push.rs` has carried since #7511, resting on the identical +/// argument. `Old ⟹ TENURED`, so `!TENURED` can only skip a subset of what the +/// runtime already skips; and the predicate's second disjunct is the +/// incremental-cycle count, because skipping the call also skips +/// `barrier_child_prologue`'s SATB shading, which is not a generational +/// question. +/// +/// It is a LIVE header test, never a static claim (#7501's shape): a parent +/// promoted between its allocation and this store reads `TENURED` here and +/// takes the barrier. The failure direction is the safe one — a receiver whose +/// generation the compiler cannot see is exactly a receiver whose header it +/// reads. #[allow(clippy::too_many_arguments)] pub(crate) fn emit_jsvalue_slot_store_pointer_tested( ctx: &mut FnCtx<'_>, @@ -582,12 +609,31 @@ pub(crate) fn emit_jsvalue_slot_store_pointer_tested( let blk = ctx.block(); emit_layout_note_slot_on_block(blk, layout_parent_bits, slot_index, &value_bits); } - { - let blk = ctx.block(); - if write_barrier_emitted { + if write_barrier_emitted { + // #7864: the parent's half. `layout_parent_bits` is the receiver's + // validated, non-forwarded GC USER POINTER — the conforming check above + // dereferences it at `-6`, and `emit_layout_note_slot_on_block` decodes + // it the same way — so it is exactly what + // `emit_parent_may_need_remembering_check` documents as its input. + // + // The barrier keeps its own block so an IR census can see whether the + // gate was reached at all: a `cond_br` INTO `class_field_set.barrier` is + // the difference between "guarded" and "silently deleted". + let barrier_idx = ctx.new_block("class_field_set.barrier"); + let barrier_label = ctx.block_label(barrier_idx); + { + let blk = ctx.block(); + let needed = emit_parent_may_need_remembering_check(blk, layout_parent_bits); + blk.cond_br(&needed, &barrier_label, &done_label); + } + ctx.current_block = barrier_idx; + { + let blk = ctx.block(); emit_write_barrier_slot_on_block(blk, barrier_parent_bits, slot_addr, &value_bits); + blk.br(&done_label); } - blk.br(&done_label); + } else { + ctx.block().br(&done_label); } ctx.current_block = done_idx; Some(value_bits) diff --git a/crates/perry-codegen/src/function.rs b/crates/perry-codegen/src/function.rs index 689bc77050..ec1e26c2b4 100644 --- a/crates/perry-codegen/src/function.rs +++ b/crates/perry-codegen/src/function.rs @@ -48,6 +48,12 @@ pub struct LlFunction { /// "is this code hot?" rather than "should LLVM's threshold move?" read /// this. pub hot_loop_callee: bool, + /// #7864: `collectors::collect_alloc_hot_functions` admitted this function + /// — it has an in-loop direct call site (uncapped) or calls itself. Read + /// ONLY by `lower_call/new_alloc.rs::new_site_is_in_loop`; it must not be + /// used to widen `inline_hint`, whose anti-bloat cap is the whole reason + /// the two sets are separate. + pub alloc_hot: bool, /// Invoke-EH (#7302): this function contains landing pads (Itanium) or /// funclet pads (SEH), so its `define` line must carry /// `personality ptr @` — `perry_eh_personality` on Mach-O/ELF, @@ -228,6 +234,7 @@ impl LlFunction { force_inline: false, inline_hint: false, hot_loop_callee: false, + alloc_hot: false, personality: None, blocks: Vec::new(), block_counter: 0, diff --git a/crates/perry-codegen/src/lower_call/new_alloc.rs b/crates/perry-codegen/src/lower_call/new_alloc.rs index b744a6040c..307c429846 100644 --- a/crates/perry-codegen/src/lower_call/new_alloc.rs +++ b/crates/perry-codegen/src/lower_call/new_alloc.rs @@ -77,7 +77,28 @@ fn new_site_is_in_loop(ctx: &FnCtx<'_>) -> bool { // Reading `func.hot_loop_callee` here is well-ordered: `codegen/function.rs` // sets it from `cross_module.hot_loop_callees` before the entry block is // created and before any expression is lowered. - ctx.func.hot_loop_callee + if ctx.func.hot_loop_callee { + return true; + } + // #7864: the same question, asked with the right cost model. + // + // `hot_loop_callee` above carries `inline_hot_small_max_call_sites` (4), + // which is `inlinehint`'s anti-bloat backstop — it bounds a cost that + // scales with CALL SITES because LLVM duplicates the callee body at each + // one. The inline bump allocator's cost is ~268 bytes per `new` SITE in + // this function, paid once regardless of how many callers there are. So + // the cap prices a cost that does not exist here, and it excludes exactly + // the functions that earn the inline form: a recursive-descent evaluator's + // hot function has one call site per recursion arm. + // + // `gc-handoff/apps/interp.ts`'s `evalNode` had 11 (10 of them its own + // recursion) and allocated a `Value` per invocation through the outlined + // call, ~20M times. Whole-corpus A/B with `PERRY_INLINE_NEW=1` (the + // force-everywhere knob, i.e. a strict superset of this rule): `interp` + // −16.2%, `iso_miss` −10.4%, `pipeline` −8.4%, zero regressions outside a + // ±1.6% floor — and 15 of 19 binaries came out byte-identical, so the + // widening reaches four programs, not the corpus. + ctx.func.alloc_hot } /// Emit the instance allocation for `new (...)` and return the raw From e867b0558e467998c1d884fd3250cec774d40f48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 19:10:57 +0200 Subject: [PATCH 2/5] test(codegen): prove the parent-generation gate by walking its def chain, not by proximity --- .../src/expr/class_field_barrier_tests.rs | 109 +++++++++++++++--- 1 file changed, 95 insertions(+), 14 deletions(-) diff --git a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs index 136519d2b7..673d4d7ea2 100644 --- a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs +++ b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs @@ -280,13 +280,46 @@ fn block_body(ir: &str, label_prefix: &str) -> Option { (inside && !out.is_empty()).then(|| out.join("\n")) } -/// (1) + (2): the gate exists, is BRANCHED INTO, and its condition is the live -/// header test rather than a constant. +/// The instruction that defines `%reg` inside `body`, without its `%reg = ` +/// prefix. `None` for a constant operand or a value defined elsewhere. +fn def_of<'a>(body: &'a str, reg: &str) -> Option<&'a str> { + let needle = format!("{reg} = "); + body.lines() + .map(str::trim) + .find(|l| l.starts_with(&needle)) + .map(|l| l[needle.len()..].trim()) +} + +/// The `i`th SSA operand (`%…`) of an instruction. +fn operand(instr: &str, i: usize) -> Option { + instr + .match_indices('%') + .nth(i) + .map(|(pos, _)| { + instr[pos..] + .split(|c: char| c == ',' || c.is_whitespace() || c == ')') + .next() + .unwrap() + .to_string() + }) +} + +/// (1) + (2): the gate exists, is BRANCHED INTO, and its condition **is** the +/// live header test — proved by walking the def chain, not by looking for the +/// instructions somewhere nearby. +/// +/// ★ The weaker version of this test (assert the block *contains* an +/// `and i8 …, 32` and the incremental global) passed a deliberate sabotage that +/// hard-wired the branch to `br i1 false` while leaving the now-dead predicate +/// instructions in the block. That is precisely the "gate that cannot fail" +/// shape CLAUDE.md catalogues, so the assertion walks: /// -/// This is the assertion the sabotage targets: replace -/// `emit_parent_may_need_remembering_check(...)` with a literal `"true"` and -/// the branch stops being a `br i1 %reg`; replace it with `"false"` and the -/// TENURED mask / incremental-count loads vanish from the branching block. +/// cond → `or i1 %a, %b` +/// %a → `icmp ne i8 %t, 0` → %t → `and i8 %f, 32` → %f → `load i8` +/// %b → `icmp ne i32 %c, 0` → %c → atomic load of the incremental count +/// +/// Any constant condition, any dropped disjunct, and any substitution of a +/// different predicate breaks a named link in that chain. #[test] fn the_class_field_barrier_sits_behind_a_live_parent_generation_test() { assert_default_barrier_env_not_disabled(); @@ -298,17 +331,65 @@ fn the_class_field_barrier_sits_behind_a_live_parent_generation_test() { remembered-set call from the nursery:\n{ir}" ) }); + + let cond = branch + .trim_start() + .strip_prefix("br i1 ") + .and_then(|rest| rest.split(',').next()) + .map(str::trim) + .unwrap_or(""); + assert!( + cond.starts_with('%'), + "the gate's condition is the constant `{cond}` — the branch cannot \ + fail, so the barrier is either always taken (no win) or NEVER taken \ + (a stranded child on the next minor GC): {branch}" + ); + let or_instr = def_of(&body, cond).unwrap_or_else(|| { + panic!("the gate's condition {cond} is not defined in the branching block:\n{body}") + }); + assert!( + or_instr.starts_with("or i1 "), + "the gate's condition is `{or_instr}`, not the disjunction of the \ + generational and incremental clauses:\n{body}" + ); + let tenured_cmp_reg = operand(or_instr, 0).expect("or lhs"); + let incremental_cmp_reg = operand(or_instr, 1).expect("or rhs"); + + // Clause 1: gc_flags & GC_FLAG_TENURED != 0, off a real i8 header load. + let tenured_cmp = def_of(&body, &tenured_cmp_reg).unwrap_or_default(); + assert!( + tenured_cmp.starts_with("icmp ne i8 ") && tenured_cmp.ends_with(", 0"), + "the generational clause is `{tenured_cmp}`, not `gc_flags & TENURED != 0`:\n{body}" + ); + let mask_reg = operand(tenured_cmp, 0).expect("icmp lhs"); + let mask = def_of(&body, &mask_reg).unwrap_or_default(); + assert!( + mask.starts_with(TENURED_MASK) && mask.ends_with(TENURED_VALUE), + "the generational clause masks `{mask}` rather than GC_FLAG_TENURED \ + (0x20) — a different bit would answer a different question:\n{body}" + ); + let flags_reg = operand(mask, 0).expect("and lhs"); + assert!( + def_of(&body, &flags_reg).unwrap_or_default().starts_with("load i8"), + "the masked value is not loaded from the parent's GcHeader, so the \ + gate rests on something other than the live header:\n{body}" + ); + + // Clause 2: the incremental-cycle count. Dropping it would also drop + // barrier_child_prologue's SATB shading, which is not a generational + // question and must never be skipped while a cycle is live. + let incremental_cmp = def_of(&body, &incremental_cmp_reg).unwrap_or_default(); assert!( - body.contains(TENURED_MASK) && body.contains(TENURED_VALUE), - "the branching block does not mask GC_FLAG_TENURED (0x20) out of \ - gc_flags, so the branch rests on something other than the parent's \ - generation:\n{body}" + incremental_cmp.starts_with("icmp ne i32 ") && incremental_cmp.ends_with(", 0"), + "the incremental clause is `{incremental_cmp}`:\n{body}" ); + let count_reg = operand(incremental_cmp, 0).expect("icmp lhs"); assert!( - body.contains(INCREMENTAL_GLOBAL), - "the predicate dropped its incremental-cycle disjunct; skipping the \ - barrier also skips barrier_child_prologue's SATB shading, which is \ - NOT a generational question:\n{body}" + def_of(&body, &count_reg) + .unwrap_or_default() + .contains(INCREMENTAL_GLOBAL), + "the incremental clause does not read {INCREMENTAL_GLOBAL}; skipping \ + the barrier also skips SATB shading:\n{body}" ); // The barrier must be on the TAKEN edge. A swapped `cond_br` compiles, // prints the right answer, and strands a child on the next minor GC. From 0b949954333e29a473d8d9fca93590734c895bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 19:18:28 +0200 Subject: [PATCH 3/5] docs(changelog): interp round 5 fragment --- .../7864-interp-round5-alloc-and-barrier.md | 85 +++++++++++++++++++ .../src/expr/class_field_barrier_tests.rs | 25 +++--- 2 files changed, 96 insertions(+), 14 deletions(-) create mode 100644 changelog.d/7864-interp-round5-alloc-and-barrier.md diff --git a/changelog.d/7864-interp-round5-alloc-and-barrier.md b/changelog.d/7864-interp-round5-alloc-and-barrier.md new file mode 100644 index 0000000000..12666ccf19 --- /dev/null +++ b/changelog.d/7864-interp-round5-alloc-and-barrier.md @@ -0,0 +1,85 @@ +### `interp` 1.095 → 0.844 s, `iso_miss` 1.464 → 1.234 s: two independent codegen gates + +Quiet M1 mini, best-of-5, exit-checked, outputs byte-compared to +`node --experimental-strip-types`. The four binaries that come out +**byte-identical** across the two arms (`churn`, `push_cls`, `retain`, `fib40`) +set the run's noise floor at −0.1%/+0.3%, which is what makes the rest credible. + +| bench | before | after | delta | +|---|--:|--:|--:| +| `interp` | 1.0945 | **0.8441** | **−22.9%** | +| `cycles` | 0.1115 | **0.0866** | **−22.3%** | +| `iso_miss` | 1.4635 | **1.2338** | **−15.7%** | +| `tree` | 1.1665 | 1.0222 | −12.4% | +| `tree_wide` | 1.6464 | 1.5216 | −7.6% | +| `deeplist` | 0.1071 | 0.1018 | −4.9% | +| `pipeline` | 0.2738 | 0.2643 | −3.5% | +| the other 12 | — | — | within ±0.3% | + +#### 1. The class-field write barrier now tests the parent's generation + +`expr/write_barrier.rs::emit_jsvalue_slot_store_pointer_tested` (#7511) put the +store's three bookkeeping calls behind one live test of the stored **value** — +"does this publish a heap pointer at all". It never asked the barrier's other +question, "is the parent old enough for anyone to care", even though +`emit_parent_may_need_remembering_check` sits 400 lines up in the same file. +That predicate had exactly one caller: `expr/array_push.rs`. + +HIR rewrites every closed-shape object literal into a `new` of a synthesized +anon-shape class, so `{ kind: "num", num: n }` reaches a shared +`_constructor` that writes its fields into an instance allocated a few +instructions earlier **in the nursery** — the `!TENURED` case, where the minor +GC retraces the parent anyway and the remembered-set record is pure cost. The +same predicate now gates the class-field store, on the identical argument +(`Old ⟹ TENURED`, plus the incremental-cycle count so SATB shading is never +skipped). It stays a **live header test**: a parent promoted between its +allocation and the store reads `TENURED` and takes the call. + +#### 2. A hot recursive function may inline its bump allocator + +`lower_call/new_alloc.rs::new_site_is_in_loop` admitted a `new` site to the +inline bump allocator only if it was lexically in a loop or its function was in +`collect_hot_loop_callees` — a set capped at **4 direct call sites module-wide**. +That cap is `inlinehint`'s anti-bloat backstop, where cost scales with call +sites because LLVM duplicates the body at each one. The inline bump allocator +costs ~268 bytes **per `new` site in the function**, once, whatever the caller +count, so the cap priced a cost that does not exist — and excluded exactly the +functions that earn it. `interp.ts`'s `evalNode` is the shape: the hottest +function in the program, one allocation per invocation, and 11 direct call +sites because ten of them are its own recursion. + +New collector `collect_alloc_hot_functions` answers the allocator's question +with the allocator's cost model: ≥1 in-loop direct call site (**uncapped**), or +direct self-recursion — a function that calls itself is a loop the lexical test +cannot see. It is a second set, not a widening of `hot_loop_callees`: raising +the shared cap to 32 instead buys `interp` −26.5% but **regresses `iso_miss` ++4.0%**, because it also moves `inlinehint`. + +`interp`'s compiled binary grows 16 KB (+0.13%). + +#### Validation + +19-program corpus, both arms: outputs byte-identical to node and exit 0, +including the `iso_miss` `misses 0` counter and `shapes`' `1176000`. 10 of 19 +binaries are byte-identical across the arms, so only 9 needed timing at all. +The nine that differ also pass under `PERRY_GC_VERIFY_EVACUATION=1 +PERRY_GC_FORCE_EVACUATE=1`, and five of them additionally under +`PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 +PERRY_GC_SCHEDULE_RATE=1`. + +`expr/class_field_barrier_tests.rs` is the gate, and it is **sabotage-verified** +rather than merely green. Its first draft asserted the TENURED mask and the +incremental-count load were *present in the branching block* — and passed a +sabotage that hard-wired the branch to `br i1 false` while leaving the dead +predicate instructions behind it. It now walks the def chain from the branch +condition (`or i1` → `icmp ne i8 … , 0` → `and i8 …, 32` → `load i8`; and +`icmp ne i32` → the atomic count load), and both sabotages — constant condition, +swapped successors — go red with the diagnostic that names the failure. + +#### Refuted + +`PERRY_WRITE_BARRIERS=0` is **not** a ceiling probe for barrier cost. It makes +`interp` 4.3× and `iso_miss` 5.4× *slower*: the knob is compile-time and the +GC's evacuation policy requires generated barriers to be active, so turning +them off makes the copying minor ineligible and the program falls back to full +mark-sweeps. It measures "no generational GC", not "no barrier". diff --git a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs index 673d4d7ea2..fe04141650 100644 --- a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs +++ b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs @@ -35,9 +35,7 @@ use crate::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::Type; -use perry_hir::{ - Class, ClassField, Expr, Function, Module, ModuleInitKind, Param, Stmt, -}; +use perry_hir::{Class, ClassField, Expr, Function, Module, ModuleInitKind, Param, Stmt}; /// The block that exists only when the #7864 gate was emitted. const BARRIER_BLOCK: &str = "class_field_set.barrier"; @@ -292,16 +290,13 @@ fn def_of<'a>(body: &'a str, reg: &str) -> Option<&'a str> { /// The `i`th SSA operand (`%…`) of an instruction. fn operand(instr: &str, i: usize) -> Option { - instr - .match_indices('%') - .nth(i) - .map(|(pos, _)| { - instr[pos..] - .split(|c: char| c == ',' || c.is_whitespace() || c == ')') - .next() - .unwrap() - .to_string() - }) + instr.match_indices('%').nth(i).map(|(pos, _)| { + instr[pos..] + .split(|c: char| c == ',' || c.is_whitespace() || c == ')') + .next() + .unwrap() + .to_string() + }) } /// (1) + (2): the gate exists, is BRANCHED INTO, and its condition **is** the @@ -370,7 +365,9 @@ fn the_class_field_barrier_sits_behind_a_live_parent_generation_test() { ); let flags_reg = operand(mask, 0).expect("and lhs"); assert!( - def_of(&body, &flags_reg).unwrap_or_default().starts_with("load i8"), + def_of(&body, &flags_reg) + .unwrap_or_default() + .starts_with("load i8"), "the masked value is not loaded from the parent's GcHeader, so the \ gate rests on something other than the live header:\n{body}" ); From ae296f716b569ba874e6cca4b20c87d8f7c375b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 19:44:51 +0200 Subject: [PATCH 4/5] test(codegen): pin the alloc-hot gate live on a recursive function, and cold elsewhere --- .../src/lower_call/alloc_hot_tests.rs | 252 ++++++++++++++++++ crates/perry-codegen/src/lower_call/mod.rs | 2 + 2 files changed, 254 insertions(+) create mode 100644 crates/perry-codegen/src/lower_call/alloc_hot_tests.rs diff --git a/crates/perry-codegen/src/lower_call/alloc_hot_tests.rs b/crates/perry-codegen/src/lower_call/alloc_hot_tests.rs new file mode 100644 index 0000000000..c8676a95e6 --- /dev/null +++ b/crates/perry-codegen/src/lower_call/alloc_hot_tests.rs @@ -0,0 +1,252 @@ +//! #7864: a self-recursive function's `new` sites take the INLINE bump +//! allocator. +//! +//! The subject is [`super::new_alloc::new_site_is_in_loop`]'s second arm and the +//! `collectors::collect_alloc_hot_functions` set behind it. +//! +//! This is a liveness gate, and it exists because the failure mode is silence. +//! `js_object_alloc_class_inline_keys` performs the identical bump alloc and +//! returns the identical user pointer, so a gate that stops firing changes no +//! output, breaks no other test, and simply makes every allocation in a +//! recursive-descent evaluator cost a cross-crate call again — `interp.ts`'s +//! −22.9% quietly evaporating with nothing to show for it. Only the emitted +//! `alloc.fast` / `js_inline_arena_slow_alloc` shape separates the two. +//! +//! The negative half is the anti-bloat property: a `new` in a function that is +//! neither in a loop, nor called from one, nor recursive keeps the outlined +//! call. Without it this file would pass just as happily if the gate had been +//! widened to "always", which is the ~268-bytes-per-site default the +//! `[#bloat]` comment in `new_alloc.rs` exists to refuse. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{ + BinaryOp, Class, ClassField, CompareOp, Expr, Function, Module, ModuleInitKind, Param, Stmt, +}; + +/// Emitted only by the inline bump allocator. Both are CALL/LABEL forms, not +/// bare symbol names: `runtime_decls` emits a `declare` for +/// `js_inline_arena_slow_alloc` into every module whether or not anything calls +/// it, so a symbol-presence check answers "the runtime exists", not "the +/// inline allocator was chosen". (The first draft of this file asserted on the +/// bare name and the negative arm failed against a correctly-outlined module.) +const INLINE_SLOW_CALL: &str = "call ptr @js_inline_arena_slow_alloc("; +const INLINE_FAST_BLOCK: &str = "\nalloc.fast"; +/// Emitted only by the outlined allocator. +const OUTLINED_CALL: &str = "call i64 @js_object_alloc_class_inline_keys"; + +const N_ID: u32 = 11; +const WALK_ID: u32 = 700; + +fn ir_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: true, + 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(), + namespace_member_nested: 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 cell_class() -> Class { + Class { + id: 3, + name: "Cell".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![ClassField { + name: "v".to_string(), + key_expr: None, + ty: Type::Number, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + }], + 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 walk(n) { if (n > 0) walk(n - 1); return new Cell(n) }` — the +/// recursive-descent shape, with NO loop anywhere and its entry call in +/// straight-line module init. `recurse = false` drops the self-call, which is +/// the only difference between the two arms. +fn walk_module(recurse: bool) -> Module { + let mut m = Module::new("alloc_hot.ts"); + m.classes = vec![cell_class()]; + let mut body: Vec = Vec::new(); + if recurse { + body.push(Stmt::If { + condition: Expr::Compare { + op: CompareOp::Gt, + left: Box::new(Expr::LocalGet(N_ID)), + right: Box::new(Expr::Number(0.0)), + }, + then_branch: vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(WALK_ID)), + args: vec![Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(Expr::LocalGet(N_ID)), + right: Box::new(Expr::Number(1.0)), + }], + type_args: Vec::new(), + byte_offset: 0, + })], + else_branch: None, + }); + } + body.push(Stmt::Return(Some(Expr::New { + class_name: "Cell".to_string(), + args: vec![Expr::LocalGet(N_ID)], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }))); + m.functions = vec![Function { + id: WALK_ID, + name: "walk".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: N_ID, + name: "n".to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Named("Cell".to_string()), + 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, + }]; + // Straight-line, ONE call site, no loop: the only thing that can admit + // `walk` is the recursion itself. + m.init = vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(WALK_ID)), + args: vec![Expr::Number(8.0)], + type_args: Vec::new(), + byte_offset: 0, + })]; + m.init_kind = ModuleInitKind::Eager; + m +} + +fn ir_for(m: Module) -> String { + String::from_utf8(compile_module(&m, ir_opts()).expect("module compiles")) + .expect("LLVM IR should be UTF-8") +} + +/// Guard against `PERRY_INLINE_NEW`, which forces the inline form everywhere +/// and would make the positive arm pass and the negative arm fail for a reason +/// that has nothing to do with the gate. +fn assert_inline_new_not_forced() { + assert!( + std::env::var_os("PERRY_INLINE_NEW").is_none(), + "these tests describe the DEFAULT gate; PERRY_INLINE_NEW must be unset" + ); +} + +#[test] +fn a_self_recursive_function_inlines_its_bump_allocator() { + assert_inline_new_not_forced(); + let ir = ir_for(walk_module(true)); + assert!( + ir.contains(INLINE_SLOW_CALL) && ir.contains(INLINE_FAST_BLOCK), + "`walk` is self-recursive and allocates per level, but its `new` took \ + the outlined allocator — recursion IS a loop, and the lexical test \ + cannot see it:\n{ir}" + ); + assert!( + !ir.contains(OUTLINED_CALL), + "the outlined allocator is still emitted for the recursive function's \ + only `new` site:\n{ir}" + ); +} + +/// The anti-bloat half. Identical module minus the self-call: one call site, no +/// loop, not recursive — nothing about it says "runs many times", so it keeps +/// the outlined call and contributes nothing to binary growth. +#[test] +fn a_cold_straight_line_function_keeps_the_outlined_allocator() { + assert_inline_new_not_forced(); + let ir = ir_for(walk_module(false)); + assert!( + ir.contains(OUTLINED_CALL), + "a `new` in a cold, non-recursive, non-in-loop function took the \ + inline bump allocator — the gate has been widened to `always`, which \ + is ~268 bytes per site across the whole program:\n{ir}" + ); + assert!( + !ir.contains(INLINE_SLOW_CALL), + "the inline bump allocator reached a cold site:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index f6b90a862e..d32ecbfa3f 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -31,6 +31,8 @@ use crate::expr::{variant_name, FnCtx}; // `native_module_dispatch.rs` (#1105 followup): per-branch // extraction of the original `lower_call.rs`'s 4.3k-LOC body so // every file in this directory stays under 2000 lines. +#[cfg(test)] +mod alloc_hot_tests; mod atomics; pub(crate) mod buffer_intrinsic; mod builtin; From b622d359068e3ebf6ea77d12e3a40fa1d4934cad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 19:46:57 +0200 Subject: [PATCH 5/5] docs: point the round-5 comments at PR #7871 --- ...d-barrier.md => 7871-interp-round5-alloc-and-barrier.md} | 0 crates/perry-codegen/src/codegen/function.rs | 2 +- crates/perry-codegen/src/codegen/mod.rs | 2 +- crates/perry-codegen/src/codegen/opts.rs | 2 +- crates/perry-codegen/src/collectors/hot_callees.rs | 2 +- crates/perry-codegen/src/expr/class_field_barrier_tests.rs | 6 +++--- crates/perry-codegen/src/expr/write_barrier.rs | 4 ++-- crates/perry-codegen/src/function.rs | 2 +- crates/perry-codegen/src/lower_call/alloc_hot_tests.rs | 2 +- crates/perry-codegen/src/lower_call/new_alloc.rs | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) rename changelog.d/{7864-interp-round5-alloc-and-barrier.md => 7871-interp-round5-alloc-and-barrier.md} (100%) diff --git a/changelog.d/7864-interp-round5-alloc-and-barrier.md b/changelog.d/7871-interp-round5-alloc-and-barrier.md similarity index 100% rename from changelog.d/7864-interp-round5-alloc-and-barrier.md rename to changelog.d/7871-interp-round5-alloc-and-barrier.md diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index f92aa67787..103f88eea5 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -434,7 +434,7 @@ pub(super) fn compile_function( // threshold move" — an `alwaysinline` callee is excluded from the latter // and is the hottest possible case for the former. lf.hot_loop_callee = cross_module.hot_loop_callees.contains(&f.id); - // #7864: the allocator's hotness set. Set from the same well-ordered point + // #7871: the allocator's hotness set. Set from the same well-ordered point // as `hot_loop_callee` (before the entry block exists and before any // expression is lowered), for the same reason. lf.alloc_hot = cross_module.alloc_hot_functions.contains(&f.id); diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 190b74d063..633e6d38a3 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1706,7 +1706,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> hir, crate::codegen::helpers::inline_hot_small_max_call_sites(), ), - // #7864: the allocator's own "is this hot" set — same in-loop proxy, + // #7871: the allocator's own "is this hot" set — same in-loop proxy, // no call-site cap (the cap prices `inlinehint`'s duplication, which // the inline bump allocator does not incur), plus direct recursion. alloc_hot_functions: crate::collectors::collect_alloc_hot_functions(hir), diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index 1190479ae9..b47d8cea34 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -976,7 +976,7 @@ pub(crate) struct CrossModuleCtx { /// `PERRY_INLINE_HOT_SMALL` is off (the flag is checked at the decision /// site, so the set is still populated but simply not consulted). pub hot_loop_callees: std::collections::HashSet, - /// #7864: `FuncId`s in THIS module whose `new` sites earn the inline bump + /// #7871: `FuncId`s in THIS module whose `new` sites earn the inline bump /// allocator — `collectors::collect_alloc_hot_functions`. Deliberately a /// SECOND set rather than a widening of `hot_loop_callees`: that one gates /// `inlinehint`, whose cost scales with call sites, and the two must not diff --git a/crates/perry-codegen/src/collectors/hot_callees.rs b/crates/perry-codegen/src/collectors/hot_callees.rs index 9bec3970cd..ba7f237bd6 100644 --- a/crates/perry-codegen/src/collectors/hot_callees.rs +++ b/crates/perry-codegen/src/collectors/hot_callees.rs @@ -99,7 +99,7 @@ pub fn collect_hot_loop_callees(hir: &Module, max_call_sites: u32) -> HashSet) -> bool { if ctx.func.hot_loop_callee { return true; } - // #7864: the same question, asked with the right cost model. + // #7871: the same question, asked with the right cost model. // // `hot_loop_callee` above carries `inline_hot_small_max_call_sites` (4), // which is `inlinehint`'s anti-bloat backstop — it bounds a cost that