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.

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

Morph ASG(ONE_FLD_STRUCT, CALL) using BITCAST #61049

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 0 additions & 6 deletions src/coreclr/jit/copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,19 +257,13 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
//
// Returns:
// - lclNum if the local is participating in SSA;
// - fieldLclNum if the parent local can be replaced by its only field;
// - BAD_VAR_NUM otherwise.
//
unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
{
unsigned lclNum = lclNode->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);

if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this))
{
lclNum = varDsc->lvFieldLclStart;
}

if (!lvaInSsa(lclNum))
{
return BAD_VAR_NUM;
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/earlyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,7 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK

GenTree* treeRhs = ssaDefAsg->gtGetOp2();

if (treeRhs->OperIsScalarLocal() && lvaInSsa(treeRhs->AsLclVarCommon()->GetLclNum()) &&
treeRhs->AsLclVarCommon()->HasSsaName())
if (treeRhs->OperIsScalarLocal() && treeRhs->AsLclVarCommon()->HasSsaName())
{
// Recursively track the Rhs
unsigned rhsLclNum = treeRhs->AsLclVarCommon()->GetLclNum();
Expand Down
24 changes: 24 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23307,6 +23307,30 @@ unsigned GenTreeHWIntrinsic::GetResultOpNumForFMA(GenTree* use, GenTree* op1, Ge
}
#endif // TARGET_XARCH && FEATURE_HW_INTRINSICS

//------------------------------------------------------------------------
// HasSsaName: Does this local node have an SSA name?
//
// Return Value:
// Whether this node's SSA name is not RESERVED_SSA_NUM.
//
// Notes:
// If the node has an SSA name, it will always represent a local that
// participates in SSA. Thus, "HasSsaName" implies "lvaIsInSsa". The
// opposite is not true - a node may be in an unreachable block not
// visited by the renamer.
//
bool GenTreeLclVarCommon::HasSsaName() const
{
bool hasSsaName = GetSsaNum() != SsaConfig::RESERVED_SSA_NUM;

if (hasSsaName)
{
assert(JitTls::GetCompiler()->lvaInSsa(GetLclNum()));
}

return hasSsaName;
}

unsigned GenTreeLclFld::GetSize() const
{
return TypeIs(TYP_STRUCT) ? GetLayout()->GetSize() : genTypeSize(TypeGet());
Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3550,10 +3550,7 @@ struct GenTreeLclVarCommon : public GenTreeUnOp
_gtSsaNum = ssaNum;
}

bool HasSsaName()
{
return (GetSsaNum() != SsaConfig::RESERVED_SSA_NUM);
}
bool HasSsaName() const;

#if DEBUGGABLE_GENTREE
GenTreeLclVarCommon() : GenTreeUnOp()
Expand Down
45 changes: 45 additions & 0 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,9 @@ GenTree* Lowering::LowerNode(GenTree* node)
LowerRet(node->AsUnOp());
break;

case GT_BITCAST:
return LowerBitcast(node->AsUnOp());

case GT_RETURNTRAP:
ContainCheckReturnTrap(node->AsOp());
break;
Expand Down Expand Up @@ -3360,6 +3363,44 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
ContainCheckRet(ret);
}

//------------------------------------------------------------------------
// LowerBitcast: Lower a GT_BITCAST node.
//
// Will remove bitcasts that do not move between different register files.
// Such bitcasts are generated in block morphing for assignments of struct
// locals that "can be replaced with their one field" to calls. This makes
// the frontend (SSA) simpler, but here, in the backend, they end up as
// unnecessary no-op BITCAST(type -> type) nodes, so we remove them.
//
// Arguments:
// bitcast - the bitcast node to lower
//
// Return Value:
// The next node to lower.
//
GenTree* Lowering::LowerBitcast(GenTreeUnOp* bitcast)
{
GenTree* nextNode = bitcast->gtNext;
GenTree* src = bitcast->gtGetOp1();

if (bitcast->TypeGet() == src->TypeGet())
{
LIR::Use bitcastUse;
if (BlockRange().TryGetUse(bitcast, &bitcastUse))
{
bitcastUse.ReplaceWith(src);
}
else
{
src->SetUnusedValue();
}

BlockRange().Remove(bitcast);
}

return nextNode;
}

//----------------------------------------------------------------------------------------------
// LowerStoreLocCommon: platform idependent part of local var or field store lowering.
//
Expand Down Expand Up @@ -3814,6 +3855,10 @@ void Lowering::LowerCallStruct(GenTreeCall* call)
GenTree* user = callUse.User();
switch (user->OperGet())
{
case GT_BITCAST:
assert(genTypeSize(user) == genTypeSize(call));
break;

case GT_RETURN:
case GT_STORE_LCL_VAR:
case GT_STORE_BLK:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ class Lowering final : public Phase
GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
void LowerJmpMethod(GenTree* jmp);
void LowerRet(GenTreeUnOp* ret);
GenTree* LowerBitcast(GenTreeUnOp* node);
void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar);
void LowerRetStruct(GenTreeUnOp* ret);
void LowerRetSingleRegStructLclVar(GenTreeUnOp* ret);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3523,11 +3523,11 @@ int LinearScan::BuildStoreLoc(GenTreeLclVarCommon* storeLoc)
else if (op1->isContained() && op1->OperIs(GT_BITCAST))
{
GenTree* bitCastSrc = op1->gtGetOp1();
RegisterType registerType = bitCastSrc->TypeGet();
RegisterType registerType = regType(getDefType(bitCastSrc));
singleUseRef = BuildUse(bitCastSrc, allRegs(registerType));

Interval* srcInterval = singleUseRef->getInterval();
assert(srcInterval->registerType == registerType);
assert(srcInterval->registerType == getDefType(bitCastSrc));
srcCount = 1;
}
#ifndef TARGET_64BIT
Expand Down
25 changes: 22 additions & 3 deletions src/coreclr/jit/morphblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class MorphInitBlockHelper
OneAsgBlock,
StructBlock,
SkipCallSrc,
BitcastCallSrc,
SkipMultiRegIntrinsicSrc,
Nop
};
Expand Down Expand Up @@ -806,15 +807,33 @@ void MorphCopyBlockHelper::TrySpecialCases()
m_asg->gtOp1 = lclVar;
}
}

if (m_dst->OperIs(GT_LCL_VAR))
{
LclVarDsc* varDsc = m_comp->lvaGetDesc(m_dst->AsLclVar());
if (varTypeIsStruct(varDsc) && varDsc->CanBeReplacedWithItsField(m_comp))
{
assert(m_comp->fgGlobalMorph);

JITDUMP("Morphing a single reg call return into BITCAST assigned to the promoted field\n");
m_transformationDecision = BlockTransformation::BitcastCallSrc;

unsigned fieldNum = varDsc->lvFieldLclStart;
LclVarDsc* fieldDsc = m_comp->lvaGetDesc(fieldNum);
var_types fieldType = fieldDsc->TypeGet();
assert(!varTypeIsStruct(fieldType));

m_dst->gtFlags |= GTF_DONT_CSE;
JITDUMP("Not morphing a single reg call return\n");
m_transformationDecision = BlockTransformation::SkipCallSrc;
m_result = m_asg;
m_dst->ChangeType(fieldType);
m_dst->AsLclVar()->SetLclNum(fieldNum);

assert(fieldDsc->lvNormalizeOnLoad() || !varTypeIsSmall(fieldDsc));
GenTree* bitcast = m_comp->gtNewOperNode(GT_BITCAST, genActualType(fieldType), m_src);
m_src = bitcast;
m_asg->gtOp2 = bitcast;

m_asg->ChangeType(fieldType);
m_result = m_asg;
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7264,7 +7264,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack<BasicBlock*>* blo
// To be invariant a LclVar node must not be the LHS of an assignment ...
bool isInvariant = !user->OperIs(GT_ASG) || (user->AsOp()->gtGetOp1() != tree);
// and the variable must be in SSA ...
isInvariant = isInvariant && m_compiler->lvaInSsa(lclNum) && lclVar->HasSsaName();
isInvariant = isInvariant && lclVar->HasSsaName();
// and the SSA definition must be outside the loop we're hoisting from ...
isInvariant = isInvariant &&
!m_compiler->optLoopTable[m_loopNum].lpContains(
Expand Down Expand Up @@ -8455,7 +8455,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
{
// If it's a local byref for which we recorded a value number, use that...
GenTreeLclVar* argLcl = arg->AsLclVar();
if (lvaInSsa(argLcl->GetLclNum()) && argLcl->HasSsaName())
if (argLcl->HasSsaName())
{
ValueNum argVN =
lvaTable[argLcl->GetLclNum()].GetPerSsaData(argLcl->GetSsaNum())->m_vnPair.GetLiberal();
Expand Down Expand Up @@ -8533,7 +8533,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
if (rhsVN != ValueNumStore::NoVN)
{
rhsVN = vnStore->VNNormalValue(rhsVN);
if (lvaInSsa(lhsLcl->GetLclNum()) && lhsLcl->HasSsaName())
if (lhsLcl->HasSsaName())
{
lvaTable[lhsLcl->GetLclNum()]
.GetPerSsaData(lhsLcl->GetSsaNum())
Expand Down
16 changes: 2 additions & 14 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,7 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse)
return nullptr;
}

LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclUse);
if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
}
LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclUse);
LclSsaVarDsc* ssaDef = varDsc->GetPerSsaData(ssaNum);

// RangeCheck does not care about uninitialized variables.
Expand Down Expand Up @@ -493,10 +489,6 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse)
UINT64 RangeCheck::HashCode(unsigned lclNum, unsigned ssaNum)
{
LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum);
if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
lclNum = varDsc->lvFieldLclStart;
}
assert(ssaNum != SsaConfig::RESERVED_SSA_NUM);
return UINT64(lclNum) << 32 | ssaNum;
}
Expand Down Expand Up @@ -563,11 +555,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
return;
}

LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
}
LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum());
ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
MergeEdgeAssertions(normalLclVN, assertions, pRange);
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/ssabuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -745,13 +745,6 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block)
unsigned lclNum = lclNode->GetLclNum();
LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum);

if (!m_pCompiler->lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
lclNum = varDsc->lvFieldLclStart;
varDsc = m_pCompiler->lvaGetDesc(lclNum);
assert(isFullDef);
}

if (m_pCompiler->lvaInSsa(lclNum))
{
// Promoted variables are not in SSA, only their fields are.
Expand Down
20 changes: 1 addition & 19 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8188,24 +8188,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)

fgValueNumberLocalStore(tree, lclVarTree, offset, storeSize, rhsVNPair);
}
else if (lclVarTree->HasSsaName())
{
// The local wasn't in SSA, the tree is still an SSA def. There is only one
// case when this can happen - a promoted "CanBeReplacedWithItsField" struct.
assert((lhs == lclVarTree) && rhs->IsCall() && isEntire);
assert(lhsVarDsc->CanBeReplacedWithItsField(this));
// Give a new, unique, VN to the field.
LclVarDsc* fieldVarDsc = lvaGetDesc(lhsVarDsc->lvFieldLclStart);
LclSsaVarDsc* fieldVarSsaDsc = fieldVarDsc->GetPerSsaData(lclVarTree->GetSsaNum());
ValueNum newUniqueVN = vnStore->VNForExpr(compCurBB, fieldVarDsc->TypeGet());

fieldVarSsaDsc->m_vnPair.SetBoth(newUniqueVN);

JITDUMP("Tree [%06u] assigned VN to the only field V%02u/%u of promoted struct V%02u: new uniq ",
dspTreeID(tree), lhsVarDsc->lvFieldLclStart, lclVarTree->GetSsaNum(), lhsLclNum);
JITDUMPEXEC(vnPrint(newUniqueVN, 1));
JITDUMP("\n");
}
else if (lhsVarDsc->IsAddressExposed())
{
fgMutateAddressExposedLocal(tree DEBUGARG("INITBLK/COPYBLK - address-exposed local"));
Expand Down Expand Up @@ -8324,7 +8306,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
{
unsigned lclNum = lclFld->GetLclNum();

if (!lvaInSsa(lclFld->GetLclNum()) || !lclFld->HasSsaName())
if (!lclFld->HasSsaName())
{
lclFld->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclFld->TypeGet()));
}
Expand Down