Do not consider InUse registers for assigning to consecutive candidat…

…es (dotnet#95612) * Use multi-reg load/store for EncodeToUtf8 * Do not allocate consecutive register if one of them is in use at current location * Do not use register in spill heuristics if it is already assigned to consecutive register refposition * Revert "Use multi-reg load/store for EncodeToUtf8" This reverts commit ec31dfe. * Make spilling of consecutive register harder --------- Co-authored-by: Swapnil Gaikwad <[email protected]>
xktder · Dec 5, 2023 · 24ee598 · 24ee598
1 parent 471f73a
commit 24ee598
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 17 deletions.
diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp
@@ -3241,13 +3241,15 @@ regNumber LinearScan::assignCopyReg(RefPosition* refPosition)
 
     assert(allocatedReg != REG_NA);
 
+    // restore the related interval
+    currentInterval->relatedInterval = savedRelatedInterval;
+
     INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_COPY_REG, currentInterval, allocatedReg, nullptr, registerScore));
 
     // Now restore the old info
-    currentInterval->relatedInterval = savedRelatedInterval;
-    currentInterval->physReg         = oldPhysReg;
-    currentInterval->assignedReg     = oldRegRecord;
-    currentInterval->isActive        = true;
+    currentInterval->physReg     = oldPhysReg;
+    currentInterval->assignedReg = oldRegRecord;
+    currentInterval->isActive    = true;
 
     return allocatedReg;
 }
@@ -11949,18 +11951,32 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
                 continue;
             }
 
-            if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
-                                                   !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
+            RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
+
+            if (reloadRefPosition != nullptr)
             {
-                // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
-                // is not an actual ref. In those cases, we will reload in future (next) refPosition.
-                // For such cases, consider the spill cost of next refposition.
-                // See notes in "spillInterval()".
-                RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
-                if (reloadRefPosition != nullptr)
+                if ((recentRefPosition != nullptr) &&
+                    (recentRefPosition->RegOptional() &&
+                     !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
                 {
+                    // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
+                    // is not an actual ref. In those cases, we will reload in future (next) refPosition.
+                    // For such cases, consider the spill cost of next refposition.
+                    // See notes in "spillInterval()".
                     currentSpillWeight = linearScan->getWeight(reloadRefPosition);
                 }
+#ifdef TARGET_ARM64
+                else if (assignedInterval->getNextRefPosition()->needsConsecutive)
+                {
+                    // If next refposition is part of consecutive registers and there is already a register
+                    // assigned to it then try not to reassign for currentRefPosition, because with that,
+                    // other registers for the next consecutive register assignment would have to be copied
+                    // to different consecutive registers since this register is busy from this point onwards.
+                    //
+                    // Have it as a candidate, but make its spill cost higher than others.
+                    currentSpillWeight = linearScan->getWeight(reloadRefPosition) * 10;
+                }
+#endif
             }
         }
 #ifdef TARGET_ARM64

diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp
@@ -144,11 +144,14 @@ bool LinearScan::canAssignNextConsecutiveRegisters(RefPosition* firstRefPosition
                 nextRefPosition = getNextConsecutiveRefPosition(nextRefPosition);
             }
 
-            // If regToAssign is not free, check if it is already assigned to the interval corresponding
-            // to the subsequent nextRefPosition. If yes, it would just use regToAssign for that nextRefPosition.
-            if ((nextRefPosition->getInterval() != nullptr) &&
-                (nextRefPosition->getInterval()->assignedReg != nullptr) &&
-                ((nextRefPosition->getInterval()->assignedReg->regNum == regToAssign)))
+            Interval* interval = nextRefPosition->getInterval();
+
+            // If regToAssign is not free, make sure it is not in use at current location.
+            // If not, then check if it is already assigned to the interval corresponding
+            // to the subsequent nextRefPosition.
+            // If yes, it would just use regToAssign for that nextRefPosition.
+            if ((interval != nullptr) && !isRegInUse(regToAssign, interval->registerType) &&
+                (interval->assignedReg != nullptr) && ((interval->assignedReg->regNum == regToAssign)))
             {
                 continue;
             }