Skip to content

Commit

Permalink
Support deleting "entire" stores in the VN-based dead store removal p…
Browse files Browse the repository at this point in the history
…hase (#79505)

* Add a flag for explicit inits

* Fix VN

* Add support for "whole" stores
  • Loading branch information
SingleAccretion authored Jan 4, 2023
1 parent fc75aa0 commit 517698d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 13 deletions.
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9326,6 +9326,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[VAR_DEATH]");
}
if (tree->gtFlags & GTF_VAR_EXPLICIT_INIT)
{
chars += printf("[VAR_EXPLICIT_INIT]");
}
#if defined(DEBUG)
if (tree->gtDebugFlags & GTF_DEBUG_VAR_CSE_REF)
{
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3986,6 +3986,11 @@ bool Compiler::fgVarNeedsExplicitZeroInit(unsigned varNum, bool bbInALoop, bool
return true;
}

if (varDsc->lvHasExplicitInit)
{
return true;
}

if (fgVarIsNeverZeroInitializedInProlog(varNum))
{
return true;
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,6 @@ enum GenTreeFlags : unsigned int
// well to make sure it's the right operator for the particular flag.
//---------------------------------------------------------------------

// NB: GTF_VAR_* and GTF_REG_* share the same namespace of flags.
// These flags are also used by GT_LCL_FLD, and the last-use (DEATH) flags are also used by GenTreeCopyOrReload.

GTF_VAR_DEF = 0x80000000, // GT_LCL_VAR -- this is a definition
Expand All @@ -469,11 +468,12 @@ enum GenTreeFlags : unsigned int
// that returns its result in multiple registers such as a long multiply). Set by
// (and thus only valid after) lowering.

GTF_LIVENESS_MASK = GTF_VAR_DEF | GTF_VAR_USEASG | GTF_VAR_DEATH_MASK,
GTF_LIVENESS_MASK = GTF_VAR_DEF | GTF_VAR_USEASG | GTF_VAR_DEATH_MASK,

GTF_VAR_ITERATOR = 0x01000000, // GT_LCL_VAR -- this is a iterator reference in the loop condition
GTF_VAR_CLONED = 0x00800000, // GT_LCL_VAR -- this node has been cloned or is a clone
GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup
GTF_VAR_ITERATOR = 0x01000000, // GT_LCL_VAR -- this is a iterator reference in the loop condition
GTF_VAR_CLONED = 0x00800000, // GT_LCL_VAR -- this node has been cloned or is a clone
GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup
GTF_VAR_EXPLICIT_INIT = 0x00200000, // GT_LCL_VAR -- this node is an "explicit init" store. Valid until rationalization.

// For additional flags for GT_CALL node see GTF_CALL_M_*

Expand Down
52 changes: 44 additions & 8 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10294,6 +10294,7 @@ void Compiler::optRemoveRedundantZeroInits()
// the prolog and this explicit initialization. Therefore, it doesn't
// require zero initialization in the prolog.
lclDsc->lvHasExplicitInit = 1;
lclVar->gtFlags |= GTF_VAR_EXPLICIT_INIT;
JITDUMP("Marking V%02u as having an explicit init\n", lclNum);
}
}
Expand Down Expand Up @@ -10361,7 +10362,7 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()

LclVarDsc* varDsc = lvaGetDesc(lclNum);
unsigned defCount = varDsc->lvPerSsaData.GetCount();
if (defCount <= 2)
if (defCount <= 1)
{
continue;
}
Expand All @@ -10378,22 +10379,53 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()
JITDUMP("Considering [%06u] for removal...\n", dspTreeID(store));

GenTree* lhs = store->gtGetOp1();
if (!lhs->OperIs(GT_LCL_FLD) || ((lhs->gtFlags & GTF_VAR_USEASG) == 0) ||
(lhs->AsLclFld()->GetLclNum() != lclNum))
if (lhs->AsLclVarCommon()->GetLclNum() != lclNum)
{
JITDUMP(" -- no; composite definition\n");
continue;
}

ValueNum oldLclValue = varDsc->GetPerSsaData(defDsc->GetUseDefSsaNum())->m_vnPair.GetConservative();
ValueNum oldStoreValue =
vnStore->VNForLoad(VNK_Conservative, oldLclValue, lvaLclExactSize(lclNum), lhs->TypeGet(),
lhs->AsLclFld()->GetLclOffs(), lhs->AsLclFld()->GetSize());
ValueNum oldStoreValue;
if ((lhs->gtFlags & GTF_VAR_USEASG) == 0)
{
LclSsaVarDsc* lastDefDsc = varDsc->lvPerSsaData.GetSsaDefByIndex(defIndex - 1);
if (lastDefDsc->GetBlock() != defDsc->GetBlock())
{
JITDUMP(" -- no; last def not in the same block\n");
continue;
}

if ((lhs->gtFlags & GTF_VAR_EXPLICIT_INIT) != 0)
{
// Removing explicit inits is not profitable for primitives and not safe for structs.
JITDUMP(" -- no; 'explicit init'\n");
continue;
}

// CQ heuristic: avoid removing defs of enregisterable locals where this is likely to
// make them "must-init", extending live ranges. Here we assume the first SSA def was
// the implicit "live-in" one, which is not guaranteed, but very likely.
if ((defIndex == 1) && (varDsc->TypeGet() != TYP_STRUCT))
{
JITDUMP(" -- no; first explicit def of a non-STRUCT local\n", lclNum);
continue;
}

oldStoreValue = lastDefDsc->m_vnPair.GetConservative();
}
else
{
ValueNum oldLclValue = varDsc->GetPerSsaData(defDsc->GetUseDefSsaNum())->m_vnPair.GetConservative();
oldStoreValue =
vnStore->VNForLoad(VNK_Conservative, oldLclValue, lvaLclExactSize(lclNum), lhs->TypeGet(),
lhs->AsLclFld()->GetLclOffs(), lhs->AsLclFld()->GetSize());
}

GenTree* rhs = store->gtGetOp2();
ValueNum storeValue;
if (lhs->TypeIs(TYP_STRUCT) && rhs->IsIntegralConst(0))
{
storeValue = vnStore->VNForZeroObj(lhs->AsLclFld()->GetLayout());
storeValue = vnStore->VNForZeroObj(lhs->AsLclVarCommon()->GetLayout(this));
}
else
{
Expand All @@ -10419,6 +10451,10 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()

madeChanges = true;
}
else
{
JITDUMP(" -- no; not redundant\n");
}
}
}
}
Expand Down

0 comments on commit 517698d

Please sign in to comment.