From d5dc1080cd9fe795002e1a7db0a87d0c522a77d7 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 15 Jun 2021 19:26:48 -0700 Subject: [PATCH 01/20] Print single-def --- src/coreclr/jit/lclvars.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index b2ca56b406ba4b..f02cdbef112bd2 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -7405,11 +7405,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["); @@ -7500,6 +7495,10 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r { printf(" EH-live"); } + if (varDsc->lvSingleDef) + { + printf(" single-def"); + } #ifndef TARGET_64BIT if (varDsc->lvStructDoubleAlign) printf(" double-align"); From 5176e8555d7769a5f6cb36192b325a0784368638 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 15 Jun 2021 19:15:24 -0700 Subject: [PATCH 02/20] Rename lvEhWriteThruCandidate->lvSingleDefRegCandidate, introduce isSingleDef --- src/coreclr/jit/compiler.h | 9 +++++---- src/coreclr/jit/lclvars.cpp | 26 +++++++++++++------------- src/coreclr/jit/lsra.cpp | 8 +++++++- src/coreclr/jit/lsra.h | 4 ++++ 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f816e6917a6042..8c05b98ee7d08d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -446,10 +446,11 @@ 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, it is the criteria 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 #if ASSERTION_PROP unsigned char lvDisqualify : 1; // variable is no longer OK for add copy optimization @@ -547,7 +548,7 @@ class LclVarDsc unsigned char lvFldOrdinal; #ifdef DEBUG - unsigned char lvDisqualifyEHVarReason = 'H'; + unsigned char lvSingleDefDisqualifyReason = 'H'; #endif #if FEATURE_MULTIREG_ARGS diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index f02cdbef112bd2..59d3949980dcfe 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -2559,7 +2559,7 @@ 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)); } @@ -2567,7 +2567,7 @@ void Compiler::lvaSetVarLiveInOutOfHandler(unsigned varNum) } // 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)); } @@ -4110,7 +4110,7 @@ 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 { @@ -4118,25 +4118,25 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, bool bbIsReturn = block->bbJumpKind == BBJ_RETURN; 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 { @@ -4146,7 +4146,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); } } @@ -4522,7 +4522,7 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers) if (!isRecompute) { varDsc->lvSingleDef = varDsc->lvIsParam; - varDsc->lvEhWriteThruCandidate = varDsc->lvIsParam; + varDsc->lvSingleDefRegCandidate = varDsc->lvIsParam; } } @@ -7422,7 +7422,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } if (lvaEnregEHVars && varDsc->lvLiveInOutOfHndlr) { - printf("%c", varDsc->lvDisqualifyEHVarReason); + printf("%c", varDsc->lvDisqualifySingleDefRegCandidate); } if (varDsc->lvLclFieldExpr) { diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index f810ca3d3aa35e..08f9930a0ada1d 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1792,9 +1792,15 @@ void LinearScan::identifyCandidates() newInt->isStructField = true; } + if (varDsc->lvSingleDefRegCandidate) + { + newInt->isSingleDef = true; + setIntervalAsSpilled(newInt); + } + if (varDsc->lvLiveInOutOfHndlr) { - newInt->isWriteThru = varDsc->lvEhWriteThruCandidate; + newInt->isWriteThru = varDsc->lvSingleDefRegCandidate; setIntervalAsSpilled(newInt); } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 10ff1f471b4a71..a730ebb72726c1 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1928,6 +1928,7 @@ class Interval : public Referenceable , isPartiallySpilled(false) #endif , isWriteThru(false) + , isSingleDef(false) #ifdef DEBUG , intervalIndex(0) #endif @@ -2023,6 +2024,9 @@ class Interval : public Referenceable // True if this interval is associated with a lclVar that is written to memory at each definition. bool isWriteThru : 1; + // True if this interval has a single definition. + bool isSingleDef : 1; + #ifdef DEBUG unsigned int intervalIndex; #endif // DEBUG From 3262f3fd6a6a5e18805c614fa733b2ca09b73c56 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 17 Jun 2021 00:21:41 -0700 Subject: [PATCH 03/20] 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. --- src/coreclr/jit/codegenlinear.cpp | 10 +++--- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/emitxarch.cpp | 7 ++-- src/coreclr/jit/lsra.cpp | 59 +++++++++++++++++++++++-------- src/coreclr/jit/lsra.h | 6 ++++ 5 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index eb942332554f34..53a59763af0ccc 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -873,9 +873,9 @@ 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->lvLiveInOutOfHndlr && !varDsc->lvSingleDefRegCandidate) { instruction storeIns = ins_Store(lclType, compiler->isSIMDTypeLocalAligned(varNum)); assert(varDsc->GetRegNum() == tree->GetRegNum()); @@ -883,7 +883,7 @@ void CodeGen::genSpillVar(GenTree* tree) } // 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. @@ -919,7 +919,7 @@ 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)); + assert((varDsc->lvLiveInOutOfHndlr | varDsc->lvSingleDefRegCandidate) && ((tree->gtFlags & GTF_VAR_DEF) != 0)); } #ifdef USING_VARIABLE_LIVE_RANGE diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index d05bfc6bbeb9fa..6c27850ca60743 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -2017,7 +2017,7 @@ inline #ifdef TARGET_AMD64 #ifndef UNIX_AMD64_ABI // On amd64, every param has a stack location, except on Unix-like systems. - assert(varDsc->lvIsParam); + assert(varDsc->lvIsParam || varDsc->lvSingleDefRegCandidate); #endif // UNIX_AMD64_ABI #else // !TARGET_AMD64 // For other targets, a stack parameter that is enregistered or prespilled diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index a0a5e3283d4e92..4ab5e7da689032 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -2159,15 +2159,16 @@ inline UNATIVE_OFFSET emitter::emitInsSizeSV(code_t code, int var, int dsp) // Dev10 804810 - failing this assert can lead to bad codegen and runtime crashes CLANG_FORMAT_COMMENT_ANCHOR; + LclVarDsc* varDsc = emitComp->lvaTable + var; + #ifdef UNIX_AMD64_ABI - LclVarDsc* varDsc = emitComp->lvaTable + var; bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; // Register passed args could have a stack offset of 0. noway_assert((int)offs < 0 || isRegPassedArg || emitComp->opts.IsOSR()); #else // !UNIX_AMD64_ABI - + bool isSingleDefSpill = varDsc->lvSingleDefRegCandidate; // OSR transitioning to RBP frame currently can have mid-frame FP - noway_assert(((int)offs < 0) || emitComp->opts.IsOSR()); + noway_assert(((int)offs < 0) || isSingleDefSpill || emitComp->opts.IsOSR()); #endif // !UNIX_AMD64_ABI } diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 08f9930a0ada1d..d90ccd38c44361 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1795,7 +1795,6 @@ void LinearScan::identifyCandidates() if (varDsc->lvSingleDefRegCandidate) { newInt->isSingleDef = true; - setIntervalAsSpilled(newInt); } if (varDsc->lvLiveInOutOfHndlr) @@ -3279,6 +3278,17 @@ void LinearScan::spillInterval(Interval* interval, RefPosition* fromRefPosition fromRefPosition->spillAfter = true; } } + + // Only handle the singledef intervals whose firstRefPosition is RefTypeDef and is not already marked as spillAfter yet. + // 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 + if (interval->isSingleDef && RefTypeIsDef(interval->firstRefPosition->refType) && + !interval->firstRefPosition->spillAfter) + { + interval->firstRefPosition->singleDefSpill = true; + } + assert(toRefPosition != nullptr); #ifdef DEBUG @@ -3961,16 +3971,16 @@ void LinearScan::unassignIntervalBlockStart(RegRecord* regRecord, VarToRegMap in // // Arguments: // currentBlock - the BasicBlock we are about to allocate registers for -// allocationPass - true if we are currently allocating registers (versus writing them back) // // Return Value: // None // // Notes: -// During the allocation pass, we use the outVarToRegMap of the selected predecessor to -// determine the lclVar locations for the inVarToRegMap. -// During the resolution (write-back) pass, we only modify the inVarToRegMap in cases where -// a lclVar was spilled after the block had been completed. +// During the allocation pass (allocationPassComplete = false), we use the outVarToRegMap +// of the selected predecessor to determine the lclVar locations for the inVarToRegMap. +// During the resolution (write-back when allocationPassComplete = true) pass, we only +// modify the inVarToRegMap in cases where a lclVar was spilled after the block had been +// completed. void LinearScan::processBlockStartLocations(BasicBlock* currentBlock) { // If we have no register candidates we should only call this method during allocation. @@ -5861,9 +5871,10 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref } } - bool reload = currentRefPosition->reload; - bool spillAfter = currentRefPosition->spillAfter; - bool writeThru = currentRefPosition->writeThru; + bool reload = currentRefPosition->reload; + bool spillAfter = currentRefPosition->spillAfter; + bool writeThru = currentRefPosition->writeThru; + bool singleDefSpill = currentRefPosition->singleDefSpill; // In the reload case we either: // - Set the register to REG_STK if it will be referenced only from the home location, or @@ -5922,7 +5933,7 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref } } else if (spillAfter && !RefTypeIsUse(currentRefPosition->refType) && - (!treeNode->IsMultiReg() || treeNode->gtGetOp1()->IsMultiRegNode())) + (treeNode != nullptr) &&(!treeNode->IsMultiReg() || treeNode->gtGetOp1()->IsMultiRegNode())) { // In the case of a pure def, don't bother spilling - just assign it to the // stack. However, we need to remember that it was spilled. @@ -5932,10 +5943,7 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref assert(interval->isSpilled); varDsc->SetRegNum(REG_STK); interval->physReg = REG_NA; - if (treeNode != nullptr) - { - writeLocalReg(treeNode->AsLclVar(), interval->varNum, REG_NA); - } + writeLocalReg(treeNode->AsLclVar(), interval->varNum, REG_NA); } else // Not reload and Not pure-def that's spillAfter { @@ -6002,7 +6010,7 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref treeNode->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx()); } } - assert(interval->isSpilled); + assert(interval->isSpilled || interval->isSingleDef); interval->physReg = REG_NA; varDsc->SetRegNum(REG_STK); } @@ -6024,6 +6032,23 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref } } } + + if (singleDefSpill && (treeNode != nullptr)) + { + // This is the first (and only def) of a single-def var (only defs are marked 'singleDef'). + // If this is already marked as SPILL, we need to definitely spill the variable. + // As such, do not mark it as GTF_SPILLED because that will keep the value in register alive. + // TODO: See if the last point really matters. + if ((treeNode->gtFlags & GTF_SPILL) == 0) + { + treeNode->gtFlags |= GTF_SPILL; + treeNode->gtFlags |= GTF_SPILLED; + if (treeNode->IsMultiReg()) + { + treeNode->SetRegSpillFlagByIdx(GTF_SPILLED, currentRefPosition->getMultiRegIdx()); + } + } + } } // Update the physRegRecord for the register, so that we know what vars are in @@ -8942,6 +8967,10 @@ void RefPosition::dump(LinearScan* linearScan) { printf(" spillAfter"); } + if (this->singleDefSpill) + { + printf(" singleDefSpill"); + } if (this->writeThru) { printf(" writeThru"); diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index a730ebb72726c1..f632c2143cc3f4 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2226,6 +2226,10 @@ class RefPosition // Spill and Copy info // reload indicates that the value was spilled, and must be reloaded here. // spillAfter indicates that the value is spilled here, so a spill must be added. + // singleDefSpill indicates that it is associated with a single-def var and if it + // is decided to get spilled, it will be spilled at firstRefPosition def. That + // way, the the value of stack will always be up-to-date and no more spills or + // resolutions (from reg to stack) will be needed for such single-def var. // copyReg indicates that the value needs to be copied to a specific register, // but that it will also retain its current assigned register. // moveReg indicates that the value needs to be moved to a different register, @@ -2244,6 +2248,7 @@ class RefPosition unsigned char reload : 1; unsigned char spillAfter : 1; + unsigned char singleDefSpill : 1; unsigned char writeThru : 1; // true if this var is defined in a register and also spilled. spillAfter must NOT be // set. @@ -2291,6 +2296,7 @@ class RefPosition , lastUse(false) , reload(false) , spillAfter(false) + , singleDefSpill(false) , writeThru(false) , copyReg(false) , moveReg(false) From bed12a0f7580f44184047ffe692d4fc1248bf4ad Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 17 Jun 2021 01:30:18 -0700 Subject: [PATCH 04/20] jit format --- src/coreclr/jit/compiler.h | 3 ++- src/coreclr/jit/emitxarch.cpp | 4 ++-- src/coreclr/jit/lclvars.cpp | 4 ++-- src/coreclr/jit/lsra.cpp | 7 ++++--- src/coreclr/jit/lsra.h | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8c05b98ee7d08d..645c2823b8600e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -450,7 +450,8 @@ class LclVarDsc // Currently, it is the criteria to decide if an EH variable can be // a register candiate or not. - unsigned char lvDisqualifySingleDefRegCandidate : 1; // tracks variable that are disqualified from register candidancy + unsigned char lvDisqualifySingleDefRegCandidate : 1; // tracks variable that are disqualified from register + // candidancy #if ASSERTION_PROP unsigned char lvDisqualify : 1; // variable is no longer OK for add copy optimization diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 4ab5e7da689032..81332098c67b40 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -1715,7 +1715,7 @@ inline unsigned emitter::insEncodeRegSIB(instruction ins, regNumber reg, code_t* } unsigned regBits = RegEncoding(reg); #else // !TARGET_AMD64 - unsigned regBits = reg; + unsigned regBits = reg; #endif // !TARGET_AMD64 assert(regBits < 8); @@ -2162,7 +2162,7 @@ inline UNATIVE_OFFSET emitter::emitInsSizeSV(code_t code, int var, int dsp) LclVarDsc* varDsc = emitComp->lvaTable + var; #ifdef UNIX_AMD64_ABI - bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; + bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; // Register passed args could have a stack offset of 0. noway_assert((int)offs < 0 || isRegPassedArg || emitComp->opts.IsOSR()); #else // !UNIX_AMD64_ABI diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 59d3949980dcfe..d7654caba38f75 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4135,7 +4135,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, } #endif // DEBUG - varDsc->lvSingleDefRegCandidate = false; + varDsc->lvSingleDefRegCandidate = false; varDsc->lvDisqualifySingleDefRegCandidate = true; } else @@ -4521,7 +4521,7 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers) // that was set by past phases. if (!isRecompute) { - varDsc->lvSingleDef = varDsc->lvIsParam; + varDsc->lvSingleDef = varDsc->lvIsParam; varDsc->lvSingleDefRegCandidate = varDsc->lvIsParam; } } diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index d90ccd38c44361..50dced84efcfec 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -3279,7 +3279,8 @@ 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 already marked as spillAfter + // yet. // 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 @@ -5932,8 +5933,8 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref assert(currentRefPosition->refType == RefTypeExpUse); } } - else if (spillAfter && !RefTypeIsUse(currentRefPosition->refType) && - (treeNode != nullptr) &&(!treeNode->IsMultiReg() || treeNode->gtGetOp1()->IsMultiRegNode())) + else if (spillAfter && !RefTypeIsUse(currentRefPosition->refType) && (treeNode != nullptr) && + (!treeNode->IsMultiReg() || treeNode->gtGetOp1()->IsMultiRegNode())) { // In the case of a pure def, don't bother spilling - just assign it to the // stack. However, we need to remember that it was spilled. diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index f632c2143cc3f4..b0c17358716952 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2248,7 +2248,7 @@ class RefPosition unsigned char reload : 1; unsigned char spillAfter : 1; - unsigned char singleDefSpill : 1; + unsigned char singleDefSpill : 1; unsigned char writeThru : 1; // true if this var is defined in a register and also spilled. spillAfter must NOT be // set. From 62b1d57d7080dec6bbdd02abf475446dee407995 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 18 Jun 2021 01:27:23 -0700 Subject: [PATCH 05/20] some fixes --- src/coreclr/jit/codegencommon.cpp | 14 +++++++------- src/coreclr/jit/codegenlinear.cpp | 22 ++++++++++++++++------ src/coreclr/jit/compiler.h | 2 ++ src/coreclr/jit/lclvars.cpp | 10 +++++++++- src/coreclr/jit/lsra.cpp | 15 ++++++++++----- src/coreclr/jit/morph.cpp | 2 ++ src/coreclr/jit/treelifeupdater.cpp | 6 +++--- 7 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 99852d7f446123..17057c78101576 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -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 variable, which may be already treated as live // in the register. - assert(varDsc->lvLiveInOutOfHndlr || ((regSet.GetMaskVars() & regMask) == 0)); + assert(varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef || ((regSet.GetMaskVars() & regMask) == 0)); regSet.AddMaskVars(regMask); } } @@ -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->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef; if (isInReg) { @@ -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 var, which always remains live on the stack. - if (!varDsc->lvLiveInOutOfHndlr) + if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef) { #ifdef DEBUG if (VarSetOps::IsMember(this, codeGen->gcInfo.gcVarPtrSetCur, bornVarIndex)) @@ -3704,7 +3704,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere } regArgTab[regArgNum + i].processed = false; - regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && varDsc->lvLiveInOutOfHndlr); + regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef)); /* mark stack arguments since we will take care of those first */ regArgTab[regArgNum + i].stackArg = (varDsc->lvIsInReg()) ? false : true; @@ -4762,7 +4762,7 @@ void CodeGen::genCheckUseBlockInit() { if (!varDsc->lvRegister) { - if (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr) + if (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) { // Var is on the stack at entry. initStkLclCnt += @@ -7125,7 +7125,7 @@ void CodeGen::genFnProlog() } bool isInReg = varDsc->lvIsInReg(); - bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr; + bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef; // Note that 'lvIsInReg()' will only be accurate for variables that are actually live-in to // the first block. This will include all possibly-uninitialized locals, whose liveness @@ -11422,7 +11422,7 @@ void CodeGen::genMultiRegStoreToLocal(GenTreeLclVar* lclNode) { varReg = REG_STK; } - if ((varReg == REG_STK) || fieldVarDsc->lvLiveInOutOfHndlr) + if ((varReg == REG_STK) || fieldVarDsc->lvLiveInOutOfHndlr || fieldVarDsc->lvSpillAtSingleDef) { if (!lclNode->AsLclVar()->IsLastUse(i)) { diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 53a59763af0ccc..84e546e1695e1a 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -175,6 +175,16 @@ void CodeGen::genCodeForBBlist() BasicBlock* block; + //////////// + //compiler->opts.dspCode = true; + //compiler->opts.dspEHTable = true; + //compiler->opts.dspGCtbls = true; + //compiler->opts.disAsm2 = true; + //compiler->opts.dspUnwind = true; + //compiler->verbose = true; + //compiler->codeGen->setVerbose(true); + /////////// + for (block = compiler->fgFirstBB; block != nullptr; block = block->bbNext) { #ifdef DEBUG @@ -229,7 +239,7 @@ void CodeGen::genCodeForBBlist() { newRegByrefSet |= varDsc->lvRegMask(); } - if (!varDsc->lvLiveInOutOfHndlr) + if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef) { #ifdef DEBUG if (verbose && VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex)) @@ -240,7 +250,7 @@ void CodeGen::genCodeForBBlist() VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex); } } - if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr) && compiler->lvaIsGCTracked(varDsc)) + if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) && compiler->lvaIsGCTracked(varDsc)) { #ifdef DEBUG if (verbose && !VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex)) @@ -875,7 +885,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->lvSingleDefRegCandidate) + if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef) { instruction storeIns = ins_Store(lclType, compiler->isSIMDTypeLocalAligned(varNum)); assert(varDsc->GetRegNum() == tree->GetRegNum()); @@ -919,7 +929,7 @@ 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 | varDsc->lvSingleDefRegCandidate) && ((tree->gtFlags & GTF_VAR_DEF) != 0)); + assert((varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) && ((tree->gtFlags & GTF_VAR_DEF) != 0)); } #ifdef USING_VARIABLE_LIVE_RANGE @@ -1055,7 +1065,7 @@ void CodeGen::genUnspillLocal( } #endif // USING_VARIABLE_LIVE_RANGE - if (!varDsc->lvLiveInOutOfHndlr) + if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef) { #ifdef DEBUG if (VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varDsc->lvVarIndex)) @@ -2049,7 +2059,7 @@ void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclN // 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 (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || !varDsc->lvLiveInOutOfHndlr) + if (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)) { // Store local variable to its home location. // Ensure that lclVar stores are typed correctly. diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 645c2823b8600e..ea6ee3dfd11dc0 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -450,6 +450,8 @@ class LclVarDsc // Currently, it is the criteria to decide if an EH variable can be // a register candiate or not. + unsigned char lvSpillAtSingleDef : 1; + unsigned char lvDisqualifySingleDefRegCandidate : 1; // tracks variable that are disqualified from register // candidancy diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index d7654caba38f75..78e0c5baa09d0c 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4117,6 +4117,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, bool bbInALoop = (block->bbFlags & BBF_BACKWARD_JUMP) != 0; bool bbIsReturn = block->bbJumpKind == BBJ_RETURN; bool needsExplicitZeroInit = fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn); + varDsc->lvSpillAtSingleDef = false; if (varDsc->lvSingleDefRegCandidate || needsExplicitZeroInit) { @@ -4557,6 +4558,8 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers) // count those in our heuristic for register allocation, since they always // must be stored, so there's no value in enregistering them at defs; only // if there are enough uses to justify it. + // + //TODO: May be applicable for single-def as well. if (varDsc->lvLiveInOutOfHndlr && !varDsc->lvDoNotEnregister && ((node->gtFlags & GTF_VAR_DEF) != 0)) { @@ -7495,10 +7498,15 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r { printf(" EH-live"); } - if (varDsc->lvSingleDef) + if (varDsc->lvSpillAtSingleDef) + { + printf(" spill-single-def"); + } + else if (varDsc->lvSingleDefRegCandidate) { printf(" single-def"); } + #ifndef TARGET_64BIT if (varDsc->lvStructDoubleAlign) printf(" double-align"); diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 50dced84efcfec..71885951fddedc 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -196,9 +196,9 @@ BasicBlock::weight_t LinearScan::getWeight(RefPosition* refPos) if (refPos->getInterval()->isSpilled) { // Decrease the weight if the interval has already been spilled. - if (varDsc->lvLiveInOutOfHndlr) + if (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) { - // An EH var is always spilled at defs, and we'll decrease the weight by half, + // An EH-var/single-def is always spilled at defs, and we'll decrease the weight by half, // since only the reload is needed. weight = weight / 2; } @@ -1800,7 +1800,7 @@ void LinearScan::identifyCandidates() if (varDsc->lvLiveInOutOfHndlr) { newInt->isWriteThru = varDsc->lvSingleDefRegCandidate; - setIntervalAsSpilled(newInt); + setIntervalAsSpilled(newInt); // TODO: Explore what happens if we mark interval as spilled right here. } INTRACK_STATS(regCandidateVarCount++); @@ -6044,11 +6044,16 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref { treeNode->gtFlags |= GTF_SPILL; treeNode->gtFlags |= GTF_SPILLED; + + if (treeNode->IsMultiReg()) { treeNode->SetRegSpillFlagByIdx(GTF_SPILLED, currentRefPosition->getMultiRegIdx()); } } + + // TODO: See if this can be outside the if-check. + varDsc->lvSpillAtSingleDef = true; } } @@ -9714,12 +9719,12 @@ void LinearScan::dumpLsraAllocationEvent( case LSRA_EVENT_DONE_KILL_GC_REFS: dumpRefPositionShort(activeRefPosition, currentBlock); - printf("Done "); + printf("Done "); break; case LSRA_EVENT_NO_GC_KILLS: dumpRefPositionShort(activeRefPosition, currentBlock); - printf("None "); + printf("None "); break; // Block boundaries diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index daca2c487e2f05..f81e8cdf3b10f8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17448,6 +17448,8 @@ void Compiler::fgRetypeImplicitByRefArgs() newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; newVarDsc->lvLiveInOutOfHndlr = varDsc->lvLiveInOutOfHndlr; + newVarDsc->lvSingleDefRegCandidate = varDsc->lvSingleDefRegCandidate; + newVarDsc->lvSpillAtSingleDef = varDsc->lvSpillAtSingleDef; newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; #endif // DEBUG diff --git a/src/coreclr/jit/treelifeupdater.cpp b/src/coreclr/jit/treelifeupdater.cpp index 20a9745362b570..4017e231295321 100644 --- a/src/coreclr/jit/treelifeupdater.cpp +++ b/src/coreclr/jit/treelifeupdater.cpp @@ -60,7 +60,7 @@ bool TreeLifeUpdater::UpdateLifeFieldVar(GenTreeLclVar* lclNode, uns { regNumber reg = lclNode->GetRegNumByIdx(multiRegIndex); bool isInReg = fldVarDsc->lvIsInReg() && reg != REG_NA; - isInMemory = !isInReg || fldVarDsc->lvLiveInOutOfHndlr; + isInMemory = !isInReg || fldVarDsc->lvLiveInOutOfHndlr || fldVarDsc->lvSpillAtSingleDef; if (isInReg) { if (isBorn) @@ -259,7 +259,7 @@ void TreeLifeUpdater::UpdateLifeVar(GenTree* tree) compiler->codeGen->genUpdateVarReg(varDsc, tree); } bool isInReg = varDsc->lvIsInReg() && tree->GetRegNum() != REG_NA; - bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr; + bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef; if (isInReg) { compiler->codeGen->genUpdateRegLife(varDsc, isBorn, isDying DEBUGARG(tree)); @@ -283,7 +283,7 @@ void TreeLifeUpdater::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; + bool isInMemory = !isInReg || fldVarDsc->lvLiveInOutOfHndlr || fldVarDsc->lvSpillAtSingleDef; bool isFieldDying = lclVarTree->AsLclVar()->IsLastUse(i); if ((isBorn && !isFieldDying) || (!isBorn && isFieldDying)) { From 4563709067fc7eb7edb296b1b9db69ba2bafc5da Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 18 Jun 2021 14:18:06 -0700 Subject: [PATCH 06/20] wip --- src/coreclr/jit/codegenlinear.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 84e546e1695e1a..7bd2e746845ada 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -175,15 +175,18 @@ void CodeGen::genCodeForBBlist() BasicBlock* block; - //////////// - //compiler->opts.dspCode = true; - //compiler->opts.dspEHTable = true; - //compiler->opts.dspGCtbls = true; - //compiler->opts.disAsm2 = true; - //compiler->opts.dspUnwind = true; - //compiler->verbose = true; - //compiler->codeGen->setVerbose(true); - /////////// + //if (compiler->info.compMethodHashPrivate == 0x939467e2) + //{ + // ////////// + // compiler->opts.dspCode = true; + // compiler->opts.dspEHTable = true; + // compiler->opts.dspGCtbls = true; + // compiler->opts.disAsm2 = true; + // compiler->opts.dspUnwind = true; + // compiler->verbose = true; + // compiler->codeGen->setVerbose(true); + // ///////// + //} for (block = compiler->fgFirstBB; block != nullptr; block = block->bbNext) { @@ -2059,7 +2062,7 @@ void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclN // 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 (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)) + if (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->lvLiveInOutOfHndlr/* && !varDsc->lvSpillAtSingleDef*/)) { // Store local variable to its home location. // Ensure that lclVar stores are typed correctly. From 470754854048413d5c83c10ea84a9fffd2f38734 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 28 Jun 2021 14:21:42 -0700 Subject: [PATCH 07/20] Add check of isSingleDef in validateInterval() --- src/coreclr/jit/lclvars.cpp | 2 +- src/coreclr/jit/lsrabuild.cpp | 28 +++++++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 78e0c5baa09d0c..837c1e923d7aef 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4112,7 +4112,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, 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 + if ((tree->gtFlags & GTF_VAR_DEF) || (tree->gtFlags & GTF_VAR_CLONED)) // Is this is a def of our variable { bool bbInALoop = (block->bbFlags & BBF_BACKWARD_JUMP) != 0; bool bbIsReturn = block->bbJumpKind == BBJ_RETURN; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index cb04ddddf51c09..9251592634fbdd 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2602,8 +2602,9 @@ void LinearScan::buildIntervals() { lsraDumpIntervals("BEFORE VALIDATING INTERVALS"); dumpRefPositions("BEFORE VALIDATING INTERVALS"); - validateIntervals(); } + validateIntervals(); + #endif // DEBUG } @@ -2614,8 +2615,7 @@ void LinearScan::buildIntervals() // // Notes: If an undefined use is encountered, it merely prints a message. // -// TODO-Cleanup: This should probably assert, or at least print the message only -// when doing a JITDUMP. +// TODO-Cleanup: This should probably assert. // void LinearScan::validateIntervals() { @@ -2630,19 +2630,33 @@ void LinearScan::validateIntervals() Interval* interval = getIntervalForLocalVar(i); bool defined = false; - printf("-----------------\n"); + bool singleDefined = false; + JITDUMP("-----------------\n"); for (RefPosition* ref = interval->firstRefPosition; ref != nullptr; ref = ref->nextRefPosition) { - ref->dump(this); + if (VERBOSE) + { + ref->dump(this); + } RefType refType = ref->refType; if (!defined && RefTypeIsUse(refType)) { if (compiler->info.compMethodName != nullptr) { - printf("%s: ", compiler->info.compMethodName); + JITDUMP("%s: ", compiler->info.compMethodName); } - printf("LocalVar V%02u: undefined use at %u\n", interval->varNum, ref->nodeLocation); + JITDUMP("LocalVar V%02u: undefined use at %u\n", interval->varNum, ref->nodeLocation); } + + // For single-def intervals, the only the first refposition should be a RefTypeDef + if (interval->isSingleDef && RefTypeIsDef(refType)) + { + if (ref != interval->firstRefPosition) + { + printf("here"); + } + } + // Note that there can be multiple last uses if they are on disjoint paths, // so we can't really check the lastUse flag if (ref->lastUse) From 2d67df2dce295e10cc3603e30913bca8648b25fa Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 28 Jun 2021 18:25:50 -0700 Subject: [PATCH 08/20] Make isSingleDef during buildIntervals --- src/coreclr/jit/lsra.cpp | 5 ----- src/coreclr/jit/lsrabuild.cpp | 11 +++++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 71885951fddedc..0ce4bc8e622214 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1792,11 +1792,6 @@ void LinearScan::identifyCandidates() newInt->isStructField = true; } - if (varDsc->lvSingleDefRegCandidate) - { - newInt->isSingleDef = true; - } - if (varDsc->lvLiveInOutOfHndlr) { newInt->isWriteThru = varDsc->lvSingleDefRegCandidate; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 9251592634fbdd..47e26ddab95317 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -620,6 +620,12 @@ RefPosition* LinearScan::newRefPosition(Interval* theInterval, associateRefPosWithInterval(newRP); + if (RefTypeIsDef(newRP->refType)) + { + assert(theInterval != nullptr); + theInterval->isSingleDef = theInterval->firstRefPosition == newRP; + } + DBEXEC(VERBOSE, newRP->dump(this)); return newRP; } @@ -2651,10 +2657,7 @@ void LinearScan::validateIntervals() // For single-def intervals, the only the first refposition should be a RefTypeDef if (interval->isSingleDef && RefTypeIsDef(refType)) { - if (ref != interval->firstRefPosition) - { - printf("here"); - } + assert(ref == interval->firstRefPosition); } // Note that there can be multiple last uses if they are on disjoint paths, From 12ef329276ef9ec1104e983dbe3be469366973b8 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 29 Jun 2021 10:30:18 -0700 Subject: [PATCH 09/20] minor fix in lclvars.cpp --- src/coreclr/jit/lclvars.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 837c1e923d7aef..572ebbdc37aa6a 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4112,12 +4112,13 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, if (!varDsc->lvDisqualifySingleDefRegCandidate) // If this var is already disqualified, we can skip this { - if ((tree->gtFlags & GTF_VAR_DEF) || (tree->gtFlags & GTF_VAR_CLONED)) // Is this is a def of our variable + varDsc->lvSpillAtSingleDef = false; + + 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 needsExplicitZeroInit = fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn); - varDsc->lvSpillAtSingleDef = false; if (varDsc->lvSingleDefRegCandidate || needsExplicitZeroInit) { @@ -7425,7 +7426,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } if (lvaEnregEHVars && varDsc->lvLiveInOutOfHndlr) { - printf("%c", varDsc->lvDisqualifySingleDefRegCandidate); + printf("%c", varDsc->lvSingleDefDisqualifyReason); } if (varDsc->lvLclFieldExpr) { From a5dbc39359e6f92ad0c8739aded0f70c2e41d1a5 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 29 Jun 2021 11:20:06 -0700 Subject: [PATCH 10/20] some fixes after self CR --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/codegenlinear.cpp | 2 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/emitxarch.cpp | 2 +- src/coreclr/jit/lsra.cpp | 8 +++++++- src/coreclr/jit/lsrabuild.cpp | 11 +++++------ 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 17057c78101576..0434818a48c822 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3704,7 +3704,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere } regArgTab[regArgNum + i].processed = false; - regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef)); + regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef)); // TODO: double check if lvSpillAtSingleDef is needed here? /* mark stack arguments since we will take care of those first */ regArgTab[regArgNum + i].stackArg = (varDsc->lvIsInReg()) ? false : true; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 7bd2e746845ada..a3931a658805dd 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -2062,7 +2062,7 @@ void CodeGen::genSpillLocal(unsigned varNum, var_types type, GenTreeLclVar* lclN // 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 (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->lvLiveInOutOfHndlr/* && !varDsc->lvSpillAtSingleDef*/)) + if (((lclNode->gtFlags & GTF_VAR_DEF) != 0) || (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef)) { // Store local variable to its home location. // Ensure that lclVar stores are typed correctly. diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 6c27850ca60743..a37b6aa727a53b 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -2017,7 +2017,7 @@ inline #ifdef TARGET_AMD64 #ifndef UNIX_AMD64_ABI // On amd64, every param has a stack location, except on Unix-like systems. - assert(varDsc->lvIsParam || varDsc->lvSingleDefRegCandidate); + assert(varDsc->lvIsParam || varDsc->lvSingleDefRegCandidate || varDsc->lvSpillAtSingleDef); #endif // UNIX_AMD64_ABI #else // !TARGET_AMD64 // For other targets, a stack parameter that is enregistered or prespilled diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 81332098c67b40..3e4e57b862235f 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -2166,7 +2166,7 @@ inline UNATIVE_OFFSET emitter::emitInsSizeSV(code_t code, int var, int dsp) // Register passed args could have a stack offset of 0. noway_assert((int)offs < 0 || isRegPassedArg || emitComp->opts.IsOSR()); #else // !UNIX_AMD64_ABI - bool isSingleDefSpill = varDsc->lvSingleDefRegCandidate; + bool isSingleDefSpill = varDsc->lvSpillAtSingleDef; // OSR transitioning to RBP frame currently can have mid-frame FP noway_assert(((int)offs < 0) || isSingleDefSpill || emitComp->opts.IsOSR()); #endif // !UNIX_AMD64_ABI diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 0ce4bc8e622214..f1f86bf2c540d7 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -3279,9 +3279,15 @@ void LinearScan::spillInterval(Interval* interval, RefPosition* fromRefPosition // 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 + // + // Note: Only mark "singleDefSpill" for those intervals who ever get spilled. The intervals that are never spilled + // will not be marked as "singleDefSpill" and hence won't get spilled at the first definition. if (interval->isSingleDef && RefTypeIsDef(interval->firstRefPosition->refType) && !interval->firstRefPosition->spillAfter) { + //TODO: Check if it is beneficial to spill at def, meaning, is it in hot block and if yes, don't worry about + // doing the spill.... + // Also check how many uses are present for this variable. If USE > 3, then only do this optimization. interval->firstRefPosition->singleDefSpill = true; } @@ -6006,7 +6012,7 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref treeNode->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx()); } } - assert(interval->isSpilled || interval->isSingleDef); + assert(interval->isSpilled); interval->physReg = REG_NA; varDsc->SetRegNum(REG_STK); } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 47e26ddab95317..f5f4d781d759ec 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2616,12 +2616,12 @@ void LinearScan::buildIntervals() #ifdef DEBUG //------------------------------------------------------------------------ -// validateIntervals: A DEBUG-only method that checks that the lclVar RefPositions -// do not reflect uses of undefined values +// validateIntervals: A DEBUG-only method that checks that: +// - the lclVar RefPositions do not reflect uses of undefined values +// - A singleDef interval should have just first RefPosition as RefTypeDef. // -// Notes: If an undefined use is encountered, it merely prints a message. -// -// TODO-Cleanup: This should probably assert. +// TODO-Cleanup: If an undefined use is encountered, it merely prints a message +// but probably assert. // void LinearScan::validateIntervals() { @@ -2636,7 +2636,6 @@ void LinearScan::validateIntervals() Interval* interval = getIntervalForLocalVar(i); bool defined = false; - bool singleDefined = false; JITDUMP("-----------------\n"); for (RefPosition* ref = interval->firstRefPosition; ref != nullptr; ref = ref->nextRefPosition) { From 803e59e578e753627227eafac9fd173dc50cda20 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 12:15:19 -0700 Subject: [PATCH 11/20] Updated some comments --- src/coreclr/jit/codegencommon.cpp | 6 +++--- src/coreclr/jit/codegenlinear.cpp | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 0434818a48c822..9eb33d5791231b 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -552,8 +552,8 @@ 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. + // 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)); regSet.AddMaskVars(regMask); } @@ -777,7 +777,7 @@ 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. + // unless it is an EH/"spill at single-def" var, which always remains live on the stack. if (!varDsc->lvLiveInOutOfHndlr && !varDsc->lvSpillAtSingleDef) { #ifdef DEBUG diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index a3931a658805dd..73dab981a76cba 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -931,7 +931,8 @@ void CodeGen::genSpillVar(GenTree* tree) } else { - // We only have 'GTF_SPILL' and 'GTF_SPILLED' on a def of a write-thru lclVar. + // 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)); } @@ -2057,10 +2058,10 @@ 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 && !varDsc->lvSpillAtSingleDef)) { From 1171e15669702e350f74049476e0ca095d5ce86d Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 12:24:21 -0700 Subject: [PATCH 12/20] Remove lvSpillAtSingleDef from some asserts --- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/emitxarch.cpp | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a37b6aa727a53b..d05bfc6bbeb9fa 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -2017,7 +2017,7 @@ inline #ifdef TARGET_AMD64 #ifndef UNIX_AMD64_ABI // On amd64, every param has a stack location, except on Unix-like systems. - assert(varDsc->lvIsParam || varDsc->lvSingleDefRegCandidate || varDsc->lvSpillAtSingleDef); + assert(varDsc->lvIsParam); #endif // UNIX_AMD64_ABI #else // !TARGET_AMD64 // For other targets, a stack parameter that is enregistered or prespilled diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 3e4e57b862235f..2ff40377c6b325 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -2159,16 +2159,15 @@ inline UNATIVE_OFFSET emitter::emitInsSizeSV(code_t code, int var, int dsp) // Dev10 804810 - failing this assert can lead to bad codegen and runtime crashes CLANG_FORMAT_COMMENT_ANCHOR; - LclVarDsc* varDsc = emitComp->lvaTable + var; - #ifdef UNIX_AMD64_ABI + LclVarDsc* varDsc = emitComp->lvaTable + var; bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; // Register passed args could have a stack offset of 0. noway_assert((int)offs < 0 || isRegPassedArg || emitComp->opts.IsOSR()); #else // !UNIX_AMD64_ABI - bool isSingleDefSpill = varDsc->lvSpillAtSingleDef; + // OSR transitioning to RBP frame currently can have mid-frame FP - noway_assert(((int)offs < 0) || isSingleDefSpill || emitComp->opts.IsOSR()); + noway_assert(((int)offs < 0) || emitComp->opts.IsOSR()); #endif // !UNIX_AMD64_ABI } From d792cfc5e71dd9dad567d37cbe42516d1ccfd094 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 14:01:06 -0700 Subject: [PATCH 13/20] Use singleDefSpill information in getWeight() --- src/coreclr/jit/lsra.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index f1f86bf2c540d7..65348777d64024 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -196,7 +196,7 @@ BasicBlock::weight_t LinearScan::getWeight(RefPosition* refPos) if (refPos->getInterval()->isSpilled) { // Decrease the weight if the interval has already been spilled. - if (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) + if (varDsc->lvLiveInOutOfHndlr || refPos->getInterval()->firstRefPosition->singleDefSpill) { // An EH-var/single-def is always spilled at defs, and we'll decrease the weight by half, // since only the reload is needed. From 25770dfaba9b032fa6d78f0e1256c55b116420d9 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 14:44:16 -0700 Subject: [PATCH 14/20] Remove lvSpillAtSingleDef from some more checks --- src/coreclr/jit/codegencommon.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 9eb33d5791231b..40f753ffffd18e 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3704,7 +3704,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere } regArgTab[regArgNum + i].processed = false; - regArgTab[regArgNum + i].writeThru = (varDsc->lvIsInReg() && (varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef)); // TODO: double check if lvSpillAtSingleDef is needed here? + 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; @@ -4762,7 +4762,7 @@ void CodeGen::genCheckUseBlockInit() { if (!varDsc->lvRegister) { - if (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) + if (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr) { // Var is on the stack at entry. initStkLclCnt += @@ -7125,7 +7125,7 @@ void CodeGen::genFnProlog() } bool isInReg = varDsc->lvIsInReg(); - bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef; + bool isInMemory = !isInReg || varDsc->lvLiveInOutOfHndlr; // Note that 'lvIsInReg()' will only be accurate for variables that are actually live-in to // the first block. This will include all possibly-uninitialized locals, whose liveness From 4d3b6b6196fde59ec7205487c20cafbf5cc47cd7 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 17:56:16 -0700 Subject: [PATCH 15/20] Mark lvSpillAtSingleDef whenever refPosition->singleDefSpill==true --- src/coreclr/jit/codegenlinear.cpp | 13 ------------- src/coreclr/jit/compiler.h | 11 ++++++++--- src/coreclr/jit/emitxarch.cpp | 6 +++--- src/coreclr/jit/lclvars.cpp | 2 -- src/coreclr/jit/lsra.cpp | 30 ++++++++++++++---------------- src/coreclr/jit/morph.cpp | 1 + 6 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 73dab981a76cba..0a7b33b9057f6e 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -175,19 +175,6 @@ void CodeGen::genCodeForBBlist() BasicBlock* block; - //if (compiler->info.compMethodHashPrivate == 0x939467e2) - //{ - // ////////// - // compiler->opts.dspCode = true; - // compiler->opts.dspEHTable = true; - // compiler->opts.dspGCtbls = true; - // compiler->opts.disAsm2 = true; - // compiler->opts.dspUnwind = true; - // compiler->verbose = true; - // compiler->codeGen->setVerbose(true); - // ///////// - //} - for (block = compiler->fgFirstBB; block != nullptr; block = block->bbNext) { #ifdef DEBUG diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ea6ee3dfd11dc0..3d2e00a47ad0f2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -447,14 +447,19 @@ class LclVarDsc // after lvaMarkLocalVars: identifies locals that are suitable for optAddCopies unsigned char lvSingleDefRegCandidate : 1; // variable has a single def and hence is a register candidate - // Currently, it is the criteria to decide if an EH variable can be + // Currently, this is only used to decide if an EH variable can be // a register candiate or not. - unsigned char lvSpillAtSingleDef : 1; - 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 unsigned char lvVolatileHint : 1; // hint for AssertionProp diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 2ff40377c6b325..a0a5e3283d4e92 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -1715,7 +1715,7 @@ inline unsigned emitter::insEncodeRegSIB(instruction ins, regNumber reg, code_t* } unsigned regBits = RegEncoding(reg); #else // !TARGET_AMD64 - unsigned regBits = reg; + unsigned regBits = reg; #endif // !TARGET_AMD64 assert(regBits < 8); @@ -2160,8 +2160,8 @@ inline UNATIVE_OFFSET emitter::emitInsSizeSV(code_t code, int var, int dsp) CLANG_FORMAT_COMMENT_ANCHOR; #ifdef UNIX_AMD64_ABI - LclVarDsc* varDsc = emitComp->lvaTable + var; - bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; + LclVarDsc* varDsc = emitComp->lvaTable + var; + bool isRegPassedArg = varDsc->lvIsParam && varDsc->lvIsRegArg; // Register passed args could have a stack offset of 0. noway_assert((int)offs < 0 || isRegPassedArg || emitComp->opts.IsOSR()); #else // !UNIX_AMD64_ABI diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 572ebbdc37aa6a..8ff7b42c94f911 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4559,8 +4559,6 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers) // count those in our heuristic for register allocation, since they always // must be stored, so there's no value in enregistering them at defs; only // if there are enough uses to justify it. - // - //TODO: May be applicable for single-def as well. if (varDsc->lvLiveInOutOfHndlr && !varDsc->lvDoNotEnregister && ((node->gtFlags & GTF_VAR_DEF) != 0)) { diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 65348777d64024..7004347b5bce3d 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -5876,7 +5876,6 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref bool reload = currentRefPosition->reload; bool spillAfter = currentRefPosition->spillAfter; bool writeThru = currentRefPosition->writeThru; - bool singleDefSpill = currentRefPosition->singleDefSpill; // In the reload case we either: // - Set the register to REG_STK if it will be referenced only from the home location, or @@ -6035,25 +6034,24 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref } } - if (singleDefSpill && (treeNode != nullptr)) + if (currentRefPosition->singleDefSpill && (treeNode != nullptr)) { - // This is the first (and only def) of a single-def var (only defs are marked 'singleDef'). - // If this is already marked as SPILL, we need to definitely spill the variable. - // As such, do not mark it as GTF_SPILLED because that will keep the value in register alive. - // TODO: See if the last point really matters. - if ((treeNode->gtFlags & GTF_SPILL) == 0) - { - treeNode->gtFlags |= GTF_SPILL; - treeNode->gtFlags |= GTF_SPILLED; - + // This is the first (and only) def of a single-def var (only defs are marked 'singleDefSpill'). + // Mark it as GTF_SPILL, so it is spilled immediately to the stack at definition and + // GTF_SPILLED, so the variable stays live in the register. + // + // TODO: This approach would still create the resolution moves but during codegen, will check for + // `lvSpillAtSingleDef` to decide whether to generate spill or not. In future, see if there is some + // better way to avoid resolution moves, perhaps by updating the varDsc->SetRegNum(REG_STK) in this + // method? + treeNode->gtFlags |= GTF_SPILL; + treeNode->gtFlags |= GTF_SPILLED; - if (treeNode->IsMultiReg()) - { - treeNode->SetRegSpillFlagByIdx(GTF_SPILLED, currentRefPosition->getMultiRegIdx()); - } + if (treeNode->IsMultiReg()) + { + treeNode->SetRegSpillFlagByIdx(GTF_SPILLED, currentRefPosition->getMultiRegIdx()); } - // TODO: See if this can be outside the if-check. varDsc->lvSpillAtSingleDef = true; } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f81e8cdf3b10f8..eddf01d6fa3350 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17448,6 +17448,7 @@ void Compiler::fgRetypeImplicitByRefArgs() newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; newVarDsc->lvLiveInOutOfHndlr = varDsc->lvLiveInOutOfHndlr; + newVarDsc->lvSingleDef = varDsc->lvSingleDef; newVarDsc->lvSingleDefRegCandidate = varDsc->lvSingleDefRegCandidate; newVarDsc->lvSpillAtSingleDef = varDsc->lvSpillAtSingleDef; newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; From 22a1a7d5103916972f1421dfd6c36b72ed90e331 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 17:57:14 -0700 Subject: [PATCH 16/20] Add TODO for SingleDefVarCandidate --- src/coreclr/jit/lclvars.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 8ff7b42c94f911..313a0bbd1df8c8 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4112,12 +4112,12 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, if (!varDsc->lvDisqualifySingleDefRegCandidate) // If this var is already disqualified, we can skip this { - varDsc->lvSpillAtSingleDef = false; - 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; + // 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->lvSingleDefRegCandidate || needsExplicitZeroInit) From e3383c11769f88109137d55d3b0d83e656f7e9e6 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 30 Jun 2021 18:24:59 -0700 Subject: [PATCH 17/20] Some notes on setting singleDefSpill --- src/coreclr/jit/lsra.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 7004347b5bce3d..5678d8366ff508 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1795,7 +1795,7 @@ void LinearScan::identifyCandidates() if (varDsc->lvLiveInOutOfHndlr) { newInt->isWriteThru = varDsc->lvSingleDefRegCandidate; - setIntervalAsSpilled(newInt); // TODO: Explore what happens if we mark interval as spilled right here. + setIntervalAsSpilled(newInt); } INTRACK_STATS(regCandidateVarCount++); @@ -3285,9 +3285,9 @@ void LinearScan::spillInterval(Interval* interval, RefPosition* fromRefPosition if (interval->isSingleDef && RefTypeIsDef(interval->firstRefPosition->refType) && !interval->firstRefPosition->spillAfter) { - //TODO: Check if it is beneficial to spill at def, meaning, is it in hot block and if yes, don't worry about - // doing the spill.... - // Also check how many uses are present for this variable. If USE > 3, then only do this optimization. + // TODO-CQ: Check if it is beneficial to spill at def, meaning, if it is a hot block don't worry about + // doing the spill. Another option is to track number of refpositions and a interval has more than X refpositions + // then perform this optimization. interval->firstRefPosition->singleDefSpill = true; } From 8facc97f7df01071f2ea55f9399084e8591d3c1c Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 1 Jul 2021 16:37:43 -0700 Subject: [PATCH 18/20] jit format --- src/coreclr/jit/codegenlinear.cpp | 3 ++- src/coreclr/jit/lclvars.cpp | 6 +++--- src/coreclr/jit/lsra.cpp | 11 ++++++----- src/coreclr/jit/morph.cpp | 12 ++++++------ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 0a7b33b9057f6e..d7df6ed1551059 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -240,7 +240,8 @@ void CodeGen::genCodeForBBlist() VarSetOps::RemoveElemD(compiler, gcInfo.gcVarPtrSetCur, varIndex); } } - if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) && compiler->lvaIsGCTracked(varDsc)) + if ((!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr || varDsc->lvSpillAtSingleDef) && + compiler->lvaIsGCTracked(varDsc)) { #ifdef DEBUG if (verbose && !VarSetOps::IsMember(compiler, gcInfo.gcVarPtrSetCur, varIndex)) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 313a0bbd1df8c8..33c5abaa30df78 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4114,8 +4114,8 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, { 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); @@ -7505,7 +7505,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r { printf(" single-def"); } - + #ifndef TARGET_64BIT if (varDsc->lvStructDoubleAlign) printf(" double-align"); diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 5678d8366ff508..e99c6ee7b22e35 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -3286,7 +3286,8 @@ void LinearScan::spillInterval(Interval* interval, RefPosition* fromRefPosition !interval->firstRefPosition->spillAfter) { // TODO-CQ: Check if it is beneficial to spill at def, meaning, if it is a hot block don't worry about - // doing the spill. Another option is to track number of refpositions and a interval has more than X refpositions + // doing the spill. Another option is to track number of refpositions and a interval has more than X + // refpositions // then perform this optimization. interval->firstRefPosition->singleDefSpill = true; } @@ -5873,9 +5874,9 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref } } - bool reload = currentRefPosition->reload; - bool spillAfter = currentRefPosition->spillAfter; - bool writeThru = currentRefPosition->writeThru; + bool reload = currentRefPosition->reload; + bool spillAfter = currentRefPosition->spillAfter; + bool writeThru = currentRefPosition->writeThru; // In the reload case we either: // - Set the register to REG_STK if it will be referenced only from the home location, or @@ -6039,7 +6040,7 @@ void LinearScan::resolveLocalRef(BasicBlock* block, GenTreeLclVar* treeNode, Ref // This is the first (and only) def of a single-def var (only defs are marked 'singleDefSpill'). // Mark it as GTF_SPILL, so it is spilled immediately to the stack at definition and // GTF_SPILLED, so the variable stays live in the register. - // + // // TODO: This approach would still create the resolution moves but during codegen, will check for // `lvSpillAtSingleDef` to decide whether to generate spill or not. In future, see if there is some // better way to avoid resolution moves, perhaps by updating the varDsc->SetRegNum(REG_STK) in this diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index eddf01d6fa3350..aab5c9b36ec486 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17444,14 +17444,14 @@ void Compiler::fgRetypeImplicitByRefArgs() newVarDsc->lvAddrExposed = varDsc->lvAddrExposed; newVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; #ifdef DEBUG - newVarDsc->lvLclBlockOpAddr = varDsc->lvLclBlockOpAddr; - newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; - newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; - newVarDsc->lvLiveInOutOfHndlr = varDsc->lvLiveInOutOfHndlr; + newVarDsc->lvLclBlockOpAddr = varDsc->lvLclBlockOpAddr; + newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; + newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; + newVarDsc->lvLiveInOutOfHndlr = varDsc->lvLiveInOutOfHndlr; newVarDsc->lvSingleDef = varDsc->lvSingleDef; newVarDsc->lvSingleDefRegCandidate = varDsc->lvSingleDefRegCandidate; - newVarDsc->lvSpillAtSingleDef = varDsc->lvSpillAtSingleDef; - newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; + newVarDsc->lvSpillAtSingleDef = varDsc->lvSpillAtSingleDef; + newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; #endif // DEBUG // If the promotion is dependent, the promoted temp would just be committed From d3e03a6558dfc3cb01fa0e184f6238b665383fec Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 8 Jul 2021 09:24:25 -0700 Subject: [PATCH 19/20] review feedback --- src/coreclr/jit/codegencommon.cpp | 8 ++++---- src/coreclr/jit/codegenlinear.cpp | 13 ++++++------- src/coreclr/jit/compiler.h | 10 ++++++++++ src/coreclr/jit/lsra.cpp | 3 +-- src/coreclr/jit/treelifeupdater.cpp | 6 +++--- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 40f753ffffd18e..aeecb0553b989b 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -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); } } @@ -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) { @@ -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)) @@ -11422,7 +11422,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)) { diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index d7df6ed1551059..b822f233e66db3 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -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)) @@ -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)) @@ -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()); @@ -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 @@ -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)) @@ -2051,7 +2050,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. diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 3d2e00a47ad0f2..1383b57a7dd3a2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1039,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 diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index e99c6ee7b22e35..4bbb877fda8c33 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -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 diff --git a/src/coreclr/jit/treelifeupdater.cpp b/src/coreclr/jit/treelifeupdater.cpp index 4017e231295321..15c32596cc4223 100644 --- a/src/coreclr/jit/treelifeupdater.cpp +++ b/src/coreclr/jit/treelifeupdater.cpp @@ -60,7 +60,7 @@ bool TreeLifeUpdater::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) @@ -259,7 +259,7 @@ void TreeLifeUpdater::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)); @@ -283,7 +283,7 @@ void TreeLifeUpdater::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)) { From 649c6c4532ec9c7b4dbcde58b69d91fdfbb634c7 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 9 Jul 2021 12:12:01 -0700 Subject: [PATCH 20/20] review comments --- src/coreclr/jit/morph.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index aab5c9b36ec486..526a996e1b40ca 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -17441,17 +17441,18 @@ void Compiler::fgRetypeImplicitByRefArgs() #endif // DEBUG // Propagate address-taken-ness and do-not-enregister-ness. - newVarDsc->lvAddrExposed = varDsc->lvAddrExposed; - newVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; -#ifdef DEBUG - newVarDsc->lvLclBlockOpAddr = varDsc->lvLclBlockOpAddr; - newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; - newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; + newVarDsc->lvAddrExposed = varDsc->lvAddrExposed; + newVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; newVarDsc->lvLiveInOutOfHndlr = varDsc->lvLiveInOutOfHndlr; newVarDsc->lvSingleDef = varDsc->lvSingleDef; newVarDsc->lvSingleDefRegCandidate = varDsc->lvSingleDefRegCandidate; newVarDsc->lvSpillAtSingleDef = varDsc->lvSpillAtSingleDef; - newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; +#ifdef DEBUG + newVarDsc->lvLclBlockOpAddr = varDsc->lvLclBlockOpAddr; + newVarDsc->lvLclFieldExpr = varDsc->lvLclFieldExpr; + newVarDsc->lvVMNeedsStackAddr = varDsc->lvVMNeedsStackAddr; + newVarDsc->lvSingleDefDisqualifyReason = varDsc->lvSingleDefDisqualifyReason; + newVarDsc->lvLiveAcrossUCall = varDsc->lvLiveAcrossUCall; #endif // DEBUG // If the promotion is dependent, the promoted temp would just be committed