Do not set pre-decided consecutive registers as busy (again) (#85566)
authorKunal Pathak <Kunal.Pathak@microsoft.com>
Mon, 1 May 2023 12:29:50 +0000 (05:29 -0700)
committerGitHub <noreply@github.com>
Mon, 1 May 2023 12:29:50 +0000 (05:29 -0700)
src/coreclr/jit/lsra.cpp
src/coreclr/jit/lsra.h
src/coreclr/jit/lsraarm64.cpp

index cf79a6e..d803ae8 100644 (file)
@@ -4818,6 +4818,12 @@ void LinearScan::allocateRegisters()
             copyRegsToFree        = RBM_NONE;
             regsInUseThisLocation = regsInUseNextLocation;
             regsInUseNextLocation = RBM_NONE;
+#ifdef TARGET_ARM64
+            if (hasConsecutiveRegister)
+            {
+                consecutiveRegsInUseThisLocation = RBM_NONE;
+            }
+#endif
             if ((regsToFree | delayRegsToFree) != RBM_NONE)
             {
                 freeRegisters(regsToFree);
@@ -5433,6 +5439,7 @@ void LinearScan::allocateRegisters()
                         // It doesn't satisfy, so do a copyReg for the first RefPosition to such a register, so
                         // it would be possible to allocate consecutive registers to the subsequent RefPositions.
                         regNumber copyReg = assignCopyReg<true>(&currentRefPosition);
+                        assignConsecutiveRegisters(&currentRefPosition, copyReg);
 
                         if (copyReg != assignedRegister)
                         {
@@ -5440,6 +5447,16 @@ void LinearScan::allocateRegisters()
                             regMaskTP copyRegMask     = getRegMask(copyReg, currentInterval->registerType);
                             regMaskTP assignedRegMask = getRegMask(assignedRegister, currentInterval->registerType);
 
+                            if ((consecutiveRegsInUseThisLocation & assignedRegMask) != RBM_NONE)
+                            {
+                                // If assigned register is one of the consecutive register we are about to assign
+                                // to the subsequent RefPositions, do not mark it busy otherwise when we allocate
+                                // for that particular subsequent RefPosition, we will not find any candidates
+                                // available. Not marking it busy should be fine because we have already set the
+                                // register assignments for all the consecutive refpositions.
+                                assignedRegMask = RBM_NONE;
+                            }
+
                             // For consecutive register, although it shouldn't matter what the assigned register was,
                             // because we have just assigned it `copyReg` and that's the one in-use, and not the
                             // one that was assigned previously. However, in situation where an upper-vector restore
@@ -5480,7 +5497,6 @@ void LinearScan::allocateRegisters()
                             currentRefPosition.registerAssignment = assignedRegBit;
                         }
 
-                        assignConsecutiveRegisters(&currentRefPosition, copyReg);
                         continue;
                     }
                 }
@@ -5538,6 +5554,16 @@ void LinearScan::allocateRegisters()
                             // registers.
                             assignConsecutiveRegisters(&currentRefPosition, copyReg);
                         }
+
+                        if ((consecutiveRegsInUseThisLocation & assignedRegMask) != RBM_NONE)
+                        {
+                            // If assigned register is one of the consecutive register we are about to assign
+                            // to the subsequent RefPositions, do not mark it busy otherwise when we allocate
+                            // for that particular subsequent RefPosition, we will not find any candidates
+                            // available. Not marking it busy should be fine because we have already set the
+                            // register assignments for all the consecutive refpositions.
+                            assignedRegMask = RBM_NONE;
+                        }
                     }
 #endif
                     // For consecutive register, although it shouldn't matter what the assigned register was,
index f9de491..d869f8f 100644 (file)
@@ -1839,6 +1839,9 @@ private:
     regMaskTP regsBusyUntilKill;
     regMaskTP regsInUseThisLocation;
     regMaskTP regsInUseNextLocation;
+#ifdef TARGET_ARM64
+    regMaskTP consecutiveRegsInUseThisLocation;
+#endif
     bool isRegBusy(regNumber reg, var_types regType)
     {
         regMaskTP regMask = getRegMask(reg, regType);
index c8c6d96..40513da 100644 (file)
@@ -67,6 +67,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
     assert(firstRefPosition->assignedReg() == firstRegAssigned);
     assert(firstRefPosition->isFirstRefPositionOfConsecutiveRegisters());
     assert(emitter::isVectorRegister(firstRegAssigned));
+    assert(consecutiveRegsInUseThisLocation == RBM_NONE);
 
     RefPosition* consecutiveRefPosition = getNextConsecutiveRefPosition(firstRefPosition);
     regNumber    regToAssign            = firstRegAssigned == REG_FP_LAST ? REG_FP_FIRST : REG_NEXT(firstRegAssigned);
@@ -75,7 +76,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
     assert(firstRefPosition->refType != RefTypeUpperVectorRestore);
 
     INDEBUG(int refPosCount = 1);
-    regMaskTP busyConsecutiveRegMask = (((1ULL << firstRefPosition->regCount) - 1) << firstRegAssigned);
+    consecutiveRegsInUseThisLocation = (((1ULL << firstRefPosition->regCount) - 1) << firstRegAssigned);
 
     while (consecutiveRefPosition != nullptr)
     {
@@ -95,7 +96,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
                 // RefTypeUpperVectorRestore positions of corresponding variables for which (another criteria)
                 // we are trying to find consecutive registers.
 
-                consecutiveRefPosition->registerAssignment &= ~busyConsecutiveRegMask;
+                consecutiveRefPosition->registerAssignment &= ~consecutiveRegsInUseThisLocation;
             }
             consecutiveRefPosition = getNextConsecutiveRefPosition(consecutiveRefPosition);
         }