Updating jit/valuenum to properly handle the single-precision versions of the math...

Commit migrated from https://github.com/dotnet/coreclr/commit/43ea736713dd1bf83957c8685a2f8a8a6d2ff88d

diff --git a/src/coreclr/src/jit/utils.cpp b/src/coreclr/src/jit/utils.cpp
index df5bd2b..bb76a73 100644 (file)
--- a/src/coreclr/src/jit/utils.cpp
+++ b/src/coreclr/src/jit/utils.cpp
@@ -1748,7 +1748,7 @@ double FloatingPointUtils::round(double x)
 {
     // If the number has no fractional part do nothing
     // This shortcut is necessary to workaround precision loss in borderline cases on some platforms
-    if (x == ((double)((__int64)x)))
+    if (x == (double)((INT64)x))
     {
         return x;
     }
@@ -1766,3 +1766,43 @@ double FloatingPointUtils::round(double x)
 
     return _copysign(flrTempVal, x);
 }
+
+// Windows x86 and Windows ARM/ARM64 may not define _copysignf() but they do define _copysign().
+// We will redirect the macro to this other functions if the macro is not defined for the platform.
+// This has the side effect of a possible implicit upcasting for arguments passed in and an explicit
+// downcasting for the _copysign() call.
+#if (defined(_TARGET_X86_) || defined(_TARGET_ARM_) || defined(_TARGET_ARM64_)) && !defined(FEATURE_PAL)
+
+#if !defined(_copysignf)
+#define _copysignf (float)_copysign
+#endif
+
+#endif
+
+// Rounds a single-precision floating-point value to the nearest integer,
+// and rounds midpoint values to the nearest even number.
+// Note this should align with classlib in floatsingle.cpp
+// Specializing for x86 using a x87 instruction is optional since
+// this outcome is identical across targets.
+float FloatingPointUtils::round(float x)
+{
+    // If the number has no fractional part do nothing
+    // This shortcut is necessary to workaround precision loss in borderline cases on some platforms
+    if (x == (float)((INT32)x))
+    {
+        return x;
+    }
+
+    // We had a number that was equally close to 2 integers.
+    // We need to return the even one.
+
+    float tempVal    = (x + 0.5f);
+    float flrTempVal = floorf(tempVal);
+
+    if ((flrTempVal == tempVal) && (fmodf(tempVal, 2.0f) != 0))
+    {
+        flrTempVal -= 1.0f;
+    }
+
+    return _copysignf(flrTempVal, x);
+}

diff --git a/src/coreclr/src/jit/utils.h b/src/coreclr/src/jit/utils.h
index 1cd3590..dbd5fd5 100644 (file)
--- a/src/coreclr/src/jit/utils.h
+++ b/src/coreclr/src/jit/utils.h
@@ -638,6 +638,8 @@ public:
     static unsigned __int64 convertDoubleToUInt64(double d);
 
     static double round(double x);
+
+    static float round(float x);
 };
 
 // The CLR requires that critical section locks be initialized via its ClrCreateCriticalSection API...but

diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp
index 4d20087..cf3af6a 100644 (file)
--- a/src/coreclr/src/jit/valuenum.cpp
+++ b/src/coreclr/src/jit/valuenum.cpp
@@ -1615,6 +1615,7 @@ double ValueNumStore::GetConstantDouble(ValueNum argVN)
 {
     assert(IsVNConstant(argVN));
     var_types argVNtyp = TypeOfVN(argVN);
+    assert(varTypeIsFloating(argVNtyp));
 
     double result = 0;
 
@@ -1632,6 +1633,27 @@ double ValueNumStore::GetConstantDouble(ValueNum argVN)
     return result;
 }
 
+// Given a float constant value number return its value as a float.
+//
+float ValueNumStore::GetConstantSingle(ValueNum argVN)
+{
+    assert(IsVNConstant(argVN));
+    var_types argVNtyp = TypeOfVN(argVN);
+    assert(argVNtyp == TYP_FLOAT);
+
+    float result = 0;
+
+    switch (argVNtyp)
+    {
+        case TYP_FLOAT:
+            result = ConstantValue<float>(argVN);
+            break;
+        default:
+            unreached();
+    }
+    return result;
+}
+
 // Compute the proper value number when the VNFunc has all constant arguments
 // This essentially performs constant folding at value numbering time
 //
@@ -3269,46 +3291,106 @@ ValueNum ValueNumStore::EvalMathFuncUnary(var_types typ, CorInfoIntrinsics gtMat
     assert(arg0VN == VNNormVal(arg0VN));
     if (IsVNConstant(arg0VN) && Compiler::IsTargetIntrinsic(gtMathFN))
     {
-        // If the math intrinsic is not implemented by target-specific instructions, such as implemented
-        // by user calls, then don't do constant folding on it. This minimizes precision loss.
-        // I *may* need separate tracks for the double/float -- if the intrinsic funcs have overloads for these.
-        double arg0Val = GetConstantDouble(arg0VN);
+        assert(varTypeIsFloating(TypeOfVN(arg0VN)));
 
-        double res = 0.0;
-        switch (gtMathFN)
-        {
-            case CORINFO_INTRINSIC_Sin:
-                res = sin(arg0Val);
-                break;
-            case CORINFO_INTRINSIC_Cos:
-                res = cos(arg0Val);
-                break;
-            case CORINFO_INTRINSIC_Sqrt:
-                res = sqrt(arg0Val);
-                break;
-            case CORINFO_INTRINSIC_Abs:
-                res = fabs(arg0Val); // The result and params are doubles.
-                break;
-            case CORINFO_INTRINSIC_Round:
-                res = FloatingPointUtils::round(arg0Val);
-                break;
-            default:
-                unreached(); // the above are the only math intrinsics at the time of this writing.
-        }
         if (typ == TYP_DOUBLE)
         {
+            // Both operand and its result must be of the same floating point type.
+            assert(typ == TypeOfVN(arg0VN));
+
+            // If the math intrinsic is not implemented by target-specific instructions, such as implemented
+            // by user calls, then don't do constant folding on it. This minimizes precision loss.
+            double arg0Val = GetConstantDouble(arg0VN);
+
+            double res = 0.0;
+            switch (gtMathFN)
+            {
+                case CORINFO_INTRINSIC_Sin:
+                    res = sin(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Cos:
+                    res = cos(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Sqrt:
+                    res = sqrt(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Abs:
+                    res = fabs(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Round:
+                    res = FloatingPointUtils::round(arg0Val);
+                    break;
+                default:
+                    unreached(); // the above are the only math intrinsics at the time of this writing.
+            }
+
             return VNForDoubleCon(res);
         }
         else if (typ == TYP_FLOAT)
         {
-            return VNForFloatCon(float(res));
+            // Both operand and its result must be of the same floating point type.
+            assert(typ == TypeOfVN(arg0VN));
+
+            // If the math intrinsic is not implemented by target-specific instructions, such as implemented
+            // by user calls, then don't do constant folding on it. This minimizes precision loss.
+            float arg0Val = GetConstantSingle(arg0VN);
+
+            float res = 0.0f;
+            switch (gtMathFN)
+            {
+                case CORINFO_INTRINSIC_Sin:
+                    res = sinf(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Cos:
+                    res = cosf(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Sqrt:
+                    res = sqrtf(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Abs:
+                    res = fabsf(arg0Val);
+                    break;
+                case CORINFO_INTRINSIC_Round:
+                    res = FloatingPointUtils::round(arg0Val);
+                    break;
+                default:
+                    unreached(); // the above are the only math intrinsics at the time of this writing.
+            }
+
+            return VNForFloatCon(res);
         }
         else
         {
             assert(typ == TYP_INT);
             assert(gtMathFN == CORINFO_INTRINSIC_Round);
 
-            return VNForIntCon(int(res));
+            int res = 0;
+            if (gtMathFN == CORINFO_INTRINSIC_Round)
+            {
+                switch (TypeOfVN(arg0VN))
+                {
+                    case TYP_DOUBLE:
+                    {
+                        double arg0Val = GetConstantDouble(arg0VN);
+                        res            = int(FloatingPointUtils::round(arg0Val));
+                        break;
+                    }
+                    case TYP_FLOAT:
+                    {
+                        float arg0Val = GetConstantSingle(arg0VN);
+                        res           = int(FloatingPointUtils::round(arg0Val));
+                        break;
+                    }
+                    default:
+                        unreached();
+                }
+            }
+            else
+            {
+                unreached(); // the above is the only math intrinsics at the time of this writing.
+            }
+
+            return VNForIntCon(res);
         }
     }
     else

diff --git a/src/coreclr/src/jit/valuenum.h b/src/coreclr/src/jit/valuenum.h
index e6e0e43..98d5091 100644 (file)
--- a/src/coreclr/src/jit/valuenum.h
+++ b/src/coreclr/src/jit/valuenum.h
@@ -205,6 +205,7 @@ private:
     int GetConstantInt32(ValueNum argVN);
     INT64 GetConstantInt64(ValueNum argVN);
     double GetConstantDouble(ValueNum argVN);
+    float GetConstantSingle(ValueNum argVN);
 
     // Assumes that all the ValueNum arguments of each of these functions have been shown to represent constants.
     // Assumes that "vnf" is a operator of the appropriate arity (unary for the first, binary for the second).