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

Use SSA def descriptors in copy propagation #65250

Merged
merged 2 commits into from
Feb 25, 2022
Merged
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
65 changes: 58 additions & 7 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,13 @@ class SsaDefArray
assert(ssaNum != SsaConfig::RESERVED_SSA_NUM);
return GetSsaDefByIndex(ssaNum - GetMinSsaNum());
}

// Get an SSA number associated with the specified SSA def (that must be in this array).
unsigned GetSsaNum(T* ssaDef)
{
assert((m_array <= ssaDef) && (ssaDef < &m_array[m_count]));
return GetMinSsaNum() + static_cast<unsigned>(ssaDef - &m_array[0]);
}
};

enum RefCountState
Expand Down Expand Up @@ -1082,6 +1089,13 @@ class LclVarDsc
return lvPerSsaData.GetSsaDef(ssaNum);
}

// Returns the SSA number for "ssaDef". Requires "ssaDef" to be a valid definition
// of this variable.
unsigned GetSsaNumForSsaDef(LclSsaVarDsc* ssaDef)
{
return lvPerSsaData.GetSsaNum(ssaDef);
}

var_types GetRegisterType(const GenTreeLclVarCommon* tree) const;

var_types GetRegisterType() const;
Expand Down Expand Up @@ -7399,17 +7413,54 @@ class Compiler

public:
// VN based copy propagation.
typedef ArrayStack<GenTree*> GenTreePtrStack;
typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*> LclNumToGenTreePtrStack;

// In DEBUG builds, we'd like to know the tree that the SSA definition was pushed for.
// While for ordinary SSA defs it will be available (as an ASG) in the SSA descriptor,
// for locals which will use "definitions from uses", it will not be, so we store it
// in this class instead.
class CopyPropSsaDef
{
LclSsaVarDsc* m_ssaDef;
#ifdef DEBUG
GenTree* m_defNode;
#endif
public:
CopyPropSsaDef(LclSsaVarDsc* ssaDef, GenTree* defNode)
: m_ssaDef(ssaDef)
#ifdef DEBUG
, m_defNode(defNode)
#endif
{
}

LclSsaVarDsc* GetSsaDef() const
{
return m_ssaDef;
}

#ifdef DEBUG
GenTree* GetDefNode() const
{
return m_defNode;
}
#endif
};

typedef ArrayStack<CopyPropSsaDef> CopyPropSsaDefStack;
typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, CopyPropSsaDefStack*> LclNumToLiveDefsMap;

// Copy propagation functions.
void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName);
void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
void optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
void optCopyPropPushDef(GenTreeOp* asg,
GenTreeLclVarCommon* lclNode,
unsigned lclNum,
LclNumToLiveDefsMap* curSsaName);
unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode);
int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2);
int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2);
void optVnCopyProp();
INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName));
INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName));

/**************************************************************************
* Early value propagation
Expand Down
181 changes: 110 additions & 71 deletions src/coreclr/jit/copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
* of the graph originating from the block. Refer SSA renaming for any additional info.
* "curSsaName" tracks the currently live definitions.
*/
void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName)
void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName)
{
for (Statement* const stmt : block->Statements())
{
Expand All @@ -42,7 +42,7 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt
continue;
}

GenTreePtrStack* stack = nullptr;
CopyPropSsaDefStack* stack = nullptr;
curSsaName->Lookup(lclNum, &stack);
stack->Pop();
if (stack->Empty())
Expand All @@ -55,12 +55,12 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt
}

#ifdef DEBUG
void Compiler::optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)
void Compiler::optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)
{
JITDUMP("{ ");
for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
for (LclNumToLiveDefsMap::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
{
GenTreeLclVarCommon* lclVar = iter.GetValue()->Top()->AsLclVarCommon();
GenTreeLclVarCommon* lclVar = iter.GetValue()->Top().GetDefNode()->AsLclVarCommon();
unsigned ssaLclNum = optIsSsaLocal(lclVar);
assert(ssaLclNum != BAD_VAR_NUM);

Expand All @@ -82,7 +82,7 @@ void Compiler::optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)
* Given the "lclVar" and "copyVar" compute if the copy prop will be beneficial.
*
*/
int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2)
int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2)
{
int score = 0;

Expand Down Expand Up @@ -126,16 +126,17 @@ int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDs
// tree - The local tree to perform copy propagation on
// lclNum - The local number of said tree
// curSsaName - The map from lclNum to its recently live definitions as a stack

void Compiler::optCopyProp(Statement* stmt,
GenTreeLclVarCommon* tree,
unsigned lclNum,
LclNumToGenTreePtrStack* curSsaName)
//
void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName)
{
assert((lclNum != BAD_VAR_NUM) && (optIsSsaLocal(tree) == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0));
assert(tree->gtVNPair.GetConservative() != ValueNumStore::NoVN);
assert(tree->gtVNPair.BothDefined());

for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
LclVarDsc* varDsc = lvaGetDesc(lclNum);
ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative();
assert(lclDefVN != ValueNumStore::NoVN);

for (LclNumToLiveDefsMap::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
{
unsigned newLclNum = iter.Get();

Expand All @@ -145,35 +146,30 @@ void Compiler::optCopyProp(Statement* stmt,
continue;
}

LclVarDsc* varDsc = lvaGetDesc(lclNum);
LclVarDsc* newLclVarDsc = lvaGetDesc(newLclNum);
GenTree* newLclDefNode = iter.GetValue()->Top(); // Note that this "def" node can actually be a use (for
// parameters and other use-before-def locals).
CopyPropSsaDef newLclDef = iter.GetValue()->Top();
LclSsaVarDsc* newLclSsaDef = newLclDef.GetSsaDef();

// Do not copy propagate if the old and new lclVar have different 'doNotEnregister' settings.
// This is primarily to avoid copy propagating to IND(ADDR(LCL_VAR)) where the replacement lclVar
// is not marked 'lvDoNotEnregister'.
// However, in addition, it may not be profitable to propagate a 'doNotEnregister' lclVar to an
// existing use of an enregisterable lclVar.
if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister)
// Likewise, nothing to do if the most recent def is not available.
if (newLclSsaDef == nullptr)
{
continue;
}

if ((gsShadowVarInfo != nullptr) && newLclVarDsc->lvIsParam &&
(gsShadowVarInfo[newLclNum].shadowCopy != BAD_VAR_NUM))
{
continue;
}
ValueNum newLclDefVN = newLclSsaDef->m_vnPair.GetConservative();
assert(newLclDefVN != ValueNumStore::NoVN);

ValueNum newLclDefVN = GetUseAsgDefVNOrTreeVN(newLclDefNode);
if (newLclDefVN == ValueNumStore::NoVN)
if (newLclDefVN != lclDefVN)
{
continue;
}

ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative();
if (newLclDefVN != lclDefVN)
// Do not copy propagate if the old and new lclVar have different 'doNotEnregister' settings.
// This is primarily to avoid copy propagating to IND(ADDR(LCL_VAR)) where the replacement lclVar
// is not marked 'lvDoNotEnregister'.
// However, in addition, it may not be profitable to propagate a 'doNotEnregister' lclVar to an
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
// existing use of an enregisterable lclVar.
LclVarDsc* newLclVarDsc = lvaGetDesc(newLclNum);
if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister)
{
continue;
}
Expand All @@ -182,6 +178,7 @@ void Compiler::optCopyProp(Statement* stmt,
{
continue;
}

// Check whether the newLclNum is live before being substituted. Otherwise, we could end
// up in a situation where there must've been a phi node that got pruned because the variable
// is not live anymore. For example,
Expand All @@ -197,7 +194,7 @@ void Compiler::optCopyProp(Statement* stmt,
// 'c' with 'x.'

// We compute liveness only on tracked variables. And all SSA locals are tracked.
assert(lvaGetDesc(newLclNum)->lvTracked);
assert(newLclVarDsc->lvTracked);

// Because of this dependence on live variable analysis, CopyProp phase is immediately
// after Liveness, SSA and VN.
Expand All @@ -206,21 +203,6 @@ void Compiler::optCopyProp(Statement* stmt,
continue;
}

unsigned newSsaNum = SsaConfig::RESERVED_SSA_NUM;
if (newLclDefNode->gtFlags & GTF_VAR_DEF)
{
newSsaNum = GetSsaNumForLocalVarDef(newLclDefNode);
}
else // parameters, this pointer etc.
{
newSsaNum = newLclDefNode->AsLclVarCommon()->GetSsaNum();
}

if (newSsaNum == SsaConfig::RESERVED_SSA_NUM)
{
continue;
}

var_types newLclType = newLclVarDsc->TypeGet();
if (!newLclVarDsc->lvNormalizeOnLoad())
{
Expand All @@ -238,12 +220,15 @@ void Compiler::optCopyProp(Statement* stmt,
JITDUMP("VN based copy assertion for ");
printTreeID(tree);
printf(" V%02d " FMT_VN " by ", lclNum, lclDefVN);
printTreeID(newLclDefNode);
printTreeID(newLclDef.GetDefNode());
printf(" V%02d " FMT_VN ".\n", newLclNum, newLclDefVN);
DISPNODE(tree);
}
#endif

unsigned newSsaNum = newLclVarDsc->GetSsaNumForSsaDef(newLclSsaDef);
assert(newSsaNum != SsaConfig::RESERVED_SSA_NUM);

tree->AsLclVarCommon()->SetLclNum(newLclNum);
tree->AsLclVarCommon()->SetSsaNum(newSsaNum);
gtUpdateSideEffects(stmt, tree);
Expand Down Expand Up @@ -288,6 +273,74 @@ unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
return lclNum;
}

//------------------------------------------------------------------------------
// optCopyPropPushDef: Push the new live SSA def on the stack for "lclNode".
//
// Arguments:
// asg - The assignment node for this def (will be "nullptr" for "use" defs)
// lclNode - The local tree representing "the def" (that can actually be a use)
// lclNum - The local's number (see "optIsSsaLocal")
// curSsaName - The map of local numbers to stacks of their defs
//
void Compiler::optCopyPropPushDef(GenTreeOp* asg,
GenTreeLclVarCommon* lclNode,
unsigned lclNum,
LclNumToLiveDefsMap* curSsaName)
{
assert((lclNum != BAD_VAR_NUM) && (lclNum == optIsSsaLocal(lclNode)));

// Shadowed parameters are special: they will (at most) have one use, that is one on the RHS of an
// assignment to their shadow, and we must not substitute them anywhere. So we'll not push any defs.
if ((gsShadowVarInfo != nullptr) && lvaGetDesc(lclNum)->lvIsParam &&
(gsShadowVarInfo[lclNum].shadowCopy != BAD_VAR_NUM))
{
assert(!curSsaName->Lookup(lclNum));
return;
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
}

unsigned ssaDefNum = SsaConfig::RESERVED_SSA_NUM;
if (asg == nullptr)
{
// Parameters, this pointer etc.
assert((lclNode->gtFlags & GTF_VAR_DEF) == 0);
assert(lclNode->GetSsaNum() == SsaConfig::FIRST_SSA_NUM);
ssaDefNum = lclNode->GetSsaNum();
}
else
{
assert((lclNode->gtFlags & GTF_VAR_DEF) != 0);
ssaDefNum = GetSsaNumForLocalVarDef(lclNode);

// This will be "RESERVED_SSA_NUM" for promoted struct fields assigned using the parent struct.
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning to add an assert for this? Also, what about the TODO-CQ? Is that something that is planned to address in subsequent PR?

Copy link
Contributor Author

@SingleAccretion SingleAccretion Feb 15, 2022

Choose a reason for hiding this comment

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

Will assert, seems like that would catch unexpected cases.

I am personally not planning on fixing it (well, #61049 would fix it, but it requires more design work), this is just a general TODO-CQ to make the reader aware of this inefficiency (the fact that it is not "by design").

// TODO-CQ: fix this.
assert((ssaDefNum != SsaConfig::RESERVED_SSA_NUM) || lvaGetDesc(lclNode)->CanBeReplacedWithItsField(this));
}

// The default is "not available".
LclSsaVarDsc* ssaDef = nullptr;
if (ssaDefNum != SsaConfig::RESERVED_SSA_NUM)
{
// This code preserves previous behavior. In principle, "ssaDefVN" should
// always be obtained directly from the SSA def descriptor and be valid.
ValueNum ssaDefVN = GetUseAsgDefVNOrTreeVN(lclNode);
assert(ssaDefVN != ValueNumStore::NoVN);

if (ssaDefVN != ValueNumStore::VNForVoid())
{
ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaDefNum);
}
}

CopyPropSsaDefStack* defStack;
if (!curSsaName->Lookup(lclNum, &defStack))
{
defStack = new (curSsaName->GetAllocator()) CopyPropSsaDefStack(curSsaName->GetAllocator());
curSsaName->Set(lclNum, defStack);
}

defStack->Push(CopyPropSsaDef(ssaDef, lclNode));
}

//------------------------------------------------------------------------------
// optBlockCopyProp : Perform copy propagation using currently live definitions on the current block's
// variables. Also as new definitions are encountered update the "curSsaName" which
Expand All @@ -296,8 +349,8 @@ unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
// Arguments:
// block - Block the tree belongs to
// curSsaName - The map from lclNum to its recently live definitions as a stack

void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName)
//
void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName)
{
#ifdef DEBUG
JITDUMP("Copy Assertion for " FMT_BB "\n", block->bbNum);
Expand Down Expand Up @@ -333,13 +386,7 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
continue;
}

GenTreePtrStack* stack;
if (!curSsaName->Lookup(lclNum, &stack))
{
stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
}
stack->Push(lclDefNode);
curSsaName->Set(lclNum, stack, LclNumToGenTreePtrStack::Overwrite);
optCopyPropPushDef(tree->AsOp(), lclDefNode, lclNum, curSsaName);
}
// TODO-CQ: propagate on LCL_FLDs too.
else if (tree->OperIs(GT_LCL_VAR) && ((tree->gtFlags & GTF_VAR_DEF) == 0))
Expand All @@ -351,19 +398,11 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
continue;
}

// If we encounter first use of a param or this pointer add it as a live definition.
// Since they are always live, we'll do it only once.
if (lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg))
// If we encounter first use of a param or this pointer add it as a
// live definition. Since they are always live, we'll do it only once.
if ((lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg)) && !curSsaName->Lookup(lclNum))
SingleAccretion marked this conversation as resolved.
Show resolved Hide resolved
{
GenTreePtrStack* stack;
if (!curSsaName->Lookup(lclNum, &stack))
{
assert(tree->AsLclVarCommon()->GetSsaNum() == SsaConfig::FIRST_SSA_NUM);

stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
stack->Push(tree);
curSsaName->Set(lclNum, stack);
}
optCopyPropPushDef(nullptr, tree->AsLclVarCommon(), lclNum, curSsaName);
}

// TODO-Review: EH successor/predecessor iteration seems broken.
Expand Down Expand Up @@ -422,7 +461,7 @@ void Compiler::optVnCopyProp()
class CopyPropDomTreeVisitor : public DomTreeVisitor<CopyPropDomTreeVisitor>
{
// The map from lclNum to its recently live definitions as a stack.
LclNumToGenTreePtrStack m_curSsaName;
LclNumToLiveDefsMap m_curSsaName;

public:
CopyPropDomTreeVisitor(Compiler* compiler)
Expand Down
Loading