JIT: Clean up RefPosition "killed registers" storage (#103560)

Avoid some ifdefs and rename the field to reflect more specifically what it
represents.

Also avoid calling `getWeight` entirely for `RefTypeKill` ref positions, even
for dumping. Dumping this info does not make much sense since there was a
requirement that it was never called in non-dump paths anyway.

Author: jakobbotsch

src/coreclr/jit/lsra.cpp

@@ -178,22 +178,16 @@ void lsraAssignRegToTree(GenTree* tree, regNumber reg, unsigned regIdx)
 //
 // Returns:
 //    Weight of ref position.
-weight_t LinearScan::getWeight(RefPosition* refPos DEBUG_ARG(bool forDump))
+weight_t LinearScan::getWeight(RefPosition* refPos)
 {
+    // RefTypeKill does not have a valid treeNode field, so we do not expect
+    // to see getWeight called for it
+    assert(refPos->refType != RefTypeKill);
+
     weight_t weight;
     GenTree* treeNode = refPos->treeNode;
 
-#ifdef HAS_MORE_THAN_64_REGISTERS
-    // If refType is `RefTypeKill`, we are using the killRegisterAssignment
-    // and treeNode field is garbage.
-    assert(forDump || refPos->refType != RefTypeKill);
-#endif
-
-    if (treeNode != nullptr
-#ifdef DEBUG
-        && (refPos->refType != RefTypeKill)
-#endif
-    )
+    if (treeNode != nullptr)
     {
         if (isCandidateLocalRef(treeNode))
         {
@@ -241,21 +235,6 @@ weight_t LinearScan::getWeight(RefPosition* refPos DEBUG_ARG(bool forDump))
     return weight;
 }
 
-#ifdef DEBUG
-//-------------------------------------------------------------
-// getWeightForDump: Returns the weight of the RefPosition, for dump
-//
-// Arguments:
-//    refPos   -   ref position
-//
-// Returns:
-//    Weight of ref position.
-weight_t LinearScan::getWeightForDump(RefPosition* refPos)
-{
-    return getWeight(refPos, true);
-}
-#endif
-
 // allRegs represents a set of registers that can
 // be used to allocate the specified type in any point
 // in time (more of a 'bank' of registers).
@@ -303,11 +282,7 @@ void LinearScan::updateNextFixedRef(RegRecord* regRecord, RefPosition* nextRefPo
     RefPosition* kill = nextKill;
     while ((kill != nullptr) && (kill->nodeLocation < nextLocation))
     {
-#ifdef HAS_MORE_THAN_64_REGISTERS
-        if (kill->killRegisterAssignment.IsRegNumInMask(regRecord->regNum))
-#else
-        if ((kill->registerAssignment & genSingleTypeRegMask(regRecord->regNum)) != RBM_NONE)
-#endif
+        if (kill->killedRegisters.IsRegNumInMask(regRecord->regNum))
         {
             nextLocation = kill->nodeLocation;
             break;
@@ -4066,7 +4041,7 @@ void LinearScan::processKills(RefPosition* killRefPosition)
 {
     RefPosition* nextKill = killRefPosition->nextRefPosition;
 
-    regMaskTP killedRegs = killRefPosition->getKillRegisterAssignment();
+    regMaskTP killedRegs = killRefPosition->getKilledRegisters();
     while (killedRegs.IsNonEmpty())
     {
         regNumber  killedReg        = genFirstRegNumFromMaskAndToggle(killedRegs);
@@ -4086,9 +4061,9 @@ void LinearScan::processKills(RefPosition* killRefPosition)
         updateNextFixedRef(regRecord, regNextRefPos, nextKill);
     }
 
-    regsBusyUntilKill &= ~killRefPosition->getKillRegisterAssignment();
+    regsBusyUntilKill &= ~killRefPosition->getKilledRegisters();
     INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KILL_REGS, nullptr, REG_NA, nullptr, NONE,
-                                    killRefPosition->getKillRegisterAssignment()));
+                                    killRefPosition->getKilledRegisters()));
 }
 
 //------------------------------------------------------------------------
@@ -10420,36 +10395,34 @@ void RefPosition::dump(LinearScan* linearScan)
 
     printf(" %s ", getRefTypeName(refType));
 
-    if (this->IsPhysRegRef())
+    if (IsPhysRegRef())
     {
-        this->getReg()->tinyDump();
+        getReg()->tinyDump();
     }
     else if (getInterval())
     {
-        this->getInterval()->tinyDump();
+        getInterval()->tinyDump();
     }
-    if ((refType != RefTypeKill) && this->treeNode)
+    if ((refType != RefTypeKill) && treeNode)
     {
         printf("%s", treeNode->OpName(treeNode->OperGet()));
-        if (this->treeNode->IsMultiRegNode())
+        if (treeNode->IsMultiRegNode())
         {
-            printf("[%d]", this->multiRegIdx);
+            printf("[%d]", multiRegIdx);
         }
         printf(" ");
     }
-    printf(FMT_BB " ", this->bbNum);
+    printf(FMT_BB " ", bbNum);
 
     printf("regmask=");
-#ifdef HAS_MORE_THAN_64_REGISTERS
     if (refType == RefTypeKill)
     {
-        linearScan->compiler->dumpRegMask(getKillRegisterAssignment());
+        linearScan->compiler->dumpRegMask(getKilledRegisters());
     }
     else
-#endif // HAS_MORE_THAN_64_REGISTERS
     {
         var_types type = TYP_UNKNOWN;
-        if ((refType == RefTypeBB) || (refType == RefTypeKillGCRefs) || (refType == RefTypeKill))
+        if ((refType == RefTypeBB) || (refType == RefTypeKillGCRefs))
         {
             // These refTypes do not have intervals
             type = TYP_INT;
@@ -10463,57 +10436,61 @@ void RefPosition::dump(LinearScan* linearScan)
 
     printf(" minReg=%d", minRegCandidateCount);
 
-    if (this->lastUse)
+    if (lastUse)
     {
         printf(" last");
     }
-    if (this->reload)
+    if (reload)
     {
         printf(" reload");
     }
-    if (this->spillAfter)
+    if (spillAfter)
     {
         printf(" spillAfter");
     }
-    if (this->singleDefSpill)
+    if (singleDefSpill)
     {
         printf(" singleDefSpill");
     }
-    if (this->writeThru)
+    if (writeThru)
     {
         printf(" writeThru");
     }
-    if (this->moveReg)
+    if (moveReg)
     {
         printf(" move");
     }
-    if (this->copyReg)
+    if (copyReg)
     {
         printf(" copy");
     }
-    if (this->isFixedRegRef)
+    if (isFixedRegRef)
     {
         printf(" fixed");
     }
-    if (this->isLocalDefUse)
+    if (isLocalDefUse)
     {
         printf(" local");
     }
-    if (this->delayRegFree)
+    if (delayRegFree)
     {
         printf(" delay");
     }
-    if (this->outOfOrder)
+    if (outOfOrder)
     {
         printf(" outOfOrder");
     }
 
-    if (this->RegOptional())
+    if (RegOptional())
     {
         printf(" regOptional");
     }
 
-    printf(" wt=%.2f", linearScan->getWeightForDump(this));
+    if (refType != RefTypeKill)
+    {
+        printf(" wt=%.2f", linearScan->getWeight(this));
+    }
+
     printf(">\n");
 }
 
@@ -11116,7 +11093,7 @@ void LinearScan::TupleStyleDump(LsraTupleDumpMode mode)
                                 printf("\n        Kill: ");
                                 killPrinted = true;
                             }
-                            compiler->dumpRegMask(currentRefPosition->getKillRegisterAssignment());
+                            compiler->dumpRegMask(currentRefPosition->getKilledRegisters());
                             printf(" ");
                             break;
                         case RefTypeFixedReg:
@@ -12130,7 +12107,7 @@ void LinearScan::verifyFinalAllocation()
 
             case RefTypeKill:
                 dumpLsraAllocationEvent(LSRA_EVENT_KILL_REGS, nullptr, REG_NA, currentBlock, NONE,
-                                        currentRefPosition.getKillRegisterAssignment());
+                                        currentRefPosition.getKilledRegisters());
                 break;
 
             case RefTypeFixedReg:

src/coreclr/jit/lsra.h

@@ -1185,10 +1185,7 @@ class LinearScan : public LinearScanInterface
 
     void associateRefPosWithInterval(RefPosition* rp);
 
-    weight_t getWeight(RefPosition* refPos DEBUG_ARG(bool forDump = false));
-#ifdef DEBUG
-    weight_t getWeightForDump(RefPosition* refPos);
-#endif // DEBUG
+    weight_t getWeight(RefPosition* refPos);
 
     /*****************************************************************************
      * Register management
@@ -2480,9 +2477,7 @@ class RefPosition
             // After the allocation pass, this contains the actual assignment
             SingleTypeRegSet registerAssignment;
         };
-#ifdef HAS_MORE_THAN_64_REGISTERS
-        regMaskTP killRegisterAssignment;
-#endif
+        regMaskTP killedRegisters;
     };
     unsigned int bbNum;
 
@@ -2665,14 +2660,10 @@ class RefPosition
         return referent->registerType;
     }
 
-    regMaskTP getKillRegisterAssignment()
+    regMaskTP getKilledRegisters()
     {
         assert(refType == RefTypeKill);
-#ifdef HAS_MORE_THAN_64_REGISTERS
-        return killRegisterAssignment;
-#else
-        return registerAssignment;
-#endif
+        return killedRegisters;
     }
 
     // Returns true if it is a reference on a GenTree node.

src/coreclr/jit/lsrabuild.cpp

@@ -637,16 +637,14 @@ RefPosition* LinearScan::newRefPosition(Interval*        theInterval,
         theInterval->isSingleDef = theInterval->firstRefPosition == newRP;
     }
 #ifdef DEBUG
-#ifdef HAS_MORE_THAN_64_REGISTERS
     // Need to do this here do the dump can print the mask correctly.
     // Doing in DEBUG so we do not incur of cost of this check for
     // every RefPosition. We will set this anyway in addKillForRegs()
     // in RELEASE.
     if (theRefType == RefTypeKill)
     {
-        newRP->killRegisterAssignment = mask;
+        newRP->killedRegisters = mask;
     }
-#endif // HAS_MORE_THAN_64_REGISTERS
 #endif
     DBEXEC(VERBOSE, newRP->dump(this));
     return newRP;
@@ -710,9 +708,7 @@ void LinearScan::addKillForRegs(regMaskTP mask, LsraLocation currentLoc)
 
     RefPosition* pos = newRefPosition((Interval*)nullptr, currentLoc, RefTypeKill, nullptr, mask.getLow());
 
-#ifdef HAS_MORE_THAN_64_REGISTERS
-    pos->killRegisterAssignment = mask;
-#endif
+    pos->killedRegisters = mask;
 
     *killTail = pos;
     killTail  = &pos->nextRefPosition;