From 0484490d4a1b30095204f372ab7f9c09861ab6a2 Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Fri, 12 Jun 2020 18:21:47 -0700 Subject: [PATCH] Fix propagation of basic block flags for inlinee return expressions. block flags from inlinee return expressions. The assumption there was that `fgUpdateInlineReturnExpressionPlaceHolder` is the only place where `GT_RET_EXPR`'s are replaced with the actual nodes from inlinees. That assumption was incorrect and this change fixes the remaining places. Fixes #37574. --- src/coreclr/src/jit/compiler.h | 9 +++++---- src/coreclr/src/jit/flowgraph.cpp | 6 +++++- src/coreclr/src/jit/gentree.h | 25 +++++++++---------------- src/coreclr/src/jit/importer.cpp | 22 ++++++++++++---------- src/coreclr/src/jit/inline.h | 2 ++ 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index c47a0b6ad928d5..573f2678067da4 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -4198,10 +4198,11 @@ class Compiler InlineCandidateInfo** ppInlineCandidateInfo, InlineResult* inlineResult); - void impInlineRecordArgInfo(InlineInfo* pInlineInfo, - GenTree* curArgVal, - unsigned argNum, - InlineResult* inlineResult); + void impInlineRecordArgInfo(InlineInfo* pInlineInfo, + GenTree* curArgVal, + unsigned argNum, + unsigned __int64 bbFlags, + InlineResult* inlineResult); void impInlineInitVars(InlineInfo* pInlineInfo); diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 3472ce02d3e9c9..03c7ba605a76e3 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -23643,6 +23643,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) const InlArgInfo& argInfo = inlArgInfo[argNum]; const bool argIsSingleDef = !argInfo.argHasLdargaOp && !argInfo.argHasStargOp; GenTree* const argNode = inlArgInfo[argNum].argNode; + unsigned __int64 bbFlags = inlArgInfo[argNum].bbFlags; if (argInfo.argHasTmp) { @@ -23705,6 +23706,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) } #endif // DEBUG } + block->bbFlags |= (bbFlags & BBF_SPLIT_GAINED); } else if (argInfo.argIsByRefToStructLocal) { @@ -23758,7 +23760,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) // // Chase through any GT_RET_EXPRs to find the actual argument // expression. - GenTree* actualArgNode = argNode->gtRetExprVal(); + GenTree* actualArgNode = argNode->gtRetExprVal(&bbFlags); // For case (1) // @@ -23834,6 +23836,8 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) // since the box itself will be ignored. gtTryRemoveBoxUpstreamEffects(argNode); } + + block->bbFlags |= (bbFlags & BBF_SPLIT_GAINED); } } } diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 99c075ccfeaabc..8a5dc3b1ebdff5 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -1707,7 +1707,7 @@ struct GenTree inline GenTree* gtEffectiveVal(bool commaOnly = false); // Tunnel through any GT_RET_EXPRs - inline GenTree* gtRetExprVal(unsigned __int64* pbbFlags = nullptr); + inline GenTree* gtRetExprVal(unsigned __int64* pbbFlags); // Return the child of this node if it is a GT_RELOAD or GT_COPY; otherwise simply return the node itself inline GenTree* gtSkipReloadOrCopy(); @@ -7053,23 +7053,16 @@ inline GenTree* GenTree::gtRetExprVal(unsigned __int64* pbbFlags) GenTree* retExprVal = this; unsigned __int64 bbFlags = 0; - for (;;) + assert(pbbFlags != nullptr); + + for (; retExprVal->gtOper == GT_RET_EXPR; retExprVal = retExprVal->AsRetExpr()->gtInlineCandidate) { - if (retExprVal->gtOper == GT_RET_EXPR) - { - GenTreeRetExpr* retExp = retExprVal->AsRetExpr(); - retExprVal = retExp->gtInlineCandidate; - bbFlags = retExp->bbFlags; - } - else - { - if (pbbFlags != nullptr) - { - *pbbFlags = bbFlags; - } - return retExprVal; - } + bbFlags = retExprVal->AsRetExpr()->bbFlags; } + + *pbbFlags = bbFlags; + + return retExprVal; } inline GenTree* GenTree::gtSkipReloadOrCopy() diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 1182d916cd0d6f..1caa1836cdd137 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -18856,10 +18856,8 @@ void Compiler::impCheckCanInline(GenTreeCall* call, // properties are used later by impInlineFetchArg to determine how best to // pass the argument into the inlinee. -void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, - GenTree* curArgVal, - unsigned argNum, - InlineResult* inlineResult) +void Compiler::impInlineRecordArgInfo( + InlineInfo* pInlineInfo, GenTree* curArgVal, unsigned argNum, unsigned __int64 bbFlags, InlineResult* inlineResult) { InlArgInfo* inlCurArgInfo = &pInlineInfo->inlArgInfo[argNum]; @@ -18870,6 +18868,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, } inlCurArgInfo->argNode = curArgVal; + inlCurArgInfo->bbFlags = bbFlags; GenTree* lclVarTree; @@ -19026,9 +19025,10 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) if (thisArg) { - inlArgInfo[0].argIsThis = true; - GenTree* actualThisArg = thisArg->GetNode()->gtRetExprVal(); - impInlineRecordArgInfo(pInlineInfo, actualThisArg, argCnt, inlineResult); + inlArgInfo[0].argIsThis = true; + unsigned __int64 bbFlags = 0; + GenTree* actualThisArg = thisArg->GetNode()->gtRetExprVal(&bbFlags); + impInlineRecordArgInfo(pInlineInfo, actualThisArg, argCnt, bbFlags, inlineResult); if (inlineResult->IsFailure()) { @@ -19063,8 +19063,9 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) continue; } - GenTree* actualArg = use.GetNode()->gtRetExprVal(); - impInlineRecordArgInfo(pInlineInfo, actualArg, argCnt, inlineResult); + unsigned __int64 bbFlags = 0; + GenTree* actualArg = use.GetNode()->gtRetExprVal(&bbFlags); + impInlineRecordArgInfo(pInlineInfo, actualArg, argCnt, bbFlags, inlineResult); if (inlineResult->IsFailure()) { @@ -20295,7 +20296,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if ((objClass == nullptr) || !isExact) { // Walk back through any return expression placeholders - actualThisObj = thisObj->gtRetExprVal(); + unsigned __int64 bbFlags = 0; + actualThisObj = thisObj->gtRetExprVal(&bbFlags); // See if we landed on a call to a special intrinsic method if (actualThisObj->IsCall()) diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h index ff046826d43590..4fd585de9ff81d 100644 --- a/src/coreclr/src/jit/inline.h +++ b/src/coreclr/src/jit/inline.h @@ -541,6 +541,8 @@ struct InlineCandidateInfo : public GuardedDevirtualizationCandidateInfo struct InlArgInfo { + unsigned __int64 bbFlags; // basic block flags that need to be added when replacing GT_RET_EXPR + // with argNode GenTree* argNode; // caller node for this argument GenTree* argBashTmpNode; // tmp node created, if it may be replaced with actual arg unsigned argTmpNum; // the argument tmp number