Propagate preferences (#19429)
authorCarol Eidt <carol.eidt@microsoft.com>
Sat, 2 Feb 2019 14:37:34 +0000 (06:37 -0800)
committerGitHub <noreply@github.com>
Sat, 2 Feb 2019 14:37:34 +0000 (06:37 -0800)
* Propagate preferences

Instead of selecting a single relatedInterval for preferencing,
follow the chain of relatedIntervals (preferenced intervals).

Change when preferences are propagated to the relatedInterval;
do it in allocateRegisters, so that we can update it when we see a last use.

Also tune when and how intervals are preferenced, including allowing multiple
uses on an RMW node to have the target as their preference.

Fixes #11463
Contributes to #16359

src/jit/lsra.cpp
src/jit/lsra.h
src/jit/lsrabuild.cpp
src/jit/lsraxarch.cpp
tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.csproj [new file with mode: 0644]

index d92e884..ac5b749 100644 (file)
@@ -2236,17 +2236,22 @@ void LinearScan::dumpVarRefPositions(const char* title)
 
         for (unsigned i = 0; i < compiler->lvaCount; i++)
         {
-            printf("--- V%02u\n", i);
+            printf("--- V%02u", i);
 
             LclVarDsc* varDsc = compiler->lvaTable + i;
             if (varDsc->lvIsRegCandidate())
             {
                 Interval* interval = getIntervalForLocalVar(varDsc->lvVarIndex);
+                printf("  (Interval %d)\n", interval->intervalIndex);
                 for (RefPosition* ref = interval->firstRefPosition; ref != nullptr; ref = ref->nextRefPosition)
                 {
                     ref->dump();
                 }
             }
+            else
+            {
+                printf("\n");
+            }
         }
         printf("\n");
     }
@@ -2668,61 +2673,84 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
     {
         preferences = candidates;
     }
-    regMaskTP relatedPreferences = RBM_NONE;
 
 #ifdef DEBUG
     candidates = stressLimitRegs(refPosition, candidates);
 #endif
     assert(candidates != RBM_NONE);
 
-    // If the related interval has no further references, it is possible that it is a source of the
-    // node that produces this interval.  However, we don't want to use the relatedInterval for preferencing
-    // if its next reference is not a new definition (as it either is or will become live).
     Interval* relatedInterval = currentInterval->relatedInterval;
-    if (relatedInterval != nullptr)
+    if (currentInterval->isSpecialPutArg)
+    {
+        // This is not actually a preference, it's merely to track the lclVar that this
+        // "specialPutArg" is using.
+        relatedInterval = nullptr;
+    }
+    Interval* nextRelatedInterval  = relatedInterval;
+    Interval* finalRelatedInterval = relatedInterval;
+    Interval* rangeEndInterval     = relatedInterval;
+    regMaskTP relatedPreferences   = (relatedInterval == nullptr) ? RBM_NONE : relatedInterval->getCurrentPreferences();
+    LsraLocation rangeEndLocation  = refPosition->getRangeEndLocation();
+    bool         preferCalleeSave  = currentInterval->preferCalleeSave;
+    bool         avoidByteRegs     = false;
+#ifdef _TARGET_X86_
+    if ((relatedPreferences & ~RBM_BYTE_REGS) != RBM_NONE)
     {
-        RefPosition* nextRelatedRefPosition = relatedInterval->getNextRefPosition();
-        if (nextRelatedRefPosition != nullptr)
-        {
-            // Don't use the relatedInterval for preferencing if its next reference is not a new definition,
-            // or if it is only related because they are multi-reg targets of the same node.
-            if (!RefTypeIsDef(nextRelatedRefPosition->refType))
-            {
-                relatedInterval = nullptr;
-            }
-            // Is the relatedInterval not assigned and simply a copy to another relatedInterval?
-            else if ((relatedInterval->assignedReg == nullptr) && (relatedInterval->relatedInterval != nullptr) &&
-                     (nextRelatedRefPosition->nextRefPosition != nullptr) &&
-                     (nextRelatedRefPosition->nextRefPosition->nextRefPosition == nullptr) &&
-                     (nextRelatedRefPosition->nextRefPosition->nodeLocation <
-                      relatedInterval->relatedInterval->getNextRefLocation()))
-            {
-                // The current relatedInterval has only two remaining RefPositions, both of which
-                // occur prior to the next RefPosition for its relatedInterval.
-                // It is likely a copy.
-                relatedInterval = relatedInterval->relatedInterval;
-            }
-        }
+        avoidByteRegs = true;
     }
+#endif
 
-    if (relatedInterval != nullptr)
+    // Follow the chain of related intervals, as long as:
+    // - The next reference is a def. We don't want to use the relatedInterval for preferencing if its next reference
+    //   is not a new definition (as it either is or will become live).
+    // - The next (def) reference is downstream. Otherwise we could iterate indefinitely because the preferences can be
+    // circular.
+    // - The intersection of preferenced registers is non-empty.
+    //
+    while (nextRelatedInterval != nullptr)
     {
-        // If the related interval already has an assigned register, then use that
-        // as the related preference.  We'll take the related
-        // interval preferences into account in the loop over all the registers.
+        RefPosition* nextRelatedRefPosition = nextRelatedInterval->getNextRefPosition();
 
-        if (relatedInterval->assignedReg != nullptr)
+        // Only use the relatedInterval for preferencing if the related interval's next reference
+        // is a new definition.
+        if ((nextRelatedRefPosition != nullptr) && RefTypeIsDef(nextRelatedRefPosition->refType))
         {
-            relatedPreferences = genRegMask(relatedInterval->assignedReg->regNum);
+            finalRelatedInterval = nextRelatedInterval;
+            nextRelatedInterval  = nullptr;
+
+            // First, get the preferences for this interval
+            regMaskTP thisRelatedPreferences = finalRelatedInterval->getCurrentPreferences();
+            // Now, determine if they are compatible and update the relatedPreferences that we'll consider.
+            regMaskTP newRelatedPreferences = thisRelatedPreferences & relatedPreferences;
+            if (newRelatedPreferences != RBM_NONE && (!avoidByteRegs || thisRelatedPreferences != RBM_BYTE_REGS))
+            {
+                bool thisIsSingleReg = isSingleRegister(newRelatedPreferences);
+                if (!thisIsSingleReg || (finalRelatedInterval->isLocalVar &&
+                                         getRegisterRecord(genRegNumFromMask(newRelatedPreferences))->isFree()))
+                {
+                    relatedPreferences = newRelatedPreferences;
+                    // If this Interval has a downstream def without a single-register preference, continue to iterate.
+                    if (nextRelatedRefPosition->nodeLocation > rangeEndLocation)
+                    {
+                        preferCalleeSave    = (preferCalleeSave || finalRelatedInterval->preferCalleeSave);
+                        rangeEndLocation    = nextRelatedRefPosition->getRangeEndLocation();
+                        rangeEndInterval    = finalRelatedInterval;
+                        nextRelatedInterval = finalRelatedInterval->relatedInterval;
+                    }
+                }
+            }
         }
         else
         {
-            relatedPreferences = relatedInterval->registerPreferences;
+            if (nextRelatedInterval == relatedInterval)
+            {
+                relatedInterval    = nullptr;
+                relatedPreferences = RBM_NONE;
+            }
+            nextRelatedInterval = nullptr;
         }
     }
 
-    bool preferCalleeSave = currentInterval->preferCalleeSave;
-
     // For floating point, we want to be less aggressive about using callee-save registers.
     // So in that case, we just need to ensure that the current RefPosition is covered.
     RefPosition* rangeEndRefPosition;
@@ -2730,27 +2758,27 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
     if (useFloatReg(currentInterval->registerType))
     {
         rangeEndRefPosition = refPosition;
+        preferCalleeSave    = currentInterval->preferCalleeSave;
     }
     else
     {
-        rangeEndRefPosition = currentInterval->lastRefPosition;
-        // If we have a relatedInterval that is not currently occupying a register,
-        // and whose lifetime begins after this one ends,
+        rangeEndRefPosition = refPosition->getRangeEndRef();
+        // If we have a chain of related intervals, and a finalRelatedInterval that
+        // is not currently occupying a register, and whose lifetime begins after this one,
         // we want to try to select a register that will cover its lifetime.
-        if ((relatedInterval != nullptr) && (relatedInterval->assignedReg == nullptr) &&
-            (relatedInterval->getNextRefLocation() >= rangeEndRefPosition->nodeLocation))
+        if ((rangeEndInterval != nullptr) && (rangeEndInterval->assignedReg == nullptr) &&
+            (rangeEndInterval->getNextRefLocation() >= rangeEndRefPosition->nodeLocation))
         {
-            lastRefPosition  = relatedInterval->lastRefPosition;
-            preferCalleeSave = relatedInterval->preferCalleeSave;
+            lastRefPosition = rangeEndInterval->lastRefPosition;
         }
     }
 
     // If this has a delayed use (due to being used in a rmw position of a
     // non-commutative operator), its endLocation is delayed until the "def"
     // position, which is one location past the use (getRefEndLocation() takes care of this).
-    LsraLocation rangeEndLocation = rangeEndRefPosition->getRefEndLocation();
-    LsraLocation lastLocation     = lastRefPosition->getRefEndLocation();
-    regNumber    prevReg          = REG_NA;
+    rangeEndLocation          = rangeEndRefPosition->getRefEndLocation();
+    LsraLocation lastLocation = lastRefPosition->getRefEndLocation();
+    regNumber    prevReg      = REG_NA;
 
     if (currentInterval->assignedReg)
     {
@@ -2911,7 +2939,7 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
         // are "local last uses" of the Interval - otherwise the liveRange would interfere with the reg.
         if (nextPhysRefLocation == rangeEndLocation && rangeEndRefPosition->isFixedRefOfReg(regNum))
         {
-            INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_INCREMENT_RANGE_END, currentInterval, regNum));
+            INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_INCREMENT_RANGE_END, currentInterval));
             nextPhysRefLocation++;
         }
 
@@ -2923,7 +2951,7 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
                 score |= COVERS;
             }
         }
-        if (relatedInterval != nullptr && (candidateBit & relatedPreferences) != RBM_NONE)
+        if ((candidateBit & relatedPreferences) != RBM_NONE)
         {
             score |= RELATED_PREFERENCE;
             if (nextPhysRefLocation > relatedInterval->lastRefPosition->nodeLocation)
@@ -3033,10 +3061,6 @@ regNumber LinearScan::tryAllocateFreeReg(Interval* currentInterval, RefPosition*
         foundReg                        = availablePhysRegInterval->regNum;
         regMaskTP foundRegMask          = genRegMask(foundReg);
         refPosition->registerAssignment = foundRegMask;
-        if (relatedInterval != nullptr)
-        {
-            relatedInterval->updateRegisterPreferences(foundRegMask);
-        }
     }
 
     return foundReg;
@@ -5689,6 +5713,14 @@ void LinearScan::allocateRegisters()
                 {
                     currentInterval->isActive = false;
                 }
+
+                // Update the register preferences for the relatedInterval, if this is 'preferencedToDef'.
+                // Don't propagate to subsequent relatedIntervals; that will happen as they are allocated, and we
+                // don't know yet whether the register will be retained.
+                if (currentInterval->relatedInterval != nullptr)
+                {
+                    currentInterval->relatedInterval->updateRegisterPreferences(assignedRegBit);
+                }
             }
 
             lastAllocatedRefPosition = currentRefPosition;
@@ -8609,7 +8641,6 @@ void Interval::dump()
     {
         printf(" RelatedInterval ");
         relatedInterval->microDump();
-        printf("[%p]", dspPtr(relatedInterval));
     }
 
     printf("\n");
@@ -8633,16 +8664,16 @@ void Interval::tinyDump()
 // print out extremely concise representation
 void Interval::microDump()
 {
+    if (isLocalVar)
+    {
+        printf("<V%02u/L%u>", varNum, intervalIndex);
+        return;
+    }
     char intervalTypeChar = 'I';
     if (isInternal)
     {
         intervalTypeChar = 'T';
     }
-    else if (isLocalVar)
-    {
-        intervalTypeChar = 'L';
-    }
-
     printf("<%c%u>", intervalTypeChar, intervalIndex);
 }
 
@@ -9282,10 +9313,6 @@ void LinearScan::dumpLsraAllocationEvent(LsraDumpEvent event,
             }
             printf("Restr %-4s ", getRegName(reg));
             dumpRegRecords();
-            if (activeRefPosition != nullptr)
-            {
-                printf(emptyRefPositionFormat, "");
-            }
             break;
 
         // Done with GC Kills
@@ -9650,8 +9677,9 @@ void LinearScan::dumpRefPositionShort(RefPosition* refPosition, BasicBlock* curr
         if (block == nullptr)
         {
             printf(regNameFormat, "END");
-            printf("               ");
-            printf(regNameFormat, "");
+            printf("        ");
+            // We still need to print this refposition.
+            lastPrintedRefPosition = nullptr;
         }
         else
         {
index db80795..7ea0986 100644 (file)
@@ -1493,7 +1493,8 @@ private:
     // As we build uses, we may want to preference the next definition (i.e. the register produced
     // by the current node) to the same register as one of its uses. This is done by setting
     // 'tgtPrefUse' to that RefPosition.
-    RefPosition* tgtPrefUse = nullptr;
+    RefPosition* tgtPrefUse  = nullptr;
+    RefPosition* tgtPrefUse2 = nullptr;
 
     // The following keep track of information about internal (temporary register) intervals
     // during the building of a single node.
@@ -1512,6 +1513,7 @@ private:
     void clearBuildState()
     {
         tgtPrefUse               = nullptr;
+        tgtPrefUse2              = nullptr;
         internalCount            = 0;
         setInternalRegsDelayFree = false;
         pendingDelayFree         = false;
@@ -1528,7 +1530,7 @@ private:
     // These methods return the number of sources.
     int BuildNode(GenTree* stmt);
 
-    GenTree* getTgtPrefOperand(GenTreeOp* tree);
+    void getTgtPrefOperands(GenTreeOp* tree, bool& prefOp1, bool& prefOp2);
     bool supportsSpecialPutArg();
 
     int BuildSimple(GenTree* tree);
@@ -1753,11 +1755,12 @@ public:
     }
 
     // Assign the related interval, but only if it isn't already assigned.
-    void assignRelatedIntervalIfUnassigned(Interval* newRelatedInterval)
+    bool assignRelatedIntervalIfUnassigned(Interval* newRelatedInterval)
     {
         if (relatedInterval == nullptr)
         {
             assignRelatedInterval(newRelatedInterval);
+            return true;
         }
         else
         {
@@ -1769,16 +1772,22 @@ public:
                 printf(" already has a related interval\n");
             }
 #endif // DEBUG
+            return false;
         }
     }
 
-    // Update the registerPreferences on the interval.
-    // If there are conflicting requirements on this interval, set the preferences to
-    // the union of them.  That way maybe we'll get at least one of them.
-    // An exception is made in the case where one of the existing or new
-    // preferences are all callee-save, in which case we "prefer" the callee-save
+    // Get the current preferences for this Interval.
+    // Note that when we have an assigned register we don't necessarily update the
+    // registerPreferences to that register, as there may be multiple, possibly disjoint,
+    // definitions. This method will return the current assigned register if any, or
+    // the 'registerPreferences' otherwise.
+    //
+    regMaskTP getCurrentPreferences()
+    {
+        return (assignedReg == nullptr) ? registerPreferences : genRegMask(assignedReg->regNum);
+    }
 
-    void updateRegisterPreferences(regMaskTP preferences)
+    void mergeRegisterPreferences(regMaskTP preferences)
     {
         // We require registerPreferences to have been initialized.
         assert(registerPreferences != RBM_NONE);
@@ -1832,6 +1841,25 @@ public:
         }
         registerPreferences = newPreferences;
     }
+
+    // Update the registerPreferences on the interval.
+    // If there are conflicting requirements on this interval, set the preferences to
+    // the union of them.  That way maybe we'll get at least one of them.
+    // An exception is made in the case where one of the existing or new
+    // preferences are all callee-save, in which case we "prefer" the callee-save
+
+    void updateRegisterPreferences(regMaskTP preferences)
+    {
+        // If this interval is preferenced, that interval may have already been assigned a
+        // register, and we want to include that in the preferences.
+        if ((relatedInterval != nullptr) && !relatedInterval->isActive)
+        {
+            mergeRegisterPreferences(relatedInterval->getCurrentPreferences());
+        }
+
+        // Now merge the new preferences.
+        mergeRegisterPreferences(preferences);
+    }
 };
 
 class RefPosition
@@ -2043,6 +2071,24 @@ public:
         return delayRegFree ? nodeLocation + 1 : nodeLocation;
     }
 
+    RefPosition* getRangeEndRef()
+    {
+        if (lastUse || nextRefPosition == nullptr)
+        {
+            return this;
+        }
+        // It would seem to make sense to only return 'nextRefPosition' if it is a lastUse,
+        // and otherwise return `lastRefPosition', but that tends to  excessively lengthen
+        // the range for heuristic purposes.
+        // TODO-CQ: Look into how this might be improved .
+        return nextRefPosition;
+    }
+
+    LsraLocation getRangeEndLocation()
+    {
+        return getRangeEndRef()->getRefEndLocation();
+    }
+
     bool isIntervalRef()
     {
         return (!isPhysRegRef && (referent != nullptr));
index 02464a1..1a54d86 100644 (file)
@@ -381,7 +381,7 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp)
 #endif // DEBUG
     {
         // Set preferences so that this register set will be preferred for earlier refs
-        theInterval->updateRegisterPreferences(rp->registerAssignment);
+        theInterval->mergeRegisterPreferences(rp->registerAssignment);
     }
 }
 
@@ -2359,6 +2359,38 @@ void LinearScan::validateIntervals()
 }
 #endif // DEBUG
 
+#ifdef _TARGET_XARCH_
+//------------------------------------------------------------------------
+// setTgtPref: Set a  preference relationship between the given Interval
+//             and a Use RefPosition.
+//
+// Arguments:
+//    interval   - An interval whose defining instruction has tgtPrefUse as a use
+//    tgtPrefUse - The use RefPosition
+//
+// Notes:
+//    This is called when we would like tgtPrefUse and this def to get the same register.
+//    This is only desirable if the use is a last use, which it is if it is a non-local,
+//    *or* if it is a lastUse.
+//     Note that we don't yet have valid lastUse information in the RefPositions that we're building
+//    (every RefPosition is set as a lastUse until we encounter a new use), so we have to rely on the treeNode.
+//    This may be called for multiple uses, in which case 'interval' will only get preferenced at most
+//    to the first one (if it didn't already have a 'relatedInterval'.
+//
+void setTgtPref(Interval* interval, RefPosition* tgtPrefUse)
+{
+    if (tgtPrefUse != nullptr)
+    {
+        Interval* useInterval = tgtPrefUse->getInterval();
+        if (!useInterval->isLocalVar || (tgtPrefUse->treeNode == nullptr) ||
+            ((tgtPrefUse->treeNode->gtFlags & GTF_VAR_DEATH) != 0))
+        {
+            // Set the use interval as related to the interval we're defining.
+            useInterval->assignRelatedIntervalIfUnassigned(interval);
+        }
+    }
+}
+#endif // _TARGET_XARCH_
 //------------------------------------------------------------------------
 // BuildDef: Build a RefTypeDef RefPosition for the given node
 //
@@ -2430,10 +2462,10 @@ RefPosition* LinearScan::BuildDef(GenTree* tree, regMaskTP dstCandidates, int mu
         RefInfoListNode* refInfo = listNodePool.GetNode(defRefPosition, tree);
         defList.Append(refInfo);
     }
-    if (tgtPrefUse != nullptr)
-    {
-        interval->assignRelatedIntervalIfUnassigned(tgtPrefUse->getInterval());
-    }
+#ifdef _TARGET_XARCH_
+    setTgtPref(interval, tgtPrefUse);
+    setTgtPref(interval, tgtPrefUse2);
+#endif // _TARGET_XARCH_
     return defRefPosition;
 }
 
@@ -2757,7 +2789,6 @@ int LinearScan::BuildDelayFreeUses(GenTree* node, regMaskTP candidates)
 int LinearScan::BuildBinaryUses(GenTreeOp* node, regMaskTP candidates)
 {
 #ifdef _TARGET_XARCH_
-    RefPosition* tgtPrefUse = nullptr;
     if (node->OperIsBinary() && isRMWRegOper(node))
     {
         return BuildRMWUses(node, candidates);
@@ -3156,7 +3187,6 @@ int LinearScan::BuildPutArgReg(GenTreeUnOp* node)
         // Preference the destination to the interval of the first register defined by the first operand.
         assert(use->getInterval()->isLocalVar);
         isSpecialPutArg = true;
-        tgtPrefUse      = use;
     }
 
 #ifdef _TARGET_ARM_
@@ -3178,6 +3208,7 @@ int LinearScan::BuildPutArgReg(GenTreeUnOp* node)
         if (isSpecialPutArg)
         {
             def->getInterval()->isSpecialPutArg = true;
+            def->getInterval()->assignRelatedInterval(use->getInterval());
         }
     }
     return srcCount;
index 493463a..cc6419a 100644 (file)
@@ -699,7 +699,21 @@ int LinearScan::BuildNode(GenTree* tree)
     return srcCount;
 }
 
-GenTree* LinearScan::getTgtPrefOperand(GenTreeOp* tree)
+//------------------------------------------------------------------------
+// getTgtPrefOperands: Identify whether the operands of an Op should be preferenced to the target.
+//
+// Arguments:
+//    tree    - the node of interest.
+//    prefOp1 - a bool "out" parameter indicating, on return, whether op1 should be preferenced to the target.
+//    prefOp2 - a bool "out" parameter indicating, on return, whether op2 should be preferenced to the target.
+//
+// Return Value:
+//    This has two "out" parameters for returning the results (see above).
+//
+// Notes:
+//    The caller is responsible for initializing the two "out" parameters to false.
+//
+void LinearScan::getTgtPrefOperands(GenTreeOp* tree, bool& prefOp1, bool& prefOp2)
 {
     // If op2 of a binary-op gets marked as contained, then binary-op srcCount will be 1.
     // Even then we would like to set isTgtPref on Op1.
@@ -708,23 +722,20 @@ GenTree* LinearScan::getTgtPrefOperand(GenTreeOp* tree)
         GenTree* op1 = tree->gtGetOp1();
         GenTree* op2 = tree->gtGetOp2();
 
-        // Commutative opers like add/mul/and/or/xor could reverse the order of
-        // operands if it is safe to do so.  In such a case we would like op2 to be
-        // target preferenced instead of op1.
-        if (tree->OperIsCommutative() && op1->isContained() && op2 != nullptr)
-        {
-            op1 = op2;
-            op2 = tree->gtGetOp1();
-        }
-
         // If we have a read-modify-write operation, we want to preference op1 to the target,
         // if it is not contained.
         if (!op1->isContained() && !op1->OperIs(GT_LIST))
         {
-            return op1;
+            prefOp1 = true;
+        }
+
+        // Commutative opers like add/mul/and/or/xor could reverse the order of operands if it is safe to do so.
+        // In that case we will preference both, to increase the chance of getting a match.
+        if (tree->OperIsCommutative() && op2 != nullptr && !op2->isContained())
+        {
+            prefOp2 = true;
         }
     }
-    return nullptr;
 }
 
 //------------------------------------------------------------------------------
@@ -806,8 +817,10 @@ int LinearScan::BuildRMWUses(GenTreeOp* node, regMaskTP candidates)
     }
 #endif // _TARGET_X86_
 
-    GenTree* tgtPrefOperand = getTgtPrefOperand(node);
-    assert((tgtPrefOperand == nullptr) || (tgtPrefOperand == op1) || node->OperIsCommutative());
+    bool prefOp1 = false;
+    bool prefOp2 = false;
+    getTgtPrefOperands(node, prefOp1, prefOp2);
+    assert(!prefOp2 || node->OperIsCommutative());
     assert(!isReverseOp || node->OperIsCommutative());
 
     // Determine which operand, if any, should be delayRegFree. Normally, this would be op2,
@@ -842,7 +855,8 @@ int LinearScan::BuildRMWUses(GenTreeOp* node, regMaskTP candidates)
     }
     if (delayUseOperand != nullptr)
     {
-        assert(delayUseOperand != tgtPrefOperand);
+        assert(!prefOp1 || delayUseOperand != op1);
+        assert(!prefOp2 || delayUseOperand != op2);
     }
 
     if (isReverseOp)
@@ -852,7 +866,7 @@ int LinearScan::BuildRMWUses(GenTreeOp* node, regMaskTP candidates)
     }
 
     // Build first use
-    if (tgtPrefOperand == op1)
+    if (prefOp1)
     {
         assert(!op1->isContained());
         tgtPrefUse = BuildUse(op1, op1Candidates);
@@ -869,10 +883,10 @@ int LinearScan::BuildRMWUses(GenTreeOp* node, regMaskTP candidates)
     // Build second use
     if (op2 != nullptr)
     {
-        if (tgtPrefOperand == op2)
+        if (prefOp2)
         {
             assert(!op2->isContained());
-            tgtPrefUse = BuildUse(op2, op2Candidates);
+            tgtPrefUse2 = BuildUse(op2, op2Candidates);
             srcCount++;
         }
         else if (delayUseOperand == op2)
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.cs
new file mode 100644 (file)
index 0000000..28ef079
--- /dev/null
@@ -0,0 +1,110 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.CompilerServices;
+
+public class C
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public void M()
+    {
+    }
+}
+
+public struct S1
+{
+    private C _c;
+
+    public S1(C c) => _c = c;
+    public void M() => _c.M();
+}
+
+public struct S2
+{
+    private S1 _s;
+
+    public S2(S1 s) => _s = s;
+    public void M() => _s.M();
+}
+
+public struct S3
+{
+    private S2 _s;
+
+    public S3(S2 s) => _s = s;
+    public void M() => _s.M();
+}
+
+public struct S4
+{
+    private S3 _s;
+
+    public S4(S3 s) => _s = s;
+    public void M() => _s.M();
+}
+
+public struct S5
+{
+    private S4 _s;
+
+    public S5(S4 s) => _s = s;
+    public void M() => _s.M();
+}
+
+public static class GitHub_18542
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaClass()
+    {
+        var c = new C();
+        c.M();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaStruct1()
+    {
+        var s1 = new S1(new C());
+        s1.M();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaStruct2()
+    {
+        var s2 = new S2(new S1(new C()));
+        s2.M();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaStruct3()
+    {
+        var s3 = new S3(new S2(new S1(new C())));
+        s3.M();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaStruct4()
+    {
+        var s4 = new S4(new S3(new S2(new S1(new C()))));
+        s4.M();
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void ViaStruct5()
+    {
+        var s5 = new S5(new S4(new S3(new S2(new S1(new C())))));
+        s5.M();
+    }
+
+    static int Main()
+    {
+        ViaClass();
+        ViaStruct1();
+        ViaStruct2();
+        ViaStruct3();
+        ViaStruct4();
+        ViaStruct5();
+        return 100;
+    }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18542/GitHub_18542.csproj
new file mode 100644 (file)
index 0000000..0b9fa76
--- /dev/null
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType></DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>