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

Spill single-def variable at definition to avoid further spilling #54345

Merged
merged 20 commits into from
Jul 10, 2021
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
14 changes: 7 additions & 7 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,9 @@ void CodeGenInterface::genUpdateRegLife(const LclVarDsc* varDsc, bool isBorn, bo
else
{
// If this is going live, the register must not have a variable in it, except
// in the case of an exception variable, which may be already treated as live
// in the register.
assert(varDsc->lvLiveInOutOfHndlr || ((regSet.GetMaskVars() & regMask) == 0));
// in the case of an exception or "spill at single-def" variable, which may be already treated
// as live in the register.
assert(varDsc->IsAlwaysAliveInMemory() || ((regSet.GetMaskVars() & regMask) == 0));
regSet.AddMaskVars(regMask);
}
}
Expand Down Expand Up @@ -736,7 +736,7 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
bool isGCRef = (varDsc->TypeGet() == TYP_REF);
bool isByRef = (varDsc->TypeGet() == TYP_BYREF);
bool isInReg = varDsc->lvIsInReg();
bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr;
bool isInMemory = !isInReg || varDsc->IsAlwaysAliveInMemory();

if (isInReg)
{
Expand Down Expand Up @@ -777,8 +777,8 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
if (varDsc->lvIsInReg())
{
// If this variable is going live in a register, it is no longer live on the stack,
// unless it is an EH var, which always remains live on the stack.
if (!varDsc->lvLiveInOutOfHndlr)
// unless it is an EH/"spill at single-def" var, which always remains live on the stack.
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (VarSetOps::IsMember(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex))
Expand Down Expand Up @@ -11422,7 +11422,7 @@ void CodeGen::genMultiRegStoreToLocal(GenTreeLclVar* lclNode)
{
varReg = REG_STK;
}
if ((varReg == REG_STK) || fieldVarDsc->lvLiveInOutOfHndlr)
if ((varReg == REG_STK) || fieldVarDsc->IsAlwaysAliveInMemory())
{
if (!lclNode->AsLclVar()->IsLastUse(i))
{
Expand Down
27 changes: 14 additions & 13 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ void CodeGen::genCodeForBBlist()
{
newRegByrefSet |= varDsc->lvRegMask();
}
if (!varDsc->lvLiveInOutOfHndlr)
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (verbose && VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
Expand All @@ -240,7 +240,7 @@ void CodeGen::genCodeForBBlist()
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex);
}
}
if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr) && compiler->lvaIsGCTracked(varDsc))
if ((!varDsc->lvIsInReg() || varDsc->IsAlwaysAliveInMemory()) && compiler->lvaIsGCTracked(varDsc))
{
#ifdef DEBUG
if (verbose && !VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
Expand Down Expand Up @@ -873,17 +873,17 @@ void CodeGen::genSpillVar(GenTree* tree)
var_types lclType = varDsc->GetActualRegisterType();
emitAttr size = emitTypeSize(lclType);

// If this is a write-thru variable, we don't actually spill at a use, but we will kill the var in the reg
// (below).
if (!varDsc->lvLiveInOutOfHndlr)
// If this is a write-thru or a single-def variable, we don't actually spill at a use,
// but we will kill the var in the reg (below).
if (!varDsc->IsAlwaysAliveInMemory())
{
instruction storeIns = ins_Store(lclType, compiler->isSIMDTypeLocalAligned(varNum));
assert(varDsc->GetRegNum() == tree->GetRegNum());
inst_TT_RV(storeIns, size, tree, tree->GetRegNum());
}

// We should only have both GTF_SPILL (i.e. the flag causing this method to be called) and
// GTF_SPILLED on a write-thru def, for which we should not be calling this method.
// GTF_SPILLED on a write-thru/single-def def, for which we should not be calling this method.
assert((tree->gtFlags & GTF_SPILLED) == 0);

// Remove the live var from the register.
Expand Down Expand Up @@ -918,8 +918,9 @@ void CodeGen::genSpillVar(GenTree* tree)
}
else
{
// We only have 'GTF_SPILL' and 'GTF_SPILLED' on a def of a write-thru lclVar.
assert(varDsc->lvLiveInOutOfHndlr && ((tree->gtFlags & GTF_VAR_DEF) != 0));
// We only have 'GTF_SPILL' and 'GTF_SPILLED' on a def of a write-thru lclVar
// or a single-def var that is to be spilled at its definition.
assert((varDsc->IsAlwaysAliveInMemory()) && ((tree->gtFlags & GTF_VAR_DEF) != 0));
}

#ifdef USING_VARIABLE_LIVE_RANGE
Expand Down Expand Up @@ -1055,7 +1056,7 @@ void CodeGen::genUnspillLocal(
}
#endif // USING_VARIABLE_LIVE_RANGE

if (!varDsc->lvLiveInOutOfHndlr)
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex))
Expand Down Expand Up @@ -2044,12 +2045,12 @@ void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclN

// We have a register candidate local that is marked with GTF_SPILL.
// This flag generally means that we need to spill this local.
// The exception is the case of a use of an EH var use that is being "spilled"
// The exception is the case of a use of an EH/spill-at-single-def var use that is being "spilled"
// to the stack, indicated by GTF_SPILL (note that all EH lclVar defs are always
// spilled, i.e. write-thru).
// An EH var use is always valid on the stack (so we don't need to actually spill it),
// spilled, i.e. write-thru. Likewise, single-def vars that are spilled at its definitions).
// An EH or single-def var use is always valid on the stack (so we don't need to actually spill it),
// but the GTF_SPILL flag records the fact that the register value is going dead.
if (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || !varDsc->lvLiveInOutOfHndlr)
if (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->IsAlwaysAliveInMemory()))
{
// Store local variable to its home location.
// Ensure that lclVar stores are typed correctly.
Expand Down
27 changes: 23 additions & 4 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,19 @@ class LclVarDsc
// before lvaMarkLocalVars: identifies ref type locals that can get type updates
// after lvaMarkLocalVars: identifies locals that are suitable for optAddCopies

unsigned char lvEhWriteThruCandidate : 1; // variable has a single def and hence is a register candidate if
// if it is an EH variable
unsigned char lvSingleDefRegCandidate : 1; // variable has a single def and hence is a register candidate
// Currently, this is only used to decide if an EH variable can be
// a register candiate or not.

unsigned char lvDisqualifyForEhWriteThru : 1; // tracks variable that are disqualified from register candidancy
unsigned char lvDisqualifySingleDefRegCandidate : 1; // tracks variable that are disqualified from register
// candidancy

unsigned char lvSpillAtSingleDef : 1; // variable has a single def (as determined by LSRA interval scan)
// and is spilled making it candidate to spill right after the
// first (and only) definition.
// Note: We cannot reuse lvSingleDefRegCandidate because it is set
Copy link
Contributor

Choose a reason for hiding this comment

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

does it mean that lvSingleDefRegCandidate==true does not imply that the variable has a single def for lvSpillAtSingleDef purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's correct. lvSingleDefRegCandidate is only used so we can decide if we should enregister EH-var or not (which happens in earlier phase). Things change after that and we can't rely on that information. The best safe place I thought about checking for single-def is while building intervals. I wanted to do it for EH-var as well, but unfortunately we need that information before LSRA.

// in earlier phase and the information might not be appropriate
// in LSRA.

#if ASSERTION_PROP
unsigned char lvDisqualify : 1; // variable is no longer OK for add copy optimization
Expand Down Expand Up @@ -547,7 +556,7 @@ class LclVarDsc
unsigned char lvFldOrdinal;

#ifdef DEBUG
unsigned char lvDisqualifyEHVarReason = 'H';
unsigned char lvSingleDefDisqualifyReason = 'H';
#endif

#if FEATURE_MULTIREG_ARGS
Expand Down Expand Up @@ -1030,6 +1039,16 @@ class LclVarDsc
return IsEnregisterableType();
}

//-----------------------------------------------------------------------------
// IsAlwaysAliveInMemory: Determines if this variable's value is always
// up-to-date on stack. This is possible if this is an EH-var or
// we decided to spill after single-def.
//
bool IsAlwaysAliveInMemory() const
{
return lvLiveInOutOfHndlr || lvSpillAtSingleDef;
}

bool CanBeReplacedWithItsField(Compiler* comp) const;

#ifdef DEBUG
Expand Down
48 changes: 27 additions & 21 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2559,15 +2559,15 @@ void Compiler::lvaSetVarLiveInOutOfHandler(unsigned varNum)
noway_assert(lvaTable[i].lvIsStructField);
lvaTable[i].lvLiveInOutOfHndlr = 1;
// For now, only enregister an EH Var if it is a single def and whose refCnt > 1.
if (!lvaEnregEHVars || !lvaTable[i].lvEhWriteThruCandidate || lvaTable[i].lvRefCnt() <= 1)
if (!lvaEnregEHVars || !lvaTable[i].lvSingleDefRegCandidate || lvaTable[i].lvRefCnt() <= 1)
{
lvaSetVarDoNotEnregister(i DEBUGARG(DNER_LiveInOutOfHandler));
}
}
}

// For now, only enregister an EH Var if it is a single def and whose refCnt > 1.
if (!lvaEnregEHVars || !varDsc->lvEhWriteThruCandidate || varDsc->lvRefCnt() <= 1)
if (!lvaEnregEHVars || !varDsc->lvSingleDefRegCandidate || varDsc->lvRefCnt() <= 1)
{
lvaSetVarDoNotEnregister(varNum DEBUGARG(DNER_LiveInOutOfHandler));
}
Expand Down Expand Up @@ -4110,33 +4110,35 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt,
}
}

if (!varDsc->lvDisqualifyForEhWriteThru) // If this EH var already disqualified, we can skip this
if (!varDsc->lvDisqualifySingleDefRegCandidate) // If this var is already disqualified, we can skip this
{
if (tree->gtFlags & GTF_VAR_DEF) // Is this is a def of our variable
{
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;
// TODO: Zero-inits in LSRA are created with below condition. Try to use similar condition here as well.
// if (compiler->info.compInitMem || varTypeIsGC(varDsc->TypeGet()))
bool needsExplicitZeroInit = fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn);

if (varDsc->lvEhWriteThruCandidate || needsExplicitZeroInit)
if (varDsc->lvSingleDefRegCandidate || needsExplicitZeroInit)
{
#ifdef DEBUG
if (needsExplicitZeroInit)
{
varDsc->lvDisqualifyEHVarReason = 'Z';
JITDUMP("EH Var V%02u needs explicit zero init. Disqualified as a register candidate.\n",
varDsc->lvSingleDefDisqualifyReason = 'Z';
JITDUMP("V%02u needs explicit zero init. Disqualified as a single-def register candidate.\n",
lclNum);
}
else
{
varDsc->lvDisqualifyEHVarReason = 'M';
JITDUMP("EH Var V%02u has multiple definitions. Disqualified as a register candidate.\n",
varDsc->lvSingleDefDisqualifyReason = 'M';
JITDUMP("V%02u has multiple definitions. Disqualified as a single-def register candidate.\n",
lclNum);
}

#endif // DEBUG
varDsc->lvEhWriteThruCandidate = false;
varDsc->lvDisqualifyForEhWriteThru = true;
varDsc->lvSingleDefRegCandidate = false;
varDsc->lvDisqualifySingleDefRegCandidate = true;
}
else
{
Expand All @@ -4146,7 +4148,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt,
if (!varTypeNeedsPartialCalleeSave(varDsc->lvType))
#endif
{
varDsc->lvEhWriteThruCandidate = true;
varDsc->lvSingleDefRegCandidate = true;
JITDUMP("Marking EH Var V%02u as a register candidate.\n", lclNum);
}
}
Expand Down Expand Up @@ -4521,8 +4523,8 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers)
// that was set by past phases.
if (!isRecompute)
{
varDsc->lvSingleDef = varDsc->lvIsParam;
varDsc->lvEhWriteThruCandidate = varDsc->lvIsParam;
varDsc->lvSingleDef = varDsc->lvIsParam;
varDsc->lvSingleDefRegCandidate = varDsc->lvIsParam;
}
}

Expand Down Expand Up @@ -7405,11 +7407,6 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r
printf(" HFA(%s) ", varTypeName(varDsc->GetHfaType()));
}

if (varDsc->lvLiveInOutOfHndlr)
{
printf(" EH");
}

if (varDsc->lvDoNotEnregister)
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
{
printf(" do-not-enreg[");
Expand All @@ -7427,7 +7424,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r
}
if (lvaEnregEHVars && varDsc->lvLiveInOutOfHndlr)
{
printf("%c", varDsc->lvDisqualifyEHVarReason);
printf("%c", varDsc->lvSingleDefDisqualifyReason);
}
if (varDsc->lvLclFieldExpr)
{
Expand Down Expand Up @@ -7500,6 +7497,15 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r
{
printf(" EH-live");
}
if (varDsc->lvSpillAtSingleDef)
{
printf(" spill-single-def");
}
else if (varDsc->lvSingleDefRegCandidate)
{
printf(" single-def");
}

#ifndef TARGET_64BIT
if (varDsc->lvStructDoubleAlign)
printf(" double-align");
Expand Down
Loading