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

Handle embedded assignments in copy propagation #63447

Merged
merged 4 commits into from
Jan 21, 2022
Merged

Handle embedded assignments in copy propagation #63447

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
51c9148
Clean things up a little
SingleAccretion Jan 5, 2022
01a6e5e
Delete a redundant condition
SingleAccretion Jan 5, 2022
4011712
Handle embedded assignments in copy propagation
SingleAccretion Jan 6, 2022
db9a3f9
Simplify optIsSsaLocal
SingleAccretion Jan 6, 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
7 changes: 2 additions & 5 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7370,14 +7370,11 @@ class Compiler
typedef ArrayStack<GenTree*> GenTreePtrStack;
typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*> LclNumToGenTreePtrStack;

// Kill set to track variables with intervening definitions.
VARSET_TP optCopyPropKillSet;

// Copy propagation functions.
void optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName);
void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
unsigned optIsSsaLocal(GenTree* tree);
unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode);
int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2);
void optVnCopyProp();
INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName));
Expand Down
234 changes: 91 additions & 143 deletions src/coreclr/jit/copyprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,16 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt
{
for (GenTree* const tree : stmt->TreeList())
{
if (!tree->IsLocal())
{
continue;
}
const unsigned lclNum = optIsSsaLocal(tree);
if (lclNum == BAD_VAR_NUM)
{
continue;
}
if (tree->gtFlags & GTF_VAR_DEF)
GenTreeLclVarCommon* lclDefNode = nullptr;
if (tree->OperIs(GT_ASG) && tree->DefinesLocal(this, &lclDefNode))
{
const unsigned lclNum = optIsSsaLocal(lclDefNode);

if (lclNum == BAD_VAR_NUM)
{
continue;
}

GenTreePtrStack* stack = nullptr;
curSsaName->Lookup(lclNum, &stack);
stack->Pop();
Expand Down Expand Up @@ -123,97 +122,67 @@ int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDs
// definitions share the same value number. If so, then we can make the replacement.
//
// Arguments:
// block - Block the tree belongs to
// stmt - Statement the tree belongs to
// tree - The tree to perform copy propagation on
// 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(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName)
void Compiler::optCopyProp(Statement* stmt,
GenTreeLclVarCommon* tree,
unsigned lclNum,
LclNumToGenTreePtrStack* curSsaName)
{
// TODO-Review: EH successor/predecessor iteration seems broken.
if (block->bbCatchTyp == BBCT_FINALLY || block->bbCatchTyp == BBCT_FAULT)
{
return;
}

// If not local nothing to do.
if (!tree->IsLocal())
{
return;
}
if (tree->OperGet() == GT_PHI_ARG || tree->OperGet() == GT_LCL_FLD)
{
return;
}

// Propagate only on uses.
if (tree->gtFlags & GTF_VAR_DEF)
{
return;
}
const unsigned lclNum = optIsSsaLocal(tree);

// Skip non-SSA variables.
if (lclNum == BAD_VAR_NUM)
{
return;
}

assert((lclNum != BAD_VAR_NUM) && (optIsSsaLocal(tree) == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0));
assert(tree->gtVNPair.GetConservative() != ValueNumStore::NoVN);

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

GenTree* op = iter.GetValue()->Top();

// Nothing to do if same.
if (lclNum == newLclNum)
{
continue;
}

// Skip variables with assignments embedded in the statement (i.e., with a comma). Because we
// are not currently updating their SSA names as live in the copy-prop pass of the stmt.
if (VarSetOps::IsMember(this, optCopyPropKillSet, lvaTable[newLclNum].lvVarIndex))
{
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).

// 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 (lvaTable[lclNum].lvDoNotEnregister != lvaTable[newLclNum].lvDoNotEnregister)
if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister)
{
continue;
}

if (op->gtFlags & GTF_VAR_CAST)
if ((gsShadowVarInfo != nullptr) && newLclVarDsc->lvIsParam &&
(gsShadowVarInfo[newLclNum].shadowCopy == lclNum))
{
continue;
}
if (gsShadowVarInfo != nullptr && lvaTable[newLclNum].lvIsParam &&
gsShadowVarInfo[newLclNum].shadowCopy == lclNum)
{
continue;
}
ValueNum opVN = GetUseAsgDefVNOrTreeVN(op);
if (opVN == ValueNumStore::NoVN)

ValueNum newLclDefVN = GetUseAsgDefVNOrTreeVN(newLclDefNode);
if (newLclDefVN == ValueNumStore::NoVN)
{
continue;
}
if (op->TypeGet() != tree->TypeGet())

if (newLclDefNode->TypeGet() != tree->TypeGet())
{
continue;
}
if (opVN != tree->gtVNPair.GetConservative())

if (newLclDefVN != tree->gtVNPair.GetConservative())
{
continue;
}
if (optCopyProp_LclVarScore(lvaGetDesc(lclNum), lvaGetDesc(newLclNum), true) <= 0)

if (optCopyProp_LclVarScore(varDsc, newLclVarDsc, true) <= 0)
{
continue;
}
Expand All @@ -230,34 +199,25 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
// node x2 = phi(x0, x1) which can then be used to substitute 'c' with. But because of pruning
// there would be no such phi node. To solve this we'll check if 'x' is live, before replacing
// 'c' with 'x.'
if (!lvaTable[newLclNum].lvVerTypeInfo.IsThisPtr())
{
if (lvaTable[newLclNum].IsAddressExposed())
{
continue;
}

// We compute liveness only on tracked variables. So skip untracked locals.
if (!lvaTable[newLclNum].lvTracked)
{
continue;
}
// We compute liveness only on tracked variables. And all SSA locals are tracked.
assert(lvaGetDesc(newLclNum)->lvTracked);

// Because of this dependence on live variable analysis, CopyProp phase is immediately
// after Liveness, SSA and VN.
if (!VarSetOps::IsMember(this, compCurLife, lvaTable[newLclNum].lvVarIndex))
{
continue;
}
// Because of this dependence on live variable analysis, CopyProp phase is immediately
// after Liveness, SSA and VN.
if ((newLclNum != info.compThisArg) && !VarSetOps::IsMember(this, compCurLife, newLclVarDsc->lvVarIndex))
{
continue;
}

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

if (newSsaNum == SsaConfig::RESERVED_SSA_NUM)
Expand All @@ -271,48 +231,42 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
JITDUMP("VN based copy assertion for ");
printTreeID(tree);
printf(" V%02d " FMT_VN " by ", lclNum, tree->GetVN(VNK_Conservative));
printTreeID(op);
printf(" V%02d " FMT_VN ".\n", newLclNum, op->GetVN(VNK_Conservative));
gtDispTree(tree, nullptr, nullptr, true);
printTreeID(newLclDefNode);
printf(" V%02d " FMT_VN ".\n", newLclNum, newLclDefNode->GetVN(VNK_Conservative));
DISPNODE(tree);
}
#endif

tree->AsLclVarCommon()->SetLclNum(newLclNum);
tree->AsLclVarCommon()->SetSsaNum(newSsaNum);
gtUpdateSideEffects(stmt, tree);

#ifdef DEBUG
if (verbose)
{
printf("copy propagated to:\n");
gtDispTree(tree, nullptr, nullptr, true);
DISPNODE(tree);
}
#endif
break;
}
return;
}

//------------------------------------------------------------------------------
// optIsSsaLocal : helper to check if the tree is a local that participates in SSA numbering.
//
// Arguments:
// tree - The tree to perform the check on;
// lclNode - The local tree to perform the check on;
//
// 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(GenTree* tree)
unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
{
if (!tree->IsLocal())
{
return BAD_VAR_NUM;
}

GenTreeLclVarCommon* lclNode = tree->AsLclVarCommon();
unsigned lclNum = lclNode->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);
unsigned lclNum = lclNode->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);

if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this))
{
Expand Down Expand Up @@ -355,68 +309,63 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
VarSetOps::Assign(this, compCurLife, block->bbLiveIn);
for (Statement* const stmt : block->Statements())
{
VarSetOps::ClearD(this, optCopyPropKillSet);

// Walk the tree to find if any local variable can be replaced with current live definitions.
// Simultaneously, push live definitions on the stack - that logic must be in sync with the
// SSA renaming process.
for (GenTree* const tree : stmt->TreeList())
{
treeLifeUpdater.UpdateLife(tree);

optCopyProp(block, stmt, tree, curSsaName);

// TODO-Review: Merge this loop with the following loop to correctly update the
// live SSA num while also propagating copies.
//
// 1. This loop performs copy prop with currently live (on-top-of-stack) SSA num.
// 2. The subsequent loop maintains a stack for each lclNum with
// currently active SSA numbers when definitions are encountered.
//
// If there is an embedded definition using a "comma" in a stmt, then the currently
// live SSA number will get updated only in the next loop (2). However, this new
// definition is now supposed to be live (on tos). If we did not update the stacks
// using (2), copy prop (1) will use a SSA num defined outside the stmt ignoring the
// embedded update. Killing the variable is a simplification to produce 0 ASM diffs
// for an update release.
//
const unsigned lclNum = optIsSsaLocal(tree);
if ((lclNum != BAD_VAR_NUM) && (tree->gtFlags & GTF_VAR_DEF))
GenTreeLclVarCommon* lclDefNode = nullptr;
if (tree->OperIs(GT_ASG) && tree->DefinesLocal(this, &lclDefNode))
{
VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[lclNum].lvVarIndex);
}
}
const unsigned lclNum = optIsSsaLocal(lclDefNode);

// This logic must be in sync with SSA renaming process.
for (GenTree* const tree : stmt->TreeList())
{
const unsigned lclNum = optIsSsaLocal(tree);
if (lclNum == BAD_VAR_NUM)
{
continue;
}
if (lclNum == BAD_VAR_NUM)
{
continue;
}

// As we encounter a definition add it to the stack as a live definition.
if (tree->gtFlags & GTF_VAR_DEF)
{
GenTreePtrStack* stack;
if (!curSsaName->Lookup(lclNum, &stack))
{
stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
}
stack->Push(tree);
stack->Push(lclDefNode);
curSsaName->Set(lclNum, stack, LclNumToGenTreePtrStack::Overwrite);
}
// If we encounter first use of a param or this pointer add it as a live definition.
// Since they are always live, do it only once.
else if ((tree->gtOper == GT_LCL_VAR) && !(tree->gtFlags & GTF_VAR_USEASG) &&
(lvaTable[lclNum].lvIsParam || lvaTable[lclNum].lvVerTypeInfo.IsThisPtr()))
// TODO-CQ: propagate on LCL_FLDs too.
else if (tree->OperIs(GT_LCL_VAR) && ((tree->gtFlags & GTF_VAR_DEF) == 0))
{
GenTreePtrStack* stack;
if (!curSsaName->Lookup(lclNum, &stack))
const unsigned lclNum = optIsSsaLocal(tree->AsLclVarCommon());

if (lclNum == BAD_VAR_NUM)
{
stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
stack->Push(tree);
curSsaName->Set(lclNum, stack);
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))
{
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);
}
}

// TODO-Review: EH successor/predecessor iteration seems broken.
if ((block->bbCatchTyp == BBCT_FINALLY) || (block->bbCatchTyp == BBCT_FAULT))
{
continue;
}

optCopyProp(stmt, tree->AsLclVarCommon(), lclNum, curSsaName);
}
}
}
Expand Down Expand Up @@ -462,7 +411,6 @@ void Compiler::optVnCopyProp()
}

VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this));
VarSetOps::AssignNoCopy(this, optCopyPropKillSet, VarSetOps::MakeEmpty(this));

class CopyPropDomTreeVisitor : public DomTreeVisitor<CopyPropDomTreeVisitor>
{
Expand Down