Skip to content

Commit

Permalink
Substitute GT_RET_EXPR in inline candidate arguments (#69117)
Browse files Browse the repository at this point in the history 
Also rename GTF_CALL_M_LATE_DEVIRT -> GTF_CALL_M_HAS_LATE_DEVIRT_INFO
  • Loading branch information
@jakobbotsch
jakobbotsch authored May 13, 2022
1 parent 88fb966 commit 4ecd060
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 90 deletions.
97 changes: 50 additions & 47 deletions src/coreclr/jit/fginline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,22 @@ PhaseStatus Compiler::fgInline()
// candidate stage. So scan the tree looking for those early failures.
fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
#endif
// See if we need to replace some return value place holders.
// Also, see if this replacement enables further devirtualization.
//
// Note we have both preorder and postorder callbacks here.
//
// 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.
fgWalkTree(stmt->GetRootNodePointer(), fgUpdateInlineReturnExpressionPlaceHolder, fgLateDevirtualization,
&madeChanges);

GenTree* expr = stmt->GetRootNode();

Expand Down Expand Up @@ -162,23 +178,6 @@ PhaseStatus Compiler::fgInline()
}
}

// See if we need to replace some return value place holders.
// Also, see if this replacement enables further devirtualization.
//
// Note we have both preorder and postorder callbacks here.
//
// 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.
fgWalkTree(stmt->GetRootNodePointer(), fgUpdateInlineReturnExpressionPlaceHolder, fgLateDevirtualization,
(void*)&madeChanges);

// See if stmt is of the form GT_COMMA(call, nop)
// If yes, we can get rid of GT_COMMA.
if (expr->OperGet() == GT_COMMA && expr->AsOp()->gtOp1->OperGet() == GT_CALL &&
Expand Down Expand Up @@ -492,7 +491,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
Compiler* comp = data->compiler;
CORINFO_CLASS_HANDLE retClsHnd = NO_CLASS_HANDLE;

while (tree->OperGet() == GT_RET_EXPR)
while (tree->OperIs(GT_RET_EXPR))
{
// We are going to copy the tree from the inlinee,
// so record the handle now.
Expand All @@ -505,15 +504,21 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
// Skip through chains of GT_RET_EXPRs (say from nested inlines)
// to the actual tree to use.
//
// Also 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.
BasicBlockFlags bbFlags;
GenTree* inlineCandidate = tree;
do
{
GenTreeRetExpr* retExpr = inlineCandidate->AsRetExpr();
inlineCandidate = retExpr->gtInlineCandidate;
bbFlags = retExpr->bbFlags;
} 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.
//
BasicBlockFlags bbFlags = BBF_EMPTY;
GenTree* inlineCandidate = tree->gtRetExprVal(&bbFlags);
inlineCandidate = comp->gtFoldExpr(inlineCandidate);
var_types retType = tree->TypeGet();
inlineCandidate = comp->gtFoldExpr(inlineCandidate);
var_types retType = tree->TypeGet();

#ifdef DEBUG
if (comp->verbose)
Expand Down Expand Up @@ -716,10 +721,14 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
const bool isLateDevirtualization = true;
const bool explicitTailCall = call->IsTailPrefixedCall();

if ((call->gtCallMoreFlags & GTF_CALL_M_LATE_DEVIRT) != 0)
if ((call->gtCallMoreFlags & GTF_CALL_M_HAS_LATE_DEVIRT_INFO) != 0)
{
context = call->gtLateDevirtualizationInfo->exactContextHnd;
call->gtLateDevirtualizationInfo = nullptr;
context = call->gtLateDevirtualizationInfo->exactContextHnd;
// Note: we might call this multiple times for the same trees.
// If the devirtualization below succeeds, the call becomes
// non-virtual and we won't get here again. If it does not
// succeed we might get here again so we keep the late devirt
// info.
}

comp->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, isLateDevirtualization,
Expand Down Expand Up @@ -1537,8 +1546,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
bool argHasPutArg = !varTypeIsStruct(arg->GetSignatureType()) &&
(genTypeSize(argNode) != genTypeSize(arg->GetSignatureType()));

BasicBlockFlags bbFlags = BBF_EMPTY;
argNode = argNode->gtRetExprVal(&bbFlags);
assert(!argNode->OperIs(GT_RET_EXPR));

if (argInfo.argHasTmp)
{
Expand Down Expand Up @@ -1601,7 +1609,6 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
}
#endif // DEBUG
}
block->bbFlags |= (bbFlags & BBF_SPLIT_GAINED);
}
else if (argInfo.argIsByRefToStructLocal)
{
Expand Down Expand Up @@ -1649,39 +1656,37 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
// This helper call is marked as a "Special DCE" helper during
// importation, over in fgGetStaticsCCtorHelper.
//
// (2) NYI. If, after tunneling through GT_RET_VALs, we find that
// the actual arg expression has no side effects, we can skip
// appending all together. This will help jit TP a bit.
// (2) NYI. If we find that the actual arg expression
// has no side effects, we can skip appending all
// together. This will help jit TP a bit.
//
// Chase through any GT_RET_EXPRs to find the actual argument
// expression.
GenTree* actualArgNode = argNode->gtRetExprVal(&bbFlags);
assert(!argNode->OperIs(GT_RET_EXPR));

// For case (1)
//
// Look for the following tree shapes
// prejit: (IND (ADD (CONST, CALL(special dce helper...))))
// jit : (COMMA (CALL(special dce helper...), (FIELD ...)))
if (actualArgNode->gtOper == GT_COMMA)
if (argNode->gtOper == GT_COMMA)
{
// Look for (COMMA (CALL(special dce helper...), (FIELD ...)))
GenTree* op1 = actualArgNode->AsOp()->gtOp1;
GenTree* op2 = actualArgNode->AsOp()->gtOp2;
GenTree* op1 = argNode->AsOp()->gtOp1;
GenTree* op2 = argNode->AsOp()->gtOp2;
if (op1->IsCall() &&
((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
(op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0))
{
JITDUMP("\nPerforming special dce on unused arg [%06u]:"
" actual arg [%06u] helper call [%06u]\n",
argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
argNode->gtTreeID, argNode->gtTreeID, op1->gtTreeID);
// Drop the whole tree
append = false;
}
}
else if (actualArgNode->gtOper == GT_IND)
else if (argNode->gtOper == GT_IND)
{
// Look for (IND (ADD (CONST, CALL(special dce helper...))))
GenTree* addr = actualArgNode->AsOp()->gtOp1;
GenTree* addr = argNode->AsOp()->gtOp1;

if (addr->gtOper == GT_ADD)
{
Expand All @@ -1694,7 +1699,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
// Drop the whole tree
JITDUMP("\nPerforming special dce on unused arg [%06u]:"
" actual arg [%06u] helper call [%06u]\n",
argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
argNode->gtTreeID, argNode->gtTreeID, op1->gtTreeID);
append = false;
}
}
Expand Down Expand Up @@ -1731,8 +1736,6 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
// since the box itself will be ignored.
gtTryRemoveBoxUpstreamEffects(argNode);
}

block->bbFlags |= (bbFlags & BBF_SPLIT_GAINED);
}
}
}
Expand Down
36 changes: 0 additions & 36 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6161,42 +6161,6 @@ GenTree* GenTree::gtGetParent(GenTree*** pUse)
return user;
}

//-------------------------------------------------------------------------
// gtRetExprVal - walk back through GT_RET_EXPRs
//
// Arguments:
// pbbFlags - out-parameter that is set to the flags of the basic block
// containing the inlinee return value. The value is 0
// for unsuccessful inlines.
//
// Returns:
// tree representing return value from a successful inline,
// or original call for failed or yet to be determined inline.
//
// Notes:
// Multi-level inlines can form chains of GT_RET_EXPRs.
// This method walks back to the root of the chain.
//
GenTree* GenTree::gtRetExprVal(BasicBlockFlags* pbbFlags /* = nullptr */)
{
GenTree* retExprVal = this;
BasicBlockFlags bbFlags = BBF_EMPTY;

while (retExprVal->OperIs(GT_RET_EXPR))
{
const GenTreeRetExpr* retExpr = retExprVal->AsRetExpr();
bbFlags = retExpr->bbFlags;
retExprVal = retExpr->gtInlineCandidate;
}

if (pbbFlags != nullptr)
{
*pbbFlags = bbFlags;
}

return retExprVal;
}

//------------------------------------------------------------------------------
// OperRequiresAsgFlag : Check whether the operation requires GTF_ASG flag regardless
// of the children's flags.
Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1762,9 +1762,6 @@ struct GenTree

inline GenTree* gtCommaAssignVal();

// Tunnel through any GT_RET_EXPRs
GenTree* gtRetExprVal(BasicBlockFlags* pbbFlags = nullptr);

// Return the child of this node if it is a GT_RELOAD or GT_COPY; otherwise simply return the node itself
inline GenTree* gtSkipReloadOrCopy();

Expand Down Expand Up @@ -3751,7 +3748,7 @@ enum GenTreeCallFlags : unsigned int
GTF_CALL_M_EXP_RUNTIME_LOOKUP = 0x02000000, // this call needs to be tranformed into CFG for the dynamic dictionary expansion feature.
GTF_CALL_M_STRESS_TAILCALL = 0x04000000, // the call is NOT "tail" prefixed but GTF_CALL_M_EXPLICIT_TAILCALL was added because of tail call stress mode
GTF_CALL_M_EXPANDED_EARLY = 0x08000000, // the Virtual Call target address is expanded and placed in gtControlExpr in Morph rather than in Lower
GTF_CALL_M_LATE_DEVIRT = 0x10000000, // this call has late devirtualzation info
GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x10000000, // this call has late devirtualzation info
};

inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a)
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10557,7 +10557,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
(origCall->gtClassProfileCandidateInfo == nullptr))
{
JITDUMP("\nSaving context %p for call [%06u]\n", exactContextHnd, dspTreeID(origCall));
origCall->gtCallMoreFlags |= GTF_CALL_M_LATE_DEVIRT;
origCall->gtCallMoreFlags |= GTF_CALL_M_HAS_LATE_DEVIRT_INFO;
LateDevirtualizationInfo* const info = new (this, CMK_Inlining) LateDevirtualizationInfo;
info->exactContextHnd = exactContextHnd;
origCall->gtLateDevirtualizationInfo = info;
Expand Down Expand Up @@ -20097,7 +20097,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo,
inlCurArgInfo->arg = arg;
GenTree* curArgVal = arg->GetNode();

curArgVal = curArgVal->gtRetExprVal();
assert(!curArgVal->OperIs(GT_RET_EXPR));

if (curArgVal->gtOper == GT_MKREFANY)
{
Expand Down Expand Up @@ -20693,7 +20693,8 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In
const var_types lclTyp = lclInfo.lclTypeInfo;
GenTree* op1 = nullptr;

GenTree* argNode = argInfo.arg->GetNode()->gtRetExprVal();
GenTree* argNode = argInfo.arg->GetNode();
assert(!argNode->OperIs(GT_RET_EXPR));

if (argInfo.argIsInvariant && !argCanBeModified)
{
Expand Down Expand Up @@ -21824,6 +21825,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,
// it's a union field used for other things by virtual
// stubs)
call->gtInlineCandidateInfo = nullptr;
call->gtCallMoreFlags &= ~GTF_CALL_M_HAS_LATE_DEVIRT_INFO;

#if defined(DEBUG)
if (verbose)
Expand Down

0 comments on commit 4ecd060

Please sign in to comment.