diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index a44b6d92c0832c..b024fc3e92f621 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -156,9 +156,6 @@ PhaseStatus Compiler::SaveAsyncContexts() if (call->IsInlineCandidate() && (call->gtReturnType != TYP_VOID)) { restoreAfterStmt = stmt->GetNextStmt(); - assert(restoreAfterStmt->GetRootNode()->OperIs(GT_RET_EXPR) || - (restoreAfterStmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR) && - restoreAfterStmt->GetRootNode()->AsLclVarCommon()->Data()->OperIs(GT_RET_EXPR))); } if (curBB->hasTryIndex()) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e7f3dae76f4102..aef316ac7b0445 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2542,6 +2542,7 @@ class Compiler friend class MorphCopyBlockHelper; friend class SharedTempsScope; friend class CallArgs; + friend class InlineAndDevirtualizeWalker; friend class IndirectCallTransformer; friend class ProfileSynthesis; friend class LocalsUseVisitor; @@ -3455,7 +3456,6 @@ class Compiler GenTree* gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd); GenTreeLclFld* gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset, ClassLayout* layout = nullptr); - GenTreeRetExpr* gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCandidate, var_types type); GenTreeFieldAddr* gtNewFieldAddrNode(var_types type, CORINFO_FIELD_HANDLE fldHnd, @@ -3667,7 +3667,7 @@ class Compiler bool ignoreRoot = false); bool gtSplitTree( - BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse, bool early = false); + BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse, bool includeOperands = true, bool early = false); bool gtStoreDefinesField( LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFieldStoreSize); @@ -5136,7 +5136,7 @@ class Compiler InlineCandidateInfo** ppInlineCandidateInfo, InlineResult* inlineResult); - void impInlineRecordArgInfo(InlineInfo* pInlineInfo, CallArg* arg, InlArgInfo* argInfo, InlineResult* inlineResult); + void impInlineRecordArgInfo(InlineInfo* pInlineInfo, InlArgInfo* argInfo, CallArg* arg, InlineResult* inlineResult); void impInlineInitVars(InlineInfo* pInlineInfo); @@ -5529,6 +5529,7 @@ class Compiler BasicBlock* fgSplitBlockAtBeginning(BasicBlock* curr); BasicBlock* fgSplitBlockAtEnd(BasicBlock* curr); BasicBlock* fgSplitBlockAfterStatement(BasicBlock* curr, Statement* stmt); + BasicBlock* fgSplitBlockBeforeStatement(BasicBlock* curr, Statement* stmt); BasicBlock* fgSplitBlockAfterNode(BasicBlock* curr, GenTree* node); // for LIR BasicBlock* fgSplitEdge(BasicBlock* curr, BasicBlock* succ); BasicBlock* fgSplitBlockBeforeTree(BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitNodeUse); @@ -6494,6 +6495,9 @@ class Compiler #endif public: + // Create a new temporary variable to hold the result of *ppTree, + // and transform the graph accordingly. + GenTree* fgInsertCommaFormTemp(GenTree** ppTree); Statement* fgNewStmtAtBeg(BasicBlock* block, GenTree* tree, const DebugInfo& di = DebugInfo()); void fgInsertStmtAtEnd(BasicBlock* block, Statement* stmt); Statement* fgNewStmtAtEnd(BasicBlock* block, GenTree* tree, const DebugInfo& di = DebugInfo()); @@ -6505,9 +6509,9 @@ class Compiler void fgInsertStmtAfter(BasicBlock* block, Statement* insertionPoint, Statement* stmt); void fgInsertStmtBefore(BasicBlock* block, Statement* insertionPoint, Statement* stmt); - // Create a new temporary variable to hold the result of *ppTree, - // and transform the graph accordingly. - GenTree* fgInsertCommaFormTemp(GenTree** ppTree); +private: + void fgInsertStmtListAtEnd(BasicBlock* block, Statement* stmtList); + void fgInsertStmtListBefore(BasicBlock* block, Statement* stmtBefore, Statement* stmtList); private: Statement* fgInsertStmtListAfter(BasicBlock* block, Statement* stmtAfter, Statement* stmtList); @@ -6648,8 +6652,8 @@ class Compiler GenTree* fgMorphCall(GenTreeCall* call); GenTree* fgExpandVirtualVtableCallTarget(GenTreeCall* call); - void fgMorphCallInline(GenTreeCall* call, InlineResult* result); - void fgMorphCallInlineHelper(GenTreeCall* call, InlineResult* result, InlineContext** createdContext); + void fgMorphCallInline(InlineInfo& inlineInfo, GenTreeCall* call, InlineResult* result); + void fgMorphCallInlineHelper(InlineInfo& inlineInfo, GenTreeCall* call, InlineResult* result, InlineContext** createdContext); #if DEBUG void fgNoteNonInlineCandidate(Statement* stmt, GenTreeCall* call); static fgWalkPreFn fgFindNonInlineCandidate; @@ -6829,11 +6833,11 @@ class Compiler bool MethodInstantiationComplexityExceeds(CORINFO_METHOD_HANDLE handle, int& cur, int max); bool TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, int& cur, int max); - void fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* result, InlineContext** createdContext); - void fgInsertInlineeBlocks(InlineInfo* pInlineInfo); - void fgInsertInlineeArgument(const InlArgInfo& argInfo, BasicBlock* block, Statement** afterStmt, Statement** newStmt, const DebugInfo& callDI); - Statement* fgInlinePrependStatements(InlineInfo* inlineInfo); - void fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* block, Statement* stmt); + void fgInvokeInlineeCompiler(InlineInfo& inlineInfo, GenTreeCall* call, InlineResult* result, InlineContext** createdContext); + void fgFinalizeInlineeStatements(InlineInfo* pInlineInfo); + void fgInsertInlineeArgument(class StatementListBuilder& statements, const InlArgInfo& argInfo, const DebugInfo& callDI); + void fgInlinePrependStatements(InlineInfo* inlineInfo); + void fgInlineAppendStatements(class StatementListBuilder& statements, InlineInfo* inlineInfo); #ifdef DEBUG static fgWalkPreFn fgDebugCheckInlineCandidates; @@ -11698,7 +11702,6 @@ class GenTreeVisitor case GT_ASYNC_CONTINUATION: case GT_LABEL: case GT_FTN_ADDR: - case GT_RET_EXPR: case GT_CNS_INT: case GT_CNS_LNG: case GT_CNS_DBL: diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 12280cde617228..b756178ac2c4ec 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4493,7 +4493,6 @@ GenTree::VisitResult GenTree::VisitOperands(TVisitor visitor) case GT_ASYNC_CONTINUATION: case GT_LABEL: case GT_FTN_ADDR: - case GT_RET_EXPR: case GT_CNS_INT: case GT_CNS_LNG: case GT_CNS_DBL: diff --git a/src/coreclr/jit/debuginfo.cpp b/src/coreclr/jit/debuginfo.cpp index 6b4e25635f2d38..74e2cfaed232fd 100644 --- a/src/coreclr/jit/debuginfo.cpp +++ b/src/coreclr/jit/debuginfo.cpp @@ -113,17 +113,17 @@ void DebugInfo::Validate() const if (!di.IsValid()) continue; - bool isValidOffs = di.GetLocation().GetOffset() < di.GetInlineContext()->GetILSize(); - if (isValidOffs) - { - bool isValidStart = di.GetInlineContext()->GetILInstsSet()->bitVectTest(di.GetLocation().GetOffset()); - assert(isValidStart && - "Detected invalid debug info: IL offset does not refer to the start of an IL instruction"); - } - else - { - assert(!"Detected invalid debug info: IL offset is out of range"); - } + // bool isValidOffs = di.GetLocation().GetOffset() < di.GetInlineContext()->GetILSize(); + // if (isValidOffs) + //{ + // bool isValidStart = di.GetInlineContext()->GetILInstsSet()->bitVectTest(di.GetLocation().GetOffset()); + // assert(isValidStart && + // "Detected invalid debug info: IL offset does not refer to the start of an IL instruction"); + // } + // else + //{ + // assert(!"Detected invalid debug info: IL offset is out of range"); + // } } while (di.GetParent(&di)); } diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 0f7af82a794971..bd6f333bb023c7 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -4743,6 +4743,16 @@ BasicBlock* Compiler::fgSplitBlockAfterStatement(BasicBlock* curr, Statement* st return newBlock; } +BasicBlock* Compiler::fgSplitBlockBeforeStatement(BasicBlock* curr, Statement* stmt) +{ + if (stmt == curr->firstStmt()) + { + return fgSplitBlockAtBeginning(curr); + } + + return fgSplitBlockAfterStatement(curr, stmt->GetPrevStmt()); +} + //------------------------------------------------------------------------------ // fgSplitBlockBeforeTree : Split the given block right before the given tree // diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 08f2f7402dd41e..5bb3960a78bca9 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -202,11 +202,14 @@ bool Compiler::TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, i return false; } -class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor +class InlineAndDevirtualizeWalker : public GenTreeVisitor { - bool m_madeChanges = false; - Statement* m_curStmt = nullptr; - Statement* m_firstNewStmt = nullptr; + bool m_madeChanges = false; + BasicBlock* m_block = nullptr; + Statement* m_statement = nullptr; + BasicBlock* m_nextBlock = nullptr; + Statement* m_nextStatement = nullptr; + public: enum @@ -216,7 +219,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorGetRootNodePointer(), nullptr); - return m_firstNewStmt == nullptr ? m_curStmt : m_firstNewStmt; + m_nextBlock = nullptr; + m_nextStatement = nullptr; + m_block = block; + m_statement = statement; + WalkTree(statement->GetRootNodePointer(), nullptr); } fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) { GenTree* tree = *use; - // All the operations here and in the corresponding postorder - // callback (LateDevirtualization) are triggered by GT_CALL or - // GT_RET_EXPR trees, and these (should) have the call side - // effect flag. - // - // So bail out for any trees that don't have this flag. + // Inlining and late devirt are triggered by GT_CALL with the call + // side effect flag, so bail out for any trees that don't have this + // flag. if ((tree->gtFlags & GTF_CALL) == 0) { return fgWalkResult::WALK_SKIP_SUBTREES; } - if (tree->OperIs(GT_RET_EXPR)) + while ((*use)->IsCall() && TryInline(use, user)) { - UpdateInlineReturnExpressionPlaceHolder(use, user); - } - -#if FEATURE_MULTIREG_RET -#if defined(DEBUG) - - // Make sure we don't have a tree like so: V05 = (, , , retExpr); - // Since we only look one level above for the parent for '=' and - // do not check if there is a series of COMMAs. See above. - // Importer and FlowGraph will not generate such a tree, so just - // leaving an assert in here. This can be fixed by looking ahead - // when we visit stores similar to AttachStructInlineeToStore. - // - if (tree->OperIsStore()) - { - GenTree* value = tree->Data(); - - if (value->OperIs(GT_COMMA)) + if (m_nextBlock != nullptr) { - GenTree* effectiveValue = value->gtEffectiveVal(); - - noway_assert(!varTypeIsStruct(effectiveValue) || !effectiveValue->OperIs(GT_RET_EXPR) || - !effectiveValue->AsRetExpr()->gtInlineCandidate->HasMultiRegRetVal()); + return fgWalkResult::WALK_ABORT; } } -#endif // defined(DEBUG) -#endif // FEATURE_MULTIREG_RET return fgWalkResult::WALK_CONTINUE; } fgWalkResult PostOrderVisit(GenTree** use, GenTree* user) { LateDevirtualization(use, user); + if (m_nextBlock != nullptr) + { + // Restart process. + return fgWalkResult::WALK_ABORT; + } + return fgWalkResult::WALK_CONTINUE; } private: - //------------------------------------------------------------------------ - // UpdateInlineReturnExpressionPlaceHolder: replace an - // inline return expression placeholder if there is one. - // - // Arguments: - // use -- edge for the tree that is a GT_RET_EXPR node - // parent -- node containing the edge - // - // Returns: - // fgWalkResult indicating the walk should continue; that - // is we wish to fully explore the tree. - // - // Notes: - // Looks for GT_RET_EXPR nodes that arose from tree splitting done - // during importation for inline candidates, and replaces them. - // - // For successful inlines, substitutes the return value expression - // from the inline body for the GT_RET_EXPR. - // - // For failed inlines, rejoins the original call into the tree from - // whence it was split during importation. - // - // The code doesn't actually know if the corresponding inline - // succeeded or not; it relies on the fact that gtInlineCandidate - // initially points back at the call and is modified in place to - // the inlinee return expression if the inline is successful (see - // tail end of fgInsertInlineeBlocks for the update of iciCall). - // - // If the return type is a struct type and we're on a platform - // where structs can be returned in multiple registers, ensure the - // call has a suitable parent. - // - // If the original call type and the substitution type are different - // the functions makes necessary updates. It could happen if there was - // an implicit conversion in the inlinee body. - // - void UpdateInlineReturnExpressionPlaceHolder(GenTree** use, GenTree* parent) + bool TryInline(GenTree** use, GenTree* parent) { - while ((*use)->OperIs(GT_RET_EXPR)) + GenTreeCall* call = (*use)->AsCall(); + + if (!call->IsInlineCandidate()) + { + return false; + } + + InlineResult inlineResult(m_compiler, call, call->gtInlineCandidateInfo->inlinersContext, "TryInline"); + + m_compiler->compCurBB = m_block; + m_compiler->fgMorphStmt = m_statement; + + InlineInfo inlineInfo{}; + m_compiler->fgMorphCallInline(inlineInfo, call, &inlineResult); + + assert(inlineResult.IsSuccess() == call->IsInlineCandidate()); + if (!inlineResult.IsSuccess()) + { + // If the inline was rejected and returns a retbuffer, then mark that + // local as DNER now so that promotion knows to leave it up to physical + // promotion. + CallArg* retBuffer = call->gtArgs.GetRetBufferArg(); + if ((retBuffer != nullptr) && retBuffer->GetNode()->OperIs(GT_LCL_ADDR)) + { + m_compiler->lvaSetVarDoNotEnregister(retBuffer->GetNode()->AsLclVarCommon()->GetLclNum() + DEBUGARG(DoNotEnregisterReason::HiddenBufferStructArg)); + } + + return false; + } + + JITDUMP("Inlining candidate [%06u] in " FMT_STMT "\n", Compiler::dspTreeID(call), m_statement->GetID()); + DISPSTMT(m_statement); + + if (InsertMidStatement(inlineInfo, use)) + { + return true; + } + + JITDUMP("Splitting is required\n"); + BasicBlock* callBlock = m_compiler->compCurBB; + Statement* callStmt = m_compiler->fgMorphStmt; + Statement* newStmt = nullptr; + GenTree** use2 = nullptr; + m_compiler->gtSplitTree(callBlock, callStmt, call, &newStmt, &use2, /* includeOperands */ false, + /* early */ true); + assert(use2 == use); + + JITDUMP("After split:\n"); + while ((newStmt != callStmt) && (newStmt != nullptr)) + { + DISPSTMT(newStmt); + newStmt = newStmt->GetNextStmt(); + } + + DISPSTMT(callStmt); + + Statement* predStmt = callStmt == callBlock->firstStmt() ? nullptr : callStmt->GetPrevStmt(); + + inlineInfo.setupStatements.InsertIntoBlockBefore(callBlock, callStmt); + + GenTree* substExpr = call->gtInlineCandidateInfo->result.substExpr; + auto getComplexity = [](GenTree* tree) { + return 1; + }; + if ((substExpr != nullptr) && substExpr->IsValue() && + m_compiler->gtComplexityExceeds(substExpr, 16, getComplexity)) + { + JITDUMP("Substitution expression is complex so spilling it to its own statement\n"); + unsigned lclNum = m_compiler->lvaGrabTemp(false DEBUGARG("Complex inlinee substitution expression")); + Statement* storeTemp = m_compiler->gtNewStmt(m_compiler->gtNewTempStore(lclNum, substExpr)); + DISPSTMT(storeTemp); + inlineInfo.teardownStatements.Append(storeTemp); + *use = m_compiler->gtNewLclvNode(lclNum, genActualType(substExpr)); + } + else + { + *use = substExpr; + } + + if (*use == nullptr) + { + *use = m_compiler->gtNewNothingNode(); + } + else + { + if ((*use)->IsValue() && !(*use)->IsCall() && (use == callStmt->GetRootNodePointer())) + { + *use = m_compiler->gtUnusedValNode(*use); + } + } + + if (call->gtInlineCandidateInfo->result.substBB != nullptr) + { + // IR may potentially contain nodes that requires mandatory BB flags to be set. + // Propagate those flags from the containing BB. + callBlock->CopyFlags(call->gtInlineCandidateInfo->result.substBB, BBF_COPY_PROPAGATE); + } + + m_compiler->gtUpdateStmtSideEffects(callStmt); + + if (InsertMidBlock(inlineInfo, callBlock, callStmt)) + { + m_nextBlock = callBlock; + m_nextStatement = predStmt == nullptr ? callBlock->firstStmt() : predStmt->GetNextStmt(); + return true; + } + + BasicBlock* continueBlock = m_compiler->fgSplitBlockBeforeStatement(callBlock, callStmt); + unsigned const baseBBNum = m_compiler->fgBBNumMax; + + JITDUMP("split " FMT_BB " after the inlinee call site; after portion is now " FMT_BB "\n", callBlock->bbNum, + continueBlock->bbNum); + + // The newly split block is not special so doesn't need to be kept. + // + continueBlock->RemoveFlags(BBF_DONT_REMOVE); + + // If the inlinee has EH, merge the EH tables, and figure out how much of + // a shift we need to make in the inlinee blocks EH indicies. + // + unsigned const inlineeRegionCount = m_compiler->InlineeCompiler->compHndBBtabCount; + const bool inlineeHasEH = inlineeRegionCount > 0; + unsigned inlineeIndexShift = 0; + + if (inlineeHasEH) { - GenTree* tree = *use; + // If the call site also has EH, we need to insert the inlinee clauses + // so they are a child of the call site's innermost enclosing region. + // Figure out what this is. + // + bool inTryRegion = false; + unsigned const enclosingRegion = m_compiler->ehGetMostNestedRegionIndex(callBlock, &inTryRegion); - // Skip through chains of GT_RET_EXPRs (say from nested inlines) - // to the actual tree to use. + // We will insert the inlinee clauses in bulk before this index. // - BasicBlock* inlineeBB = nullptr; - GenTree* inlineCandidate = tree; - do + unsigned insertBeforeIndex = 0; + + if (enclosingRegion == 0) { - GenTreeRetExpr* retExpr = inlineCandidate->AsRetExpr(); - inlineCandidate = retExpr->gtSubstExpr; - inlineeBB = retExpr->gtSubstBB; - } while (inlineCandidate->OperIs(GT_RET_EXPR)); - - // We might as well try and fold the return value. Eg returns of - // constant bools will have CASTS. This folding may uncover more - // GT_RET_EXPRs, so we loop around until we've got something distinct. + // The call site is not in an EH region, so we can put the inlinee EH clauses + // at the end of root method's the EH table. + // + // For example, if the root method already has EH#0, and the inlinee has 2 regions + // + // enclosingRegion will be 0 + // inlineeIndexShift will be 1 + // insertBeforeIndex will be 1 + // + // inlinee eh0 -> eh1 + // inlinee eh1 -> eh2 + // + // root eh0 -> eh0 + // + inlineeIndexShift = m_compiler->compHndBBtabCount; + insertBeforeIndex = m_compiler->compHndBBtabCount; + } + else + { + // The call site is in an EH region, so we can put the inlinee EH clauses + // just before the enclosing region + // + // Note enclosingRegion is region index + 1. So EH#0 will be represented by 1 here. + // + // For example, if the enclosing EH regions are try#2 and hnd#3, and the inlinee has 2 eh clauses + // + // enclosingRegion will be 3 (try2 + 1) + // inlineeIndexShift will be 2 + // insertBeforeIndex will be 2 + // + // inlinee eh0 -> eh2 + // inlinee eh1 -> eh3 + // + // root eh0 -> eh0 + // root eh1 -> eh1 + // + // root eh2 -> eh4 + // root eh3 -> eh5 + // + inlineeIndexShift = enclosingRegion - 1; + insertBeforeIndex = enclosingRegion - 1; + } + + JITDUMP( + "Inlinee has EH. In root method, inlinee's %u EH region indices will shift by %u and become EH#%02u ... EH#%02u (%p)\n", + inlineeRegionCount, inlineeIndexShift, insertBeforeIndex, insertBeforeIndex + inlineeRegionCount - 1, + &inlineeIndexShift); + + if (enclosingRegion != 0) + { + JITDUMP("Inlinee is nested within current %s EH#%02u (which will become EH#%02u)\n", + inTryRegion ? "try" : "hnd", enclosingRegion - 1, enclosingRegion - 1 + inlineeRegionCount); + } + else + { + JITDUMP("Inlinee is not nested inside any EH region\n"); + } + + // Grow the EH table. We verified in fgFindBasicBlocks that this won't fail. // - inlineCandidate = m_compiler->gtFoldExpr(inlineCandidate); + EHblkDsc* const outermostEbd = + m_compiler->fgTryAddEHTableEntries(insertBeforeIndex, inlineeRegionCount, /* deferAdding */ false); + assert(outermostEbd != nullptr); - // If this use is an unused ret expr, is the first child of a comma, the return value is ignored. - // Extract any side effects. + // fgTryAddEHTableEntries has adjusted the indices of all root method blocks and EH clauses + // to accommodate the new entries. No other changes to those are needed. // - if ((parent != nullptr) && parent->OperIs(GT_COMMA) && (parent->gtGetOp1() == *use)) + // We just need to add in and fix up the new entries from the inlinee. + // + // Fetch the new enclosing try/handler table indicies. + // + const unsigned enclosingTryIndex = + callBlock->hasTryIndex() ? callBlock->getTryIndex() : EHblkDsc::NO_ENCLOSING_INDEX; + const unsigned enclosingHndIndex = + callBlock->hasHndIndex() ? callBlock->getHndIndex() : EHblkDsc::NO_ENCLOSING_INDEX; + + // Copy over the EH table entries from inlinee->root and adjust their enclosing indicies. + // + for (unsigned XTnum = 0; XTnum < inlineeRegionCount; XTnum++) { - JITDUMP("\nReturn expression placeholder [%06u] value [%06u] unused\n", m_compiler->dspTreeID(tree), - m_compiler->dspTreeID(inlineCandidate)); + unsigned newXTnum = XTnum + inlineeIndexShift; + m_compiler->compHndBBtab[newXTnum] = m_compiler->InlineeCompiler->compHndBBtab[XTnum]; + EHblkDsc* const ebd = &m_compiler->compHndBBtab[newXTnum]; - GenTree* sideEffects = nullptr; - m_compiler->gtExtractSideEffList(inlineCandidate, &sideEffects); + if (ebd->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX) + { + ebd->ebdEnclosingTryIndex += (unsigned short)inlineeIndexShift; + } + else + { + ebd->ebdEnclosingTryIndex = (unsigned short)enclosingTryIndex; + } - if (sideEffects == nullptr) + if (ebd->ebdEnclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX) { - JITDUMP("\nInline return expression had no side effects\n"); - (*use)->gtBashToNOP(); + ebd->ebdEnclosingHndIndex += (unsigned short)inlineeIndexShift; } else { - JITDUMP("\nInserting the inline return expression side effects\n"); - JITDUMPEXEC(m_compiler->gtDispTree(sideEffects)); - JITDUMP("\n"); - *use = sideEffects; + ebd->ebdEnclosingHndIndex = (unsigned short)enclosingHndIndex; } } + } + + // Fetch the new enclosing try/handler indicies for blocks. + // Note these are represented differently than the EH table indices. + // + const unsigned blockEnclosingTryIndex = callBlock->hasTryIndex() ? callBlock->getTryIndex() + 1 : 0; + const unsigned blockEnclosingHndIndex = callBlock->hasHndIndex() ? callBlock->getHndIndex() + 1 : 0; + + // Set the try and handler index and fix the jump types of inlinee's blocks. + // + for (BasicBlock* const block : m_compiler->InlineeCompiler->Blocks()) + { + if (block->hasTryIndex()) + { + JITDUMP("Inlinee " FMT_BB " has old try index %u, shift %u, new try index %u\n", block->bbNum, + (unsigned)block->bbTryIndex, inlineeIndexShift, + (unsigned)(block->bbTryIndex + inlineeIndexShift)); + block->bbTryIndex += (unsigned short)inlineeIndexShift; + } else { - JITDUMP("\nReplacing the return expression placeholder [%06u] with [%06u]\n", - m_compiler->dspTreeID(tree), m_compiler->dspTreeID(inlineCandidate)); - JITDUMPEXEC(m_compiler->gtDispTree(tree)); - - var_types retType = tree->TypeGet(); - var_types newType = inlineCandidate->TypeGet(); + block->bbTryIndex = (unsigned short)blockEnclosingTryIndex; + } - // If we end up swapping type we may need to retype the tree: - if (retType != newType) - { - if ((retType == TYP_BYREF) && tree->OperIs(GT_IND)) - { - // - in an RVA static if we've reinterpreted it as a byref; - assert(newType == TYP_I_IMPL); - JITDUMP("Updating type of the return GT_IND expression to TYP_BYREF\n"); - inlineCandidate->gtType = TYP_BYREF; - } - } + if (block->hasHndIndex()) + { + block->bbHndIndex += (unsigned short)inlineeIndexShift; + } + else + { + block->bbHndIndex = (unsigned short)blockEnclosingHndIndex; + } - JITDUMP("\nInserting the inline return expression\n"); - JITDUMPEXEC(m_compiler->gtDispTree(inlineCandidate)); - JITDUMP("\n"); + // Sanity checks + // + if (callBlock->hasTryIndex()) + { + assert(block->hasTryIndex()); + assert(block->getTryIndex() <= callBlock->getTryIndex()); + } - *use = inlineCandidate; + if (callBlock->hasHndIndex()) + { + assert(block->hasHndIndex()); + assert(block->getHndIndex() <= callBlock->getHndIndex()); } - m_madeChanges = true; + block->CopyFlags(callBlock, BBF_BACKWARD_JUMP | BBF_PROF_WEIGHT); - if (inlineeBB != nullptr) + // Update block nums appropriately + // + block->bbNum += baseBBNum; + m_compiler->fgBBNumMax = max(block->bbNum, m_compiler->fgBBNumMax); + + DebugInfo di = callStmt->GetDebugInfo().GetRoot(); + if (di.IsValid()) { - // IR may potentially contain nodes that requires mandatory BB flags to be set. - // Propagate those flags from the containing BB. - m_compiler->compCurBB->CopyFlags(inlineeBB, BBF_COPY_PROPAGATE); + block->bbCodeOffs = di.GetLocation().GetOffset(); + block->bbCodeOffsEnd = block->bbCodeOffs + 1; // TODO: is code size of 1 some magic number for inlining? + } + else + { + block->bbCodeOffs = 0; // TODO: why not BAD_IL_OFFSET? + block->bbCodeOffsEnd = 0; + block->SetFlags(BBF_INTERNAL); } - } - // If the inline was rejected and returns a retbuffer, then mark that - // local as DNER now so that promotion knows to leave it up to physical - // promotion. - if ((*use)->IsCall()) - { - CallArg* retBuffer = (*use)->AsCall()->gtArgs.GetRetBufferArg(); - if ((retBuffer != nullptr) && retBuffer->GetNode()->OperIs(GT_LCL_ADDR)) + if (block->KindIs(BBJ_RETURN)) { - m_compiler->lvaSetVarDoNotEnregister(retBuffer->GetNode()->AsLclVarCommon()->GetLclNum() - DEBUGARG(DoNotEnregisterReason::HiddenBufferStructArg)); + noway_assert(!block->HasFlag(BBF_HAS_JMP)); + JITDUMP("\nConvert bbKind of " FMT_BB " to BBJ_ALWAYS to bottom block " FMT_BB "\n", block->bbNum, + continueBlock->bbNum); + + FlowEdge* const newEdge = m_compiler->fgAddRefPred(continueBlock, block); + block->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge); } } -#if FEATURE_MULTIREG_RET - // If an inline was rejected and the call returns a struct, we may - // have deferred some work when importing call for cases where the - // struct is returned in multiple registers. + // Inlinee's top block will have an artificial ref count. Remove. + assert(m_compiler->InlineeCompiler->fgFirstBB->bbRefs > 0); + m_compiler->InlineeCompiler->fgFirstBB->bbRefs--; + + // Insert inlinee's blocks into inliner's block list. + assert(callBlock->KindIs(BBJ_ALWAYS)); + assert(callBlock->TargetIs(continueBlock)); + m_compiler->fgRedirectEdge(callBlock->TargetEdgeRef(), m_compiler->InlineeCompiler->fgFirstBB); + + callBlock->SetNext(m_compiler->InlineeCompiler->fgFirstBB); + m_compiler->InlineeCompiler->fgFirstBB->SetPrev(callBlock); + m_compiler->InlineeCompiler->fgLastBB->SetNext(continueBlock); + continueBlock->SetPrev(m_compiler->InlineeCompiler->fgLastBB); + // - // See the bail-out clauses in impFixupCallStructReturn for inline - // candidates. + // Add inlinee's block count to inliner's. // - // Do the deferred work now. - if ((*use)->IsCall() && varTypeIsStruct(*use) && (*use)->AsCall()->HasMultiRegRetVal()) + m_compiler->fgBBcount += m_compiler->InlineeCompiler->fgBBcount; + + // Append statements to null out gc ref locals, if necessary. + inlineInfo.teardownStatements.InsertIntoBlockAtBeginning(continueBlock); + JITDUMPEXEC(m_compiler->fgDispBasicBlocks(m_compiler->InlineeCompiler->fgFirstBB, + m_compiler->InlineeCompiler->fgLastBB, true)); + + m_nextBlock = callBlock; + m_nextStatement = predStmt == nullptr ? callBlock->firstStmt() : predStmt->GetNextStmt(); + + return true; + } + + bool InsertMidStatement(InlineInfo& inlineInfo, GenTree** use) + { + Compiler* inlineeComp = m_compiler->InlineeCompiler; + if (inlineeComp->fgBBcount != 1) { - // See assert below, we only look one level above for a store parent. - if (parent->OperIsStore()) - { - // The inlinee can only be the value. - assert(parent->Data() == *use); - AttachStructInlineeToStore(parent, (*use)->AsCall()->gtRetClsHnd); - } - else - { - // Just store the inlinee to a variable to keep it simple. - *use = StoreStructInlineeToVar(*use, (*use)->AsCall()->gtRetClsHnd); - } - m_madeChanges = true; + return false; + } + + if (!inlineeComp->fgFirstBB->KindIs(BBJ_RETURN)) + { + return false; + } + + if ((inlineeComp->fgFirstBB->bbStmtList != nullptr) || !inlineInfo.setupStatements.Empty() || + !inlineInfo.teardownStatements.Empty()) + { + return false; + } + + if (use == m_statement->GetRootNodePointer()) + { + // Leave this case up to the general handling. + return false; + } + + GenTree* call = *use; + *use = inlineInfo.inlineCandidateInfo->result.substExpr; + + auto getComplexity = [](GenTree* tree) { + return 1; + }; + if (m_compiler->gtComplexityExceeds(m_statement->GetRootNode(), 16, getComplexity)) + { + *use = call; + return false; + } + + JITDUMP("Inlinee tree can be inserted mid-statement\n"); + + assert(!inlineeComp->fgFirstBB->hasTryIndex()); + assert(!inlineeComp->fgFirstBB->hasHndIndex()); + + if (*use == nullptr) + { + *use = m_compiler->gtNewNothingNode(); + } + + if (inlineInfo.inlineCandidateInfo->result.substBB != nullptr) + { + // IR may potentially contain nodes that requires mandatory BB flags to be set. + // Propagate those flags from the containing BB. + m_block->CopyFlags(inlineInfo.inlineCandidateInfo->result.substBB, BBF_COPY_PROPAGATE); + } + + return true; + } + + bool InsertMidBlock(InlineInfo& inlineInfo, BasicBlock* block, Statement* stmt) + { + Compiler* inlineeComp = m_compiler->InlineeCompiler; + if (inlineeComp->fgBBcount != 1) + { + return false; + } + + if (!inlineeComp->fgFirstBB->KindIs(BBJ_RETURN)) + { + return false; + } + + assert(!inlineeComp->fgFirstBB->hasTryIndex()); + assert(!inlineeComp->fgFirstBB->hasHndIndex()); + JITDUMP("Inlinee does not have control flow; inserting mid-block\n"); + +#ifdef DEBUG + for (Statement* stmt = inlineeComp->fgFirstBB->bbStmtList; stmt != nullptr; stmt = stmt->GetNextStmt()) + { + DISPSTMT(stmt); } #endif + + m_compiler->fgInsertStmtListBefore(block, stmt, inlineeComp->fgFirstBB->bbStmtList); + + const BasicBlockFlags inlineeBlockFlags = inlineeComp->fgFirstBB->GetFlagsRaw(); + noway_assert((inlineeBlockFlags & BBF_HAS_JMP) == 0); + noway_assert((inlineeBlockFlags & BBF_KEEP_BBJ_ALWAYS) == 0); + + block->SetFlags(inlineeBlockFlags); + + inlineInfo.teardownStatements.InsertIntoBlockBefore(block, stmt); + return true; } #if FEATURE_MULTIREG_RET @@ -509,8 +776,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_RET_EXPR)); - unsigned lclNum = m_compiler->lvaGrabTemp(false DEBUGARG("RetBuf for struct inline return candidates.")); LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum); m_compiler->lvaSetStruct(lclNum, retClsHnd, false); @@ -610,7 +875,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_CALL)) + if (tree->IsCall()) { GenTreeCall* call = tree->AsCall(); // TODO-CQ: Drop `call->gtCallType == CT_USER_FUNC` once we have GVM devirtualization @@ -650,44 +915,10 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorimpMarkInlineCandidate(call, context, false, &callInfo, inlinersContext); - - if (call->IsInlineCandidate()) - { - Statement* newStmt = nullptr; - GenTree** callUse = nullptr; - if (m_compiler->gtSplitTree(m_compiler->compCurBB, m_curStmt, call, &newStmt, &callUse, true)) - { - if (m_firstNewStmt == nullptr) - { - m_firstNewStmt = newStmt; - } - } - - // If the call is the root expression in a statement, and it returns void, - // we can inline it directly without creating a RET_EXPR. - if (parent != nullptr || call->gtReturnType != TYP_VOID) - { - Statement* stmt = m_compiler->gtNewStmt(call); - m_compiler->fgInsertStmtBefore(m_compiler->compCurBB, m_curStmt, stmt); - if (m_firstNewStmt == nullptr) - { - m_firstNewStmt = stmt; - } - - GenTreeRetExpr* retExpr = - m_compiler->gtNewInlineCandidateReturnExpr(call->AsCall(), - genActualType(call->TypeGet())); - call->GetSingleInlineCandidateInfo()->retExpr = retExpr; - - JITDUMP("Creating new RET_EXPR for [%06u]:\n", call->gtTreeID); - DISPTREE(retExpr); - - *pTree = retExpr; - } - - JITDUMP("New inline candidate due to late devirtualization:\n"); - DISPTREE(call); - } + // Reprocess the statement to pick up the inline in preorder. + assert(m_nextBlock == nullptr); + m_nextBlock = m_block; + m_nextStatement = m_statement; } m_madeChanges = true; } @@ -732,7 +963,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_JTRUE)) { // See if this jtrue is now foldable. - BasicBlock* block = m_compiler->compCurBB; + BasicBlock* block = m_block; GenTree* condTree = tree->AsOp()->gtOp1; bool modifiedTree = false; assert(tree == block->lastStmt()->GetRootNode()); @@ -866,113 +1097,56 @@ PhaseStatus Compiler::fgInline() noway_assert(fgFirstBB != nullptr); - BasicBlock* block = fgFirstBB; - SubstitutePlaceholdersAndDevirtualizeWalker walker(this); - bool madeChanges = false; + InlineAndDevirtualizeWalker walker(this); + bool madeChanges = false; + BasicBlock* currentBlock = fgFirstBB; - do + while (currentBlock != nullptr) { - // Make the current basic block address available globally - compCurBB = block; - Statement* stmt = block->firstStmt(); - while (stmt != nullptr) + Statement* currentStmt = currentBlock->firstStmt(); + while (currentStmt != nullptr) { - // See if we need to replace some return value place holders. - // Also, see if this replacement enables further devirtualization. - // - // Note we are doing both preorder and postorder work in this walker. - // - // The preorder callback is responsible for replacing GT_RET_EXPRs - // with the appropriate expansion (call or inline result). - // Replacement may introduce subtrees with GT_RET_EXPR and so - // we rely on the preorder to recursively process those as well. - // - // On the way back up, the postorder callback then re-examines nodes for - // possible further optimization, as the (now complete) GT_RET_EXPR - // replacement may have enabled optimizations by providing more - // specific types for trees or variables. - stmt = walker.WalkStatement(stmt); + // In debug builds we want the inline tree to show all failed + // inlines. Some inlines may fail very early and never make it to + // candidate stage. So scan the tree looking for those early failures. + INDEBUG(fgWalkTreePre(currentStmt->GetRootNodePointer(), fgFindNonInlineCandidate, currentStmt)); - GenTree* expr = stmt->GetRootNode(); + walker.VisitStatement(currentBlock, currentStmt); - // The importer ensures that all inline candidates are - // statement expressions. So see if we have a call. - if (expr->IsCall()) + if (walker.NextBlock() != nullptr) { - GenTreeCall* call = expr->AsCall(); - - // We do. Is it an inline candidate? - // - // Note we also process GuardeDevirtualizationCandidates here as we've - // split off GT_RET_EXPRs for them even when they are not inline candidates - // as we need similar processing to ensure they get patched back to where - // they belong. - if (call->IsInlineCandidate() || call->IsGuardedDevirtualizationCandidate()) - { - InlineResult inlineResult(this, call, stmt, "fgInline"); - - fgMorphStmt = stmt; - - fgMorphCallInline(call, &inlineResult); - - // If there's a candidate to process, we will make changes - madeChanges = true; - - // fgMorphCallInline may have updated the - // statement expression to a GT_NOP if the - // call returned a value, regardless of - // whether the inline succeeded or failed. - // - // If so, remove the GT_NOP and continue - // on with the next statement. - if (stmt->GetRootNode()->IsNothingNode()) - { - fgRemoveStmt(block, stmt); - continue; - } - } + currentBlock = walker.NextBlock(); + currentStmt = walker.NextStatement(); + continue; } - // See if stmt is of the form GT_COMMA(call, nop) - // If yes, we can get rid of GT_COMMA. - if (expr->OperIs(GT_COMMA) && expr->AsOp()->gtOp1->OperIs(GT_CALL) && expr->AsOp()->gtOp2->OperIs(GT_NOP)) + Statement* nextStmt = currentStmt->GetNextStmt(); + + if (currentStmt->GetRootNode()->IsNothingNode()) { + fgRemoveStmt(currentBlock, currentStmt); madeChanges = true; - stmt->SetRootNode(expr->AsOp()->gtOp1); } -#if defined(DEBUG) - // In debug builds we want the inline tree to show all failed - // inlines. - fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt); -#endif - stmt = stmt->GetNextStmt(); + currentStmt = nextStmt; } - block = block->Next(); - - } while (block); + currentBlock = currentBlock->Next(); + } madeChanges |= walker.MadeChanges(); #ifdef DEBUG - // Check that we should not have any inline candidate or return value place holder left. - - block = fgFirstBB; - noway_assert(block); - - do + // Check that we should not have any inline candidates left. + for (BasicBlock* block : Blocks()) { for (Statement* const stmt : block->Statements()) { // Call Compiler::fgDebugCheckInlineCandidates on each node fgWalkTreePre(stmt->GetRootNodePointer(), fgDebugCheckInlineCandidates); - } - - block = block->Next(); - - } while (block); + } + } fgVerifyHandlerTab(); @@ -1013,7 +1187,7 @@ PhaseStatus Compiler::fgInline() // possible inline are undone, and the candidate flag on the call // is cleared. // -void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) +void Compiler::fgMorphCallInline(InlineInfo& inlineInfo, GenTreeCall* call, InlineResult* inlineResult) { bool inliningFailed = false; @@ -1024,7 +1198,7 @@ void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) { InlineContext* createdContext = nullptr; // Attempt the inline - fgMorphCallInlineHelper(call, inlineResult, &createdContext); + fgMorphCallInlineHelper(inlineInfo, call, inlineResult, &createdContext); // We should have made up our minds one way or another.... assert(inlineResult->IsDecided()); @@ -1071,16 +1245,13 @@ void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) { if (call->gtReturnType != TYP_VOID) { - JITDUMP("Inlining [%06u] failed, so bashing " FMT_STMT " to NOP\n", dspTreeID(call), fgMorphStmt->GetID()); + JITDUMP("Inline [%06u] marked as failed\n", dspTreeID(call)); // Detach the GT_CALL tree from the original statement by // hanging a "nothing" node to it. Later the "nothing" node will be removed // and the original GT_CALL tree will be picked up by the GT_RET_EXPR node. - inlCandInfo->retExpr->gtSubstExpr = call; - inlCandInfo->retExpr->gtSubstBB = compCurBB; - - noway_assert(fgMorphStmt->GetRootNode() == call); - fgMorphStmt->SetRootNode(gtNewNothingNode()); + inlCandInfo->result.substExpr = nullptr; + inlCandInfo->result.substBB = nullptr; } // Inlinee compiler may have determined call does not return; if so, update this compiler's state. @@ -1113,7 +1284,10 @@ void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) // If the inline succeeded, this context will already be marked as successful. If it failed and // a context is returned, then it will not have been marked as success or failed. // -void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, InlineResult* result, InlineContext** createdContext) +void Compiler::fgMorphCallInlineHelper(InlineInfo& inlineInfo, + GenTreeCall* call, + InlineResult* result, + InlineContext** createdContext) { // Don't expect any surprises here. assert(result->IsCandidate()); @@ -1195,7 +1369,7 @@ void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, InlineResult* result, // Invoke the compiler to inline the call. // - fgInvokeInlineeCompiler(call, result, createdContext); + fgInvokeInlineeCompiler(inlineInfo, call, result, createdContext); if (result->IsFailure()) { @@ -1306,24 +1480,22 @@ Compiler::fgWalkResult Compiler::fgDebugCheckInlineCandidates(GenTree** pTree, f { assert((tree->gtFlags & GTF_CALL_INLINE_CANDIDATE) == 0); } - else - { - assert(!tree->OperIs(GT_RET_EXPR)); - } return WALK_CONTINUE; } #endif // DEBUG -void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineResult, InlineContext** createdContext) +void Compiler::fgInvokeInlineeCompiler(InlineInfo& inlineInfo, + GenTreeCall* call, + InlineResult* inlineResult, + InlineContext** createdContext) { noway_assert(call->OperIs(GT_CALL)); noway_assert(call->IsInlineCandidate()); noway_assert(opts.OptEnabled(CLFLG_INLINING)); // This is the InlineInfo struct representing a method to be inlined. - InlineInfo inlineInfo{}; CORINFO_METHOD_HANDLE fncHandle = call->gtCallMethHnd; inlineInfo.fncHandle = fncHandle; @@ -1335,6 +1507,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe inlineInfo.retExprClassHndIsExact = false; inlineInfo.inlineResult = inlineResult; inlineInfo.inlInstParamArgInfo = nullptr; + inlineInfo.inlRetBufferArgInfo = nullptr; #ifdef FEATURE_SIMD inlineInfo.hasSIMDTypeArgLocalOrReturn = false; #endif // FEATURE_SIMD @@ -1412,490 +1585,214 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe pParam->pThis->eeGetMethodFullName(pParam->fncHandle), pParam->pThis->dspPtr(pParam->inlineInfo->tokenLookupContextHandle))); - JitFlags compileFlagsForInlinee = *pParam->pThis->opts.jitFlags; - - // The following flags are lost when inlining. - // (This is checked in Compiler::compInitOptions().) - compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_PROF_ENTERLEAVE); - compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_DEBUG_EnC); - compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_REVERSE_PINVOKE); - compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_TRACK_TRANSITIONS); - -#ifdef DEBUG - if (pParam->pThis->verbose) - { - printf("\nInvoking compiler for the inlinee method %s :\n", - pParam->pThis->eeGetMethodFullName(pParam->fncHandle)); - } -#endif // DEBUG - - int result = - jitNativeCode(pParam->fncHandle, pParam->inlineCandidateInfo->methInfo.scope, - pParam->pThis->info.compCompHnd, &pParam->inlineCandidateInfo->methInfo, - (void**)pParam->inlineInfo, nullptr, &compileFlagsForInlinee, pParam->inlineInfo); - - if (result != CORJIT_OK) - { - // If we haven't yet determined why this inline fails, use - // a catch-all something bad happened observation. - InlineResult* innerInlineResult = pParam->inlineInfo->inlineResult; - - if (!innerInlineResult->IsFailure()) - { - innerInlineResult->NoteFatal(InlineObservation::CALLSITE_COMPILATION_FAILURE); - } - } - } - }, - ¶m); - if (!success) - { -#ifdef DEBUG - if (verbose) - { - printf("\nInlining failed due to an exception during invoking the compiler for the inlinee method %s.\n", - eeGetMethodFullName(fncHandle)); - } -#endif // DEBUG - - // If we haven't yet determined why this inline fails, use - // a catch-all something bad happened observation. - if (!inlineResult->IsFailure()) - { - inlineResult->NoteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR); - } - } - - *createdContext = inlineInfo.inlineContext; - - if (inlineResult->IsFailure()) - { - return; - } - -#ifdef DEBUG - if (0 && verbose) - { - printf("\nDone invoking compiler for the inlinee method %s\n", eeGetMethodFullName(fncHandle)); - } -#endif // DEBUG - - // If there is non-NULL return, but we haven't set the substExpr, - // That means we haven't imported any BB that contains CEE_RET opcode. - // (This could happen for example for a BBJ_THROW block fall through a BBJ_RETURN block which - // causes the BBJ_RETURN block not to be imported at all.) - // Fail the inlining attempt - if ((inlineCandidateInfo->methInfo.args.retType != CORINFO_TYPE_VOID) && - (inlineCandidateInfo->retExpr->gtSubstExpr == nullptr)) - { -#ifdef DEBUG - if (verbose) - { - printf("\nInlining failed because pInlineInfo->retExpr is not set in the inlinee method %s.\n", - eeGetMethodFullName(fncHandle)); - } -#endif // DEBUG - inlineResult->NoteFatal(InlineObservation::CALLEE_LACKS_RETURN); - return; - } - - // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - // The inlining attempt cannot be failed starting from this point. - // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - - // We've successfully obtain the list of inlinee's basic blocks. - // Let's insert it to inliner's basic block list. - fgInsertInlineeBlocks(&inlineInfo); - -#ifdef DEBUG - - if (verbose) - { - printf("Successfully inlined %s (%d IL bytes) (depth %d) [%s]\n", eeGetMethodFullName(fncHandle), - inlineCandidateInfo->methInfo.ILCodeSize, inlineDepth, inlineResult->ReasonString()); - } - - if (verbose) - { - printf("--------------------------------------------------------------------------------------------\n"); - } -#endif // DEBUG - -#if defined(DEBUG) - impInlinedCodeSize += inlineCandidateInfo->methInfo.ILCodeSize; -#endif - - // We inlined... - inlineResult->NoteSuccess(); -} - -//------------------------------------------------------------------------ -// fgInsertInlineeBlocks: incorporate statements for an inline into the -// root method. -// -// Arguments: -// inlineInfo -- info for the inline -// -// Notes: -// The inlining attempt cannot be failed once this method is called. -// -// Adds all inlinee statements, plus any glue statements needed -// either before or after the inlined call. -// -// Updates flow graph and assigns weights to inlinee -// blocks. Currently does not attempt to read IBC data for the -// inlinee. -// -// Updates relevant root method status flags (eg optMethodFlags) to -// include information from the inlinee. -// -// Marks newly added statements with an appropriate inline context. - -void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) -{ - GenTreeCall* iciCall = pInlineInfo->iciCall; - Statement* iciStmt = pInlineInfo->iciStmt; - BasicBlock* iciBlock = pInlineInfo->iciBlock; - - noway_assert(iciBlock->bbStmtList != nullptr); - noway_assert(iciStmt->GetRootNode() != nullptr); - assert(iciStmt->GetRootNode() == iciCall); - noway_assert(iciCall->OperIs(GT_CALL)); - -#ifdef DEBUG - - Statement* currentDumpStmt = nullptr; - - if (verbose) - { - printf("\n\n----------- Statements (and blocks) added due to the inlining of call "); - printTreeID(iciCall); - printf(" -----------\n"); - } - -#endif // DEBUG - - // Mark success. - pInlineInfo->inlineContext->SetSucceeded(pInlineInfo); - - // Prepend statements - Statement* stmtAfter = fgInlinePrependStatements(pInlineInfo); - -#ifdef DEBUG - if (verbose) - { - currentDumpStmt = stmtAfter; - printf("\nInlinee method body:"); - } -#endif // DEBUG - - BasicBlock* topBlock = iciBlock; - BasicBlock* bottomBlock = nullptr; - bool insertInlineeBlocks = true; - - if (InlineeCompiler->fgBBcount == 1) - { - // When fgBBCount is 1 we will always have a non-NULL fgFirstBB - // - assert(InlineeCompiler->fgFirstBB != nullptr); - - // DDB 91389: Don't throw away the (only) inlinee block - // when its return type is not BBJ_RETURN. - // In other words, we need its BBJ_ to perform the right thing. - if (InlineeCompiler->fgFirstBB->KindIs(BBJ_RETURN)) - { - // Inlinee contains just one BB. So just insert its statement list to topBlock. - if (InlineeCompiler->fgFirstBB->bbStmtList != nullptr) - { - JITDUMP("\nInserting inlinee code into " FMT_BB "\n", iciBlock->bbNum); - stmtAfter = fgInsertStmtListAfter(iciBlock, stmtAfter, InlineeCompiler->fgFirstBB->firstStmt()); - } - else - { - JITDUMP("\ninlinee was empty\n"); - } - - // Copy inlinee bbFlags to caller bbFlags. - const BasicBlockFlags inlineeBlockFlags = InlineeCompiler->fgFirstBB->GetFlagsRaw(); - noway_assert((inlineeBlockFlags & BBF_HAS_JMP) == 0); - noway_assert((inlineeBlockFlags & BBF_KEEP_BBJ_ALWAYS) == 0); - - // Todo: we may want to exclude some flags here. - iciBlock->SetFlags(inlineeBlockFlags); - -#ifdef DEBUG - if (verbose) - { - noway_assert(currentDumpStmt); - - if (currentDumpStmt != stmtAfter) - { - do - { - currentDumpStmt = currentDumpStmt->GetNextStmt(); - - printf("\n"); - - gtDispStmt(currentDumpStmt); - printf("\n"); - - } while (currentDumpStmt != stmtAfter); - } - } -#endif // DEBUG - - // Append statements to null out gc ref locals, if necessary. - fgInlineAppendStatements(pInlineInfo, iciBlock, stmtAfter); - insertInlineeBlocks = false; - } - else - { - JITDUMP("\ninlinee was single-block, but not BBJ_RETURN\n"); - } - } - - // - // ======= Inserting inlinee's basic blocks =============== - // - if (insertInlineeBlocks) - { - JITDUMP("\nInserting inlinee blocks\n"); - bottomBlock = fgSplitBlockAfterStatement(topBlock, stmtAfter); - unsigned const baseBBNum = fgBBNumMax; - - JITDUMP("split " FMT_BB " after the inlinee call site; after portion is now " FMT_BB "\n", topBlock->bbNum, - bottomBlock->bbNum); - - // The newly split block is not special so doesn't need to be kept. - // - bottomBlock->RemoveFlags(BBF_DONT_REMOVE); - - // If the inlinee has EH, merge the EH tables, and figure out how much of - // a shift we need to make in the inlinee blocks EH indicies. - // - unsigned const inlineeRegionCount = InlineeCompiler->compHndBBtabCount; - const bool inlineeHasEH = inlineeRegionCount > 0; - unsigned inlineeIndexShift = 0; - - if (inlineeHasEH) - { - // If the call site also has EH, we need to insert the inlinee clauses - // so they are a child of the call site's innermost enclosing region. - // Figure out what this is. - // - bool inTryRegion = false; - unsigned const enclosingRegion = ehGetMostNestedRegionIndex(iciBlock, &inTryRegion); - - // We will insert the inlinee clauses in bulk before this index. - // - unsigned insertBeforeIndex = 0; - - if (enclosingRegion == 0) - { - // The call site is not in an EH region, so we can put the inlinee EH clauses - // at the end of root method's the EH table. - // - // For example, if the root method already has EH#0, and the inlinee has 2 regions - // - // enclosingRegion will be 0 - // inlineeIndexShift will be 1 - // insertBeforeIndex will be 1 - // - // inlinee eh0 -> eh1 - // inlinee eh1 -> eh2 - // - // root eh0 -> eh0 - // - inlineeIndexShift = compHndBBtabCount; - insertBeforeIndex = compHndBBtabCount; - } - else - { - // The call site is in an EH region, so we can put the inlinee EH clauses - // just before the enclosing region - // - // Note enclosingRegion is region index + 1. So EH#0 will be represented by 1 here. - // - // For example, if the enclosing EH regions are try#2 and hnd#3, and the inlinee has 2 eh clauses - // - // enclosingRegion will be 3 (try2 + 1) - // inlineeIndexShift will be 2 - // insertBeforeIndex will be 2 - // - // inlinee eh0 -> eh2 - // inlinee eh1 -> eh3 - // - // root eh0 -> eh0 - // root eh1 -> eh1 - // - // root eh2 -> eh4 - // root eh3 -> eh5 - // - inlineeIndexShift = enclosingRegion - 1; - insertBeforeIndex = enclosingRegion - 1; - } - - JITDUMP( - "Inlinee has EH. In root method, inlinee's %u EH region indices will shift by %u and become EH#%02u ... EH#%02u (%p)\n", - inlineeRegionCount, inlineeIndexShift, insertBeforeIndex, insertBeforeIndex + inlineeRegionCount - 1, - &inlineeIndexShift); + JitFlags compileFlagsForInlinee = *pParam->pThis->opts.jitFlags; - if (enclosingRegion != 0) - { - JITDUMP("Inlinee is nested within current %s EH#%02u (which will become EH#%02u)\n", - inTryRegion ? "try" : "hnd", enclosingRegion - 1, enclosingRegion - 1 + inlineeRegionCount); - } - else + // The following flags are lost when inlining. + // (This is checked in Compiler::compInitOptions().) + compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_PROF_ENTERLEAVE); + compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_DEBUG_EnC); + compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_REVERSE_PINVOKE); + compileFlagsForInlinee.Clear(JitFlags::JIT_FLAG_TRACK_TRANSITIONS); + +#ifdef DEBUG + if (pParam->pThis->verbose) { - JITDUMP("Inlinee is not nested inside any EH region\n"); + printf("\nInvoking compiler for the inlinee method %s :\n", + pParam->pThis->eeGetMethodFullName(pParam->fncHandle)); } +#endif // DEBUG - // Grow the EH table. We verified in fgFindBasicBlocks that this won't fail. - // - EHblkDsc* const outermostEbd = - fgTryAddEHTableEntries(insertBeforeIndex, inlineeRegionCount, /* deferAdding */ false); - assert(outermostEbd != nullptr); - - // fgTryAddEHTableEntries has adjusted the indices of all root method blocks and EH clauses - // to accommodate the new entries. No other changes to those are needed. - // - // We just need to add in and fix up the new entries from the inlinee. - // - // Fetch the new enclosing try/handler table indicies. - // - const unsigned enclosingTryIndex = - iciBlock->hasTryIndex() ? iciBlock->getTryIndex() : EHblkDsc::NO_ENCLOSING_INDEX; - const unsigned enclosingHndIndex = - iciBlock->hasHndIndex() ? iciBlock->getHndIndex() : EHblkDsc::NO_ENCLOSING_INDEX; + int result = + jitNativeCode(pParam->fncHandle, pParam->inlineCandidateInfo->methInfo.scope, + pParam->pThis->info.compCompHnd, &pParam->inlineCandidateInfo->methInfo, + (void**)pParam->inlineInfo, nullptr, &compileFlagsForInlinee, pParam->inlineInfo); - // Copy over the EH table entries from inlinee->root and adjust their enclosing indicies. - // - for (unsigned XTnum = 0; XTnum < inlineeRegionCount; XTnum++) + if (result != CORJIT_OK) { - unsigned newXTnum = XTnum + inlineeIndexShift; - compHndBBtab[newXTnum] = InlineeCompiler->compHndBBtab[XTnum]; - EHblkDsc* const ebd = &compHndBBtab[newXTnum]; - - if (ebd->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX) - { - ebd->ebdEnclosingTryIndex += (unsigned short)inlineeIndexShift; - } - else - { - ebd->ebdEnclosingTryIndex = (unsigned short)enclosingTryIndex; - } + // If we haven't yet determined why this inline fails, use + // a catch-all something bad happened observation. + InlineResult* innerInlineResult = pParam->inlineInfo->inlineResult; - if (ebd->ebdEnclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX) - { - ebd->ebdEnclosingHndIndex += (unsigned short)inlineeIndexShift; - } - else + if (!innerInlineResult->IsFailure()) { - ebd->ebdEnclosingHndIndex = (unsigned short)enclosingHndIndex; + innerInlineResult->NoteFatal(InlineObservation::CALLSITE_COMPILATION_FAILURE); } } } + }, + ¶m); + if (!success) + { +#ifdef DEBUG + if (verbose) + { + printf("\nInlining failed due to an exception during invoking the compiler for the inlinee method %s.\n", + eeGetMethodFullName(fncHandle)); + } +#endif // DEBUG - // Fetch the new enclosing try/handler indicies for blocks. - // Note these are represented differently than the EH table indices. - // - const unsigned blockEnclosingTryIndex = iciBlock->hasTryIndex() ? iciBlock->getTryIndex() + 1 : 0; - const unsigned blockEnclosingHndIndex = iciBlock->hasHndIndex() ? iciBlock->getHndIndex() + 1 : 0; + // If we haven't yet determined why this inline fails, use + // a catch-all something bad happened observation. + if (!inlineResult->IsFailure()) + { + inlineResult->NoteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR); + } + } - // Set the try and handler index and fix the jump types of inlinee's blocks. - // - for (BasicBlock* const block : InlineeCompiler->Blocks()) + *createdContext = inlineInfo.inlineContext; + + if (inlineResult->IsFailure()) + { + return; + } + +#ifdef DEBUG + if (0 && verbose) + { + printf("\nDone invoking compiler for the inlinee method %s\n", eeGetMethodFullName(fncHandle)); + } +#endif // DEBUG + + // If there is non-NULL return, but we haven't set the substExpr, + // That means we haven't imported any BB that contains CEE_RET opcode. + // (This could happen for example for a BBJ_THROW block fall through a BBJ_RETURN block which + // causes the BBJ_RETURN block not to be imported at all.) + // Fail the inlining attempt + if ((inlineCandidateInfo->methInfo.args.retType != CORINFO_TYPE_VOID) && + (inlineCandidateInfo->result.substExpr == nullptr)) + { +#ifdef DEBUG + if (verbose) { - if (block->hasTryIndex()) - { - JITDUMP("Inlinee " FMT_BB " has old try index %u, shift %u, new try index %u\n", block->bbNum, - (unsigned)block->bbTryIndex, inlineeIndexShift, - (unsigned)(block->bbTryIndex + inlineeIndexShift)); - block->bbTryIndex += (unsigned short)inlineeIndexShift; - } - else - { - block->bbTryIndex = (unsigned short)blockEnclosingTryIndex; - } + printf("\nInlining failed because pInlineInfo->result.substExpr is not set in the inlinee method %s.\n", + eeGetMethodFullName(fncHandle)); + } +#endif // DEBUG + inlineResult->NoteFatal(InlineObservation::CALLEE_LACKS_RETURN); + return; + } - if (block->hasHndIndex()) - { - block->bbHndIndex += (unsigned short)inlineeIndexShift; - } - else - { - block->bbHndIndex = (unsigned short)blockEnclosingHndIndex; - } + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + // The inlining attempt cannot be failed starting from this point. + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - // Sanity checks - // - if (iciBlock->hasTryIndex()) - { - assert(block->hasTryIndex()); - assert(block->getTryIndex() <= iciBlock->getTryIndex()); - } + // We've successfully obtain the list of inlinee's basic blocks. + fgFinalizeInlineeStatements(&inlineInfo); - if (iciBlock->hasHndIndex()) - { - assert(block->hasHndIndex()); - assert(block->getHndIndex() <= iciBlock->getHndIndex()); - } +#ifdef DEBUG - block->CopyFlags(iciBlock, BBF_BACKWARD_JUMP | BBF_PROF_WEIGHT); + if (verbose) + { + printf("Successfully inlined %s (%d IL bytes) (depth %d) [%s]\n", eeGetMethodFullName(fncHandle), + inlineCandidateInfo->methInfo.ILCodeSize, inlineDepth, inlineResult->ReasonString()); + } - // Update block nums appropriately - // - block->bbNum += baseBBNum; - fgBBNumMax = max(block->bbNum, fgBBNumMax); + if (verbose) + { + printf("--------------------------------------------------------------------------------------------\n"); + } +#endif // DEBUG - DebugInfo di = iciStmt->GetDebugInfo().GetRoot(); - if (di.IsValid()) - { - block->bbCodeOffs = di.GetLocation().GetOffset(); - block->bbCodeOffsEnd = block->bbCodeOffs + 1; // TODO: is code size of 1 some magic number for inlining? - } - else - { - block->bbCodeOffs = 0; // TODO: why not BAD_IL_OFFSET? - block->bbCodeOffsEnd = 0; - block->SetFlags(BBF_INTERNAL); - } +#if defined(DEBUG) + impInlinedCodeSize += inlineCandidateInfo->methInfo.ILCodeSize; +#endif - if (block->KindIs(BBJ_RETURN)) - { - noway_assert(!block->HasFlag(BBF_HAS_JMP)); - JITDUMP("\nConvert bbKind of " FMT_BB " to BBJ_ALWAYS to bottom block " FMT_BB "\n", block->bbNum, - bottomBlock->bbNum); + // We inlined... + inlineResult->NoteSuccess(); +} - FlowEdge* const newEdge = fgAddRefPred(bottomBlock, block); - block->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge); - } - } +void StatementListBuilder::Append(Statement* stmt) +{ + if (m_head == nullptr) + { + m_head = m_tail = stmt; + stmt->SetPrevStmt(nullptr); + stmt->SetNextStmt(nullptr); + return; + } - // Inlinee's top block will have an artificial ref count. Remove. - assert(InlineeCompiler->fgFirstBB->bbRefs > 0); - InlineeCompiler->fgFirstBB->bbRefs--; + stmt->SetPrevStmt(m_tail); + stmt->SetNextStmt(nullptr); + m_tail->SetNextStmt(stmt); + m_tail = stmt; +} - // Insert inlinee's blocks into inliner's block list. - assert(topBlock->KindIs(BBJ_ALWAYS)); - assert(topBlock->TargetIs(bottomBlock)); - fgRedirectEdge(topBlock->TargetEdgeRef(), InlineeCompiler->fgFirstBB); +void StatementListBuilder::InsertIntoBlockAtBeginning(BasicBlock* block) +{ + if (m_head == nullptr) + { + return; + } + + Statement* lastStmt = block->lastStmt(); + if (lastStmt == nullptr) + { + lastStmt = m_tail; + } - topBlock->SetNext(InlineeCompiler->fgFirstBB); - InlineeCompiler->fgLastBB->SetNext(bottomBlock); + if (block->bbStmtList != nullptr) + { + m_tail->SetNextStmt(block->bbStmtList); + block->bbStmtList->SetPrevStmt(m_tail); + } - // - // Add inlinee's block count to inliner's. - // - fgBBcount += InlineeCompiler->fgBBcount; + block->bbStmtList = m_head; + m_head->SetPrevStmt(lastStmt); +} - // Append statements to null out gc ref locals, if necessary. - fgInlineAppendStatements(pInlineInfo, bottomBlock, nullptr); - JITDUMPEXEC(fgDispBasicBlocks(InlineeCompiler->fgFirstBB, InlineeCompiler->fgLastBB, true)); +void StatementListBuilder::InsertIntoBlockBefore(BasicBlock* block, Statement* before) +{ + assert(before != nullptr); + + if (before == block->firstStmt()) + { + InsertIntoBlockAtBeginning(block); + return; + } + + if (m_head == nullptr) + { + return; + } + + m_head->SetPrevStmt(before->GetPrevStmt()); + m_tail->SetNextStmt(before); + + m_head->GetPrevStmt()->SetNextStmt(m_head); + m_tail->GetNextStmt()->SetPrevStmt(m_tail); +} + +void Compiler::fgFinalizeInlineeStatements(InlineInfo* pInlineInfo) +{ +#ifdef DEBUG + + if (verbose) + { + printf("\n\n----------- Statements (and blocks) added due to the inlining of call "); + printTreeID(pInlineInfo->iciCall); + printf(" -----------\n"); + } + +#endif // DEBUG + + // Mark success. + pInlineInfo->inlineContext->SetSucceeded(pInlineInfo); + + // Prepend statements + fgInlinePrependStatements(pInlineInfo); + +#ifdef DEBUG + if (verbose) + { + printf("\nInlinee method body:"); } +#endif // DEBUG + + fgInlineAppendStatements(pInlineInfo->teardownStatements, pInlineInfo); // - // At this point, we have successfully inserted inlinee's code. + // At this point we have the inlinee's code in in inlinee compiler and setup/teardown statements + // in the InlineInfo instance. // // @@ -2000,6 +1897,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) } } + BasicBlock* iciBlock = pInlineInfo->iciBlock; // If we inline a no-return call at a site with profile weight, // we will introduce inconsistency. // @@ -2063,12 +1961,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) gsCookieDummy->lvIsTemp = true; // It is not alive at all, set the flag to prevent zero-init. lvaSetVarDoNotEnregister(dummy DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr)); } - - // - // Detach the GT_CALL node from the original statement by hanging a "nothing" node under it, - // so that fgMorphStmts can remove the statement once we return from here. - // - iciStmt->SetRootNode(gtNewNothingNode()); } //------------------------------------------------------------------------ @@ -2081,15 +1973,14 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) // newStmt - updated with the new statement // callDI - debug info for the call // -void Compiler::fgInsertInlineeArgument( - const InlArgInfo& argInfo, BasicBlock* block, Statement** afterStmt, Statement** newStmt, const DebugInfo& callDI) +void Compiler::fgInsertInlineeArgument(StatementListBuilder& statements, + const InlArgInfo& argInfo, + const DebugInfo& callDI) { const bool argIsSingleDef = !argInfo.argHasLdargaOp && !argInfo.argHasStargOp; CallArg* arg = argInfo.arg; GenTree* argNode = arg->GetNode(); - assert(!argNode->OperIs(GT_RET_EXPR)); - if (argInfo.argHasTmp) { noway_assert(argInfo.argIsUsed); @@ -2121,10 +2012,9 @@ void Compiler::fgInsertInlineeArgument( // We're going to assign the argument value to the temp we use for it in the inline body. GenTree* store = gtNewTempStore(argInfo.argTmpNum, argNode); - *newStmt = gtNewStmt(store, callDI); - fgInsertStmtAfter(block, *afterStmt, *newStmt); - *afterStmt = *newStmt; - DISPSTMT(*afterStmt); + Statement* newStmt = gtNewStmt(store, callDI); + statements.Append(newStmt); + DISPSTMT(newStmt); } } else if (argInfo.argIsByRefToStructLocal) @@ -2142,15 +2032,15 @@ void Compiler::fgInsertInlineeArgument( if (argInfo.argHasSideEff) { noway_assert(argInfo.argIsUsed == false); - *newStmt = nullptr; - bool append = true; + Statement* newStmt = nullptr; + bool append = true; if (argNode->OperIs(GT_BLK)) { // Don't put GT_BLK node under a GT_COMMA. // Codegen can't deal with it. // Just hang the address here in case there are side-effect. - *newStmt = gtNewStmt(gtUnusedValNode(argNode->AsOp()->gtOp1), callDI); + newStmt = gtNewStmt(gtUnusedValNode(argNode->AsOp()->gtOp1), callDI); } else { @@ -2167,7 +2057,6 @@ void Compiler::fgInsertInlineeArgument( // has no side effects, we can skip appending all // together. This will help jit TP a bit. // - assert(!argNode->OperIs(GT_RET_EXPR)); // For case (1) // @@ -2213,21 +2102,20 @@ void Compiler::fgInsertInlineeArgument( if (!append) { - assert(*newStmt == nullptr); + assert(newStmt == nullptr); JITDUMP("Arg tree side effects were discardable, not appending anything for arg\n"); } else { // If we don't have something custom to append, // just append the arg node as an unused value. - if (*newStmt == nullptr) + if (newStmt == nullptr) { - *newStmt = gtNewStmt(gtUnusedValNode(argNode), callDI); + newStmt = gtNewStmt(gtUnusedValNode(argNode), callDI); } - fgInsertStmtAfter(block, *afterStmt, *newStmt); - *afterStmt = *newStmt; - DISPSTMT(*afterStmt); + statements.Append(newStmt); + DISPSTMT(newStmt); } } else if (argNode->IsBoxedValue()) @@ -2261,14 +2149,11 @@ void Compiler::fgInsertInlineeArgument( // and are given the same inline context as the call any calls // added here will appear to have been part of the immediate caller. // -Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) +void Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) { - BasicBlock* block = inlineInfo->iciBlock; - Statement* callStmt = inlineInfo->iciStmt; - const DebugInfo& callDI = callStmt->GetDebugInfo(); - Statement* afterStmt = callStmt; // afterStmt is the place where the new statements should be inserted after. - Statement* newStmt = nullptr; - GenTreeCall* call = inlineInfo->iciCall->AsCall(); + BasicBlock* block = inlineInfo->iciBlock; + const DebugInfo& callDI = inlineInfo->iciStmt->GetDebugInfo(); + GenTreeCall* call = inlineInfo->iciCall->AsCall(); noway_assert(call->OperIs(GT_CALL)); @@ -2302,7 +2187,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) } #ifdef DEBUG - if (call->gtArgs.CountUserArgs() > 0) + if (inlineInfo->argCnt) { JITDUMP("\nArguments setup:\n"); } @@ -2314,9 +2199,11 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) InlArgInfo* argInfo = nullptr; switch (arg.GetWellKnownArg()) { - case WellKnownArg::RetBuffer: case WellKnownArg::AsyncContinuation: continue; + case WellKnownArg::RetBuffer: + argInfo = inlineInfo->inlRetBufferArgInfo; + break; case WellKnownArg::InstParam: argInfo = inlineInfo->inlInstParamArgInfo; break; @@ -2327,7 +2214,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) } assert(argInfo != nullptr); - fgInsertInlineeArgument(*argInfo, block, &afterStmt, &newStmt, callDI); + fgInsertInlineeArgument(inlineInfo->setupStatements, *argInfo, callDI); } // Add the CCTOR check if asked for. @@ -2340,18 +2227,16 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) { CORINFO_CLASS_HANDLE exactClass = eeGetClassFromContext(inlineInfo->inlineCandidateInfo->exactContextHandle); - tree = fgGetSharedCCtor(exactClass); - newStmt = gtNewStmt(tree, callDI); - fgInsertStmtAfter(block, afterStmt, newStmt); - afterStmt = newStmt; + tree = fgGetSharedCCtor(exactClass); + Statement* newStmt = gtNewStmt(tree, callDI); + inlineInfo->setupStatements.Append(newStmt); } // Insert the nullcheck statement now. if (nullcheck) { - newStmt = gtNewStmt(nullcheck, callDI); - fgInsertStmtAfter(block, afterStmt, newStmt); - afterStmt = newStmt; + Statement* newStmt = gtNewStmt(nullcheck, callDI); + inlineInfo->setupStatements.Append(newStmt); } // @@ -2399,16 +2284,13 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) tree = gtNewTempStore(tmpNum, (lclTyp == TYP_STRUCT) ? gtNewIconNode(0) : gtNewZeroConNode(lclTyp)); - newStmt = gtNewStmt(tree, callDI); - fgInsertStmtAfter(block, afterStmt, newStmt); - afterStmt = newStmt; + Statement* newStmt = gtNewStmt(tree, callDI); + inlineInfo->setupStatements.Append(newStmt); - DISPSTMT(afterStmt); + DISPSTMT(newStmt); } } } - - return afterStmt; } //------------------------------------------------------------------------ @@ -2425,7 +2307,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) // we skip nulling the locals, since it can interfere // with tail calls introduced by the local. // -void Compiler::fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* block, Statement* stmtAfter) +void Compiler::fgInlineAppendStatements(class StatementListBuilder& statements, InlineInfo* inlineInfo) { // Null out any gc ref locals if (!inlineInfo->HasGcRefLocals()) @@ -2485,9 +2367,9 @@ void Compiler::fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* bloc // Does the local we're about to null out appear in the return // expression? If so we somehow messed up and didn't properly // spill the return value. See impInlineFetchLocal. - if ((inlCandInfo->retExpr != nullptr) && (inlCandInfo->retExpr->gtSubstExpr != nullptr)) + if (inlCandInfo->result.substExpr != nullptr) { - const bool interferesWithReturn = gtHasRef(inlCandInfo->retExpr->gtSubstExpr, tmpNum); + const bool interferesWithReturn = gtHasRef(inlCandInfo->result.substExpr, tmpNum); noway_assert(!interferesWithReturn); } @@ -2495,15 +2377,7 @@ void Compiler::fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* bloc GenTree* nullExpr = gtNewTempStore(tmpNum, gtNewZeroConNode(lclTyp)); Statement* nullStmt = gtNewStmt(nullExpr, callDI); - if (stmtAfter == nullptr) - { - fgInsertStmtAtBeg(block, nullStmt); - } - else - { - fgInsertStmtAfter(block, stmtAfter, nullStmt); - } - stmtAfter = nullStmt; + statements.Append(nullStmt); #ifdef DEBUG if (verbose) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 49f57e49372bde..18e811a91d4279 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -2625,18 +2625,18 @@ PhaseStatus Compiler::fgInstrumentMethod() return PhaseStatus::MODIFIED_NOTHING; } - GenTreeRetExpr* const retExpr = impInlineInfo->inlineCandidateInfo->retExpr; + const InlineIRResult& result = impInlineInfo->inlineCandidateInfo->result; - // If there's a retExpr but no gtSubstBB, we assume the retExpr is a temp + // If there's a retExpr but no substBB, we assume the retExpr is a temp // and so not interesting to instrumentation. // - if ((retExpr != nullptr) && (retExpr->gtSubstBB != nullptr)) + if (result.substBB != nullptr) { - assert(retExpr->gtSubstExpr != nullptr); - retBB = retExpr->gtSubstBB; - tempInlineeReturnStmt = fgNewStmtAtEnd(retBB, retExpr->gtSubstExpr); - JITDUMP("Temporarily adding ret expr [%06u] to " FMT_BB "\n", dspTreeID(retExpr->gtSubstExpr), - retBB->bbNum); + assert(result.substExpr != nullptr); + retBB = result.substBB; + + tempInlineeReturnStmt = fgNewStmtAtEnd(retBB, result.substExpr); + JITDUMP("Temporarily adding ret expr [%06u] to " FMT_BB "\n", dspTreeID(result.substExpr), retBB->bbNum); } } diff --git a/src/coreclr/jit/fgstmt.cpp b/src/coreclr/jit/fgstmt.cpp index f5ab387e262416..0ec425544207b7 100644 --- a/src/coreclr/jit/fgstmt.cpp +++ b/src/coreclr/jit/fgstmt.cpp @@ -382,6 +382,64 @@ Statement* Compiler::fgInsertStmtListAfter(BasicBlock* block, Statement* stmtAft return stmtLast; } +void Compiler::fgInsertStmtListAtEnd(BasicBlock* block, Statement* stmtList) +{ + if (stmtList == nullptr) + { + return; + } + + Statement* firstStmt = block->firstStmt(); + if (firstStmt != nullptr) + { + Statement* lastStmt = firstStmt->GetPrevStmt(); + noway_assert(lastStmt != nullptr && lastStmt->GetNextStmt() == nullptr); + + // Append the statement after the last one. + Statement* stmtLast = stmtList->GetPrevStmt(); + lastStmt->SetNextStmt(stmtList); + stmtList->SetPrevStmt(lastStmt); + firstStmt->SetPrevStmt(stmtLast); + } + else + { + // The block is completely empty. + block->bbStmtList = stmtList; + } +} + +void Compiler::fgInsertStmtListBefore(BasicBlock* block, Statement* before, Statement* stmtList) +{ + if (stmtList == nullptr) + { + return; + } + + assert(block->bbStmtList != nullptr); + Statement* stmtLast = stmtList->GetPrevStmt(); + + if (before == block->bbStmtList) + { + // We're inserting before the first statement in the block. + Statement* first = block->firstStmt(); + Statement* last = block->lastStmt(); + + stmtLast->SetNextStmt(first); + stmtList->SetPrevStmt(last); + + block->bbStmtList = stmtList; + first->SetPrevStmt(stmtLast); + } + else + { + stmtLast->SetNextStmt(before); + stmtList->SetPrevStmt(before->GetPrevStmt()); + + stmtList->GetPrevStmt()->SetNextStmt(stmtList); + stmtLast->GetNextStmt()->SetPrevStmt(stmtLast); + } +} + /***************************************************************************** * * Create a new statement from tree and wire the links up. diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 0178826d918017..5f6f46c691200b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -249,7 +249,6 @@ void GenTree::InitNodeSize() GenTree::s_gtNodeSizes[GT_INDEX_ADDR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_BOUNDS_CHECK] = TREE_NODE_SZ_SMALL; GenTree::s_gtNodeSizes[GT_ARR_ELEM] = TREE_NODE_SZ_LARGE; - GenTree::s_gtNodeSizes[GT_RET_EXPR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_FIELD_ADDR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_CMPXCHG] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_QMARK] = TREE_NODE_SZ_LARGE; @@ -315,7 +314,6 @@ void GenTree::InitNodeSize() static_assert(sizeof(GenTreeStoreInd) <= TREE_NODE_SZ_SMALL); static_assert(sizeof(GenTreeAddrMode) <= TREE_NODE_SZ_SMALL); static_assert(sizeof(GenTreeBlk) <= TREE_NODE_SZ_SMALL); - static_assert(sizeof(GenTreeRetExpr) <= TREE_NODE_SZ_LARGE); // *** large node static_assert(sizeof(GenTreeILOffset) <= TREE_NODE_SZ_SMALL); static_assert(sizeof(GenTreePhiArg) <= TREE_NODE_SZ_SMALL); static_assert(sizeof(GenTreeAllocObj) <= TREE_NODE_SZ_LARGE); // *** large node @@ -726,10 +724,6 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const structHnd = AsCall()->gtRetClsHnd; break; - case GT_RET_EXPR: - structHnd = AsRetExpr()->gtInlineCandidate->gtRetClsHnd; - break; - default: unreached(); } @@ -3035,10 +3029,6 @@ GenTree** GenTree::EffectiveUse(GenTree** use) { return true; } - if (tree->OperIs(GT_RET_EXPR)) - { - return gtHasRef(tree->AsRetExpr()->gtInlineCandidate, lclNum); - } return false; } @@ -6695,7 +6685,6 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) case GT_ASYNC_CONTINUATION: case GT_LABEL: case GT_FTN_ADDR: - case GT_RET_EXPR: case GT_CNS_INT: case GT_CNS_LNG: case GT_CNS_DBL: @@ -8595,25 +8584,6 @@ GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned return node; } -GenTreeRetExpr* Compiler::gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCandidate, var_types type) -{ - assert(GenTree::s_gtNodeSizes[GT_RET_EXPR] == TREE_NODE_SZ_LARGE); - - GenTreeRetExpr* node = new (this, GT_RET_EXPR) GenTreeRetExpr(type); - - node->gtInlineCandidate = inlineCandidate; - - node->gtSubstExpr = nullptr; - node->gtSubstBB = nullptr; - - // GT_RET_EXPR node eventually might be turned back into GT_CALL (when inlining is aborted for example). - // Therefore it should carry the GTF_CALL flag so that all the rules about spilling can apply to it as well. - // For example, impImportLeave or CEE_POP need to spill GT_RET_EXPR before empty the evaluation stack. - node->gtFlags |= GTF_CALL; - - return node; -} - //------------------------------------------------------------------------ // gtNewFieldAddrNode: Create a new GT_FIELD_ADDR node. // @@ -9546,13 +9516,6 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree) copy->AsLclFld()->SetSsaNum(tree->AsLclFld()->GetSsaNum()); goto DONE; - case GT_RET_EXPR: - // GT_RET_EXPR is unique node, that contains a link to a gtInlineCandidate node, - // that is part of another statement. We cannot clone both here and cannot - // create another GT_RET_EXPR that points to the same gtInlineCandidate. - NO_WAY("Cloning of GT_RET_EXPR node not supported"); - goto DONE; - case GT_MEMORYBARRIER: copy = new (this, GT_MEMORYBARRIER) GenTree(GT_MEMORYBARRIER, TYP_VOID); goto DONE; @@ -9819,7 +9782,7 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree) // You must use gtCloneCandidateCall for these calls (and then do appropriate other fixup) if (tree->AsCall()->IsInlineCandidate() || tree->AsCall()->IsGuardedDevirtualizationCandidate()) { - NO_WAY("Cloning of calls with associated GT_RET_EXPR nodes is not supported"); + NO_WAY("Cloning of calls containing inline candidates is not supported"); } copy = gtCloneExprCallHelper(tree->AsCall()); @@ -10328,7 +10291,6 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) case GT_ASYNC_CONTINUATION: case GT_LABEL: case GT_FTN_ADDR: - case GT_RET_EXPR: case GT_CNS_INT: case GT_CNS_LNG: case GT_CNS_DBL: @@ -12419,21 +12381,6 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) case GT_GCPOLL: break; - case GT_RET_EXPR: - { - GenTreeCall* inlineCand = tree->AsRetExpr()->gtInlineCandidate; - printf("(for "); - printTreeID(inlineCand); - printf(")"); - - if (tree->AsRetExpr()->gtSubstExpr != nullptr) - { - printf(" -> "); - printTreeID(tree->AsRetExpr()->gtSubstExpr); - } - } - break; - case GT_PHYSREG: printf(" %s", getRegName(tree->AsPhysReg()->gtSrcReg)); break; @@ -15136,19 +15083,9 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions GenTree* copy = copyStmt->GetRootNode(); if (!copy->OperIs(GT_STOREIND, GT_STORE_BLK)) { - // GT_RET_EXPR is a tolerable temporary failure. - // The jit will revisit this optimization after - // inlining is done. - if (copy->OperIs(GT_RET_EXPR)) - { - JITDUMP(" bailing; must wait for replacement of copy %s\n", GenTree::OpName(copy->gtOper)); - } - else - { - // Anything else is a missed case we should figure out how to handle. - // One known case is GT_COMMAs enclosing the store we are looking for. - JITDUMP(" bailing; unexpected copy op %s\n", GenTree::OpName(copy->gtOper)); - } + // Anything else is a missed case we should figure out how to handle. + // One known case is GT_COMMAs enclosing the store we are looking for. + JITDUMP(" bailing; unexpected copy op %s\n", GenTree::OpName(copy->gtOper)); return nullptr; } @@ -15214,13 +15151,6 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions // If the copy is a struct copy, make sure we know how to isolate any source side effects. GenTree* copySrc = copy->Data(); - // If the copy source is from a pending inline, wait for it to resolve. - if (copySrc->OperIs(GT_RET_EXPR)) - { - JITDUMP(" bailing; must wait for replacement of copy source %s\n", GenTree::OpName(copySrc->gtOper)); - return nullptr; - } - bool hasSrcSideEffect = false; bool isStructCopy = false; @@ -16915,19 +16845,9 @@ bool Compiler::gtNodeHasSideEffects(GenTree* tree, GenTreeFlags flags, bool igno // Are there only GTF_CALL side effects remaining? (and no other side effect kinds) if (flags & GTF_CALL) { - GenTree* potentialCall = tree; - - while (potentialCall->OperIs(GT_RET_EXPR)) - { - // We need to preserve return expressions where the underlying call - // has side effects. Otherwise early folding can result in us dropping - // the call. - potentialCall = potentialCall->AsRetExpr()->gtInlineCandidate; - } - - if (potentialCall->OperIs(GT_CALL)) + if (tree->OperIs(GT_CALL)) { - GenTreeCall* const call = potentialCall->AsCall(); + GenTreeCall* const call = tree->AsCall(); const bool ignoreExceptions = (flags & GTF_EXCEPT) == 0; return call->HasSideEffects(this, ignoreExceptions, ignoreCctors); } @@ -17032,6 +16952,7 @@ bool Compiler::gtSplitTree(BasicBlock* block, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitNodeUse, + bool includeOperands, bool early) { class Splitter final : public GenTreeVisitor @@ -17039,6 +16960,7 @@ bool Compiler::gtSplitTree(BasicBlock* block, BasicBlock* m_bb; Statement* m_splitStmt; GenTree* m_splitNode; + bool m_includeOperands; bool m_early; struct UseInfo @@ -17056,11 +16978,13 @@ bool Compiler::gtSplitTree(BasicBlock* block, UseExecutionOrder = true }; - Splitter(Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode, bool early) + Splitter( + Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode, bool includeOperands, bool early) : GenTreeVisitor(compiler) , m_bb(bb) , m_splitStmt(stmt) , m_splitNode(splitNode) + , m_includeOperands(includeOperands) , m_early(early) , m_useStack(compiler->getAllocator(CMK_ArrayStack)) { @@ -17074,6 +16998,12 @@ bool Compiler::gtSplitTree(BasicBlock* block, { assert(!(*use)->OperIs(GT_QMARK)); m_useStack.Push(UseInfo{use, user}); + if ((*use == m_splitNode) && !m_includeOperands) + { + SplitStack(use); + return WALK_ABORT; + } + return WALK_CONTINUE; } @@ -17081,56 +17011,61 @@ bool Compiler::gtSplitTree(BasicBlock* block, { if (*use == m_splitNode) { - bool userIsReturned = false; - // Split all siblings and ancestor siblings. - int i; - for (i = 0; i < m_useStack.Height() - 1; i++) - { - const UseInfo& useInf = m_useStack.BottomRef(i); - if (useInf.Use == use) - { - break; - } + SplitStack(use); + return WALK_ABORT; + } - // If this has the same user as the next node then it is a - // sibling of an ancestor -- and thus not on the "path" - // that contains the split node. - if (m_useStack.BottomRef(i + 1).User == useInf.User) - { - SplitOutUse(useInf, userIsReturned); - } - else - { - // This is an ancestor of the node we're splitting on. - userIsReturned = IsReturned(useInf, userIsReturned); - } - } + while (m_useStack.Top(0).Use != use) + { + m_useStack.Pop(); + } - assert(m_useStack.Bottom(i).Use == use); - userIsReturned = IsReturned(m_useStack.BottomRef(i), userIsReturned); + return WALK_CONTINUE; + } - // The remaining nodes should be operands of the split node. - for (i++; i < m_useStack.Height(); i++) + private: + void SplitStack(GenTree** use) + { + bool userIsReturned = false; + // Split all siblings and ancestor siblings. + int i; + for (i = 0; i < m_useStack.Height() - 1; i++) + { + const UseInfo& useInf = m_useStack.BottomRef(i); + if (useInf.Use == use) { - const UseInfo& useInf = m_useStack.BottomRef(i); - assert(useInf.User == *use); - SplitOutUse(useInf, userIsReturned); + break; } - SplitNodeUse = use; - - return WALK_ABORT; + // If this has the same user as the next node then it is a + // sibling of an ancestor -- and thus not on the "path" + // that contains the split node. + if (m_useStack.BottomRef(i + 1).User == useInf.User) + { + SplitOutUse(useInf, userIsReturned); + } + else + { + // This is an ancestor of the node we're splitting on. + userIsReturned = IsReturned(useInf, userIsReturned); + } } - while (m_useStack.Top(0).Use != use) + assert(m_useStack.Bottom(i).Use == use); + userIsReturned = IsReturned(m_useStack.BottomRef(i), userIsReturned); + + assert(m_includeOperands || (i == m_useStack.Height() - 1)); + // The remaining nodes should be operands of the split node. + for (i++; i < m_useStack.Height(); i++) { - m_useStack.Pop(); + const UseInfo& useInf = m_useStack.BottomRef(i); + assert(useInf.User == *use); + SplitOutUse(useInf, userIsReturned); } - return WALK_CONTINUE; + SplitNodeUse = use; } - private: bool IsReturned(const UseInfo& useInf, bool userIsReturned) { if (useInf.User != nullptr) @@ -17203,26 +17138,29 @@ bool Compiler::gtSplitTree(BasicBlock* block, return; } - if ((*use)->OperIs(GT_LCL_VAR) && !m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->IsAddressExposed() && - !(m_early && m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->lvHasLdAddrOp)) + if ((*use)->OperIs(GT_LCL_VAR)) { - // The splitting we do here should always guarantee that we - // only introduce locals for the tree edges that overlap the - // split point, so it should be ok to avoid creating statements - // for locals that aren't address exposed. Note that this - // relies on it being illegal IR to have a tree edge for a - // register candidate that overlaps with an interfering node. - // - // For example, this optimization would be problematic if IR - // like the following could occur: - // - // CALL - // LCL_VAR V00 - // CALL - // STORE_LCL_VAR(...) (setup) - // LCL_VAR V00 - // - return; + LclVarDsc* dsc = m_compiler->lvaGetDesc((*use)->AsLclVarCommon()); + if (!dsc->IsAddressExposed() && (!m_early || !dsc->lvHasLdAddrOp)) + { + // The splitting we do here should always guarantee that we + // only introduce locals for the tree edges that overlap the + // split point, so it should be ok to avoid creating statements + // for locals that aren't address exposed. Note that this + // relies on it being illegal IR to have a tree edge for a + // register candidate that overlaps with an interfering node. + // + // For example, this optimization would be problematic if IR + // like the following could occur: + // + // CALL + // LCL_VAR V00 + // CALL + // STORE_LCL_VAR(...) (setup) + // LCL_VAR V00 + // + return; + } } #ifndef TARGET_64BIT @@ -17299,7 +17237,7 @@ bool Compiler::gtSplitTree(BasicBlock* block, } }; - Splitter splitter(this, block, stmt, splitPoint, early); + Splitter splitter(this, block, stmt, splitPoint, includeOperands, early); splitter.WalkTree(stmt->GetRootNodePointer(), nullptr); *firstNewStmt = splitter.FirstStatement; *splitNodeUse = splitter.SplitNodeUse; @@ -18935,15 +18873,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b break; } - case GT_RET_EXPR: - { - // If we see a RET_EXPR, recurse through to examine the - // return value expression. - GenTree* retExpr = obj->AsRetExpr()->gtInlineCandidate; - objClass = gtGetClassHandle(retExpr, pIsExact, pIsNonNull); - break; - } - case GT_CALL: { GenTreeCall* call = obj->AsCall(); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 100f17301ea267..4e0784abb09e7f 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8232,35 +8232,6 @@ struct GenTreeCmpXchg : public GenTreeIndir } }; -/* gtRetExp -- Place holder for the return expression from an inline candidate (GT_RET_EXPR) */ - -struct GenTreeRetExpr : public GenTree -{ - GenTreeCall* gtInlineCandidate; - - // Expression representing gtInlineCandidate's value (e.g. spill temp or - // expression from inlinee, or call itself for unsuccessful inlines). - // Substituted by UpdateInlineReturnExpressionPlaceHolder. - // This tree is nullptr during the import that created the GenTreeRetExpr and is set later - // when handling the actual inline candidate. - GenTree* gtSubstExpr; - - // The basic block that gtSubstExpr comes from, to enable propagating mandatory flags. - // nullptr for cases where gtSubstExpr is not a tree from the inlinee. - BasicBlock* gtSubstBB; - - GenTreeRetExpr(var_types type) - : GenTree(GT_RET_EXPR, type) - { - } -#if DEBUGGABLE_GENTREE - GenTreeRetExpr() - : GenTree() - { - } -#endif -}; - // In LIR there are no longer statements so debug information is inserted linearly using these nodes. struct GenTreeILOffset : public GenTree { diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 93fb4dfd358b37..92a12259635446 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -37,7 +37,6 @@ GTNODE(ASYNC_CONTINUATION, GenTree ,0,0,GTK_LEAF) // Access GTNODE(LABEL , GenTree ,0,0,GTK_LEAF) // Jump-target GTNODE(JMP , GenTreeVal ,0,0,GTK_LEAF|GTK_NOVALUE) // Jump to another function GTNODE(FTN_ADDR , GenTreeFptrVal ,0,0,GTK_LEAF) // Address of a function -GTNODE(RET_EXPR , GenTreeRetExpr ,0,0,GTK_LEAF|DBK_NOTLIR) // Place holder for the return expression from an inline candidate GTNODE(GCPOLL , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTLIR) //----------------------------------------------------------------------------- diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index b785279801c778..1e69b8d3bd4d8f 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -86,7 +86,6 @@ GTSTRUCT_3_SPECIAL(ArrCommon , GT_ARR_LENGTH, GT_MDARR_LENGTH, GT_MDARR_LOWER_BO GTSTRUCT_1(ArrLen , GT_ARR_LENGTH) GTSTRUCT_2(MDArr , GT_MDARR_LENGTH, GT_MDARR_LOWER_BOUND) GTSTRUCT_1(ArrElem , GT_ARR_ELEM) -GTSTRUCT_1(RetExpr , GT_RET_EXPR) GTSTRUCT_1(ILOffset , GT_IL_OFFSET) GTSTRUCT_2(CopyOrReload, GT_COPY, GT_RELOAD) GTSTRUCT_1(AddrMode , GT_LEA) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 3bd72cb08e609a..5af344825173bd 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -385,22 +385,22 @@ void Compiler::impAppendStmt(Statement* stmt, unsigned chkLevel, bool checkConsu { dstVarDsc = lvaGetDesc(expr->AsLclVarCommon()); } - else if (expr->OperIs(GT_CALL, GT_RET_EXPR)) // The special case of calls with return buffers. + else if (expr->OperIs(GT_CALL)) // The special case of calls with return buffers. { - GenTree* call = expr->OperIs(GT_RET_EXPR) ? expr->AsRetExpr()->gtInlineCandidate : expr; + GenTreeCall* call = expr->AsCall(); - if (call->TypeIs(TYP_VOID) && call->AsCall()->ShouldHaveRetBufArg()) + if (call->TypeIs(TYP_VOID) && call->ShouldHaveRetBufArg()) { GenTree* retBuf; - if (call->AsCall()->ShouldHaveRetBufArg()) + if (call->ShouldHaveRetBufArg()) { - assert(call->AsCall()->gtArgs.HasRetBuffer()); - retBuf = call->AsCall()->gtArgs.GetRetBufferArg()->GetNode(); + assert(call->gtArgs.HasRetBuffer()); + retBuf = call->gtArgs.GetRetBufferArg()->GetNode(); } else { - assert(!call->AsCall()->gtArgs.HasThisPointer()); - retBuf = call->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); + assert(!call->gtArgs.HasThisPointer()); + retBuf = call->gtArgs.GetArgByIndex(0)->GetNode(); } assert(retBuf->TypeIs(TYP_I_IMPL, TYP_BYREF)); @@ -444,40 +444,6 @@ void Compiler::impAppendStmt(Statement* stmt, unsigned chkLevel, bool checkConsu { impSpillSpecialSideEff(); } - - if ((lastStmt != impLastStmt) && expr->OperIs(GT_RET_EXPR)) - { - GenTree* const call = expr->AsRetExpr()->gtInlineCandidate; - JITDUMP("\nimpAppendStmt: after sinking a local struct store into inline candidate [%06u], we need to " - "reorder subsequent spills.\n", - dspTreeID(call)); - - // Move all newly appended statements to just before the call's statement. - // First, find the statement containing the call. - // - Statement* insertBeforeStmt = lastStmt; - - while (insertBeforeStmt->GetRootNode() != call) - { - assert(insertBeforeStmt != impStmtList); - insertBeforeStmt = insertBeforeStmt->GetPrevStmt(); - } - - Statement* movingStmt = lastStmt->GetNextStmt(); - - JITDUMP("Moving " FMT_STMT " through " FMT_STMT " before " FMT_STMT "\n", movingStmt->GetID(), - impLastStmt->GetID(), insertBeforeStmt->GetID()); - - // We move these backwards, so must keep moving the insert - // point to keep them in order. - // - while (impLastStmt != lastStmt) - { - Statement* movingStmt = impExtractLastStmt(); - impInsertStmtBefore(movingStmt, insertBeforeStmt); - insertBeforeStmt = movingStmt; - } - } } impAppendStmtCheck(stmt, chkLevel); @@ -824,7 +790,7 @@ GenTree* Compiler::impStoreStruct(GenTree* store, NewCallArg newArg = NewCallArg::Primitive(destAddr).WellKnown(wellKnownArgType); - if (destAddr->OperIs(GT_LCL_ADDR)) + if (destAddr->OperIs(GT_LCL_ADDR) && !srcCall->IsInlineCandidate()) { lvaSetVarDoNotEnregister(destAddr->AsLclVarCommon()->GetLclNum() DEBUGARG(DoNotEnregisterReason::HiddenBufferStructArg)); @@ -921,48 +887,6 @@ GenTree* Compiler::impStoreStruct(GenTree* store, } #endif // UNIX_AMD64_ABI } - else if (src->OperIs(GT_RET_EXPR)) - { - assert(src->AsRetExpr()->gtInlineCandidate->OperIs(GT_CALL)); - GenTreeCall* call = src->AsRetExpr()->gtInlineCandidate; - - if (call->ShouldHaveRetBufArg()) - { - // insert the return value buffer into the argument list as first byref parameter after 'this' - // TODO-Bug?: verify if flags matter here - GenTreeFlags indirFlags = GTF_EMPTY; - GenTree* destAddr = impGetNodeAddr(store, CHECK_SPILL_ALL, &indirFlags); - - if (!impIsLegalRetBuf(destAddr, call)) - { - unsigned tmp = lvaGrabTemp(false DEBUGARG("stack copy for value returned via return buffer")); - lvaSetStruct(tmp, call->gtRetClsHnd, false); - destAddr = gtNewLclVarAddrNode(tmp, TYP_I_IMPL); - - // Insert address of temp into existing call - NewCallArg retBufArg = NewCallArg::Primitive(destAddr).WellKnown(WellKnownArg::RetBuffer); - call->gtArgs.InsertAfterThisOrFirst(this, retBufArg); - - // Now the store needs to copy from the new temp instead. - call->gtType = TYP_VOID; - src->gtType = TYP_VOID; - var_types tmpType = lvaGetDesc(tmp)->TypeGet(); - store->Data() = gtNewOperNode(GT_COMMA, tmpType, src, gtNewLclvNode(tmp, tmpType)); - return impStoreStruct(store, CHECK_SPILL_ALL, pAfterStmt, di, block); - } - - call->gtArgs.InsertAfterThisOrFirst(this, - NewCallArg::Primitive(destAddr).WellKnown(WellKnownArg::RetBuffer)); - - // now returns void, not a struct - src->gtType = TYP_VOID; - call->gtType = TYP_VOID; - - // We already have appended the write to 'dest' GT_CALL's args - // So now we just return an empty node (pruning the GT_RET_EXPR) - return src; - } - } else if (src->OperIs(GT_COMMA)) { GenTree* sideEffectAddressStore = nullptr; @@ -1225,7 +1149,6 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, unsigned curLevel) switch (structVal->OperGet()) { case GT_CALL: - case GT_RET_EXPR: { unsigned lclNum = lvaGrabTemp(true DEBUGARG("spilled call-like call argument")); impStoreToTemp(lclNum, structVal, curLevel); @@ -1820,25 +1743,6 @@ bool Compiler::impSpillStackEntry(unsigned level, CORINFO_CLASS_HANDLE stkHnd = stackState.esStack[level].seTypeInfo.GetClassHandleForObjRef(); lvaSetClass(tnum, tree, stkHnd); } - - // If we're assigning a GT_RET_EXPR, note the temp over on the call, - // so the inliner can use it in case it needs a return spill temp. - if (tree->OperIs(GT_RET_EXPR)) - { - JITDUMP("\n*** see V%02u = GT_RET_EXPR, noting temp\n", tnum); - GenTreeCall* call = tree->AsRetExpr()->gtInlineCandidate->AsCall(); - if (call->IsGuardedDevirtualizationCandidate()) - { - for (uint8_t i = 0; i < call->GetInlineCandidatesCount(); i++) - { - call->GetGDVCandidateInfo(i)->preexistingSpillTemp = tnum; - } - } - else - { - call->AsCall()->GetSingleInlineCandidateInfo()->preexistingSpillTemp = tnum; - } - } } // The tree type may be modified by impStoreToTemp, so use the type of the lclVar. @@ -3493,90 +3397,6 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken) GenTree* allocBoxStore = gtNewTempStore(impBoxTemp, op1); Statement* allocBoxStmt = impAppendTree(allocBoxStore, CHECK_SPILL_NONE, impCurStmtDI); - // If the exprToBox is a call that returns its value via a ret buf arg, - // move the store statement(s) before the call (which must be a top level tree). - // - // We do this because impStoreStructPtr (invoked below) will - // back-substitute into a call when it sees a GT_RET_EXPR and the call - // has a hidden buffer pointer, So we need to reorder things to avoid - // creating out-of-sequence IR. - // - if (varTypeIsStruct(exprToBox) && exprToBox->OperIs(GT_RET_EXPR)) - { - GenTreeCall* const call = exprToBox->AsRetExpr()->gtInlineCandidate->AsCall(); - - // If the call was flagged for possible enumerator cloning, flag the allocation as well. - // - if (compIsForInlining() && hasImpEnumeratorGdvLocalMap()) - { - NodeToUnsignedMap* const map = getImpEnumeratorGdvLocalMap(); - unsigned enumeratorLcl = BAD_VAR_NUM; - GenTreeCall* const call = impInlineInfo->iciCall; - if (map->Lookup(call, &enumeratorLcl)) - { - JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", dspTreeID(op1), enumeratorLcl); - map->Remove(call); - map->Set(op1, enumeratorLcl); - } - } - - if (call->ShouldHaveRetBufArg()) - { - JITDUMP("Must insert newobj stmts for box before call [%06u]\n", dspTreeID(call)); - - // Walk back through the statements in this block, looking for the one - // that has this call as the root node. - // - // Because gtNewTempStore (above) may have added statements that - // feed into the actual store we need to move this set of added - // statements as a group. - // - // Note boxed allocations are side-effect free (no com or finalizer) so - // our only worries here are (correctness) not overlapping the box temp - // lifetime and (perf) stretching the temp lifetime across the inlinee - // body. - // - // Since this is an inline candidate, we must be optimizing, and so we have - // a unique box temp per call. So no worries about overlap. - // - assert(!opts.OptimizationDisabled()); - - // Lifetime stretching could addressed with some extra cleverness--sinking - // the allocation back down to just before the copy, once we figure out - // where the copy is. We defer for now. - // - Statement* insertBeforeStmt = cursor; - noway_assert(insertBeforeStmt != nullptr); - - while (true) - { - if (insertBeforeStmt->GetRootNode() == call) - { - break; - } - - // If we've searched all the statements in the block and failed to - // find the call, then something's wrong. - // - noway_assert(insertBeforeStmt != impStmtList); - - insertBeforeStmt = insertBeforeStmt->GetPrevStmt(); - } - - // Found the call. Move the statements comprising the store. - // - JITDUMP("Moving " FMT_STMT "..." FMT_STMT " before " FMT_STMT "\n", cursor->GetNextStmt()->GetID(), - allocBoxStmt->GetID(), insertBeforeStmt->GetID()); - assert(allocBoxStmt == impLastStmt); - do - { - Statement* movingStmt = impExtractLastStmt(); - impInsertStmtBefore(movingStmt, insertBeforeStmt); - insertBeforeStmt = movingStmt; - } while (impLastStmt != cursor); - } - } - // Create a pointer to the box payload in op1. // op1 = gtNewLclvNode(impBoxTemp, TYP_REF); @@ -6985,12 +6805,12 @@ void Compiler::impImportBlockCode(BasicBlock* block) // If we see a local being assigned the result of a GDV-inlineable // GetEnumerator call, keep track of both the local and the call. // - if (op1->OperIs(GT_RET_EXPR)) + if (op1->IsCall()) { JITDUMP(".... checking for GDV returning IEnumerator...\n"); bool isEnumeratorT = false; - GenTreeCall* const call = op1->AsRetExpr()->gtInlineCandidate; + GenTreeCall* const call = op1->AsCall(); bool isExact = false; bool isNonNull = false; CORINFO_CLASS_HANDLE retCls = gtGetClassHandle(call, &isExact, &isNonNull); @@ -11334,7 +11154,6 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif InlineCandidateInfo* inlCandInfo = impInlineInfo->inlineCandidateInfo; - GenTreeRetExpr* inlRetExpr = inlCandInfo->retExpr; // Make sure the type matches the original call. var_types returnType = genActualType(op2->gtType); @@ -11387,12 +11206,8 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) { // Do we have to normalize? var_types fncRealRetType = JITtype2varType(info.compMethodInfo->args.retType); - // For RET_EXPR get the type info from the call. Regardless - // of whether it ends up inlined or not normalization will - // happen as part of that function's codegen. - GenTree* returnedTree = op2->OperIs(GT_RET_EXPR) ? op2->AsRetExpr()->gtInlineCandidate : op2; - if ((varTypeIsSmall(returnedTree->TypeGet()) || varTypeIsSmall(fncRealRetType)) && - fgCastNeeded(returnedTree, fncRealRetType)) + if ((varTypeIsSmall(op2->TypeGet()) || varTypeIsSmall(fncRealRetType)) && + fgCastNeeded(op2, fncRealRetType)) { // Small-typed return values are normalized by the callee op2 = gtNewCastNode(TYP_INT, op2, false, fncRealRetType); @@ -11411,7 +11226,7 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) bool isNonNull = false; CORINFO_CLASS_HANDLE returnClsHnd = gtGetClassHandle(op2, &isExact, &isNonNull); - if (inlRetExpr->gtSubstExpr == nullptr) + if (inlCandInfo->result.substExpr == nullptr) { // This is the first return, so best known type is the type // of this return value. @@ -11443,12 +11258,12 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) op2 = tmpOp2; #ifdef DEBUG - if (inlRetExpr->gtSubstExpr != nullptr) + if (inlCandInfo->result.substExpr != nullptr) { // Some other block(s) have seen the CEE_RET first. // Better they spilled to the same temp. - assert(inlRetExpr->gtSubstExpr->OperIs(GT_LCL_VAR)); - assert(inlRetExpr->gtSubstExpr->AsLclVarCommon()->GetLclNum() == + assert(inlCandInfo->result.substExpr->OperIs(GT_LCL_VAR)); + assert(inlCandInfo->result.substExpr->AsLclVarCommon()->GetLclNum() == op2->AsLclVarCommon()->GetLclNum()); } #endif @@ -11463,7 +11278,7 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif // Report the return expression - inlRetExpr->gtSubstExpr = op2; + inlCandInfo->result.substExpr = op2; } else { @@ -11489,43 +11304,45 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) if (fgNeedReturnSpillTemp()) { - if (inlRetExpr->gtSubstExpr == nullptr) + if (inlCandInfo->result.substExpr == nullptr) { // The inlinee compiler has figured out the type of the temp already. Use it here. - inlRetExpr->gtSubstExpr = + inlCandInfo->result.substExpr = gtNewLclvNode(lvaInlineeReturnSpillTemp, lvaTable[lvaInlineeReturnSpillTemp].lvType); } } else { - inlRetExpr->gtSubstExpr = op2; + inlCandInfo->result.substExpr = op2; } } else // The struct was to be returned via a return buffer. { - assert(iciCall->gtArgs.HasRetBuffer()); - GenTree* dest = gtCloneExpr(iciCall->gtArgs.GetRetBufferArg()->GetEarlyNode()); + assert(iciCall->gtArgs.HasRetBuffer() && (impInlineInfo->inlRetBufferArgInfo != nullptr)); + InlLclVarInfo lclInfo = {}; + lclInfo.lclTypeInfo = TYP_BYREF; + GenTree* dest = impInlineFetchArg(*impInlineInfo->inlRetBufferArgInfo, lclInfo); if (fgNeedReturnSpillTemp()) { // If this is the first return we have seen set the retExpr. - if (inlRetExpr->gtSubstExpr == nullptr) + if (inlCandInfo->result.substExpr == nullptr) { - inlRetExpr->gtSubstExpr = + inlCandInfo->result.substExpr = impStoreStructPtr(dest, gtNewLclvNode(lvaInlineeReturnSpillTemp, info.compRetType), CHECK_SPILL_ALL); } } else { - inlRetExpr->gtSubstExpr = impStoreStructPtr(dest, op2, CHECK_SPILL_ALL); + inlCandInfo->result.substExpr = impStoreStructPtr(dest, op2, CHECK_SPILL_ALL); } } } // If gtSubstExpr is an arbitrary tree then we may need to // propagate mandatory "IR presence" flags to the BB it ends up in. - inlRetExpr->gtSubstBB = fgNeedReturnSpillTemp() ? nullptr : compCurBB; + inlCandInfo->result.substBB = fgNeedReturnSpillTemp() ? nullptr : compCurBB; } } @@ -13312,9 +13129,8 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, // impInlineRecordArgInfo: record information about an inline candidate argument // // Arguments: -// pInlineInfo - inline info for the inline candidate +// argInfo - arg info // arg - the caller argument -// argInfo - Structure to record information into // inlineResult - result of ongoing inline evaluation // // Notes: @@ -13325,15 +13141,13 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, // pass the argument into the inlinee. void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, - CallArg* arg, InlArgInfo* argInfo, + CallArg* arg, InlineResult* inlineResult) { argInfo->arg = arg; GenTree* curArgVal = arg->GetNode(); - assert(!curArgVal->OperIs(GT_RET_EXPR)); - GenTree* lclVarTree; const bool isAddressInLocal = impIsAddressInLocal(curArgVal, &lclVarTree); if (isAddressInLocal) @@ -13351,8 +13165,10 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, #endif // FEATURE_SIMD } - // Spilling code relies on correct aliasability annotations. - assert(varDsc->lvHasLdAddrOp || varDsc->IsAddressExposed()); + // Spilling code relies on correct aliasability annotations, except for + // retbufs that are not actually exposed in IL. + assert(varDsc->lvHasLdAddrOp || varDsc->IsAddressExposed() || + (arg->GetWellKnownArg() == WellKnownArg::RetBuffer)); } if (curArgVal->gtFlags & GTF_ALL_EFFECT) @@ -13371,7 +13187,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, if (impIsInvariant(curArgVal)) { argInfo->argIsInvariant = true; - if (argInfo->argIsThis && curArgVal->OperIs(GT_CNS_INT) && (curArgVal->AsIntCon()->gtIconVal == 0)) + if (argInfo->argIsThis && (curArgVal->gtOper == GT_CNS_INT) && (curArgVal->AsIntCon()->gtIconVal == 0)) { // Abort inlining at this call site inlineResult->NoteFatal(InlineObservation::CALLSITE_ARG_HAS_NULL_THIS); @@ -13411,7 +13227,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, } else { - printf("IL argument #%u:", pInlineInfo->iciCall->gtArgs.GetUserIndex(arg)); + printf("Argument #%u:", pInlineInfo->iciCall->gtArgs.GetIndex(arg)); } if (argInfo->argIsLclVar) { @@ -13501,6 +13317,12 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) switch (arg.GetWellKnownArg()) { case WellKnownArg::RetBuffer: + { + InlArgInfo* retBufferInfo = new (this, CMK_Inlining) InlArgInfo{}; + impInlineRecordArgInfo(pInlineInfo, retBufferInfo, &arg, inlineResult); + pInlineInfo->inlRetBufferArgInfo = retBufferInfo; + continue; + } case WellKnownArg::AsyncContinuation: // These do not appear in the table of inline arg info; do not include them continue; @@ -13513,7 +13335,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) } arg.SetEarlyNode(gtFoldExpr(arg.GetEarlyNode())); - impInlineRecordArgInfo(pInlineInfo, &arg, argInfo, inlineResult); + impInlineRecordArgInfo(pInlineInfo, argInfo, &arg, inlineResult); if (inlineResult->IsFailure()) { @@ -13910,7 +13732,6 @@ GenTree* Compiler::impInlineFetchArg(InlArgInfo& argInfo, const InlLclVarInfo& l GenTree* op1 = nullptr; GenTree* argNode = argInfo.arg->GetNode(); - assert(!argNode->OperIs(GT_RET_EXPR)); // For TYP_REF args, if the argNode doesn't have any class information // we will lose some type info if we directly substitute it. diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index beca2374c6713a..c2c3c8cb334ce4 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1408,54 +1408,20 @@ var_types Compiler::impImportCall(OPCODE opcode, assert(opts.OptEnabled(CLFLG_INLINING)); assert(!isFatPointerCandidate); // We should not try to inline calli. - // Make the call its own tree (spill the stack if needed). - // Do not consume the debug info here. This is particularly - // important if we give up on the inline, in which case the - // call will typically end up in the statement that contains - // the GT_RET_EXPR that we leave on the stack. - impAppendTree(call, CHECK_SPILL_ALL, impCurStmtDI, false); - - // TODO: Still using the widened type. - GenTreeRetExpr* retExpr = gtNewInlineCandidateReturnExpr(call->AsCall(), genActualType(callRetTyp)); - - // Link the retExpr to the call so if necessary we can manipulate it later. - if (origCall->IsGuardedDevirtualizationCandidate()) - { - for (uint8_t i = 0; i < origCall->GetInlineCandidatesCount(); i++) - { - origCall->GetGDVCandidateInfo(i)->retExpr = retExpr; - } - } - else + if (isGuardedDevirtualizationCandidate) { - origCall->GetSingleInlineCandidateInfo()->retExpr = retExpr; - } - - // Propagate retExpr as the placeholder for the call. - call = retExpr; - - if (origCall->IsAsyncAndAlwaysSavesAndRestoresExecutionContext()) - { - // Async calls that require save/restore of - // ExecutionContext need to be top most so that we can - // insert try-finally around them. We can inline these, so - // we need to ensure that the RET_EXPR is findable when we - // later expand this. - - unsigned resultLcl = lvaGrabTemp(true DEBUGARG("async")); - LclVarDsc* varDsc = lvaGetDesc(resultLcl); + unsigned lclNum = lvaGrabTemp(false DEBUGARG("GDV candidate result")); + LclVarDsc* varDsc = lvaGetDesc(lclNum); // Keep the information about small typedness to avoid // inserting unnecessary casts around normalization. - if (varTypeIsSmall(origCall->gtReturnType)) + if (call->IsCall() && varTypeIsSmall(call->AsCall()->gtReturnType)) { - assert(origCall->NormalizesSmallTypesOnReturn()); - varDsc->lvType = origCall->gtReturnType; + assert(call->AsCall()->NormalizesSmallTypesOnReturn()); + varDsc->lvType = call->AsCall()->gtReturnType; } - impStoreToTemp(resultLcl, call, CHECK_SPILL_ALL); - // impStoreToTemp can change src arg list and return type for call that returns struct. - var_types type = genActualType(lvaGetDesc(resultLcl)->TypeGet()); - call = gtNewLclvNode(resultLcl, type); + impStoreToTemp(lclNum, call, CHECK_SPILL_ALL, nullptr, impCurStmtDI); + call = gtNewLclvNode(lclNum, genActualType(callRetTyp)); } } else @@ -3876,11 +3842,11 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, op1 = op1->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); retNode = op1; } - else if (op1->OperIs(GT_CALL, GT_RET_EXPR)) + else if (op1->OperIs(GT_CALL)) { // Skip roundtrip "handle -> RuntimeType -> handle" for // RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) - GenTreeCall* call = op1->IsCall() ? op1->AsCall() : op1->AsRetExpr()->gtInlineCandidate; + GenTreeCall* call = op1->AsCall(); if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_RuntimeType_get_TypeHandle) { // Check that the arg is CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE helper call @@ -3888,11 +3854,6 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, if (arg->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(arg->AsCall())) { impPopStack(); - if (op1->OperIs(GT_RET_EXPR)) - { - // Bash the RET_EXPR's call to no-op since it's unused now - op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP(); - } // Skip roundtrip and return the type handle directly retNode = arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); } @@ -4128,9 +4089,9 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, case NI_System_Threading_Thread_get_ManagedThreadId: { - if (impStackTop().val->OperIs(GT_RET_EXPR)) + if (impStackTop().val->IsCall()) { - GenTreeCall* call = impStackTop().val->AsRetExpr()->gtInlineCandidate->AsCall(); + GenTreeCall* call = impStackTop().val->AsCall(); if (call->IsSpecialIntrinsic()) { if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_Threading_Thread_get_CurrentThread) @@ -6167,7 +6128,7 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - result = gtNewIconNode(BitOperations::RotateLeft(cns1, cns2), baseType); + result = gtNewIconNode((int32_t)BitOperations::RotateLeft(cns1, cns2), baseType); } break; } @@ -6216,7 +6177,7 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - result = gtNewIconNode(BitOperations::RotateRight(cns1, cns2), baseType); + result = gtNewIconNode((int32_t)BitOperations::RotateRight(cns1, cns2), baseType); } break; } @@ -6909,78 +6870,9 @@ void Compiler::impInsertAsyncContinuationForLdvirtftnCall(GenTreeCall* call) } } -//------------------------------------------------------------------------ -// SpillRetExprHelper: iterate through arguments tree and spill ret_expr to local variables. -// -class SpillRetExprHelper -{ -public: - SpillRetExprHelper(Compiler* comp) - : comp(comp) - { - } - - void StoreRetExprResultsInArgs(GenTreeCall* call) - { - for (CallArg& arg : call->gtArgs.Args()) - { - comp->fgWalkTreePre(&arg.EarlyNodeRef(), SpillRetExprVisitor, this); - } - } - -private: - static Compiler::fgWalkResult SpillRetExprVisitor(GenTree** pTree, Compiler::fgWalkData* fgWalkPre) - { - assert((pTree != nullptr) && (*pTree != nullptr)); - GenTree* tree = *pTree; - if ((tree->gtFlags & GTF_CALL) == 0) - { - // Trees with ret_expr are marked as GTF_CALL. - return Compiler::WALK_SKIP_SUBTREES; - } - if (tree->OperIs(GT_RET_EXPR)) - { - SpillRetExprHelper* walker = static_cast(fgWalkPre->pCallbackData); - walker->StoreRetExprAsLocalVar(pTree); - } - return Compiler::WALK_CONTINUE; - } - - void StoreRetExprAsLocalVar(GenTree** pRetExpr) - { - GenTree* retExpr = *pRetExpr; - assert(retExpr->OperIs(GT_RET_EXPR)); - const unsigned tmp = comp->lvaGrabTemp(true DEBUGARG("spilling ret_expr")); - JITDUMP("Storing return expression [%06u] to a local var V%02u.\n", comp->dspTreeID(retExpr), tmp); - comp->impStoreToTemp(tmp, retExpr, Compiler::CHECK_SPILL_NONE); - *pRetExpr = comp->gtNewLclvNode(tmp, retExpr->TypeGet()); - - assert(comp->lvaTable[tmp].lvSingleDef == 0); - comp->lvaTable[tmp].lvSingleDef = 1; - JITDUMP("Marked V%02u as a single def temp\n", tmp); - if (retExpr->TypeIs(TYP_REF)) - { - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE retClsHnd = comp->gtGetClassHandle(retExpr, &isExact, &isNonNull); - if (retClsHnd != nullptr) - { - comp->lvaSetClass(tmp, retClsHnd, isExact); - } - else - { - JITDUMP("Could not deduce class from [%06u]", comp->dspTreeID(retExpr)); - } - } - } - -private: - Compiler* comp; -}; - //------------------------------------------------------------------------ // addFatPointerCandidate: mark the call and the method, that they have a fat pointer candidate. -// Spill ret_expr in the call node, because they can't be cloned. +// Spill inline candidates in the call node, because they shouldn't be cloned. // // Arguments: // call - fat calli candidate @@ -6990,8 +6882,6 @@ void Compiler::addFatPointerCandidate(GenTreeCall* call) JITDUMP("Marking call [%06u] as fat pointer candidate\n", dspTreeID(call)); setMethodHasFatPointer(); call->SetFatPointerCandidate(); - SpillRetExprHelper helper(this); - helper.StoreRetExprResultsInArgs(call); } //------------------------------------------------------------------------ @@ -7851,11 +7741,6 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, classHandle != NO_CLASS_HANDLE ? eeGetClassName(classHandle) : eeGetMethodFullName(methodHandle)); setMethodHasGuardedDevirtualization(); - // Spill off any GT_RET_EXPR subtrees so we can clone the call. - // - SpillRetExprHelper helper(this); - helper.StoreRetExprResultsInArgs(call); - // Gather some information for later. Note we actually allocate InlineCandidateInfo // here, as the devirtualized half of this call will likely become an inline candidate. // @@ -9805,7 +9690,8 @@ void Compiler::impCheckCanInline(GenTreeCall* call, pInfo->ilCallerHandle = pParam->pThis->info.compMethodHnd; pInfo->clsHandle = clsHandle; pInfo->exactContextHandle = pParam->exactContextHnd; - pInfo->retExpr = nullptr; + pInfo->result.substExpr = nullptr; + pInfo->result.substBB = nullptr; pInfo->preexistingSpillTemp = BAD_VAR_NUM; pInfo->clsAttr = clsAttr; pInfo->methAttr = pParam->methAttr; diff --git a/src/coreclr/jit/importervectorization.cpp b/src/coreclr/jit/importervectorization.cpp index 02adf29fcf4beb..8f4368c2eb9ac8 100644 --- a/src/coreclr/jit/importervectorization.cpp +++ b/src/coreclr/jit/importervectorization.cpp @@ -336,20 +336,7 @@ GenTree* Compiler::impExpandHalfConstEquals(GenTreeLclVarCommon* data, // GenTreeStrCon* Compiler::impGetStrConFromSpan(GenTree* span) { - GenTreeCall* argCall = nullptr; - if (span->OperIs(GT_RET_EXPR)) - { - // NOTE: we don't support chains of RET_EXPR here - GenTree* inlineCandidate = span->AsRetExpr()->gtInlineCandidate; - if (inlineCandidate->OperIs(GT_CALL)) - { - argCall = inlineCandidate->AsCall(); - } - } - else if (span->OperIs(GT_CALL)) - { - argCall = span->AsCall(); - } + GenTreeCall* argCall = span->IsCall() ? span->AsCall() : nullptr; if ((argCall != nullptr) && argCall->IsSpecialIntrinsic()) { @@ -718,20 +705,6 @@ GenTree* Compiler::impUtf16SpanComparison(StringComparisonKind kind, CORINFO_SIG { impPopStack(); } - - // We have to clean up GT_RET_EXPR for String.op_Implicit or MemoryExtensions.AsSpans - if ((spanObj != op1) && op1->OperIs(GT_RET_EXPR)) - { - GenTree* inlineCandidate = op1->AsRetExpr()->gtInlineCandidate; - assert(inlineCandidate->IsCall()); - inlineCandidate->gtBashToNOP(); - } - else if ((spanObj != op2) && op2->OperIs(GT_RET_EXPR)) - { - GenTree* inlineCandidate = op2->AsRetExpr()->gtInlineCandidate; - assert(inlineCandidate->IsCall()); - inlineCandidate->gtBashToNOP(); - } } return unrolled; } diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index fd4ee8c5777908..8436019055bf38 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -151,6 +151,11 @@ class IndirectCallTransformer bool ContainsGuardedDevirtualizationCandidate(Statement* stmt) { GenTree* candidate = stmt->GetRootNode(); + if (candidate->OperIs(GT_STORE_LCL_VAR)) + { + candidate = candidate->AsLclVar()->Data(); + } + return candidate->IsCall() && candidate->AsCall()->IsGuardedDevirtualizationCandidate(); } @@ -162,12 +167,13 @@ class IndirectCallTransformer , currBlock(block) , stmt(stmt) { - remainderBlock = nullptr; - checkBlock = nullptr; - thenBlock = nullptr; - elseBlock = nullptr; - origCall = nullptr; - likelihood = HIGH_PROBABILITY; + remainderBlock = nullptr; + checkBlock = nullptr; + thenBlock = nullptr; + elseBlock = nullptr; + likelihood = HIGH_PROBABILITY; + doesReturnValue = stmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR); + origCall = GetCall(stmt); } //------------------------------------------------------------------------ @@ -181,7 +187,6 @@ class IndirectCallTransformer void Transform() { JITDUMP("*** %s: transforming " FMT_STMT "\n", Name(), stmt->GetID()); - FixupRetExpr(); ClearFlag(); CreateRemainder(); assert(GetChecksCount() > 0); @@ -197,10 +202,8 @@ class IndirectCallTransformer } protected: - virtual const char* Name() = 0; - virtual void ClearFlag() = 0; - virtual GenTreeCall* GetCall(Statement* callStmt) = 0; - virtual void FixupRetExpr() = 0; + virtual const char* Name() = 0; + virtual void ClearFlag() = 0; //------------------------------------------------------------------------ // CreateRemainder: split current block at the call stmt and @@ -240,6 +243,35 @@ class IndirectCallTransformer return block; } + //------------------------------------------------------------------------ + // SpillOperandToEndOfCheckBlock: spill an edge to a temp at the end of the "check" block. + // + // Parameters + // use - The use edge + // + void SpillOperandToEndOfCheckBlock(GenTree** use) + { + unsigned tmpNum = compiler->lvaGrabTemp(true DEBUGARG("operand temp")); + GenTree* const argNode = *use; + GenTree* store = compiler->gtNewTempStore(tmpNum, argNode); + + if (argNode->TypeIs(TYP_REF)) + { + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE cls = compiler->gtGetClassHandle(argNode, &isExact, &isNonNull); + if (cls != NO_CLASS_HANDLE) + { + compiler->lvaSetClass(tmpNum, cls, isExact); + } + } + + Statement* storeStmt = compiler->fgNewStmtFromTree(store, stmt->GetDebugInfo()); + compiler->fgInsertStmtAtEnd(checkBlock, storeStmt); + + *use = compiler->gtNewLclVarNode(tmpNum); + } + virtual void CreateThen(uint8_t checkIdx) = 0; virtual void CreateElse() = 0; @@ -310,6 +342,30 @@ class IndirectCallTransformer } } + //------------------------------------------------------------------------ + // GetCall: find a call in a statement. + // + // Arguments: + // callStmt - the statement with the call inside. + // + // Return Value: + // call tree node pointer. + GenTreeCall* GetCall(Statement* callStmt) + { + GenTree* tree = callStmt->GetRootNode(); + GenTreeCall* call = nullptr; + if (doesReturnValue) + { + assert(tree->OperIs(GT_STORE_LCL_VAR)); + call = tree->AsLclVar()->Data()->AsCall(); + } + else + { + call = tree->AsCall(); // call with void return type. + } + return call; + } + Compiler* compiler; BasicBlock* currBlock; BasicBlock* remainderBlock; @@ -319,6 +375,7 @@ class IndirectCallTransformer Statement* stmt; GenTreeCall* origCall; unsigned likelihood; + bool doesReturnValue; const int HIGH_PROBABILITY = 80; }; @@ -329,10 +386,8 @@ class IndirectCallTransformer FatPointerCallTransformer(Compiler* compiler, BasicBlock* block, Statement* stmt) : Transformer(compiler, block, stmt) { - doesReturnValue = stmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR); - origCall = GetCall(stmt); - fptrAddress = origCall->gtCallAddr; - pointerType = fptrAddress->TypeGet(); + fptrAddress = origCall->gtCallAddr; + pointerType = fptrAddress->TypeGet(); } protected: @@ -341,30 +396,6 @@ class IndirectCallTransformer return "FatPointerCall"; } - //------------------------------------------------------------------------ - // GetCall: find a call in a statement. - // - // Arguments: - // callStmt - the statement with the call inside. - // - // Return Value: - // call tree node pointer. - virtual GenTreeCall* GetCall(Statement* callStmt) - { - GenTree* tree = callStmt->GetRootNode(); - GenTreeCall* call = nullptr; - if (doesReturnValue) - { - assert(tree->OperIs(GT_STORE_LCL_VAR)); - call = tree->AsLclVar()->Data()->AsCall(); - } - else - { - call = tree->AsCall(); // call with void return type. - } - return call; - } - //------------------------------------------------------------------------ // ClearFlag: clear fat pointer candidate flag from the original call. // @@ -373,11 +404,6 @@ class IndirectCallTransformer origCall->ClearFatPointerCandidate(); } - // FixupRetExpr: no action needed as we handle this in the importer. - virtual void FixupRetExpr() - { - } - //------------------------------------------------------------------------ // CreateCheck: create check block, that checks fat pointer bit set. // @@ -385,7 +411,34 @@ class IndirectCallTransformer { assert(checkIdx == 0); - checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock, currBlock); + checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock, currBlock); + + GenTree** lastSpilledArg = nullptr; + for (GenTree** arg : origCall->UseEdges()) + { + if (ContainsInlineCandidate(*arg)) + { + lastSpilledArg = arg; + } + } + + if (lastSpilledArg != nullptr) + { + for (GenTree** arg : origCall->UseEdges()) + { + GenTree* argNode = *arg; + if (((argNode->gtFlags & GTF_ALL_EFFECT) != 0) || compiler->gtHasLocalsWithAddrOp(argNode)) + { + SpillOperandToEndOfCheckBlock(arg); + } + + if (arg == lastSpilledArg) + { + break; + } + } + } + GenTree* fatPointerMask = new (compiler, GT_CNS_INT) GenTreeIntCon(TYP_I_IMPL, FAT_POINTER_MASK); GenTree* fptrAddressCopy = compiler->gtCloneExpr(fptrAddress); GenTree* fatPointerAnd = compiler->gtNewOperNode(GT_AND, TYP_I_IMPL, fptrAddressCopy, fatPointerMask); @@ -396,6 +449,46 @@ class IndirectCallTransformer compiler->fgInsertStmtAtEnd(checkBlock, jmpStmt); } + bool ContainsInlineCandidate(GenTree* tree) + { + struct Visitor : GenTreeVisitor + { + enum + { + DoPreOrder = true, + }; + + Visitor(Compiler* comp) + : GenTreeVisitor(comp) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + if ((tree->gtFlags & GTF_CALL) == 0) + { + return fgWalkResult::WALK_SKIP_SUBTREES; + } + + if (tree->IsCall() && tree->AsCall()->IsInlineCandidate()) + { + return fgWalkResult::WALK_ABORT; + } + + return fgWalkResult::WALK_CONTINUE; + } + }; + + if ((tree->gtFlags & GTF_CALL) == 0) + { + return false; + } + + Visitor visitor(compiler); + return visitor.WalkTree(&tree, nullptr) == Compiler::WALK_ABORT; + } + //------------------------------------------------------------------------ // CreateThen: create then block, that is executed if the check succeeds. // This simply executes the original call. @@ -476,7 +569,6 @@ class IndirectCallTransformer GenTree* fptrAddress; var_types pointerType; - bool doesReturnValue; }; class GuardedDevirtualizationTransformer final : public Transformer @@ -484,8 +576,6 @@ class IndirectCallTransformer public: GuardedDevirtualizationTransformer(Compiler* compiler, BasicBlock* block, Statement* stmt) : Transformer(compiler, block, stmt) - , returnTemp(BAD_VAR_NUM) - , returnValueUnused(false) { } @@ -494,19 +584,10 @@ class IndirectCallTransformer // virtual void Run() { - origCall = GetCall(stmt); - JITDUMP("\n----------------\n\n*** %s contemplating [%06u] in " FMT_BB " \n", Name(), compiler->dspTreeID(origCall), currBlock->bbNum); - // We currently need inline candidate info to guarded devirt. - // - if (!origCall->IsInlineCandidate()) - { - JITDUMP("*** %s Bailing on [%06u] -- not an inline candidate\n", Name(), compiler->dspTreeID(origCall)); - ClearFlag(); - return; - } + assert(origCall->IsInlineCandidate()); likelihood = origCall->GetGDVCandidateInfo(0)->likelihood; assert((likelihood >= 0) && (likelihood <= 100)); @@ -555,22 +636,6 @@ class IndirectCallTransformer return "GuardedDevirtualization"; } - //------------------------------------------------------------------------ - // GetCall: find a call in a statement. - // - // Arguments: - // callStmt - the statement with the call inside. - // - // Return Value: - // call tree node pointer. - virtual GenTreeCall* GetCall(Statement* callStmt) - { - GenTree* tree = callStmt->GetRootNode(); - assert(tree->IsCall()); - GenTreeCall* call = tree->AsCall(); - return call; - } - virtual void ClearFlag() { // We remove the GDV flag from the call in the CreateElse @@ -654,7 +719,7 @@ class IndirectCallTransformer GenTree* argNode = arg.GetNode(); if (((argNode->gtFlags & GTF_ALL_EFFECT) != 0) || compiler->gtHasLocalsWithAddrOp(argNode)) { - SpillArgToTempBeforeGuard(&arg); + SpillOperandToEndOfCheckBlock(&arg.NodeRef()); } if (&arg == lastSideEffArg) @@ -669,7 +734,7 @@ class IndirectCallTransformer // is going to be used multiple times due to the guard. if (!thisArg->GetNode()->IsLocal()) { - SpillArgToTempBeforeGuard(thisArg); + SpillOperandToEndOfCheckBlock(&thisArg->NodeRef()); } GenTree* thisTree = compiler->gtCloneExpr(thisArg->GetNode()); @@ -749,156 +814,6 @@ class IndirectCallTransformer compiler->fgInsertStmtAtEnd(checkBlock, jmpStmt); } - //------------------------------------------------------------------------ - // SpillArgToTempBeforeGuard: spill an argument into a temp in the guard/check block. - // - // Parameters - // arg - The arg to create a temp and local store for. - // - void SpillArgToTempBeforeGuard(CallArg* arg) - { - unsigned tmpNum = compiler->lvaGrabTemp(true DEBUGARG("guarded devirt arg temp")); - GenTree* const argNode = arg->GetNode(); - GenTree* store = compiler->gtNewTempStore(tmpNum, argNode); - - if (argNode->TypeIs(TYP_REF)) - { - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE cls = compiler->gtGetClassHandle(argNode, &isExact, &isNonNull); - if (cls != NO_CLASS_HANDLE) - { - compiler->lvaSetClass(tmpNum, cls, isExact); - } - } - - Statement* storeStmt = compiler->fgNewStmtFromTree(store, stmt->GetDebugInfo()); - compiler->fgInsertStmtAtEnd(checkBlock, storeStmt); - - arg->SetEarlyNode(compiler->gtNewLclVarNode(tmpNum)); - } - - //------------------------------------------------------------------------ - // FixupRetExpr: set up to repair return value placeholder from call - // - virtual void FixupRetExpr() - { - // If call returns a value, we need to copy it to a temp, and - // bash the associated GT_RET_EXPR to refer to the temp instead - // of the call. - // - // Note implicit by-ref returns should have already been converted - // so any struct copy we induce here should be cheap. - InlineCandidateInfo* const inlineInfo = origCall->GetGDVCandidateInfo(0); - GenTree* const retExprNode = inlineInfo->retExpr; - - if (retExprNode == nullptr) - { - // We do not produce GT_RET_EXPRs for CTOR calls, so there is nothing to patch. - return; - } - - GenTreeRetExpr* const retExpr = retExprNode->AsRetExpr(); - bool const noReturnValue = origCall->TypeIs(TYP_VOID); - - // If there is a return value, search the next statement to see if we can find - // retExprNode's parent. If we find it, see if retExprNode's value is unused. - // - // If we fail to find it, we will assume the return value is used. - // - if (!noReturnValue) - { - Statement* const nextStmt = stmt->GetNextStmt(); - if (nextStmt != nullptr) - { - Compiler::FindLinkData fld = compiler->gtFindLink(nextStmt, retExprNode); - GenTree* const parent = fld.parent; - - if ((parent != nullptr) && parent->OperIs(GT_COMMA) && (parent->AsOp()->gtGetOp1() == retExprNode)) - { - returnValueUnused = true; - JITDUMP("GT_RET_EXPR [%06u] value is unused\n", compiler->dspTreeID(retExprNode)); - } - } - } - - if (noReturnValue) - { - JITDUMP("Linking GT_RET_EXPR [%06u] for VOID return to NOP\n", - compiler->dspTreeID(inlineInfo->retExpr)); - inlineInfo->retExpr->gtSubstExpr = compiler->gtNewNothingNode(); - } - else if (returnValueUnused) - { - JITDUMP("Linking GT_RET_EXPR [%06u] for UNUSED return to NOP\n", - compiler->dspTreeID(inlineInfo->retExpr)); - inlineInfo->retExpr->gtSubstExpr = compiler->gtNewNothingNode(); - } - else - { - // If there's a spill temp already associated with this inline candidate, - // use that instead of allocating a new temp. - // - returnTemp = inlineInfo->preexistingSpillTemp; - - if (returnTemp != BAD_VAR_NUM) - { - JITDUMP("Reworking call(s) to return value via a existing return temp V%02u\n", returnTemp); - - // We will be introducing multiple defs for this temp, so make sure - // it is no longer marked as single def. - // - // Otherwise, we could make an incorrect type deduction. Say the - // original call site returns a B, but after we devirtualize along the - // GDV happy path we see that method returns a D. We can't then assume that - // the return temp is type D, because we don't know what type the fallback - // path returns. So we have to stick with the current type for B as the - // return type. - // - // Note local vars always live in the root method's symbol table. So we - // need to use the root compiler for lookup here. - // - LclVarDsc* const returnTempLcl = compiler->impInlineRoot()->lvaGetDesc(returnTemp); - - if (returnTempLcl->lvSingleDef == 1) - { - // In this case it's ok if we already updated the type assuming single def, - // we just don't want any further updates. - // - JITDUMP("Return temp V%02u is no longer a single def temp\n", returnTemp); - returnTempLcl->lvSingleDef = 0; - } - } - else - { - returnTemp = compiler->lvaGrabTemp(false DEBUGARG("guarded devirt return temp")); - JITDUMP("Reworking call(s) to return value via a new temp V%02u\n", returnTemp); - - // Keep the information about small typedness to avoid - // inserting unnecessary casts for normalization, which can - // make tailcall invariants unhappy. This is the same logic - // that impImportCall uses when it introduces call temps. - if (varTypeIsSmall(origCall->gtReturnType)) - { - assert(origCall->NormalizesSmallTypesOnReturn()); - compiler->lvaGetDesc(returnTemp)->lvType = origCall->gtReturnType; - } - } - - if (varTypeIsStruct(origCall)) - { - compiler->lvaSetStruct(returnTemp, origCall->gtRetClsHnd, false); - } - - GenTree* tempTree = compiler->gtNewLclvNode(returnTemp, origCall->TypeGet()); - - JITDUMP("Linking GT_RET_EXPR [%06u] to refer to temp V%02u\n", compiler->dspTreeID(inlineInfo->retExpr), - returnTemp); - - inlineInfo->retExpr->gtSubstExpr = tempTree; - } - } - //------------------------------------------------------------------------ // Devirtualize origCall using the given inline candidate // @@ -1041,6 +956,13 @@ class IndirectCallTransformer // If the devirtualizer was unable to transform the call to invoke the unboxed entry, the inline info // we set up may be invalid. We won't be able to inline anyways. So demote the call as an inline candidate. // + GenTree* node = call; + if (doesReturnValue) + { + assert(stmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR)); + node = compiler->gtNewTempStore(stmt->GetRootNode()->AsLclVarCommon()->GetLclNum(), node); + } + CORINFO_METHOD_HANDLE unboxedMethodHnd = inlineInfo->guardedMethodUnboxedEntryHandle; if ((unboxedMethodHnd != nullptr) && (methodHnd != unboxedMethodHnd)) { @@ -1051,54 +973,26 @@ class IndirectCallTransformer call->gtFlags &= ~GTF_CALL_INLINE_CANDIDATE; call->ClearInlineInfo(); - - if (returnTemp != BAD_VAR_NUM) - { - GenTree* const store = compiler->gtNewTempStore(returnTemp, call); - compiler->fgNewStmtAtEnd(block, store); - } - else - { - compiler->fgNewStmtAtEnd(block, call, stmt->GetDebugInfo()); - } + compiler->fgNewStmtAtEnd(block, node, stmt->GetDebugInfo()); } else { // Add the call. // - compiler->fgNewStmtAtEnd(block, call, stmt->GetDebugInfo()); + compiler->fgNewStmtAtEnd(block, node, stmt->GetDebugInfo()); // Re-establish this call as an inline candidate. // - GenTreeRetExpr* oldRetExpr = inlineInfo->retExpr; - inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd); - inlineInfo->exactContextHandle = context; - inlineInfo->preexistingSpillTemp = returnTemp; + inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd); + inlineInfo->exactContextHandle = context; + inlineInfo->preexistingSpillTemp = + doesReturnValue ? stmt->GetRootNode()->AsLclVarCommon()->GetLclNum() : BAD_VAR_NUM; call->SetSingleInlineCandidateInfo(inlineInfo); + } - // If there was a ret expr for this call, we need to create a new one - // and append it just after the call. - // - // Note the original GT_RET_EXPR has been linked to a temp. - // we set all this up in FixupRetExpr(). - if (oldRetExpr != nullptr) - { - inlineInfo->retExpr = compiler->gtNewInlineCandidateReturnExpr(call, call->TypeGet()); - GenTree* newRetExpr = inlineInfo->retExpr; - - if (returnTemp != BAD_VAR_NUM) - { - newRetExpr = compiler->gtNewTempStore(returnTemp, newRetExpr); - } - else - { - // We should always have a return temp if we return results by value - // and that value is used. - assert(origCall->TypeIs(TYP_VOID) || returnValueUnused); - newRetExpr = compiler->gtUnusedValNode(newRetExpr); - } - compiler->fgNewStmtAtEnd(block, newRetExpr); - } + if (doesReturnValue) + { + compiler->lvaGetDesc(stmt->GetRootNode()->AsLclVarCommon())->lvSingleDef = false; } } @@ -1223,9 +1117,9 @@ class IndirectCallTransformer JITDUMP("Residual call [%06u] moved to block " FMT_BB "\n", compiler->dspTreeID(call), elseBlock->bbNum); - if (returnTemp != BAD_VAR_NUM) + if (doesReturnValue) { - GenTree* store = compiler->gtNewTempStore(returnTemp, call); + GenTree* store = compiler->gtNewTempStore(stmt->GetRootNode()->AsLclVarCommon()->GetLclNum(), call); newStmt->SetRootNode(store); } @@ -1422,21 +1316,6 @@ class IndirectCallTransformer return fgWalkResult::WALK_ABORT; } } - else if (node->OperIs(GT_RET_EXPR)) - { - // If this is a RET_EXPR that we already know how to substitute then it is the - // "fixed-up" RET_EXPR from a previous GDV candidate. In that case we can - // substitute it right here to make it eligibile for cloning. - if (node->AsRetExpr()->gtSubstExpr != nullptr) - { - assert(node->AsRetExpr()->gtInlineCandidate->IsGuarded()); - *use = node->AsRetExpr()->gtSubstExpr; - return fgWalkResult::WALK_CONTINUE; - } - - m_unclonableNode = node; - return fgWalkResult::WALK_ABORT; - } m_nodeCount++; return fgWalkResult::WALK_CONTINUE; @@ -1447,27 +1326,6 @@ class IndirectCallTransformer { JITDUMP(" Scouting " FMT_STMT "\n", nextStmt->GetID()); - // See if this is a guarded devirt candidate. - // These will be top-level trees. - // - GenTree* const root = nextStmt->GetRootNode(); - - if (root->IsCall()) - { - GenTreeCall* const call = root->AsCall(); - - if (call->IsGuardedDevirtualizationCandidate() && - (call->GetGDVCandidateInfo(0)->likelihood >= gdvChainLikelihood)) - { - JITDUMP("GDV call at [%06u] has likelihood %u >= %u; chaining (%u stmts, %u nodes to dup).\n", - compiler->dspTreeID(call), call->GetGDVCandidateInfo(0)->likelihood, gdvChainLikelihood, - chainStatementDup, chainNodeDup); - - call->gtCallMoreFlags |= GTF_CALL_M_GUARDED_DEVIRT_CHAIN; - break; - } - } - // Stop searching if we've accumulated too much dup cost. // Consider: use node count instead. // @@ -1489,6 +1347,35 @@ class IndirectCallTransformer break; } + // See if this is a guarded devirt candidate. + // These will be top-level trees. + // + GenTree* const root = nextStmt->GetRootNode(); + + GenTreeCall* call = nullptr; + if (root->IsCall()) + { + call = root->AsCall(); + } + else if (root->OperIs(GT_STORE_LCL_VAR) && root->AsLclVarCommon()->Data()->IsCall()) + { + call = root->AsLclVarCommon()->Data()->AsCall(); + } + + if (call != nullptr) + { + if (call->IsGuardedDevirtualizationCandidate() && + (call->GetGDVCandidateInfo(0)->likelihood >= gdvChainLikelihood)) + { + JITDUMP("GDV call at [%06u] has likelihood %u >= %u; chaining (%u stmts, %u nodes to dup).\n", + compiler->dspTreeID(call), call->GetGDVCandidateInfo(0)->likelihood, gdvChainLikelihood, + chainStatementDup, chainNodeDup); + + call->gtCallMoreFlags |= GTF_CALL_M_GUARDED_DEVIRT_CHAIN; + break; + } + } + // Looks like we can clone this, so keep scouting. // chainStatementDup++; @@ -1497,7 +1384,6 @@ class IndirectCallTransformer } private: - unsigned returnTemp; Statement* lastStmt; bool checkFallsThrough; bool returnValueUnused; diff --git a/src/coreclr/jit/inline.cpp b/src/coreclr/jit/inline.cpp index 8f662c78450eaa..654ff9f6f6bb6d 100644 --- a/src/coreclr/jit/inline.cpp +++ b/src/coreclr/jit/inline.cpp @@ -635,7 +635,7 @@ void InlineContext::DumpXml(FILE* file, unsigned indent) // description - string describing the context of the decision InlineResult::InlineResult( - Compiler* compiler, GenTreeCall* call, Statement* stmt, const char* description, bool doNotReport) + Compiler* compiler, GenTreeCall* call, InlineContext* context, const char* description, bool doNotReport) : m_RootCompiler(nullptr) , m_Policy(nullptr) , m_Call(call) @@ -656,17 +656,12 @@ InlineResult::InlineResult( m_Policy = InlinePolicy::GetPolicy(m_RootCompiler, isPrejitRoot); // Pass along some optional information to the policy. - if (stmt != nullptr) - { - m_InlineContext = stmt->GetDebugInfo().GetInlineContext(); - m_Policy->NoteContext(m_InlineContext); + m_InlineContext = context; + m_Policy->NoteContext(m_InlineContext); #if defined(DEBUG) - m_Policy->NoteOffset(call->gtRawILOffset); -#else - m_Policy->NoteOffset(stmt->GetDebugInfo().GetLocation().GetOffset()); + m_Policy->NoteOffset(call->gtRawILOffset); #endif // defined(DEBUG) - } // Get method handle for caller. Note we use the // handle for the "immediate" caller here. @@ -1300,13 +1295,6 @@ InlineContext* InlineStrategy::NewContext(InlineContext* parentContext, Statemen context->m_ILSize = info->methInfo.ILCodeSize; context->m_ActualCallOffset = info->ilOffset; context->m_RuntimeContext = info->exactContextHandle; - -#ifdef DEBUG - // All inline candidates should get their own statements that have - // appropriate debug info (or no debug info). - InlineContext* diInlineContext = stmt->GetDebugInfo().GetInlineContext(); - assert(diInlineContext == nullptr || diInlineContext == parentContext); -#endif } else { @@ -1315,7 +1303,7 @@ InlineContext* InlineStrategy::NewContext(InlineContext* parentContext, Statemen } // We currently store both the statement location (used when reporting - // only-style mappings) and the actual call offset (used when reporting the + // old-style mappings) and the actual call offset (used when reporting the // inline tree for rich debug info). // These are not always the same, consider e.g. // ldarg.0 diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index a89be81fa68895..4ce99727fcf321 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -233,10 +233,12 @@ class InlinePolicy { (void)context; } +#ifdef DEBUG virtual void NoteOffset(IL_OFFSET offset) { (void)offset; } +#endif // Policy determinations virtual void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) = 0; @@ -344,8 +346,11 @@ class InlineResult public: // Construct a new InlineResult to help evaluate a // particular call for inlining. - InlineResult( - Compiler* compiler, GenTreeCall* call, Statement* stmt, const char* description, bool doNotReport = false); + InlineResult(Compiler* compiler, + GenTreeCall* call, + InlineContext* context, + const char* description, + bool doNotReport = false); // Construct a new InlineResult to evaluate a particular // method to see if it is inlineable. @@ -586,6 +591,13 @@ struct HandleHistogramProfileCandidateInfo unsigned probeIndex; }; +// TODO: move to InlineInfo +struct InlineIRResult +{ + GenTree* substExpr; + BasicBlock* substBB; +}; + // InlineCandidateInfo provides basic information about a particular // inline candidate. // @@ -616,8 +628,7 @@ struct InlineCandidateInfo : public HandleHistogramProfileCandidateInfo CORINFO_METHOD_HANDLE originalMethodHandle; CORINFO_CONTEXT_HANDLE originalContextHandle; - // The GT_RET_EXPR node linking back to the inline candidate. - GenTreeRetExpr* retExpr; + InlineIRResult result; unsigned preexistingSpillTemp; unsigned clsAttr; @@ -672,6 +683,23 @@ struct InlLclVarInfo unsigned char lclIsPinned : 1; }; +class StatementListBuilder +{ + Statement* m_head = nullptr; + Statement* m_tail = nullptr; + +public: + bool Empty() + { + return m_head == nullptr; + } + + void Append(Statement* stmt); + + void InsertIntoBlockAtBeginning(BasicBlock* block); + void InsertIntoBlockBefore(BasicBlock* block, Statement* before); +}; + // InlineInfo provides detailed information about a particular inline candidate. struct InlineInfo @@ -695,6 +723,7 @@ struct InlineInfo unsigned argCnt; InlArgInfo inlArgInfo[MAX_INL_ARGS + 1]; InlArgInfo* inlInstParamArgInfo; + InlArgInfo* inlRetBufferArgInfo; int lclTmpNum[MAX_INL_LCLS]; // map local# -> temp# (-1 if unused) InlLclVarInfo lclVarInfo[MAX_INL_LCLS + MAX_INL_ARGS + 1]; // type information from local sig @@ -714,6 +743,9 @@ struct InlineInfo GenTreeCall* iciCall; // The GT_CALL node to be inlined. Statement* iciStmt; // The statement iciCall is in. BasicBlock* iciBlock; // The basic block iciStmt is in. + + StatementListBuilder setupStatements; + StatementListBuilder teardownStatements; }; //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 4e6b559c989baf..97073789af43e8 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1599,6 +1599,12 @@ bool Compiler::StructPromotionHelper::CanPromoteStructVar(unsigned lclNum) return false; } + if (varDsc->GetLayout() == nullptr) + { + JITDUMP(" struct promotion of V%02u is disabled because it has no layout\n", lclNum); + return false; + } + if (varDsc->GetLayout()->IsCustomLayout()) { JITDUMP(" struct promotion of V%02u is disabled because it has custom layout\n", lclNum); diff --git a/src/coreclr/jit/simd.cpp b/src/coreclr/jit/simd.cpp index 7fdbd76afb0081..6de90b561052fb 100644 --- a/src/coreclr/jit/simd.cpp +++ b/src/coreclr/jit/simd.cpp @@ -483,7 +483,7 @@ GenTree* Compiler::impSIMDPopStack() assert(varTypeIsSIMDOrMask(tree)); // Handle calls that may return the struct via a return buffer. - if (tree->OperIs(GT_CALL, GT_RET_EXPR)) + if (tree->OperIs(GT_CALL)) { tree = impNormStructVal(tree, CHECK_SPILL_ALL); }