[mono][llvm] Recognize MathF.Abs/Log and Single/Double.MinNumber/MaxNumber as intrinsics#129699
Merged
Merged
Conversation
…umber as intrinsics
Mono's LLVM backend has end-to-end plumbing for OP_FMA, OP_FCOPYSIGN,
OP_SQRTF, etc., but a handful of obvious BCL entry points fall through
to the C# implementations:
* `MathF.Abs(float)` and `MathF.Log(float)` are not recognized in the
MathF block of intrinsics.c, even though MathF.Sqrt/Sin/Cos/Exp/Log2/
Log10/Floor/Ceiling/Truncate all are. (Math.Abs(float) is recognized
via the Math (float) fallback, but Math.Log(float) doesn't exist —
only MathF.Log does.) Adding INTRINS_LOGF + OP_LOGF closes the
gap; MathF.Abs reuses the existing OP_ABSF / INTRINS_ABSF.
* `Single.MinNumber/MaxNumber` and `Double.MinNumber/MaxNumber`
(forwarders for INumber<TSelf>.{Min,Max}Number) are IEEE 754-2008
numNum semantics: when exactly one argument is NaN, return the
non-NaN; when both are NaN, return NaN. These map exactly to
llvm.minnum / llvm.maxnum, which on AArch64 lower to a single
fminnm/fmaxnm instruction. Without recognition the C# bodies inline
to a fcmp+select chain — semantically correct, but several
instructions on every target.
Mono had no recognition for the Single/Double primitive classes
previously; this adds a focused block that only handles MinNumber and
MaxNumber. (Min/Max forward to Math/MathF and are caught by the
existing recognition there.)
Adds:
* OP_LOGF / OP_FMINNUM / OP_FMAXNUM / OP_RMINNUM / OP_RMAXNUM in
mini-ops.h.
* INTRINS_LOGF / INTRINS_MINNUM / INTRINS_MINNUMF / INTRINS_MAXNUM /
INTRINS_MAXNUMF in llvm-intrinsics.h.
* Case handlers in mini-llvm.c (OP_LOGF reuses the existing
scalar-1-arg pattern; the four num/numF ops share a single block
that selects the correct intrinsic by opcode).
* Recognition in intrinsics.c — MathF.Abs/MathF.Log added to the
existing MathF (float) block; MinNumber/MaxNumber added in a new
Single/Double block placed after the Math block.
Validation: on the PR dotnet#129299 branch (emsdk 5.0.6 / LLVM 23) stacked
with the Min/Max NaN fix from PR dotnet#129593, full AOT + LLVM run of
System.Runtime.Tests on iossimulator-arm64:
HalfTests 1442/1442 pass
HalfTests_GenericMath 356/356 pass
SingleTests 1334/1334 pass
DoubleTests 1559/1559 pass
(no regression vs the dotnet#129593-only baseline; covers MinNumber/
MaxNumber on Single/Double)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @vitek-karas |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends Mono’s LLVM backend intrinsic recognition and lowering to cover additional scalar floating-point APIs, enabling more direct mapping to LLVM intrinsics for MathF.Abs, MathF.Log, and the IEEE-754-2019 MinNumber/MaxNumber helpers exposed on Single/Double.
Changes:
- Adds new mini opcodes for
logfand NaN-suppressing min/max-number operations. - Registers new LLVM intrinsic IDs and emits the corresponding intrinsic calls in the LLVM lowering.
- Recognizes
MathF.Abs(float),MathF.Log(float), andSingle/Double.MinNumber/MaxNumberduring intrinsic expansion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/mono/mono/mini/mini-ops.h | Adds new IR opcodes for OP_LOGF and NaN-suppressing min/max-number ops. |
| src/mono/mono/mini/llvm-intrinsics.h | Adds LLVM intrinsic registrations for log (float) and minnum/maxnum (float/double). |
| src/mono/mono/mini/mini-llvm.c | Lowers the new opcodes to llvm.log.* and llvm.minnum/maxnum.* intrinsic calls. |
| src/mono/mono/mini/intrinsics.c | Recognizes the new BCL methods and emits the new mini opcodes when compiling with LLVM. |
Two changes in response to review feedback on dotnet#129699: * Add `Abs` recognition to the new Single/Double intrinsic block (Tanner's review comment). `Single.Abs(float)` and `Double.Abs(double)` are `[Intrinsic]` BCL forwarders to `MathF.Abs`/`Math.Abs`. Today on Mono these usually inline into the existing MathF/Math recognition, but adding direct recognition matches RyuJIT's `lookupPrimitiveFloatNamedIntrinsic` pattern (one lookup path regardless of which entry point a user reaches for) and keeps the lowering working even when the JIT inliner declines. Reuses the existing `OP_ABSF`/`OP_ABS` opcodes + `INTRINS_ABSF`/`INTRINS_FABS` lowering -- no new infrastructure. * Fix incorrect comment wording in three files (Copilot review nits). The MinNumber/MaxNumber comments referenced "Math.MinNumber/MaxNumber" and "MathF.MinNumber/MaxNumber forwarders", but those APIs don't exist on Math/MathF -- only on the primitive Single/Double/Half types via INumber<TSelf>. Also corrects a "numNum/maxNum" typo to "minNum/maxNum" in intrinsics.c. Build verification: `./build.sh mono+libs -os osx -arch arm64 -c Release` clean (0 errors, 0 warnings) on osx-arm64. No behavioral validation re-run -- the Abs recognition reuses already-validated `OP_ABSF`/`OP_ABS` lowering, and the comment changes are text-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tannergooding
approved these changes
Jun 22, 2026
Member
Author
|
/ba-g failure is unrelated |
lewing
added a commit
that referenced
this pull request
Jun 23, 2026
…test exemption (#129727) ## Summary Follow-up to #129593 (`Math.Min/Max` LLVM lowering) and #129699 (`MathF.Abs/Log` + `Single/Double.MinNumber/MaxNumber` intrinsic gaps). Two related WASM-targeted improvements: ### 1. `Math.Truncate` / `Math.CopySign` / `Math.ScaleB` for interpreter + jiterpreter (commit 1) A few BCL math entry points have direct WASM hardware support (`f64.trunc`, `f64.copysign`) or already had interp opcodes (`scalbn` / `scalbnf`) but weren't wired all the way through: * **`Math.Truncate(double)` / `MathF.Truncate(float)`**: not recognized at the IL→MINT_* lowering step in `transform.c`, so the interpreter walked the BCL C# implementations on every call. Adds `MINT_TRUNC`/`MINT_TRUNCF` opcodes in matched D/F-block positions (preserving the existing `(MINT_ASINF - MINT_ASIN)` shift in `transform.c`), `MATH_UNOP(trunc)` / `MATH_UNOPF(truncf)` dispatch in `interp.c`, and recognition in `transform.c`. In the jiterpreter these lower directly to native WASM `f64.trunc` / `f32.trunc` via `mathIntrinsicTable` — no libm import needed. * **`Math.CopySign(double, double)` / `MathF.CopySign(float, float)`**: same shape. New `MINT_COPYSIGN`/`MINT_COPYSIGNF` opcodes + `MATH_BINOP(copysign)` dispatch + recognition. Jiterpreter lowers to native `f64.copysign` / `f32.copysign`. * **`Math.ScaleB(double, int)` / `MathF.ScaleB(float, int)`**: already had `MINT_SCALEB`/`MINT_SCALEBF` opcodes and interp dispatch (via libm `scalbn`/`scalbnf`), but the jiterpreter had no handler — encountering the opcode during tracing forced a bailout to the interpreter for the rest of the trace. The `(float, int) -> float` signature doesn't fit the uniform-float-only `mathIntrinsicTable` shape, so this mirrors the existing `MINT_FMA` special case in `jiterpreter-trace-generator.ts`: direct `callImport("scalbn"/"scalbnf")` plus matching imports and type signatures in `jiterpreter.ts`. No more trace-abort on `Math.ScaleB` in hot WASM code. ### 2. Refine #129593's `[ActiveIssue]` scope on Half/BFloat16 conversion theories (commit 2) PR #129593 added `[ActiveIssue("#103347", TestPlatforms.Browser)]` on `BFloat16Tests.ExplicitConversion_From{Single,Double}` to mirror the existing exemption on `HalfTests.ExplicitConversion_FromSingle`. Both were applied at the `[Theory]` level, disabling the entire parameterized test on Browser — including the ~47 non-NaN rows covering ULP rounding, subnormals, sign handling, and overflow. Only the ~4 NaN rows per theory actually hit the WASM `f32.min`/`f64.min`/`f32.add` NaN-payload canonicalization issue tracked in #103347. Filter the NaN rows out of the `..._TestData` member-data sources when `PlatformDetection.IsWasm` is true (covers both Browser and WASI, which share the underlying WASM-spec NaN canonicalization behavior), and drop the `[ActiveIssue]` markers. Non-NaN rows then run on every platform; on WASM the test summary simply shows fewer rows (no `[ActiveIssue]` skip-noise). ## Validation * `./build.sh mono+libs -os osx -arch arm64 -c Release` clean (0 errors, 0 warnings). * Built `System.Runtime.Tests` for `net11.0-unix` and ran `--filter "FullyQualifiedName~ExplicitConversion_FromSingle|FullyQualifiedName~ExplicitConversion_FromDouble"` on host osx-arm64: **`Passed: 204, Failed: 0, Skipped: 0`**, matching the row count from before the test-data filter on non-WASM. * Browser-WASM verification was attempted locally but blocked by an emscripten `npm install` failure on this host (Python 3.10+ requirement). CI's `runtime-wasm` legs will exercise both the new interp/jiterp opcodes (against BCL `Math` / `MathF` tests) and the test-data filter (against the affected `HalfTests`/`BFloat16Tests` rows) on browser-wasm. ## Risk / scope * Interpreter additions reuse existing `MATH_UNOP`/`MATH_BINOP` macros and the proven jiterpreter `mathIntrinsicTable` lowering — no new infrastructure. * SCALEB jiterpreter path mirrors the existing `MINT_FMA` special case exactly, just with the int second arg. * Test-data filter is pure C# in two test files; the only platform-conditional is `PlatformDetection.IsWasm`. Filter happens at `MemberData` enumeration time, which runs in the target test process (i.e. the WASM runtime on WASM, the host on every other platform), so the row-set behaves consistently with execution. * Drops the existing `HalfTests.ExplicitConversion_FromSingle` `[ActiveIssue]` (had been in place since #103347 was filed). #103347 stays open — the underlying WASM payload-canonicalization is unchanged; we're just narrowing the scope of the test exemption. cc @vargaz @kotlarmilos @lambdageek @tannergooding @pavelsavara > [!NOTE] > This pull request was produced by GitHub Copilot during the AI-assisted investigation that started with #129593 and continued through #129699. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #129593, which switched the scalar
Math.Min/Max(float|double)lowering tollvm.minimum/llvm.maximumand removed themono_use_fast_mathgate on the recognition. While auditing the Mono LLVM-AOT intrinsic recognition for that change, I noticed a few related gaps inintrinsics.c:MathF.Abs(float)—Math.Abs(float)is recognized via theMath(float) fallback, but theMathFblock listsMathF.Sqrt/Sin/Cos/Exp/Floor/Ceiling/Log2/Log10/Truncateand is missingAbs. Reusing the existingOP_ABSF/INTRINS_ABSFlowering closes the asymmetry.MathF.Log(float)—MathF.Log2/Log10are recognized butMathF.Logisn't, and the underlying entry doesn't exist either (noINTRINS_LOGF/OP_LOGF). Adding both lowersMathF.Logto a singlellvm.log.f32intrinsic call.Single.MinNumber/MaxNumber/Double.MinNumber/MaxNumber— the IEEE 754-2019minimumNumber/maximumNumberBCL APIs (NaN-suppressing, signed-zero ordered) live on the primitive types asINumber<TSelf>forwarders. They have no Mono recognition today, so the C# bodies (which inline to aselect(fcmp olt)chain) run end-to-end. Mapping tollvm.minnum.f32/f64/llvm.maxnum.f32/f64lowers them to a single instruction on every target —fminnm/fmaxnmon AArch64,f32.min/f32.maxon WASM (semantically correct for the NaN-suppressing contract),vminss/vminsdfamily on x86.This is also a direct answer to @tannergooding's review question on #129593 about whether the surrounding
Math.Min/Maxfamily has LLVM intrinsics — yes for theNumbervariants.MinMagnitude/MaxMagnitude/MinMagnitudeNumber/MaxMagnitudeNumberdon't have direct LLVM intrinsics and would need hand-rolled IR sequences (fabs+ magnitude reducer + sign recovery); deferring those to a separate change if there's interest.What's added
src/mono/mono/mini/mini-ops.h:OP_LOGF,OP_FMINNUM,OP_FMAXNUM,OP_RMINNUM,OP_RMAXNUM. Placed in matchedD/F-block positions so the existing(MINT_ASINF - MINT_ASIN)style float-shift conventions stay valid.src/mono/mono/mini/llvm-intrinsics.h:INTRINS_LOGF,INTRINS_MINNUM,INTRINS_MINNUMF,INTRINS_MAXNUM,INTRINS_MAXNUMF. Registered viaINTRINS_OVRwith a single overload type — same pattern as the existingINTRINS_OVR(POW, …)/INTRINS_OVR(COPYSIGN, …)/INTRINS_OVR(FMA, …)entries (which are 2-arg and 3-arg intrinsics respectively).src/mono/mono/mini/mini-llvm.c: case handlers.OP_LOGFmirrors the existing scalar-1-arg shape; the four*MINNUM/*MAXNUMops share a single block selecting the intrinsic by opcode.src/mono/mono/mini/intrinsics.c: recognition.MathF.AbsandMathF.Loggo into the existingMathF(float) block.MinNumber/MaxNumbergo into a newSingle/Doubleclass-block placed after theMathblock (Mono had no recognition for the primitiveSingle/Doubleclasses previously — this adds a focused one that only handlesMinNumber/MaxNumber).Validation
On a worktree that stacks this PR plus the WASM-specific follow-up over PR #129593's emsdk 5.0.6 / LLVM 23 branch, full AOT + LLVM run of
System.Runtime.Testsoniossimulator-arm64:System.Tests.HalfTestsSystem.Tests.HalfTests_GenericMathSystem.Tests.SingleTestsSystem.Tests.DoubleTestsTotal 4691/4691, zero failures. The
Single/Doubletest classes exerciseMinNumber/MaxNumberfor both float and double across NaN, signed-zero, infinity, and ordered-value cases.Risk / scope
MinNumber/MaxNumberlowering changes observable behavior in one direction: it preserves NaN-suppression semantics (which the C# body also implements), so consumer-observable results are unchanged.INTRINS_LOGF/INTRINS_MINNUM/INTRINS_MAXNUMare LLVM core intrinsics dating to LLVM 2.x — every toolchain in use here has them.System.Tests.SingleTests/DoubleTests/HalfTestswhich include theNumbervariants.cc @vargaz @kotlarmilos @lambdageek @tannergooding
Note
This pull request was produced by GitHub Copilot during an AI-assisted investigation continuing from #129593.