Skip to content

Commit

Permalink
Improvements for dead store removal. (dotnet#38004)
Browse files Browse the repository at this point in the history 
1. Don't mark fields of dependently promoted structs as untracked.

2. Remove some stores whose lhs local has a ref count of 1 when running
late liveness. We can rely on ref counts since they are calculated right
before the late liveness pass.

3. Remove dead GT_STORE_BLK in addition to GT_STOREIND in the late
liveness pass.

4. Remove dead stores to untracked locals in the late liveness pass.

5. Allow optRemoveRedundantZeroInits to remove some redundant initializations
of tracked locals. Move the phase to right after liveness so that SSA is correct
after removing assignments to tracked locals.
  • Loading branch information
@erozenfeld
erozenfeld authored Jun 27, 2020
1 parent 5988e9c commit 5e153a2
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 71 deletions.
2 changes: 0 additions & 2 deletions src/coreclr/src/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4757,8 +4757,6 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
DoPhase(this, PHASE_BUILD_SSA, &Compiler::fgSsaBuild);
}

DoPhase(this, PHASE_ZERO_INITS, &Compiler::optRemoveRedundantZeroInits);

if (doEarlyProp)
{
// Propagate array length and rewrite getType() method call
Expand Down
17 changes: 1 addition & 16 deletions src/coreclr/src/jit/lclvars.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3411,23 +3411,8 @@ void Compiler::lvaSortByRefCount()
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_IsStruct));
}
}
else if (varDsc->lvIsStructField && (lvaGetParentPromotionType(lclNum) != PROMOTION_TYPE_INDEPENDENT) &&
(lvaGetDesc(varDsc->lvParentLcl)->lvRefCnt() > 1))
else if (varDsc->lvIsStructField && (lvaGetParentPromotionType(lclNum) != PROMOTION_TYPE_INDEPENDENT))
{
// SSA must exclude struct fields that are not independently promoted
// as dependent fields could be assigned using a CopyBlock
// resulting in a single node causing multiple SSA definitions
// which isn't currently supported by SSA
//
// If the parent struct local ref count is less than 2, then either the struct is no longer
// referenced or the field is no longer referenced: we increment the struct local ref count in incRefCnts
// for each field use when the struct is dependently promoted. This can happen, e.g, if we've removed
// a block initialization for the struct. In that case we can still track the local field.
//
// TODO-CQ: Consider using lvLclBlockOpAddr and only marking these LclVars
// untracked when a blockOp is used to assign the struct.
//
varDsc->lvTracked = 0; // so, don't mark as tracked
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_DepField));
}
else if (varDsc->lvPinned)
Expand Down
115 changes: 79 additions & 36 deletions src/coreclr/src/jit/liveness.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1644,14 +1644,40 @@ bool Compiler::fgComputeLifeUntrackedLocal(VARSET_TP& life,
{
assert(lclVarNode != nullptr);

bool isDef = ((lclVarNode->gtFlags & GTF_VAR_DEF) != 0);

// We have accurate ref counts when running late liveness so we can eliminate
// some stores if the lhs local has a ref count of 1.
if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned)
{
if (varDsc.lvIsStructField)
{
if ((lvaGetDesc(varDsc.lvParentLcl)->lvRefCnt() == 1) &&
(lvaGetParentPromotionType(&varDsc) == PROMOTION_TYPE_DEPENDENT))
{
return true;
}
}
else if (varTypeIsStruct(varDsc.lvType))
{
if (lvaGetPromotionType(&varDsc) != PROMOTION_TYPE_INDEPENDENT)
{
return true;
}
}
else
{
return true;
}
}

if (!varTypeIsStruct(varDsc.lvType) || (lvaGetPromotionType(&varDsc) == PROMOTION_TYPE_NONE))
{
return false;
}

VARSET_TP fieldSet(VarSetOps::MakeEmpty(this));
bool fieldsAreTracked = true;
bool isDef = ((lclVarNode->gtFlags & GTF_VAR_DEF) != 0);

for (unsigned i = varDsc.lvFieldLclStart; i < varDsc.lvFieldLclStart + varDsc.lvFieldCnt; ++i)
{
Expand Down Expand Up @@ -1682,8 +1708,12 @@ bool Compiler::fgComputeLifeUntrackedLocal(VARSET_TP& life,
if (isDef)
{
VARSET_TP liveFields(VarSetOps::Intersection(this, life, fieldSet));
VarSetOps::DiffD(this, fieldSet, keepAliveVars);
VarSetOps::DiffD(this, life, fieldSet);
if ((lclVarNode->gtFlags & GTF_VAR_USEASG) == 0)
{
VarSetOps::DiffD(this, fieldSet, keepAliveVars);
VarSetOps::DiffD(this, life, fieldSet);
}

if (fieldsAreTracked && VarSetOps::IsEmpty(this, liveFields))
{
// None of the fields were live, so this is a dead store.
Expand All @@ -1693,12 +1723,14 @@ bool Compiler::fgComputeLifeUntrackedLocal(VARSET_TP& life,
VARSET_TP keepAliveFields(VarSetOps::Intersection(this, fieldSet, keepAliveVars));
noway_assert(VarSetOps::IsEmpty(this, keepAliveFields));

// Do not consider this store dead if the parent local variable is an address exposed local.
return !varDsc.lvAddrExposed;
// Do not consider this store dead if the parent local variable is an address exposed local or
// if the struct has a custom layout and holes.
return !(varDsc.lvAddrExposed || (varDsc.lvCustomLayout && varDsc.lvContainsHoles));
}
}
return false;
}

// This is a use.

// Are the variables already known to be alive?
Expand Down Expand Up @@ -1934,18 +1966,27 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
if (isDeadStore)
{
LIR::Use addrUse;
if (blockRange.TryGetUse(node, &addrUse) && (addrUse.User()->OperGet() == GT_STOREIND))
if (blockRange.TryGetUse(node, &addrUse) &&
(addrUse.User()->OperIs(GT_STOREIND, GT_STORE_BLK, GT_STORE_OBJ)))
{
// Remove the store. DCE will iteratively clean up any ununsed operands.
GenTreeStoreInd* const store = addrUse.User()->AsStoreInd();
GenTreeIndir* const store = addrUse.User()->AsIndir();

JITDUMP("Removing dead indirect store:\n");
DISPNODE(store);

assert(store->Addr() == node);
blockRange.Delete(this, block, node);

store->Data()->SetUnusedValue();
GenTree* data =
store->OperIs(GT_STOREIND) ? store->AsStoreInd()->Data() : store->AsBlk()->Data();
data->SetUnusedValue();
if (data->isIndir())
{
// This is a block assignment. An indirection of the rhs is not considered
// to happen until the assignment so mark it as non-faulting.
data->gtFlags |= GTF_IND_NONFAULTING;
}

blockRange.Remove(store);

Expand All @@ -1965,39 +2006,41 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
if (varDsc.lvTracked)
{
isDeadStore = fgComputeLifeTrackedLocalDef(life, keepAliveVars, varDsc, lclVarNode);
if (isDeadStore)
{
JITDUMP("Removing dead store:\n");
DISPNODE(lclVarNode);
}
else
{
isDeadStore = fgComputeLifeUntrackedLocal(life, keepAliveVars, varDsc, lclVarNode);
}

// Remove the store. DCE will iteratively clean up any ununsed operands.
lclVarNode->gtOp1->SetUnusedValue();
if (isDeadStore)
{
JITDUMP("Removing dead store:\n");
DISPNODE(lclVarNode);

// If the store is marked as a late argument, it is referenced by a call. Instead of removing
// it, bash it to a NOP.
if ((node->gtFlags & GTF_LATE_ARG) != 0)
{
JITDUMP("node is a late arg; replacing with NOP\n");
node->gtBashToNOP();
// Remove the store. DCE will iteratively clean up any ununsed operands.
lclVarNode->gtOp1->SetUnusedValue();

// NOTE: this is a bit of a hack. We need to keep these nodes around as they are
// referenced by the call, but they're considered side-effect-free non-value-producing
// nodes, so they will be removed if we don't do this.
node->gtFlags |= GTF_ORDER_SIDEEFF;
}
else
{
blockRange.Remove(node);
}
// If the store is marked as a late argument, it is referenced by a call. Instead of removing
// it, bash it to a NOP.
if ((node->gtFlags & GTF_LATE_ARG) != 0)
{
JITDUMP("node is a late arg; replacing with NOP\n");
node->gtBashToNOP();

assert(!opts.MinOpts());
fgStmtRemoved = true;
// NOTE: this is a bit of a hack. We need to keep these nodes around as they are
// referenced by the call, but they're considered side-effect-free non-value-producing
// nodes, so they will be removed if we don't do this.
node->gtFlags |= GTF_ORDER_SIDEEFF;
}
else
{
blockRange.Remove(node);
}

assert(!opts.MinOpts());
fgStmtRemoved = true;
}
else
{
fgComputeLifeUntrackedLocal(life, keepAliveVars, varDsc, lclVarNode);
}

break;
}

Expand Down Expand Up @@ -2533,7 +2576,7 @@ void Compiler::fgInterBlockLocalVarLiveness()
if (isFinallyVar)
{
// Set lvMustInit only if we have a non-arg, GC pointer.
if (!varDsc->lvIsParam && varTypeIsGC(varDsc->TypeGet()) && !fieldOfDependentlyPromotedStruct)
if (!varDsc->lvIsParam && varTypeIsGC(varDsc->TypeGet()))
{
varDsc->lvMustInit = true;
}
Expand Down
96 changes: 79 additions & 17 deletions src/coreclr/src/jit/optimizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9241,6 +9241,9 @@ void Compiler::optRemoveRedundantZeroInits()
block = block->GetUniqueSucc())
{
block->bbFlags |= BBF_MARKED;
CompAllocator allocator(getAllocator(CMK_ZeroInit));
LclVarRefCounts defsInBlock(allocator);
bool removedTrackedDefs = false;
for (Statement* stmt = block->FirstNonPhiDef(); stmt != nullptr;)
{
Statement* next = stmt->GetNextStmt();
Expand Down Expand Up @@ -9274,6 +9277,48 @@ void Compiler::optRemoveRedundantZeroInits()
refCounts.Set(lclNum, 1);
}

if ((tree->gtFlags & GTF_VAR_DEF) == 0)
{
break;
}

// We need to count the number of tracked var defs in the block
// so that we can update block->bbVarDef if we remove any tracked var defs.

LclVarDsc* const lclDsc = lvaGetDesc(lclNum);
if (lclDsc->lvTracked)
{
unsigned* pDefsCount = defsInBlock.LookupPointer(lclNum);
if (pDefsCount != nullptr)
{
*pDefsCount = (*pDefsCount) + 1;
}
else
{
defsInBlock.Set(lclNum, 1);
}
}
else if (varTypeIsStruct(lclDsc) && ((tree->gtFlags & GTF_VAR_USEASG) == 0) &&
lvaGetPromotionType(lclDsc) != PROMOTION_TYPE_NONE)
{
for (unsigned i = lclDsc->lvFieldLclStart; i < lclDsc->lvFieldLclStart + lclDsc->lvFieldCnt;
++i)
{
if (lvaGetDesc(i)->lvTracked)
{
unsigned* pDefsCount = defsInBlock.LookupPointer(i);
if (pDefsCount != nullptr)
{
*pDefsCount = (*pDefsCount) + 1;
}
else
{
defsInBlock.Set(i, 1);
}
}
}
}

break;
}
case GT_ASG:
Expand Down Expand Up @@ -9339,26 +9384,29 @@ void Compiler::optRemoveRedundantZeroInits()

if (treeOp->gtOp2->IsIntegralConst(0))
{
if (!lclDsc->lvTracked)
{
bool bbInALoop = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
bool bbIsReturn = block->bbJumpKind == BBJ_RETURN;
bool bbInALoop = (block->bbFlags & BBF_BACKWARD_JUMP) != 0;
bool bbIsReturn = block->bbJumpKind == BBJ_RETURN;

if (BitVecOps::IsMember(&bitVecTraits, zeroInitLocals, lclNum) ||
(lclDsc->lvIsStructField &&
BitVecOps::IsMember(&bitVecTraits, zeroInitLocals, lclDsc->lvParentLcl)) ||
!fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn))
if (BitVecOps::IsMember(&bitVecTraits, zeroInitLocals, lclNum) ||
(lclDsc->lvIsStructField &&
BitVecOps::IsMember(&bitVecTraits, zeroInitLocals, lclDsc->lvParentLcl)) ||
(!lclDsc->lvTracked && !fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn)))
{
// We are guaranteed to have a zero initialization in the prolog or a
// dominating explicit zero initialization and the local hasn't been redefined
// between the prolog and this explicit zero initialization so the assignment
// can be safely removed.
if (tree == stmt->GetRootNode())
{
// We are guaranteed to have a zero initialization in the prolog or a
// dominating explicit zero initialization and the local hasn't been redefined
// between the prolog and this explicit zero initialization so the assignment
// can be safely removed.
if (tree == stmt->GetRootNode())
fgRemoveStmt(block, stmt);
removedExplicitZeroInit = true;
lclDsc->lvSuppressedZeroInit = 1;

if (lclDsc->lvTracked)
{
fgRemoveStmt(block, stmt);
removedExplicitZeroInit = true;
lclDsc->lvSuppressedZeroInit = 1;
lclDsc->setLvRefCnt(lclDsc->lvRefCnt() - 1);
removedTrackedDefs = true;
unsigned* pDefsCount = defsInBlock.LookupPointer(lclNum);
*pDefsCount = (*pDefsCount) - 1;
}
}
}
Expand Down Expand Up @@ -9392,6 +9440,20 @@ void Compiler::optRemoveRedundantZeroInits()
}
stmt = next;
}

if (removedTrackedDefs)
{
LclVarRefCounts::KeyIterator iter(defsInBlock.Begin());
LclVarRefCounts::KeyIterator end(defsInBlock.End());
for (; !iter.Equal(end); iter++)
{
unsigned int lclNum = iter.Get();
if (defsInBlock[lclNum] == 0)
{
VarSetOps::RemoveElemD(this, block->bbVarDef, lvaGetDesc(lclNum)->lvVarIndex);
}
}
}
}

for (BasicBlock* block = fgFirstBB; (block != nullptr) && ((block->bbFlags & BBF_MARKED) != 0);
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/src/jit/ssabuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,9 @@ void SsaBuilder::Build()
m_pCompiler->fgLocalVarLiveness();
EndPhase(PHASE_BUILD_SSA_LIVENESS);

m_pCompiler->optRemoveRedundantZeroInits();
EndPhase(PHASE_ZERO_INITS);

// Mark all variables that will be tracked by SSA
for (unsigned lclNum = 0; lclNum < m_pCompiler->lvaCount; lclNum++)
{
Expand Down

0 comments on commit 5e153a2

Please sign in to comment.