Skip to content

Commit

Permalink
Spill single-def variable at definition to avoid further spilling (#5…
Browse files Browse the repository at this point in the history 
…4345)

* Print single-def

* Rename lvEhWriteThruCandidate->lvSingleDefRegCandidate, introduce isSingleDef

* Introduce singleDefSpillAfter

If a single-def variable is decided to get spilled in its lifetime, then
spill it at the firstRefPosition RefTypeDef so the value of the variable
is always valid on the stack. Going forward, no more spills will be needed
for such variable or no more resolutions (reg to stack) will be needed for
such single-def variables.

* jit format

* some fixes

* wip

* Add check of isSingleDef in validateInterval()

* Make isSingleDef during buildIntervals

* minor fix in lclvars.cpp

* some fixes after self CR

* Updated some comments

* Remove lvSpillAtSingleDef from some asserts

* Use singleDefSpill information in getWeight()

* Remove lvSpillAtSingleDef from some more checks

* Mark lvSpillAtSingleDef whenever refPosition->singleDefSpill==true

* Add TODO for SingleDefVarCandidate

* Some notes on setting singleDefSpill

* jit format

* review feedback

* review comments
  • Loading branch information
@kunalspathak
kunalspathak authored Jul 10, 2021
1 parent 413f0de commit 27b564a
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 81 deletions.
14 changes: 7 additions & 7 deletions src/coreclr/jit/codegencommon.cpp
View file
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
View file
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
View file
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
// 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
View file
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)
{
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

0 comments on commit 27b564a

Please sign in to comment.