From: Kunal Pathak Date: Fri, 20 Nov 2020 21:30:42 +0000 (-0800) Subject: Register allocation cleanup (#44977) X-Git-Tag: submit/tizen/20210909.063632~4487 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=07193b828792e3e3a4754854741f7e47764b964a;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Register allocation cleanup (#44977) * Register allocation cleanup - Comment updates - Misc code cleanup * jit format --- diff --git a/docs/design/coreclr/jit/lsra-detail.md b/docs/design/coreclr/jit/lsra-detail.md index a637b8a..47292da 100644 --- a/docs/design/coreclr/jit/lsra-detail.md +++ b/docs/design/coreclr/jit/lsra-detail.md @@ -387,7 +387,7 @@ well as supporting components) in more depth. - `LinearScan::identifyCandidates` - This mostly duplicates what is done in - `Compiler::lvaMarkLocalVars(). There are differences in what + `Compiler::lvaMarkLocalVars()`. There are differences in what constitutes a register candidate vs. what is tracked, but we should probably handle them in `Compiler::lvaMarkLocalVars()` when it is called after `Lowering`. @@ -445,7 +445,7 @@ node, which builds `RefPositions` according to the liveness model described abov - First, we create `RefPosition`s to define any internal registers that are required. As we create new `Interval`s for these, we add the definition `RefPosition` to an array, - `InternalDefs`. This allows us to create the corresponding uses of these internal + `internalDefs`. This allows us to create the corresponding uses of these internal registers later. - Then we create `RefPosition`s for each use in the instruction. @@ -474,7 +474,7 @@ node, which builds `RefPositions` according to the liveness model described abov we need to ensure that the other source isn't given the same register as the target. For this, we annotate the use `RefPosition` with `delayRegFree`. -- Next we create the uses of the internal registers, using the `InternalDefs` array. +- Next we create the uses of the internal registers, using the `internalDefs` array. This is cleared before the next instruction is handled. - Next, any registers in the kill set for the instruction are killed. This is performed @@ -652,7 +652,7 @@ LinearScanAllocation(List refPositions) - When `COMPlus_EnableEHWriteThru == 0`, any variable that's live in to an exception region is always referenced on the stack. - - See [Enable EHWriteThru by default](#enable-EHWriteThru-by-default). + - See [Enable EHWriteThru by default](#enable-ehwritethru-by-default). - Code generation (genGenerateCode) diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 548dc85..1dd05c2 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -4274,7 +4274,7 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers) // If, in minopts/debug, we really want to allow locals to become // unreferenced later, we'll have to explicitly clear this bit. varDsc->setLvRefCnt(0); - varDsc->setLvRefCntWtd(0); + varDsc->setLvRefCntWtd(BB_ZERO_WEIGHT); // Special case for some varargs params ... these must // remain unreferenced. diff --git a/src/coreclr/src/jit/lsra.cpp b/src/coreclr/src/jit/lsra.cpp index b09b45d..6ee1f39 100644 --- a/src/coreclr/src/jit/lsra.cpp +++ b/src/coreclr/src/jit/lsra.cpp @@ -4179,9 +4179,10 @@ void LinearScan::setIntervalAsSpilled(Interval* interval) } //------------------------------------------------------------------------ -// spill: Spill this Interval between "fromRefPosition" and "toRefPosition" +// spill: Spill the "interval" starting from "fromRefPosition" (upto "toRefPosition") // // Arguments: +// interval - The interval that contains the RefPosition to be spilled // fromRefPosition - The RefPosition at which the Interval is to be spilled // toRefPosition - The RefPosition at which it must be reloaded (debug only arg) // @@ -4372,6 +4373,7 @@ void LinearScan::unassignPhysReg(RegRecord* regRec, RefPosition* spillRefPositio { Interval* assignedInterval = regRec->assignedInterval; assert(assignedInterval != nullptr); + assert(spillRefPosition == nullptr || spillRefPosition->getInterval() == assignedInterval); regNumber thisRegNum = regRec->regNum; // Is assignedInterval actually still assigned to this register? diff --git a/src/coreclr/src/jit/lsra.h b/src/coreclr/src/jit/lsra.h index c2adcf2..0b1d994 100644 --- a/src/coreclr/src/jit/lsra.h +++ b/src/coreclr/src/jit/lsra.h @@ -371,7 +371,7 @@ class RefInfoListNodePool final public: RefInfoListNodePool(Compiler* compiler, unsigned preallocate = defaultPreallocation); - RefInfoListNode* GetNode(RefPosition* r, GenTree* t, unsigned regIdx = 0); + RefInfoListNode* GetNode(RefPosition* r, GenTree* t); void ReturnNode(RefInfoListNode* listNode); }; @@ -520,7 +520,7 @@ public: // interval to which this register is currently allocated. // If the interval is inactive (isActive == false) then it is not currently live, - // and the register call be unassigned (i.e. setting assignedInterval to nullptr) + // and the register can be unassigned (i.e. setting assignedInterval to nullptr) // without spilling the register. Interval* assignedInterval; // Interval to which this register was previously allocated, and which was unassigned @@ -1494,11 +1494,11 @@ private: // i.e. whose consuming node has not yet been handled. RefInfoListNodePool listNodePool; - // The defList is used for the transient RefInfo that is computed by - // the Build methods, and used in building RefPositions. - // When Def RefPositions are built for a node, their NodeInfo is placed - // in the defList. As the consuming node is handled, it moves the NodeInfo - // into an ordered useList corresponding to the uses for that node. + // When Def RefPositions are built for a node, their RefInfoListNode + // (GenTree* to RefPosition* mapping) is placed in the defList. + // As the consuming node is handled, it removes the RefInfoListNode from the + // defList, use the interval associated with the corresponding Def RefPosition and + // use it to build the Use RefPosition. RefInfoList defList; // As we build uses, we may want to preference the next definition (i.e. the register produced @@ -1925,13 +1925,12 @@ public: GenTree* treeNode; unsigned int bbNum; + LsraLocation nodeLocation; + // Prior to the allocation pass, registerAssignment captures the valid registers - // for this RefPosition. An empty set means that any register is valid. A non-empty - // set means that it must be one of the given registers (may be the full set if the - // only constraint is that it must reside in SOME register) + // for this RefPosition. // After the allocation pass, this contains the actual assignment - LsraLocation nodeLocation; - regMaskTP registerAssignment; + regMaskTP registerAssignment; RefType refType; diff --git a/src/coreclr/src/jit/lsrabuild.cpp b/src/coreclr/src/jit/lsrabuild.cpp index 7b581f2..49e486b 100644 --- a/src/coreclr/src/jit/lsrabuild.cpp +++ b/src/coreclr/src/jit/lsrabuild.cpp @@ -103,15 +103,13 @@ RefInfoListNodePool::RefInfoListNodePool(Compiler* compiler, unsigned preallocat // pool. // // Arguments: -// l - - The `LsraLocation` for the `RefInfo` value. -// i - The interval for the `RefInfo` value. -// t - The IR node for the `RefInfo` value -// regIdx - The register index for the `RefInfo` value. +// r - The `RefPosition` for the `RefInfo` value. +// t - The IR node for the `RefInfo` value // // Returns: // A pooled or newly-allocated `RefInfoListNode`, depending on the // contents of the pool. -RefInfoListNode* RefInfoListNodePool::GetNode(RefPosition* r, GenTree* t, unsigned regIdx) +RefInfoListNode* RefInfoListNodePool::GetNode(RefPosition* r, GenTree* t) { RefInfoListNode* head = m_freeList; if (head == nullptr) @@ -1695,11 +1693,11 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, BasicBlock* block, Lsra #ifdef DEBUG int newDefListCount = defList.Count(); - int produce = newDefListCount - oldDefListCount; + // Currently produce is unused, but need to strengthen an assert to check if produce is + // as expected. See https://github.com/dotnet/runtime/issues/8678 + int produce = newDefListCount - oldDefListCount; assert((consume == 0) || (ComputeAvailableSrcCount(tree) == consume)); -#endif // DEBUG -#ifdef DEBUG // If we are constraining registers, modify all the RefPositions we've just built to specify the // minimum reg count required. if ((getStressLimitRegs() != LSRA_LIMIT_NONE) || (getSelectionHeuristics() != LSRA_SELECT_DEFAULT)) @@ -2095,7 +2093,6 @@ void LinearScan::buildIntervals() { setBlockSequence(); } - curBBNum = blockSequence[bbSeqCount - 1]->bbNum; // Next, create ParamDef RefPositions for all the tracked parameters, in order of their varIndex. // Assign these RefPositions to the (nonexistent) BB0. @@ -2266,7 +2263,7 @@ void LinearScan::buildIntervals() { JITDUMP(" Marking RP #%d of V%02u as spillAfter\n", interval->recentRefPosition->rpNum, interval->varNum); - interval->recentRefPosition->spillAfter; + interval->recentRefPosition->spillAfter = true; } } }