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

Decompose some bitwise operations in HIR to allow more overall optimizations to kick in #104517

Merged
merged 14 commits into from
Jul 13, 2024
Merged

Decompose some bitwise operations in HIR to allow more overall optimizations to kick in #104517

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3a05e3b
Decompose some bitwise operations in HIR to allow more overall optimi…
tannergooding Jul 7, 2024
51ece71
Ensure that we actually remove the underlying op
tannergooding Jul 7, 2024
7574110
Ensure the AND_NOT decomposition is still folded during import for mi…
tannergooding Jul 7, 2024
3ad1ec2
Ensure we propagate AllBitsSet into simd GT_XOR on xarch
tannergooding Jul 7, 2024
d08dcb4
Ensure that we prefer AndNot over TernaryLogic
tannergooding Jul 7, 2024
2c93a18
Cleanup the TernaryLogic lowering code
tannergooding Jul 8, 2024
763997c
Ensure that TernaryLogic picks the best operand for containment
tannergooding Jul 8, 2024
bd9f4f6
Ensure we swap the operands that are being checked for containment
tannergooding Jul 8, 2024
c4b0c3a
Ensure that TernaryLogic is simplified where possible
tannergooding Jul 8, 2024
c5080de
Merge remote-tracking branch 'dotnet/main' into simd-andn
tannergooding Jul 8, 2024
5dfc7ed
Apply formatting patch
tannergooding Jul 9, 2024
083686c
Merge branch 'main' into simd-andn
tannergooding Jul 10, 2024
4e7ca77
Merge branch 'main' into simd-andn
tannergooding Jul 11, 2024
af92525
Merge branch 'main' into simd-andn
tannergooding Jul 13, 2024
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
Next Next commit
Decompose some bitwise operations in HIR to allow more overall optimi… 
…zations to kick in
  • Loading branch information
@tannergooding
tannergooding committed Jul 7, 2024
commit 3a05e3ba72177d1c86dce732cac4d62171e5ecfa
93 changes: 35 additions & 58 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20861,12 +20861,6 @@ GenTree* Compiler::gtNewSimdBinOpNode(
}
}
}

if (op == GT_AND_NOT)
{
// GT_AND_NOT expects `op1 & ~op2`, but xarch does `~op1 & op2`
needsReverseOps = true;
}
break;
}
#endif // TARGET_XARCH
Expand Down Expand Up @@ -20897,11 +20891,34 @@ GenTree* Compiler::gtNewSimdBinOpNode(

if (intrinsic != NI_Illegal)
{
if (op == GT_AND_NOT)
{
assert(fgNodeThreading == NodeThreading::LIR);

#if defined(TARGET_XARCH)
// GT_AND_NOT expects `op1 & ~op2`, but xarch does `~op1 & op2`
// We specially handle this here since we're only producing a
// native intrinsic node in LIR

std::swap(op1, op2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remind me - is it fine to do this swap here in terms of side-effect reordering?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, it's LIR so I guess it is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, in this case its fine specifically because we've validated we're in LIR already.

Otherwise, we should be applying GTF_REVERSE_OPS or have some comment about expecting the user to have already spilled side effects, etc.

#endif // TARGET_XARCH
}
return gtNewSimdHWIntrinsicNode(type, op1, op2, intrinsic, simdBaseJitType, simdSize);
}

switch (op)
{
case GT_AND_NOT:
{
// Prior to LIR, we want to explicitly decompose this operation so that downstream phases can
// appropriately optimize around the individual operations being performed, particularly ~op2,
// and produce overall better codegen.
assert(fgNodeThreading != NodeThreading::LIR);

op2 = gtNewSimdUnOpNode(GT_NOT, type, op2, simdBaseJitType, simdSize);
return gtNewSimdBinOpNode(GT_AND, type, op1, op2, simdBaseJitType, simdSize);
}

#if defined(TARGET_XARCH)
case GT_RSZ:
{
Expand Down Expand Up @@ -21066,9 +21083,6 @@ GenTree* Compiler::gtNewSimdBinOpNode(
vecCon1->gtSimdVal.u64[i] = 0x00FF00FF00FF00FF;
}

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

// Vector256<short> maskedProduct = Avx2.And(widenedProduct, vecCon1).AsInt16()
GenTree* maskedProduct = gtNewSimdBinOpNode(GT_AND, widenedType, widenedProduct, vecCon1,
widenedSimdBaseJitType, widenedSimdSize);
Expand Down Expand Up @@ -22033,9 +22047,6 @@ GenTree* Compiler::gtNewSimdCmpOpNode(
v = gtNewSimdHWIntrinsicNode(type, v, gtNewIconNode(SHUFFLE_ZZXX, TYP_INT), NI_SSE2_Shuffle,
CORINFO_TYPE_INT, simdSize);

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

op2 = gtNewSimdBinOpNode(GT_AND, type, u, v, simdBaseJitType, simdSize);
return gtNewSimdBinOpNode(GT_OR, type, op1, op2, simdBaseJitType, simdSize);
}
Expand Down Expand Up @@ -24146,9 +24157,6 @@ GenTree* Compiler::gtNewSimdNarrowNode(

GenTree* vecCon2 = gtCloneExpr(vecCon1);

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

tmp1 = gtNewSimdBinOpNode(GT_AND, type, op1, vecCon1, simdBaseJitType, simdSize);
tmp2 = gtNewSimdBinOpNode(GT_AND, type, op2, vecCon2, simdBaseJitType, simdSize);
tmp3 = gtNewSimdHWIntrinsicNode(type, tmp1, tmp2, NI_AVX2_PackUnsignedSaturate, CORINFO_TYPE_UBYTE,
Expand Down Expand Up @@ -24187,9 +24195,6 @@ GenTree* Compiler::gtNewSimdNarrowNode(

GenTree* vecCon2 = gtCloneExpr(vecCon1);

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

tmp1 = gtNewSimdBinOpNode(GT_AND, type, op1, vecCon1, simdBaseJitType, simdSize);
tmp2 = gtNewSimdBinOpNode(GT_AND, type, op2, vecCon2, simdBaseJitType, simdSize);
tmp3 = gtNewSimdHWIntrinsicNode(type, tmp1, tmp2, NI_AVX2_PackUnsignedSaturate, CORINFO_TYPE_USHORT,
Expand Down Expand Up @@ -24291,9 +24296,6 @@ GenTree* Compiler::gtNewSimdNarrowNode(

GenTree* vecCon2 = gtCloneExpr(vecCon1);

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

tmp1 = gtNewSimdBinOpNode(GT_AND, type, op1, vecCon1, simdBaseJitType, simdSize);
tmp2 = gtNewSimdBinOpNode(GT_AND, type, op2, vecCon2, simdBaseJitType, simdSize);

Expand Down Expand Up @@ -24330,9 +24332,6 @@ GenTree* Compiler::gtNewSimdNarrowNode(

GenTree* vecCon2 = gtCloneExpr(vecCon1);

// Validate we can't use AVX512F_VL_TernaryLogic here
assert(!canUseEvexEncodingDebugOnly());

tmp1 = gtNewSimdBinOpNode(GT_AND, type, op1, vecCon1, simdBaseJitType, simdSize);
tmp2 = gtNewSimdBinOpNode(GT_AND, type, op2, vecCon2, simdBaseJitType, simdSize);

Expand Down Expand Up @@ -27821,6 +27820,14 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp,
assert(!isScalar);
assert(op2->TypeIs(simdType));

if (comp->fgNodeThreading != NodeThreading::LIR)
{
// We don't want to support creating AND_NOT nodes prior to LIR
// as it can break important optimizations. We'll produces this
// in lowering instead.
break;
}

#if defined(TARGET_XARCH)
if (simdSize == 64)
{
Expand Down Expand Up @@ -30155,13 +30162,8 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
bool isScalar = false;
genTreeOps oper = tree->GetOperForHWIntrinsicId(&isScalar);

#if defined(TARGET_XARCH)
if (oper == GT_AND_NOT)
{
// xarch does: ~op1 & op2, we need op1 & ~op2
std::swap(op1, op2);
}
#endif // TARGET_XARCH
// We shouldn't find AND_NOT nodes since it should only be produced in lowering
assert(oper != GT_AND_NOT);

GenTree* cnsNode = nullptr;
GenTree* otherNode = nullptr;
Expand Down Expand Up @@ -30674,31 +30676,6 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
break;
}

case GT_AND_NOT:
{
// Handle `x & ~0 == x` and `0 & ~x == 0`
if (cnsNode->IsVectorZero())
{
if (cnsNode == op1)
{
resultNode = gtWrapWithSideEffects(cnsNode, otherNode, GTF_ALL_EFFECT);
break;
}
else
{
resultNode = otherNode;
}
break;
}

// Handle `x & ~AllBitsSet == 0`
if (cnsNode->IsVectorAllBitsSet() && (cnsNode == op2))
{
resultNode = gtWrapWithSideEffects(cnsNode, otherNode, GTF_ALL_EFFECT);
}
break;
}

case GT_DIV:
{
if (varTypeIsFloating(simdBaseType))
Expand Down Expand Up @@ -31089,12 +31066,12 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
{
switch (ni)
{
case NI_Vector128_ConditionalSelect:
#if defined(TARGET_XARCH)
case NI_Vector128_ConditionalSelect:
case NI_Vector256_ConditionalSelect:
case NI_Vector512_ConditionalSelect:
#elif defined(TARGET_ARM64)
case NI_Vector64_ConditionalSelect:
case NI_AdvSimd_BitwiseSelect:
case NI_Sve_ConditionalSelect:
#endif
{
Expand Down
34 changes: 34 additions & 0 deletions src/coreclr/jit/hwintrinsicarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,40 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
break;
}

case NI_AdvSimd_BitwiseClear:
{
assert(sig->numArgs == 2);

// We don't want to support creating AND_NOT nodes prior to LIR
// as it can break important optimizations. We'll produces this
// in lowering instead so decompose into the individual operations
// on import

op2 = impSIMDPopStack();
op1 = impSIMDPopStack();

op2 = gtNewSimdUnOpNode(GT_NOT, retType, op2, simdBaseJitType, simdSize);
retNode = gtNewSimdBinOpNode(GT_AND, retType, op1, op2, simdBaseJitType, simdSize);
break;
}

case NI_AdvSimd_OrNot:
{
assert(sig->numArgs == 2);

// We don't want to support creating OR_NOT nodes prior to LIR
// as it can break important optimizations. We'll produces this
// in lowering instead so decompose into the individual operations
// on import

op2 = impSIMDPopStack();
op1 = impSIMDPopStack();

op2 = gtNewSimdUnOpNode(GT_NOT, retType, op2, simdBaseJitType, simdSize);
retNode = gtNewSimdBinOpNode(GT_OR, retType, op1, op2, simdBaseJitType, simdSize);
break;
}

case NI_Vector64_AndNot:
case NI_Vector128_AndNot:
{
Expand Down
40 changes: 40 additions & 0 deletions src/coreclr/jit/hwintrinsiccodegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2856,6 +2856,46 @@ void CodeGen::genAvxFamilyIntrinsic(GenTreeHWIntrinsic* node, insOpts instOption
break;
}

case NI_EVEX_XnorMask:
{
assert(instOptions == INS_OPTS_NONE);

uint32_t simdSize = node->GetSimdSize();
uint32_t count = simdSize / genTypeSize(baseType);

if (count <= 8)
{
assert((count == 2) || (count == 4) || (count == 8));
ins = INS_kxnorb;
}
else if (count == 16)
{
ins = INS_kxnorw;
}
else if (count == 32)
{
ins = INS_kxnord;
}
else
{
assert(count == 64);
ins = INS_kxnorq;
}

op1Reg = op1->GetRegNum();

GenTree* op2 = node->Op(2);
regNumber op2Reg = op2->GetRegNum();

assert(emitter::isMaskReg(targetReg));
assert(emitter::isMaskReg(op1Reg));
assert(emitter::isMaskReg(op2Reg));

// Use EA_32BYTE to ensure the VEX.L bit gets set
emit->emitIns_R_R_R(ins, EA_32BYTE, targetReg, op1Reg, op2Reg);
break;
}

case NI_AVX512F_ConvertToInt32:
case NI_AVX512F_ConvertToUInt32:
case NI_AVX512F_ConvertToUInt32WithTruncation:
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/hwintrinsiclistarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ HARDWARE_INTRINSIC(AdvSimd, AddScalar,
HARDWARE_INTRINSIC(AdvSimd, AddWideningLower, 8, 2, true, {INS_saddl, INS_uaddl, INS_saddl, INS_uaddl, INS_saddl, INS_uaddl, INS_saddw, INS_uaddw, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromSecondArg|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(AdvSimd, AddWideningUpper, 16, 2, true, {INS_saddl2, INS_uaddl2, INS_saddl2, INS_uaddl2, INS_saddl2, INS_uaddl2, INS_saddw2, INS_uaddw2, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromSecondArg|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(AdvSimd, And, -1, 2, true, {INS_and, INS_and, INS_and, INS_and, INS_and, INS_and, INS_and, INS_and, INS_and, INS_and}, HW_Category_SIMD, HW_Flag_Commutative)
HARDWARE_INTRINSIC(AdvSimd, BitwiseClear, -1, 2, true, {INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic}, HW_Category_SIMD, HW_Flag_NoFlag)
HARDWARE_INTRINSIC(AdvSimd, BitwiseClear, -1, 2, true, {INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic, INS_bic}, HW_Category_SIMD, HW_Flag_SpecialImport)
HARDWARE_INTRINSIC(AdvSimd, BitwiseSelect, -1, 3, true, {INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl, INS_bsl}, HW_Category_SIMD, HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(AdvSimd, Ceiling, -1, 1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_frintp, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag)
HARDWARE_INTRINSIC(AdvSimd, CeilingScalar, 8, 1, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_frintp, INS_frintp}, HW_Category_SIMD, HW_Flag_SIMDScalar)
Expand Down Expand Up @@ -383,7 +383,7 @@ HARDWARE_INTRINSIC(AdvSimd, NegateSaturate,
HARDWARE_INTRINSIC(AdvSimd, NegateScalar, 8, 1, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fneg, INS_fneg}, HW_Category_SIMD, HW_Flag_SIMDScalar)
HARDWARE_INTRINSIC(AdvSimd, Not, -1, 1, true, {INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn, INS_mvn}, HW_Category_SIMD, HW_Flag_NoFlag)
HARDWARE_INTRINSIC(AdvSimd, Or, -1, 2, true, {INS_orr, INS_orr, INS_orr, INS_orr, INS_orr, INS_orr, INS_orr, INS_orr, INS_orr, INS_orr}, HW_Category_SIMD, HW_Flag_Commutative)
HARDWARE_INTRINSIC(AdvSimd, OrNot, -1, 2, true, {INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn}, HW_Category_SIMD, HW_Flag_NoFlag)
HARDWARE_INTRINSIC(AdvSimd, OrNot, -1, 2, true, {INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn, INS_orn}, HW_Category_SIMD, HW_Flag_SpecialImport)
HARDWARE_INTRINSIC(AdvSimd, PolynomialMultiply, -1, 2, true, {INS_pmul, INS_pmul, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Commutative)
HARDWARE_INTRINSIC(AdvSimd, PolynomialMultiplyWideningLower, 8, 2, true, {INS_pmull, INS_pmull, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg|HW_Flag_Commutative)
HARDWARE_INTRINSIC(AdvSimd, PolynomialMultiplyWideningUpper, 16, 2, true, {INS_pmull2, INS_pmull2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg|HW_Flag_Commutative)
Expand Down
Loading
Loading