Ensure that constant folding for long->float is handled correctly (#90325)
authorTanner Gooding <tagoo@outlook.com>
Mon, 14 Aug 2023 15:17:56 +0000 (08:17 -0700)
committerGitHub <noreply@github.com>
Mon, 14 Aug 2023 15:17:56 +0000 (08:17 -0700)
* 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
src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.cs [new file with mode: 0644]
src/tests/JIT/Regression/JitBlue/Runtime_90323/Runtime_90323.csproj [new file with mode: 0644]

index c7df58a..d02ce47 100644 (file)
@@ -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 (file)
index 0000000..2e6c5a5
--- /dev/null
@@ -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 (file)
index 0000000..6175d4e
--- /dev/null
@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" />
+    <ProjectReference Include="$(TestLibraryProjectPath)" />
+  </ItemGroup>
+</Project>