Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@kunalspathak
kunalspathak committed Jul 8, 2021
1 parent ca42b6e commit 98203d3
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 16 deletions.
8 changes: 4 additions & 4 deletions src/coreclr/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ void CodeGenInterface::genUpdateRegLife(const LclVarDsc* varDsc, bool isBorn, bo
// If this is going live, the register must not have a variable in it, except
// in the case of an exception or "spill at single-def" variable, which may be already treated
// as live in the register.
assert(varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef || ((regSet.GetMaskVars() & regMask) == 0));
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 || varDsc->lvSpillAtSingleDef;
bool isInMemory = !isInReg || varDsc->IsAlwaysAliveInMemory();

if (isInReg)
{
Expand Down Expand Up @@ -778,7 +778,7 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
{
// If this variable is going live in a register, it is no longer live on the stack,
// unless it is an EH/"spill at single-def" var, which always remains live on the stack.
if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (VarSetOps::IsMember(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex))
Expand Down Expand Up @@ -11428,7 +11428,7 @@ void CodeGen::genMultiRegStoreToLocal(GenTreeLclVar* lclNode)
{
varReg = REG_STK;
}
if ((varReg == REG_STK) || fieldVarDsc->lvLiveInOutOfHndlr || fieldVarDsc->lvSpillAtSingleDef)
if ((varReg == REG_STK) || fieldVarDsc->IsAlwaysAliveInMemory())
{
if (!lclNode->AsLclVar()->IsLastUse(i))
{
Expand Down
13 changes: 6 additions & 7 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 && !varDsc->lvSpillAtSingleDef)
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (verbose && VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
Expand All @@ -240,8 +240,7 @@ void CodeGen::genCodeForBBlist()
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex);
}
}
if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) &&
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 @@ -876,7 +875,7 @@ void CodeGen::genSpillVar(GenTree* tree)

// 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->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)
if (!varDsc->IsAlwaysAliveInMemory())
{
instruction storeIns = ins_Store(lclType, compiler->isSIMDTypeLocalAligned(varNum));
assert(varDsc->GetRegNum() == tree->GetRegNum());
Expand Down Expand Up @@ -921,7 +920,7 @@ void CodeGen::genSpillVar(GenTree* tree)
{
// 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->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) && ((tree->gtFlags & GTF_VAR_DEF) != 0));
assert((varDsc->IsAlwaysAliveInMemory()) && ((tree->gtFlags & GTF_VAR_DEF) != 0));
}

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

if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)
if (!varDsc->IsAlwaysAliveInMemory())
{
#ifdef DEBUG
if (VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex))
Expand Down Expand Up @@ -2057,7 +2056,7 @@ void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclN
// 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 && !varDsc->lvSpillAtSingleDef))
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
10 changes: 10 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,16 @@ class LclVarDsc
return GetRegisterType() != TYP_UNDEF;
}

//-----------------------------------------------------------------------------
// 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
3 changes: 1 addition & 2 deletions src/coreclr/jit/lsra.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3274,8 +3274,7 @@ void LinearScan::spillInterval(Interval* interval, RefPosition* fromRefPosition
}
}

// Only handle the singledef intervals whose firstRefPosition is RefTypeDef and is not already marked as spillAfter
// yet.
// Only handle the singledef intervals whose firstRefPosition is RefTypeDef and is not yet marked as spillAfter.
// The singledef intervals whose firstRefPositions are already marked as spillAfter, no need to mark them as
// singleDefSpill because they will always get spilled at firstRefPosition.
// This helps in spilling the singleDef at definition
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/treelifeupdater.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ bool TreeLifeUpdater<ForCodeGen>::UpdateLifeFieldVar(GenTreeLclVar* lclNode, uns
{
regNumber reg = lclNode->GetRegNumByIdx(multiRegIndex);
bool isInReg = fldVarDsc->lvIsInReg() && reg != REG_NA;
isInMemory = !isInReg || fldVarDsc->lvLiveInOutOfHndlr || fldVarDsc->lvSpillAtSingleDef;
isInMemory = !isInReg || fldVarDsc->IsAlwaysAliveInMemory();
if (isInReg)
{
if (isBorn)
Expand Down Expand Up @@ -259,7 +259,7 @@ void TreeLifeUpdater<ForCodeGen>::UpdateLifeVar(GenTree* tree)
compiler->codeGen->genUpdateVarReg(varDsc, tree);
}
bool isInReg = varDsc->lvIsInReg() && tree->GetRegNum() != REG_NA;
bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef;
bool isInMemory = !isInReg || varDsc->IsAlwaysAliveInMemory();
if (isInReg)
{
compiler->codeGen->genUpdateRegLife(varDsc, isBorn, isDying DEBUGARG(tree));
Expand All @@ -283,7 +283,7 @@ void TreeLifeUpdater<ForCodeGen>::UpdateLifeVar(GenTree* tree)
unsigned fldVarIndex = fldVarDsc->lvVarIndex;
regNumber reg = lclVarTree->AsLclVar()->GetRegNumByIdx(i);
bool isInReg = fldVarDsc->lvIsInReg() && reg != REG_NA;
bool isInMemory = !isInReg || fldVarDsc->lvLiveInOutOfHndlr || fldVarDsc->lvSpillAtSingleDef;
bool isInMemory = !isInReg || fldVarDsc->IsAlwaysAliveInMemory();
bool isFieldDying = lclVarTree->AsLclVar()->IsLastUse(i);
if ((isBorn && !isFieldDying) || (!isBorn && isFieldDying))
{
Expand Down

0 comments on commit 98203d3

Please sign in to comment.