Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization. Use Min/Max intrinsics if one of arguments is constant. #69434

Merged
merged 13 commits into from
Jun 23, 2022
38 changes: 38 additions & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,9 @@ struct GenTree
#endif // DEBUG

inline bool IsIntegralConst(ssize_t constVal) const;
inline bool IsFloatNaN() const;
inline bool IsFloatPositiveZero() const;
inline bool IsFloatNegativeZero() const;
inline bool IsVectorZero() const;
inline bool IsVectorAllBitsSet() const;
inline bool IsVectorConst();
Expand Down Expand Up @@ -8298,6 +8300,42 @@ inline bool GenTree::IsIntegralConst(ssize_t constVal) const
return false;
}

//-------------------------------------------------------------------
// IsFloatNaN: returns true if this is exactly a const float value of NaN
//
// Returns:
// True if this represents a const floating-point value of NaN.
// Will return false otherwise.
//
inline bool GenTree::IsFloatNaN() const
{
if (IsCnsFltOrDbl())
{
double constValue = AsDblCon()->gtDconVal;
return FloatingPointUtils::isNaN(constValue);
}

return false;
}

//-------------------------------------------------------------------
// IsFloatPositiveZero: returns true if this is exactly a const float value of negative zero (-0.0)
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
//
// Returns:
// True if this represents a const floating-point value of exactly negative zero (-0.0).
// Will return false if the value is negative zero (+0.0).
//
inline bool GenTree::IsFloatNegativeZero() const
{
if (IsCnsFltOrDbl())
{
double constValue = AsDblCon()->gtDconVal;
return FloatingPointUtils::isNegativeZero(constValue);
}

return false;
}

//-------------------------------------------------------------------
// IsFloatPositiveZero: returns true if this is exactly a const float value of postive zero (+0.0)
//
Expand Down
90 changes: 89 additions & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4503,13 +4503,101 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
case NI_System_Math_Log:
case NI_System_Math_Log2:
case NI_System_Math_Log10:
#ifdef TARGET_ARM64
#if defined(TARGET_ARM64)
// ARM64 has fmax/fmin which are IEEE754:2019 minimum/maximum compatible
// TODO-XARCH-CQ: Enable this for XARCH when one of the arguments is a constant
// so we can then emit maxss/minss and avoid NaN/-0.0 handling
case NI_System_Math_Max:
case NI_System_Math_Min:
#endif

#if defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH)
case NI_System_Math_Max:
case NI_System_Math_Min:
{
assert(varTypeIsFloating(callType));

if ((impStackTop().val->IsCnsFltOrDbl() || impStackTop(1).val->IsCnsFltOrDbl()))
{
GenTree* op2 = impPopStack().val;
GenTree* op1 = impPopStack().val;

// 1. NaN is propagated if either input is NaN
if (op1->IsFloatNaN() || op2->IsFloatNaN())
{
GenTree* ret = op1->IsFloatNaN() ? op1 : op2;
retNode = gtNewSimdHWIntrinsicNode(callType, ret, NI_Vector128_ToScalar, callJitType, 16);
SkiFoD marked this conversation as resolved.
Show resolved Hide resolved
break;
}

if (ni == NI_System_Math_Max)
{
// Ensuring +0.0 is returned if both inputs are zero since both inputs being zero,
// of either sign, means the second argument is returned
//
// 2 if both are +0.0 then take op2
// 2.1 if op2 is +0.0 and op1 is 0.0 of either sign then take op2
if ((op1->IsFloatPositiveZero() && op2->IsFloatPositiveZero()) ||
(op1->IsFloatNegativeZero() && op2->IsFloatPositiveZero()))
{
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType, 16);
SkiFoD marked this conversation as resolved.
Show resolved Hide resolved
break;
}
// 2.2 if op1 is +0.0 and op2 is 0.0 of either sign then take op1
else if (op1->IsFloatPositiveZero() && op2->IsFloatNegativeZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType, 16);
break;
}
}

if (ni == NI_System_Math_Min)
{
// Ensuring -0.0 is returned if both inputs are zero since both inputs being zero,
// of either sign, means the second argument is returned
//
// 3 if both are -0.0 then take op2
// 3.1 if op2 is -0.0 and op1 is 0.0 of either sign then take op2
if ((op1->IsFloatNegativeZero() && op2->IsFloatNegativeZero()) ||
(op1->IsFloatPositiveZero() && op2->IsFloatNegativeZero()))
{
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType, 16);
break;
}
// 3.2 if op1 is -0.0 and op2 is 0.0 of either sign then take op1
else if (op1->IsFloatNegativeZero() && op2->IsFloatPositiveZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType, 16);
break;
}
}

// 4. If none fits the conditions
var_types insType = TYP_SIMD16;
NamedIntrinsic hwintrinsicName;

if (callType == TYP_DOUBLE)
{
hwintrinsicName = (ni == NI_System_Math_Max) ? NI_SSE2_Max : NI_SSE2_Min;
}
else
{
hwintrinsicName = (ni == NI_System_Math_Max) ? NI_SSE_Max : NI_SSE_Min;
}

op2 = gtNewSimdHWIntrinsicNode(insType, op2, NI_Vector128_CreateScalarUnsafe, callJitType, 16);
op1 = gtNewSimdHWIntrinsicNode(insType, op1, NI_Vector128_CreateScalarUnsafe, callJitType, 16);

GenTree* res = gtNewSimdHWIntrinsicNode(insType, op1, op2, hwintrinsicName, callJitType, 16);
retNode = gtNewSimdHWIntrinsicNode(callType, res, NI_Vector128_ToScalar, callJitType, 16);
break;
}

// Go down to the direct Math.Min/Math.Max call
break;
}
#endif

case NI_System_Math_Pow:
case NI_System_Math_Round:
case NI_System_Math_Sin:
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2294,6 +2294,22 @@ bool FloatingPointUtils::isNaN(double val)
return (bits & 0x7FFFFFFFFFFFFFFFULL) > 0x7FF0000000000000ULL;
}

//------------------------------------------------------------------------
// isNegativeZero: Determines whether the specified value is negative zero (-0.0)
//
// Arguments:
// val - value to check for (-0.0)
//
// Return Value:
// True if val is (-0.0)
//

bool FloatingPointUtils::isNegativeZero(double val)
{
UINT64 bits = *reinterpret_cast<UINT64*>(&val);
return bits == 0x8000000000000000ULL;
}

//------------------------------------------------------------------------
// maximum: This matches the IEEE 754:2019 `maximum` function
// It propagates NaN inputs back to the caller and
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,8 @@ class FloatingPointUtils

static bool isNaN(double val);

static bool isNegativeZero(double val);

static double maximum(double val1, double val2);

static float maximum(float val1, float val2);
Expand Down