perf(codegen): keep Math.*-result arithmetic on the inline fmul fast path (#6511)

[proggeramlug]

Fixes #6511.

Root cause

The #5970 BigInt-correctness routing (632e68e) sends mul/div/mod/sub/pow and the bitwise ops through the ToNumeric-running dynamic helper whenever an operand is not statically numeric — and is_numeric_expr had no arms for the dedicated Math.* HIR nodes (MathSqrt, MathSin, …). Every Math.* call result failed the "statically numeric" test, so Math.sqrt(i) * Math.sin(i * 0.001) lost its inline fmul and paid two non-leaf re-coercion calls per iteration (~1.45x on the issue's kernel).

Fix

Recognize all Math.* expression nodes as statically numeric. Every one of their lowerings coerces its operands internally (js_math_to_number — BigInt and Symbol throw per spec) and emits a raw-f64-returning LLVM intrinsic or runtime helper, never a NaN-tagged value, so a Math.* result can never be the boxed BigInt the #5970 routing exists to catch. The routing itself is untouched: a possibly-object operand still takes the dynamic helper (pinned by a new negative-control test).

Measurements

Issue repro, arm64, identical runtime archives, alternating runs, min of 6, checksum identical to node (chk=1710):

build	time
current main (pre-fix)	43 ms
with this fix	14 ms

Faster than the original 0.5.1220 baseline too — the restored fast path no longer emits the two js_number_coerce leaf calls 0.5.1220 still made, and node runs the same file in 24 ms on this machine.

Verification

• New native_proof_regressions/math_mul_fastpath.rs: the issue's accumulator-loop shape emits inline fmul + llvm.sqrt.f64/llvm.sin.f64 with no js_dynamic_mul/js_number_coerce calls; div/sub stay inline; an Any-typed operand still routes through js_dynamic_mul (fix(runtime,codegen): #5894 — ToNumeric on object operands for BigInt arithmetic/bitwise ops #5970 semantics preserved).
• Full native_proof_regressions suite: 242 passed. perry-codegen unit tests: 188 passed.
• Gap tests over the fix(runtime,codegen): #5894 — ToNumeric on object operands for BigInt arithmetic/bitwise ops #5970 surfaces (test_gap_bigint*, test_gap_number_math, test_gap_2754_2907_2908_bigint_semantics, test_gap_6339_i32_arith_chain_copy, test_gap_number_nondecimal_overflow, test_gap_number_string_2864_2948_2855): byte-identical to node.

No version bump / changelog per the external-contribution convention (maintainer folds metadata at merge).

Note: the required lint job is currently red on every open PR from a main-side issue — the "Public benchmark evidence freshness" step fails with public artifact benchmark inputs changed; regenerate it with ./benchmarks/run_public_baseline.sh (see #6509/#6510/#6512/#6514). Additionally, once that step is fixed, the file-size gate behind it will flag crates/perry-hir/src/lower/widget_decl.rs (2372 > 2000 lines, grew in #6513). Neither is caused by this diff.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved numeric detection for a wide range of Math.* expressions so arithmetic using their results preferentially uses optimized numeric operations when safe.
  • Ensured correct dynamic arithmetic behavior is preserved when operands can be Any/object-like, keeping BigInt-aware lowering as needed.

• Tests

  • Added native and IR/LLVM-based regression tests covering Math.* multiply/divide/subtract routing and intrinsic usage, including cases that must avoid or allow dynamic helpers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b6a67e9-33a1-4bb4-86f2-c56358cb91bf

📥 Commits

Reviewing files that changed from the base of the PR and between 855fc59 and cf752da.

📒 Files selected for processing (2)

• crates/perry-codegen/src/type_analysis/numeric/tests.rs
• crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/perry-codegen/src/type_analysis/numeric/tests.rs

---

📝 Walkthrough

Walkthrough

is_numeric_expr now recognizes listed Expr::Math* builtins as numeric. Codegen tests verify inline arithmetic for Math results and dynamic multiplication for potentially non-numeric operands.

Changes

Math numeric fast paths

Layer / File(s)	Summary
Math expression numeric classification crates/perry-codegen/src/type_analysis/numeric.rs	The numeric-expression predicate classifies the listed Expr::Math* variants as statically numeric and enables module tests.
Synthetic codegen coverage crates/perry-codegen/src/type_analysis/numeric/tests.rs	Tests assert LLVM intrinsics and inline arithmetic for Math results, while Type::Any multiplication uses js_dynamic_mul.
Native arithmetic lowering regressions crates/perry-codegen/tests/native_proof_regressions.rs, crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs	Regression tests verify inline fmul, fdiv, and fsub lowering and retain dynamic multiplication for potentially non-numeric operands.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

• [perf regression] 0.5.1260 routes Math.*-result multiplies through a re-coercing boxed multiply-helper instead of inline fmul (~1.45x on sqrt(x)*sin(y)) #6511 — Covers the Math-result multiplication regression addressed by numeric classification and regression tests.

Possibly related PRs

• PerryTS/perry#5565 — Broadens numeric-expression classification for native numeric fast paths.
• PerryTS/perry#5798 — Updates dynamic multiplication handling used when operands require runtime type handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: keeping Math.* result arithmetic on the inline fmul fast path.
Description check	✅ Passed	It includes the issue, root cause, fix, measurements, and verification, covering the template’s core content despite different headings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs`:
- Around line 45-66: Add equivalent coverage for the Math.sqrt/Math.sin multiply
lowering behavior to cargo-test-visible unit tests under the perry-codegen
source, reusing the existing IR-generation helpers where available. Preserve
assertions for sqrt and sin intrinsics, inline fmul emission, and absence of
js_dynamic_mul and js_number_coerce; keep the existing integration regression
test unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d261dd9e-7bf4-4f0c-bfc7-cdd9cc9bf105

📥 Commits

Reviewing files that changed from the base of the PR and between b2bf975 and 128cdca.

📒 Files selected for processing (3)

• crates/perry-codegen/src/type_analysis/numeric.rs
• crates/perry-codegen/tests/native_proof_regressions.rs
• crates/perry-codegen/tests/native_proof_regressions/math_mul_fastpath.rs

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-codegen/src/type_analysis/numeric/tests.rs (1)

141-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Remove the unrelated multiplication from this fmul assertion.

Line 168 already generates fmul for i * 0.001, so the positive assertion can pass without proving that the outer Math.sqrt(...) * Math.sin(...) was lowered inline. Give MathSin a non-multiplication operand so the only fmul represents the route under test.

Proposed test adjustment

-    // The `#6511` repro shape: `for (i = 0; i < 64; i++) acc += Math.sqrt(i)
-    // * Math.sin(i * 0.001);`
+    // Isolate multiplication between two Math.* results so any emitted
+    // `fmul` necessarily represents the routing under test.
...
-                            Expr::MathSin(Box::new(mul(Expr::LocalGet(2), Expr::Number(0.001)))),
+                            Expr::MathSin(Box::new(Expr::Number(0.001))),

Also applies to: 168-168, 180-183

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/type_analysis/numeric/tests.rs` around lines 141 -
142, Update the numeric test around MathSin and its fmul assertions so MathSin
receives a non-multiplication operand, removing the unrelated outer
Math.sqrt(...) * Math.sin(...) multiplication. Ensure the remaining fmul is
produced only by the route under test, while preserving the existing positive
and negative assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-codegen/src/type_analysis/numeric/tests.rs`:
- Around line 141-142: Update the numeric test around MathSin and its fmul
assertions so MathSin receives a non-multiplication operand, removing the
unrelated outer Math.sqrt(...) * Math.sin(...) multiplication. Ensure the
remaining fmul is produced only by the route under test, while preserving the
existing positive and negative assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 222bc2c1-b665-439c-9158-dcf36d2215cc

📥 Commits

Reviewing files that changed from the base of the PR and between 128cdca and 855fc59.

📒 Files selected for processing (2)

• crates/perry-codegen/src/type_analysis/numeric.rs
• crates/perry-codegen/src/type_analysis/numeric/tests.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/perry-codegen/src/type_analysis/numeric.rs

[proggeramlug]

Nitpick addressed in the latest commit: the MathSin operand is now the call-free Math.sin(i) in both test twins, so the probe function's only fmul is the Math-result multiply under test — the positive assertion no longer gets a free pass from the repro's inner i * 0.001 multiply.