diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dc3e3c4171e798..dc40710409dd88 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2583,7 +2583,7 @@ class Compiler GenTree* gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd); GenTreeLclFld* gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset); - GenTree* gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_types type, BasicBlockFlags bbFlags); + GenTreeRetExpr* gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCandidate, var_types type); GenTreeField* gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 9131c7eb6f3558..55443f8d0a2b70 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -100,7 +100,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_RET_EXPR)) { - UpdateInlineReturnExpressionPlaceHolder(tree, user); + UpdateInlineReturnExpressionPlaceHolder(use, user); } #if FEATURE_MULTIREG_RET @@ -121,9 +121,10 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtEffectiveVal(/*commaOnly*/ true); - noway_assert(!varTypeIsStruct(effectiveValue) || (effectiveValue->OperGet() != GT_RET_EXPR) || - !m_compiler->IsMultiRegReturnedType(effectiveValue->AsRetExpr()->gtRetClsHnd, - CorInfoCallConvExtension::Managed)); + noway_assert( + !varTypeIsStruct(effectiveValue) || (effectiveValue->OperGet() != GT_RET_EXPR) || + !m_compiler->IsMultiRegReturnedType(effectiveValue->AsRetExpr()->gtInlineCandidate->gtRetClsHnd, + CorInfoCallConvExtension::Managed)); } } @@ -144,8 +145,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_RET_EXPR)) + while ((*use)->OperIs(GT_RET_EXPR)) { + GenTree* tree = *use; // We are going to copy the tree from the inlinee, // so record the handle now. // if (varTypeIsStruct(tree)) { - retClsHnd = tree->AsRetExpr()->gtRetClsHnd; + retClsHnd = tree->AsRetExpr()->gtInlineCandidate->gtRetClsHnd; } // Skip through chains of GT_RET_EXPRs (say from nested inlines) // to the actual tree to use. // - BasicBlockFlags bbFlags; - GenTree* inlineCandidate = tree; + BasicBlock* prevInlineeBB = nullptr; + BasicBlock* inlineeBB = nullptr; + GenTree* inlineCandidate = tree; do { GenTreeRetExpr* retExpr = inlineCandidate->AsRetExpr(); - inlineCandidate = retExpr->gtInlineCandidate; - bbFlags = retExpr->bbFlags; + inlineCandidate = retExpr->gtSubstExpr; + prevInlineeBB = inlineeBB; + inlineeBB = retExpr->gtSubstBB; } while (inlineCandidate->OperIs(GT_RET_EXPR)); // We might as well try and fold the return value. Eg returns of @@ -235,15 +239,43 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorReplaceWith(inlineCandidate, m_compiler); + *use = inlineCandidate; m_madeChanges = true; - m_compiler->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED); + + // TODO-Inlining: The inliner had strange previous behavior where + // it for successful inlines that were chains would propagate the + // flags from the second-to-last inlinee's BB. Remove this once we + // can take these changes. We still need to propagate the mandatory + // "IR-presence" flags. + // Furthermore, we should really only propagate BBF_COPY_PROPAGATE + // flags here. BBF_SPLIT_GAINED includes BBF_PROF_WEIGHT, and + // propagating that has the effect that inlining a tree from a hot + // block into a block without profile weights means we suddenly + // start to see the inliner block as hot and treat future inline + // candidates more aggressively. + // + BasicBlockFlags newBBFlags = BBF_EMPTY; + if (prevInlineeBB != nullptr) + { + newBBFlags |= prevInlineeBB->bbFlags; + + if (inlineeBB != nullptr) + { + newBBFlags |= inlineeBB->bbFlags & BBF_COPY_PROPAGATE; + } + } + else if (inlineeBB != nullptr) + { + newBBFlags |= inlineeBB->bbFlags; + } + + m_compiler->compCurBB->bbFlags |= (newBBFlags & BBF_SPLIT_GAINED); #ifdef DEBUG if (m_compiler->verbose) { printf("\nInserting the inline return expression\n"); - m_compiler->gtDispTree(tree); + m_compiler->gtDispTree(inlineCandidate); printf("\n"); } #endif // DEBUG @@ -275,13 +307,13 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperIs(GT_ASG)) { // The inlinee can only be the RHS. - assert(parent->gtGetOp2() == tree); + assert(parent->gtGetOp2() == *use); AttachStructInlineeToAsg(parent->AsOp(), retClsHnd); } else { // Just assign the inlinee to a variable to keep it simple. - tree->ReplaceWith(AssignStructInlineeToVar(tree, retClsHnd), m_compiler); + *use = AssignStructInlineeToVar(*use, retClsHnd); } m_madeChanges = true; } @@ -853,8 +885,6 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe inlineInfo.iciStmt = fgMorphStmt; inlineInfo.iciBlock = compCurBB; inlineInfo.thisDereferencedFirst = false; - inlineInfo.retExpr = nullptr; - inlineInfo.retBB = nullptr; inlineInfo.retExprClassHnd = nullptr; inlineInfo.retExprClassHndIsExact = false; inlineInfo.inlineResult = inlineResult; @@ -1006,12 +1036,12 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe } #endif // DEBUG - // If there is non-NULL return, but we haven't set the pInlineInfo->retExpr, + // 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->fncRetType != TYP_VOID && inlineInfo.retExpr == nullptr) + if ((inlineCandidateInfo->fncRetType != TYP_VOID) && (inlineCandidateInfo->retExpr->gtSubstExpr == nullptr)) { #ifdef DEBUG if (verbose) @@ -1386,40 +1416,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) lvaSetVarDoNotEnregister(dummy DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr)); } - // If there is non-NULL return, replace the GT_CALL with its return value expression, - // so later it will be picked up by the GT_RET_EXPR node. - if ((pInlineInfo->inlineCandidateInfo->fncRetType != TYP_VOID) || (iciCall->gtReturnType == TYP_STRUCT)) - { - noway_assert(pInlineInfo->retExpr); -#ifdef DEBUG - if (verbose) - { - printf("\nReturn expression for call at "); - printTreeID(iciCall); - printf(" is\n"); - gtDispTree(pInlineInfo->retExpr); - } -#endif // DEBUG - - // Replace the call with the return expression. Note that iciCall won't be part of the IR - // but may still be referenced from a GT_RET_EXPR node. We will replace GT_RET_EXPR node - // in UpdateInlineReturnExpressionPlaceHolder. At that time we will also update the flags - // on the basic block of GT_RET_EXPR node. - if (iciCall->gtInlineCandidateInfo->retExpr->OperGet() == GT_RET_EXPR) - { - // Save the basic block flags from the retExpr basic block. - iciCall->gtInlineCandidateInfo->retExpr->AsRetExpr()->bbFlags = pInlineInfo->retBB->bbFlags; - } - - if (bottomBlock != nullptr) - { - // We've split the iciblock into two and the RET_EXPR was possibly moved to the bottomBlock - // so let's update its flags with retBB's ones - bottomBlock->bbFlags |= pInlineInfo->retBB->bbFlags & BBF_COMPACT_UPD; - } - iciCall->ReplaceWith(pInlineInfo->retExpr, this); - } - // // 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. @@ -1839,6 +1835,7 @@ void Compiler::fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* bloc InlLclVarInfo* lclVarInfo = inlineInfo->lclVarInfo; unsigned gcRefLclCnt = inlineInfo->numberOfGcRefLocals; const unsigned argCnt = inlineInfo->argCnt; + InlineCandidateInfo* inlCandInfo = inlineInfo->inlineCandidateInfo; for (unsigned lclNum = 0; lclNum < lclCnt; lclNum++) { @@ -1873,10 +1870,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. - GenTree* retExpr = inlineInfo->retExpr; - if (retExpr != nullptr) + if ((inlCandInfo->retExpr != nullptr) && (inlCandInfo->retExpr->gtSubstExpr != nullptr)) { - const bool interferesWithReturn = gtHasRef(inlineInfo->retExpr, tmpNum); + const bool interferesWithReturn = gtHasRef(inlCandInfo->retExpr->gtSubstExpr, tmpNum); noway_assert(!interferesWithReturn); } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 2321a624fc74a1..e3d52047170f65 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7364,7 +7364,7 @@ GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned return node; } -GenTree* Compiler::gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_types type, BasicBlockFlags bbFlags) +GenTreeRetExpr* Compiler::gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCandidate, var_types type) { assert(GenTree::s_gtNodeSizes[GT_RET_EXPR] == TREE_NODE_SZ_LARGE); @@ -7372,14 +7372,10 @@ GenTree* Compiler::gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_ node->gtInlineCandidate = inlineCandidate; - node->bbFlags = bbFlags; + node->gtSubstExpr = nullptr; + node->gtSubstBB = nullptr; - if (varTypeIsStruct(inlineCandidate) && !inlineCandidate->OperIsBlkOp()) - { - node->gtRetClsHnd = gtGetStructHandle(inlineCandidate); - } - - // GT_RET_EXPR node eventually might be bashed back to GT_CALL (when inlining is aborted for example). + // 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; @@ -11484,10 +11480,16 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) case GT_RET_EXPR: { - GenTree* const associatedTree = tree->AsRetExpr()->gtInlineCandidate; - printf("(inl return %s ", tree->IsCall() ? " from call" : "expr"); - printTreeID(associatedTree); + GenTreeCall* inlineCand = tree->AsRetExpr()->gtInlineCandidate; + printf("(for "); + printTreeID(inlineCand); printf(")"); + + if (tree->AsRetExpr()->gtSubstExpr != nullptr) + { + printf(" -> "); + printTreeID(tree->AsRetExpr()->gtSubstExpr); + } } break; @@ -17367,7 +17369,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) structHnd = tree->AsCall()->gtRetClsHnd; break; case GT_RET_EXPR: - structHnd = tree->AsRetExpr()->gtRetClsHnd; + structHnd = tree->AsRetExpr()->gtInlineCandidate->gtRetClsHnd; break; case GT_FIELD: info.compCompHnd->getFieldType(tree->AsField()->gtFldHnd, &structHnd); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 70abd0a98ebb19..04e90363c5248c 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -7234,11 +7234,18 @@ struct GenTreeStoreInd : public GenTreeIndir struct GenTreeRetExpr : public GenTree { - GenTree* gtInlineCandidate; - - BasicBlockFlags bbFlags; - - CORINFO_CLASS_HANDLE gtRetClsHnd; + 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) { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index f2ac7b70511bad..9495b45b817613 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -11294,13 +11294,15 @@ 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); - var_types originalCallType = impInlineInfo->inlineCandidateInfo->fncRetType; + var_types originalCallType = inlCandInfo->fncRetType; if ((returnType != originalCallType) && (originalCallType == TYP_STRUCT)) { - originalCallType = impNormStructType(impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass); + originalCallType = impNormStructType(inlCandInfo->methInfo.args.retTypeClass); } if (returnType != originalCallType) @@ -11370,7 +11372,7 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) bool isNonNull = false; CORINFO_CLASS_HANDLE returnClsHnd = gtGetClassHandle(op2, &isExact, &isNonNull); - if (impInlineInfo->retExpr == nullptr) + if (inlRetExpr->gtSubstExpr == nullptr) { // This is the first return, so best known type is the type // of this return value. @@ -11394,12 +11396,12 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) op2 = tmpOp2; #ifdef DEBUG - if (impInlineInfo->retExpr) + if (inlRetExpr->gtSubstExpr != nullptr) { // Some other block(s) have seen the CEE_RET first. // Better they spilled to the same temp. - assert(impInlineInfo->retExpr->gtOper == GT_LCL_VAR); - assert(impInlineInfo->retExpr->AsLclVarCommon()->GetLclNum() == + assert(inlRetExpr->gtSubstExpr->gtOper == GT_LCL_VAR); + assert(inlRetExpr->gtSubstExpr->AsLclVarCommon()->GetLclNum() == op2->AsLclVarCommon()->GetLclNum()); } #endif @@ -11414,7 +11416,7 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif // Report the return expression - impInlineInfo->retExpr = op2; + inlRetExpr->gtSubstExpr = op2; } else { @@ -11440,16 +11442,16 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) if (fgNeedReturnSpillTemp()) { - if (impInlineInfo->retExpr == nullptr) + if (inlRetExpr->gtSubstExpr == nullptr) { // The inlinee compiler has figured out the type of the temp already. Use it here. - impInlineInfo->retExpr = + inlRetExpr->gtSubstExpr = gtNewLclvNode(lvaInlineeReturnSpillTemp, lvaTable[lvaInlineeReturnSpillTemp].lvType); } } else { - impInlineInfo->retExpr = op2; + inlRetExpr->gtSubstExpr = op2; } } else // The struct was to be returned via a return buffer. @@ -11460,24 +11462,37 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) if (fgNeedReturnSpillTemp()) { // If this is the first return we have seen set the retExpr. - if (impInlineInfo->retExpr == nullptr) + if (inlRetExpr->gtSubstExpr == nullptr) { - impInlineInfo->retExpr = + inlRetExpr->gtSubstExpr = impAssignStructPtr(dest, gtNewLclvNode(lvaInlineeReturnSpillTemp, info.compRetType), retClsHnd, CHECK_SPILL_ALL); } } else { - impInlineInfo->retExpr = impAssignStructPtr(dest, op2, retClsHnd, CHECK_SPILL_ALL); + inlRetExpr->gtSubstExpr = impAssignStructPtr(dest, op2, retClsHnd, CHECK_SPILL_ALL); } } } - if (impInlineInfo->retExpr != nullptr) - { - impInlineInfo->retBB = compCurBB; - } + // TODO-Inlining: Setting gtSubstBB unconditionally here does not + // make sense. The intention is to be able to propagate BB flags in + // UpdateInlineReturnExpressionPlaceHolder, but we only need to + // propagate the mandatory flags in case gtSubstExpr is a tree + // (e.g. GT_ARR_LENGTH in gtSubstExpr means we need to set + // BBF_HAS_IDX_LEN on the final BB that the substituted expression + // ends up in -- so we should not need to set this for the spill + // temp cases above). + // + // However, during substitution we actually propagate all + // BBF_SPLIT_GAINED flags which includes BBF_PROF_WEIGHT. The net + // effect of this is that if we inline a tree from a hot block into + // a block without profile data we suddenly start to believe the + // inliner block is hot, and then future inline candidates are + // treated more aggressively. Changing this leads to large diffs. + assert(inlRetExpr->gtSubstExpr != nullptr); + inlRetExpr->gtSubstBB = compCurBB; } } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 78f05c62192fd4..bfd042fd102e83 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1389,7 +1389,7 @@ var_types Compiler::impImportCall(OPCODE opcode, impAppendTree(call, CHECK_SPILL_ALL, impCurStmtDI, false); // TODO: Still using the widened type. - GenTree* retExpr = gtNewInlineCandidateReturnExpr(call, genActualType(callRetTyp), compCurBB->bbFlags); + GenTreeRetExpr* retExpr = gtNewInlineCandidateReturnExpr(call->AsCall(), genActualType(callRetTyp)); // Link the retExpr to the call so if necessary we can manipulate it later. origCall->gtInlineCandidateInfo->retExpr = retExpr; diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 6af4872473855d..894cdd5ab58593 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -707,15 +707,8 @@ class IndirectCallTransformer // Note implicit by-ref returns should have already been converted // so any struct copy we induce here should be cheap. InlineCandidateInfo* const inlineInfo = origCall->gtInlineCandidateInfo; - GenTree* const retExpr = inlineInfo->retExpr; - // Sanity check the ret expr if non-null: it should refer to the original call. - if (retExpr != nullptr) - { - assert(retExpr->AsRetExpr()->gtInlineCandidate == origCall); - } - - if (origCall->TypeGet() != TYP_VOID) + if (!origCall->TypeIs(TYP_VOID)) { // If there's a spill temp already associated with this inline candidate, // use that instead of allocating a new temp. @@ -763,12 +756,12 @@ class IndirectCallTransformer GenTree* tempTree = compiler->gtNewLclvNode(returnTemp, origCall->TypeGet()); - JITDUMP("Bashing GT_RET_EXPR [%06u] to refer to temp V%02u\n", compiler->dspTreeID(retExpr), + JITDUMP("Linking GT_RET_EXPR [%06u] to refer to temp V%02u\n", compiler->dspTreeID(inlineInfo->retExpr), returnTemp); - retExpr->ReplaceWith(tempTree, compiler); + inlineInfo->retExpr->gtSubstExpr = tempTree; } - else if (retExpr != nullptr) + else if (inlineInfo->retExpr != nullptr) { // We still oddly produce GT_RET_EXPRs for some void // returning calls. Just bash the ret expr to a NOP. @@ -777,8 +770,9 @@ class IndirectCallTransformer // benefit they provide is stitching back larger trees for failed inlines // of void-returning methods. But then the calls likely sit in commas and // the benefit of a larger tree is unclear. - JITDUMP("Bashing GT_RET_EXPR [%06u] for VOID return to NOP\n", compiler->dspTreeID(retExpr)); - retExpr->gtBashToNOP(); + JITDUMP("Linking GT_RET_EXPR [%06u] for VOID return to NOP\n", + compiler->dspTreeID(inlineInfo->retExpr)); + inlineInfo->retExpr->gtSubstExpr = compiler->gtNewNothingNode(); } else { @@ -918,7 +912,7 @@ class IndirectCallTransformer // Re-establish this call as an inline candidate. // - GenTree* oldRetExpr = inlineInfo->retExpr; + GenTreeRetExpr* oldRetExpr = inlineInfo->retExpr; inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd); inlineInfo->exactContextHnd = context; inlineInfo->preexistingSpillTemp = returnTemp; @@ -927,24 +921,24 @@ class IndirectCallTransformer // 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 bashed to a temp. + // Note the original GT_RET_EXPR has been linked to a temp. // we set all this up in FixupRetExpr(). if (oldRetExpr != nullptr) { - GenTree* retExpr = - compiler->gtNewInlineCandidateReturnExpr(call, call->TypeGet(), thenBlock->bbFlags); - inlineInfo->retExpr = retExpr; + inlineInfo->retExpr = compiler->gtNewInlineCandidateReturnExpr(call, call->TypeGet()); + + GenTree* newRetExpr = inlineInfo->retExpr; if (returnTemp != BAD_VAR_NUM) { - retExpr = compiler->gtNewTempAssign(returnTemp, retExpr); + newRetExpr = compiler->gtNewTempAssign(returnTemp, newRetExpr); } else { // We should always have a return temp if we return results by value assert(origCall->TypeGet() == TYP_VOID); } - compiler->fgNewStmtAtEnd(thenBlock, retExpr); + compiler->fgNewStmtAtEnd(thenBlock, newRetExpr); } } } @@ -1112,10 +1106,10 @@ class IndirectCallTransformer unsigned chainLikelihood = 0; GenTreeCall* chainedCall = nullptr; - // Helper class to check a statement for uncloneable nodes and count + // Helper class to check/fix a statement for clonability and count // the total number of nodes // - class UnclonableVisitor final : public GenTreeVisitor + class ClonabilityVisitor final : public GenTreeVisitor { public: enum @@ -1126,8 +1120,8 @@ class IndirectCallTransformer GenTree* m_unclonableNode; unsigned m_nodeCount; - UnclonableVisitor(Compiler* compiler) - : GenTreeVisitor(compiler), m_unclonableNode(nullptr), m_nodeCount(0) + ClonabilityVisitor(Compiler* compiler) + : GenTreeVisitor(compiler), m_unclonableNode(nullptr), m_nodeCount(0) { } @@ -1147,6 +1141,16 @@ class IndirectCallTransformer } 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; } @@ -1192,19 +1196,20 @@ class IndirectCallTransformer // See if this statement's tree is one that we can clone. // - UnclonableVisitor unclonableVisitor(compiler); - unclonableVisitor.WalkTree(nextStmt->GetRootNodePointer(), nullptr); + ClonabilityVisitor clonabilityVisitor(compiler); + clonabilityVisitor.WalkTree(nextStmt->GetRootNodePointer(), nullptr); - if (unclonableVisitor.m_unclonableNode != nullptr) + if (clonabilityVisitor.m_unclonableNode != nullptr) { - JITDUMP(" node [%06u] can't be cloned\n", compiler->dspTreeID(unclonableVisitor.m_unclonableNode)); + JITDUMP(" node [%06u] can't be cloned\n", + compiler->dspTreeID(clonabilityVisitor.m_unclonableNode)); break; } // Looks like we can clone this, so keep scouting. // chainStatementDup++; - chainNodeDup += unclonableVisitor.m_nodeCount; + chainNodeDup += clonabilityVisitor.m_nodeCount; } } diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index 753711cd279a19..3b3a35dce39489 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -606,15 +606,22 @@ struct GuardedDevirtualizationCandidateInfo : HandleHistogramProfileCandidateInf // struct InlineCandidateInfo : public GuardedDevirtualizationCandidateInfo { - CORINFO_METHOD_INFO methInfo; - CORINFO_METHOD_HANDLE ilCallerHandle; // the logical IL caller of this inlinee. + CORINFO_METHOD_INFO methInfo; + + // the logical IL caller of this inlinee. + CORINFO_METHOD_HANDLE ilCallerHandle; CORINFO_CLASS_HANDLE clsHandle; CORINFO_CONTEXT_HANDLE exactContextHnd; - GenTree* retExpr; - unsigned preexistingSpillTemp; - unsigned clsAttr; - unsigned methAttr; - IL_OFFSET ilOffset; // actual IL offset of instruction that resulted in this inline candidate + + // The GT_RET_EXPR node linking back to the inline candidate. + GenTreeRetExpr* retExpr; + + unsigned preexistingSpillTemp; + unsigned clsAttr; + unsigned methAttr; + + // actual IL offset of instruction that resulted in this inline candidate + IL_OFFSET ilOffset; CorInfoInitClassResult initClassResult; var_types fncRetType; bool exactContextNeedsRuntimeLookup; @@ -678,8 +685,6 @@ struct InlineInfo InlineResult* inlineResult; - GenTree* retExpr; // The return expression of the inlined candidate. - BasicBlock* retBB; // The basic block of the return expression of the inlined candidate. CORINFO_CLASS_HANDLE retExprClassHnd; bool retExprClassHndIsExact; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d4eee4b3d21c30..b7b4766b387d5f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5526,6 +5526,8 @@ void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) { bool inliningFailed = false; + InlineCandidateInfo* inlCandInfo = call->gtInlineCandidateInfo; + // Is this call an inline candidate? if (call->IsInlineCandidate()) { @@ -5582,6 +5584,8 @@ void Compiler::fgMorphCallInline(GenTreeCall* call, InlineResult* inlineResult) // 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()); @@ -5685,23 +5689,7 @@ void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, InlineResult* result, } lvaCount = startVars; - -#ifdef DEBUG - if (verbose) - { - // printf("Inlining failed. Restore lvaCount to %d.\n", lvaCount); - } -#endif - - return; } - -#ifdef DEBUG - if (verbose) - { - // printf("After inlining lvaCount=%d.\n", lvaCount); - } -#endif } //------------------------------------------------------------------------