Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 @@ -8287,6 +8287,8 @@ class Compiler
CORINFO_RESOLVED_TOKEN* pResolvedToken,
CORINFO_RESOLVED_TOKEN* pUnboxedResolvedToken);

bool canKeepNonInlineableGdvCandidate(GenTreeCall* call);

int getGDVMaxTypeChecks()
{
int typeChecks = JitConfig.JitGuardedDevirtualizationMaxTypeChecks();
Expand Down
150 changes: 101 additions & 49 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2157,14 +2157,15 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN

assert(retRegCount >= 2);

if (!call->CanTailCall() && !call->IsInlineCandidate())
if (!call->CanTailCall() && !call->IsInlineCandidate() && !call->IsGuardedDevirtualizationCandidate())
{
// Force a call returning multi-reg struct to be always of the IR form
// tmp = call
//
// No need to assign a multi-reg struct to a local var if:
// - It is a tail call or
// - The call is marked for in-lining later
// - The call is marked for in-lining later or
// - The call is a guarded devirtualization candidate (fate not yet known)
return impStoreMultiRegValueToVar(call, retClsHnd DEBUGARG(call->GetUnmanagedCallConv()));
}
return call;
Expand Down Expand Up @@ -9034,16 +9035,23 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* 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.
//
InlineCandidateInfo* pInfo = new (this, CMK_Inlining) InlineCandidateInfo;

pInfo->guardedMethodHandle = methodHandle;
pInfo->guardedMethodInstParamLookup = {};
pInfo->guardedMethodResolvedToken = {};
pInfo->guardedMethodUnboxedResolvedToken = {};
pInfo->guardedClassHandle = classHandle;
pInfo->likelihood = likelihood;
pInfo->exactContextHandle = contextHandle;
pInfo->originalMethodHandle = originalMethodHandle;
// Value-initialize: the candidate may be expanded without ever going through
// impCheckCanInline, so every field has to be in a well-defined state.
//
InlineCandidateInfo* pInfo = new (this, CMK_Inlining) InlineCandidateInfo{};

pInfo->guardedMethodHandle = methodHandle;
pInfo->guardedClassHandle = classHandle;
pInfo->likelihood = likelihood;
pInfo->exactContextHandle = contextHandle;
pInfo->originalMethodHandle = originalMethodHandle;
pInfo->clsAttr = classAttr;
pInfo->methAttr = methodAttr;
pInfo->preexistingSpillTemp = BAD_VAR_NUM;

// The call only carries its IL offset in debug builds, and not this early.
//
pInfo->ilOffset = BAD_IL_OFFSET;

if (instParamLookup != nullptr)
{
Expand Down Expand Up @@ -9106,6 +9114,63 @@ void Compiler::impConvertToUserCallAndMarkForInlining(GenTreeCall* call)
}
}

//------------------------------------------------------------------------
// canKeepNonInlineableGdvCandidate: check if we can keep a guarded devirtualization
// candidate whose target we're not going to inline.
//
// Arguments:
// call -- guarded devirtualization candidate
//
// Return Value:
// true if the candidate can be kept just for the sake of devirtualization.
//
// Notes:
// A direct call is cheaper than a virtual/interface call, so we keep such candidates
// by default. The bail-outs below mirror the call-site checks in
// impMarkInlineCandidateHelper; keep the two in sync.
//
bool Compiler::canKeepNonInlineableGdvCandidate(GenTreeCall* call)
{
assert(call->IsGuardedDevirtualizationCandidate());

if (JitConfig.JitGuardedDevirtualizationRequireInlining() != 0)
{
return false;
}

// Class-based GDV only for now; method-based (e.g. delegate) GDV still requires inlining.
//
if (call->GetGDVCandidateInfo(0)->guardedClassHandle == NO_CLASS_HANDLE)
{
return false;
}

// An explicit tail call has to stay a tail call, so don't perturb its shape.
// Implicit ones are fine: fgMorphPotentialTailCall can tail call out of the
// BBJ_ALWAYS blocks the expansion produces.
//
if (call->IsTailPrefixedCall())
{
return false;
}

// Except recursive ones, where turning the call into a loop is more valuable.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit odd to me. If we fail to form a GDV we won't be able to do the tail recursion->loop trick either.

//
if (call->IsImplicitTailCall() && gtIsRecursiveCall(call))
{
return false;
}

// NextCallReturnAddress needs the call to stay exactly where it is.
//
if (info.compHasNextCallRetAddr)
{
return false;
}

return true;
}

//------------------------------------------------------------------------
// impMarkInlineCandidate: determine if this call can be subsequently inlined
//
Expand All @@ -9117,8 +9182,8 @@ void Compiler::impConvertToUserCallAndMarkForInlining(GenTreeCall* call)
//
// Notes:
// Mostly a wrapper for impMarkInlineCandidateHelper that also undoes
// guarded devirtualization for virtual calls where the method we'd
// devirtualize to cannot be inlined.
// guarded devirtualization when it's not worth doing (or not legal) once
// we know we can't inline the target.

void Compiler::impMarkInlineCandidate(GenTree* callNode,
CORINFO_CONTEXT_HANDLE exactContextHnd,
Expand All @@ -9140,19 +9205,33 @@ void Compiler::impMarkInlineCandidate(GenTree* callNode,
if (call->IsGuardedDevirtualizationCandidate())
{
assert(call->GetInlineCandidatesCount() > 0);

// We usually still want to devirtualize a target we can't inline,
// see canKeepNonInlineableGdvCandidate for the exceptions.
//
const bool keepNonInlineable = canKeepNonInlineableGdvCandidate(call);

for (uint8_t candidateId = 0; candidateId < call->GetInlineCandidatesCount(); candidateId++)
{
InlineResult inlineResult(this, call, nullptr, "impMarkInlineCandidate for GDV");

// Do the actual evaluation
impMarkInlineCandidateHelper(call, candidateId, exactContextHnd, callInfo, inlinersContext, &inlineResult);
// Ignore non-inlineable candidates
// TODO: Consider keeping them to just devirtualize without inlining, at least for interface
// calls on NativeAOT, but that requires more changes elsewhere too.

if (!inlineResult.IsCandidate())
{
call->RemoveGDVCandidateInfo(this, candidateId);
candidateId--;
if (!keepNonInlineable)
{
call->RemoveGDVCandidateInfo(this, candidateId);
candidateId--;
continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the revoking dump message like we had before?

}

JITDUMP("Keeping GDV candidate %u of call [%06u] for devirtualization only: target can't be inlined\n",
candidateId, dspTreeID(call));

assert(!call->GetGDVCandidateInfo(candidateId)->isInlineable);
assert(call->GetGDVCandidateInfo(candidateId)->guardedClassHandle != NO_CLASS_HANDLE);
}
}

Expand All @@ -9170,25 +9249,6 @@ void Compiler::impMarkInlineCandidate(GenTree* callNode,
InlineResult inlineResult(this, call, nullptr, "impMarkInlineCandidate");
impMarkInlineCandidateHelper(call, 0, exactContextHnd, callInfo, inlinersContext, &inlineResult);
}

// If this call is an inline candidate or is not a guarded devirtualization
// candidate, we're done.
if (call->IsInlineCandidate() || !call->IsGuardedDevirtualizationCandidate())
{
return;
}

// If we can't inline the call we'd guardedly devirtualize to,
// we undo the guarded devirtualization, as the benefit from
// just guarded devirtualization alone is likely not worth the
// extra jit time and code size.
//
// TODO: it is possibly interesting to allow this, but requires
// fixes elsewhere too...
JITDUMP("Revoking guarded devirtualization candidacy for call [%06u]: target method can't be inlined\n",
dspTreeID(call));

call->ClearInlineInfo();
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -9477,6 +9537,8 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call,
call->SetSingleInlineCandidateInfo(inlineCandidateInfo);
}

inlineCandidateInfo->isInlineable = true;

// Let the strategy know there's another candidate.
impInlineRoot()->m_inlineStrategy->NoteCandidate();

Expand Down Expand Up @@ -11045,17 +11107,7 @@ void Compiler::impCheckCanInline(GenTreeCall* call,
}
else
{
pInfo = new (pParam->pThis, CMK_Inlining) InlineCandidateInfo;

// Null out bits we don't use when we're just inlining
//
pInfo->guardedClassHandle = nullptr;
pInfo->guardedMethodHandle = nullptr;
pInfo->guardedMethodInstParamLookup = {};
pInfo->guardedMethodResolvedToken = {};
pInfo->guardedMethodUnboxedResolvedToken = {};
pInfo->originalMethodHandle = nullptr;
pInfo->likelihood = 0;
pInfo = new (pParam->pThis, CMK_Inlining) InlineCandidateInfo{};
}

pInfo->methInfo = methInfo;
Expand Down
85 changes: 49 additions & 36 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -572,16 +572,6 @@ class IndirectCallTransformer
JITDUMP("\n----------------\n\n*** %s contemplating [%06u] in " FMT_BB " \n", Name(),
m_compiler->dspTreeID(m_origCall), m_currBlock->bbNum);

// We currently need inline candidate info to guarded devirt.
//
if (!m_origCall->IsInlineCandidate())
{
JITDUMP("*** %s Bailing on [%06u] -- not an inline candidate\n", Name(),
m_compiler->dspTreeID(m_origCall));
ClearFlag();
return;
}

m_likelihood = m_origCall->GetGDVCandidateInfo(0)->likelihood;
assert((m_likelihood >= 0) && (m_likelihood <= 100));
JITDUMP("Likelihood of correct guess is %u\n", m_likelihood);
Expand Down Expand Up @@ -848,10 +838,20 @@ class IndirectCallTransformer
}
else
{
// If there's a spill temp already associated with this inline candidate,
// use that instead of allocating a new temp.
// Only candidates that made it through impMarkInlineCandidateHelper get a
// spill temp, so candidate 0 may not be the one carrying it.
//
m_returnTemp = inlineInfo->preexistingSpillTemp;
m_returnTemp = BAD_VAR_NUM;
for (uint8_t i = 0; i < m_origCall->GetInlineCandidatesCount(); i++)
{
const unsigned spillTemp = m_origCall->GetGDVCandidateInfo(i)->preexistingSpillTemp;
if (spillTemp != BAD_VAR_NUM)
{
// Same call site, so all candidates must agree.
assert((m_returnTemp == BAD_VAR_NUM) || (m_returnTemp == spillTemp));
m_returnTemp = spillTemp;
}
}

if (m_returnTemp != BAD_VAR_NUM)
{
Expand Down Expand Up @@ -958,22 +958,6 @@ class IndirectCallTransformer
GenTreeCall* call = m_compiler->gtCloneCandidateCall(m_origCall);
call->gtArgs.GetThisArg()->SetEarlyNode(m_compiler->gtNewLclvNode(thisTemp, TYP_REF));

// If the original call was flagged as one that might inspire enumerator de-abstraction
// cloning, move the flag to the devirtualized call.
//
if (m_compiler->hasImpEnumeratorGdvLocalMap())
{
Compiler::NodeToUnsignedMap* const map = m_compiler->getImpEnumeratorGdvLocalMap();
unsigned enumeratorLcl = BAD_VAR_NUM;
if (map->Lookup(m_origCall, &enumeratorLcl))
{
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", m_compiler->dspTreeID(call),
enumeratorLcl);
map->Remove(m_origCall);
map->Set(call, enumeratorLcl);
}
}

INDEBUG(call->SetIsGuarded());

JITDUMP("Direct call [%06u] in block " FMT_BB "\n", m_compiler->dspTreeID(call), block->bbNum);
Expand Down Expand Up @@ -1022,16 +1006,26 @@ class IndirectCallTransformer
//
assert(!call->IsVirtual() && !call->IsDelegateInvoke());

// 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.
// Don't inline if the candidate was kept for devirtualization only, or if the
// devirtualizer couldn't use the unboxed entry (which invalidates the inline info).
// Either way we keep the direct call, we just don't re-mark it as a candidate.
//
CORINFO_METHOD_HANDLE unboxedMethodHnd = inlineInfo->guardedMethodUnboxedResolvedToken.hMethod;
if ((unboxedMethodHnd != nullptr) && (methodHnd != unboxedMethodHnd))
const bool unboxedEntryMismatch = (unboxedMethodHnd != nullptr) && (methodHnd != unboxedMethodHnd);

if (!inlineInfo->isInlineable || unboxedEntryMismatch)
{
// Demote this call to a non-inline candidate
//
JITDUMP("Devirtualization was unable to use the unboxed entry; so marking call (to boxed entry) as not "
"inlineable\n");
if (unboxedEntryMismatch)
{
JITDUMP("Devirtualization was unable to use the unboxed entry; so marking call (to boxed entry) as "
"not inlineable\n");
}
else
{
JITDUMP("Target of this GDV candidate is not inlineable; leaving the devirtualized call as a plain "
"direct call\n");
m_compiler->Metrics.NoInlineGDV++;
}

call->gtFlags &= ~GTF_CALL_INLINE_CANDIDATE;
call->ClearInlineInfo();
Expand All @@ -1048,6 +1042,25 @@ class IndirectCallTransformer
}
else
{
// If the original call was flagged as one that might inspire enumerator
// de-abstraction cloning, move the flag to the devirtualized call.
//
// Done here rather than right after the clone so a candidate we won't inline
// doesn't consume the mapping and hide it from one we will.
//
if (m_compiler->hasImpEnumeratorGdvLocalMap())
{
Compiler::NodeToUnsignedMap* const map = m_compiler->getImpEnumeratorGdvLocalMap();
unsigned enumeratorLcl = BAD_VAR_NUM;
if (map->Lookup(m_origCall, &enumeratorLcl))
{
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", m_compiler->dspTreeID(call),
enumeratorLcl);
map->Remove(m_origCall);
map->Set(call, enumeratorLcl);
}
}

// Add the call.
//
m_compiler->fgNewStmtAtEnd(block, call, m_stmt->GetDebugInfo());
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,11 @@ struct InlineCandidateInfo : public HandleHistogramProfileCandidateInfo
unsigned clsAttr;
unsigned methAttr;

// True if the target of this candidate can be inlined. GDV candidates are kept
// around even when it can't be, so this is what tells the two apart.
//
bool isInlineable;

CorInfoInitClassResult initClassResult;
InlineContext* inlinersContext;

Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,11 @@ RELEASE_CONFIG_INTEGER(JitEnableGuardedDevirtualization, "JitEnableGuardedDevirt
// Max number is MAX_GDV_TYPE_CHECKS defined above ^. -1 means it's up to JIT to decide
RELEASE_CONFIG_INTEGER(JitGuardedDevirtualizationMaxTypeChecks, "JitGuardedDevirtualizationMaxTypeChecks", -1)

// Whether a guarded devirtualization candidate has to be inlineable.
// 0 - keep it even if we won't inline it, a direct call is still cheaper.
// 1 - drop it if the target can't be inlined (legacy behavior).
RELEASE_CONFIG_INTEGER(JitGuardedDevirtualizationRequireInlining, "JitGuardedDevirtualizationRequireInlining", 0)

// Various policies for GuardedDevirtualization (0x4B == 75)
RELEASE_CONFIG_INTEGER(JitGuardedDevirtualizationChainLikelihood, "JitGuardedDevirtualizationChainLikelihood", 0x4B)
RELEASE_CONFIG_INTEGER(JitGuardedDevirtualizationChainStatements, "JitGuardedDevirtualizationChainStatements", 1)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/jitmetadatalist.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ JITMETADATAMETRIC(GDV, int, 0)
JITMETADATAMETRIC(ClassGDV, int, 0)
JITMETADATAMETRIC(MethodGDV, int, 0)
JITMETADATAMETRIC(MultiGuessGDV, int, 0)
JITMETADATAMETRIC(NoInlineGDV, int, 0)
JITMETADATAMETRIC(ChainedGDV, int, 0)
JITMETADATAMETRIC(EnumeratorGDV, int, 0)
JITMETADATAMETRIC(InlinerBranchFold, int, 0)
Expand Down
Loading