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.

Sign up for GitHub

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
Merged

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

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
363ef7e
Add xarch optimization for min/max (#65625)
SkiFoD May 19, 2022
7331188
Changes according to the requirements (#65625)
SkiFoD May 19, 2022
70ff940
Merge branch 'main' into skifod/issue-65625
SkiFoD Jun 2, 2022
7f58b25
Draft for Math.Max/Math.Min optimization (#65625)
SkiFoD Jun 2, 2022
0f74bdc
Draft for optimizing Math.Max/Math.Min with a const (#65625)
SkiFoD Jun 3, 2022
d1b4594
Fix tests (#65625)
SkiFoD Jun 3, 2022
9347238
Refactoring of the conditions (#65625)
SkiFoD Jun 3, 2022
75a7135
Fix of the summary (#65625)
SkiFoD Jun 20, 2022
1ec62d0
Merge branch 'main' into skifod/issue-65625
SkiFoD Jun 20, 2022
fd79612
Refactoring due to the PR comments (#65625)
SkiFoD Jun 20, 2022
1aed08f
Add spilling side effect + Fix of formats (#65625)
SkiFoD Jun 22, 2022
b271ac9
Update src/coreclr/jit/importer.cpp
Jun 22, 2022
29d7af9
Update src/coreclr/jit/importer.cpp
Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 45 additions & 67 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4517,106 +4517,84 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
{
assert(varTypeIsFloating(callType));

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

unsigned int insSimdSize = 16; //(callType == TYP_DOUBLE) ? 32 : 16;

// 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, insSimdSize);
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)
{
// 2. if both inputs are zero
if (op1->IsFloatPositiveZero() && op2->IsFloatPositiveZero())
// 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()))
{
// IsFloatPositiveZero doesn't gurantee that we don't have op1 = -0.0 op2 = -0.0,
// so we check the sign.
// 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.1 if op1 is +0.0 and op2 is either sign then take op1
if (op1->IsFloatPositiveZero() && !op1->IsFloatNegativeZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType,
insSimdSize);
break;
}
// 2.2 if op2 is +0.0 and op1 is either sign then take op2
else if (op2->IsFloatPositiveZero() && !op2->IsFloatNegativeZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType,
insSimdSize);
break;
}
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType, 16);
SkiFoD marked this conversation as resolved.
Show resolved Hide resolved
break;
}

// 2.3. if op1 is +0.0 then take op2
// 2.4. if op2 is +0.0 then take op2
if ((op1->IsFloatPositiveZero() && !op1->IsFloatNegativeZero()) ||
(op2->IsFloatPositiveZero() && !op2->IsFloatNegativeZero()))
// 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, op2, NI_Vector128_ToScalar, callJitType,
insSimdSize);
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType, 16);
break;
}
}

if (ni == NI_System_Math_Min)
{
// 3. if both inputs are zero
if (op1->IsFloatPositiveZero() && op2->IsFloatPositiveZero())
// 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()))
{
// IsFloatPositiveZero doesn't gurantee that we don't have op1 = -0.0 op2 = -0.0,
// so we check the sign.
// 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.1 if op1 is -0.0 and op2 is either sign then take op1
if (op1->IsFloatNegativeZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType,
insSimdSize);
break;
}
// 3.2 if op2 is -0.0 and op1 is either sign then take op2
else if (op2->IsFloatNegativeZero())
{
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType,
insSimdSize);
break;
}
retNode = gtNewSimdHWIntrinsicNode(callType, op2, NI_Vector128_ToScalar, callJitType, 16);
break;
}

// 3.3. if op1 is -0.0 then take op2
// 3.4. if op2 is -0.0 then take op2
if (op1->IsFloatNegativeZero() || op2->IsFloatNegativeZero())
// 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, op2, NI_Vector128_ToScalar, callJitType,
insSimdSize);
retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType, 16);
break;
}
}

impPushOnStack(op1, callType);
impPushOnStack(op2, callType);
// 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;
//// If it's any other constant then it needs to be op1 for both (Math.Min/Math.Max).
// retNode = gtNewSimdHWIntrinsicNode(callType, op1, NI_Vector128_ToScalar, callJitType,
// insSimdSize);
// break;
}

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

Expand Down