From 128cdca5f77fd758aa1342d1041dc841101fb85c Mon Sep 17 00:00:00 2001 From: Ralph Date: Fri, 17 Jul 2026 03:37:36 -0700 Subject: [PATCH 1/3] perf(codegen): keep Math.*-result arithmetic on the inline fmul fast path (#6511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Between 0.5.1220 and 0.5.1260, a multiply whose operands are Math.* results (e.g. `Math.sqrt(i) * Math.sin(i * 0.001)`) lost its inline `fmul` and started routing through the BigInt-aware dynamic helper — two non-leaf ToNumeric calls per iteration, ~1.45x wall-clock on the issue's mixed-transcendental kernel. Root cause: the #5970 correctness routing sends every mul/div/mod/sub/ pow/bitwise op through `js_dynamic_` when an operand is not statically numeric, and `is_numeric_expr` had no arms for the dedicated Math.* HIR nodes, so every Math.* call result failed the test. All Math.* lowerings coerce their operands internally (`js_math_to_number`: BigInt/Symbol throw) and emit a raw-f64-returning intrinsic or runtime helper, so their results can never be a boxed BigInt — recognize all of them as statically numeric. Repro A/B (arm64, same runtime archives, alternating runs, min of 6, chk identical to node): 43 ms before, 14 ms after — the fast path now skips the two `js_number_coerce` leaf calls 0.5.1220 still made, so this lands faster than the original baseline too. Verification: new native-proof IR tests pin the routing (inline fmul, no js_dynamic_mul / js_number_coerce for Math-result multiplies; an Any-typed operand still takes the BigInt-aware helper), the full native_proof_regressions suite (242) and perry-codegen unit tests (188) pass, and the bigint/math gap tests (the #5970 surfaces) are byte-identical to node. Fixes #6511 Co-Authored-By: Claude Fable 5 --- .../src/type_analysis/numeric.rs | 47 +++++++ .../tests/native_proof_regressions.rs | 3 + .../math_mul_fastpath.rs | 118 ++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index 528d295b8f..bca0c32c88 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -124,6 +124,53 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { Expr::Binary { op, .. } => !matches!(op, BinaryOp::Add), Expr::Update { .. } => true, Expr::DateNow => true, + // Math.* builtins always evaluate to a real numeric double: every + // lowering coerces its operands internally (ToNumber via + // `lower_math_operand` / `js_math_to_number` — BigInt and Symbol + // throw) and emits a raw-f64-returning intrinsic or runtime helper, + // never a NaN-tagged value. Without these arms, `Math.sqrt(x) * + // Math.sin(y)` fails the "statically numeric" test and the multiply + // is routed through the BigInt-aware dynamic helper — two non-leaf + // re-coercion calls per operation instead of an inline `fmul` + // (#6511, a ~1.45x regression introduced by the #5970 routing). + Expr::MathFloor(..) + | Expr::MathCeil(..) + | Expr::MathRound(..) + | Expr::MathTrunc(..) + | Expr::MathSign(..) + | Expr::MathAbs(..) + | Expr::MathSqrt(..) + | Expr::MathLog(..) + | Expr::MathLog2(..) + | Expr::MathLog10(..) + | Expr::MathPow(..) + | Expr::MathMin(..) + | Expr::MathMax(..) + | Expr::MathMinSpread(..) + | Expr::MathMaxSpread(..) + | Expr::MathImul(..) + | Expr::MathRandom + | Expr::MathSin(..) + | Expr::MathCos(..) + | Expr::MathTan(..) + | Expr::MathAsin(..) + | Expr::MathAcos(..) + | Expr::MathAtan(..) + | Expr::MathAtan2(..) + | Expr::MathCbrt(..) + | Expr::MathHypot(..) + | Expr::MathFround(..) + | Expr::MathF16round(..) + | Expr::MathClz32(..) + | Expr::MathExpm1(..) + | Expr::MathLog1p(..) + | Expr::MathSinh(..) + | Expr::MathCosh(..) + | Expr::MathTanh(..) + | Expr::MathAsinh(..) + | Expr::MathAcosh(..) + | Expr::MathAtanh(..) + | Expr::MathExp(..) => true, // Unary `-x` / `+x` / `~x` always evaluate to a JS number by // ToNumber/ToInt32 semantics, so the result feeds the native f64 // path (#5497, Lever E). The unary lowering coerces the operand diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index a9111731e4..9d0a18b6a8 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -13390,3 +13390,6 @@ mod invalidation; #[path = "native_proof_regressions/integer_modulo.rs"] mod integer_modulo; + +#[path = "native_proof_regressions/math_mul_fastpath.rs"] +mod math_mul_fastpath; diff --git a/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs b/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs new file mode 100644 index 0000000000..eca271f5b3 --- /dev/null +++ b/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs @@ -0,0 +1,118 @@ +//! #6511 — a multiply whose operands are `Math.*` results must stay on the +//! inline `fmul` fast path. The #5970 BigInt-correctness routing sends any +//! operand that is not statically numeric through `js_dynamic_mul` (two +//! non-leaf ToNumeric calls per operation); `Math.*` builtins always return a +//! real numeric double, so `is_numeric_expr` must recognize them and keep the +//! `Math.sqrt(i) * Math.sin(i * 0.001)` accumulator loop call-free. + +use super::*; + +fn mul(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(left), + right: Box::new(right), + } +} + +fn sqrt_times_sin_loop_ir() -> String { + // The issue's repro shape: `for (i = 0; i < 64; i++) acc += Math.sqrt(i) + // * Math.sin(i * 0.001);` + compile_ir( + "math_result_multiply.ts", + vec![ + number_let(1, "acc", true, int(0)), + number_let(3, "iterations", false, int(64)), + for_loop( + 2, + local(3), + vec![Stmt::Expr(Expr::LocalSet( + 1, + Box::new(add( + local(1), + mul( + Expr::MathSqrt(Box::new(local(2))), + Expr::MathSin(Box::new(mul(local(2), number(0.001)))), + ), + )), + ))], + ), + Stmt::Return(Some(local(1))), + ], + ) +} + +#[test] +fn math_result_multiply_stays_inline_fmul() { + let ir = sqrt_times_sin_loop_ir(); + assert!( + ir.contains("call double @llvm.sqrt.f64") && ir.contains("call double @llvm.sin.f64"), + "Math.sqrt / Math.sin should lower to their intrinsics:\n{ir}" + ); + assert!( + ir.contains("fmul double"), + "a multiply of Math.* results must emit an inline fmul:\n{ir}" + ); + assert!( + !ir.contains("call double @js_dynamic_mul"), + "a multiply of Math.* results must not route through the boxed \ + BigInt-aware multiply helper:\n{ir}" + ); + assert!( + !ir.contains("call double @js_number_coerce"), + "Math.* results are already raw doubles — the fast path must not \ + re-coerce them:\n{ir}" + ); +} + +#[test] +fn math_result_divide_and_subtract_stay_inline() { + let ir = compile_ir( + "math_result_div_sub.ts", + vec![ + number_let(1, "a", false, int(9)), + number_let(2, "b", false, int(3)), + Stmt::Return(Some(Expr::Binary { + op: BinaryOp::Div, + left: Box::new(Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(Expr::MathCbrt(Box::new(local(1)))), + right: Box::new(Expr::MathLog(Box::new(local(2)))), + }), + right: Box::new(Expr::MathExp(Box::new(local(2)))), + })), + ], + ); + assert!( + ir.contains("fdiv double") && ir.contains("fsub double"), + "divide/subtract of Math.* results must stay inline:\n{ir}" + ); + assert!( + !ir.contains("call double @js_dynamic_div") && !ir.contains("call double @js_dynamic_sub"), + "divide/subtract of Math.* results must not route through the \ + dynamic helpers:\n{ir}" + ); +} + +#[test] +fn dynamic_operand_multiply_keeps_bigint_aware_helper() { + // #5970's correctness routing must survive: an operand that may be an + // object (possible boxed BigInt / BigInt-returning valueOf) still goes + // through the ToNumeric-running dynamic helper. + let module = module_with_classes_and_params( + "math_dynamic_operand_multiply.ts", + Vec::new(), + vec![param(2, "x", Type::Any)], + Type::Number, + vec![Stmt::Return(Some(mul( + Expr::MathSqrt(Box::new(int(4))), + local(2), + )))], + ); + let ir = compile_ir_for_module_with_opts(module, empty_opts()).unwrap(); + assert!( + ir.contains("call double @js_dynamic_mul"), + "a possibly-object operand must keep the BigInt-aware dynamic \ + multiply routing:\n{ir}" + ); +} From 855fc59bcdd6e3f7372e9e26b3b6587b6ecb1fe9 Mon Sep 17 00:00:00 2001 From: Ralph Date: Fri, 17 Jul 2026 03:44:14 -0700 Subject: [PATCH 2/3] test(codegen): cargo-test-visible unit twin for the Math fmul fast-path routing CodeRabbit review: integration suites under crates/*/tests/*.rs only run on nightly/tag workflows, so mirror the #6511 routing assertions (inline fmul + intrinsics, no js_dynamic_mul/js_number_coerce; Any operand keeps the #5970 dynamic-helper routing) as --lib unit tests under type_analysis/numeric/ where every per-PR cargo-test sees them. Co-Authored-By: Claude Fable 5 --- .../src/type_analysis/numeric.rs | 3 + .../src/type_analysis/numeric/tests.rs | 222 ++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 crates/perry-codegen/src/type_analysis/numeric/tests.rs diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index bca0c32c88..458b311e0d 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -380,3 +380,6 @@ pub(crate) fn is_bool_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { _ => false, } } + +#[cfg(test)] +mod tests; diff --git a/crates/perry-codegen/src/type_analysis/numeric/tests.rs b/crates/perry-codegen/src/type_analysis/numeric/tests.rs new file mode 100644 index 0000000000..86d99bc29d --- /dev/null +++ b/crates/perry-codegen/src/type_analysis/numeric/tests.rs @@ -0,0 +1,222 @@ +//! #6511 — cargo-test-visible coverage for the Math.*-result arithmetic +//! routing (the integration twin lives in +//! `tests/native_proof_regressions/math_mul_fastpath.rs` and only runs on +//! nightly/tag workflows). A multiply of `Math.*` results must stay on the +//! inline `fmul` fast path; a possibly-object operand must keep the +//! BigInt-aware `js_dynamic_mul` routing from #5970. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::{ + BinaryOp, CompareOp, Expr, Function, Module, ModuleInitKind, Param, Stmt, UpdateOp, +}; +use perry_types::Type; + +fn ir_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: false, + 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(), + 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 probe_module(name: &str, params: Vec, body: Vec) -> Module { + Module { + name: name.to_string(), + imports: Vec::new(), + exports: Vec::new(), + classes: Vec::new(), + interfaces: Vec::new(), + type_aliases: Vec::new(), + enums: Vec::new(), + globals: Vec::new(), + functions: vec![Function { + id: 1, + name: "probe".to_string(), + type_params: Vec::new(), + params, + return_type: Type::Number, + 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, + }], + script_global_functions: Vec::new(), + references_global_this: false, + annexb_global_undefined_names: Vec::new(), + init: Vec::new(), + exported_native_instances: Vec::new(), + exported_func_return_native_instances: Vec::new(), + exported_objects: Vec::new(), + exported_functions: Vec::new(), + widgets: Vec::new(), + uses_fetch: false, + uses_webassembly: false, + extern_funcs: Vec::new(), + init_was_unrolled: false, + has_top_level_await: false, + init_kind: ModuleInitKind::Eager, + async_step_closures: std::collections::HashSet::new(), + closure_display_names: std::collections::HashMap::new(), + class_display_names: std::collections::HashMap::new(), + closure_source_text: std::collections::HashMap::new(), + async_generator_funcs: std::collections::HashSet::new(), + gen_param_prologue_len: std::collections::HashMap::new(), + } +} + +fn emitted_ir(module: Module) -> String { + String::from_utf8(compile_module(&module, ir_opts()).unwrap()).expect("LLVM IR should be UTF-8") +} + +fn number_let(id: u32, name: &str, mutable: bool, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Number, + mutable, + init: Some(init), + } +} + +fn mul(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(left), + right: Box::new(right), + } +} + +#[test] +fn math_result_multiply_stays_inline_fmul() { + // The #6511 repro shape: `for (i = 0; i < 64; i++) acc += Math.sqrt(i) + // * Math.sin(i * 0.001);` + let ir = emitted_ir(probe_module( + "math_result_multiply_unit.ts", + Vec::new(), + vec![ + number_let(1, "acc", true, Expr::Integer(0)), + number_let(3, "iterations", false, Expr::Integer(64)), + Stmt::For { + init: Some(Box::new(number_let(2, "i", true, Expr::Integer(0)))), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(2)), + right: Box::new(Expr::LocalGet(3)), + }), + update: Some(Expr::Update { + id: 2, + op: UpdateOp::Increment, + prefix: false, + }), + body: vec![Stmt::Expr(Expr::LocalSet( + 1, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(1)), + right: Box::new(mul( + Expr::MathSqrt(Box::new(Expr::LocalGet(2))), + Expr::MathSin(Box::new(mul(Expr::LocalGet(2), Expr::Number(0.001)))), + )), + }), + ))], + }, + Stmt::Return(Some(Expr::LocalGet(1))), + ], + )); + assert!( + ir.contains("call double @llvm.sqrt.f64") && ir.contains("call double @llvm.sin.f64"), + "Math.sqrt / Math.sin should lower to their intrinsics:\n{ir}" + ); + assert!( + ir.contains("fmul double"), + "a multiply of Math.* results must emit an inline fmul:\n{ir}" + ); + assert!( + !ir.contains("call double @js_dynamic_mul"), + "a multiply of Math.* results must not route through the boxed \ + BigInt-aware multiply helper:\n{ir}" + ); + assert!( + !ir.contains("call double @js_number_coerce"), + "Math.* results are already raw doubles — the fast path must not \ + re-coerce them:\n{ir}" + ); +} + +#[test] +fn dynamic_operand_multiply_keeps_bigint_aware_helper() { + // #5970's correctness routing must survive: an operand that may be an + // object (possible boxed BigInt / BigInt-returning valueOf) still goes + // through the ToNumeric-running dynamic helper. + let ir = emitted_ir(probe_module( + "math_dynamic_operand_multiply_unit.ts", + vec![Param { + id: 2, + name: "x".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + vec![Stmt::Return(Some(mul( + Expr::MathSqrt(Box::new(Expr::Integer(4))), + Expr::LocalGet(2), + )))], + )); + assert!( + ir.contains("call double @js_dynamic_mul"), + "a possibly-object operand must keep the BigInt-aware dynamic \ + multiply routing:\n{ir}" + ); +} From cf752da196d31f3f2bbe8c5f2f9661c88fc1c885 Mon Sep 17 00:00:00 2001 From: Ralph Date: Fri, 17 Jul 2026 03:49:25 -0700 Subject: [PATCH 3/3] test(codegen): make the lone fmul prove the Math-result multiply route CodeRabbit re-review: the MathSin operand's `i * 0.001` emitted its own fmul, so the positive `fmul double` assertion could pass without the outer Math.sqrt(i) * Math.sin(...) being inline. Use a call-free `Math.sin(i)` operand in both test twins so the only fmul in the probe is the multiply under test. Co-Authored-By: Claude Fable 5 --- crates/perry-codegen/src/type_analysis/numeric/tests.rs | 8 +++++--- .../tests/native_proof_regressions/math_mul_fastpath.rs | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/perry-codegen/src/type_analysis/numeric/tests.rs b/crates/perry-codegen/src/type_analysis/numeric/tests.rs index 86d99bc29d..45296fa1da 100644 --- a/crates/perry-codegen/src/type_analysis/numeric/tests.rs +++ b/crates/perry-codegen/src/type_analysis/numeric/tests.rs @@ -138,8 +138,10 @@ fn mul(left: Expr, right: Expr) -> Expr { #[test] fn math_result_multiply_stays_inline_fmul() { - // The #6511 repro shape: `for (i = 0; i < 64; i++) acc += Math.sqrt(i) - // * Math.sin(i * 0.001);` + // The #6511 repro's accumulator-loop shape, with a call-free MathSin + // operand (`Math.sin(i)`, not the repro's `i * 0.001`) so the only + // `fmul` in the function is the Math-result multiply under test: + // `for (i = 0; i < 64; i++) acc += Math.sqrt(i) * Math.sin(i);` let ir = emitted_ir(probe_module( "math_result_multiply_unit.ts", Vec::new(), @@ -165,7 +167,7 @@ fn math_result_multiply_stays_inline_fmul() { left: Box::new(Expr::LocalGet(1)), right: Box::new(mul( Expr::MathSqrt(Box::new(Expr::LocalGet(2))), - Expr::MathSin(Box::new(mul(Expr::LocalGet(2), Expr::Number(0.001)))), + Expr::MathSin(Box::new(Expr::LocalGet(2))), )), }), ))], diff --git a/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs b/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs index eca271f5b3..976f166d81 100644 --- a/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs +++ b/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs @@ -16,8 +16,10 @@ fn mul(left: Expr, right: Expr) -> Expr { } fn sqrt_times_sin_loop_ir() -> String { - // The issue's repro shape: `for (i = 0; i < 64; i++) acc += Math.sqrt(i) - // * Math.sin(i * 0.001);` + // The issue's accumulator-loop shape, with a call-free MathSin operand + // (`Math.sin(i)`, not the repro's `i * 0.001`) so the only `fmul` in the + // function is the Math-result multiply under test: + // `for (i = 0; i < 64; i++) acc += Math.sqrt(i) * Math.sin(i);` compile_ir( "math_result_multiply.ts", vec![ @@ -32,7 +34,7 @@ fn sqrt_times_sin_loop_ir() -> String { local(1), mul( Expr::MathSqrt(Box::new(local(2))), - Expr::MathSin(Box::new(mul(local(2), number(0.001)))), + Expr::MathSin(Box::new(local(2))), ), )), ))],