Skip to content

Commit

Permalink
Use proper context in initClass for GDV (#87847)
Browse files Browse the repository at this point in the history 
Co-authored-by: David Wrighton <davidwr@microsoft.com>
  • Loading branch information
@EgorBo @davidwrighton
EgorBo and davidwrighton committed Jul 26, 2023
1 parent d389ab9 commit 7ebf967
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 40 deletions.
13 changes: 7 additions & 6 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7181,12 +7181,13 @@ class Compiler

bool isCompatibleMethodGDV(GenTreeCall* call, CORINFO_METHOD_HANDLE gdvTarget);

void addGuardedDevirtualizationCandidate(GenTreeCall* call,
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
unsigned methodAttr,
unsigned classAttr,
unsigned likelihood);
void addGuardedDevirtualizationCandidate(GenTreeCall* call,
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
CORINFO_CONTEXT_HANDLE contextHandle,
unsigned methodAttr,
unsigned classAttr,
unsigned likelihood);

int getGDVMaxTypeChecks()
{
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/fginline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
// info.
}

m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr,
CORINFO_CONTEXT_HANDLE contextInput = context;
m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
isLateDevirtualization, explicitTailCall);
m_madeChanges = true;
}
Expand Down
35 changes: 23 additions & 12 deletions src/coreclr/jit/importercalls.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5990,8 +5990,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
likelyHood += 100 - likelyHood * numExactClasses;
}

addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactMethodAttrs, clsAttrs,
likelyHood);
addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, dvInfo.exactContext, exactMethodAttrs,
clsAttrs, likelyHood);
}

if (call->GetInlineCandidatesCount() == numExactClasses)
Expand All @@ -6018,6 +6018,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
CORINFO_METHOD_HANDLE likelyMethod = likelyMethodes[candidateId];
unsigned likelihood = likelihoods[candidateId];

CORINFO_CONTEXT_HANDLE likelyContext = NULL;

uint32_t likelyClassAttribs = 0;
if (likelyClass != NO_CLASS_HANDLE)
{
Expand Down Expand Up @@ -6053,7 +6055,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
break;
}

likelyMethod = dvInfo.devirtualizedMethod;
likelyContext = dvInfo.exactContext;
likelyMethod = dvInfo.devirtualizedMethod;
}

uint32_t likelyMethodAttribs = info.compCompHnd->getMethodAttribs(likelyMethod);
Expand Down Expand Up @@ -6121,8 +6124,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,

// Add this as a potential candidate.
//
addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyMethodAttribs, likelyClassAttribs,
likelihood);
addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs,
likelyClassAttribs, likelihood);
}
}

Expand All @@ -6147,12 +6150,13 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
// classAttr - attributes of the class
// likelihood - odds that this class is the class seen at runtime
//
void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call,
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
unsigned methodAttr,
unsigned classAttr,
unsigned likelihood)
void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call,
CORINFO_METHOD_HANDLE methodHandle,
CORINFO_CLASS_HANDLE classHandle,
CORINFO_CONTEXT_HANDLE contextHandle,
unsigned methodAttr,
unsigned classAttr,
unsigned likelihood)
{
// This transformation only makes sense for delegate and virtual calls
assert(call->IsDelegateInvoke() || call->IsVirtual());
Expand Down Expand Up @@ -6225,6 +6229,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call,
pInfo->guardedClassHandle = classHandle;
pInfo->likelihood = likelihood;
pInfo->requiresInstMethodTableArg = false;
pInfo->exactContextHnd = contextHandle;

// If the guarded class is a value class, look for an unboxed entry point.
//
Expand Down Expand Up @@ -6286,8 +6291,14 @@ void Compiler::impMarkInlineCandidate(GenTree* callNode,
{
InlineResult inlineResult(this, call, nullptr, "impMarkInlineCandidate for GDV");

CORINFO_CONTEXT_HANDLE moreExactContext = call->GetGDVCandidateInfo(candidateId)->exactContextHnd;
if (moreExactContext == NULL)
{
moreExactContext = exactContextHnd;
}

// Do the actual evaluation
impMarkInlineCandidateHelper(call, candidateId, exactContextHnd, exactContextNeedsRuntimeLookup, callInfo,
impMarkInlineCandidateHelper(call, candidateId, moreExactContext, exactContextNeedsRuntimeLookup, callInfo,
ilOffset, &inlineResult);
// Ignore non-inlineable candidates
// TODO: Consider keeping them to just devirtualize without inlining, at least for interface
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/indirectcalltransformer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ class IndirectCallTransformer

JITDUMP("Direct call [%06u] in block " FMT_BB "\n", compiler->dspTreeID(call), block->bbNum);

CORINFO_METHOD_HANDLE methodHnd = call->gtCallMethHnd;
CORINFO_METHOD_HANDLE methodHnd = inlineInfo->guardedMethodHandle;
CORINFO_CONTEXT_HANDLE context = inlineInfo->exactContextHnd;
if (clsHnd != NO_CLASS_HANDLE)
{
Expand All @@ -872,7 +872,8 @@ class IndirectCallTransformer
unsigned methodFlags = compiler->info.compCompHnd->getMethodAttribs(methodHnd);
const bool isLateDevirtualization = true;
const bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0;
compiler->impDevirtualizeCall(call, nullptr, &methodHnd, &methodFlags, &context, nullptr,
CORINFO_CONTEXT_HANDLE contextInput = context;
compiler->impDevirtualizeCall(call, nullptr, &methodHnd, &methodFlags, &contextInput, &context,
isLateDevirtualization, explicitTailCall);
}
else
Expand Down
32 changes: 19 additions & 13 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,10 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
}

MethodDesc decl = HandleToObject(info->virtualMethod);

// Transform from the unboxing thunk to the normal method
decl = decl.IsUnboxingThunk() ? decl.GetUnboxedMethod() : decl;

Debug.Assert(!decl.HasInstantiation);

if ((info->context != null) && decl.OwningType.IsInterface)
Expand Down Expand Up @@ -1369,7 +1373,6 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
#endif
);
}
info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN);
}
else
{
Expand All @@ -1382,26 +1385,29 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
, methodWithTokenImpl
#endif
);
}

if (unboxingStub)
{
info->resolvedTokenDevirtualizedUnboxedMethod = info->resolvedTokenDevirtualizedMethod;
info->resolvedTokenDevirtualizedUnboxedMethod.tokenContext = contextFromMethod(nonUnboxingImpl);
info->resolvedTokenDevirtualizedUnboxedMethod.hMethod = ObjectToHandle(nonUnboxingImpl);
}
else
{
info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN);
}
if (unboxingStub)
{
info->resolvedTokenDevirtualizedUnboxedMethod = info->resolvedTokenDevirtualizedMethod;
info->resolvedTokenDevirtualizedUnboxedMethod.tokenContext = contextFromMethod(nonUnboxingImpl);
info->resolvedTokenDevirtualizedUnboxedMethod.hMethod = ObjectToHandle(nonUnboxingImpl);
}
else
{
info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN);
}

#if READYTORUN
// Testing has not shown that concerns about virtual matching are significant
// Only generate verification for builds with the stress mode enabled
if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
{
ISymbolNode virtualResolutionNode = _compilation.SymbolNodeFactory.CheckVirtualFunctionOverride(methodWithTokenDecl, objType, methodWithTokenImpl);
AddPrecodeFixup(virtualResolutionNode);
if (!methodWithTokenDecl.Method.OwningType.IsValueType || !methodWithTokenImpl.Method.OwningType.IsValueType)
{
ISymbolNode virtualResolutionNode = _compilation.SymbolNodeFactory.CheckVirtualFunctionOverride(methodWithTokenDecl, objType, methodWithTokenImpl);
AddPrecodeFixup(virtualResolutionNode);
}
}
#endif
info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_SUCCESS;
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1015,12 +1015,6 @@ CorInfoInitClassResult MethodContext::repInitClass(CORINFO_FIELD_HANDLE field,
key.method = CastHandle(method);
key.context = CastHandle(context);

if ((InitClass == nullptr) || (InitClass->GetIndex(key) == -1))
{
// We could try additional inlines with stress modes, just reject them.
return CORINFO_INITCLASS_DONT_INLINE;
}

DWORD value = InitClass->Get(key);
DEBUG_REP(dmpInitClass(key, value));
CorInfoInitClassResult result = (CorInfoInitClassResult)value;
Expand Down

0 comments on commit 7ebf967

Please sign in to comment.