Fix ARM issues with Containment
authorCarol Eidt <carol.eidt@microsoft.com>
Fri, 18 Aug 2017 19:12:40 +0000 (12:12 -0700)
committerCarol Eidt <carol.eidt@microsoft.com>
Thu, 24 Aug 2017 03:42:18 +0000 (20:42 -0700)
These are changes that should have been part of PR #13198:
- Correctly contain struct arguments
- Correctly get number of source registers
- Clear register assignments on `GT_FIELD_LIST` nodes (thanks @hseok-oh).
- Remove now-redundant `ContainCheck` methods for armarch targets.

src/jit/lower.cpp
src/jit/lowerarmarch.cpp
src/jit/lsraarm.cpp
src/jit/lsraarm64.cpp
src/jit/lsraarmarch.cpp

index d137297..3e3f5a4 100644 (file)
@@ -859,6 +859,17 @@ GenTreePtr Lowering::NewPutArg(GenTreeCall* call, GenTreePtr arg, fgArgTabEntryP
     isOnStack = info->regNum == REG_STK;
 #endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING
 
+#ifdef _TARGET_ARMARCH_
+    if (varTypeIsStruct(type))
+    {
+        arg->SetContained();
+        if ((arg->OperGet() == GT_OBJ) && (arg->AsObj()->Addr()->OperGet() == GT_LCL_VAR_ADDR))
+        {
+            MakeSrcContained(arg, arg->AsObj()->Addr());
+        }
+    }
+#endif
+
 #ifdef _TARGET_ARM_
     // Struct can be split into register(s) and stack on ARM
     if (info->isSplit)
@@ -873,6 +884,7 @@ GenTreePtr Lowering::NewPutArg(GenTreeCall* call, GenTreePtr arg, fgArgTabEntryP
         putArg = new (comp, GT_PUTARG_SPLIT)
             GenTreePutArgSplit(arg, info->slotNum PUT_STRUCT_ARG_STK_ONLY_ARG(info->numSlots), info->numRegs,
                                call->IsFastTailCall(), call);
+        putArg->gtRegNum = info->regNum;
 
         // If struct argument is morphed to GT_FIELD_LIST node(s),
         // we can know GC info by type of each GT_FIELD_LIST node.
@@ -1272,16 +1284,21 @@ void Lowering::LowerArg(GenTreeCall* call, GenTreePtr* ppArg)
             GenTreePtr argLo = arg->gtGetOp1();
             GenTreePtr argHi = arg->gtGetOp2();
 
-            GenTreeFieldList* fieldList = new (comp, GT_FIELD_LIST) GenTreeFieldList(argLo, 0, TYP_INT, nullptr);
-            (void)new (comp, GT_FIELD_LIST) GenTreeFieldList(argHi, 4, TYP_INT, fieldList);
+            GenTreeFieldList* fieldListLow = new (comp, GT_FIELD_LIST) GenTreeFieldList(argLo, 0, TYP_INT, nullptr);
+            GenTreeFieldList* fieldListHigh =
+                new (comp, GT_FIELD_LIST) GenTreeFieldList(argHi, 4, TYP_INT, fieldListLow);
 
-            putArg           = NewPutArg(call, fieldList, info, TYP_VOID);
+            putArg           = NewPutArg(call, fieldListLow, info, TYP_INT);
             putArg->gtRegNum = info->regNum;
 
             BlockRange().InsertBefore(arg, putArg);
             BlockRange().Remove(arg);
-            *ppArg     = fieldList;
-            info->node = fieldList;
+            *ppArg     = fieldListLow;
+            info->node = fieldListLow;
+
+            // Clear the register assignments on the fieldList nodes, as these are contained.
+            fieldListLow->gtRegNum  = REG_NA;
+            fieldListHigh->gtRegNum = REG_NA;
         }
         else
         {
index f944b42..efd2f8d 100644 (file)
@@ -215,6 +215,7 @@ void Lowering::LowerStoreLoc(GenTreeLclVarCommon* storeLoc)
             }
         }
     }
+    ContainCheckStoreLoc(storeLoc);
 }
 
 //------------------------------------------------------------------------
@@ -447,6 +448,9 @@ void Lowering::LowerCast(GenTree* tree)
         tree->gtOp.gtOp1 = tmp;
         BlockRange().InsertAfter(op1, tmp);
     }
+
+    // Now determine if we have operands that should be contained.
+    ContainCheckCast(tree->AsCast());
 }
 
 //------------------------------------------------------------------------
@@ -516,36 +520,6 @@ void Lowering::ContainCheckCallOperands(GenTreeCall* call)
             }
         }
     }
-    GenTreePtr args = call->gtCallArgs;
-    while (args)
-    {
-        GenTreePtr arg = args->gtOp.gtOp1;
-        if (!(args->gtFlags & GTF_LATE_ARG))
-        {
-            TreeNodeInfo* argInfo = &(arg->gtLsraInfo);
-            if (arg->gtOper == GT_PUTARG_STK)
-            {
-                GenTreePtr putArgChild = arg->gtOp.gtOp1;
-                if (putArgChild->OperGet() == GT_FIELD_LIST)
-                {
-                    MakeSrcContained(arg, putArgChild);
-                }
-                else if (putArgChild->OperGet() == GT_OBJ)
-                {
-                    MakeSrcContained(arg, putArgChild);
-                    GenTreePtr objChild = putArgChild->gtOp.gtOp1;
-                    if (objChild->OperGet() == GT_LCL_VAR_ADDR)
-                    {
-                        // We will generate all of the code for the GT_PUTARG_STK, the GT_OBJ and the GT_LCL_VAR_ADDR
-                        // as one contained operation
-                        //
-                        MakeSrcContained(putArgChild, objChild);
-                    }
-                }
-            }
-        }
-        args = args->gtOp.gtOp2;
-    }
 }
 
 //------------------------------------------------------------------------
index 53da45b..9a3d32b 100644 (file)
@@ -40,8 +40,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 //
 void Lowering::TreeNodeInfoInitReturn(GenTree* tree)
 {
-    ContainCheckRet(tree->AsOp());
-
     TreeNodeInfo* info     = &(tree->gtLsraInfo);
     LinearScan*   l        = m_lsra;
     Compiler*     compiler = comp;
@@ -107,8 +105,6 @@ void Lowering::TreeNodeInfoInitReturn(GenTree* tree)
 
 void Lowering::TreeNodeInfoInitLclHeap(GenTree* tree)
 {
-    ContainCheckLclHeap(tree->AsOp());
-
     TreeNodeInfo* info     = &(tree->gtLsraInfo);
     LinearScan*   l        = m_lsra;
     Compiler*     compiler = comp;
@@ -276,7 +272,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
 
         case GT_CAST:
         {
-            ContainCheckCast(tree->AsCast());
             info->srcCount = 1;
             assert(info->dstCount == 1);
 
@@ -419,7 +414,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
         case GT_AND:
         case GT_OR:
         case GT_XOR:
-            ContainCheckBinary(tree->AsOp());
             info->srcCount = tree->gtOp.gtOp2->isContained() ? 1 : 2;
             assert(info->dstCount == 1);
             break;
@@ -548,7 +542,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
             break;
 
         case GT_ARR_OFFSET:
-            ContainCheckArrOffset(tree->AsArrOffs());
             // This consumes the offset, if any, the arrObj and the effective index,
             // and produces the flattened offset for this dimension.
             assert(info->dstCount == 1);
index 0e0c2c6..3ae8e3f 100644 (file)
@@ -384,7 +384,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
             break;
 
         case GT_LOCKADD:
-            ContainCheckBinary(tree->AsOp());
             info->srcCount = tree->gtOp.gtOp2->isContained() ? 1 : 2;
             assert(info->dstCount == 1);
             break;
@@ -433,7 +432,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
 
         case GT_LCLHEAP:
         {
-            ContainCheckLclHeap(tree->AsOp());
             assert(info->dstCount == 1);
 
             // Need a variable number of temp regs (see genLclHeap() in codegenamd64.cpp):
@@ -575,7 +573,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
             break;
 
         case GT_ARR_OFFSET:
-            ContainCheckArrOffset(tree->AsArrOffs());
             // This consumes the offset, if any, the arrObj and the effective index,
             // and produces the flattened offset for this dimension.
             info->srcCount = tree->gtArrOffs.gtOffset->isContained() ? 2 : 3;
@@ -685,8 +682,6 @@ void Lowering::TreeNodeInfoInit(GenTree* tree)
 //
 void Lowering::TreeNodeInfoInitReturn(GenTree* tree)
 {
-    ContainCheckRet(tree->AsOp());
-
     TreeNodeInfo* info     = &(tree->gtLsraInfo);
     LinearScan*   l        = m_lsra;
     Compiler*     compiler = comp;
index 08fb4ba..471fff4 100644 (file)
@@ -236,8 +236,6 @@ void Lowering::TreeNodeInfoInitIndir(GenTreeIndir* indirTree)
 //
 void Lowering::TreeNodeInfoInitShiftRotate(GenTree* tree)
 {
-    ContainCheckShiftRotate(tree->AsOp());
-
     TreeNodeInfo* info = &(tree->gtLsraInfo);
     LinearScan*   l    = m_lsra;
 
@@ -534,6 +532,7 @@ void Lowering::TreeNodeInfoInitCall(GenTreeCall* call)
         else if (argNode->OperGet() == GT_PUTARG_SPLIT)
         {
             fgArgTabEntryPtr curArgTabEntry = compiler->gtArgEntryByNode(call, argNode);
+            info->srcCount += argNode->AsPutArgSplit()->gtNumRegs;
         }
 #endif
         else
@@ -665,7 +664,7 @@ void Lowering::TreeNodeInfoInitPutArgStk(GenTreePutArgStk* argNode)
                 }
             }
 
-            // We will generate all of the code for the GT_PUTARG_STK and it's child node
+            // We will generate all of the code for the GT_PUTARG_STK and its child node
             // as one contained operation
             //
             argNode->gtLsraInfo.srcCount = putArgChild->gtLsraInfo.srcCount;
@@ -928,7 +927,7 @@ void Lowering::TreeNodeInfoInitBlockStore(GenTreeBlk* blkNode)
                 {
                     // The block size argument is a third argument to GT_STORE_DYN_BLK
                     assert(blkNode->gtOper == GT_STORE_DYN_BLK);
-                    blkNode->gtLsraInfo.setSrcCount(3);
+                    blkNode->gtLsraInfo.srcCount++;
                     GenTree* blockSize = blkNode->AsDynBlk()->gtDynamicSize;
                     blockSize->gtLsraInfo.setSrcCandidates(l, RBM_ARG_2);
                 }