diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index 528d295b8f..458b311e0d 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 @@ -333,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..45296fa1da --- /dev/null +++ b/crates/perry-codegen/src/type_analysis/numeric/tests.rs @@ -0,0 +1,224 @@ +//! #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'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(), + 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(Expr::LocalGet(2))), + )), + }), + ))], + }, + 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}" + ); +} 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..976f166d81 --- /dev/null +++ b/crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs @@ -0,0 +1,120 @@ +//! #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 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![ + 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(local(2))), + ), + )), + ))], + ), + 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}" + ); +}