Skip to content

Commit

Permalink
Make intrinsic nodes multi op (aka delete GT_LIST) (#59912)
Browse files Browse the repository at this point in the history
* Introducing GenTreeMultiOp

* Rewrite gtNewSIMDNode

* Rewrite gtNewSIMDVectorZero

* Rewrite gtNewHWIntrinsicNode

* Rewrite GenTreeSIMD::OperIsMemoryLoad

* Rewrite GenTreeHWIntrinsic::OperIsMemoryLoad

* Rewrite GenTreeHWIntrinsic::OperIsMemoryStore

* Rewrite GenTree::IsIntegralConstVector

* Rewrite GenTree::NullOp1Legal

* Rewrite GenTree::IsSIMDZero

* Rewrite GenTree::isCommutativeSIMDIntrinsic

* Rewrite GenTree::isCommutativeHWIntrinsic

* Rewrite GenTree::isContainableHWIntrinsic

* Rewrite GenTree::isRMWHWIntrinsic

* Rewrite GenTreeVisitor

* Rewrite GenTreeUseEdgeIterator

* Rewrite GenTree::VisitOperands

* Rewrite GenTree::TryGetUse

* Rewrite gtGetChildPointer

* Rewrite gtHasRef

* Rewrite fgSetTreeSeqHelper

* Rewrite GenTree::NumChildren

* Rewrite GenTree::GetChild

* Rewrite GenTree::Compare

* Rewrite gtCloneExpr

* Rewrite gtSetEvalOrder

* Rewrite gtHashValue

* Rewrite gtDispTree

* Rewrite fgDebugCheckFlags

* Add genConsumeMultiOpOperands

* Rewrite genConsumeRegs

* Rewrite HWIntrinsic::HWIntrinsic

* Rewrite HWIntrinsic::InitializeOperands

* Delete HWIntrinsicInfo::lookupNumArgs

* Delete HWIntrinsicInfo::lookupLastOp

* Rewrite HWIntrinsicImmOpHelper ARM64

* Rewrite inst_RV_TT_IV

* Rewrite inst_RV_RV_TT

* Rewrite genSIMDIntrinsic XARCH

* Rewrite genSIMDIntrinsicInit XARCH

* Rewrite genSIMDIntrinsicInitN XARCH

* Rewrite genSIMDIntrinsicUnOp XARCH

* Rewrite genSIMDIntrinsic32BitConvert XARCH

* Rewrite genSIMDIntrinsic64BitConvert XARCH

* Rewrite genSIMDIntrinsicWiden XARCH

* Rewrite genSIMDIntrinsicNarrow XARCH

* Rewrite genSIMDIntrinsicBinOp XARCH

* Rewrite genSIMDIntrinsicRelOp XARCH

* Rewrite genSIMDIntrinsicShuffleSSE2 XARCH

* Rewrite genSIMDIntrinsicUpperSave XARCH

* Rewrite genSIMDIntrinsicUpperRestore XARCH

* Rewrite genSIMDIntrinsic ARM64

* Rewrite genSIMDIntrinsicInit ARM64

* Rewrite genSIMDIntrinsicInitN ARM64

* Rewrite genSIMDIntrinsicUnOp ARM64

* Rewrite genSIMDIntrinsicWiden ARM64

* Rewrite genSIMDIntrinsicNarrow ARM64

* Rewrite genSIMDIntrinsicBinOp ARM64

* Rewrite genSIMDIntrinsicUpperSave ARM64

* Rewrite genSIMDIntrinsicUpperRestore ARM64

* Rewrite genHWIntrinsic_R_RM XARCH

* Rewrite genHWIntrinsic_R_RM_I XARCH

* Rewrite genHWIntrinsic_R_R_RM XARCH

* Rewrite genHWIntrinsic_R_R_RM_I XARCH

* Rewrite genHWIntrinsic_R_R_RM_R XARCH

* Rewrite genHWIntrinsic_R_R_R_RM XARCH

* Rewrite genHWIntrinsic XARCH

* Rewrite genBaseIntrinsic XARCH

* Rewrite genX86BaseIntrinsic XARCH

* Rewrite genSSEIntrinsic XARCH

* Rewrite genSSE2Intrinsic XARCH

* Rewrite genSSE41Intrinsic XARCH

* Rewrite genSSE42Intrinsic XARCH

* Rewrite genAvxOrAvx2Intrinsic XARCH

* Rewrite genBMI1OrBMI2Intrinsic XARCH

* Rewrite genFMAIntrinsic XARCH

* Rewrite genLZCNTIntrinsic XARCH

* Rewrite genPOPCNTIntrinsic XARCH

* Rewrite genXCNTIntrinsic XARCH

* Rewrite genHWIntrinsic ARM64

* Rewrite insertUpperVectorSave

* Rewrite insertUpperVectorRestore

* Rewrite getKillSetForHWIntrinsic

* Rewrite BuildSIMD XARCH

* Rewrite BuildOperandUses/BuildDelayFreeUses

* Rewrite BuildSIMD ARM64

* Rewrite BuildHWIntrinsic XARCH

* Rewrite LowerSIMD XARCH

* Rewrite ContainCheckSIMD XARCH

* Rewrite LowerHWIntrinsicCC XARCH

* Rewrite LowerFusedMultiplyAdd XARCH

* Rewrite LowerHWIntrinsic XARCH

* Rewrite LowerHWIntrinsicCmpOp XARCH

* Rewrite LowerHWIntrinsicGetElement XARCH

* Rewrite LowerHWIntrinsicWithElement XARCH

* Rewrite LowerHWIntrinsicCreate XARCH

* Rewrite LowerHWIntrinsicDot XARCH

* Rewrite LowerHWIntrinsicToScalar XARCH

* Rewrite IsContainableHWIntrinsicOp XARCH

* Rewrite ContainCheckHWIntrinsic XARCH

* Rewrite IsValidConstForMovImm ARM64

* Rewrite LowerHWIntrinsic ARM64

* Rewrite LowerHWIntrinsicFusedMultiplyAddScalar ARM64

* Rewrite LowerHWIntrinsicCmpOp ARM64

* Rewrite LowerHWIntrinsicCreate ARM64

* Rewrite LowerHWIntrinsicDot ARM64

* Rewrite ContainCheckStoreLoc ARM64

* Rewrite ContainCheckSIMD ARM64

* Rewrite ContainCheckHWIntrinsic ARM64

* Rewrite DecomposeHWIntrinsicGetElement X86

* Rewrite DecomposeHWIntrinsic X86

* Rewrite Rationalizer::RewriteNode

* Rewrite optIsCSEcandidate

* Rewrite fgValueNumberTree

* Rewrite fgValueNumberSimd

* Rewrite fgValueNumberHWIntrinsic

* Rewrite GetVNFuncForNode

* Rewrite fgMorphTree & fgMorphSmpOpOptional

* Rewrite fgMorphFieldToSimdGetElement/fgMorphField

* Rewrite fgMorphOneAsgBlockOp

* Rewrite impInlineFetchArg

* Rewrite impSIMDRelOp

* Rewrite impSIMDIntrinsic

* Rewrite impBaseIntrinsic XARCH

* Rewrite impAvxOrAvx2Intrinsic XARCH

* Rewrite impSpecialIntrinsic ARM64

* Fix SSA Builder comments

* Delete GT_LIST

* Support GTF_REVERSE_OPS for GenTreeMultiOp

It turns out that in the time this change has been
sitting there, 3 new methods in the SPMI benchmarks
collection appeared, and it turns out they regress
because of the lack of GTF_REVERSE_OPS.

So, implement support for it....

This makes me quite sad, but it does make this change
a pure zero-diff one, which is good.

* Fix Linux x86 build break

* Fix formatting

* Improve readability through the use of a local

* Support external operand arrays in GenTreeMultiOp

* Fix formatting

* Tweak a constructor call
  • Loading branch information
SingleAccretion authored Nov 20, 2021
1 parent ce93c29 commit 87b92fd
Show file tree
Hide file tree
Showing 42 changed files with 2,068 additions and 2,483 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1258,8 +1258,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
optAssertionKind assertionKind,
bool helperCallArgs)
{
assert((op1 != nullptr) && !op1->OperIs(GT_LIST));
assert((op2 == nullptr) || !op2->OperIs(GT_LIST));
assert(op1 != nullptr);
assert(!helperCallArgs || (op2 != nullptr));

AssertionDsc assertion = {OAK_INVALID};
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1130,9 +1130,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void genConsumeRegs(GenTree* tree);
void genConsumeOperands(GenTreeOp* tree);
#ifdef FEATURE_HW_INTRINSICS
void genConsumeHWIntrinsicOperands(GenTreeHWIntrinsic* tree);
#endif // FEATURE_HW_INTRINSICS
#if defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS)
void genConsumeMultiOpOperands(GenTreeMultiOp* tree);
#endif
void genEmitGSCookieCheck(bool pushReg);
void genCodeForShift(GenTree* tree);

Expand Down
77 changes: 37 additions & 40 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3879,7 +3879,7 @@ void CodeGen::genSIMDIntrinsic(GenTreeSIMD* simdNode)
noway_assert(!"SIMD intrinsic with unsupported base type.");
}

switch (simdNode->gtSIMDIntrinsicID)
switch (simdNode->GetSIMDIntrinsicId())
{
case SIMDIntrinsicInit:
genSIMDIntrinsicInit(simdNode);
Expand Down Expand Up @@ -4039,15 +4039,15 @@ instruction CodeGen::getOpForSIMDIntrinsic(SIMDIntrinsicID intrinsicId, var_type
//
void CodeGen::genSIMDIntrinsicInit(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicInit);
assert(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicInit);

GenTree* op1 = simdNode->gtGetOp1();
GenTree* op1 = simdNode->Op(1);
var_types baseType = simdNode->GetSimdBaseType();
regNumber targetReg = simdNode->GetRegNum();
assert(targetReg != REG_NA);
var_types targetType = simdNode->TypeGet();

genConsumeOperands(simdNode);
genConsumeMultiOpOperands(simdNode);
regNumber op1Reg = op1->IsIntegralConst(0) ? REG_ZR : op1->GetRegNum();

// TODO-ARM64-CQ Add LD1R to allow SIMDIntrinsicInit from contained memory
Expand Down Expand Up @@ -4090,16 +4090,18 @@ void CodeGen::genSIMDIntrinsicInit(GenTreeSIMD* simdNode)
//
void CodeGen::genSIMDIntrinsicInitN(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicInitN);
assert(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicInitN);

regNumber targetReg = simdNode->GetRegNum();
assert(targetReg != REG_NA);

var_types targetType = simdNode->TypeGet();

var_types baseType = simdNode->GetSimdBaseType();
var_types targetType = simdNode->TypeGet();
var_types baseType = simdNode->GetSimdBaseType();
emitAttr baseTypeSize = emitTypeSize(baseType);
regNumber vectorReg = targetReg;
size_t initCount = simdNode->GetOperandCount();

regNumber vectorReg = targetReg;
assert((initCount * baseTypeSize) <= simdNode->GetSimdSize());

if (varTypeIsFloating(baseType))
{
Expand All @@ -4108,24 +4110,17 @@ void CodeGen::genSIMDIntrinsicInitN(GenTreeSIMD* simdNode)
vectorReg = simdNode->GetSingleTempReg(RBM_ALLFLOAT);
}

emitAttr baseTypeSize = emitTypeSize(baseType);

// We will first consume the list items in execution (left to right) order,
// and record the registers.
regNumber operandRegs[FP_REGSIZE_BYTES];
unsigned initCount = 0;
for (GenTree* list = simdNode->gtGetOp1(); list != nullptr; list = list->gtGetOp2())
for (size_t i = 1; i <= initCount; i++)
{
assert(list->OperGet() == GT_LIST);
GenTree* listItem = list->gtGetOp1();
assert(listItem->TypeGet() == baseType);
assert(!listItem->isContained());
regNumber operandReg = genConsumeReg(listItem);
operandRegs[initCount] = operandReg;
initCount++;
}
GenTree* operand = simdNode->Op(i);
assert(operand->TypeIs(baseType));
assert(!operand->isContained());

assert((initCount * baseTypeSize) <= simdNode->GetSimdSize());
operandRegs[i - 1] = genConsumeReg(operand);
}

if (initCount * baseTypeSize < EA_16BYTE)
{
Expand Down Expand Up @@ -4164,25 +4159,25 @@ void CodeGen::genSIMDIntrinsicInitN(GenTreeSIMD* simdNode)
//
void CodeGen::genSIMDIntrinsicUnOp(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicCast ||
simdNode->gtSIMDIntrinsicID == SIMDIntrinsicConvertToSingle ||
simdNode->gtSIMDIntrinsicID == SIMDIntrinsicConvertToInt32 ||
simdNode->gtSIMDIntrinsicID == SIMDIntrinsicConvertToDouble ||
simdNode->gtSIMDIntrinsicID == SIMDIntrinsicConvertToInt64);
assert((simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicCast) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicConvertToSingle) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicConvertToInt32) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicConvertToDouble) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicConvertToInt64));

GenTree* op1 = simdNode->gtGetOp1();
GenTree* op1 = simdNode->Op(1);
var_types baseType = simdNode->GetSimdBaseType();
regNumber targetReg = simdNode->GetRegNum();
assert(targetReg != REG_NA);
var_types targetType = simdNode->TypeGet();

genConsumeOperands(simdNode);
genConsumeMultiOpOperands(simdNode);
regNumber op1Reg = op1->GetRegNum();

assert(genIsValidFloatReg(op1Reg));
assert(genIsValidFloatReg(targetReg));

instruction ins = getOpForSIMDIntrinsic(simdNode->gtSIMDIntrinsicID, baseType);
instruction ins = getOpForSIMDIntrinsic(simdNode->GetSIMDIntrinsicId(), baseType);
emitAttr attr = (simdNode->GetSimdSize() > 8) ? EA_16BYTE : EA_8BYTE;

if (GetEmitter()->IsMovInstruction(ins))
Expand All @@ -4208,17 +4203,19 @@ void CodeGen::genSIMDIntrinsicUnOp(GenTreeSIMD* simdNode)
//
void CodeGen::genSIMDIntrinsicBinOp(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicSub || simdNode->gtSIMDIntrinsicID == SIMDIntrinsicBitwiseAnd ||
simdNode->gtSIMDIntrinsicID == SIMDIntrinsicBitwiseOr || simdNode->gtSIMDIntrinsicID == SIMDIntrinsicEqual);
assert((simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicSub) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicBitwiseAnd) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicBitwiseOr) ||
(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicEqual));

GenTree* op1 = simdNode->gtGetOp1();
GenTree* op2 = simdNode->gtGetOp2();
GenTree* op1 = simdNode->Op(1);
GenTree* op2 = simdNode->Op(2);
var_types baseType = simdNode->GetSimdBaseType();
regNumber targetReg = simdNode->GetRegNum();
assert(targetReg != REG_NA);
var_types targetType = simdNode->TypeGet();

genConsumeOperands(simdNode);
genConsumeMultiOpOperands(simdNode);
regNumber op1Reg = op1->GetRegNum();
regNumber op2Reg = op2->GetRegNum();

Expand All @@ -4228,7 +4225,7 @@ void CodeGen::genSIMDIntrinsicBinOp(GenTreeSIMD* simdNode)

// TODO-ARM64-CQ Contain integer constants where posible

instruction ins = getOpForSIMDIntrinsic(simdNode->gtSIMDIntrinsicID, baseType);
instruction ins = getOpForSIMDIntrinsic(simdNode->GetSIMDIntrinsicId(), baseType);
emitAttr attr = (simdNode->GetSimdSize() > 8) ? EA_16BYTE : EA_8BYTE;
insOpts opt = genGetSimdInsOpt(attr, baseType);

Expand Down Expand Up @@ -4257,9 +4254,9 @@ void CodeGen::genSIMDIntrinsicBinOp(GenTreeSIMD* simdNode)
//
void CodeGen::genSIMDIntrinsicUpperSave(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicUpperSave);
assert(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicUpperSave);

GenTree* op1 = simdNode->gtGetOp1();
GenTree* op1 = simdNode->Op(1);
GenTreeLclVar* lclNode = op1->AsLclVar();
LclVarDsc* varDsc = compiler->lvaGetDesc(lclNode);
assert(emitTypeSize(varDsc->GetRegisterType(lclNode)) == 16);
Expand Down Expand Up @@ -4307,9 +4304,9 @@ void CodeGen::genSIMDIntrinsicUpperSave(GenTreeSIMD* simdNode)
//
void CodeGen::genSIMDIntrinsicUpperRestore(GenTreeSIMD* simdNode)
{
assert(simdNode->gtSIMDIntrinsicID == SIMDIntrinsicUpperRestore);
assert(simdNode->GetSIMDIntrinsicId() == SIMDIntrinsicUpperRestore);

GenTree* op1 = simdNode->gtGetOp1();
GenTree* op1 = simdNode->Op(1);
assert(op1->IsLocal());
GenTreeLclVar* lclNode = op1->AsLclVar();
LclVarDsc* varDsc = compiler->lvaGetDesc(lclNode);
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
// This is handled at the time we call genConsumeReg() on the GT_COPY
break;

case GT_LIST:
case GT_FIELD_LIST:
// Should always be marked contained.
assert(!"LIST, FIELD_LIST nodes should always be marked contained.");
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,8 @@ unsigned CodeGenInterface::InferStructOpSizeAlign(GenTree* op, unsigned* alignme
{
opSize = (unsigned)op2->AsIntCon()->gtIconVal;
GenTree* op1 = op->AsOp()->gtOp1;
assert(op1->OperGet() == GT_LIST);
// TODO-List-Cleanup: this looks like some really old dead code.
// assert(op1->OperGet() == GT_LIST);
GenTree* dstAddr = op1->AsOp()->gtOp1;
if (dstAddr->OperGet() == GT_ADDR)
{
Expand Down
60 changes: 17 additions & 43 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1623,14 +1623,18 @@ void CodeGen::genConsumeRegs(GenTree* tree)
else if (tree->OperIs(GT_HWINTRINSIC))
{
// Only load/store HW intrinsics can be contained (and the address may also be contained).
HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(tree->AsHWIntrinsic()->gtHWIntrinsicId);
HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(tree->AsHWIntrinsic()->GetHWIntrinsicId());
assert((category == HW_Category_MemoryLoad) || (category == HW_Category_MemoryStore));
int numArgs = HWIntrinsicInfo::lookupNumArgs(tree->AsHWIntrinsic());
genConsumeAddress(tree->gtGetOp1());
size_t numArgs = tree->AsHWIntrinsic()->GetOperandCount();
genConsumeAddress(tree->AsHWIntrinsic()->Op(1));
if (category == HW_Category_MemoryStore)
{
assert((numArgs == 2) && !tree->gtGetOp2()->isContained());
genConsumeReg(tree->gtGetOp2());
assert(numArgs == 2);

GenTree* op2 = tree->AsHWIntrinsic()->Op(2);
assert(op2->isContained());

genConsumeReg(op2);
}
else
{
Expand Down Expand Up @@ -1674,7 +1678,6 @@ void CodeGen::genConsumeRegs(GenTree* tree)
// Return Value:
// None.
//

void CodeGen::genConsumeOperands(GenTreeOp* tree)
{
GenTree* firstOp = tree->gtOp1;
Expand All @@ -1690,54 +1693,25 @@ void CodeGen::genConsumeOperands(GenTreeOp* tree)
}
}

#ifdef FEATURE_HW_INTRINSICS
#if defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS)
//------------------------------------------------------------------------
// genConsumeHWIntrinsicOperands: Do liveness update for the operands of a GT_HWINTRINSIC node
// genConsumeOperands: Do liveness update for the operands of a multi-operand node,
// currently GT_SIMD or GT_HWINTRINSIC
//
// Arguments:
// node - the GenTreeHWIntrinsic node whose operands will have their liveness updated.
// tree - the GenTreeMultiOp whose operands will have their liveness updated.
//
// Return Value:
// None.
//

void CodeGen::genConsumeHWIntrinsicOperands(GenTreeHWIntrinsic* node)
void CodeGen::genConsumeMultiOpOperands(GenTreeMultiOp* tree)
{
int numArgs = HWIntrinsicInfo::lookupNumArgs(node);
GenTree* op1 = node->gtGetOp1();
if (op1 == nullptr)
for (GenTree* operand : tree->Operands())
{
assert((numArgs == 0) && (node->gtGetOp2() == nullptr));
return;
}
if (op1->OperIs(GT_LIST))
{
int foundArgs = 0;
assert(node->gtGetOp2() == nullptr);
for (GenTreeArgList* list = op1->AsArgList(); list != nullptr; list = list->Rest())
{
GenTree* operand = list->Current();
genConsumeRegs(operand);
foundArgs++;
}
assert(foundArgs == numArgs);
}
else
{
genConsumeRegs(op1);
GenTree* op2 = node->gtGetOp2();
if (op2 != nullptr)
{
genConsumeRegs(op2);
assert(numArgs == 2);
}
else
{
assert(numArgs == 1);
}
genConsumeRegs(operand);
}
}
#endif // FEATURE_HW_INTRINSICS
#endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS)

#if FEATURE_PUT_STRUCT_ARG_STK
//------------------------------------------------------------------------
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
// This is handled at the time we call genConsumeReg() on the GT_COPY
break;

case GT_LIST:
case GT_FIELD_LIST:
// Should always be marked contained.
assert(!"LIST, FIELD_LIST nodes should always be marked contained.");
Expand Down
Loading

0 comments on commit 87b92fd

Please sign in to comment.