From 43ea736713dd1bf83957c8685a2f8a8a6d2ff88d Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 6 Feb 2017 06:44:14 +0000 Subject: Updating jit/valuenum to properly handle the single-precision versions of the math intrinsics. --- src/jit/utils.cpp | 42 +++++++++++++++- src/jit/utils.h | 2 + src/jit/valuenum.cpp | 136 +++++++++++++++++++++++++++++++++++++++++---------- src/jit/valuenum.h | 1 + 4 files changed, 153 insertions(+), 28 deletions(-) diff --git a/src/jit/utils.cpp b/src/jit/utils.cpp index df5bd2bea6..bb76a730d9 100644 --- a/src/jit/utils.cpp +++ b/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/jit/utils.h b/src/jit/utils.h index 1cd35903dd..dbd5fd5a1f 100644 --- a/src/jit/utils.h +++ b/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/jit/valuenum.cpp b/src/jit/valuenum.cpp index 4d20087b17..cf3af6ab75 100644 --- a/src/jit/valuenum.cpp +++ b/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(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/jit/valuenum.h b/src/jit/valuenum.h index e6e0e43a33..98d5091f5d 100644 --- a/src/jit/valuenum.h +++ b/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). -- cgit v1.2.3