Skip to content

Commit

Permalink
Support Write-Thru of EH variables in LSRA (#543)
Browse files Browse the repository at this point in the history
* Support Write-Thru of EH variables in LSRA

Mark EH variables (those that are live in or out of exception regions) only as lvLiveInOutOfHndlr, not necessarily lvDoNotEnregister
During register allocation, mark these as write-thru, and mark all defs as write-thru, ensuring that the stack value is always valid.
Mark those defs with GTF_SPILLED (this the "reload" flag and is not currently used for pure defs) to indicate that it should be kept in the register.
Mark blocks that enter EH regions as having no predecessor, and set the location of all live-in vars to be on the stack.
Change genFnPrologCalleeRegArgs to store EH vars also to the stack if they have a register assignment.

Tuned throughput to compensate for extra processing by rearranging some fields and short-circuiting the physical register RefPositions during allocation.

It is disabled by default
  • Loading branch information
CarolEidt authored Feb 19, 2020
1 parent 40a5cff commit 3be5238
Show file tree
Hide file tree
Showing 12 changed files with 1,010 additions and 462 deletions.
72 changes: 45 additions & 27 deletions src/coreclr/src/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,10 @@ void CodeGenInterface::genUpdateRegLife(const LclVarDsc* varDsc, bool isBorn, bo
}
else
{
assert((regSet.GetMaskVars() & regMask) == 0);
// 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));
regSet.AddMaskVars(regMask);
}
}
Expand Down Expand Up @@ -681,12 +684,14 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
unsigned deadVarIndex = 0;
while (deadIter.NextElem(&deadVarIndex))
{
unsigned varNum = lvaTrackedIndexToLclNum(deadVarIndex);
LclVarDsc* varDsc = lvaGetDesc(varNum);
bool isGCRef = (varDsc->TypeGet() == TYP_REF);
bool isByRef = (varDsc->TypeGet() == TYP_BYREF);
unsigned varNum = lvaTrackedIndexToLclNum(deadVarIndex);
LclVarDsc* varDsc = lvaGetDesc(varNum);
bool isGCRef = (varDsc->TypeGet() == TYP_REF);
bool isByRef = (varDsc->TypeGet() == TYP_BYREF);
bool isInReg = varDsc->lvIsInReg();
bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr;

if (varDsc->lvIsInReg())
if (isInReg)
{
// TODO-Cleanup: Move the code from compUpdateLifeVar to genUpdateRegLife that updates the
// gc sets
Expand All @@ -701,8 +706,8 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
}
codeGen->genUpdateRegLife(varDsc, false /*isBorn*/, true /*isDying*/ DEBUGARG(nullptr));
}
// This isn't in a register, so update the gcVarPtrSetCur.
else if (isGCRef || isByRef)
// Update the gcVarPtrSetCur if it is in memory.
if (isInMemory && (isGCRef || isByRef))
{
VarSetOps::RemoveElemD(this, codeGen->gcInfo.gcVarPtrSetCur, deadVarIndex);
JITDUMP("\t\t\t\t\t\t\tV%02u becoming dead\n", varNum);
Expand All @@ -724,13 +729,18 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)

if (varDsc->lvIsInReg())
{
#ifdef DEBUG
if (VarSetOps::IsMember(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex))
// 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)
{
JITDUMP("\t\t\t\t\t\t\tRemoving V%02u from gcVarPtrSetCur\n", varNum);
}
#ifdef DEBUG
if (VarSetOps::IsMember(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex))
{
JITDUMP("\t\t\t\t\t\t\tRemoving V%02u from gcVarPtrSetCur\n", varNum);
}
#endif // DEBUG
VarSetOps::RemoveElemD(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex);
VarSetOps::RemoveElemD(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex);
}
codeGen->genUpdateRegLife(varDsc, true /*isBorn*/, false /*isDying*/ DEBUGARG(nullptr));
regMaskTP regMask = varDsc->lvRegMask();
if (isGCRef)
Expand All @@ -742,9 +752,9 @@ void Compiler::compChangeLife(VARSET_VALARG_TP newLife)
codeGen->gcInfo.gcRegByrefSetCur |= regMask;
}
}
// This isn't in a register, so update the gcVarPtrSetCur
else if (lvaIsGCTracked(varDsc))
{
// This isn't in a register, so update the gcVarPtrSetCur to show that it's live on the stack.
VarSetOps::AddElemD(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex);
JITDUMP("\t\t\t\t\t\t\tV%02u becoming live\n", varNum);
}
Expand Down Expand Up @@ -3269,6 +3279,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
// 1 means the first part of a register argument
// 2, 3 or 4 means the second,third or fourth part of a multireg argument
bool stackArg; // true if the argument gets homed to the stack
bool writeThru; // true if the argument gets homed to both stack and register
bool processed; // true after we've processed the argument (and it is in its final location)
bool circular; // true if this register participates in a circular dependency loop.

Expand Down Expand Up @@ -3605,6 +3616,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
}

regArgTab[regArgNum + i].processed = false;
regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && varDsc->lvLiveInOutOfHndlr);

/* mark stack arguments since we will take care of those first */
regArgTab[regArgNum + i].stackArg = (varDsc->lvIsInReg()) ? false : true;
Expand Down Expand Up @@ -3765,9 +3777,9 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
noway_assert(((regArgMaskLive & RBM_FLTARG_REGS) == 0) &&
"Homing of float argument registers with circular dependencies not implemented.");

/* Now move the arguments to their locations.
* First consider ones that go on the stack since they may
* free some registers. */
// Now move the arguments to their locations.
// First consider ones that go on the stack since they may free some registers.
// Also home writeThru args, since they're also homed to the stack.

regArgMaskLive = regState->rsCalleeRegArgMaskLiveIn; // reset the live in to what it was at the start
for (argNum = 0; argNum < argMax; argNum++)
Expand Down Expand Up @@ -3805,7 +3817,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
// If this arg is never on the stack, go to the next one.
if (varDsc->lvType == TYP_LONG)
{
if (regArgTab[argNum].slot == 1 && !regArgTab[argNum].stackArg)
if (regArgTab[argNum].slot == 1 && !regArgTab[argNum].stackArg && !regArgTab[argNum].writeThru)
{
continue;
}
Expand Down Expand Up @@ -3839,7 +3851,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere

noway_assert(varDsc->lvIsParam);
noway_assert(varDsc->lvIsRegArg);
noway_assert(varDsc->lvIsInReg() == false ||
noway_assert(varDsc->lvIsInReg() == false || varDsc->lvLiveInOutOfHndlr ||
(varDsc->lvType == TYP_LONG && varDsc->GetOtherReg() == REG_STK && regArgTab[argNum].slot == 2));

var_types storeType = TYP_UNDEF;
Expand Down Expand Up @@ -3906,13 +3918,17 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
#endif // USING_SCOPE_INFO
}

/* mark the argument as processed */

regArgTab[argNum].processed = true;
regArgMaskLive &= ~genRegMask(srcRegNum);
// Mark the argument as processed, and set it as no longer live in srcRegNum,
// unless it is a writeThru var, in which case we home it to the stack, but
// don't mark it as processed until below.
if (!regArgTab[argNum].writeThru)
{
regArgTab[argNum].processed = true;
regArgMaskLive &= ~genRegMask(srcRegNum);
}

#if defined(TARGET_ARM)
if (storeType == TYP_DOUBLE)
if ((storeType == TYP_DOUBLE) && !regArgTab[argNum].writeThru)
{
regArgTab[argNum + 1].processed = true;
regArgMaskLive &= ~genRegMask(REG_NEXT(srcRegNum));
Expand Down Expand Up @@ -4618,7 +4634,7 @@ void CodeGen::genCheckUseBlockInit()
{
if (!varDsc->lvRegister)
{
if (!varDsc->lvIsInReg())
if (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr)
{
// Var is on the stack at entry.
initStkLclCnt +=
Expand Down Expand Up @@ -7233,7 +7249,9 @@ void CodeGen::genFnProlog()
continue;
}

if (varDsc->lvIsInReg())
bool isInReg = varDsc->lvIsInReg();
bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr;
if (isInReg)
{
regMaskTP regMask = genRegMask(varDsc->GetRegNum());
if (!varDsc->IsFloatRegType())
Expand Down Expand Up @@ -7264,7 +7282,7 @@ void CodeGen::genFnProlog()
initFltRegs |= regMask;
}
}
else
if (isInMemory)
{
INIT_STK:

Expand Down
94 changes: 65 additions & 29 deletions src/coreclr/src/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,18 @@ void CodeGen::genCodeForBBlist()
{
newRegByrefSet |= varDsc->lvRegMask();
}
#ifdef DEBUG
if (verbose && VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
if (!varDsc->lvLiveInOutOfHndlr)
{
VarSetOps::AddElemD(compiler, removedGCVars, varIndex);
}
#ifdef DEBUG
if (verbose && VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
{
VarSetOps::AddElemD(compiler, removedGCVars, varIndex);
}
#endif // DEBUG
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex);
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex);
}
}
else if (compiler->lvaIsGCTracked(varDsc))
if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr) && compiler->lvaIsGCTracked(varDsc))
{
#ifdef DEBUG
if (verbose && !VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex))
Expand Down Expand Up @@ -823,10 +826,20 @@ void CodeGen::genSpillVar(GenTree* tree)
var_types lclTyp = genActualType(varDsc->TypeGet());
emitAttr size = emitTypeSize(lclTyp);

instruction storeIns = ins_Store(lclTyp, compiler->isSIMDTypeLocalAligned(varNum));
assert(varDsc->GetRegNum() == tree->GetRegNum());
inst_TT_RV(storeIns, size, tree, tree->GetRegNum());
// 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)
{
instruction storeIns = ins_Store(lclTyp, 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.
assert((tree->gtFlags & GTF_SPILLED) == 0);

// Remove the live var from the register.
genUpdateRegLife(varDsc, /*isBorn*/ false, /*isDying*/ true DEBUGARG(tree));
gcInfo.gcMarkRegSetNpt(varDsc->lvRegMask());

Expand All @@ -847,10 +860,19 @@ void CodeGen::genSpillVar(GenTree* tree)
}

tree->gtFlags &= ~GTF_SPILL;
varDsc->SetRegNum(REG_STK);
if (varTypeIsMultiReg(tree))
// If this is NOT a write-thru, reset the var location.
if ((tree->gtFlags & GTF_SPILLED) == 0)
{
varDsc->SetOtherReg(REG_STK);
varDsc->SetRegNum(REG_STK);
if (varTypeIsMultiReg(tree))
{
varDsc->SetOtherReg(REG_STK);
}
}
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));
}

#ifdef USING_VARIABLE_LIVE_RANGE
Expand Down Expand Up @@ -1030,13 +1052,16 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree)
}
#endif // USING_VARIABLE_LIVE_RANGE

#ifdef DEBUG
if (VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex))
if (!varDsc->lvLiveInOutOfHndlr)
{
JITDUMP("\t\t\t\t\t\t\tRemoving V%02u from gcVarPtrSetCur\n", lcl->GetLclNum());
}
#ifdef DEBUG
if (VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex))
{
JITDUMP("\t\t\t\t\t\t\tRemoving V%02u from gcVarPtrSetCur\n", lcl->GetLclNum());
}
#endif // DEBUG
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex);
VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex);
}

#ifdef DEBUG
if (compiler->verbose)
Expand Down Expand Up @@ -1316,15 +1341,15 @@ regNumber CodeGen::genConsumeReg(GenTree* tree)
LclVarDsc* varDsc = &compiler->lvaTable[lcl->GetLclNum()];
assert(varDsc->lvLRACandidate);

if ((tree->gtFlags & GTF_VAR_DEATH) != 0)
{
gcInfo.gcMarkRegSetNpt(genRegMask(varDsc->GetRegNum()));
}
else if (varDsc->GetRegNum() == REG_STK)
if (varDsc->GetRegNum() == REG_STK)
{
// We have loaded this into a register only temporarily
gcInfo.gcMarkRegSetNpt(genRegMask(tree->GetRegNum()));
}
else if ((tree->gtFlags & GTF_VAR_DEATH) != 0)
{
gcInfo.gcMarkRegSetNpt(genRegMask(varDsc->GetRegNum()));
}
}
else
{
Expand Down Expand Up @@ -1852,13 +1877,24 @@ void CodeGen::genProduceReg(GenTree* tree)

if (genIsRegCandidateLocal(tree))
{
// Store local variable to its home location.
// Ensure that lclVar stores are typed correctly.
unsigned varNum = tree->AsLclVarCommon()->GetLclNum();
assert(!compiler->lvaTable[varNum].lvNormalizeOnStore() ||
(tree->TypeGet() == genActualType(compiler->lvaTable[varNum].TypeGet())));
inst_TT_RV(ins_Store(tree->gtType, compiler->isSIMDTypeLocalAligned(varNum)), emitTypeSize(tree->TypeGet()),
tree, tree->GetRegNum());
unsigned varNum = tree->AsLclVarCommon()->GetLclNum();
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
assert(!varDsc->lvNormalizeOnStore() || (tree->TypeGet() == genActualType(varDsc->TypeGet())));

// If we reach here, 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"
// 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),
// but the GTF_SPILL flag records the fact that the register value is going dead.
if (((tree->gtFlags & GTF_VAR_DEF) != 0) || !varDsc->lvLiveInOutOfHndlr)
{
// Store local variable to its home location.
// Ensure that lclVar stores are typed correctly.
inst_TT_RV(ins_Store(tree->gtType, compiler->isSIMDTypeLocalAligned(varNum)),
emitTypeSize(tree->TypeGet()), tree, tree->GetRegNum());
}
}
else
{
Expand Down
31 changes: 31 additions & 0 deletions src/coreclr/src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4522,6 +4522,37 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags

EndPhase(PHASE_CLONE_FINALLY);

#if DEBUG
if (lvaEnregEHVars)
{
unsigned methHash = info.compMethodHash();
char* lostr = getenv("JitEHWTHashLo");
unsigned methHashLo = 0;
bool dump = false;
if (lostr != nullptr)
{
sscanf_s(lostr, "%x", &methHashLo);
dump = true;
}
char* histr = getenv("JitEHWTHashHi");
unsigned methHashHi = UINT32_MAX;
if (histr != nullptr)
{
sscanf_s(histr, "%x", &methHashHi);
dump = true;
}
if (methHash < methHashLo || methHash > methHashHi)
{
lvaEnregEHVars = false;
}
else if (dump)
{
printf("Enregistering EH Vars for method %s, hash = 0x%x.\n", info.compFullName, info.compMethodHash());
printf(""); // flush
}
}
#endif

// Compute bbNum, bbRefs and bbPreds
//
JITDUMP("\nRenumbering the basic blocks for fgComputePreds\n");
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,8 @@ class LclVarDsc
unsigned char lvDoNotEnregister : 1; // Do not enregister this variable.
unsigned char lvFieldAccessed : 1; // The var is a struct local, and a field of the variable is accessed. Affects
// struct promotion.
unsigned char lvLiveInOutOfHndlr : 1; // The variable is live in or out of an exception handler, and therefore must
// be on the stack (at least at those boundaries.)

unsigned char lvInSsa : 1; // The variable is in SSA form (set by SsaBuilder)

Expand All @@ -424,9 +426,6 @@ class LclVarDsc
// also, lvType == TYP_STRUCT prevents enregistration. At least one of the reasons should be true.
unsigned char lvVMNeedsStackAddr : 1; // The VM may have access to a stack-relative address of the variable, and
// read/write its value.
unsigned char lvLiveInOutOfHndlr : 1; // The variable was live in or out of an exception handler, and this required
// the variable to be
// in the stack (at least at those boundaries.)
unsigned char lvLclFieldExpr : 1; // The variable is not a struct, but was accessed like one (e.g., reading a
// particular byte from an int).
unsigned char lvLclBlockOpAddr : 1; // The variable was written to via a block operation that took its address.
Expand Down Expand Up @@ -3005,6 +3004,9 @@ class Compiler
void lvaSetVarAddrExposed(unsigned varNum);
void lvaSetVarLiveInOutOfHandler(unsigned varNum);
bool lvaVarDoNotEnregister(unsigned varNum);

bool lvaEnregEHVars;

#ifdef DEBUG
// Reasons why we can't enregister. Some of these correspond to debug properties of local vars.
enum DoNotEnregisterReason
Expand All @@ -3027,6 +3029,7 @@ class Compiler
DNER_PinningRef,
#endif
};

#endif
void lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregisterReason reason));

Expand Down
Loading

0 comments on commit 3be5238

Please sign in to comment.