# This is a combination of 2 commits.

# The first commit's message is:

Mark lvDoNotEnregister lclVars as contained

Even if a lclVar is tracked, it may not be a register candidate. When determining whether a lclVar should be contained or RegOptional, take that into account.
In the interest of making this as accurate as possible, mark lclVars early as lvDoNotEnregister when they meet criteria that will later disqualify them from a register.

# This is the commit message dotnet/coreclr#2:

Formatting

Commit migrated from https://github.com/dotnet/coreclr/commit/34f400e5469900456f81226430b24e760a2d17f9

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index bc178ca..91ad29a 100644 (file)
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -2435,6 +2435,9 @@ public:
         DNER_LiveAcrossUnmanagedCall,
         DNER_BlockOp,     // Is read or written via a block operation that explicitly takes the address.
         DNER_IsStructArg, // Is a struct passed as an argument in a way that requires a stack location.
+        DNER_DepField,    // It is a field of a dependently promoted struct
+        DNER_NoRegVars,   // opts.compFlags & CLFLG_REGVAR is not set
+        DNER_MinOptsGC,   // It is a GC Ref and we are compiling MinOpts
 #ifdef JIT32_GCENCODER
         DNER_PinningRef,
 #endif

diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp
index f662bcc..60c12ba 100644 (file)
--- a/src/coreclr/src/jit/lclvars.cpp
+++ b/src/coreclr/src/jit/lclvars.cpp
@@ -2152,6 +2152,18 @@ void Compiler::lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregister
             JITDUMP("live across unmanaged call\n");
             varDsc->lvLiveAcrossUCall = 1;
             break;
+        case DNER_DepField:
+            JITDUMP("field of a dependently promoted struct\n");
+            assert(varDsc->lvIsStructField && (lvaGetParentPromotionType(varNum) != PROMOTION_TYPE_INDEPENDENT));
+            break;
+        case DNER_NoRegVars:
+            JITDUMP("opts.compFlags & CLFLG_REGVAR is not set\n");
+            assert((opts.compFlags & CLFLG_REGVAR) == 0);
+            break;
+        case DNER_MinOptsGC:
+            JITDUMP("It is a GC Ref and we are compiling MinOpts\n");
+            assert(!JitConfig.JitMinOptsTrackGCrefs() && varTypeIsGC(varDsc->TypeGet()));
+            break;
 #ifdef JIT32_GCENCODER
         case DNER_PinningRef:
             JITDUMP("pinning ref\n");
@@ -3281,6 +3293,7 @@ void Compiler::lvaSortByRefCount()
             // untracked when a blockOp is used to assign the struct.
             //
             varDsc->lvTracked = 0; // so, don't mark as tracked
+            lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_DepField));
         }
         else if (varDsc->lvPinned)
         {
@@ -3292,6 +3305,11 @@ void Compiler::lvaSortByRefCount()
         else if (opts.MinOpts() && !JitConfig.JitMinOptsTrackGCrefs() && varTypeIsGC(varDsc->TypeGet()))
         {
             varDsc->lvTracked = 0;
+            lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_MinOptsGC));
+        }
+        else if ((opts.compFlags & CLFLG_REGVAR) == 0)
+        {
+            lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_NoRegVars));
         }
 #if defined(JIT32_GCENCODER) && defined(WIN64EXCEPTIONS)
         else if (lvaIsOriginalThisArg(lclNum) && (info.compMethodInfo->options & CORINFO_GENERICS_CTXT_FROM_THIS) != 0)

diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index f7b2489..cdd755f 100644 (file)
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -108,20 +108,39 @@ bool Lowering::IsSafeToContainMem(GenTree* parentNode, GenTree* childNode)
 // IsContainableMemoryOp: Checks whether this is a memory op that can be contained.
 //
 // Arguments:
-//    node - the node of interest.
+//    node        - the node of interest.
+//    useTracked  - true if this is being called after liveness so lvTracked is correct
+//
+// Return value:
+//    True if this will definitely be a memory reference that could be contained.
 //
 // Notes:
 //    This differs from the isMemoryOp() method on GenTree because it checks for
-//    the case of an untracked local. Note that this won't include locals that
-//    for some reason do not become register candidates, nor those that get
+//    the case of doNotEnregister local. This won't include locals that
+//    for some other reason do not become register candidates, nor those that get
 //    spilled.
+//    Also, if we call this before we redo liveness analysis, any new lclVars
+//    introduced after the last dataflow analysis will not yet be marked lvTracked,
+//    so we don't use that.
 //
-// Return value:
-//    True if this will definitely be a memory reference that could be contained.
-//
-bool Lowering::IsContainableMemoryOp(GenTree* node)
+bool Lowering::IsContainableMemoryOp(GenTree* node, bool useTracked)
 {
-    return node->isMemoryOp() || (node->IsLocal() && !comp->lvaTable[node->AsLclVar()->gtLclNum].lvTracked);
+#ifdef _TARGET_XARCH_
+    if (node->isMemoryOp())
+    {
+        return true;
+    }
+    if (node->IsLocal())
+    {
+        if (!m_lsra->enregisterLocalVars)
+        {
+            return true;
+        }
+        LclVarDsc* varDsc = &comp->lvaTable[node->AsLclVar()->gtLclNum];
+        return (varDsc->lvDoNotEnregister || (useTracked && !varDsc->lvTracked));
+    }
+#endif // _TARGET_XARCH_
+    return false;
 }
 
 //------------------------------------------------------------------------
@@ -2257,7 +2276,7 @@ void Lowering::LowerCompare(GenTree* cmp)
         GenTreeIntCon* op2      = cmp->gtGetOp2()->AsIntCon();
         ssize_t        op2Value = op2->IconValue();
 
-        if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && genTypeCanRepresentValue(op1Type, op2Value))
+        if (IsContainableMemoryOp(op1, false) && varTypeIsSmall(op1Type) && genTypeCanRepresentValue(op1Type, op2Value))
         {
             //
             // If op1's type is small then try to narrow op2 so it has the same type as op1.
@@ -2287,7 +2306,8 @@ void Lowering::LowerCompare(GenTree* cmp)
                 // the result of bool returning calls.
                 //
 
-                if (castOp->OperIs(GT_CALL, GT_LCL_VAR) || castOp->OperIsLogical() || IsContainableMemoryOp(castOp))
+                if (castOp->OperIs(GT_CALL, GT_LCL_VAR) || castOp->OperIsLogical() ||
+                    IsContainableMemoryOp(castOp, false))
                 {
                     assert(!castOp->gtOverflowEx()); // Must not be an overflow checking operation
 
@@ -2332,7 +2352,7 @@ void Lowering::LowerCompare(GenTree* cmp)
                 cmp->gtOp.gtOp1 = andOp1;
                 cmp->gtOp.gtOp2 = andOp2;
 
-                if (IsContainableMemoryOp(andOp1) && andOp2->IsIntegralConst())
+                if (IsContainableMemoryOp(andOp1, false) && andOp2->IsIntegralConst())
                 {
                     //
                     // For "test" we only care about the bits that are set in the second operand (mask).

diff --git a/src/coreclr/src/jit/lower.h b/src/coreclr/src/jit/lower.h
index cd1e5ef..da9a726 100644 (file)
--- a/src/coreclr/src/jit/lower.h
+++ b/src/coreclr/src/jit/lower.h
@@ -279,7 +279,7 @@ private:
     bool IsContainableImmed(GenTree* parentNode, GenTree* childNode);
 
     // Return true if 'node' is a containable memory op.
-    bool IsContainableMemoryOp(GenTree* node);
+    bool IsContainableMemoryOp(GenTree* node, bool useTracked);
 
     // Makes 'childNode' contained in the 'parentNode'
     void MakeSrcContained(GenTreePtr parentNode, GenTreePtr childNode);

diff --git a/src/coreclr/src/jit/lowerxarch.cpp b/src/coreclr/src/jit/lowerxarch.cpp
index cda5634..bdacb5c 100644 (file)
--- a/src/coreclr/src/jit/lowerxarch.cpp
+++ b/src/coreclr/src/jit/lowerxarch.cpp
@@ -1094,7 +1094,7 @@ GenTree* Lowering::PreferredRegOptionalOperand(GenTree* tree)
 
     // This routine uses the following heuristics:
     //
-    // a) If both are tracked locals, marking the one with lower weighted
+    // a) If both are register candidates, marking the one with lower weighted
     // ref count as reg-optional would likely be beneficial as it has
     // higher probability of not getting a register.
     //
@@ -1131,9 +1131,11 @@ GenTree* Lowering::PreferredRegOptionalOperand(GenTree* tree)
         LclVarDsc* v1 = comp->lvaTable + op1->AsLclVarCommon()->GetLclNum();
         LclVarDsc* v2 = comp->lvaTable + op2->AsLclVarCommon()->GetLclNum();
 
-        if (v1->lvTracked && v2->lvTracked)
+        bool v1IsRegCandidate = !v1->lvDoNotEnregister && v1->lvTracked;
+        bool v2IsRegCandidate = !v2->lvDoNotEnregister && v2->lvTracked;
+        if (v1IsRegCandidate && v2IsRegCandidate)
         {
-            // Both are tracked locals.  The one with lower weight is less likely
+            // Both are tracked enregisterable locals.  The one with lower weight is less likely
             // to get a register and hence beneficial to mark the one with lower
             // weight as reg optional.
             if (v1->lvRefCntWtd < v2->lvRefCntWtd)
@@ -1145,15 +1147,14 @@ GenTree* Lowering::PreferredRegOptionalOperand(GenTree* tree)
                 preferredOp = op2;
             }
         }
-        else if (v2->lvTracked)
+        else if (v2IsRegCandidate)
         {
-            // v1 is an untracked lcl and it is use position is less likely to
-            // get a register.
+            // v1 is not a reg candidate and its use position is less likely to get a register.
             preferredOp = op1;
         }
-        else if (v1->lvTracked)
+        else if (v1IsRegCandidate)
         {
-            // v2 is an untracked lcl and its def position always
+            // v2 is not a reg candidate and its def position always
             // needs a reg.  Hence it is better to mark v1 as
             // reg optional.
             preferredOp = op1;

diff --git a/src/coreclr/src/jit/lsra.cpp b/src/coreclr/src/jit/lsra.cpp
index b6117d2..f15661b 100644 (file)
--- a/src/coreclr/src/jit/lsra.cpp
+++ b/src/coreclr/src/jit/lsra.cpp
@@ -10839,6 +10839,14 @@ void TreeNodeInfo::dump(LinearScan* lsra)
     {
         printf(" P");
     }
+    if (regOptional)
+    {
+        printf(" O");
+    }
+    if (isInternalRegDelayFree)
+    {
+        printf(" ID");
+    }
     printf(">\n");
 }
 

diff --git a/src/coreclr/src/jit/lsraxarch.cpp b/src/coreclr/src/jit/lsraxarch.cpp
index 03b340f..01ca450 100644 (file)
--- a/src/coreclr/src/jit/lsraxarch.cpp
+++ b/src/coreclr/src/jit/lsraxarch.cpp
@@ -401,12 +401,13 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
                 info->srcCount = 2;
                 info->dstCount = 1;
 
-                if (IsContainableMemoryOp(op2) || op2->IsCnsNonZeroFltOrDbl())
+                if (IsContainableMemoryOp(op2, true) || op2->IsCnsNonZeroFltOrDbl())
                 {
                     MakeSrcContained(tree, op2);
                 }
                 else if (tree->OperIsCommutative() &&
-                         (op1->IsCnsNonZeroFltOrDbl() || (IsContainableMemoryOp(op1) && IsSafeToContainMem(tree, op1))))
+                         (op1->IsCnsNonZeroFltOrDbl() ||
+                          (IsContainableMemoryOp(op1, true) && IsSafeToContainMem(tree, op1))))
                 {
                     // Though we have GT_ADD(op1=memOp, op2=non-memOp, we try to reorder the operands
                     // as long as it is safe so that the following efficient code sequence is generated:
@@ -630,7 +631,7 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
             {
                 other = node->gtIndex;
             }
-            else if (IsContainableMemoryOp(node->gtIndex))
+            else if (IsContainableMemoryOp(node->gtIndex, true))
             {
                 other = node->gtIndex;
             }
@@ -641,7 +642,7 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
 
             if (node->gtIndex->TypeGet() == node->gtArrLen->TypeGet())
             {
-                if (IsContainableMemoryOp(other))
+                if (IsContainableMemoryOp(other, true))
                 {
                     MakeSrcContained(tree, other);
                 }
@@ -845,8 +846,8 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
 
                 delayUseSrc = op1;
             }
-            else if ((op2 != nullptr) &&
-                     (!tree->OperIsCommutative() || (IsContainableMemoryOp(op2) && (op2->gtLsraInfo.srcCount == 0))))
+            else if ((op2 != nullptr) && (!tree->OperIsCommutative() ||
+                                          (IsContainableMemoryOp(op2, true) && (op2->gtLsraInfo.srcCount == 0))))
             {
                 delayUseSrc = op2;
             }
@@ -2118,7 +2119,8 @@ void Lowering::TreeNodeInfoInitLogicalOp(GenTree* tree)
         binOpInRMW = IsBinOpInRMWStoreInd(tree);
         if (!binOpInRMW)
         {
-            if (IsContainableMemoryOp(op2) && tree->TypeGet() == op2->TypeGet())
+            const unsigned operatorSize = genTypeSize(tree->TypeGet());
+            if (IsContainableMemoryOp(op2, true) && (genTypeSize(op2->TypeGet()) == operatorSize))
             {
                 directlyEncodable = true;
                 operand           = op2;
@@ -2126,7 +2128,8 @@ void Lowering::TreeNodeInfoInitLogicalOp(GenTree* tree)
             else if (tree->OperIsCommutative())
             {
                 if (IsContainableImmed(tree, op1) ||
-                    (IsContainableMemoryOp(op1) && tree->TypeGet() == op1->TypeGet() && IsSafeToContainMem(tree, op1)))
+                    (IsContainableMemoryOp(op1, true) && (genTypeSize(op1->TypeGet()) == operatorSize) &&
+                     IsSafeToContainMem(tree, op1)))
                 {
                     // If it is safe, we can reverse the order of operands of commutative operations for efficient
                     // codegen
@@ -2184,7 +2187,7 @@ void Lowering::TreeNodeInfoInitModDiv(GenTree* tree)
                 // everything is made explicit by adding casts.
                 assert(op1->TypeGet() == op2->TypeGet());
 
-                if (IsContainableMemoryOp(op2) || op2->IsCnsNonZeroFltOrDbl())
+                if (IsContainableMemoryOp(op2, true) || op2->IsCnsNonZeroFltOrDbl())
                 {
                     MakeSrcContained(tree, op2);
                 }
@@ -2251,7 +2254,7 @@ void Lowering::TreeNodeInfoInitModDiv(GenTree* tree)
     }
 
     // divisor can be an r/m, but the memory indirection must be of the same size as the divide
-    if (IsContainableMemoryOp(op2) && (op2->TypeGet() == tree->TypeGet()))
+    if (IsContainableMemoryOp(op2, true) && (op2->TypeGet() == tree->TypeGet()))
     {
         MakeSrcContained(tree, op2);
     }
@@ -2290,7 +2293,7 @@ void Lowering::TreeNodeInfoInitIntrinsic(GenTree* tree)
     switch (tree->gtIntrinsic.gtIntrinsicId)
     {
         case CORINFO_INTRINSIC_Sqrt:
-            if (IsContainableMemoryOp(op1) || op1->IsCnsNonZeroFltOrDbl())
+            if (IsContainableMemoryOp(op1, true) || op1->IsCnsNonZeroFltOrDbl())
             {
                 MakeSrcContained(tree, op1);
             }
@@ -2595,7 +2598,7 @@ void Lowering::TreeNodeInfoInitSIMD(GenTree* tree)
                 info->srcCount = 1;
             }
 
-            if (IsContainableMemoryOp(op1))
+            if (IsContainableMemoryOp(op1, true))
             {
                 MakeSrcContained(tree, op1);
 
@@ -2823,7 +2826,7 @@ void Lowering::TreeNodeInfoInitCast(GenTree* tree)
         // U8 -> R8 conversion requires that the operand be in a register.
         if (castOpType != TYP_ULONG)
         {
-            if (IsContainableMemoryOp(castOp) || castOp->IsCnsNonZeroFltOrDbl())
+            if (IsContainableMemoryOp(castOp, true) || castOp->IsCnsNonZeroFltOrDbl())
             {
                 MakeSrcContained(tree, castOp);
             }
@@ -3114,7 +3117,7 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
         {
             MakeSrcContained(tree, otherOp);
         }
-        else if (IsContainableMemoryOp(otherOp) && ((otherOp == op2) || IsSafeToContainMem(tree, otherOp)))
+        else if (IsContainableMemoryOp(otherOp, true) && ((otherOp == op2) || IsSafeToContainMem(tree, otherOp)))
         {
             MakeSrcContained(tree, otherOp);
         }
@@ -3137,7 +3140,7 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
         // we can treat the MemoryOp as contained.
         if (op1Type == op2Type)
         {
-            if (IsContainableMemoryOp(op1))
+            if (IsContainableMemoryOp(op1, true))
             {
                 MakeSrcContained(tree, op1);
             }
@@ -3186,11 +3189,11 @@ void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)
         // Note that TEST does not have a r,rm encoding like CMP has but we can still
         // contain the second operand because the emitter maps both r,rm and rm,r to
         // the same instruction code. This avoids the need to special case TEST here.
-        if (IsContainableMemoryOp(op2))
+        if (IsContainableMemoryOp(op2, true))
         {
             MakeSrcContained(tree, op2);
         }
-        else if (IsContainableMemoryOp(op1) && IsSafeToContainMem(tree, op1))
+        else if (IsContainableMemoryOp(op1, true) && IsSafeToContainMem(tree, op1))
         {
             MakeSrcContained(tree, op1);
         }
@@ -3281,7 +3284,7 @@ bool Lowering::TreeNodeInfoInitIfRMWMemOp(GenTreePtr storeInd)
         // On Xarch RMW operations require that the non-rmw operand be an immediate or in a register.
         // Therefore, if we have previously marked the indirOpSource as a contained memory op while lowering
         // the binary node, we need to reset that now.
-        if (IsContainableMemoryOp(indirOpSource))
+        if (IsContainableMemoryOp(indirOpSource, true))
         {
             indirOpSource->ClearContained();
         }
@@ -3400,11 +3403,11 @@ void Lowering::TreeNodeInfoInitMul(GenTreePtr tree)
     {
         assert(tree->OperGet() == GT_MUL);
 
-        if (IsContainableMemoryOp(op2) || op2->IsCnsNonZeroFltOrDbl())
+        if (IsContainableMemoryOp(op2, true) || op2->IsCnsNonZeroFltOrDbl())
         {
             MakeSrcContained(tree, op2);
         }
-        else if (op1->IsCnsNonZeroFltOrDbl() || (IsContainableMemoryOp(op1) && IsSafeToContainMem(tree, op1)))
+        else if (op1->IsCnsNonZeroFltOrDbl() || (IsContainableMemoryOp(op1, true) && IsSafeToContainMem(tree, op1)))
         {
             // Since  GT_MUL is commutative, we will try to re-order operands if it is safe to
             // generate more efficient code sequence for the case of GT_MUL(op1=memOp, op2=non-memOp)
@@ -3493,7 +3496,7 @@ void Lowering::TreeNodeInfoInitMul(GenTreePtr tree)
         }
 
         MakeSrcContained(tree, imm); // The imm is always contained
-        if (IsContainableMemoryOp(other))
+        if (IsContainableMemoryOp(other, true))
         {
             memOp = other; // memOp may be contained below
         }
@@ -3504,9 +3507,24 @@ void Lowering::TreeNodeInfoInitMul(GenTreePtr tree)
     // This is because during codegen we use 'tree' type to derive EmitTypeSize.
     // E.g op1 type = byte, op2 type = byte but GT_MUL tree type is int.
     //
-    if (memOp == nullptr && IsContainableMemoryOp(op2))
+    if (memOp == nullptr)
     {
-        memOp = op2;
+        if (IsContainableMemoryOp(op2, true) && (op2->TypeGet() == tree->TypeGet()) && IsSafeToContainMem(tree, op2))
+        {
+            memOp = op2;
+        }
+        else if (IsContainableMemoryOp(op1, true) && (op1->TypeGet() == tree->TypeGet()) &&
+                 IsSafeToContainMem(tree, op1))
+        {
+            memOp = op1;
+        }
+    }
+    else
+    {
+        if ((memOp->TypeGet() != tree->TypeGet()) || !IsSafeToContainMem(tree, memOp))
+        {
+            memOp = nullptr;
+        }
     }
 
     // To generate an LEA we need to force memOp into a register
@@ -3514,7 +3532,7 @@ void Lowering::TreeNodeInfoInitMul(GenTreePtr tree)
     //
     if (!useLeaEncoding)
     {
-        if ((memOp != nullptr) && (memOp->TypeGet() == tree->TypeGet()) && IsSafeToContainMem(tree, memOp))
+        if (memOp != nullptr)
         {
             MakeSrcContained(tree, memOp);
         }
@@ -3643,7 +3661,7 @@ bool Lowering::ExcludeNonByteableRegisters(GenTree* tree)
                 GenTree*  op1      = simdNode->gtGetOp1();
                 GenTree*  op2      = simdNode->gtGetOp2();
                 var_types baseType = simdNode->gtSIMDBaseType;
-                if (!IsContainableMemoryOp(op1) && op2->IsCnsIntOrI() && varTypeIsSmallInt(baseType))
+                if (!IsContainableMemoryOp(op1, true) && op2->IsCnsIntOrI() && varTypeIsSmallInt(baseType))
                 {
                     bool     ZeroOrSignExtnReqd = true;
                     unsigned baseSize           = genTypeSize(baseType);