Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Enable inlining for late devirtualization #110827

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5109,6 +5109,8 @@ class Compiler
SpillCliqueSucc
};

friend class SubstitutePlaceholdersAndDevirtualizeWalker;

// Abstract class for receiving a callback while walking a spill clique
class SpillCliqueWalker
{
Expand Down
88 changes: 75 additions & 13 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ bool Compiler::TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, i

class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<SubstitutePlaceholdersAndDevirtualizeWalker>
{
bool m_madeChanges = false;
bool m_madeChanges = false;
Statement* m_curStmt = nullptr;

public:
enum
Expand All @@ -219,11 +220,28 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
{
}

bool MadeChanges()
bool MadeChanges() const
{
return m_madeChanges;
}

// ------------------------------------------------------------------------
// WalkStatement: Walk the tree of a statement, and return the newly added
// statement if any, otherwise return the original statement.
//
// Arguments:
// stmt - the statement to walk.
//
// Return Value:
// The newly added statement if any, or the original statement.
//
Statement* WalkStatement(Statement* stmt)
{
m_curStmt = stmt;
WalkTree(m_curStmt->GetRootNodePointer(), nullptr);
return m_curStmt;
}

fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
{
GenTree* tree = *use;
Expand Down Expand Up @@ -586,8 +604,52 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
}

CORINFO_CONTEXT_HANDLE contextInput = context;
context = nullptr;
m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
isLateDevirtualization, explicitTailCall);
if (context != nullptr)
{
#if defined(DEBUG)
IL_OFFSET ilOffset = call->gtRawILOffset;
#else
IL_OFFSET ilOffset = m_curStmt->GetDebugInfo().GetLocation().GetOffset();
hez2010 marked this conversation as resolved.
Show resolved Hide resolved
#endif // defined(DEBUG)
CORINFO_CALL_INFO callInfo = {};
callInfo.hMethod = method;
callInfo.methodFlags = methodFlags;
m_compiler->impMarkInlineCandidate(call, context, false, &callInfo, ilOffset);

if (call->IsInlineCandidate())
{
CallArg* retBuffer = call->gtArgs.GetRetBufferArg();

if (parent != nullptr || genActualType(call->TypeGet()) != TYP_VOID || retBuffer != nullptr)
{
Statement* stmt = m_compiler->gtNewStmt(call);
m_compiler->fgInsertStmtBefore(m_compiler->compCurBB, m_curStmt, stmt);
GenTreeRetExpr* retExpr =
m_compiler->gtNewInlineCandidateReturnExpr(call->AsCall(),
genActualType(call->TypeGet()));

call->GetSingleInlineCandidateInfo()->retExpr = retExpr;

m_curStmt = stmt;
*pTree = retExpr;
}

if (retBuffer != nullptr)
{
call->GetSingleInlineCandidateInfo()->preexistingSpillTemp =
retBuffer->GetNode()->AsLclVarCommon()->GetLclNum();
hez2010 marked this conversation as resolved.
Show resolved Hide resolved
}

call->GetSingleInlineCandidateInfo()->exactContextHandle = context;
INDEBUG(call->GetSingleInlineCandidateInfo()->inlinersContext = call->gtInlineContext);

JITDUMP("New inline candidate due to late devirtualization:\n");
DISPTREE(call);
}
}
m_madeChanges = true;
}
}
Expand Down Expand Up @@ -730,17 +792,10 @@ PhaseStatus Compiler::fgInline()
do
{
// Make the current basic block address available globally
compCurBB = block;

for (Statement* const stmt : block->Statements())
compCurBB = block;
Statement* stmt = block->firstStmt();
while (stmt != nullptr)
{

#if defined(DEBUG)
// 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.
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.
//
Expand All @@ -755,7 +810,7 @@ PhaseStatus Compiler::fgInline()
// possible further optimization, as the (now complete) GT_RET_EXPR
// replacement may have enabled optimizations by providing more
// specific types for trees or variables.
walker.WalkTree(stmt->GetRootNodePointer(), nullptr);
stmt = walker.WalkStatement(stmt);

GenTree* expr = stmt->GetRootNode();

Expand Down Expand Up @@ -805,6 +860,13 @@ PhaseStatus Compiler::fgInline()
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();
}

block = block->Next();
Expand Down
25 changes: 21 additions & 4 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
(sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);

call = gtNewIndCallNode(stubAddr, callRetTyp);
call = gtNewIndCallNode(stubAddr, callRetTyp, di);

call->gtFlags |= GTF_EXCEPT | (stubAddr->gtFlags & GTF_GLOB_EFFECT);
call->gtFlags |= GTF_CALL_VIRT_STUB;
Expand Down Expand Up @@ -1027,7 +1027,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
if (!bIntrinsicImported)
{
// Keep track of the raw IL offset of the call
INDEBUG(call->AsCall()->gtRawILOffset = rawILOffset);
call->AsCall()->gtRawILOffset = rawILOffset;

// Is it an inline candidate?
impMarkInlineCandidate(call, exactContextHnd, exactContextNeedsRuntimeLookup, callInfo, rawILOffset);
Expand Down Expand Up @@ -1237,7 +1237,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
}

// Keep track of the raw IL offset of the call
INDEBUG(call->AsCall()->gtRawILOffset = rawILOffset);
call->AsCall()->gtRawILOffset = rawILOffset;

// Is it an inline candidate?
impMarkInlineCandidate(call, exactContextHnd, exactContextNeedsRuntimeLookup, callInfo, rawILOffset);
Expand Down Expand Up @@ -7695,6 +7695,19 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call,

if (!(methAttr & CORINFO_FLG_FORCEINLINE))
{
if (compCurBB->HasFlag(BBF_INTERNAL) && ehGetBlockHndDsc(compCurBB) != nullptr)
{
#ifdef DEBUG
if (verbose)
{
printf("\nWill not inline blocks that have EH descriptor when IL offset may not be valid\n");
}

#endif
inlineResult->NoteFatal(InlineObservation::CALLSITE_NOT_CANDIDATE);
return;
}

/* Don't bother inline blocks that are in the filter region */
if (bbInCatchHandlerILRange(compCurBB))
{
Expand Down Expand Up @@ -9201,9 +9214,13 @@ void Compiler::impCheckCanInline(GenTreeCall* call,
CORINFO_CLASS_HANDLE clsHandle = compCompHnd->getMethodClass(ftn);
unsigned const clsAttr = compCompHnd->getClassAttribs(clsHandle);

CallArg* pRetBuf = pParam->call->gtArgs.GetRetBufferArg();

// Return type
//
var_types const fncRetType = pParam->call->TypeGet();
var_types const fncRetType =
pRetBuf == nullptr ? pParam->call->TypeGet()
: pParam->pThis->lvaGetRealType(pRetBuf->GetEarlyNode()->AsLclVarCommon()->GetLclNum());
hez2010 marked this conversation as resolved.
Show resolved Hide resolved

#ifdef DEBUG
var_types fncRealRetType = JITtype2varType(methInfo.args.retType);
Expand Down
Loading