Skip to content

Commit

Permalink
Delete fgMorphBlockOperand (#77688)
Browse files Browse the repository at this point in the history 
* Stop calling fgMorphBlockOperand

* Fix missing NO_CSEs

* Simplify

* Delete fgMorphBlockOperand
  • Loading branch information
@SingleAccretion
SingleAccretion authored Nov 12, 2022
1 parent 3e94fdc commit 43272d6
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 207 deletions.
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5784,7 +5784,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
Original file line number Diff line number Diff line change
Expand Up @@ -8796,150 +8796,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 @@ -9365,19 +9221,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 @@ -10325,17 +10174,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 @@ -10347,14 +10190,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
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

0 comments on commit 43272d6

Please sign in to comment.