From 5d8dc414f52a27dc6d728f4db9031451995d5174 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 14 Aug 2023 08:17:56 -0700 Subject: [PATCH] Ensure that constant folding for long->float is handled correctly (#90325) * Ensure that constant folding for long->float is handled correctly * Adding a regression test ensuring long->float conversions are correctly handled * Print failure info for test * Ensure we continue doing the incorrect 2-step conversion for 32-bit, to match codegen * Fix build failure * Ensure test project uses process isolation --- src/coreclr/jit/gentree.cpp | 58 +++++++++++++++++++--- .../JitBlue/Runtime_90323/Runtime_90323.cs | 35 +++++++++++++ .../JitBlue/Runtime_90323/Runtime_90323.csproj | 12 +++++ 3 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.csproj diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index c7df58a..d02ce47 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -14881,6 +14881,21 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) goto CNS_LONG; case TYP_FLOAT: + { +#if defined(TARGET_64BIT) + if (tree->IsUnsigned()) + { + f1 = (float)UINT32(i1); + } + else + { + f1 = (float)INT32(i1); + } +#else + // 32-bit currently does a 2-step conversion, which is incorrect + // but which we are going to take a breaking change around early + // in a release cycle. + if (tree->IsUnsigned()) { f1 = forceCastToFloat(UINT32(i1)); @@ -14889,10 +14904,14 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) { f1 = forceCastToFloat(INT32(i1)); } +#endif + d1 = f1; goto CNS_DOUBLE; + } case TYP_DOUBLE: + { if (tree->IsUnsigned()) { d1 = (double)UINT32(i1); @@ -14902,6 +14921,7 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) d1 = (double)INT32(i1); } goto CNS_DOUBLE; + } default: assert(!"Bad CastToType() in gtFoldExprConst() for a cast from int"); @@ -14982,22 +15002,48 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) goto CNS_LONG; case TYP_FLOAT: - case TYP_DOUBLE: + { +#if defined(TARGET_64BIT) if (tree->IsUnsigned() && (lval1 < 0)) { - d1 = FloatingPointUtils::convertUInt64ToDouble((unsigned __int64)lval1); + f1 = FloatingPointUtils::convertUInt64ToFloat((unsigned __int64)lval1); } else { - d1 = (double)lval1; + f1 = (float)lval1; + } +#else + // 32-bit currently does a 2-step conversion, which is incorrect + // but which we are going to take a breaking change around early + // in a release cycle. + + if (tree->IsUnsigned() && (lval1 < 0)) + { + f1 = forceCastToFloat( + FloatingPointUtils::convertUInt64ToDouble((unsigned __int64)lval1)); + } + else + { + f1 = forceCastToFloat((double)lval1); } +#endif + + d1 = f1; + goto CNS_DOUBLE; + } - if (tree->CastToType() == TYP_FLOAT) + case TYP_DOUBLE: + { + if (tree->IsUnsigned() && (lval1 < 0)) + { + d1 = FloatingPointUtils::convertUInt64ToDouble((unsigned __int64)lval1); + } + else { - f1 = forceCastToFloat(d1); // truncate precision - d1 = f1; + d1 = (double)lval1; } goto CNS_DOUBLE; + } default: assert(!"Bad CastToType() in gtFoldExprConst() for a cast from long"); return tree; diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.cs b/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.cs new file mode 100644 index 0000000..2e6c5a5 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class Runtime_90323 +{ + [MethodImpl(MethodImplOptions.NoInlining)] + private static float ConvertToSingle(long value) => (float)value; + + // 32-bit currently performs a 2-step conversion which causes a different result to be produced + [ConditionalFact(typeof(TestLibrary.PlatformDetection), nameof(TestLibrary.PlatformDetection.Is64BitProcess))] + public static int TestEntryPoint() + { + bool passed = true; + + long value = 0x4000_0040_0000_0001L; + + if (ConvertToSingle(value) != (float)(value)) + { + Console.WriteLine($"Mismatch between codegen and constant folding: {ConvertToSingle(value)} != {(float)(value)}"); + passed = false; + } + + if (BitConverter.SingleToUInt32Bits((float)(value)) != 0x5E80_0001) // 4.6116866E+18f + { + Console.WriteLine($"Mismatch between constant folding and expected value: {(float)(value)} != 4.6116866E+18f; 0x{BitConverter.SingleToUInt32Bits((float)(value)):X8} != 0x5E800001"); + passed = false; + } + + return passed ? 100 : 0; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.csproj new file mode 100644 index 0000000..6175d4e --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.csproj @@ -0,0 +1,12 @@ + + + True + + true + + + + + + + -- 2.7.4