From 02ea4634ab830aba62b06462d9eb96f9c9bb01dc Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 21 Sep 2019 12:47:44 +0300 Subject: [PATCH 1/7] Store the `this` call arg in the GenTreeCall::Use --- src/jit/compiler.cpp | 2 +- src/jit/compiler.h | 4 +- src/jit/compiler.hpp | 2 +- src/jit/flowgraph.cpp | 16 ++-- src/jit/gentree.cpp | 144 +++++++++++++++------------- src/jit/gentree.h | 10 +- src/jit/gschecks.cpp | 5 +- src/jit/importer.cpp | 50 +++++----- src/jit/indirectcalltransformer.cpp | 10 +- src/jit/lower.cpp | 8 +- src/jit/morph.cpp | 62 ++++++------ 11 files changed, 164 insertions(+), 149 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index da628466faf1..f50233751990 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -7008,7 +7008,7 @@ void Compiler::compCallArgStats() argTotalCalls++; - if (!call->gtCall.gtCallObjp) + if (call->AsCall()->gtCallThisArg == nullptr) { if (call->gtCall.gtCallType == CT_HELPER) { diff --git a/src/jit/compiler.h b/src/jit/compiler.h index b46682e446d2..357f826fe445 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -10151,9 +10151,9 @@ class GenTreeVisitor { GenTreeCall* const call = node->AsCall(); - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { - result = WalkTree(&call->gtCallObjp, call); + result = WalkTree(&call->gtCallThisArg->NodeRef(), call); if (result == fgWalkResult::WALK_ABORT) { return result; diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp index 74b834f0a48a..1836dc47c5f7 100644 --- a/src/jit/compiler.hpp +++ b/src/jit/compiler.hpp @@ -4412,7 +4412,7 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_CALL: { GenTreeCall* const call = this->AsCall(); - if ((call->gtCallObjp != nullptr) && (visitor(call->gtCallObjp) == VisitResult::Abort)) + if ((call->gtCallThisArg != nullptr) && (visitor(call->gtCallThisArg->GetNode()) == VisitResult::Abort)) { return; } diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index 0a16348d5071..fe158af052ef 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -7461,7 +7461,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, { JITDUMP("optimized\n"); - GenTree* thisPointer = call->gtCallObjp; + GenTree* thisPointer = call->gtCallThisArg->GetNode(); GenTree* targetObjPointers = call->gtCallArgs->GetNode(); GenTreeCall::Use* helperArgs = nullptr; CORINFO_LOOKUP pLookup; @@ -7495,7 +7495,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, { JITDUMP("optimized\n"); - GenTree* thisPointer = call->gtCallObjp; + GenTree* thisPointer = call->gtCallThisArg->GetNode(); GenTree* targetObjPointers = call->gtCallArgs->GetNode(); GenTreeCall::Use* helperArgs = gtNewCallArgs(thisPointer, targetObjPointers); @@ -18781,9 +18781,9 @@ void Compiler::fgSetTreeSeqHelper(GenTree* tree, bool isLIR) case GT_CALL: /* We'll evaluate the 'this' argument value first */ - if (tree->gtCall.gtCallObjp) + if (tree->AsCall()->gtCallThisArg != nullptr) { - fgSetTreeSeqHelper(tree->gtCall.gtCallObjp, isLIR); + fgSetTreeSeqHelper(tree->AsCall()->gtCallThisArg->GetNode(), isLIR); } for (GenTreeCall::Use& use : tree->AsCall()->Args()) @@ -21320,12 +21320,12 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) call = tree->AsCall(); - if (call->gtCallObjp) + if (call->gtCallThisArg != nullptr) { - fgDebugCheckFlags(call->gtCallObjp); - chkFlags |= (call->gtCallObjp->gtFlags & GTF_SIDE_EFFECT); + fgDebugCheckFlags(call->gtCallThisArg->GetNode()); + chkFlags |= (call->gtCallThisArg->GetNode()->gtFlags & GTF_SIDE_EFFECT); - if (call->gtCallObjp->gtFlags & GTF_ASG) + if ((call->gtCallThisArg->GetNode()->gtFlags & GTF_ASG) != 0) { treeFlags |= GTF_ASG; } diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 3302b4345a6b..af4ecbdfaaca 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -1441,7 +1441,13 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) } } - if (!Compare(op1->AsCall()->gtCallObjp, op2->AsCall()->gtCallObjp)) + if ((op1->AsCall()->gtCallThisArg != nullptr) != (op2->AsCall()->gtCallThisArg != nullptr)) + { + return false; + } + + if ((op1->AsCall()->gtCallThisArg != nullptr) && + !Compare(op1->AsCall()->gtCallThisArg->GetNode(), op2->AsCall()->gtCallThisArg->GetNode())) { return false; } @@ -1664,10 +1670,9 @@ bool Compiler::gtHasRef(GenTree* tree, ssize_t lclNum, bool defOnly) break; case GT_CALL: - - if (tree->gtCall.gtCallObjp) + if (tree->AsCall()->gtCallThisArg != nullptr) { - if (gtHasRef(tree->gtCall.gtCallObjp, lclNum, defOnly)) + if (gtHasRef(tree->AsCall()->gtCallThisArg->GetNode(), lclNum, defOnly)) { return true; } @@ -2146,12 +2151,9 @@ unsigned Compiler::gtHashValue(GenTree* tree) break; case GT_CALL: - - if (tree->gtCall.gtCallObjp && tree->gtCall.gtCallObjp->gtOper != GT_NOP) + if ((tree->AsCall()->gtCallThisArg != nullptr) && !tree->AsCall()->gtCallThisArg->GetNode()->OperIs(GT_NOP)) { - temp = tree->gtCall.gtCallObjp; - assert(temp); - hash = genTreeHashAdd(hash, gtHashValue(temp)); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCall()->gtCallThisArg->GetNode())); } for (GenTreeCall::Use& use : tree->AsCall()->Args()) @@ -4211,9 +4213,9 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) /* Evaluate the 'this' argument, if present */ - if (tree->gtCall.gtCallObjp) + if (tree->AsCall()->gtCallThisArg != nullptr) { - GenTree* thisVal = tree->gtCall.gtCallObjp; + GenTree* thisVal = tree->AsCall()->gtCallThisArg->GetNode(); lvl2 = gtSetEvalOrder(thisVal); if (level < lvl2) @@ -4760,9 +4762,9 @@ GenTree** GenTree::gtGetChildPointer(GenTree* parent) const { GenTreeCall* call = parent->AsCall(); - if (this == call->gtCallObjp) + if ((call->gtCallThisArg != nullptr) && (this == call->gtCallThisArg->GetNode())) { - return &(call->gtCallObjp); + return &call->gtCallThisArg->NodeRef(); } for (GenTreeCall::Use& use : call->Args()) { @@ -5063,9 +5065,9 @@ bool GenTree::TryGetUse(GenTree* def, GenTree*** use) case GT_CALL: { GenTreeCall* const call = this->AsCall(); - if (def == call->gtCallObjp) + if ((call->gtCallThisArg != nullptr) && (def == call->gtCallThisArg->GetNode())) { - *use = &call->gtCallObjp; + *use = &call->gtCallThisArg->NodeRef(); return true; } if (def == call->gtControlExpr) @@ -5947,7 +5949,7 @@ GenTreeCall* Compiler::gtNewCallNode( node->gtCallType = callType; node->gtCallMethHnd = callHnd; node->gtCallArgs = args; - node->gtCallObjp = nullptr; + node->gtCallThisArg = nullptr; node->fgArgInfo = nullptr; node->callSig = nullptr; node->gtRetClsHnd = nullptr; @@ -6226,7 +6228,7 @@ fgArgTabEntry* Compiler::gtArgEntryByNode(GenTreeCall* call, GenTree* node) } else // (curArgTabEntry->parent == NULL) { - if (call->gtCallObjp == node) + if ((call->gtCallThisArg != nullptr) && (call->gtCallThisArg->GetNode() == node)) { return curArgTabEntry; } @@ -7591,7 +7593,16 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree, unsigned addFlag { GenTreeCall* copy = new (this, GT_CALL) GenTreeCall(tree->TypeGet()); - copy->gtCallObjp = tree->gtCallObjp ? gtCloneExpr(tree->gtCallObjp, addFlags, deepVarNum, deepVarVal) : nullptr; + if (tree->gtCallThisArg == nullptr) + { + copy->gtCallThisArg = nullptr; + } + else + { + copy->gtCallThisArg = + gtNewCallArgs(gtCloneExpr(tree->gtCallThisArg->GetNode(), addFlags, deepVarNum, deepVarVal)); + } + copy->gtCallMoreFlags = tree->gtCallMoreFlags; copy->gtCallArgs = nullptr; copy->gtCallLateArgs = nullptr; @@ -8046,51 +8057,54 @@ bool Compiler::gtCompareTree(GenTree* op1, GenTree* op2) GenTree* Compiler::gtGetThisArg(GenTreeCall* call) { - GenTree* thisArg = call->gtCallObjp; - if (thisArg != nullptr) + if (call->gtCallThisArg == nullptr) + { + return nullptr; + } + + GenTree* thisArg = call->gtCallThisArg->GetNode(); + if (thisArg->OperIs(GT_NOP, GT_ASG) == false) { - if (thisArg->OperIs(GT_NOP, GT_ASG) == false) + if ((thisArg->gtFlags & GTF_LATE_ARG) == 0) { - if ((thisArg->gtFlags & GTF_LATE_ARG) == 0) - { - return call->gtCallObjp; - } + return thisArg; } + } - if (call->gtCallLateArgs != nullptr) - { - unsigned argNum = 0; - fgArgTabEntry* thisArgTabEntry = gtArgEntryByArgNum(call, argNum); - GenTree* result = thisArgTabEntry->node; + if (call->gtCallLateArgs != nullptr) + { + unsigned argNum = 0; + fgArgTabEntry* thisArgTabEntry = gtArgEntryByArgNum(call, argNum); + GenTree* result = thisArgTabEntry->node; - // Assert if we used DEBUG_DESTROY_NODE. - assert(result->gtOper != GT_COUNT); + // Assert if we used DEBUG_DESTROY_NODE. + assert(result->gtOper != GT_COUNT); #if !FEATURE_FIXED_OUT_ARGS && defined(DEBUG) - // Check that call->fgArgInfo used in gtArgEntryByArgNum was not - // left outdated by assertion propogation updates. - // There is no information about registers of late args for platforms - // with FEATURE_FIXED_OUT_ARGS that is why this debug check is under - // !FEATURE_FIXED_OUT_ARGS. - regNumber thisReg = REG_ARG_0; - regList list = call->regArgList; - int index = 0; - for (GenTreeCall::Use& use : call->LateArgs()) + // Check that call->fgArgInfo used in gtArgEntryByArgNum was not + // left outdated by assertion propogation updates. + // There is no information about registers of late args for platforms + // with FEATURE_FIXED_OUT_ARGS that is why this debug check is under + // !FEATURE_FIXED_OUT_ARGS. + regNumber thisReg = REG_ARG_0; + regList list = call->regArgList; + int index = 0; + for (GenTreeCall::Use& use : call->LateArgs()) + { + assert(index < call->regArgListCount); + regNumber curArgReg = list[index]; + if (curArgReg == thisReg) { - assert(index < call->regArgListCount); - regNumber curArgReg = list[index]; - if (curArgReg == thisReg) - { - assert(result == use.GetNode()); - } - - index++; + assert(result == use.GetNode()); } -#endif // !FEATURE_FIXED_OUT_ARGS && defined(DEBUG) - return result; + index++; } +#endif // !FEATURE_FIXED_OUT_ARGS && defined(DEBUG) + + return result; } + return nullptr; } @@ -8275,7 +8289,7 @@ unsigned GenTree::NumChildren() { GenTreeCall* call = AsCall(); unsigned res = 0; - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { res++; } @@ -8452,11 +8466,11 @@ GenTree* GenTree::GetChild(unsigned childNum) { GenTreeCall* call = AsCall(); - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { if (childNum == 0) { - return call->gtCallObjp; + return call->gtCallThisArg->GetNode(); } childNum--; @@ -9008,9 +9022,9 @@ void GenTreeUseEdgeIterator::AdvanceCall() case CALL_INSTANCE: m_statePtr = call->gtCallArgs; m_advance = &GenTreeUseEdgeIterator::AdvanceCall; - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { - m_edge = &call->gtCallObjp; + m_edge = &call->gtCallThisArg->NodeRef(); return; } __fallthrough; @@ -11056,10 +11070,9 @@ void Compiler::gtDispTree(GenTree* tree, bufp = &buf[0]; - if ((call->gtCallObjp != nullptr) && (call->gtCallObjp->gtOper != GT_NOP) && - (!call->gtCallObjp->IsArgPlaceHolderNode())) + if ((call->gtCallThisArg != nullptr) && !call->gtCallThisArg->GetNode()->OperIs(GT_NOP, GT_ARGPLACE)) { - if (call->gtCallObjp->gtOper == GT_ASG) + if (call->gtCallThisArg->GetNode()->OperIs(GT_ASG)) { sprintf_s(bufp, sizeof(buf), "this SETUP%c", 0); } @@ -11067,8 +11080,8 @@ void Compiler::gtDispTree(GenTree* tree, { sprintf_s(bufp, sizeof(buf), "this in %s%c", compRegVarName(REG_ARG_0), 0); } - gtDispChild(call->gtCallObjp, indentStack, (call->gtCallObjp == lastChild) ? IIArcBottom : IIArc, - bufp, topOnly); + gtDispChild(call->gtCallThisArg->GetNode(), indentStack, + (call->gtCallThisArg->GetNode() == lastChild) ? IIArcBottom : IIArc, bufp, topOnly); } if (call->gtCallArgs) @@ -11450,7 +11463,7 @@ void Compiler::gtDispArgList(GenTreeCall* call, IndentStack* indentStack) unsigned argnum = 0; - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { argnum++; } @@ -11613,7 +11626,7 @@ void Compiler::gtDispLIRNode(GenTree* node, const char* prefixMsg /* = nullptr * if (nodeIsCall) { GenTreeCall* call = node->AsCall(); - if (operand == call->gtCallObjp) + if ((call->gtCallThisArg != nullptr) && (operand == call->gtCallThisArg->GetNode())) { sprintf_s(buf, sizeof(buf), "this in %s", compRegVarName(REG_ARG_0)); displayOperand(operand, buf, operandArc, indentStack, prefixIndent); @@ -11917,7 +11930,7 @@ GenTree* Compiler::gtFoldExprCall(GenTreeCall* call) if (ni == NI_System_Enum_HasFlag) { - GenTree* thisOp = call->gtCallObjp; + GenTree* thisOp = call->gtCallThisArg->GetNode(); GenTree* flagOp = call->gtCallArgs->GetNode(); GenTree* result = gtOptimizeEnumHasFlag(thisOp, flagOp); @@ -12274,8 +12287,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) } else { - assert(opOther->OperGet() == GT_CALL); - objOp = opOther->gtCall.gtCallObjp; + objOp = opOther->AsCall()->gtCallThisArg->GetNode(); } GenTree* const objMT = gtNewOperNode(GT_IND, TYP_I_IMPL, objOp); diff --git a/src/jit/gentree.h b/src/jit/gentree.h index c5d88e6b219b..8f916386b600 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3407,11 +3407,11 @@ struct GenTreeCall final : public GenTree } }; - GenTree* gtCallObjp; // The instance argument ('this' pointer) - Use* gtCallArgs; // The list of arguments in original evaluation order - Use* gtCallLateArgs; // On x86: The register arguments in an optimal order - // On ARM/x64: - also includes any outgoing arg space arguments - // - that were evaluated into a temp LclVar + Use* gtCallThisArg; // The instance argument ('this' pointer) + Use* gtCallArgs; // The list of arguments in original evaluation order + Use* gtCallLateArgs; // On x86: The register arguments in an optimal order + // On ARM/x64: - also includes any outgoing arg space arguments + // - that were evaluated into a temp LclVar fgArgInfo* fgArgInfo; UseList Args() diff --git a/src/jit/gschecks.cpp b/src/jit/gschecks.cpp index 5e7ae07c5c9d..26c7fb81d1b4 100644 --- a/src/jit/gschecks.cpp +++ b/src/jit/gschecks.cpp @@ -190,10 +190,11 @@ Compiler::fgWalkResult Compiler::gsMarkPtrsAndAssignGroups(GenTree** pTree, fgWa newState.isUnderIndir = false; newState.isAssignSrc = false; { - if (tree->gtCall.gtCallObjp) + if (tree->AsCall()->gtCallThisArg != nullptr) { newState.isUnderIndir = true; - comp->fgWalkTreePre(&tree->gtCall.gtCallObjp, gsMarkPtrsAndAssignGroups, (void*)&newState); + comp->fgWalkTreePre(&tree->AsCall()->gtCallThisArg->NodeRef(), gsMarkPtrsAndAssignGroups, + (void*)&newState); } for (GenTreeCall::Use& use : tree->AsCall()->Args()) diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index ba49a2d0bdf4..2a9faf4d816a 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -7688,8 +7688,8 @@ var_types Compiler::impImportCall(OPCODE opcode, // Create the actual call node - call = gtNewIndCallNode(fptr, callRetTyp, args, ilOffset); - call->gtCall.gtCallObjp = thisPtrCopy; + call = gtNewIndCallNode(fptr, callRetTyp, args, ilOffset); + call->AsCall()->gtCallThisArg = gtNewCallArgs(thisPtrCopy); call->gtFlags |= GTF_EXCEPT | (fptr->gtFlags & GTF_GLOB_EFFECT); if ((sig->sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_CORERT_ABI)) @@ -8243,7 +8243,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // Store the "this" value in the call call->gtFlags |= obj->gtFlags & GTF_GLOB_EFFECT; - call->gtCall.gtCallObjp = obj; + call->AsCall()->gtCallThisArg = gtNewCallArgs(obj); // Is this a virtual or interface call? if (call->gtCall.IsVirtual()) @@ -8403,7 +8403,6 @@ var_types Compiler::impImportCall(OPCODE opcode, // True virtual or indirect calls, shouldn't pass in a callee handle. CORINFO_METHOD_HANDLE exactCalleeHnd = ((call->gtCall.gtCallType != CT_USER_FUNC) || call->gtCall.IsVirtual()) ? nullptr : methHnd; - GenTree* thisArg = call->gtCall.gtCallObjp; if (info.compCompHnd->canTailCall(info.compMethodHnd, methHnd, exactCalleeHnd, explicitTailCall)) { @@ -8505,8 +8504,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (compIsForInlining() && opcode == CEE_CALLVIRT) { - GenTree* callObj = call->gtCall.gtCallObjp; - assert(callObj != nullptr); + GenTree* callObj = call->AsCall()->gtCallThisArg->GetNode(); if ((call->gtCall.IsVirtual() || (call->gtFlags & GTF_CALL_NULLCHECK)) && impInlineIsGuaranteedThisDerefBeforeAnySideEffects(nullptr, call->AsCall()->gtCallArgs, callObj, @@ -18187,7 +18185,7 @@ void Compiler::impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, I // Check if the callee has the same 'this' as the root. if (pInlineInfo != nullptr) { - GenTree* thisArg = pInlineInfo->iciCall->gtCall.gtCallObjp; + GenTree* thisArg = pInlineInfo->iciCall->AsCall()->gtCallThisArg->GetNode(); assert(thisArg); bool isSameThis = impIsThis(thisArg); inlineResult->NoteBool(InlineObservation::CALLSITE_IS_SAME_THIS, isSameThis); @@ -18460,7 +18458,7 @@ void Compiler::impCheckCanInline(GenTreeCall* call, if (dwRestrictions & INLINE_SAME_THIS) { - GenTree* thisArg = pParam->call->gtCall.gtCallObjp; + GenTree* thisArg = pParam->call->gtCall.gtCallThisArg->GetNode(); assert(thisArg); if (!pParam->pThis->impIsThis(thisArg)) @@ -18719,15 +18717,15 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) memset(inlArgInfo, 0, (MAX_INL_ARGS + 1) * sizeof(inlArgInfo[0])); - GenTree* thisArg = call->gtCallObjp; - unsigned argCnt = 0; // Count of the arguments + GenTreeCall::Use* thisArg = call->gtCallThisArg; + unsigned argCnt = 0; // Count of the arguments assert((methInfo->args.hasThis()) == (thisArg != nullptr)); if (thisArg) { inlArgInfo[0].argIsThis = true; - GenTree* actualThisArg = thisArg->gtRetExprVal(); + GenTree* actualThisArg = thisArg->GetNode()->gtRetExprVal(); impInlineRecordArgInfo(pInlineInfo, actualThisArg, argCnt, inlineResult); if (inlineResult->IsFailure()) @@ -18743,7 +18741,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) bool hasTypeCtxtArg = (methInfo->args.callConv & CORINFO_CALLCONV_PARAMTYPE) != 0; #if USER_ARGS_COME_LAST - unsigned typeCtxtArg = thisArg ? 1 : 0; + unsigned typeCtxtArg = (thisArg != nullptr) ? 1 : 0; #else // USER_ARGS_COME_LAST unsigned typeCtxtArg = methInfo->args.totalILArgs(); #endif // USER_ARGS_COME_LAST @@ -18784,7 +18782,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) /* We have typeless opcodes, get type information from the signature */ - if (thisArg) + if (thisArg != nullptr) { var_types sigType; @@ -18816,11 +18814,13 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) #endif // FEATURE_SIMD lclVarInfo[0].lclTypeInfo = sigType; - assert(varTypeIsGC(thisArg->gtType) || // "this" is managed - (thisArg->gtType == TYP_I_IMPL && // "this" is unmgd but the method's class doesnt care + GenTree* thisArgNode = thisArg->GetNode(); + + assert(varTypeIsGC(thisArgNode->TypeGet()) || // "this" is managed + ((thisArgNode->TypeGet() == TYP_I_IMPL) && // "this" is unmgd but the method's class doesnt care (clsAttr & CORINFO_FLG_VALUECLASS))); - if (genActualType(thisArg->gtType) != genActualType(sigType)) + if (genActualType(thisArgNode->TypeGet()) != genActualType(sigType)) { if (sigType == TYP_REF) { @@ -18832,20 +18832,20 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) /* This can only happen with byrefs <-> ints/shorts */ assert(genActualType(sigType) == TYP_I_IMPL || sigType == TYP_BYREF); - assert(genActualType(thisArg->gtType) == TYP_I_IMPL || thisArg->gtType == TYP_BYREF); + assert((genActualType(thisArgNode->TypeGet()) == TYP_I_IMPL) || (thisArgNode->TypeGet() == TYP_BYREF)); if (sigType == TYP_BYREF) { lclVarInfo[0].lclVerTypeInfo = typeInfo(varType2tiType(TYP_I_IMPL)); } - else if (thisArg->gtType == TYP_BYREF) + else if (thisArgNode->TypeGet() == TYP_BYREF) { assert(sigType == TYP_I_IMPL); /* If possible change the BYREF to an int */ - if (thisArg->IsLocalAddrExpr() != nullptr) + if (thisArgNode->IsLocalAddrExpr() != nullptr) { - thisArg->gtType = TYP_I_IMPL; + thisArgNode->gtType = TYP_I_IMPL; lclVarInfo[0].lclVerTypeInfo = typeInfo(varType2tiType(TYP_I_IMPL)); } else @@ -20020,7 +20020,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, } // See what we know about the type of 'this' in the call. - GenTree* thisObj = call->gtCallObjp->gtEffectiveVal(false); + GenTree* thisObj = call->gtCallThisArg->GetNode()->gtEffectiveVal(false); GenTree* actualThisObj = nullptr; bool isExact = false; bool objIsNonNull = false; @@ -20333,7 +20333,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, { // Pass the local var as this and the type handle as a new arg JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table arg\n"); - call->gtCallObjp = localCopyThis; + call->gtCallThisArg = gtNewCallArgs(localCopyThis); call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; // Prepend for R2L arg passing or empty L2R passing @@ -20380,7 +20380,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (localCopyThis != nullptr) { JITDUMP("Success! invoking unboxed entry point on local copy\n"); - call->gtCallObjp = localCopyThis; + call->gtCallThisArg = gtNewCallArgs(localCopyThis); call->gtCallMethHnd = unboxedEntryMethod; call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; derivedMethod = unboxedEntryMethod; @@ -20557,9 +20557,9 @@ class SpillRetExprHelper comp->fgWalkTreePre(&use.NodeRef(), SpillRetExprVisitor, this); } - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { - comp->fgWalkTreePre(&call->gtCallObjp, SpillRetExprVisitor, this); + comp->fgWalkTreePre(&call->gtCallThisArg->NodeRef(), SpillRetExprVisitor, this); } } diff --git a/src/jit/indirectcalltransformer.cpp b/src/jit/indirectcalltransformer.cpp index fb61dd2e048b..a28694d935b0 100644 --- a/src/jit/indirectcalltransformer.cpp +++ b/src/jit/indirectcalltransformer.cpp @@ -553,7 +553,7 @@ class IndirectCallTransformer checkBlock = CreateAndInsertBasicBlock(BBJ_COND, currBlock); // Fetch method table from object arg to call. - GenTree* thisTree = compiler->gtCloneExpr(origCall->gtCallObjp); + GenTree* thisTree = compiler->gtCloneExpr(origCall->gtCallThisArg->GetNode()); // Create temp for this if the tree is costly. if (!thisTree->IsLocal()) @@ -568,7 +568,7 @@ class IndirectCallTransformer // Propagate the new this to the call. Must be a new expr as the call // will live on in the else block and thisTree is used below. - origCall->gtCallObjp = compiler->gtNewLclvNode(thisTempNum, TYP_REF); + origCall->gtCallThisArg = compiler->gtNewCallArgs(compiler->gtNewLclvNode(thisTempNum, TYP_REF)); } GenTree* methodTable = compiler->gtNewIndir(TYP_I_IMPL, thisTree); @@ -657,14 +657,14 @@ class IndirectCallTransformer // copy 'this' to temp with exact type. const unsigned thisTemp = compiler->lvaGrabTemp(false DEBUGARG("guarded devirt this exact temp")); - GenTree* clonedObj = compiler->gtCloneExpr(origCall->gtCallObjp); + GenTree* clonedObj = compiler->gtCloneExpr(origCall->gtCallThisArg->GetNode()); GenTree* assign = compiler->gtNewTempAssign(thisTemp, clonedObj); compiler->lvaSetClass(thisTemp, clsHnd, true); compiler->fgNewStmtAtEnd(thenBlock, assign); // Clone call. Note we must use the special candidate helper. - GenTreeCall* call = compiler->gtCloneCandidateCall(origCall); - call->gtCallObjp = compiler->gtNewLclvNode(thisTemp, TYP_REF); + GenTreeCall* call = compiler->gtCloneCandidateCall(origCall); + call->gtCallThisArg = compiler->gtNewCallArgs(compiler->gtNewLclvNode(thisTemp, TYP_REF)); call->SetIsGuarded(); JITDUMP("Direct call [%06u] in block BB%02u\n", compiler->dspTreeID(call), thenBlock->bbNum); diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index f89ad507f7b4..1e0fb358bf02 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -1491,9 +1491,9 @@ GenTree* Lowering::LowerFloatArgReg(GenTree* arg, regNumber regNum) void Lowering::LowerArgsForCall(GenTreeCall* call) { JITDUMP("objp:\n======\n"); - if (call->gtCallObjp) + if (call->gtCallThisArg != nullptr) { - LowerArg(call, &call->gtCallObjp); + LowerArg(call, &call->gtCallThisArg->NodeRef()); } JITDUMP("\nargs:\n======\n"); @@ -5429,9 +5429,9 @@ void Lowering::CheckCallArg(GenTree* arg) // void Lowering::CheckCall(GenTreeCall* call) { - if (call->gtCallObjp != nullptr) + if (call->gtCallThisArg != nullptr) { - CheckCallArg(call->gtCallObjp); + CheckCallArg(call->gtCallThisArg->GetNode()); } for (GenTreeCall::Use& use : call->Args()) diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index f44ac6e39fb2..f5f40a7d78f8 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -65,8 +65,8 @@ GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeCall: tree->gtCall.gtCallType = CT_HELPER; tree->gtCall.gtCallMethHnd = eeFindHelper(helper); + tree->gtCall.gtCallThisArg = nullptr; tree->gtCall.gtCallArgs = args; - tree->gtCall.gtCallObjp = nullptr; tree->gtCall.gtCallLateArgs = nullptr; tree->gtCall.fgArgInfo = nullptr; tree->gtCall.gtRetClsHnd = nullptr; @@ -937,7 +937,8 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) assert(oldArgInfo->argsComplete); - // We create local, artificial GenTreeArgLists that includes the gtCallObjp, if that exists, as first argument, + // We create local, artificial GenTreeArgLists that includes the gtCallThisArg node, if that exists, as first + // argument, // so we can iterate over these argument lists more uniformly. // Need to provide a temporary non-null first arguments to these constructors: if we use them, we'll replace them GenTreeCall::Use* newArgs; @@ -945,18 +946,18 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) GenTreeCall::Use* oldArgs; GenTreeCall::Use oldArgObjp(oldCall, oldCall->gtCallArgs); - if (newCall->gtCallObjp == nullptr) + if (newCall->gtCallThisArg == nullptr) { - assert(oldCall->gtCallObjp == nullptr); + assert(oldCall->gtCallThisArg == nullptr); newArgs = newCall->gtCallArgs; oldArgs = oldCall->gtCallArgs; } else { - assert(oldCall->gtCallObjp != nullptr); - newArgObjp.SetNode(newCall->gtCallObjp); + assert(oldCall->gtCallThisArg != nullptr); + newArgObjp.SetNode(newCall->gtCallThisArg->GetNode()); newArgs = &newArgObjp; - oldArgObjp.SetNode(oldCall->gtCallObjp); + oldArgObjp.SetNode(oldCall->gtCallThisArg->GetNode()); oldArgs = &oldArgObjp; } @@ -2419,10 +2420,10 @@ void fgArgInfo::EvalArgsToTemps() } else { - /* must be the gtCallObjp */ - noway_assert(callTree->gtCall.gtCallObjp == argx); + // must be the "this" arg + noway_assert(callTree->AsCall()->gtCallThisArg->GetNode() == argx); - callTree->gtCall.gtCallObjp = setupArg; + callTree->AsCall()->gtCallThisArg->SetNode(setupArg); } } @@ -2720,7 +2721,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) unsigned numArgs = 0; // First we need to count the args - if (call->gtCallObjp) + if (call->gtCallThisArg != nullptr) { numArgs++; } @@ -2759,15 +2760,15 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) // to achieve its goal for delegate VSD call. See COMDelegate::NeedsWrapperDelegate() in the VM for details. else if (call->gtCallMoreFlags & GTF_CALL_M_SECURE_DELEGATE_INV) { - GenTree* arg = call->gtCallObjp; + GenTree* arg = call->gtCallThisArg->GetNode(); if (arg->OperIsLocal()) { arg = gtClone(arg, true); } else { - GenTree* tmp = fgInsertCommaFormTemp(&arg); - call->gtCallObjp = arg; + GenTree* tmp = fgInsertCommaFormTemp(&arg); + call->gtCallThisArg->SetNode(arg); call->gtFlags |= GTF_ASG; arg = tmp; } @@ -2917,9 +2918,9 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) call->fgArgInfo = new (this, CMK_Unknown) fgArgInfo(this, call, numArgs); // Add the 'this' argument value, if present. - argx = call->gtCallObjp; - if (argx != nullptr) + if (call->gtCallThisArg != nullptr) { + argx = call->gtCallThisArg->GetNode(); assert(argIndex == 0); assert(call->gtCallType == CT_USER_FUNC || call->gtCallType == CT_INDIRECT); assert(varTypeIsGC(argx) || (argx->gtType == TYP_I_IMPL)); @@ -3714,12 +3715,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // This information is used later to contruct the gtCallLateArgs */ // Process the 'this' argument value, if present. - argx = call->gtCallObjp; - if (argx) + if (call->gtCallThisArg != nullptr) { + argx = call->gtCallThisArg->GetNode(); fgArgTabEntry* thisArgEntry = call->fgArgInfo->GetArgEntry(0, reMorphing); argx = fgMorphTree(argx); - call->gtCallObjp = argx; + call->gtCallThisArg->SetNode(argx); // This is a register argument - possibly update it in the table. call->fgArgInfo->UpdateRegArg(thisArgEntry, argx, reMorphing); flagsSummary |= argx->gtFlags; @@ -7621,10 +7622,10 @@ void Compiler::fgMorphTailCallViaHelper(GenTreeCall* call, void* pfnCopyArgs) // First move the this pointer (if any) onto the regular arg list GenTree* thisPtr = NULL; - if (call->gtCallObjp) + if (call->gtCallThisArg != nullptr) { - GenTree* objp = call->gtCallObjp; - call->gtCallObjp = NULL; + GenTree* objp = call->gtCallThisArg->GetNode(); + call->gtCallThisArg = nullptr; if ((call->gtFlags & GTF_CALL_NULLCHECK) || call->IsVirtualVtable()) { @@ -7826,11 +7827,11 @@ void Compiler::fgMorphTailCallViaHelper(GenTreeCall* call, void* pfnCopyArgs) // for those cases where call lowering creates an embedded form temp of "this", we will // create a temp here, early, that will later get morphed correctly. - if (call->gtCallObjp) + if (call->gtCallThisArg != nullptr) { - GenTree* thisPtr = nullptr; - GenTree* objp = call->gtCallObjp; - call->gtCallObjp = nullptr; + GenTree* thisPtr = nullptr; + GenTree* objp = call->gtCallThisArg->GetNode(); + call->gtCallThisArg = nullptr; #ifdef _TARGET_X86_ if ((call->IsDelegateInvoke() || call->IsVirtualVtable()) && !objp->IsLocal()) @@ -8028,10 +8029,10 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa IL_OFFSETX callILOffset = lastStmt->gtStmtILoffsx; // Hoist arg setup statement for the 'this' argument. - GenTree* thisArg = recursiveTailCall->gtCallObjp; - if (thisArg && !thisArg->IsNothingNode() && !thisArg->IsArgPlaceHolderNode()) + GenTreeCall::Use* thisArg = recursiveTailCall->gtCallThisArg; + if ((thisArg != nullptr) && !thisArg->GetNode()->IsNothingNode() && !thisArg->GetNode()->IsArgPlaceHolderNode()) { - Statement* thisArgStmt = gtNewStmt(thisArg, callILOffset); + Statement* thisArgStmt = gtNewStmt(thisArg->GetNode(), callILOffset); fgInsertStmtBefore(block, earlyArgInsertionPoint, thisArgStmt); } @@ -18241,7 +18242,8 @@ class LocalAddressVisitor final : public GenTreeVisitor // of calls - it would be highly unsual for a struct member method to attempt to access memory // beyond "this" instance. And calling struct member methods is common enough that attempting to // mark the entire struct as address exposed results in CQ regressions. - bool isThisArg = user->IsCall() && (val.Node() == user->AsCall()->gtCallObjp); + bool isThisArg = user->IsCall() && (user->AsCall()->gtCallThisArg != nullptr) && + (val.Node() == user->AsCall()->gtCallThisArg->GetNode()); bool exposeParentLcl = varDsc->lvIsStructField && !isThisArg; m_compiler->lvaSetVarAddrExposed(exposeParentLcl ? varDsc->lvParentLcl : val.LclNum()); From 43b5dc440b775f06b8af00b0d08cd3337d4f1274 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 21 Sep 2019 14:24:52 +0300 Subject: [PATCH 2/7] Store gtCallThisArg in fgArgTabEntry --- src/jit/compiler.h | 4 +-- src/jit/gentree.cpp | 25 ++----------- src/jit/morph.cpp | 88 ++++++++++++++++----------------------------- 3 files changed, 35 insertions(+), 82 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 357f826fe445..ac2774d6fea8 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -1333,8 +1333,7 @@ struct fgArgTabEntry { GenTree* node; // Initially points to `use`'s node, but if the argument is replaced with an GT_ASG or // placeholder it will point at the actual argument node in the gtCallLateArgs list. - GenTreeCall::Use* use; // Points at the GenTreeCall::Use in the gtCallArgs for this argument - // or nullptr for the `this` argument which does not have a corresponding GenTreeCall::Use. + GenTreeCall::Use* use; // Points to the argument's GenTreeCall::Use in gtCallArgs or gtCallThisArg. unsigned argNum; // The original argument number, also specifies the required argument evaluation order from the IL @@ -2596,7 +2595,6 @@ class Compiler static fgArgTabEntry* gtArgEntryByNode(GenTreeCall* call, GenTree* node); fgArgTabEntry* gtArgEntryByLateArgIndex(GenTreeCall* call, unsigned lateArgInx); static GenTree* gtArgNodeByLateArgInx(GenTreeCall* call, unsigned lateArgInx); - bool gtArgIsThisPtr(fgArgTabEntry* argEntry); GenTree* gtNewAssignNode(GenTree* dst, GenTree* src); diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index af4ecbdfaaca..9a79928536ee 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -6219,19 +6219,9 @@ fgArgTabEntry* Compiler::gtArgEntryByNode(GenTreeCall* call, GenTree* node) { return curArgTabEntry; } - else if (curArgTabEntry->use != nullptr) + else if (curArgTabEntry->use->GetNode() == node) { - if (curArgTabEntry->use->GetNode() == node) - { - return curArgTabEntry; - } - } - else // (curArgTabEntry->parent == NULL) - { - if ((call->gtCallThisArg != nullptr) && (call->gtCallThisArg->GetNode() == node)) - { - return curArgTabEntry; - } + return curArgTabEntry; } } noway_assert(!"gtArgEntryByNode: node not found"); @@ -6294,15 +6284,6 @@ GenTree* Compiler::gtArgNodeByLateArgInx(GenTreeCall* call, unsigned lateArgInx) return argx; } -/***************************************************************************** - * - * Given an fgArgTabEntry*, return true if it is the 'this' pointer argument. - */ -bool Compiler::gtArgIsThisPtr(fgArgTabEntry* argEntry) -{ - return (argEntry->use == nullptr); -} - /***************************************************************************** * * Create a node that will assign 'src' to 'dst'. @@ -11359,7 +11340,7 @@ void Compiler::gtGetLateArgMsg( else #endif { - if (gtArgIsThisPtr(curArgTabEntry)) + if (curArgTabEntry->use == call->gtCallThisArg) { sprintf_s(bufp, bufLength, "this in %s%c", compRegVarName(argReg), 0); } diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index f5f40a7d78f8..3255630a7937 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -937,28 +937,17 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) assert(oldArgInfo->argsComplete); - // We create local, artificial GenTreeArgLists that includes the gtCallThisArg node, if that exists, as first - // argument, - // so we can iterate over these argument lists more uniformly. - // Need to provide a temporary non-null first arguments to these constructors: if we use them, we'll replace them - GenTreeCall::Use* newArgs; - GenTreeCall::Use newArgObjp(newCall, newCall->gtCallArgs); - GenTreeCall::Use* oldArgs; - GenTreeCall::Use oldArgObjp(oldCall, oldCall->gtCallArgs); + GenTreeCall::Use* newArgs = newCall->gtCallArgs; + GenTreeCall::Use* oldArgs = oldCall->gtCallArgs; - if (newCall->gtCallThisArg == nullptr) + if (newCall->gtCallThisArg != nullptr) { - assert(oldCall->gtCallThisArg == nullptr); - newArgs = newCall->gtCallArgs; - oldArgs = oldCall->gtCallArgs; - } - else - { - assert(oldCall->gtCallThisArg != nullptr); - newArgObjp.SetNode(newCall->gtCallThisArg->GetNode()); - newArgs = &newArgObjp; - oldArgObjp.SetNode(oldCall->gtCallThisArg->GetNode()); - oldArgs = &oldArgObjp; + // If we have a this arg make it appear as if it's + // a part of the args list to keep things simpler. + newCall->gtCallThisArg->SetNext(newArgs); + oldCall->gtCallThisArg->SetNext(oldArgs); + newArgs = newCall->gtCallThisArg; + oldArgs = oldCall->gtCallThisArg; } GenTree* newCurr; @@ -972,19 +961,12 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) { /* Get hold of the next argument values for the oldCall and newCall */ - newCurr = newArgs->GetNode(); - oldCurr = oldArgs->GetNode(); - if (newArgs != &newArgObjp) - { - newParent = newArgs; - oldParent = oldArgs; - } - else - { - assert(newParent == nullptr && oldParent == nullptr); - } - newArgs = newArgs->GetNext(); - oldArgs = oldArgs->GetNext(); + newCurr = newArgs->GetNode(); + oldCurr = oldArgs->GetNode(); + newParent = newArgs; + oldParent = oldArgs; + newArgs = newArgs->GetNext(); + oldArgs = oldArgs->GetNext(); fgArgTabEntry* oldArgTabEntry = nullptr; fgArgTabEntry* newArgTabEntry = nullptr; @@ -1079,6 +1061,14 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) } } + // Reset gtCallThisArg's next pointer to avoid confusion. Ideally the this + // arg would be part of the normal call arg list to avoid this acrobatics. + if (newCall->gtCallThisArg != nullptr) + { + newCall->gtCallThisArg->SetNext(nullptr); + oldCall->gtCallThisArg->SetNext(nullptr); + } + argCount = oldArgInfo->argCount; nextSlotNum = oldArgInfo->nextSlotNum; hasRegArgs = oldArgInfo->hasRegArgs; @@ -1247,11 +1237,7 @@ void fgArgInfo::UpdateRegArg(fgArgTabEntry* curArgTabEntry, GenTree* node, bool (!isLateArg && ((node->gtFlags & GTF_LATE_ARG) == 0))); assert(curArgTabEntry->numRegs != 0); - - if (curArgTabEntry->use != nullptr) - { - assert(curArgTabEntry->use->GetNode() == node); - } + assert(curArgTabEntry->use->GetNode() == node); if (curArgTabEntry->node != node) { @@ -1290,7 +1276,7 @@ void fgArgInfo::UpdateStkArg(fgArgTabEntry* curArgTabEntry, GenTree* node, bool assert((isLateArg && ((node->gtFlags & GTF_LATE_ARG) != 0)) || (!isLateArg && ((node->gtFlags & GTF_LATE_ARG) == 0))); - noway_assert(curArgTabEntry->use != nullptr); + noway_assert(curArgTabEntry->use != callTree->gtCallThisArg); assert((curArgTabEntry->regNum == REG_STK) || curArgTabEntry->isSplit); assert(curArgTabEntry->use->GetNode() == node); nextSlotNum = (unsigned)roundUp(nextSlotNum, curArgTabEntry->alignment); @@ -1380,6 +1366,7 @@ void fgArgInfo::SplitArg(unsigned argNum, unsigned numRegs, unsigned numSlots) // void fgArgInfo::EvalToTmp(fgArgTabEntry* curArgTabEntry, unsigned tmpNum, GenTree* newNode) { + assert(curArgTabEntry->use != callTree->gtCallThisArg); assert(curArgTabEntry->use->GetNode() == newNode); curArgTabEntry->node = newNode; @@ -2410,21 +2397,8 @@ void fgArgInfo::EvalArgsToTemps() if (setupArg != nullptr) { - if (curArgTabEntry->use != nullptr) - { - GenTreeCall::Use* use = curArgTabEntry->use; - /* a normal argument from the list */ - noway_assert(use->GetNode() == argx); - - use->SetNode(setupArg); - } - else - { - // must be the "this" arg - noway_assert(callTree->AsCall()->gtCallThisArg->GetNode() == argx); - - callTree->AsCall()->gtCallThisArg->SetNode(setupArg); - } + noway_assert(curArgTabEntry->use->GetNode() == argx); + curArgTabEntry->use->SetNode(setupArg); } /* deferred arg goes into the late argument list */ @@ -2926,9 +2900,9 @@ void Compiler::fgInitArgInfo(GenTreeCall* call) assert(varTypeIsGC(argx) || (argx->gtType == TYP_I_IMPL)); // This is a register argument - put it in the table. - call->fgArgInfo->AddRegArg(argIndex, argx, nullptr, genMapIntRegArgNumToRegNum(intArgRegNum), 1, 1, false, - callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) - UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); + call->fgArgInfo->AddRegArg(argIndex, argx, call->gtCallThisArg, genMapIntRegArgNumToRegNum(intArgRegNum), 1, 1, + false, callIsVararg UNIX_AMD64_ABI_ONLY_ARG(REG_STK) UNIX_AMD64_ABI_ONLY_ARG(0) + UNIX_AMD64_ABI_ONLY_ARG(0) UNIX_AMD64_ABI_ONLY_ARG(nullptr)); intArgRegNum++; #ifdef WINDOWS_AMD64_ABI From 765ea862620e4b5d291b6cae705bb3d54ebedd3f Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 21 Sep 2019 17:29:37 +0300 Subject: [PATCH 3/7] Do not store the node in fgArgTabEntry --- src/jit/assertionprop.cpp | 4 +- src/jit/compiler.h | 15 +++- src/jit/earlyprop.cpp | 4 +- src/jit/gentree.cpp | 9 +-- src/jit/lower.cpp | 50 +++++------- src/jit/morph.cpp | 148 +++++++++-------------------------- src/jit/stacklevelsetter.cpp | 2 +- 7 files changed, 78 insertions(+), 154 deletions(-) diff --git a/src/jit/assertionprop.cpp b/src/jit/assertionprop.cpp index 5c9e71abab80..424fe55bd7f4 100644 --- a/src/jit/assertionprop.cpp +++ b/src/jit/assertionprop.cpp @@ -3726,13 +3726,13 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_CHKCASTANY) || call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_CHKCASTCLASS_SPECIAL)) { - GenTree* arg1 = gtArgEntryByArgNum(call, 1)->node; + GenTree* arg1 = gtArgEntryByArgNum(call, 1)->GetNode(); if (arg1->gtOper != GT_LCL_VAR) { return nullptr; } - GenTree* arg2 = gtArgEntryByArgNum(call, 0)->node; + GenTree* arg2 = gtArgEntryByArgNum(call, 0)->GetNode(); unsigned index = optAssertionIsSubtype(arg1, arg2, assertions); if (index != NO_ASSERTION_INDEX) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index ac2774d6fea8..646718856625 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -1331,9 +1331,15 @@ struct FuncInfoDsc struct fgArgTabEntry { - GenTree* node; // Initially points to `use`'s node, but if the argument is replaced with an GT_ASG or - // placeholder it will point at the actual argument node in the gtCallLateArgs list. - GenTreeCall::Use* use; // Points to the argument's GenTreeCall::Use in gtCallArgs or gtCallThisArg. + GenTreeCall::Use* use; // Points to the argument's GenTreeCall::Use in gtCallArgs or gtCallThisArg. + GenTreeCall::Use* lateUse; // Points to the argument's GenTreeCall::Use in gtCallLateArgs, if any. + + // Get the node that coresponds to this argument entry. + // This is the "real" node and not a placeholder or setup node. + GenTree* GetNode() const + { + return lateUse == nullptr ? use->GetNode() : lateUse->GetNode(); + } unsigned argNum; // The original argument number, also specifies the required argument evaluation order from the IL @@ -1699,6 +1705,7 @@ struct fgArgTabEntry // void checkIsStruct() { + GenTree* node = GetNode(); if (isStruct) { if (!varTypeIsStruct(node) && !node->OperIs(GT_FIELD_LIST)) @@ -1918,7 +1925,7 @@ class fgArgInfo // Caller must ensure that this index is a valid arg index. GenTree* GetArgNode(unsigned argIndex) { - return GetArgEntry(argIndex)->node; + return GetArgEntry(argIndex)->GetNode(); } void Dump(Compiler* compiler); diff --git a/src/jit/earlyprop.cpp b/src/jit/earlyprop.cpp index b3752419ac6a..12d98c151b61 100644 --- a/src/jit/earlyprop.cpp +++ b/src/jit/earlyprop.cpp @@ -85,7 +85,7 @@ GenTree* Compiler::getArrayLengthFromAllocation(GenTree* tree) call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_NEWARR_1_ALIGN8)) { // This is an array allocation site. Grab the array length node. - return gtArgEntryByArgNum(call, 1)->node; + return gtArgEntryByArgNum(call, 1)->GetNode(); } } } @@ -127,7 +127,7 @@ GenTree* Compiler::getObjectHandleNodeFromAllocation(GenTree* tree) { // This is an object allocation site. Return the runtime type handle node. fgArgTabEntry* argTabEntry = gtArgEntryByArgNum(call, 0); - return argTabEntry->node; + return argTabEntry->GetNode(); } } } diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 9a79928536ee..1b047ae9bd74 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -1053,9 +1053,8 @@ void GenTreeCall::ReplaceCallOperand(GenTree** useEdge, GenTree* replacement) { assert((replacement->gtFlags & GTF_LATE_ARG) == 0); - fgArgTabEntry* fp = Compiler::gtArgEntryByNode(this, originalOperand); - assert(fp->node == originalOperand); - fp->node = replacement; + fgArgTabEntry* fp = Compiler::gtArgEntryByNode(this, replacement); + assert(fp->GetNode() == replacement); } } } @@ -6215,7 +6214,7 @@ fgArgTabEntry* Compiler::gtArgEntryByNode(GenTreeCall* call, GenTree* node) { curArgTabEntry = argTable[i]; - if (curArgTabEntry->node == node) + if (curArgTabEntry->GetNode() == node) { return curArgTabEntry; } @@ -8056,7 +8055,7 @@ GenTree* Compiler::gtGetThisArg(GenTreeCall* call) { unsigned argNum = 0; fgArgTabEntry* thisArgTabEntry = gtArgEntryByArgNum(call, argNum); - GenTree* result = thisArgTabEntry->node; + GenTree* result = thisArgTabEntry->GetNode(); // Assert if we used DEBUG_DESTROY_NODE. assert(result->gtOper != GT_COUNT); diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 1e0fb358bf02..0f154b4ece4c 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -1007,8 +1007,7 @@ GenTree* Lowering::NewPutArg(GenTreeCall* call, GenTree* arg, fgArgTabEntry* inf assert(arg != nullptr); assert(info != nullptr); - GenTree* putArg = nullptr; - bool updateArgTable = true; + GenTree* putArg = nullptr; bool isOnStack = true; isOnStack = info->regNum == REG_STK; @@ -1225,10 +1224,6 @@ GenTree* Lowering::NewPutArg(GenTreeCall* call, GenTree* arg, fgArgTabEntry* inf { putArg->gtFlags |= GTF_LATE_ARG; } - else if (updateArgTable) - { - info->node = putArg; - } return putArg; } @@ -1269,7 +1264,7 @@ void Lowering::LowerArg(GenTreeCall* call, GenTree** ppArg) } fgArgTabEntry* info = comp->gtArgEntryByNode(call, arg); - assert(info->node == arg); + assert(info->GetNode() == arg); var_types type = arg->TypeGet(); if (varTypeIsSmall(type)) @@ -1309,8 +1304,9 @@ void Lowering::LowerArg(GenTreeCall* call, GenTree** ppArg) GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, TYP_LONG, arg, nullptr); BlockRange().InsertAfter(arg, bitcast); - info->node = *ppArg = arg = bitcast; - type = TYP_LONG; + *ppArg = arg = bitcast; + assert(info->GetNode() == arg); + type = TYP_LONG; } #endif // defined(_TARGET_X86_) #endif // defined(FEATURE_SIMD) @@ -1337,8 +1333,8 @@ void Lowering::LowerArg(GenTreeCall* call, GenTree** ppArg) BlockRange().InsertBefore(arg, putArg); BlockRange().Remove(arg); - *ppArg = fieldList; - info->node = fieldList; + *ppArg = fieldList; + assert(info->GetNode() == fieldList); } else { @@ -2243,16 +2239,16 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget #ifdef DEBUG argEntry = comp->gtArgEntryByArgNum(call, 0); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_REG); - GenTree* firstArg = argEntry->node->gtOp.gtOp1; + assert(argEntry->GetNode()->OperIs(GT_PUTARG_REG)); + GenTree* firstArg = argEntry->GetNode()->AsUnOp()->gtGetOp1(); assert(firstArg->gtOper == GT_CNS_INT); #endif // Replace second arg by callTarget. argEntry = comp->gtArgEntryByArgNum(call, 1); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_REG); - GenTree* secondArg = argEntry->node->gtOp.gtOp1; + assert(argEntry->GetNode()->OperIs(GT_PUTARG_REG)); + GenTree* secondArg = argEntry->GetNode()->AsUnOp()->gtGetOp1(); ContainCheckRange(callTargetRange); BlockRange().InsertAfter(secondArg, std::move(callTargetRange)); @@ -2263,7 +2259,7 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget BlockRange().Remove(std::move(secondArgRange)); - argEntry->node->gtOp.gtOp1 = callTarget; + argEntry->GetNode()->AsUnOp()->gtOp1 = callTarget; #elif defined(_TARGET_X86_) @@ -2282,8 +2278,7 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget // arg 0 == callTarget. argEntry = comp->gtArgEntryByArgNum(call, numArgs - 1); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_STK); - GenTree* arg0 = argEntry->node->gtOp.gtOp1; + GenTree* arg0 = argEntry->GetNode()->AsPutArgStk()->gtGetOp1(); ContainCheckRange(callTargetRange); BlockRange().InsertAfter(arg0, std::move(callTargetRange)); @@ -2293,13 +2288,12 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget assert(isClosed); BlockRange().Remove(std::move(secondArgRange)); - argEntry->node->gtOp.gtOp1 = callTarget; + argEntry->GetNode()->AsPutArgStk()->gtOp1 = callTarget; // arg 1 == flags argEntry = comp->gtArgEntryByArgNum(call, numArgs - 2); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_STK); - GenTree* arg1 = argEntry->node->gtOp.gtOp1; + GenTree* arg1 = argEntry->GetNode()->AsPutArgStk()->gtGetOp1(); assert(arg1->gtOper == GT_CNS_INT); ssize_t tailCallHelperFlags = 1 | // always restore EDI,ESI,EBX @@ -2309,8 +2303,7 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget // arg 2 == numberOfNewStackArgsWords argEntry = comp->gtArgEntryByArgNum(call, numArgs - 3); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_STK); - GenTree* arg2 = argEntry->node->gtOp.gtOp1; + GenTree* arg2 = argEntry->GetNode()->AsPutArgStk()->gtGetOp1(); assert(arg2->gtOper == GT_CNS_INT); arg2->gtIntCon.gtIconVal = nNewStkArgsWords; @@ -2319,8 +2312,7 @@ GenTree* Lowering::LowerTailCallViaHelper(GenTreeCall* call, GenTree* callTarget // arg 3 == numberOfOldStackArgsWords argEntry = comp->gtArgEntryByArgNum(call, numArgs - 4); assert(argEntry != nullptr); - assert(argEntry->node->gtOper == GT_PUTARG_STK); - GenTree* arg3 = argEntry->node->gtOp.gtOp1; + GenTree* arg3 = argEntry->GetNode()->AsPutArgStk()->gtGetOp1(); assert(arg3->gtOper == GT_CNS_INT); #endif // DEBUG @@ -3264,7 +3256,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) #endif // !_TARGET_X86_ fgArgTabEntry* thisArgTabEntry = comp->gtArgEntryByArgNum(call, argNum); - thisArgNode = thisArgTabEntry->node; + thisArgNode = thisArgTabEntry->GetNode(); } else { @@ -4065,8 +4057,8 @@ GenTree* Lowering::LowerVirtualVtableCall(GenTreeCall* call) // get a reference to the thisPtr being passed fgArgTabEntry* argEntry = comp->gtArgEntryByArgNum(call, thisPtrArgNum); assert(argEntry->regNum == thisPtrArgReg); - assert(argEntry->node->gtOper == GT_PUTARG_REG); - GenTree* thisPtr = argEntry->node->gtOp.gtOp1; + assert(argEntry->GetNode()->OperIs(GT_PUTARG_REG)); + GenTree* thisPtr = argEntry->GetNode()->AsUnOp()->gtGetOp1(); // If what we are passing as the thisptr is not already a local, make a new local to place it in // because we will be creating expressions based on it. @@ -4083,7 +4075,7 @@ GenTree* Lowering::LowerVirtualVtableCall(GenTreeCall* call) vtableCallTemp = comp->lvaGrabTemp(true DEBUGARG("virtual vtable call")); } - LIR::Use thisPtrUse(BlockRange(), &(argEntry->node->gtOp.gtOp1), argEntry->node); + LIR::Use thisPtrUse(BlockRange(), &(argEntry->GetNode()->AsUnOp()->gtOp1), argEntry->GetNode()); ReplaceWithLclVar(thisPtrUse, vtableCallTemp); lclNum = vtableCallTemp; diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 3255630a7937..898923e7c78a 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -802,7 +802,7 @@ GenTree* Compiler::fgUnwrapProxy(GenTree* objRef) void fgArgTabEntry::Dump() { printf("fgArgTabEntry[arg %u", argNum); - printf(" %d.%s", node->gtTreeID, GenTree::OpName(node->gtOper)); + printf(" %d.%s", GetNode()->gtTreeID, GenTree::OpName(GetNode()->OperGet())); printf(" %s", varTypeName(argType)); if (regNum != REG_STK) { @@ -991,23 +991,10 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) // newArgTabEntry->use = newParent; - // The node field is likely to have been updated - // to point at a node in the gtCallLateArgs list - // - if (oldArgTabEntry->node == oldCurr) - { - // node is not pointing into the gtCallLateArgs list - newArgTabEntry->node = newCurr; - } - else + if (oldArgTabEntry->lateUse != nullptr) { - // node must be pointing into the gtCallLateArgs list - // - // We will fix this pointer up in the next loop - // - newArgTabEntry->node = nullptr; // For now we assign a NULL to this field - - scanRegArgs = true; + newArgTabEntry->lateUse = nullptr; + scanRegArgs = true; } // Now initialize the proper element in the argTable array @@ -1030,11 +1017,11 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) { /* Get hold of the next argument values for the oldCall and newCall */ - newCurr = newArgs->GetNode(); - newArgs = newArgs->GetNext(); + newParent = newArgs; + newArgs = newArgs->GetNext(); - oldCurr = oldArgs->GetNode(); - oldArgs = oldArgs->GetNext(); + oldParent = oldArgs; + oldArgs = oldArgs->GetNext(); fgArgTabEntry* oldArgTabEntry = nullptr; fgArgTabEntry* newArgTabEntry = nullptr; @@ -1043,18 +1030,12 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) { oldArgTabEntry = oldArgTable[inx]; - if (oldArgTabEntry->node == oldCurr) + if (oldArgTabEntry->lateUse == oldParent) { - // We have found the matching "node" field in oldArgTabEntry - newArgTabEntry = argTable[inx]; assert(newArgTabEntry != nullptr); - - // update the "node" GenTree* fields in the newArgTabEntry - // - assert(newArgTabEntry->node == nullptr); // We previously assigned NULL to this field - - newArgTabEntry->node = newCurr; + assert(newArgTabEntry->lateUse == nullptr); + newArgTabEntry->lateUse = newParent; break; } } @@ -1102,9 +1083,9 @@ fgArgTabEntry* fgArgInfo::AddRegArg(unsigned argNum, curArgTabEntry->setRegNum(0, regNum); curArgTabEntry->argNum = argNum; - curArgTabEntry->node = node; curArgTabEntry->argType = node->TypeGet(); curArgTabEntry->use = use; + curArgTabEntry->lateUse = nullptr; curArgTabEntry->slotNum = 0; curArgTabEntry->numRegs = numRegs; curArgTabEntry->numSlots = 0; @@ -1180,9 +1161,9 @@ fgArgTabEntry* fgArgInfo::AddStkArg(unsigned argNum, curArgTabEntry->setRegNum(0, REG_STK); curArgTabEntry->argNum = argNum; - curArgTabEntry->node = node; curArgTabEntry->argType = node->TypeGet(); curArgTabEntry->use = use; + curArgTabEntry->lateUse = nullptr; curArgTabEntry->slotNum = nextSlotNum; curArgTabEntry->numRegs = 0; #if defined(UNIX_AMD64_ABI) @@ -1238,24 +1219,6 @@ void fgArgInfo::UpdateRegArg(fgArgTabEntry* curArgTabEntry, GenTree* node, bool assert(curArgTabEntry->numRegs != 0); assert(curArgTabEntry->use->GetNode() == node); - - if (curArgTabEntry->node != node) - { - if (reMorphing) - { - // Find the arg in the late args list. - GenTree* argx = Compiler::gtArgNodeByLateArgInx(callTree, curArgTabEntry->GetLateArgInx()); - if (curArgTabEntry->node != argx) - { - curArgTabEntry->node = argx; - } - } - else - { - assert(!isLateArg); - curArgTabEntry->node = node; - } - } } //------------------------------------------------------------------------ @@ -1282,40 +1245,6 @@ void fgArgInfo::UpdateStkArg(fgArgTabEntry* curArgTabEntry, GenTree* node, bool nextSlotNum = (unsigned)roundUp(nextSlotNum, curArgTabEntry->alignment); assert(curArgTabEntry->slotNum == nextSlotNum); - if (curArgTabEntry->node != node) - { -#if FEATURE_FIXED_OUT_ARGS - if (isLateArg) - { - GenTree* argx = nullptr; - unsigned lateArgInx = curArgTabEntry->GetLateArgInx(); - - // Traverse the late argument list to find this argument so that we can update it. - unsigned listInx = 0; - for (GenTreeCall::Use& use : callTree->AsCall()->LateArgs()) - { - argx = use.GetNode(); - assert(!argx->IsArgPlaceHolderNode()); // No place holders nodes are in gtCallLateArgs; - if (listInx == lateArgInx) - { - break; - } - listInx++; - } - assert(listInx == lateArgInx); - assert(lateArgInx == curArgTabEntry->GetLateArgInx()); - - if (curArgTabEntry->node != argx) - { - curArgTabEntry->node = argx; - } - } - else -#endif // FEATURE_FIXED_OUT_ARGS - { - curArgTabEntry->node = node; - } - } nextSlotNum += curArgTabEntry->numSlots; } @@ -1369,7 +1298,7 @@ void fgArgInfo::EvalToTmp(fgArgTabEntry* curArgTabEntry, unsigned tmpNum, GenTre assert(curArgTabEntry->use != callTree->gtCallThisArg); assert(curArgTabEntry->use->GetNode() == newNode); - curArgTabEntry->node = newNode; + assert(curArgTabEntry->GetNode() == newNode); curArgTabEntry->tmpNum = tmpNum; curArgTabEntry->isTmp = true; } @@ -1383,7 +1312,7 @@ void fgArgInfo::ArgsComplete() { fgArgTabEntry* curArgTabEntry = argTable[curInx]; assert(curArgTabEntry != nullptr); - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); if (curArgTabEntry->regNum == REG_STK) { @@ -1446,8 +1375,7 @@ void fgArgInfo::ArgsComplete() fgArgTabEntry* prevArgTabEntry = argTable[prevInx]; assert(prevArgTabEntry->argNum < curArgTabEntry->argNum); - assert(prevArgTabEntry->node); - if (prevArgTabEntry->node->gtOper != GT_CNS_INT) + if (prevArgTabEntry->GetNode()->gtOper != GT_CNS_INT) { prevArgTabEntry->needTmp = true; needsTemps = true; @@ -1510,11 +1438,10 @@ void fgArgInfo::ArgsComplete() { fgArgTabEntry* prevArgTabEntry = argTable[prevInx]; assert(prevArgTabEntry->argNum < curArgTabEntry->argNum); - assert(prevArgTabEntry->node); // For all previous arguments, if they have any GTF_ALL_EFFECT // we require that they be evaluated into a temp - if ((prevArgTabEntry->node->gtFlags & GTF_ALL_EFFECT) != 0) + if ((prevArgTabEntry->GetNode()->gtFlags & GTF_ALL_EFFECT) != 0) { prevArgTabEntry->needTmp = true; needsTemps = true; @@ -1665,7 +1592,7 @@ void fgArgInfo::ArgsComplete() { fgArgTabEntry* curArgTabEntry = argTable[curInx]; assert(curArgTabEntry != nullptr); - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); // Examine the register args that are currently not marked needTmp // @@ -1774,11 +1701,13 @@ void fgArgInfo::SortArgs() regCount++; } + assert(curArgTabEntry->lateUse == nullptr); + // Skip any already processed args // if (!curArgTabEntry->processed) { - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); // put constants at the end of the table // @@ -1815,7 +1744,7 @@ void fgArgInfo::SortArgs() // if (!curArgTabEntry->processed) { - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); // put calls at the beginning of the table // @@ -1891,7 +1820,7 @@ void fgArgInfo::SortArgs() // if (!curArgTabEntry->processed) { - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); if ((argx->gtOper == GT_LCL_VAR) || (argx->gtOper == GT_LCL_FLD)) { @@ -1935,7 +1864,7 @@ void fgArgInfo::SortArgs() // if (!curArgTabEntry->processed) { - GenTree* argx = curArgTabEntry->node; + GenTree* argx = curArgTabEntry->GetNode(); // We should have already handled these kinds of args assert(argx->gtOper != GT_LCL_VAR); @@ -2172,7 +2101,9 @@ void fgArgInfo::EvalArgsToTemps() { fgArgTabEntry* curArgTabEntry = argTable[curInx]; - GenTree* argx = curArgTabEntry->node; + assert(curArgTabEntry->lateUse == nullptr); + + GenTree* argx = curArgTabEntry->GetNode(); GenTree* setupArg = nullptr; GenTree* defArg; @@ -2416,7 +2347,7 @@ void fgArgInfo::EvalArgsToTemps() tmpRegArgNext = tmpRegArgNext->GetNext(); } - curArgTabEntry->node = defArg; + curArgTabEntry->lateUse = tmpRegArgNext; curArgTabEntry->SetLateArgInx(regArgInx++); } @@ -3668,11 +3599,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // If we are remorphing, process the late arguments (which were determined by a previous caller). if (reMorphing) { - // We need to reMorph the gtCallLateArgs early since that is what triggers - // the expression folding and we need to have the final folded gtCallLateArgs - // available when we call UpdateRegArg so that we correctly update the fgArgInfo - // with the folded tree that represents the final optimized argument nodes. - // for (GenTreeCall::Use& use : call->LateArgs()) { use.SetNode(fgMorphTree(use.GetNode())); @@ -4071,8 +3997,8 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) (void)new (this, GT_FIELD_LIST) GenTreeFieldList(argx->gtOp.gtOp2, OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL, fieldList); fgArgTabEntry* fp = Compiler::gtArgEntryByNode(call, argx); - fp->node = fieldList; args->SetNode(fieldList); + assert(fp->GetNode() == fieldList); #else // !_TARGET_X86_ @@ -4148,8 +4074,8 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) fieldList = new (this, GT_FIELD_LIST) GenTreeFieldList(lcl, fieldVarDsc->lvFldOffset, fieldVarDsc->lvType, nullptr); fgArgTabEntry* fp = Compiler::gtArgEntryByNode(call, argx); - fp->node = fieldList; args->SetNode(fieldList); + assert(fp->GetNode() == fieldList); } else { @@ -4283,7 +4209,7 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) bool isLateArg = (use.GetNode()->gtFlags & GTF_LATE_ARG) != 0; fgArgTabEntry* fgEntryPtr = gtArgEntryByNode(call, use.GetNode()); assert(fgEntryPtr != nullptr); - GenTree* argx = fgEntryPtr->node; + GenTree* argx = fgEntryPtr->GetNode(); GenTreeCall::Use* lateUse = nullptr; GenTree* lateNode = nullptr; @@ -4345,8 +4271,6 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) // Did we replace 'argx' with a new tree? if (newArgx != argx) { - fgEntryPtr->node = newArgx; // Record the new value for the arg in the fgEntryPtr->node - // link the new arg node into either the late arg list or the gtCallArgs list if (isLateArg) { @@ -4356,6 +4280,8 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) { use.SetNode(newArgx); } + + assert(fgEntryPtr->GetNode() == newArgx); } } } @@ -5055,7 +4981,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, varDsc->setLvRefCnt(0, RCS_EARLY); args->SetNode(lcl); - argEntry->node = lcl; + assert(argEntry->GetNode() == lcl); JITDUMP("did not have to make outgoing copy for V%2d", varNum); return; @@ -8457,13 +8383,13 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) // This needs to be done after the arguments are morphed to ensure constant propagation has already taken place. if ((call->gtCallType == CT_HELPER) && (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST))) { - GenTree* value = gtArgEntryByArgNum(call, 2)->node; + GenTree* value = gtArgEntryByArgNum(call, 2)->GetNode(); if (value->IsIntegralConst(0)) { assert(value->OperGet() == GT_CNS_INT); - GenTree* arr = gtArgEntryByArgNum(call, 0)->node; - GenTree* index = gtArgEntryByArgNum(call, 1)->node; + GenTree* arr = gtArgEntryByArgNum(call, 0)->GetNode(); + GenTree* index = gtArgEntryByArgNum(call, 1)->GetNode(); // Either or both of the array and index arguments may have been spilled to temps by `fgMorphArgs`. Copy // the spill trees as well if necessary. diff --git a/src/jit/stacklevelsetter.cpp b/src/jit/stacklevelsetter.cpp index 89f6f55699c9..147cf04eb95d 100644 --- a/src/jit/stacklevelsetter.cpp +++ b/src/jit/stacklevelsetter.cpp @@ -246,7 +246,7 @@ unsigned StackLevelSetter::PopArgumentsFromCall(GenTreeCall* call) fgArgTabEntry* argTab = argInfo->ArgTable()[i]; if (argTab->numSlots != 0) { - GenTree* node = argTab->node; + GenTree* node = argTab->GetNode(); assert(node->OperIsPutArgStkOrSplit()); GenTreePutArgStk* putArg = node->AsPutArgStk(); From dd0adb37356528252050704f131a821ae6e6c837 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 21 Sep 2019 17:50:53 +0300 Subject: [PATCH 4/7] Cleanup fgArgInfo "copy constructor" --- src/jit/morph.cpp | 136 +++++++++++++--------------------------------- 1 file changed, 37 insertions(+), 99 deletions(-) diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 898923e7c78a..550d5f36d901 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -926,130 +926,68 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) argTableSize = oldArgInfo->argTableSize; argsComplete = false; argTable = nullptr; - if (argTableSize > 0) - { - argTable = new (compiler, CMK_fgArgInfoPtrArr) fgArgTabEntry*[argTableSize]; - for (unsigned inx = 0; inx < argTableSize; inx++) - { - argTable[inx] = nullptr; - } - } assert(oldArgInfo->argsComplete); - GenTreeCall::Use* newArgs = newCall->gtCallArgs; - GenTreeCall::Use* oldArgs = oldCall->gtCallArgs; - - if (newCall->gtCallThisArg != nullptr) - { - // If we have a this arg make it appear as if it's - // a part of the args list to keep things simpler. - newCall->gtCallThisArg->SetNext(newArgs); - oldCall->gtCallThisArg->SetNext(oldArgs); - newArgs = newCall->gtCallThisArg; - oldArgs = oldCall->gtCallThisArg; - } - - GenTree* newCurr; - GenTree* oldCurr; - GenTreeCall::Use* newParent = nullptr; - GenTreeCall::Use* oldParent = nullptr; - fgArgTabEntry** oldArgTable = oldArgInfo->argTable; - bool scanRegArgs = false; - - while (newArgs != nullptr) + if (argTableSize > 0) { - /* Get hold of the next argument values for the oldCall and newCall */ - - newCurr = newArgs->GetNode(); - oldCurr = oldArgs->GetNode(); - newParent = newArgs; - oldParent = oldArgs; - newArgs = newArgs->GetNext(); - oldArgs = oldArgs->GetNext(); - - fgArgTabEntry* oldArgTabEntry = nullptr; - fgArgTabEntry* newArgTabEntry = nullptr; + argTable = new (compiler, CMK_fgArgInfoPtrArr) fgArgTabEntry*[argTableSize]; - for (unsigned inx = 0; inx < argTableSize; inx++) + // Copy the old arg entries + for (unsigned i = 0; i < argTableSize; i++) { - oldArgTabEntry = oldArgTable[inx]; + argTable[i] = new (compiler, CMK_fgArgInfo) fgArgTabEntry(*oldArgInfo->argTable[i]); + } - if (oldArgTabEntry->use == oldParent) + // The copied arg entries contain pointers to old uses, they need + // to be updated to point to new uses. + if (newCall->gtCallThisArg != nullptr) + { + for (unsigned i = 0; i < argTableSize; i++) { - assert((oldParent == nullptr) == (newParent == nullptr)); - - // We have found the matching "parent" field in oldArgTabEntry - - newArgTabEntry = new (compiler, CMK_fgArgInfo) fgArgTabEntry; - - // First block copy all fields - // - *newArgTabEntry = *oldArgTabEntry; - - // Then update all GenTree* fields in the newArgTabEntry - // - newArgTabEntry->use = newParent; - - if (oldArgTabEntry->lateUse != nullptr) + if (argTable[i]->use == oldCall->gtCallThisArg) { - newArgTabEntry->lateUse = nullptr; - scanRegArgs = true; + argTable[i]->use = newCall->gtCallThisArg; + break; } - - // Now initialize the proper element in the argTable array - // - argTable[inx] = newArgTabEntry; - break; } } - // We should have found the matching oldArgTabEntry and created the newArgTabEntry - // - assert(newArgTabEntry != nullptr); - } - if (scanRegArgs) - { - newArgs = newCall->gtCallLateArgs; - oldArgs = oldCall->gtCallLateArgs; + GenTreeCall::UseIterator newUse = newCall->Args().begin(); + GenTreeCall::UseIterator newUseEnd = newCall->Args().end(); + GenTreeCall::UseIterator oldUse = oldCall->Args().begin(); + GenTreeCall::UseIterator oldUseEnd = newCall->Args().end(); - while (newArgs != nullptr) + for (; newUse != newUseEnd; ++newUse, ++oldUse) { - /* Get hold of the next argument values for the oldCall and newCall */ - - newParent = newArgs; - newArgs = newArgs->GetNext(); - - oldParent = oldArgs; - oldArgs = oldArgs->GetNext(); + for (unsigned i = 0; i < argTableSize; i++) + { + if (argTable[i]->use == &(*oldUse)) + { + argTable[i]->use = &(*newUse); + break; + } + } + } - fgArgTabEntry* oldArgTabEntry = nullptr; - fgArgTabEntry* newArgTabEntry = nullptr; + newUse = newCall->LateArgs().begin(); + newUseEnd = newCall->LateArgs().end(); + oldUse = oldCall->LateArgs().begin(); + oldUseEnd = newCall->LateArgs().end(); - for (unsigned inx = 0; inx < argTableSize; inx++) + for (; newUse != newUseEnd; ++newUse, ++oldUse) + { + for (unsigned i = 0; i < argTableSize; i++) { - oldArgTabEntry = oldArgTable[inx]; - - if (oldArgTabEntry->lateUse == oldParent) + if (argTable[i]->lateUse == &(*oldUse)) { - newArgTabEntry = argTable[inx]; - assert(newArgTabEntry != nullptr); - assert(newArgTabEntry->lateUse == nullptr); - newArgTabEntry->lateUse = newParent; + argTable[i]->lateUse = &(*newUse); break; } } } } - // Reset gtCallThisArg's next pointer to avoid confusion. Ideally the this - // arg would be part of the normal call arg list to avoid this acrobatics. - if (newCall->gtCallThisArg != nullptr) - { - newCall->gtCallThisArg->SetNext(nullptr); - oldCall->gtCallThisArg->SetNext(nullptr); - } - argCount = oldArgInfo->argCount; nextSlotNum = oldArgInfo->nextSlotNum; hasRegArgs = oldArgInfo->hasRegArgs; From 106946d2df549140e4b75d5cd30811e1b6ba6397 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 28 Sep 2019 01:40:29 +0300 Subject: [PATCH 5/7] Extract call compare code to a separate function --- src/jit/gentree.cpp | 184 ++++++++++++++++++++++++-------------------- src/jit/gentree.h | 2 + 2 files changed, 103 insertions(+), 83 deletions(-) diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 1b047ae9bd74..6a313f046344 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -1088,6 +1088,106 @@ bool GenTreeCall::AreArgsComplete() const return false; } +//-------------------------------------------------------------------------- +// Equals: Check if 2 CALL nodes are equal. +// +// Arguments: +// c1 - The first call node +// c2 - The second call node +// +// Return Value: +// true if the 2 CALL nodes have the same type and operands +// +bool GenTreeCall::Equals(GenTreeCall* c1, GenTreeCall* c2) +{ + assert(c1->OperGet() == c2->OperGet()); + + if (c1->TypeGet() != c2->TypeGet()) + { + return false; + } + + if (c1->gtCallType != c2->gtCallType) + { + return false; + } + + if (c1->gtCallType != CT_INDIRECT) + { + if (c1->gtCallMethHnd != c2->gtCallMethHnd) + { + return false; + } + +#ifdef FEATURE_READYTORUN_COMPILER + if (c1->gtEntryPoint.addr != c2->gtEntryPoint.addr) + { + return false; + } +#endif + } + else + { + if (!Compare(c1->gtCallAddr, c2->gtCallAddr)) + { + return false; + } + } + + if ((c1->gtCallThisArg != nullptr) != (c2->gtCallThisArg != nullptr)) + { + return false; + } + + if ((c1->gtCallThisArg != nullptr) && !Compare(c1->gtCallThisArg->GetNode(), c2->gtCallThisArg->GetNode())) + { + return false; + } + + GenTreeCall::UseIterator i1 = c1->Args().begin(); + GenTreeCall::UseIterator end1 = c1->Args().end(); + GenTreeCall::UseIterator i2 = c2->Args().begin(); + GenTreeCall::UseIterator end2 = c2->Args().end(); + + for (; (i1 != end1) && (i2 != end2); ++i1, ++i2) + { + if (!Compare(i1->GetNode(), i2->GetNode())) + { + return false; + } + } + + if ((i1 != end1) || (i2 != end2)) + { + return false; + } + + i1 = c1->LateArgs().begin(); + end1 = c1->LateArgs().end(); + i2 = c2->LateArgs().begin(); + end2 = c2->LateArgs().end(); + + for (; (i1 != end1) && (i2 != end2); ++i1, ++i2) + { + if (!Compare(i1->GetNode(), i2->GetNode())) + { + return false; + } + } + + if ((i1 != end1) || (i2 != end2)) + { + return false; + } + + if (!Compare(c1->gtControlExpr, c2->gtControlExpr)) + { + return false; + } + + return true; +} + #if !defined(FEATURE_PUT_STRUCT_ARG_STK) unsigned GenTreePutArgStk::getArgSize() { @@ -1412,89 +1512,7 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) return true; case GT_CALL: - - if (op1->gtCall.gtCallType != op2->gtCall.gtCallType) - { - return false; - } - - if (op1->gtCall.gtCallType != CT_INDIRECT) - { - if (op1->gtCall.gtCallMethHnd != op2->gtCall.gtCallMethHnd) - { - return false; - } - -#ifdef FEATURE_READYTORUN_COMPILER - if (op1->gtCall.gtEntryPoint.addr != op2->gtCall.gtEntryPoint.addr) - { - return false; - } -#endif - } - else - { - if (!Compare(op1->gtCall.gtCallAddr, op2->gtCall.gtCallAddr)) - { - return false; - } - } - - if ((op1->AsCall()->gtCallThisArg != nullptr) != (op2->AsCall()->gtCallThisArg != nullptr)) - { - return false; - } - - if ((op1->AsCall()->gtCallThisArg != nullptr) && - !Compare(op1->AsCall()->gtCallThisArg->GetNode(), op2->AsCall()->gtCallThisArg->GetNode())) - { - return false; - } - - { - GenTreeCall::UseIterator i1 = op1->AsCall()->Args().begin(); - GenTreeCall::UseIterator end1 = op1->AsCall()->Args().end(); - GenTreeCall::UseIterator i2 = op2->AsCall()->Args().begin(); - GenTreeCall::UseIterator end2 = op2->AsCall()->Args().end(); - - for (; (i1 != end1) && (i2 != end2); ++i1, ++i2) - { - if (!Compare(i1->GetNode(), i2->GetNode())) - { - return false; - } - } - - if ((i1 != end1) || (i2 != end2)) - { - return false; - } - - i1 = op1->AsCall()->LateArgs().begin(); - end1 = op1->AsCall()->LateArgs().end(); - i2 = op2->AsCall()->LateArgs().begin(); - end2 = op2->AsCall()->LateArgs().end(); - - for (; (i1 != end1) && (i2 != end2); ++i1, ++i2) - { - if (!Compare(i1->GetNode(), i2->GetNode())) - { - return false; - } - } - - if ((i1 != end1) || (i2 != end2)) - { - return false; - } - } - - if (!Compare(op1->AsCall()->gtControlExpr, op2->AsCall()->gtControlExpr)) - { - return false; - } - - return true; + return GenTreeCall::Equals(op1->AsCall(), op2->AsCall()); case GT_ARR_ELEM: diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 8f916386b600..265b53364636 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -4024,6 +4024,8 @@ struct GenTreeCall final : public GenTree bool AreArgsComplete() const; + static bool Equals(GenTreeCall* c1, GenTreeCall* c2); + GenTreeCall(var_types type) : GenTree(GT_CALL, type) { fgArgInfo = nullptr; From 843b2d2a2a82ad17b4a5fc21ad73a8ff69c16ee4 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 28 Sep 2019 01:27:18 +0300 Subject: [PATCH 6/7] Extra asserts --- src/jit/gentree.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 265b53364636..9117f78ebdce 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3317,6 +3317,7 @@ struct GenTreeCall final : public GenTree public: Use(GenTree* node, Use* next = nullptr) : m_node(node), m_next(next) { + assert(node != nullptr); } GenTree*& NodeRef() @@ -3326,6 +3327,7 @@ struct GenTreeCall final : public GenTree GenTree* GetNode() const { + assert(m_node != nullptr); return m_node; } From 8acfa36bc6e19271b3e0f7ae1166ba31876c98f3 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 28 Sep 2019 01:50:41 +0300 Subject: [PATCH 7/7] Add UseIterator::GetUse() --- src/jit/gentree.h | 5 +++++ src/jit/morph.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 9117f78ebdce..1d3799c8acf7 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3372,6 +3372,11 @@ struct GenTreeCall final : public GenTree return m_use; } + Use* GetUse() const + { + return m_use; + } + UseIterator& operator++() { m_use = m_use->GetNext(); diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 550d5f36d901..096c06c0d6cb 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -962,9 +962,9 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) { for (unsigned i = 0; i < argTableSize; i++) { - if (argTable[i]->use == &(*oldUse)) + if (argTable[i]->use == oldUse.GetUse()) { - argTable[i]->use = &(*newUse); + argTable[i]->use = newUse.GetUse(); break; } } @@ -979,9 +979,9 @@ fgArgInfo::fgArgInfo(GenTreeCall* newCall, GenTreeCall* oldCall) { for (unsigned i = 0; i < argTableSize; i++) { - if (argTable[i]->lateUse == &(*oldUse)) + if (argTable[i]->lateUse == oldUse.GetUse()) { - argTable[i]->lateUse = &(*newUse); + argTable[i]->lateUse = newUse.GetUse(); break; } }