From dc4eef559f22f033402ef581964528976e9f748a Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Fri, 18 Aug 2017 12:12:40 -0700 Subject: [PATCH] Fix ARM issues with Containment These are changes that should have been part of PR dotnet/coreclr#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. Commit migrated from https://github.com/dotnet/coreclr/commit/5495b89a1368ecb279f2d1b39e69fdab96b9faa4 --- src/coreclr/src/jit/lower.cpp | 27 ++++++++++++++++++---- src/coreclr/src/jit/lowerarmarch.cpp | 34 ++++------------------------ src/coreclr/src/jit/lsraarm.cpp | 7 ------ src/coreclr/src/jit/lsraarm64.cpp | 5 ---- src/coreclr/src/jit/lsraarmarch.cpp | 7 +++--- 5 files changed, 29 insertions(+), 51 deletions(-) diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index d137297fb36..3e3f5a4fc5c 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -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 { diff --git a/src/coreclr/src/jit/lowerarmarch.cpp b/src/coreclr/src/jit/lowerarmarch.cpp index f944b42a054..efd2f8db34a 100644 --- a/src/coreclr/src/jit/lowerarmarch.cpp +++ b/src/coreclr/src/jit/lowerarmarch.cpp @@ -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; - } } //------------------------------------------------------------------------ diff --git a/src/coreclr/src/jit/lsraarm.cpp b/src/coreclr/src/jit/lsraarm.cpp index 53da45b1cfc..9a3d32b8ff6 100644 --- a/src/coreclr/src/jit/lsraarm.cpp +++ b/src/coreclr/src/jit/lsraarm.cpp @@ -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); diff --git a/src/coreclr/src/jit/lsraarm64.cpp b/src/coreclr/src/jit/lsraarm64.cpp index 0e0c2c60c35..3ae8e3f759e 100644 --- a/src/coreclr/src/jit/lsraarm64.cpp +++ b/src/coreclr/src/jit/lsraarm64.cpp @@ -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; diff --git a/src/coreclr/src/jit/lsraarmarch.cpp b/src/coreclr/src/jit/lsraarmarch.cpp index 08fb4ba4092..471fff4930c 100644 --- a/src/coreclr/src/jit/lsraarmarch.cpp +++ b/src/coreclr/src/jit/lsraarmarch.cpp @@ -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); } -- GitLab