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

Jump to bottom

Delete fgMorphBlockOperand #77688

Merged
merged 4 commits into from
Nov 12, 2022
Merged

Delete fgMorphBlockOperand #77688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
558d34d
Stop calling fgMorphBlockOperand
SingleAccretion Oct 7, 2022
fe2850f
Fix missing NO_CSEs
SingleAccretion Oct 8, 2022
59a5171
Simplify
SingleAccretion Nov 1, 2022
c0e1eab
Delete fgMorphBlockOperand
SingleAccretion Nov 10, 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
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5774,7 +5774,6 @@ class Compiler
GenTree* fgMorphLeaf(GenTree* tree);
GenTree* fgMorphOneAsgBlockOp(GenTree* tree);
GenTree* fgMorphInitBlock(GenTree* tree);
GenTree* fgMorphBlockOperand(GenTree* tree, var_types asgType, ClassLayout* blockLayout, bool isBlkReqd);
GenTree* fgMorphCopyBlock(GenTree* tree);
GenTree* fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree);
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone = nullptr);
Expand Down
175 changes: 6 additions & 169 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8739,150 +8739,6 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree)
return tree;
}

//------------------------------------------------------------------------
// fgMorphBlockOperand: Canonicalize an operand of a block assignment
//
// Arguments:
// tree - The block operand
// asgType - The type of the assignment
// blockLayout - The struct layout of the block (for STRUCT "asgType"s)
// isBlkReqd - true iff this operand must remain a block node
//
// Return Value:
// Returns the morphed block operand
//
// Notes:
// This does the following:
// - Ensures that a struct operand is a block node or lclVar.
// - Ensures that any COMMAs are above ADDR nodes.
// Although 'tree' WAS an operand of a block assignment, the assignment
// may have been retyped to be a scalar assignment.
//
GenTree* Compiler::fgMorphBlockOperand(GenTree* tree, var_types asgType, ClassLayout* blockLayout, bool isBlkReqd)
{
GenTree* effectiveVal = tree->gtEffectiveVal();

if (asgType != TYP_STRUCT)
{
unsigned blockWidth = genTypeSize(asgType);

if (effectiveVal->OperIsIndir())
{
if (!isBlkReqd)
{
GenTree* addr = effectiveVal->AsIndir()->Addr();
if ((addr->OperGet() == GT_ADDR) && (addr->gtGetOp1()->TypeGet() == asgType))
{
effectiveVal = addr->gtGetOp1();
}
else if (effectiveVal->OperIsBlk())
{
effectiveVal->SetOper(GT_IND);
}
}
effectiveVal->gtType = asgType;
}
else if (effectiveVal->TypeGet() != asgType)
{
if (effectiveVal->IsCall())
{
#ifdef DEBUG
GenTreeCall* call = effectiveVal->AsCall();
assert(call->TypeGet() == TYP_STRUCT);
assert(blockWidth == info.compCompHnd->getClassSize(call->gtRetClsHnd));
#endif
}
else
{
GenTree* addr = gtNewOperNode(GT_ADDR, TYP_BYREF, effectiveVal);
effectiveVal = gtNewIndir(asgType, addr);
}
}
}
else
{
assert(blockLayout != nullptr);

GenTreeIndir* indirTree = nullptr;
GenTreeLclVarCommon* lclNode = nullptr;
bool needsIndirection = true;

if (effectiveVal->OperIsIndir())
{
indirTree = effectiveVal->AsIndir();
GenTree* addr = effectiveVal->AsIndir()->Addr();
if ((addr->OperGet() == GT_ADDR) && (addr->gtGetOp1()->OperGet() == GT_LCL_VAR))
{
lclNode = addr->gtGetOp1()->AsLclVarCommon();
}
}
else if (effectiveVal->OperGet() == GT_LCL_VAR)
{
lclNode = effectiveVal->AsLclVarCommon();
}
else if (effectiveVal->OperIs(GT_LCL_FLD))
{
needsIndirection = false;
assert(ClassLayout::AreCompatible(effectiveVal->AsLclFld()->GetLayout(), blockLayout));
}
else if (effectiveVal->IsCall())
{
needsIndirection = false;
#ifdef DEBUG
GenTreeCall* call = effectiveVal->AsCall();
assert(call->TypeGet() == TYP_STRUCT);
assert(blockLayout->GetSize() == info.compCompHnd->getClassSize(call->gtRetClsHnd));
#endif
}
#ifdef TARGET_ARM64
else if (effectiveVal->OperIsHWIntrinsic())
{
needsIndirection = false;
#ifdef DEBUG
GenTreeHWIntrinsic* intrinsic = effectiveVal->AsHWIntrinsic();
assert(intrinsic->TypeGet() == TYP_STRUCT);
assert(HWIntrinsicInfo::IsMultiReg(intrinsic->GetHWIntrinsicId()));
#endif
}
#endif // TARGET_ARM64

if (lclNode != nullptr)
{
const LclVarDsc* varDsc = lvaGetDesc(lclNode);
if (varTypeIsStruct(varDsc) && ClassLayout::AreCompatible(varDsc->GetLayout(), blockLayout))
{
if (effectiveVal != lclNode)
{
JITDUMP("Replacing block node [%06d] with lclVar V%02u\n", dspTreeID(tree), lclNode->GetLclNum());
effectiveVal = lclNode;
}
needsIndirection = false;
}
else
{
// This may be a lclVar that was determined to be address-exposed.
effectiveVal->gtFlags |= (lclNode->gtFlags & GTF_ALL_EFFECT);
}
}

if (needsIndirection)
{
if ((indirTree != nullptr) && (indirTree->OperIsBlk() || !isBlkReqd))
{
effectiveVal->gtType = asgType;
}
else
{
effectiveVal = gtNewStructVal(blockLayout, gtNewOperNode(GT_ADDR, TYP_BYREF, effectiveVal));
gtUpdateNodeSideEffects(effectiveVal);
}
}
}

assert(effectiveVal->TypeIs(asgType) || (varTypeIsSIMD(asgType) && varTypeIsStruct(effectiveVal)));
return effectiveVal;
}

#ifdef FEATURE_SIMD

//--------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -9308,19 +9164,12 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
}
#endif

// We can't CSE the LHS of an assignment. Only r-values can be CSEed.
// Previously, the "lhs" (addr) of a block op was CSE'd. So, to duplicate the former
// behavior, allow CSE'ing if is a struct type (or a TYP_REF transformed from a struct type)
// TODO-1stClassStructs: improve this.
if (op1->IsLocal() || (op1->TypeGet() != TYP_STRUCT))
{
op1->gtFlags |= GTF_DONT_CSE;
}
// Location nodes cannot be CSEd.
op1->gtFlags |= GTF_DONT_CSE;
break;

case GT_ADDR:

/* op1 of a GT_ADDR is an l-value. Only r-values can be CSEed */
// Location nodes cannot be CSEd.
op1->gtFlags |= GTF_DONT_CSE;
break;

Expand Down Expand Up @@ -10267,17 +10116,11 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
*/

GenTree* temp;
GenTree* lclVarTree;

switch (oper)
{
case GT_ASG:

lclVarTree = fgIsIndirOfAddrOfLocal(op1);
if (lclVarTree != nullptr)
{
lclVarTree->gtFlags |= GTF_VAR_DEF;
}
fgAssignSetVarDef(tree);

if (op2->OperIs(GT_CAST))
{
Expand All @@ -10289,14 +10132,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
op2 = tree->gtGetOp2();
}

fgAssignSetVarDef(tree);

/* We can't CSE the LHS of an assignment */
/* We also must set in the pre-morphing phase, otherwise assertionProp doesn't see it */
if (op1->IsLocal() || (op1->TypeGet() != TYP_STRUCT))
{
op1->gtFlags |= GTF_DONT_CSE;
}
// Location nodes cannot be CSEd.
op1->gtFlags |= GTF_DONT_CSE;
break;

case GT_CAST:
Expand Down
63 changes: 26 additions & 37 deletions src/coreclr/jit/morphblock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,8 @@ void MorphInitBlockHelper::MorphStructCases()

if (m_transformationDecision == BlockTransformation::Undefined)
{
// For an InitBlock we always require a block operand.
m_dst = m_comp->fgMorphBlockOperand(m_dst, m_dst->TypeGet(), m_blockLayout, true /*isBlkReqd*/);
m_result = m_asg;
m_transformationDecision = BlockTransformation::StructBlock;
m_dst->gtFlags |= GTF_DONT_CSE;
m_result = m_asg;
m_result->AsOp()->gtOp1 = m_dst;
m_result->gtFlags |= (m_dst->gtFlags & GTF_ALL_EFFECT);

if (m_dstVarDsc != nullptr)
{
Expand Down Expand Up @@ -401,29 +396,20 @@ GenTree* MorphInitBlockHelper::MorphBlock(Compiler* comp, GenTree* tree, bool is
JITDUMP("MorphBlock for %s tree, before:\n", (isDest ? "dst" : "src"));
DISPTREE(tree);

// Src can be a primitive type.
assert(!isDest || varTypeIsStruct(tree));

GenTree* handleTree = nullptr;
GenTree* addr = nullptr;
assert(varTypeIsStruct(tree));

if (tree->OperIs(GT_COMMA))
{
// TODO-Cleanup: this block is not needed for not struct nodes, but
// fgMorphOneAsgBlockOp works wrong without this transformation.
tree = MorphCommaBlock(comp, tree->AsOp());
if (isDest)
{
tree->SetDoNotCSE();
}
}

if (!tree->OperIsBlk())
{
JITDUMP("MorphBlock after:\n");
DISPTREE(tree);
return tree;
}

GenTree* blkAddr = tree->AsBlk()->Addr();
assert(blkAddr != nullptr);
assert(blkAddr->TypeIs(TYP_I_IMPL, TYP_BYREF, TYP_REF));
assert(!tree->OperIsIndir() || varTypeIsI(genActualType(tree->AsIndir()->Addr())));

JITDUMP("MorphBlock after:\n");
DISPTREE(tree);
Expand Down Expand Up @@ -833,13 +819,28 @@ void MorphCopyBlockHelper::PrepareSrc()
m_srcVarDsc = m_comp->lvaGetDesc(m_srcLclNum);
}

// Verify that the types on the LHS and RHS match.
// Verify that the types on the LHS and RHS match and morph away "IND<struct>" nodes.
assert(m_dst->TypeGet() == m_src->TypeGet());
// TODO-1stClassStructs: delete the "!IND" condition once "IND<struct>" nodes are no more.
if (m_dst->TypeIs(TYP_STRUCT) && !m_src->OperIs(GT_IND))
if (m_dst->TypeIs(TYP_STRUCT))
{
// TODO-1stClassStructs: delete this once "IND<struct>" nodes are no more.
if (m_src->OperIs(GT_IND))
{
m_src->SetOper(m_blockLayout->IsBlockLayout() ? GT_BLK : GT_OBJ);
m_src->AsBlk()->SetLayout(m_blockLayout);
m_src->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid;
#ifndef JIT32_GCENCODER
m_src->AsBlk()->gtBlkOpGcUnsafe = false;
#endif // !JIT32_GCENCODER
}

assert(ClassLayout::AreCompatible(m_blockLayout, m_src->GetLayout(m_comp)));
}
// TODO-1stClassStructs: produce simple "IND<simd>" nodes in importer.
else if (m_src->OperIsBlk())
{
m_src->SetOper(GT_IND);
}
}

// TrySpecialCases: check special cases that require special transformations.
Expand Down Expand Up @@ -976,7 +977,7 @@ void MorphCopyBlockHelper::MorphStructCases()
// promotion.
if ((m_srcVarDsc == nullptr) && !m_src->OperIsIndir())
{
JITDUMP(" src is a not an L-value");
JITDUMP(" src is not an L-value");
requiresCopyBlock = true;
}

Expand Down Expand Up @@ -1117,18 +1118,6 @@ void MorphCopyBlockHelper::MorphStructCases()

if (requiresCopyBlock)
{
const var_types asgType = m_dst->TypeGet();
bool isBlkReqd = (asgType == TYP_STRUCT);
m_dst = m_comp->fgMorphBlockOperand(m_dst, asgType, m_blockLayout, isBlkReqd);
m_dst->gtFlags |= GTF_DONT_CSE;
m_asg->gtOp1 = m_dst;

m_src = m_comp->fgMorphBlockOperand(m_src, asgType, m_blockLayout, isBlkReqd);
m_asg->gtOp2 = m_src;

m_asg->SetAllEffectsFlags(m_dst, m_src);
m_asg->gtFlags |= GTF_ASG;

m_result = m_asg;
m_transformationDecision = BlockTransformation::StructBlock;
}
Expand Down