Skip to content

Commit

Permalink
JIT: Stop using ref counts in forward sub (#85909)
Browse files Browse the repository at this point in the history 
Instead of going to great lengths to keep the ref counts correct in
physical promotion just stop using them in forward sub. The last use
information generalizes what forward sub needs except for when the
tracking limit is reached, which shows in the improvements we actually
get from consulting the ref counts -- the improvements are very modest
and in tests only.

This makes early liveness the only consumer of the "regular" ref counts
computed by local morph. Weighted ref counts are still used for undoing
regular promotion of implicit byref parameters.
  • Loading branch information
@jakobbotsch
jakobbotsch authored May 9, 2023
1 parent 7d54c05 commit 547c506
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 111 deletions.
73 changes: 20 additions & 53 deletions src/coreclr/jit/forwardsub.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
//
// This phase tries to reconnect trees that were split early on by
// phases like the importer and inlining. We run it before morph
// to provide more context for morph's tree based optimizations, and
// we run it after the local address visitor because that phase sets
// address exposure for locals and computes (early) ref counts.
// to provide more context for morph's tree based optimizations and
// it makes use of early liveness to know which uses are last uses.
//
// The general pattern we look for is
//
Expand All @@ -23,8 +22,7 @@
// Statement(n+1):
// ... use of lcl ...
//
// where those are the only appearances of lcl and lcl is not address
// exposed.
// where the use of lcl is a last use.
//
// The "optimization" here transforms this to
//
Expand All @@ -38,6 +36,8 @@
// For throughput, we try and early out on illegal or unprofitable cases
// before doing the more costly bits of analysis. We only scan a limited
// amount of IR and just give up if we can't find what we are looking for.
// We use the linked lists of locals to quickly find out if there is a
// candidate last use in the next statement.
//
// If we're successful we will backtrack a bit, to try and catch cases like
//
Expand All @@ -62,15 +62,12 @@
// that upstream phases didn't leave the wrong flags.
//
// For profitability we first try and avoid code growth. We do this
// by only substituting in cases where lcl has exactly one def and one use.
// This info is computed for us but the RCS_Early ref counting done during
// the immediately preceding fgMarkAddressExposedLocals phase.
// by only substituting in cases where we can see a def followed by
// a single (last) use, i.e. we do not allow substituting multiple
// uses.
//
// Because of this, once we've substituted "tree" we know that lcl is dead
// and we can remove the assignment statement.
//
// Even with ref count screening, we don't know for sure where the
// single use of local might be, so we have to seach for it.
// Once we've substituted "tree" we know that lcl is dead (since the use was a
// last use) and we can remove the assignment statement.
//
// We also take pains not to create overly large trees as the recursion
// done by morph incorporates a lot of state; deep trees may lead to
Expand Down Expand Up @@ -114,6 +111,12 @@ PhaseStatus Compiler::fgForwardSub()
}
#endif

if (!fgDidEarlyLiveness)
{
JITDUMP("Liveness information not available, skipping forward sub\n");
return PhaseStatus::MODIFIED_NOTHING;
}

bool changed = false;

for (BasicBlock* const block : Blocks())
Expand Down Expand Up @@ -189,8 +192,7 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
UseExecutionOrder = true
};

ForwardSubVisitor(Compiler* compiler, unsigned lclNum, bool livenessBased)
: GenTreeVisitor(compiler), m_lclNum(lclNum), m_livenessBased(livenessBased)
ForwardSubVisitor(Compiler* compiler, unsigned lclNum) : GenTreeVisitor(compiler), m_lclNum(lclNum)
{
LclVarDsc* dsc = compiler->lvaGetDesc(m_lclNum);
if (dsc->lvIsStructField)
Expand Down Expand Up @@ -374,15 +376,6 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
{
assert(lcl->OperIs(GT_LCL_VAR) && (lcl->GetLclNum() == m_lclNum));

if (!m_livenessBased)
{
// When not liveness based we can only get here when we have
// exactly 2 global references, and we should have already seen the
// def.
assert(m_compiler->lvaGetDesc(lcl)->lvRefCnt(RCS_EARLY) == 2);
return true;
}

LclVarDsc* dsc = m_compiler->lvaGetDesc(lcl);
GenTreeFlags deathFlags = dsc->FullDeathFlags();
return (lcl->gtFlags & deathFlags) == deathFlags;
Expand All @@ -405,7 +398,6 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
ExceptionSetFlags m_accumulatedExceptions = ExceptionSetFlags::None;
ExceptionSetFlags m_useExceptions = ExceptionSetFlags::None;
unsigned m_treeSize = 0;
bool m_livenessBased;
};

//------------------------------------------------------------------------
Expand Down Expand Up @@ -495,25 +487,9 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
return false;
}

// Only fwd sub if we expect no code duplication
// Cannot forward sub without liveness information.
//
bool livenessBased = false;
if (varDsc->lvRefCnt(RCS_EARLY) != 2)
{
if (!fgDidEarlyLiveness)
{
JITDUMP(" not asg (single-use lcl)\n");
return false;
}

if (varDsc->lvRefCnt(RCS_EARLY) < 2)
{
JITDUMP(" not asg (no use)\n");
return false;
}

livenessBased = true;
}
assert(fgDidEarlyLiveness);

// And local is unalised
//
Expand Down Expand Up @@ -577,8 +553,7 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
//
Statement* const nextStmt = stmt->GetNextStmt();

ForwardSubVisitor fsv(this, lclNum, livenessBased);
assert(fgNodeThreading == NodeThreading::AllLocals);
ForwardSubVisitor fsv(this, lclNum);
// Do a quick scan through the linked locals list to see if there is a last
// use.
bool found = false;
Expand Down Expand Up @@ -629,14 +604,6 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
//
fsv.WalkTree(nextStmt->GetRootNodePointer(), nullptr);

if (!livenessBased)
{
// LclMorph (via RCS_Early) said there was just one use.
// It had better have gotten this right.
//
assert(fsv.GetUseCount() == 1);
}

// The visitor has more contextual information and may not actually deem
// the use we found above as a valid forward sub destination so we must
// recheck it here.
Expand Down
58 changes: 0 additions & 58 deletions src/coreclr/jit/promotiondecomposition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,6 @@ class DecompositionPlan
unsigned addrLcl = m_compiler->lvaGrabTemp(true DEBUGARG("Spilling address for field-by-field copy"));
statements->AddStatement(m_compiler->gtNewTempAssign(addrLcl, addr));
addr = m_compiler->gtNewLclvNode(addrLcl, addr->TypeGet());
UpdateEarlyRefCount(m_compiler, addr);
}
}

Expand All @@ -934,7 +933,6 @@ class DecompositionPlan
else
{
addrUse = m_compiler->gtCloneExpr(addr);
UpdateEarlyRefCount(m_compiler, addrUse);
}

if (offs != 0)
Expand Down Expand Up @@ -1001,9 +999,6 @@ class DecompositionPlan
if (entry.ToLclNum != BAD_VAR_NUM)
{
dst = m_compiler->gtNewLclvNode(entry.ToLclNum, entry.Type);

if (m_compiler->lvaGetDesc(entry.ToLclNum)->lvIsStructField)
UpdateEarlyRefCount(m_compiler, dst);
}
else if (m_dst->OperIs(GT_LCL_VAR, GT_LCL_FLD))
{
Expand All @@ -1013,7 +1008,6 @@ class DecompositionPlan
dst = m_compiler->gtNewLclFldNode(m_dst->AsLclVarCommon()->GetLclNum(), entry.Type, offs);
m_compiler->lvaSetVarDoNotEnregister(m_dst->AsLclVarCommon()->GetLclNum()
DEBUGARG(DoNotEnregisterReason::LocalField));
UpdateEarlyRefCount(m_compiler, dst);
}
else
{
Expand All @@ -1026,9 +1020,6 @@ class DecompositionPlan
if (entry.FromLclNum != BAD_VAR_NUM)
{
src = m_compiler->gtNewLclvNode(entry.FromLclNum, entry.Type);

if (m_compiler->lvaGetDesc(entry.FromLclNum)->lvIsStructField)
UpdateEarlyRefCount(m_compiler, src);
}
else if (m_src->OperIs(GT_LCL_VAR, GT_LCL_FLD))
{
Expand All @@ -1037,7 +1028,6 @@ class DecompositionPlan
src = m_compiler->gtNewLclFldNode(m_src->AsLclVarCommon()->GetLclNum(), entry.Type, offs);
m_compiler->lvaSetVarDoNotEnregister(m_src->AsLclVarCommon()->GetLclNum()
DEBUGARG(DoNotEnregisterReason::LocalField));
UpdateEarlyRefCount(m_compiler, src);
}
else
{
Expand Down Expand Up @@ -1131,54 +1121,6 @@ class DecompositionPlan

indir->gtFlags |= flags;
}

//------------------------------------------------------------------------
// UpdateEarlyRefCount:
// Update early ref counts if necessary for the specified IR node.
//
// Parameters:
// comp - compiler instance
// candidate - the IR node that may be a local that should have its early
// ref counts updated.
//
static void UpdateEarlyRefCount(Compiler* comp, GenTree* candidate)
{
if (!candidate->OperIs(GT_LCL_VAR, GT_LCL_FLD, GT_LCL_ADDR))
{
return;
}

IncrementRefCount(comp, candidate->AsLclVarCommon()->GetLclNum());

LclVarDsc* varDsc = comp->lvaGetDesc(candidate->AsLclVarCommon());
if (varDsc->lvIsStructField)
{
IncrementRefCount(comp, varDsc->lvParentLcl);
}

if (varDsc->lvPromoted)
{
for (unsigned fldLclNum = varDsc->lvFieldLclStart; fldLclNum < varDsc->lvFieldLclStart + varDsc->lvFieldCnt;
fldLclNum++)
{
IncrementRefCount(comp, fldLclNum);
}
}
}

//------------------------------------------------------------------------
// IncrementRefCount:
// Increment the ref count for the specified local.
//
// Parameters:
// comp - compiler instance
// lclNum - the local
//
static void IncrementRefCount(Compiler* comp, unsigned lclNum)
{
LclVarDsc* varDsc = comp->lvaGetDesc(lclNum);
varDsc->incLvRefCntSaturating(1, RCS_EARLY);
}
};

//------------------------------------------------------------------------
Expand Down

0 comments on commit 547c506

Please sign in to comment.