Mark GT_IND nodes that represent VSD targets.

The operand to such a node must be materialized into a register on
certain platforms. This change defines a new IND-specific flag,
GTF_IND_VSD_TGT, and uses that flag to indicate that a particular
GT_IND node represents a VSD target. This flag is then observed by
lowering on the necessary platforms in order to short circuit the
logic that attempts to make the GT_IND's operand contained.

Commit migrated from https://github.com/dotnet/coreclr/commit/0188c07e524986bd6f8587e57f37f97e3143bb0e

diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h
index e0e6a1a..ab20f57 100644 (file)
--- a/src/coreclr/src/jit/gentree.h
+++ b/src/coreclr/src/jit/gentree.h
@@ -865,6 +865,10 @@ public:
 #define GTF_IND_TLS_REF 0x08000000       // GT_IND   -- the target is accessed via TLS
 #define GTF_IND_ASG_LHS 0x04000000       // GT_IND   -- this GT_IND node is (the effective val) of the LHS of an
                                          //             assignment; don't evaluate it independently.
+#define GTF_IND_VSD_TGT GTF_IND_ASG_LHS  // GT_IND   -- this GT_IND node represents the target of an indirect virtual
+                                         //             stub call. This is only valid in the backend, where
+                                         //             GTF_IND_ASG_LHS is not necessary (all such indirections will
+                                         //             be lowered to GT_STOREIND).
 #define GTF_IND_UNALIGNED 0x02000000     // GT_IND   -- the load or store is unaligned (we assume worst case
                                          //             alignment of 1 byte)
 #define GTF_IND_INVARIANT 0x01000000     // GT_IND   -- the target is invariant (a prejit indirection)

diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index f75d0af..be3429a 100644 (file)
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -3156,6 +3156,8 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call)
         // All we have to do here is add an indirection to generate the actual call target.
 
         GenTree* ind = Ind(call->gtCallAddr);
+        ind->gtFlags |= GTF_IND_VSD_TGT;
+
         BlockRange().InsertAfter(call->gtCallAddr, ind);
         call->gtCallAddr = ind;
     }
@@ -3194,7 +3196,9 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call)
 #ifndef _TARGET_X86_
             // on x64 we must materialize the target using specific registers.
             addr->gtRegNum  = REG_VIRTUAL_STUB_PARAM;
+
             indir->gtRegNum = REG_JUMP_THUNK_PARAM;
+            indir->gtFlags |= GTF_IND_VSD_TGT;
 #endif
             result = indir;
         }

diff --git a/src/coreclr/src/jit/lowerxarch.cpp b/src/coreclr/src/jit/lowerxarch.cpp
index b3c19d3..1c1f418 100644 (file)
--- a/src/coreclr/src/jit/lowerxarch.cpp
+++ b/src/coreclr/src/jit/lowerxarch.cpp
@@ -1128,7 +1128,7 @@ void Lowering::TreeNodeInfoInitCall(GenTreeCall* call)
             {
                 assert(ctrlExpr->isIndir());
 
-                ctrlExpr->gtGetOp1()->gtLsraInfo.setSrcCandidates(l, REG_VIRTUAL_STUB_TARGET);
+                ctrlExpr->gtGetOp1()->gtLsraInfo.setSrcCandidates(l, RBM_VIRTUAL_STUB_TARGET);
                 MakeSrcContained(call, ctrlExpr);
             }
             else
@@ -2752,7 +2752,6 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree)
     GenTreePtr index = nullptr;
     unsigned   mul, cns;
     bool       rev;
-    bool       modifiedSources = false;
 
 #ifdef FEATURE_SIMD
     // If indirTree is of TYP_SIMD12, don't mark addr as contained
@@ -2783,16 +2782,22 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree)
     }
 #endif // FEATURE_SIMD
 
-    // These nodes go into an addr mode:
-    // - GT_CLS_VAR_ADDR turns into a constant.
-    // - GT_LCL_VAR_ADDR is a stack addr mode.
-    if ((addr->OperGet() == GT_CLS_VAR_ADDR) || (addr->OperGet() == GT_LCL_VAR_ADDR))
+    if ((indirTree->gtFlags & GTF_IND_VSD_TGT) != 0)
     {
+        // The address of an indirection that is marked as the target of a VSD call must
+        // be computed into a register. Skip any further processing that might otherwise
+        // make it contained.
+    }
+    else if ((addr->OperGet() == GT_CLS_VAR_ADDR) || (addr->OperGet() == GT_LCL_VAR_ADDR))
+    {
+        // These nodes go into an addr mode:
+        // - GT_CLS_VAR_ADDR turns into a constant.
+        // - GT_LCL_VAR_ADDR is a stack addr mode.
+
         // make this contained, it turns into a constant that goes into an addr mode
         MakeSrcContained(indirTree, addr);
     }
-    else if (addr->IsCnsIntOrI() && addr->AsIntConCommon()->FitsInAddrBase(comp) &&
-             addr->gtLsraInfo.getDstCandidates(m_lsra) != RBM_VIRTUAL_STUB_PARAM)
+    else if (addr->IsCnsIntOrI() && addr->AsIntConCommon()->FitsInAddrBase(comp))
     {
         // Amd64:
         // We can mark any pc-relative 32-bit addr as containable, except for a direct VSD call address.
@@ -2814,17 +2819,10 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree)
     }
     else if (addr->OperGet() == GT_LEA)
     {
-        GenTreeAddrMode* lea = addr->AsAddrMode();
-        base                 = lea->Base();
-        index                = lea->Index();
-
-        m_lsra->clearOperandCounts(addr);
-        // The srcCount is decremented because addr is now "contained",
-        // then we account for the base and index below, if they are non-null.
-        info->srcCount--;
+        MakeSrcContained(indirTree, addr);
     }
     else if (comp->codeGen->genCreateAddrMode(addr, -1, true, 0, &rev, &base, &index, &mul, &cns, true /*nogen*/) &&
-             !(modifiedSources = AreSourcesPossiblyModified(indirTree, base, index)))
+             !AreSourcesPossiblyModified(indirTree, base, index))
     {
         // An addressing mode will be constructed that may cause some
         // nodes to not need a register, and cause others' lifetimes to be extended
@@ -2891,7 +2889,16 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree)
             }
         }
         assert(foundBase && foundIndex);
-        info->srcCount--; // it gets incremented below.
+
+        if (base != nullptr)
+        {
+            MakeSrcContained(indirTree, base);
+        }
+
+        if (index != nullptr)
+        {
+            MakeSrcContained(indirTree, index);
+        }
     }
     else if (addr->gtOper == GT_ARR_ELEM)
     {
@@ -2904,22 +2911,6 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree)
         assert(addr->gtLsraInfo.srcCount >= 2);
         addr->gtLsraInfo.srcCount -= 1;
     }
-    else
-    {
-        // it is nothing but a plain indir
-        info->srcCount--; // base gets added in below
-        base = addr;
-    }
-
-    if (base != nullptr)
-    {
-        info->srcCount++;
-    }
-
-    if (index != nullptr && !modifiedSources)
-    {
-        info->srcCount++;
-    }
 }
 
 void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree)

diff --git a/src/coreclr/src/jit/rationalize.cpp b/src/coreclr/src/jit/rationalize.cpp
index 11cba06..19a5984 100644 (file)
--- a/src/coreclr/src/jit/rationalize.cpp
+++ b/src/coreclr/src/jit/rationalize.cpp
@@ -662,6 +662,11 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, ArrayStack<G
             RewriteAddress(use);
             break;
 
+        case GT_IND:
+            // Clear the `GTF_IND_ASG_LHS` flag, which overlaps with `GTF_IND_VSD_TGT`.
+            node->gtFlags &= ~GTF_IND_ASG_LHS;
+            break;
+
         case GT_NOP:
             // fgMorph sometimes inserts NOP nodes between defs and uses
             // supposedly 'to prevent constant folding'. In this case, remove the

diff --git a/src/coreclr/src/jit/target.h b/src/coreclr/src/jit/target.h
index 3111483..cbf9d8a 100644 (file)
--- a/src/coreclr/src/jit/target.h
+++ b/src/coreclr/src/jit/target.h
@@ -600,6 +600,7 @@ typedef unsigned short regPairNoSmall; // arm: need 12 bits
 
   // VSD target address register
   #define REG_VIRTUAL_STUB_TARGET  REG_EAX
+  #define RBM_VIRTUAL_STUB_TARGET  RBM_EAX
 
   // Registers used by PInvoke frame setup
   #define REG_PINVOKE_FRAME        REG_EDI      // EDI is p/invoke "Frame" pointer argument to CORINFO_HELP_INIT_PINVOKE_FRAME helper