[WIP] Fix for x64 UInt64 to Double conversions (Issue 43895)

src/coreclr/jit/codegenxarch.cpp

@@ -6397,7 +6397,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
     assert(!varTypeIsFloating(srcType) && varTypeIsFloating(dstType));
 
 #if !defined(TARGET_64BIT)
-    // We expect morph to replace long to float/double casts with helper calls
+    // On x86 We expect morph to replace long to float/double casts with helper calls
     noway_assert(!varTypeIsLong(srcType));
 #endif // !defined(TARGET_64BIT)
 
@@ -6449,44 +6449,79 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
     // Note that here we need to specify srcType that will determine
     // the size of source reg/mem operand and rex.w prefix.
     instruction ins = ins_FloatConv(dstType, TYP_INT);
-    GetEmitter()->emitInsBinary(ins, emitTypeSize(srcType), treeNode, op1);
 
-    // Handle the case of srcType = TYP_ULONG. SSE2 conversion instruction
-    // will interpret ULONG value as LONG.  Hence we need to adjust the
-    // result if sign-bit of srcType is set.
-    if (srcType == TYP_ULONG)
+    if (srcType != TYP_ULONG)
     {
+        GetEmitter()->emitInsBinary(ins, emitTypeSize(srcType), treeNode, op1);
+    }
+    else // (srcType == TYP_ULONG)
+    {
+// Above for 32-bit x86 we have a noway_assert(!varTypeIsLong(srcType))
+//
+
+#ifdef TARGET_64BIT
+        // Handle the case of srcType = TYP_ULONG. SSE2 conversion instruction
+        // will interpret ULONG value as LONG.  Hence we need to adjust the
+        // result if sign-bit of srcType is set.
+
         // The instruction sequence below is less accurate than what clang
         // and gcc generate. However, we keep the current sequence for backward compatibility.
-        // If we change the instructions below, FloatingPointUtils::convertUInt64ToDobule
+        // If we change the instructions below, FloatingPointUtils::convertUInt64ToDouble
         // should be also updated for consistent conversion result.
+
         assert(dstType == TYP_DOUBLE);
         assert(op1->isUsedFromReg());
 
         // Set the flags without modifying op1.
         // test op1Reg, op1Reg
         inst_RV_RV(INS_test, op1->GetRegNum(), op1->GetRegNum(), srcType);
 
-        // No need to adjust result if op1 >= 0 i.e. positive
-        // Jge label
-        BasicBlock* label = genCreateTempLabel();
-        inst_JMP(EJ_jge, label);
+        // We will need to adjust result if op1 has the sign bit set
+        // Js hiLabel
+        BasicBlock* hiLabel = genCreateTempLabel();
+        inst_JMP(EJ_js, hiLabel);
+
+        // No adjustment is necessary, the sign bit was clear
+        //
+        GetEmitter()->emitInsBinary(ins, emitTypeSize(srcType), treeNode, op1);
+        BasicBlock* joinLabel = genCreateTempLabel();
+        inst_JMP(EJ_jmp, joinLabel);
+
+        genDefineTempLabel(hiLabel);
+
+        // We need an extra integer register
+        //
+        regNumber srcReg = op1->GetRegNum();
+        regNumber tmpReg = treeNode->GetSingleTempReg(RBM_ALLINT);
+        assert(genIsValidIntReg(tmpReg));
+
+        genSetRegToIcon(tmpReg, (ssize_t) -0x8000000000000000i64, TYP_I_IMPL);
+
+        // clear the sign bit
+        inst_RV_RV(INS_xor, srcReg, tmpReg, srcType);
+
+        // perform conversion using low 63-bits
+        GetEmitter()->emitInsBinary(ins, emitTypeSize(srcType), treeNode, op1);
 
         // Adjust the result
-        // result = result + 0x43f00000 00000000
-        // addsd resultReg,  0x43f00000 00000000
+        // result = result + 0x43e00000 00000000
+        // addsd resultReg,  0x43e00000 00000000
         CORINFO_FIELD_HANDLE* cns = &u8ToDblBitmask;
         if (*cns == nullptr)
         {
             double d;
             static_assert_no_msg(sizeof(double) == sizeof(__int64));
-            *((__int64*)&d) = 0x43f0000000000000LL;
+            *((__int64*)&d) = 0x43e0000000000000LL;
 
             *cns = GetEmitter()->emitFltOrDblConst(d, EA_8BYTE);
         }
         GetEmitter()->emitIns_R_C(INS_addsd, EA_8BYTE, treeNode->GetRegNum(), *cns, 0);
 
-        genDefineTempLabel(label);
+        // restore the sign bit to srcReg, leaving srcReg unchanged
+        inst_RV_RV(INS_xor, srcReg, tmpReg, srcType);
+
+        genDefineTempLabel(joinLabel);
+#endif // TARGET_64BIT
     }
 
     genProduceReg(treeNode);

src/coreclr/jit/lsraxarch.cpp

@@ -2624,6 +2624,11 @@ int LinearScan::BuildCast(GenTreeCast* cast)
         // rather require it to be different from operand's reg.
         buildInternalIntRegisterDefForNode(cast);
     }
+    // A cast from TYP_ULONG to TYP_DOUBLE requires a temporary register
+    if (cast->IsUnsigned() && (castType == TYP_DOUBLE) && varTypeIsLong(srcType))
+    {
+        buildInternalIntRegisterDefForNode(cast);
+    }
 #endif
 
     int srcCount = BuildOperandUses(src, candidates);

src/coreclr/jit/utils.cpp

@@ -1914,33 +1914,7 @@ unsigned CountDigits(float num, unsigned base /* = 10 */)
 
 double FloatingPointUtils::convertUInt64ToDouble(unsigned __int64 uIntVal)
 {
-    __int64 s64 = uIntVal;
-    double  d;
-    if (s64 < 0)
-    {
-#if defined(TARGET_XARCH)
-        // RyuJIT codegen and clang (or gcc) may produce different results for casting uint64 to
-        // double, and the clang result is more accurate. For example,
-        //    1) (double)0x84595161401484A0UL --> 43e08b2a2c280290  (RyuJIT codegen or VC++)
-        //    2) (double)0x84595161401484A0UL --> 43e08b2a2c280291  (clang or gcc)
-        // If the folding optimization below is implemented by simple casting of (double)uint64_val
-        // and it is compiled by clang, casting result can be inconsistent, depending on whether
-        // the folding optimization is triggered or the codegen generates instructions for casting. //
-        // The current solution is to force the same math as the codegen does, so that casting
-        // result is always consistent.
-
-        // d = (double)(int64_t)uint64 + 0x1p64
-        uint64_t adjHex = 0x43F0000000000000UL;
-        d               = (double)s64 + *(double*)&adjHex;
-#else
-        d                             = (double)uIntVal;
-#endif
-    }
-    else
-    {
-        d = (double)uIntVal;
-    }
-    return d;
+    return (double)uIntVal;
 }
 
 float FloatingPointUtils::convertUInt64ToFloat(unsigned __int64 u64)

src/tests/JIT/Regression/JitBlue/Runtime_43895/Runtime_43895.cs

@@ -0,0 +1,29 @@
+// 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;
+
+public class Runtime_43895
+{
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool Test1(double d, ulong ul)
+    {
+        return (d == (double) ul);
+    }
+
+    public static int Main()
+    {
+        bool b1 = Test1(10648738977740919977d,  10648738977740919977ul);
+
+        if (!b1)
+        {
+            Console.WriteLine("FAILED");
+            return 101;
+        }
+
+        Console.WriteLine("Passed");
+        return 100;
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_43895/Runtime_43895.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>

src/tests/JIT/SIMD/VectorConvert.cs

@@ -179,11 +179,18 @@ public static int VectorConvertDoubleInt64(Vector<Double> A)
             Vector<Int64> B = Vector.ConvertToInt64(A);
             Vector<Double> C = Vector.ConvertToDouble(B);
 
+	    Console.WriteLine("int VectorConvertDoubleInt64(Vector<Double> A) => B[] => C[]");
+	    Console.WriteLine();
+
             int returnVal = Pass;
             for (int i = 0; i < Vector<Double>.Count; i++)
             {
                 Int64 int64Val = (Int64)A[i];
                 Double cvtDblVal = (Double)int64Val;
+
+                Console.WriteLine("A[{0}] = {1}, B[{0}] = {2}, C[{0}] = {3}, int64Val ={4,9:X}, cvtDblVal = {5}",
+                                  i, A[i], B[i], C[i], int64Val, cvtDblVal);
+
                 if (B[i] != int64Val)
                 {
                     Console.WriteLine("B[" + i + "] = " + B[i] + ", int64Val = " + int64Val);
@@ -195,6 +202,8 @@ public static int VectorConvertDoubleInt64(Vector<Double> A)
                     returnVal = Fail;
                 }
             }
+	    Console.WriteLine();
+
             return returnVal;
         }
 
@@ -203,18 +212,34 @@ public static int VectorConvertDoubleUInt64(Vector<Double> A)
             Vector<UInt64> B = Vector.ConvertToUInt64(A);
             Vector<Double> C = Vector.ConvertToDouble(B);
 
+	    Console.WriteLine("int VectorConvertDoubleUInt64(Vector<Double> A) => B[] => C[]");
+	    Console.WriteLine();
+
             int returnVal = Pass;
             for (int i = 0; i < Vector<Double>.Count; i++)
             {
                 UInt64 uint64Val = (UInt64)A[i];
                 Double cvtDblVal = (Double)uint64Val;
+
+                Console.WriteLine("A[{0}] = {1}, B[{0}] = {2}, C[{0}] = {3}, uint64Val ={4,9:X}, cvtDblVal = {5}",
+                                  i, A[i], B[i], C[i], uint64Val, cvtDblVal);
+
                 if ((B[i] != uint64Val) || (C[i] != cvtDblVal))
                 {
-                    Console.WriteLine("A[{0}] = {1}, B[{0}] = {2}, C[{0}] = {3}, uint64Val = {4}, cvtDblVal = {5}",
-                                      i, A[i], B[i], C[i], uint64Val, cvtDblVal);
+                    if (B[i] != uint64Val)
+                    {
+                        Console.WriteLine("Fail: B[i] != uint64Val :  B[i] and {0,9:X}", uint64Val);
+                    }
+                    if (C[i] != cvtDblVal)
+                    {
+                        Console.WriteLine("Fail: C[i] != cvtDblVal :  C[i] and {0}", cvtDblVal);
+                    }
+
                     returnVal = Fail;
                 }
             }
+	    Console.WriteLine();
+
             return returnVal;
         }
 
@@ -528,7 +553,8 @@ static int Main()
                 returnVal = Fail;
             }
         }
-
+
+
         for (int i = 0; i < 10; i++)
         {
             Vector<Double> doubleVector1 = getRandomVector<Double>(doubles, i);