Skip to content

Commit

Permalink
Polish the implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
@hez2010
hez2010 committed Mar 3, 2025
1 parent 13ad9d8 commit 3e8ea8d
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 68 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/inc/corinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1512,16 +1512,16 @@ struct CORINFO_DEVIRTUALIZATION_INFO
// - If pResolvedTokenDevirtualizedMethod is not set to NULL and targeting an R2R image
// use it as the parameter to getCallInfo
// - isInstantiatingStub is set to TRUE if the devirtualized method is a generic method instantiating stub
// - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization
// (in which case the method handle and context will be a generic method)
// - hasGenericMethodHandleContext is set TRUE for cases where the method handle and context will be a generic method
// (for array interface method or generic virtual method devirtualization)
//
CORINFO_METHOD_HANDLE devirtualizedMethod;
CORINFO_CONTEXT_HANDLE exactContext;
CORINFO_DEVIRTUALIZATION_DETAIL detail;
CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod;
CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod;
bool isInstantiatingStub;
bool wasArrayInterfaceDevirt;
bool hasGenericMethodHandleContext;
};

//----------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2318,8 +2318,8 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const
bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const
{
return (IsVirtual() && gtCallType == CT_USER_FUNC) ||
(gtCallType == CT_INDIRECT && (gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) ||
gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT)));
// TODO: Support CORINFO_HELP_GVMLOOKUP_FOR_SLOT for NativeAOT
(gtCallType == CT_INDIRECT && gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR));
}

//-------------------------------------------------------------------------
Expand Down
87 changes: 49 additions & 38 deletions src/coreclr/jit/importercalls.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,6 @@ var_types Compiler::impImportCall(OPCODE opcode,
compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_MANAGED_VARARGS);
return TYP_UNDEF;
}

if ((mflags & CORINFO_FLG_VIRTUAL) && (sig->sigInst.methInstCount != 0) && (opcode == CEE_CALLVIRT))
{
compInlineResult->NoteFatal(InlineObservation::CALLEE_IS_GENERIC_VIRTUAL);
return TYP_UNDEF;
}
}

clsHnd = pResolvedToken->hClass;
Expand Down Expand Up @@ -384,12 +378,6 @@ var_types Compiler::impImportCall(OPCODE opcode,

case CORINFO_VIRTUALCALL_LDVIRTFTN:
{
if (compIsForInlining())
{
compInlineResult->NoteFatal(InlineObservation::CALLSITE_HAS_CALL_VIA_LDVIRTFTN);
return TYP_UNDEF;
}

assert(!(mflags & CORINFO_FLG_STATIC)); // can't call a static method
assert(!(clsFlags & CORINFO_FLG_VALUECLASS));
// OK, We've been told to call via LDVIRTFTN, so just
Expand All @@ -402,11 +390,20 @@ var_types Compiler::impImportCall(OPCODE opcode,
thisPtr = impTransformThis(thisPtr, pConstrainedResolvedToken, callInfo->thisTransform);
assert(thisPtr != nullptr);

GenTree* origThisPtr = thisPtr;

// Clone the (possibly transformed) "this" pointer
GenTree* thisPtrCopy;
thisPtr =
impCloneExpr(thisPtr, &thisPtrCopy, CHECK_SPILL_ALL, nullptr DEBUGARG("LDVIRTFTN this pointer"));

// We cloned the "this" pointer, mark it as a single def and set the class for it
if (thisPtr->TypeIs(TYP_REF) && (origThisPtr != thisPtr))
{
lvaGetDesc(thisPtr->AsLclVar())->lvSingleDef = 1;
lvaSetClass(thisPtr->AsLclVar()->GetLclNum(), origThisPtr);
}

GenTree* fptr = impImportLdvirtftn(thisPtr, pResolvedToken, callInfo);
assert(fptr != nullptr);

Expand All @@ -415,10 +412,6 @@ var_types Compiler::impImportCall(OPCODE opcode,

// Now make an indirect call through the function pointer

unsigned lclNum = lvaGrabTemp(true DEBUGARG("VirtualCall through function pointer"));
impStoreToTemp(lclNum, fptr, CHECK_SPILL_ALL);
fptr = gtNewLclvNode(lclNum, TYP_I_IMPL);

call->AsCall()->gtCallAddr = fptr;
call->gtFlags |= GTF_EXCEPT | (fptr->gtFlags & GTF_GLOB_EFFECT);

Expand All @@ -439,7 +432,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
// Sine we are jumping over some code, check that its OK to skip that code
assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
(sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);
goto DONE;
goto DEVIRT;
}

case CORINFO_CALL:
Expand Down Expand Up @@ -933,6 +926,8 @@ var_types Compiler::impImportCall(OPCODE opcode,
}
}

DEVIRT:

bool probing;
probing = impConsiderCallProbe(call->AsCall(), rawILOffset);

Expand Down Expand Up @@ -1232,7 +1227,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
// If the call is virtual, record the inliner's context for possible use during late devirt inlining.
// Also record the generics context if there is any.
//
if (call->AsCall()->IsVirtual() && (call->AsCall()->gtCallType != CT_INDIRECT))
if (call->AsCall()->IsDevirtualizationCandidate(this))
{
JITDUMP("\nSaving generic context %p and inline context %p for call [%06u]\n", dspPtr(exactContextHnd),
dspPtr(compInlineContext), dspTreeID(call->AsCall()));
Expand Down Expand Up @@ -1449,8 +1444,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
varDsc->lvType = call->AsCall()->gtReturnType;
}

// TODO-Bug: CHECK_SPILL_NONE here looks wrong.
impStoreToTemp(calliSlot, call, CHECK_SPILL_NONE);
impStoreToTemp(calliSlot, call, CHECK_SPILL_ALL);
// impStoreToTemp can change src arg list and return type for call that returns struct.
var_types type = genActualType(lvaTable[calliSlot].TypeGet());
call = gtNewLclvNode(calliSlot, type);
Expand Down Expand Up @@ -7136,7 +7130,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
}

addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs,
clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt,
clsAttrs, likelyHood, dvInfo.hasGenericMethodHandleContext,
dvInfo.isInstantiatingStub, originalContext);
}

Expand Down Expand Up @@ -7208,7 +7202,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,

likelyContext = dvInfo.exactContext;
likelyMethod = dvInfo.devirtualizedMethod;
arrayInterface = dvInfo.wasArrayInterfaceDevirt;
arrayInterface = dvInfo.hasGenericMethodHandleContext;
instantiatingStub = dvInfo.isInstantiatingStub;
}
else
Expand Down Expand Up @@ -8210,14 +8204,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS)
{
assert(!dvInfo.wasArrayInterfaceDevirt);
assert(!dvInfo.hasGenericMethodHandleContext);
derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK);
}
else
{
// Array interface devirt can return a nonvirtual generic method of the non-generic SZArrayHelper class.
//
assert(dvInfo.wasArrayInterfaceDevirt);
assert(dvInfo.hasGenericMethodHandleContext);
assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD);
derivedClass = info.compCompHnd->getMethodClass(derivedMethod);
}
Expand All @@ -8238,17 +8232,17 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

if (dvInfo.isInstantiatingStub)
{
// We should only end up with generic methods for array interface devirt.
// We should only end up with generic methods for array interface or GVM devirt.
//
assert(dvInfo.wasArrayInterfaceDevirt);
assert(dvInfo.hasGenericMethodHandleContext);

// We don't expect NAOT to end up here, since it has Array<T>
// and normal devirtualization.
// and normal devirtualization. For GVM devirt we don't support NAOT yet.
//
assert(!IsTargetAbi(CORINFO_NATIVEAOT_ABI));

// We don't expect R2R to end up here, since it does not (yet) support
// array interface devirtualization.
// array interface or GVM devirtualization.
//
assert(!opts.IsReadyToRun());

Expand Down Expand Up @@ -8354,26 +8348,43 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

JITDUMP(" %s; can devirtualize\n", note);

// Make the updates.
call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
call->gtFlags &= ~GTF_CALL_VIRT_STUB;
call->gtCallMethHnd = derivedMethod;
call->gtCallType = CT_USER_FUNC;
call->gtControlExpr = nullptr;
INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_DEVIRTUALIZED);

if (dvInfo.isInstantiatingStub)
{
// Pass the instantiating stub method desc as the inst param arg.
//
// Note different embedding would be needed for NAOT/R2R,
// but we have ruled those out above.
//
GenTree* const instParam =
gtNewIconEmbHndNode(instantiatingStub, nullptr, GTF_ICON_METHOD_HDL, instantiatingStub);
GenTree* instParam = nullptr;
// See if this is a generic virtual method call.
if (call->gtCallType == CT_INDIRECT && call->gtCallAddr->IsHelperCall(this, CORINFO_HELP_VIRTUAL_FUNC_PTR))
{
// We need to pass the RUNTIMELOOKUP helper call as the inst param arg
// instead of the instantiating stub if there is one.
//
GenTree* const methHndNode =
call->gtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetEarlyNode();
if (methHndNode->OperIs(GT_RUNTIMELOOKUP))
{
instParam = methHndNode;
}
}
// For cases where we don't have a RUNTIMELOOKUP helper call, we pass the instantiating stub.
if (instParam == nullptr)
{
instParam = gtNewIconEmbHndNode(instantiatingStub, nullptr, GTF_ICON_METHOD_HDL, instantiatingStub);
}
call->gtArgs.InsertInstParam(this, instParam);
}

// Make the updates.
call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
call->gtFlags &= ~GTF_CALL_VIRT_STUB;
call->gtCallMethHnd = derivedMethod;
call->gtCallType = CT_USER_FUNC;
call->gtControlExpr = nullptr;
INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_DEVIRTUALIZED);

// Virtual calls include an implicit null check, which we may
// now need to make explicit.
if (!objIsNonNull)
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/inline.def
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ INLINE_OBSERVATION(HAS_NO_BODY, bool, "has no body",
INLINE_OBSERVATION(HAS_NULL_FOR_LDELEM, bool, "has null pointer for ldelem", FATAL, CALLEE)
INLINE_OBSERVATION(HAS_UNMANAGED_CALLCONV, bool, "has unmanaged calling convention", FATAL, CALLEE)
INLINE_OBSERVATION(IS_ARRAY_METHOD, bool, "is array method", FATAL, CALLEE)
INLINE_OBSERVATION(IS_GENERIC_VIRTUAL, bool, "generic virtual", FATAL, CALLEE)
INLINE_OBSERVATION(IS_JIT_NOINLINE, bool, "noinline per JitNoinline", FATAL, CALLEE)
INLINE_OBSERVATION(IS_NOINLINE, bool, "noinline per IL/cached result", FATAL, CALLEE)
INLINE_OBSERVATION(IS_SYNCHRONIZED, bool, "is synchronized", FATAL, CALLEE)
Expand Down Expand Up @@ -134,7 +133,6 @@ INLINE_OBSERVATION(COMPILATION_ERROR, bool, "compilation error",
INLINE_OBSERVATION(COMPILATION_FAILURE, bool, "failed to compile", FATAL, CALLSITE)
INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX, bool, "explicit tail prefix", FATAL, CALLSITE)
INLINE_OBSERVATION(GENERIC_DICTIONARY_LOOKUP, bool, "runtime dictionary lookup", FATAL, CALLSITE)
INLINE_OBSERVATION(HAS_CALL_VIA_LDVIRTFTN, bool, "call via ldvirtftn", FATAL, CALLSITE)
INLINE_OBSERVATION(HAS_COMPLEX_HANDLE, bool, "complex handle access", FATAL, CALLSITE)
INLINE_OBSERVATION(IMPLICIT_REC_TAIL_CALL, bool, "implicit recursive tail call", FATAL, CALLSITE)
INLINE_OBSERVATION(IS_CALL_TO_HELPER, bool, "target is helper", FATAL, CALLSITE)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
info->exactContext = null;
info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_UNKNOWN;
info->isInstantiatingStub = false;
info->wasArrayInterfaceDevirt = false;
info->hasGenericMethodHandleContext = false;

TypeDesc objType = HandleToObject(info->objClass);

Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,8 +1082,8 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO
// - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table.
// - detail describes the computation done by the jit host
// - isInstantiatingStub is set to TRUE if the devirtualized method is a method instantiation stub
// - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization
// (in which case the method handle and context will be a generic method)
// - hasGenericMethodHandleContext is set TRUE for cases where the method handle and context will be a generic method
// (for array interface method or generic virtual method devirtualization)
//
public CORINFO_METHOD_STRUCT_* devirtualizedMethod;
public CORINFO_CONTEXT_STRUCT* exactContext;
Expand All @@ -1092,8 +1092,8 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO
public CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod;
public byte _isInstantiatingStub;
public bool isInstantiatingStub { get { return _isInstantiatingStub != 0; } set { _isInstantiatingStub = value ? (byte)1 : (byte)0; } }
public byte _wasArrayInterfaceDevirt;
public bool wasArrayInterfaceDevirt { get { return _wasArrayInterfaceDevirt != 0; } set { _wasArrayInterfaceDevirt = value ? (byte)1 : (byte)0; } }
public byte _hasGenericMethodHandleContext;
public bool hasGenericMethodHandleContext { get { return _hasGenericMethodHandleContext != 0; } set { _hasGenericMethodHandleContext = value ? (byte)1 : (byte)0; } }
}

//----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/superpmi/superpmi-shared/agnostic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ struct Agnostic_ResolveVirtualMethodResult
bool returnValue;
DWORDLONG devirtualizedMethod;
bool isInstantiatingStub;
bool wasArrayInterfaceDevirt;
bool hasGenericMethodHandleContext;
DWORDLONG exactContext;
DWORD detail;
Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod;
Expand Down
18 changes: 9 additions & 9 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3293,12 +3293,12 @@ void MethodContext::recResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info
key.pResolvedTokenVirtualMethod = SpmiRecordsHelper::StoreAgnostic_CORINFO_RESOLVED_TOKEN(info->pResolvedTokenVirtualMethod, ResolveToken);

Agnostic_ResolveVirtualMethodResult result;
result.returnValue = returnValue;
result.devirtualizedMethod = CastHandle(info->devirtualizedMethod);
result.isInstantiatingStub = info->isInstantiatingStub;
result.exactContext = CastHandle(info->exactContext);
result.detail = (DWORD) info->detail;
result.wasArrayInterfaceDevirt = info->wasArrayInterfaceDevirt;
result.returnValue = returnValue;
result.devirtualizedMethod = CastHandle(info->devirtualizedMethod);
result.isInstantiatingStub = info->isInstantiatingStub;
result.exactContext = CastHandle(info->exactContext);
result.detail = (DWORD)info->detail;
result.hasGenericMethodHandleContext = info->hasGenericMethodHandleContext;

if (returnValue)
{
Expand All @@ -3323,11 +3323,11 @@ void MethodContext::dmpResolveVirtualMethod(const Agnostic_ResolveVirtualMethodK
key.context,
key.pResolvedTokenVirtualMethodNonNull,
key.pResolvedTokenVirtualMethodNonNull ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.pResolvedTokenVirtualMethod).c_str() : "???");
printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", instantiatingStub-%s, wasArrayInterfaceDevirt-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}",
printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", instantiatingStub-%s, hasGenericMethodHandleContext-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}",
result.returnValue ? "true" : "false",
result.devirtualizedMethod,
result.isInstantiatingStub ? "true" : "false",
result.wasArrayInterfaceDevirt ? "true" : "false",
result.hasGenericMethodHandleContext ? "true" : "false",
result.exactContext,
result.detail,
result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedMethod).c_str() : "???",
Expand All @@ -3352,7 +3352,7 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info

info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod;
info->isInstantiatingStub = result.isInstantiatingStub;
info->wasArrayInterfaceDevirt = result.wasArrayInterfaceDevirt;
info->hasGenericMethodHandleContext = result.hasGenericMethodHandleContext;
info->exactContext = (CORINFO_CONTEXT_HANDLE) result.exactContext;
info->detail = (CORINFO_DEVIRTUALIZATION_DETAIL) result.detail;
if (result.returnValue)
Expand Down
Loading

0 comments on commit 3e8ea8d

Please sign in to comment.