Refactor register selection heuristics
authorCarol Eidt <carol.eidt@microsoft.com>
Tue, 17 Oct 2017 02:06:01 +0000 (19:06 -0700)
committerCarol Eidt <carol.eidt@microsoft.com>
Tue, 17 Oct 2017 02:06:01 +0000 (19:06 -0700)
This is in preparation for tuning these for both throughput and code quality.

src/jit/lsra.cpp
src/jit/lsra.h

index 3fa7905..451dbbe 100644 (file)
@@ -5339,6 +5339,68 @@ RegisterType LinearScan::getRegisterType(Interval* currentInterval, RefPosition*
 }
 
 //------------------------------------------------------------------------
+// isMatchingConstant: Check to see whether a given register contains the constant referenced
+//                     by the given RefPosition
+//
+// Arguments:
+//    physRegRecord:   The RegRecord for the register we're interested in.
+//    refPosition:     The RefPosition for a constant interval.
+//
+// Return Value:
+//    True iff the register was defined by an identical constant node as the current interval.
+//
+bool LinearScan::isMatchingConstant(RegRecord* physRegRecord, RefPosition* refPosition)
+{
+    if ((physRegRecord->assignedInterval == nullptr) || !physRegRecord->assignedInterval->isConstant)
+    {
+        return false;
+    }
+    noway_assert(refPosition->treeNode != nullptr);
+    GenTree* otherTreeNode = physRegRecord->assignedInterval->firstRefPosition->treeNode;
+    noway_assert(otherTreeNode != nullptr);
+
+    if (refPosition->treeNode->OperGet() == otherTreeNode->OperGet())
+    {
+        switch (otherTreeNode->OperGet())
+        {
+            case GT_CNS_INT:
+                if ((refPosition->treeNode->AsIntCon()->IconValue() == otherTreeNode->AsIntCon()->IconValue()) &&
+                    (varTypeGCtype(refPosition->treeNode) == varTypeGCtype(otherTreeNode)))
+                {
+#ifdef _TARGET_64BIT_
+                    // If the constant is negative, only reuse registers of the same type.
+                    // This is because, on a 64-bit system, we do not sign-extend immediates in registers to
+                    // 64-bits unless they are actually longs, as this requires a longer instruction.
+                    // This doesn't apply to a 32-bit system, on which long values occupy multiple registers.
+                    // (We could sign-extend, but we would have to always sign-extend, because if we reuse more
+                    // than once, we won't have access to the instruction that originally defines the constant).
+                    if ((refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()) ||
+                        (refPosition->treeNode->AsIntCon()->IconValue() >= 0))
+#endif // _TARGET_64BIT_
+                    {
+                        return true;
+                    }
+                }
+                break;
+            case GT_CNS_DBL:
+            {
+                // For floating point constants, the values must be identical, not simply compare
+                // equal.  So we compare the bits.
+                if (refPosition->treeNode->AsDblCon()->isBitwiseEqual(otherTreeNode->AsDblCon()) &&
+                    (refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()))
+                {
+                    return true;
+                }
+                break;
+            }
+            default:
+                break;
+        }
+    }
+    return false;
+}
+
+//------------------------------------------------------------------------
 // tryAllocateFreeReg: Find a free register that satisfies the requirements for refPosition,
 //                     and takes into account the preferences for the given Interval
 //
@@ -5428,7 +5490,6 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
 #ifdef DEBUG
     candidates = stressLimitRegs(refPosition, candidates);
 #endif
-    bool mustAssignARegister = true;
     assert(candidates != RBM_NONE);
 
     // If the related interval has no further references, it is possible that it is a source of the
@@ -5551,6 +5612,9 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
         }
     }
 
+    //-------------------------------------------------------------------------
+    // Register Selection
+
     RegRecord* availablePhysRegInterval = nullptr;
     bool       unassignInterval         = false;
 
@@ -5651,52 +5715,9 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
 
         // If this is a definition of a constant interval, check to see if its value is already in this register.
         if (currentInterval->isConstant && RefTypeIsDef(refPosition->refType) &&
-            (physRegRecord->assignedInterval != nullptr) && physRegRecord->assignedInterval->isConstant)
+            isMatchingConstant(physRegRecord, refPosition))
         {
-            noway_assert(refPosition->treeNode != nullptr);
-            GenTree* otherTreeNode = physRegRecord->assignedInterval->firstRefPosition->treeNode;
-            noway_assert(otherTreeNode != nullptr);
-
-            if (refPosition->treeNode->OperGet() == otherTreeNode->OperGet())
-            {
-                switch (otherTreeNode->OperGet())
-                {
-                    case GT_CNS_INT:
-                        if ((refPosition->treeNode->AsIntCon()->IconValue() ==
-                             otherTreeNode->AsIntCon()->IconValue()) &&
-                            (varTypeGCtype(refPosition->treeNode) == varTypeGCtype(otherTreeNode)))
-                        {
-#ifdef _TARGET_64BIT_
-                            // If the constant is negative, only reuse registers of the same type.
-                            // This is because, on a 64-bit system, we do not sign-extend immediates in registers to
-                            // 64-bits unless they are actually longs, as this requires a longer instruction.
-                            // This doesn't apply to a 32-bit system, on which long values occupy multiple registers.
-                            // (We could sign-extend, but we would have to always sign-extend, because if we reuse more
-                            // than once, we won't have access to the instruction that originally defines the constant).
-                            if ((refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()) ||
-                                (refPosition->treeNode->AsIntCon()->IconValue() >= 0))
-#endif // _TARGET_64BIT_
-                            {
-                                score |= VALUE_AVAILABLE;
-                            }
-                        }
-                        break;
-                    case GT_CNS_DBL:
-                    {
-                        // For floating point constants, the values must be identical, not simply compare
-                        // equal.  So we compare the bits.
-                        if (refPosition->treeNode->AsDblCon()->isBitwiseEqual(otherTreeNode->AsDblCon()) &&
-                            (refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()))
-                        {
-                            score |= VALUE_AVAILABLE;
-                        }
-                        break;
-                    }
-                    default:
-                        // for all other 'otherTreeNode->OperGet()' kinds, we leave 'score' unchanged
-                        break;
-                }
-            }
+            score |= VALUE_AVAILABLE;
         }
 
         // If the nextPhysRefLocation is a fixedRef for the rangeEndRefPosition, increment it so that
@@ -5851,10 +5872,7 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
 //
 // Note: This helper is designed to be used only from allocateBusyReg() and canSpillDoubleReg()
 //
-bool LinearScan::canSpillReg(RegRecord*   physRegRecord,
-                             LsraLocation refLocation,
-                             unsigned*    recentAssignedRefWeight,
-                             unsigned     farthestRefPosWeight)
+bool LinearScan::canSpillReg(RegRecord* physRegRecord, LsraLocation refLocation, unsigned* recentAssignedRefWeight)
 {
     assert(physRegRecord->assignedInterval != nullptr);
     RefPosition* recentAssignedRef = physRegRecord->assignedInterval->recentRefPosition;
@@ -5879,45 +5897,61 @@ bool LinearScan::canSpillReg(RegRecord*   physRegRecord,
         // of the spill candidate found so far.  We would consider spilling a greater weight
         // ref position only if the refPosition being allocated must need a reg.
         *recentAssignedRefWeight = getWeight(recentAssignedRef);
-        if (*recentAssignedRefWeight > farthestRefPosWeight)
-        {
-            return false;
-        }
     }
     return true;
 }
 
 #ifdef _TARGET_ARM_
+//------------------------------------------------------------------------
+// canSpillDoubleReg: Determine whether we can spill physRegRecord
+//
+// Arguments:
+//    physRegRecord             - reg to spill (must be a valid double register)
+//    refLocation               - Location of RefPosition where this register will be spilled
+//    recentAssignedRefWeight   - Weight of recent assigned RefPosition which will be determined in this function
+//
+// Return Value:
+//    True  - if we can spill physRegRecord
+//    False - otherwise
+//
+// Notes:
+//    This helper is designed to be used only from allocateBusyReg() and canSpillDoubleReg().
+//    The recentAssignedRefWeight is not updated if either register cannot be spilled.
+//
 bool LinearScan::canSpillDoubleReg(RegRecord*   physRegRecord,
                                    LsraLocation refLocation,
-                                   unsigned*    recentAssignedRefWeight,
-                                   unsigned     farthestRefPosWeight)
+                                   unsigned*    recentAssignedRefWeight)
 {
+    assert(genIsValidDoubleReg(physRegRecord->regNum));
     bool     retVal  = true;
     unsigned weight  = BB_ZERO_WEIGHT;
     unsigned weight2 = BB_ZERO_WEIGHT;
 
     RegRecord* physRegRecord2 = findAnotherHalfRegRec(physRegRecord);
 
-    if (physRegRecord->assignedInterval != nullptr)
-        retVal &= canSpillReg(physRegRecord, refLocation, &weight, farthestRefPosWeight);
-
+    if ((physRegRecord->assignedInterval != nullptr) && !canSpillReg(physRegRecord, refLocation, &weight))
+    {
+        return false;
+    }
     if (physRegRecord2->assignedInterval != nullptr)
-        retVal &= canSpillReg(physRegRecord2, refLocation, &weight2, farthestRefPosWeight);
-
-    if (!(weight == BB_ZERO_WEIGHT && weight2 == BB_ZERO_WEIGHT))
     {
-        // weight and/or weight2 have been updated.
-        *recentAssignedRefWeight = (weight > weight2) ? weight : weight2;
+        if (!canSpillReg(physRegRecord2, refLocation, &weight2))
+        {
+            return false;
+        }
+        if (weight2 > weight)
+        {
+            weight = weight2;
+        }
     }
-
-    return retVal;
+    *recentAssignedRefWeight = weight;
+    return true;
 }
 #endif
 
 //----------------------------------------------------------------------------
-// checkActiveInterval: Test activness of an interval
-//                      and check assertions if the interval is not active
+// isIntervalActiveHelper: Test activness of an interval
+//                          and check assertions if the interval is not active
 //
 // Arguments:
 //    interval    - An interval to be tested
@@ -5927,11 +5961,15 @@ bool LinearScan::canSpillDoubleReg(RegRecord*   physRegRecord,
 //    True - iff the interval is active
 //    False - otherwise
 //
-// Note: This helper is designed to be used only from checkActiveIntervals()
+// Note: This helper is designed to be used only from isIntervalActive()
 //
-bool LinearScan::checkActiveInterval(Interval* interval, LsraLocation refLocation)
+bool LinearScan::isIntervalActiveHelper(Interval* interval, LsraLocation refLocation)
 {
-    if (!interval->isActive)
+    if (interval->isActive)
+    {
+        return true;
+    }
+    else
     {
         RefPosition* recentAssignedRef = interval->recentRefPosition;
         // Note that we may or may not have actually handled the reference yet, so it could either
@@ -5968,14 +6006,10 @@ bool LinearScan::checkActiveInterval(Interval* interval, LsraLocation refLocatio
         }
         return false;
     }
-    return true;
 }
 
 //----------------------------------------------------------------------------------------
-// checkActiveIntervals: Test activness of a interval assinged to a register
-//                       and check assertions if the interval is not active.
-//                       We look into all intervals of two float registers consisting
-//                       a double regsiter for ARM32.
+// isIntervalActive: Determine if an Interval is active
 //
 // Arguments:
 //    physRegRecord - A register
@@ -5986,9 +6020,12 @@ bool LinearScan::checkActiveInterval(Interval* interval, LsraLocation refLocatio
 //    True - iff the all intervals are active
 //    False - otherwise
 //
-// Note: This helper is designed to be used only from allocateBusyReg()
+// Notes:
+//    This method checks assertions if the interval is not active.
+//    For a double register on ARM32, we check both of its floating point registers.
+//    This helper is designed to be used only from allocateBusyReg().
 //
-bool LinearScan::checkActiveIntervals(RegRecord* physRegRecord, LsraLocation refLocation, RegisterType registerType)
+bool LinearScan::isIntervalActive(RegRecord* physRegRecord, LsraLocation refLocation, RegisterType registerType)
 {
     Interval* assignedInterval = physRegRecord->assignedInterval;
 
@@ -6001,15 +6038,15 @@ bool LinearScan::checkActiveIntervals(RegRecord* physRegRecord, LsraLocation ref
     // Both intervals should not be nullptr at the same time, becasue we already handle this case before.
     assert(!(assignedInterval == nullptr && assignedInterval2 == nullptr));
 
-    if (assignedInterval != nullptr && !checkActiveInterval(assignedInterval, refLocation))
+    if (assignedInterval != nullptr && !isIntervalActiveHelper(assignedInterval, refLocation))
         return false;
 
-    if (assignedInterval2 != nullptr && !checkActiveInterval(assignedInterval2, refLocation))
+    if (assignedInterval2 != nullptr && !isIntervalActiveHelper(assignedInterval2, refLocation))
         return false;
 
     return true;
 #else
-    return checkActiveInterval(assignedInterval, refLocation);
+    return isIntervalActiveHelper(assignedInterval, refLocation);
 #endif
 }
 
@@ -6072,23 +6109,26 @@ void LinearScan::unassignDoublePhysReg(RegRecord* doubleRegRecord)
 //    True - if regRec is beding used
 //    False - otherwise
 //
-// Note: This helper is designed to be used only from allocateBusyReg()
+// Notes:
+//    This helper is designed to be used only from allocateBusyReg().
+//    The caller must have already checked for the case where 'refPosition' is a fixed ref.
 //
-bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition, LsraLocation* nextLocation)
+bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition)
 {
+    // We shouldn't reach this check if 'refPosition' is a FixedReg of this register.
+    assert(!refPosition->isFixedRefOfReg(regRec->regNum));
     Interval* assignedInterval = regRec->assignedInterval;
     if (assignedInterval != nullptr)
     {
-        LsraLocation refLocation     = refPosition->nodeLocation;
-        RefPosition* nextRefPosition = assignedInterval->getNextRefPosition();
-        *nextLocation                = assignedInterval->getNextRefLocation();
+        RefPosition* nextAssignedRef = assignedInterval->getNextRefPosition();
 
         // We should never spill a register that's occupied by an Interval with its next use at the current
         // location.
         // Normally this won't occur (unless we actually had more uses in a single node than there are registers),
         // because we'll always find something with a later nextLocation, but it can happen in stress when
         // we have LSRA_SELECT_NEAREST.
-        if ((*nextLocation == refLocation) && !refPosition->isFixedRegRef && nextRefPosition->RequiresRegister())
+        if ((nextAssignedRef != nullptr) && (nextAssignedRef->nodeLocation == refPosition->nodeLocation) &&
+            nextAssignedRef->RequiresRegister())
         {
             return true;
         }
@@ -6097,6 +6137,117 @@ bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition, LsraLoc
 }
 
 //------------------------------------------------------------------------
+// isSpillCandidate: Determine if a register is a spill candidate for a given RefPosition.
+//
+// Arguments:
+//    current               The interval for the current allocation
+//    refPosition           The RefPosition of the current Interval for which a register is being allocated
+//    physRegRecord         The RegRecord for the register we're considering for spill
+//    nextLocation          An out (reference) parameter in which the next use location of the
+//                          given RegRecord will be returned.
+//
+// Return Value:
+//    True iff the given register can be spilled to accommodate the given RefPosition.
+//
+bool LinearScan::isSpillCandidate(Interval*     current,
+                                  RefPosition*  refPosition,
+                                  RegRecord*    physRegRecord,
+                                  LsraLocation& nextLocation)
+{
+    regMaskTP    candidateBit = genRegMask(physRegRecord->regNum);
+    LsraLocation refLocation  = refPosition->nodeLocation;
+    if (physRegRecord->isBusyUntilNextKill)
+    {
+        return false;
+    }
+    Interval* assignedInterval = physRegRecord->assignedInterval;
+    if (assignedInterval != nullptr)
+    {
+        nextLocation = assignedInterval->getNextRefLocation();
+    }
+#ifdef _TARGET_ARM_
+    RegRecord* physRegRecord2    = nullptr;
+    Interval*  assignedInterval2 = nullptr;
+
+    // For ARM32, a double occupies a consecutive even/odd pair of float registers.
+    if (current->registerType == TYP_DOUBLE)
+    {
+        assert(genIsValidDoubleReg(physRegRecord->regNum));
+        physRegRecord2 = findAnotherHalfRegRec(physRegRecord);
+        if (physRegRecord2->isBusyUntilNextKill)
+        {
+            return false;
+        }
+        assignedInterval2 = physRegRecord2->assignedInterval;
+        if ((assignedInterval2 != nullptr) && (assignedInterval2->getNextRefLocation() > nextLocation))
+        {
+            nextLocation = assignedInterval2->getNextRefLocation();
+        }
+    }
+#endif
+
+    // If there is a fixed reference at the same location (and it's not due to this reference),
+    // don't use it.
+    if (physRegRecord->conflictingFixedRegReference(refPosition))
+    {
+        return false;
+    }
+
+    if (refPosition->isFixedRefOfRegMask(candidateBit))
+    {
+        // Either there is a fixed reference due to this node, or one associated with a
+        // fixed use fed by a def at this node.
+        // In either case, we must use this register as it's the only candidate
+        // TODO-CQ: At the time we allocate a register to a fixed-reg def, if it's not going
+        // to remain live until the use, we should set the candidates to allRegs(regType)
+        // to avoid a spill - codegen can then insert the copy.
+        // If this is marked as allocateIfProfitable, the caller will compare the weights
+        // of this RefPosition and the RefPosition to which it is currently assigned.
+        assert(refPosition->isFixedRegRef ||
+               (refPosition->nextRefPosition != nullptr && refPosition->nextRefPosition->isFixedRegRef));
+        return true;
+    }
+
+    // If this register is not assigned to an interval, either
+    // - it has a FixedReg reference at the current location that is not this reference, OR
+    // - this is the special case of a fixed loReg, where this interval has a use at the same location
+    // In either case, we cannot use it
+    CLANG_FORMAT_COMMENT_ANCHOR;
+
+#ifdef _TARGET_ARM_
+    if (assignedInterval == nullptr && assignedInterval2 == nullptr)
+#else
+    if (assignedInterval == nullptr)
+#endif
+    {
+        RefPosition* nextPhysRegPosition = physRegRecord->getNextRefPosition();
+        assert(nextPhysRegPosition->nodeLocation == refLocation && candidateBit != refPosition->registerAssignment);
+        return false;
+    }
+
+    if (!isIntervalActive(physRegRecord, refLocation, current->registerType))
+    {
+        return false;
+    }
+
+    if (isRegInUse(physRegRecord, refPosition))
+    {
+        return false;
+    }
+
+#ifdef _TARGET_ARM_
+    if (current->registerType == TYP_DOUBLE)
+    {
+        if (isRegInUse(physRegRecord2, refPosition))
+        {
+            return false;
+        }
+    }
+#endif
+    return true;
+}
+
+//------------------------------------------------------------------------
 // allocateBusyReg: Find a busy register that satisfies the requirements for refPosition,
 //                  and that can be spilled.
 //
@@ -6121,6 +6272,7 @@ bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition, LsraLoc
 //        The ref position chosen for spilling will not only be lowest weight
 //        of all but also has a weight lower than 'refPosition'.  If there is
 //        no such ref position, reg will not be allocated.
+//
 regNumber LinearScan::allocateBusyReg(Interval* current, RefPosition* refPosition, bool allocateIfProfitable)
 {
     regNumber foundReg = REG_NA;
@@ -6178,148 +6330,50 @@ regNumber LinearScan::allocateBusyReg(Interval* current, RefPosition* refPositio
         {
             continue;
         }
-        RegRecord* physRegRecord = getRegisterRecord(regNum);
-#ifdef _TARGET_ARM_
-        RegRecord* physRegRecord2 = nullptr;
-        // For ARM32, let's consider two float registers consisting a double reg together,
-        // when allocaing a double register.
-        if (current->registerType == TYP_DOUBLE)
-        {
-            assert(genIsValidDoubleReg(regNum));
-            physRegRecord2 = findAnotherHalfRegRec(physRegRecord);
-        }
-#endif
-
-        if (physRegRecord->isBusyUntilNextKill)
-        {
-            continue;
-        }
-        Interval* assignedInterval = physRegRecord->assignedInterval;
-#ifdef _TARGET_ARM_
-        Interval* assignedInterval2 = (physRegRecord2 == nullptr) ? nullptr : physRegRecord2->assignedInterval;
-#endif
-
-        // If there is a fixed reference at the same location (and it's not due to this reference),
-        // don't use it.
-
-        if (physRegRecord->conflictingFixedRegReference(refPosition))
+        RegRecord*   physRegRecord  = getRegisterRecord(regNum);
+        RegRecord*   physRegRecord2 = nullptr; // only used for _TARGET_ARM_
+        LsraLocation nextLocation   = MinLocation;
+        LsraLocation physRegNextLocation;
+        if (!isSpillCandidate(current, refPosition, physRegRecord, nextLocation))
         {
             assert(candidates != candidateBit);
             continue;
         }
 
-        LsraLocation physRegNextLocation = MaxLocation;
-        if (refPosition->isFixedRefOfRegMask(candidateBit))
+        // We've passed the preliminary checks for a spill candidate.
+        // Now, if we have a recentAssignedRef, check that it is going to be OK to spill it.
+        Interval*    assignedInterval        = physRegRecord->assignedInterval;
+        unsigned     recentAssignedRefWeight = BB_ZERO_WEIGHT;
+        RefPosition* recentAssignedRef;
+        RefPosition* recentAssignedRef2 = nullptr;
+#ifdef _TARGET_ARM_
+        if (current->registerType == TYP_DOUBLE)
         {
-            // Either there is a fixed reference due to this node, or one associated with a
-            // fixed use fed by a def at this node.
-            // In either case, we must use this register as it's the only candidate
-            // TODO-CQ: At the time we allocate a register to a fixed-reg def, if it's not going
-            // to remain live until the use, we should set the candidates to allRegs(regType)
-            // to avoid a spill - codegen can then insert the copy.
-            assert(candidates == candidateBit);
-
-            // If a refPosition has a fixed reg as its candidate and is also marked
-            // as allocateIfProfitable, we should allocate fixed reg only if the
-            // weight of this ref position is greater than the weight of the ref
-            // position to which fixed reg is assigned.  Such a case would arise
-            // on x86 under LSRA stress.
-            if (!allocateIfProfitable)
+            recentAssignedRef           = (assignedInterval == nullptr) ? nullptr : assignedInterval->recentRefPosition;
+            physRegRecord2              = findAnotherHalfRegRec(physRegRecord);
+            Interval* assignedInterval2 = physRegRecord2->assignedInterval;
+            recentAssignedRef2 = (assignedInterval2 == nullptr) ? nullptr : assignedInterval2->recentRefPosition;
+            if (!canSpillDoubleReg(physRegRecord, refLocation, &recentAssignedRefWeight))
             {
-                physRegNextLocation  = MaxLocation;
-                farthestRefPosWeight = BB_MAX_WEIGHT;
+                continue;
             }
         }
         else
+#else
         {
-            physRegNextLocation = physRegRecord->getNextRefLocation();
-
-            // If refPosition requires a fixed register, we should reject all others.
-            // Otherwise, we will still evaluate all phyRegs though their next location is
-            // not better than farthestLocation found so far.
-            //
-            // TODO: this method should be using an approach similar to tryAllocateFreeReg()
-            // where it uses a regOrder array to avoid iterating over any but the single
-            // fixed candidate.
-            if (refPosition->isFixedRegRef && physRegNextLocation < farthestLocation)
+            recentAssignedRef = assignedInterval->recentRefPosition;
+            if (!canSpillReg(physRegRecord, refLocation, &recentAssignedRefWeight))
             {
                 continue;
             }
         }
-
-        // If this register is not assigned to an interval, either
-        // - it has a FixedReg reference at the current location that is not this reference, OR
-        // - this is the special case of a fixed loReg, where this interval has a use at the same location
-        // In either case, we cannot use it
-        CLANG_FORMAT_COMMENT_ANCHOR;
-
-#ifdef _TARGET_ARM_
-        if (assignedInterval == nullptr && assignedInterval2 == nullptr)
-#else
-        if (assignedInterval == nullptr)
 #endif
+            if (recentAssignedRefWeight > farthestRefPosWeight)
         {
-            RefPosition* nextPhysRegPosition = physRegRecord->getNextRefPosition();
-
-#ifndef _TARGET_ARM64_
-            // TODO-Cleanup: Revisit this after Issue #3524 is complete
-            // On ARM64 the nodeLocation is not always == refLocation, Disabling this assert for now.
-            assert(nextPhysRegPosition->nodeLocation == refLocation && candidateBit != candidates);
-#endif
             continue;
         }
 
-#ifdef _TARGET_ARM_
-        RefPosition* recentAssignedRef = (assignedInterval == nullptr) ? nullptr : assignedInterval->recentRefPosition;
-        RefPosition* recentAssignedRef2 =
-            (assignedInterval2 == nullptr) ? nullptr : assignedInterval2->recentRefPosition;
-#else
-        RefPosition* recentAssignedRef = assignedInterval->recentRefPosition;
-#endif
-
-        if (!checkActiveIntervals(physRegRecord, refLocation, current->registerType))
-        {
-            continue;
-        }
-
-        // If we have a recentAssignedRef, check that it is going to be OK to spill it
-        //
-        // TODO-Review: Under what conditions recentAssginedRef would be null?
-        unsigned recentAssignedRefWeight = BB_ZERO_WEIGHT;
-
-#ifdef _TARGET_ARM_
-        if (current->registerType == TYP_DOUBLE)
-        {
-            if (!canSpillDoubleReg(physRegRecord, refLocation, &recentAssignedRefWeight, farthestRefPosWeight))
-                continue;
-        }
-        else
-#endif
-            // This if-stmt is associated with the above else
-            if (!canSpillReg(physRegRecord, refLocation, &recentAssignedRefWeight, farthestRefPosWeight))
-        {
-            continue;
-        }
-
-        LsraLocation nextLocation = MinLocation;
-
-        if (isRegInUse(physRegRecord, refPosition, &nextLocation))
-        {
-            continue;
-        }
-
-#ifdef _TARGET_ARM_
-        if (current->registerType == TYP_DOUBLE)
-        {
-            LsraLocation nextLocation2 = MinLocation;
-            if (isRegInUse(physRegRecord2, refPosition, &nextLocation2))
-            {
-                continue;
-            }
-            nextLocation = (nextLocation > nextLocation2) ? nextLocation : nextLocation2;
-        }
-#endif
-
+        physRegNextLocation = physRegRecord->getNextRefLocation();
         if (nextLocation > physRegNextLocation)
         {
             nextLocation = physRegNextLocation;
@@ -6382,7 +6436,7 @@ regNumber LinearScan::allocateBusyReg(Interval* current, RefPosition* refPositio
                     if (recentAssignedRef2 != nullptr)
                         isBetterLocation &= (recentAssignedRef2->reload && recentAssignedRef2->AllocateIfProfitable());
 #else
-                    isBetterLocation   = (recentAssignedRef != nullptr) && recentAssignedRef->reload &&
+                    isBetterLocation = (recentAssignedRef != nullptr) && recentAssignedRef->reload &&
                                        recentAssignedRef->AllocateIfProfitable();
 #endif
                 }
@@ -6449,7 +6503,8 @@ regNumber LinearScan::allocateBusyReg(Interval* current, RefPosition* refPositio
         }
         else
         {
-            assert(farthestLocation > refLocation || refPosition->isFixedRegRef);
+            assert(farthestLocation > refLocation || refPosition->isFixedRegRef ||
+                   (refPosition->nextRefPosition != nullptr && refPosition->nextRefPosition->isFixedRegRef));
         }
     }
 #endif // DEBUG
index d149b32..7cfb7ee 100644 (file)
@@ -699,23 +699,17 @@ private:
 #ifdef _TARGET_ARM_
     bool isSecondHalfReg(RegRecord* regRec, Interval* interval);
     RegRecord* findAnotherHalfRegRec(RegRecord* regRec);
-    bool canSpillDoubleReg(RegRecord*   physRegRecord,
-                           LsraLocation refLocation,
-                           unsigned*    recentAssignedRefWeight,
-                           unsigned     farthestRefPosWeight);
+    bool canSpillDoubleReg(RegRecord* physRegRecord, LsraLocation refLocation, unsigned* recentAssignedRefWeight);
     void unassignDoublePhysReg(RegRecord* doubleRegRecord);
 #endif
     void updateAssignedInterval(RegRecord* reg, Interval* interval, RegisterType regType);
     void updatePreviousInterval(RegRecord* reg, Interval* interval, RegisterType regType);
     bool canRestorePreviousInterval(RegRecord* regRec, Interval* assignedInterval);
     bool isAssignedToInterval(Interval* interval, RegRecord* regRec);
-    bool checkActiveInterval(Interval* interval, LsraLocation refLocation);
-    bool checkActiveIntervals(RegRecord* physRegRecord, LsraLocation refLocation, RegisterType registerType);
-    bool canSpillReg(RegRecord*   physRegRecord,
-                     LsraLocation refLocation,
-                     unsigned*    recentAssignedRefWeight,
-                     unsigned     farthestRefPosWeight);
-    bool isRegInUse(RegRecord* regRec, RefPosition* refPosition, LsraLocation* nextLocation);
+    bool isIntervalActiveHelper(Interval* interval, LsraLocation refLocation);
+    bool isIntervalActive(RegRecord* physRegRecord, LsraLocation refLocation, RegisterType registerType);
+    bool canSpillReg(RegRecord* physRegRecord, LsraLocation refLocation, unsigned* recentAssignedRefWeight);
+    bool isRegInUse(RegRecord* regRec, RefPosition* refPosition);
 
     RefType CheckBlockType(BasicBlock* block, BasicBlock* prevBlock);
 
@@ -888,13 +882,14 @@ private:
      ****************************************************************************/
     RegisterType getRegisterType(Interval* currentInterval, RefPosition* refPosition);
     regNumber tryAllocateFreeReg(Interval* current, RefPosition* refPosition);
-    RegRecord* findBestPhysicalReg(RegisterType regType,
-                                   LsraLocation endLocation,
-                                   regMaskTP    candidates,
-                                   regMaskTP    preferences);
     regNumber allocateBusyReg(Interval* current, RefPosition* refPosition, bool allocateIfProfitable);
     regNumber assignCopyReg(RefPosition* refPosition);
 
+    bool isMatchingConstant(RegRecord* physRegRecord, RefPosition* refPosition);
+    bool isSpillCandidate(Interval*     current,
+                          RefPosition*  refPosition,
+                          RegRecord*    physRegRecord,
+                          LsraLocation& nextLocation);
     void checkAndAssignInterval(RegRecord* regRec, Interval* interval);
     void assignPhysReg(RegRecord* regRec, Interval* interval);
     void assignPhysReg(regNumber reg, Interval* interval)