From f3d013f9160e6582d5f4b6d61e83e37d5176b4ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 20:25:02 +0200 Subject: [PATCH] fix: replay #9071 onto main without reverting #9060 and #9062 The branch's commit was authored over a tree predating #9060/#9062 and committed onto a newer base, so it carried deletions of files its own parent contains: #9060's whole `stable_packed_accumulator.rs` (273 lines) and its build-cache knob, plus #9062's `for_multi_decl_tests.rs`, its `issue_9052_for_lexical_declarators.rs` integration test, the `Module` field and its stable-hash entry, and ~40 test-fixture updates. 477 deletions in total. Replayed the six files that are genuinely this change onto current main instead: closure.rs, function.rs, helpers.rs, hoisted_callback_calls.rs, collectors/mod.rs and the fragment. Net diff vs main is now additions only, with zero deletions. Also fixes two gate failures of its own: * PERRY_CALLEE_BINDING_RESOLUTION was unregistered, failing `codegen_env_vars_are_build_cache_inputs`. Registered as an INPUT: the two settings emit different call sequences, so a cached object from one must not serve the other. * `local_binding_type_audit.py` wants the new `local_type_hint` read classified. Recorded as `runtime-validated`: the declared function type only narrows which bindings are ATTEMPTED, while `js_closure_resolve_arrow_direct_call(handle, arity)` validates identity and arity at runtime, so a wrong declared type yields a failed resolution and the ordinary dynamic call rather than a wrong callee. Fragment renumbered 0000 -> 9071. --- changelog.d/9071-callee-binding-resolution.md | 3 + crates/perry-codegen/src/codegen/closure.rs | 17 ++ crates/perry-codegen/src/codegen/function.rs | 9 + crates/perry-codegen/src/codegen/helpers.rs | 114 +++++++++++++ .../src/collectors/hoisted_callback_calls.rs | 157 ++++++++++++++++++ crates/perry-codegen/src/collectors/mod.rs | 4 +- .../perry/src/commands/compile/build_cache.rs | 4 + scripts/local_binding_type_allowlist.json | 20 ++- 8 files changed, 321 insertions(+), 7 deletions(-) create mode 100644 changelog.d/9071-callee-binding-resolution.md diff --git a/changelog.d/9071-callee-binding-resolution.md b/changelog.d/9071-callee-binding-resolution.md new file mode 100644 index 0000000000..32d7e9f6ad --- /dev/null +++ b/changelog.d/9071-callee-binding-resolution.md @@ -0,0 +1,3 @@ +### Changed + +- Loop-called arrows held in immutable bindings — captured, module-global, or parameter — are resolved once at body entry and dispatched directly, instead of paying the full `js_closure_callN` dispatcher on every call. The isolated captured-arrow call drops from 8.0 to 4.4 ns/op; a resolution that fails (non-arrow, bound function, reassigned binding, pre-initialization sentinel) keeps the exact dispatcher fallback per call. diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 1a2b549263..c06a359850 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -1306,6 +1306,23 @@ pub(super) fn compile_closure( super::arguments::ArgumentsCallee::CurrentClosure, ); + // #9060 follow-up: resolve loop-called immutable callee bindings once at + // entry — parameters, captured bindings, and module globals (the + // module-wide reassignment oracle is in scope here). `%this_closure` is a + // live parameter of every closure body, so capture-slot reads are direct. + // Skipped for async bodies: entry SSA values do not survive the CPS + // rewrite. + if !is_async { + let param_ids: std::collections::HashSet = params.iter().map(|p| p.id).collect(); + super::helpers::emit_callee_binding_resolutions( + &mut ctx, + body, + ¶m_ids, + Some(module_reassigned_locals), + true, + ); + } + if is_async { stmt::lower_async_rejecting_stmts(&mut ctx, body) .with_context(|| format!("lowering async closure body func_id={}", func_id))?; diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 98c0b2e846..af581de982 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -1397,6 +1397,15 @@ pub(super) fn compile_function( } } + // #9060 follow-up: resolve loop-called immutable callee bindings once at + // entry (parameters only here — the plain-function path has no module-wide + // reassignment oracle in scope for captures/globals). Skipped for async + // bodies: their entry SSA values do not survive the CPS rewrite. + if !f.is_async { + let param_ids: std::collections::HashSet = f.params.iter().map(|p| p.id).collect(); + super::helpers::emit_callee_binding_resolutions(&mut ctx, &f.body, ¶m_ids, None, false); + } + if f.is_async { stmt::lower_async_rejecting_top_level_stmts(&mut ctx, &f.body) .with_context(|| format!("lowering async body of '{}'", f.name))?; diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index c2fb659a7a..f61618e916 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -1692,3 +1692,117 @@ mod native_roots_target_tests { ); } } + +/// `PERRY_CALLEE_BINDING_RESOLUTION` gate (default on): resolve loop-called +/// immutable callee bindings once at body entry. `=0`/`off`/`false` restores +/// per-call `js_closure_callN` dispatch for A/B bisection. +pub(super) fn callee_binding_resolution_enabled() -> bool { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + !matches!( + std::env::var("PERRY_CALLEE_BINDING_RESOLUTION").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} + +/// Populate `resolved_arrow_callback_targets` for loop-called immutable callee +/// bindings — the generalization of `codegen/method.rs`'s callback-parameter +/// resolution to plain function and closure bodies, and to captured bindings +/// and module globals. +/// +/// Every read here is RAW and cannot throw: a parameter or plain local is a +/// slot load; a captured binding is a capture-slot load (plus a +/// `js_box_get_bits` cell read for a boxed capture — the untrusted entry, +/// which returns the TDZ sentinel rather than throwing); a module global is a +/// global load. A sentinel or non-closure value simply resolves to null and +/// every call keeps its full-dispatcher fallback, so a body that runs before a +/// captured binding initializes behaves exactly as before. The binding being +/// unassigned module-wide (the collector's admission) is what makes the +/// entry-resolved identity stand for every later call. +/// +/// The `Function` type-hint check mirrors the guarded direct-dispatch arm in +/// `lower_call/early_branches.rs` — the ONLY consumer of the map — so a +/// resolution is never emitted for a binding whose call sites cannot use it. +pub(super) fn emit_callee_binding_resolutions( + ctx: &mut crate::expr::FnCtx<'_>, + body: &[perry_hir::Stmt], + param_ids: &std::collections::HashSet, + // `None` = the caller has no module-wide reassignment oracle; only + // parameters (whose writes are all in this body) are admitted then. + module_reassigned: Option<&std::collections::HashSet>, + this_closure_available: bool, +) { + use crate::types::{DOUBLE, I32, I64, PTR}; + if !callee_binding_resolution_enabled() { + return; + } + let empty = std::collections::HashSet::new(); + let (capture_ids, module_global_ids) = if module_reassigned.is_some() { + ( + ctx.closure_captures.keys().copied().collect(), + ctx.module_globals.keys().copied().collect(), + ) + } else { + (empty.clone(), empty.clone()) + }; + let candidates = crate::collectors::collect_loop_called_callee_bindings( + body, + param_ids, + &capture_ids, + &module_global_ids, + module_reassigned.unwrap_or(&empty), + ); + for (id, arity) in candidates { + if ctx + .resolved_arrow_callback_targets + .contains_key(&(id, arity)) + { + continue; + } + if !matches!( + ctx.local_type_hint(&id), + Some(perry_hir::types::Type::Function(function)) + if !function.is_async && !function.is_generator + ) { + continue; + } + let value_box = if let Some(&capture_idx) = ctx.closure_captures.get(&id) { + if !this_closure_available { + continue; + } + let offset = crate::target_layout::closure_header_size_bytes(ctx.target_triple) + + 8 * u64::from(capture_idx); + let blk = ctx.block(); + let slot_addr = blk.add(I64, "%this_closure", &offset.to_string()); + let slot_ptr = blk.inttoptr(I64, &slot_addr); + let bits = blk.load(I64, &slot_ptr); + if ctx.boxed_vars.contains(&id) { + let blk = ctx.block(); + let cell_bits = blk.call(I64, "js_box_get_bits", &[(I64, &bits)]); + ctx.block().bitcast_i64_to_double(&cell_bits) + } else { + ctx.block().bitcast_i64_to_double(&bits) + } + } else if let Some(global_name) = ctx.module_globals.get(&id).cloned() { + let g_ref = format!("@{global_name}"); + ctx.block().load(DOUBLE, &g_ref) + } else if let Some(slot) = ctx.locals.get(&id).cloned() { + if ctx.boxed_vars.contains(&id) { + continue; + } + ctx.block().load(DOUBLE, &slot) + } else { + continue; + }; + let handle = crate::expr::unbox_to_i64(ctx.block(), &value_box); + let fn_ptr = ctx.block().call( + PTR, + "js_closure_resolve_arrow_direct_call", + &[(I64, &handle), (I32, &arity.to_string())], + ); + ctx.resolved_arrow_callback_targets + .insert((id, arity), fn_ptr); + } +} diff --git a/crates/perry-codegen/src/collectors/hoisted_callback_calls.rs b/crates/perry-codegen/src/collectors/hoisted_callback_calls.rs index 0301356439..c4396e8288 100644 --- a/crates/perry-codegen/src/collectors/hoisted_callback_calls.rs +++ b/crates/perry-codegen/src/collectors/hoisted_callback_calls.rs @@ -254,3 +254,160 @@ pub(crate) fn collect_hoisted_callback_calls(method: &Function) -> Vec, + capture_ids: &std::collections::HashSet, + module_global_ids: &std::collections::HashSet, + module_reassigned: &std::collections::HashSet, +) -> Vec<(u32, usize)> { + let body_reassigned = super::reassigned_locals(body); + let mut in_loop: BTreeSet<(u32, usize)> = BTreeSet::new(); + fn scan_expr(expr: &Expr, loop_depth: usize, out: &mut BTreeSet<(u32, usize)>) { + if matches!(expr, Expr::Closure { .. }) { + return; + } + if let Expr::Call { callee, args, .. } = expr { + if let Expr::LocalGet(id) = callee.as_ref() { + if loop_depth > 0 && args.len() <= 16 { + out.insert((*id, args.len())); + } + } + } + perry_hir::walker::walk_expr_children(expr, &mut |child| { + scan_expr(child, loop_depth, out); + }); + } + fn scan_stmt(stmt: &Stmt, loop_depth: usize, out: &mut BTreeSet<(u32, usize)>) { + match stmt { + Stmt::While { condition, body } => { + scan_expr(condition, loop_depth + 1, out); + for s in body { + scan_stmt(s, loop_depth + 1, out); + } + } + Stmt::DoWhile { body, condition } => { + for s in body { + scan_stmt(s, loop_depth + 1, out); + } + scan_expr(condition, loop_depth + 1, out); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(init) = init { + scan_stmt(init, loop_depth, out); + } + if let Some(condition) = condition { + scan_expr(condition, loop_depth + 1, out); + } + if let Some(update) = update { + scan_expr(update, loop_depth + 1, out); + } + for s in body { + scan_stmt(s, loop_depth + 1, out); + } + } + Stmt::Let { + init: Some(expr), .. + } => scan_expr(expr, loop_depth, out), + Stmt::Expr(expr) | Stmt::Throw(expr) => scan_expr(expr, loop_depth, out), + Stmt::Return(Some(expr)) => scan_expr(expr, loop_depth, out), + Stmt::If { + condition, + then_branch, + else_branch, + } => { + scan_expr(condition, loop_depth, out); + for s in then_branch { + scan_stmt(s, loop_depth, out); + } + if let Some(body) = else_branch { + for s in body { + scan_stmt(s, loop_depth, out); + } + } + } + Stmt::Try { + body, + catch, + finally, + } => { + for s in body { + scan_stmt(s, loop_depth, out); + } + if let Some(catch) = catch { + for s in &catch.body { + scan_stmt(s, loop_depth, out); + } + } + if let Some(body) = finally { + for s in body { + scan_stmt(s, loop_depth, out); + } + } + } + Stmt::Switch { + discriminant, + cases, + } => { + scan_expr(discriminant, loop_depth, out); + for case in cases { + if let Some(test) = &case.test { + scan_expr(test, loop_depth, out); + } + for s in &case.body { + scan_stmt(s, loop_depth, out); + } + } + } + Stmt::Labeled { body, .. } => scan_stmt(body, loop_depth, out), + Stmt::Let { init: None, .. } + | Stmt::Return(None) + | Stmt::Break + | Stmt::Continue + | Stmt::LabeledBreak(_) + | Stmt::LabeledContinue(_) + | Stmt::PreallocateBoxes(_) + | Stmt::PreallocateTdzBoxes(_) + | Stmt::ReleaseBoxes(_) => {} + } + } + for stmt in body { + scan_stmt(stmt, 0, &mut in_loop); + } + in_loop + .into_iter() + .filter(|(id, _)| { + (param_ids.contains(id) || capture_ids.contains(id) || module_global_ids.contains(id)) + && !body_reassigned.contains(id) + && !module_reassigned.contains(id) + }) + .collect() +} diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 6d38e022ff..93f371693e 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -69,7 +69,9 @@ pub(crate) use hir_facts::{ collect_native_region_fact_graph, collect_native_region_fact_graph_with_spec_params, NativeRegionFactGraph, }; -pub(crate) use hoisted_callback_calls::collect_hoisted_callback_calls; +pub(crate) use hoisted_callback_calls::{ + collect_hoisted_callback_calls, collect_loop_called_callee_bindings, +}; pub(crate) use hot_callees::{ collect_alloc_hot_functions, collect_hot_loop_callees, collect_recursion_participants, collect_self_recursive_allocators, diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 8aca884bcb..246a0d9506 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -49,6 +49,10 @@ const BUILD_CACHE_ENV_VARS: &[&str] = &[ // `disable-tail-calls` before the optimizer. It changes the generated // code of the functions it trips on, so it is a cache input. "PERRY_LL_TRE_MAX_ALLOCA_WALK", + // #9071: gates resolving a loop-called immutable callee binding once at + // body entry instead of per call — the two settings emit different call + // sequences, so a cached object from one must not serve the other. + "PERRY_CALLEE_BINDING_RESOLUTION", // #9060: gates whether a reduce accumulator earns the stable-packed fast // clone's numeric proof — with it on, `s += arr[i]` lowers to an inline // fadd instead of `js_dynamic_string_or_number_add`, so the two settings diff --git a/scripts/local_binding_type_allowlist.json b/scripts/local_binding_type_allowlist.json index f251731282..2afb3d7b6d 100644 --- a/scripts/local_binding_type_allowlist.json +++ b/scripts/local_binding_type_allowlist.json @@ -17,6 +17,14 @@ "classification": "metadata-only", "reason": "The module map is copied into the closure's source-type metadata; representation consumers use the separate runtime-proof map or emit a live-value guard." }, + { + "path": "crates/perry-codegen/src/codegen/helpers.rs", + "function": "emit_callee_binding_resolutions", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The declared function type only narrows which loop-called bindings are ATTEMPTED for entry resolution; it licenses no call. The resolution itself goes through js_closure_resolve_arrow_direct_call(handle, arity), which validates the value is an arrow closure of that arity at runtime, so a wrong or stale declared type yields a failed resolution and the ordinary dynamic call, never a wrong callee. Async and generator function types are excluded at this read." + }, { "path": "crates/perry-codegen/src/codegen/mod.rs", "function": "compile_module", @@ -172,18 +180,18 @@ { "path": "crates/perry-codegen/src/expr/property_get.rs", "function": "lower", - "access": "stable_local_type_proof", + "access": "local_type_hint", "count": 1, - "classification": "representation-proven", - "reason": "The proof API supplies only runtime-derived initializer evidence and rejects the binding after any write in the region." + "classification": "runtime-validated", + "reason": "The hint only recognizes the compiler-private synthetic arguments-length marker type; that binding exists solely in direct-call-only clones whose caller materialized the boxed actual-argument count, and the public method retains ordinary Arguments semantics." }, { "path": "crates/perry-codegen/src/expr/property_get.rs", "function": "lower", - "access": "local_type_hint", + "access": "stable_local_type_proof", "count": 1, - "classification": "runtime-validated", - "reason": "The hint only recognizes the compiler-private synthetic arguments-length marker type; that binding exists solely in direct-call-only clones whose caller materialized the boxed actual-argument count, and the public method retains ordinary Arguments semantics." + "classification": "representation-proven", + "reason": "The proof API supplies only runtime-derived initializer evidence and rejects the binding after any write in the region." }, { "path": "crates/perry-codegen/src/expr/property_set.rs",