From bb74bdf4a784b6139e8bce371b1773817efa4e3a Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 21 Jul 2026 15:24:09 -0700 Subject: [PATCH 1/2] Fix Mono LLVM lowering of MinNumber/MaxNumber to preserve the sign of zero double.MinNumber/MaxNumber (IEEE 754-2019 minimumNumber/maximumNumber) treat -0 as less than +0, but the scalar lowering used llvm.minnum/maxnum, which leave the sign of zero unspecified -- e.g. minnum(+0, -0) returns +0 on x86. Lower to llvm.minimumnum/maximumnum instead, which are NaN-suppressing and sign-of-zero aware. Min/Max already use llvm.minimum/maximum and are correct. Un-skips Runtime_130831 on Mono now that the divergence is fixed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/mono/mono/mini/intrinsics.c | 8 +++--- src/mono/mono/mini/llvm-intrinsics.h | 25 +++++++++++-------- src/mono/mono/mini/mini-llvm.c | 20 ++++++++------- .../JitBlue/Runtime_130831/Runtime_130831.cs | 1 - 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index a9b62e8d36e3dd..17a9c14f81b464 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -295,10 +295,10 @@ llvm_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign // corresponding APIs on Math/MathF -- e.g. there is no `Math.MinNumber` -- // so the existing Math/MathF block above doesn't catch them. // - // * MinNumber / MaxNumber: IEEE 754-2008 minNum / maxNum (NaN-suppressing). - // Lower to llvm.minnum / llvm.maxnum, which on AArch64 maps to a single - // fminnm / fmaxnm instruction; matches the BCL spec "if either is NaN - // return the non-NaN; if both are NaN return NaN". + // * MinNumber / MaxNumber: IEEE 754-2019 minimumNumber / maximumNumber + // (NaN-suppressing and sign-of-zero aware, treating -0 as less than +0). + // Lower to llvm.minimumnum / llvm.maximumnum, which on AArch64 maps to a + // single fminnm / fmaxnm instruction; see mini-llvm.c and llvm-intrinsics.h. // * Abs: BCL forwarder to MathF.Abs / Math.Abs. Today this usually inlines // into the Math/MathF recognition above, but adding direct recognition // keeps the lowering working even if the JIT inliner declines. diff --git a/src/mono/mono/mini/llvm-intrinsics.h b/src/mono/mono/mini/llvm-intrinsics.h index 0b00d40b1e2485..e9e77fe39924b0 100644 --- a/src/mono/mono/mini/llvm-intrinsics.h +++ b/src/mono/mono/mini/llvm-intrinsics.h @@ -98,18 +98,21 @@ INTRINS_OVR(TRUNCF, trunc, Generic, LLVMFloatType ()) INTRINS_OVR(COPYSIGN, copysign, Generic, LLVMDoubleType ()) INTRINS_OVR(COPYSIGNF, copysign, Generic, LLVMFloatType ()) /* - * IEEE 754-2008 minNum/maxNum (NaN-suppressing). When exactly one operand - * is NaN they return the other; when both are NaN they return NaN. This is - * what `float.MinNumber` / `double.MinNumber` (and the Max variants, - * surfaced via INumber on the primitive Single/Double/Half types) - * are documented to do, and on AArch64 these lower to single fminnm/fmaxnm - * instructions. Use llvm.minimum/maximum (see above) for the NaN-propagating - * Math.Min/Math.Max instead. + * IEEE 754-2019 minimumNumber/maximumNumber (NaN-suppressing and sign-of-zero + * aware). When exactly one operand is NaN they return the other; when both are + * NaN they return NaN; and they treat -0 as less than +0. This is what + * `float.MinNumber` / `double.MinNumber` (and the Max variants, surfaced via + * INumber on the primitive Single/Double/Half types) are documented to + * do, and on AArch64 these lower to single fminnm/fmaxnm instructions. We avoid + * llvm.minnum/maxnum (IEEE 754-2008 minNum/maxNum) because those leave the + * sign of zero unspecified -- e.g. minnum(+0, -0) may return +0 on x86. + * Use llvm.minimum/maximum (see above) for the NaN-propagating Math.Min/Math.Max + * instead. */ -INTRINS_OVR(MINNUM, minnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MINNUMF, minnum, Generic, LLVMFloatType ()) -INTRINS_OVR(MAXNUM, maxnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MAXNUMF, maxnum, Generic, LLVMFloatType ()) +INTRINS_OVR(MINIMUMNUM, minimumnum, Generic, LLVMDoubleType ()) +INTRINS_OVR(MINIMUMNUMF, minimumnum, Generic, LLVMFloatType ()) +INTRINS_OVR(MAXIMUMNUM, maximumnum, Generic, LLVMDoubleType ()) +INTRINS_OVR(MAXIMUMNUMF, maximumnum, Generic, LLVMFloatType ()) INTRINS_OVR(EXPECT_I8, expect, Generic, LLVMInt8Type ()) INTRINS_OVR(EXPECT_I1, expect, Generic, LLVMInt1Type ()) INTRINS_OVR(CTPOP_I32, ctpop, Generic, LLVMInt32Type ()) diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index 324aca3ed4eda2..1b608b2294372f 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -7627,21 +7627,23 @@ MONO_RESTORE_WARNING case OP_RMINNUM: case OP_RMAXNUM: { /* - * IEEE 754-2008 minNum/maxNum (NaN-suppressing). Maps directly to - * llvm.minnum/maxnum, which is what `float.MinNumber` / - * `double.MinNumber` (and the Max variants, surfaced via - * INumber on the primitive Single/Double/Half types) specify. - * On AArch64 this lowers to a single fminnm/fmaxnm instruction. + * IEEE 754-2019 minimumNumber/maximumNumber (NaN-suppressing and + * sign-of-zero aware). Maps directly to llvm.minimumnum/maximumnum, + * which is what `float.MinNumber` / `double.MinNumber` (and the Max + * variants, surfaced via INumber on the primitive + * Single/Double/Half types) specify. On AArch64 this lowers to a + * single fminnm/fmaxnm instruction. We avoid llvm.minnum/maxnum + * because those leave the sign of zero unspecified. */ gboolean is_r4 = ins->opcode == OP_RMINNUM || ins->opcode == OP_RMAXNUM; LLVMTypeRef t = is_r4 ? LLVMFloatType () : LLVMDoubleType (); LLVMValueRef args [2] = { convert (ctx, lhs, t), convert (ctx, rhs, t) }; IntrinsicId iid; switch (ins->opcode) { - case OP_FMAXNUM: iid = INTRINS_MAXNUM; break; - case OP_FMINNUM: iid = INTRINS_MINNUM; break; - case OP_RMAXNUM: iid = INTRINS_MAXNUMF; break; - case OP_RMINNUM: iid = INTRINS_MINNUMF; break; + case OP_FMAXNUM: iid = INTRINS_MAXIMUMNUM; break; + case OP_FMINNUM: iid = INTRINS_MINIMUMNUM; break; + case OP_RMAXNUM: iid = INTRINS_MAXIMUMNUMF; break; + case OP_RMINNUM: iid = INTRINS_MINIMUMNUMF; break; default: g_assert_not_reached (); break; } values [ins->dreg] = call_intrins (ctx, iid, args, dname); diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_130831/Runtime_130831.cs b/src/tests/JIT/Regression/JitBlue/Runtime_130831/Runtime_130831.cs index 37597d88bc61ea..42a58b1d9c5b48 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_130831/Runtime_130831.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_130831/Runtime_130831.cs @@ -12,7 +12,6 @@ public static class Runtime_130831 // Compare the exact bit pattern: Double/Single.Equals treat -0.0 and +0.0 as equal, // so a plain Assert.Equal would not observe a wrong-signed-zero result. [Fact] - [SkipOnMono("https://github.com/dotnet/runtime/issues/131130", TestPlatforms.Any)] public static void TestEntryPoint() { Assert.Equal(BitConverter.DoubleToInt64Bits(-0.0), BitConverter.DoubleToInt64Bits(MinNegZeroConst(+0.0))); From ae552059b8b06da1546e37ba99eeee6d7bc65068 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 22 Jul 2026 04:35:06 -0700 Subject: [PATCH 2/2] Compose MinNumber/MaxNumber from llvm.minimum/maximum to avoid the miscompiled minimumnum/maximumnum llvm.minimumnum/maximumnum have the right IEEE 754-2019 semantics but are miscompiled by the x86 backend in the LLVM version Mono builds against, regressing Runtime_98068.TestMinNumber on the Mono LLVM AOT leg. Lower MinNumber/MaxNumber by composing llvm.minimum/maximum (sign-of-zero aware, already used for Math.Min/Max) with an explicit NaN fixup instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/mono/mono/mini/intrinsics.c | 4 +-- src/mono/mono/mini/llvm-intrinsics.h | 18 +++---------- src/mono/mono/mini/mini-llvm.c | 39 ++++++++++++++++------------ 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index 17a9c14f81b464..7e026e0e68bd98 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -297,8 +297,8 @@ llvm_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign // // * MinNumber / MaxNumber: IEEE 754-2019 minimumNumber / maximumNumber // (NaN-suppressing and sign-of-zero aware, treating -0 as less than +0). - // Lower to llvm.minimumnum / llvm.maximumnum, which on AArch64 maps to a - // single fminnm / fmaxnm instruction; see mini-llvm.c and llvm-intrinsics.h. + // Lowered in mini-llvm.c by composing llvm.minimum / llvm.maximum with an + // explicit NaN fixup; see the OP_FMINNUM case there and llvm-intrinsics.h. // * Abs: BCL forwarder to MathF.Abs / Math.Abs. Today this usually inlines // into the Math/MathF recognition above, but adding direct recognition // keeps the lowering working even if the JIT inliner declines. diff --git a/src/mono/mono/mini/llvm-intrinsics.h b/src/mono/mono/mini/llvm-intrinsics.h index e9e77fe39924b0..47b706297e06d2 100644 --- a/src/mono/mono/mini/llvm-intrinsics.h +++ b/src/mono/mono/mini/llvm-intrinsics.h @@ -98,21 +98,11 @@ INTRINS_OVR(TRUNCF, trunc, Generic, LLVMFloatType ()) INTRINS_OVR(COPYSIGN, copysign, Generic, LLVMDoubleType ()) INTRINS_OVR(COPYSIGNF, copysign, Generic, LLVMFloatType ()) /* - * IEEE 754-2019 minimumNumber/maximumNumber (NaN-suppressing and sign-of-zero - * aware). When exactly one operand is NaN they return the other; when both are - * NaN they return NaN; and they treat -0 as less than +0. This is what - * `float.MinNumber` / `double.MinNumber` (and the Max variants, surfaced via - * INumber on the primitive Single/Double/Half types) are documented to - * do, and on AArch64 these lower to single fminnm/fmaxnm instructions. We avoid - * llvm.minnum/maxnum (IEEE 754-2008 minNum/maxNum) because those leave the - * sign of zero unspecified -- e.g. minnum(+0, -0) may return +0 on x86. - * Use llvm.minimum/maximum (see above) for the NaN-propagating Math.Min/Math.Max - * instead. + * `float.MinNumber` / `double.MinNumber` (and the Max variants) are lowered in + * mini-llvm.c by composing llvm.minimum/maximum (above) with an explicit NaN + * fixup; see the OP_FMINNUM case there for why we don't use llvm.minnum/maxnum + * or llvm.minimumnum/maximumnum directly. */ -INTRINS_OVR(MINIMUMNUM, minimumnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MINIMUMNUMF, minimumnum, Generic, LLVMFloatType ()) -INTRINS_OVR(MAXIMUMNUM, maximumnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MAXIMUMNUMF, maximumnum, Generic, LLVMFloatType ()) INTRINS_OVR(EXPECT_I8, expect, Generic, LLVMInt8Type ()) INTRINS_OVR(EXPECT_I1, expect, Generic, LLVMInt1Type ()) INTRINS_OVR(CTPOP_I32, ctpop, Generic, LLVMInt32Type ()) diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index 1b608b2294372f..f3166c3d2881f1 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -7628,25 +7628,32 @@ MONO_RESTORE_WARNING case OP_RMAXNUM: { /* * IEEE 754-2019 minimumNumber/maximumNumber (NaN-suppressing and - * sign-of-zero aware). Maps directly to llvm.minimumnum/maximumnum, - * which is what `float.MinNumber` / `double.MinNumber` (and the Max - * variants, surfaced via INumber on the primitive - * Single/Double/Half types) specify. On AArch64 this lowers to a - * single fminnm/fmaxnm instruction. We avoid llvm.minnum/maxnum - * because those leave the sign of zero unspecified. + * sign-of-zero aware), as specified by `float.MinNumber` / + * `double.MinNumber` (and the Max variants, surfaced via + * INumber on the primitive Single/Double/Half types). + * + * We compose this from llvm.minimum/maximum (sign-of-zero aware but + * NaN-propagating) plus an explicit NaN fixup, rather than lowering + * directly to an intrinsic: llvm.minnum/maxnum leave the sign of zero + * unspecified (minnum(+0, -0) may return +0 on x86, see dotnet/runtime + * #131130) and llvm.minimumnum/maximumnum are miscompiled by the x86 + * backend in the LLVM version we build against. When exactly one operand + * is NaN we return the other; when both are NaN the NaN flows through. */ gboolean is_r4 = ins->opcode == OP_RMINNUM || ins->opcode == OP_RMAXNUM; + gboolean is_max = ins->opcode == OP_FMAXNUM || ins->opcode == OP_RMAXNUM; LLVMTypeRef t = is_r4 ? LLVMFloatType () : LLVMDoubleType (); - LLVMValueRef args [2] = { convert (ctx, lhs, t), convert (ctx, rhs, t) }; - IntrinsicId iid; - switch (ins->opcode) { - case OP_FMAXNUM: iid = INTRINS_MAXIMUMNUM; break; - case OP_FMINNUM: iid = INTRINS_MINIMUMNUM; break; - case OP_RMAXNUM: iid = INTRINS_MAXIMUMNUMF; break; - case OP_RMINNUM: iid = INTRINS_MINIMUMNUMF; break; - default: g_assert_not_reached (); break; - } - values [ins->dreg] = call_intrins (ctx, iid, args, dname); + LLVMValueRef l = convert (ctx, lhs, t); + LLVMValueRef r = convert (ctx, rhs, t); + LLVMValueRef args [2] = { l, r }; + IntrinsicId iid = is_max ? (is_r4 ? INTRINS_MAXIMUMF : INTRINS_MAXIMUM) + : (is_r4 ? INTRINS_MINIMUMF : INTRINS_MINIMUM); + LLVMValueRef result = call_intrins (ctx, iid, args, ""); + LLVMValueRef l_nan = LLVMBuildFCmp (builder, LLVMRealUNO, l, l, ""); + LLVMValueRef r_nan = LLVMBuildFCmp (builder, LLVMRealUNO, r, r, ""); + result = LLVMBuildSelect (builder, r_nan, l, result, ""); + result = LLVMBuildSelect (builder, l_nan, r, result, dname); + values [ins->dreg] = result; break; }