diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index a9b62e8d36e3dd..7e026e0e68bd98 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). + // 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 0b00d40b1e2485..47b706297e06d2 100644 --- a/src/mono/mono/mini/llvm-intrinsics.h +++ b/src/mono/mono/mini/llvm-intrinsics.h @@ -98,18 +98,11 @@ 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. + * `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(MINNUM, minnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MINNUMF, minnum, Generic, LLVMFloatType ()) -INTRINS_OVR(MAXNUM, maxnum, Generic, LLVMDoubleType ()) -INTRINS_OVR(MAXNUMF, maxnum, 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..f3166c3d2881f1 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -7627,24 +7627,33 @@ 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` / + * IEEE 754-2019 minimumNumber/maximumNumber (NaN-suppressing and + * 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) specify. - * On AArch64 this lowers to a single fminnm/fmaxnm instruction. + * 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_MAXNUM; break; - case OP_FMINNUM: iid = INTRINS_MINNUM; break; - case OP_RMAXNUM: iid = INTRINS_MAXNUMF; break; - case OP_RMINNUM: iid = INTRINS_MINNUMF; 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; } 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)));