From 6120decd08fec33f84f0251b28ac1e1a91f75a6b Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 15 Apr 2026 21:09:39 +0900 Subject: [PATCH 1/5] Part 1: add tests and instParamLookup refactor --- src/coreclr/inc/corinfo.h | 9 +- src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/jit/compiler.h | 15 +- src/coreclr/jit/importercalls.cpp | 199 +++++++++--------- src/coreclr/jit/morph.cpp | 19 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 29 ++- .../tools/Common/JitInterface/CorInfoTypes.cs | 10 +- .../tools/superpmi/superpmi-shared/agnostic.h | 3 +- .../superpmi-shared/methodcontext.cpp | 13 +- src/coreclr/vm/jitinterface.cpp | 40 ++-- .../VirtualMethods/generic_virtual_methods.cs | 182 +++++++++++++--- 11 files changed, 316 insertions(+), 213 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 4dacf6cfad171b..3968964e2ead48 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1153,6 +1153,7 @@ struct CORINFO_CONST_LOOKUP // IAT_PVALUE --> "addr" stores a pointer to a location which will hold the real handle // IAT_RELPVALUE --> "addr" stores a relative pointer to a location which will hold the real handle // IAT_PPVALUE --> "addr" stores a double indirection to a location which will hold the real handle + // IAT_VALUE with a nullptr handle indicates no constant lookup is needed InfoAccessType accessType; union @@ -1600,17 +1601,15 @@ struct CORINFO_DEVIRTUALIZATION_INFO // - details on the computation done by the jit host // - 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 - // - needsMethodContext is set TRUE if the devirtualized method may require a method context - // (in which case the method handle and context will be a generic method) + // - instParamLookup contains all the information necessary to pass the instantiation parameter for + // the devirtualized method. // CORINFO_METHOD_HANDLE devirtualizedMethod; CORINFO_CONTEXT_HANDLE exactContext; CORINFO_DEVIRTUALIZATION_DETAIL detail; CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; - bool isInstantiatingStub; - bool needsMethodContext; + CORINFO_LOOKUP instParamLookup; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index c87fdefeed1272..9816fd7d1744fa 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -37,11 +37,11 @@ #include -constexpr GUID JITEEVersionIdentifier = { /* 89e70385-3f0d-4fbd-9270-0425c0db321b */ - 0x89e70385, - 0x3f0d, - 0x4fbd, - {0x92, 0x70, 0x04, 0x25, 0xc0, 0xdb, 0x32, 0x1b} +constexpr GUID JITEEVersionIdentifier = { /* fc5f63e7-921b-4091-b920-8df8d7b872c1 */ + 0xfc5f63e7, + 0x921b, + 0x4091, + {0xb9, 0x20, 0x8d, 0xf8, 0xd7, 0xb8, 0x72, 0xc1} }; #endif // JIT_EE_VERSIONING_GUID_H diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index edadb7d14f7bfc..f010a7458bfe3e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6600,13 +6600,11 @@ class Compiler GenTree* fgCreateCallDispatcherAndGetResult(GenTreeCall* origCall, CORINFO_METHOD_HANDLE callTargetStubHnd, CORINFO_METHOD_HANDLE dispatcherHnd); - GenTree* getLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_LOOKUP* pLookup, - GenTreeFlags handleFlags, - void* compileTimeHandle); - GenTree* getRuntimeLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_LOOKUP* pLookup, - void* compileTimeHandle); + GenTree* getLookupTree(CORINFO_LOOKUP* pLookup, + GenTreeFlags handleFlags, + void* compileTimeHandle); + GenTree* getRuntimeLookupTree(CORINFO_LOOKUP* pLookup, + void* compileTimeHandle); GenTree* getVirtMethodPointerTree(GenTree* thisPtr, CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_CALL_INFO* pCallInfo); @@ -7501,8 +7499,7 @@ class Compiler unsigned methodAttr, unsigned classAttr, unsigned likelihood, - bool arrayInterface, - bool instantiatingStub, + CORINFO_LOOKUP instParamLookup, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 9ac0c3848c6260..8642dff306dcfb 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7676,8 +7676,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, } addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, - clsAttrs, likelyHood, dvInfo.needsMethodContext, - dvInfo.isInstantiatingStub, baseMethod, originalContext); + clsAttrs, likelyHood, dvInfo.instParamLookup, baseMethod, + originalContext); } if (call->GetInlineCandidatesCount() == numExactClasses) @@ -7700,11 +7700,14 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Iterate over the guesses for (int candidateId = 0; candidateId < candidatesCount; candidateId++) { - CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; - CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; - unsigned likelihood = likelihoods[candidateId]; - bool needsMethodContext = false; - bool instantiatingStub = false; + CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; + CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; + unsigned likelihood = likelihoods[candidateId]; + + CORINFO_LOOKUP instParamLookup{}; + instParamLookup.lookupKind.needsRuntimeLookup = false; + instParamLookup.constLookup.accessType = IAT_VALUE; + instParamLookup.constLookup.handle = nullptr; CORINFO_CONTEXT_HANDLE likelyContext = originalContext; @@ -7747,10 +7750,9 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, break; } - likelyContext = dvInfo.exactContext; - likelyMethod = dvInfo.devirtualizedMethod; - needsMethodContext = dvInfo.needsMethodContext; - instantiatingStub = dvInfo.isInstantiatingStub; + likelyContext = dvInfo.exactContext; + likelyMethod = dvInfo.devirtualizedMethod; + instParamLookup = dvInfo.instParamLookup; } else { @@ -7825,8 +7827,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Add this as a potential candidate. // addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs, - likelyClassAttribs, likelihood, needsMethodContext, instantiatingStub, - baseMethod, originalContext); + likelyClassAttribs, likelihood, instParamLookup, baseMethod, + originalContext); } } @@ -7851,8 +7853,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // methodAttr - attributes of the method // classAttr - attributes of the class // likelihood - odds that this class is the class seen at runtime -// needsMethodContext - devirtualized method may need generic method context (e.g. array interfaces) -// instantiatingStub - devirtualized method in an instantiating stub +// instParamLookup - the lookup information required by instantiation param // originalMethodHandle - method handle of base method (before devirt) // originalContextHandle - context for the original call // @@ -7863,8 +7864,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, unsigned methodAttr, unsigned classAttr, unsigned likelihood, - bool needsMethodContext, - bool instantiatingStub, + CORINFO_LOOKUP instParamLookup, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle) { @@ -7919,9 +7919,22 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, // We're all set, proceed with candidate creation. // + assert(!instParamLookup.lookupKind.needsRuntimeLookup); + const bool needsMethodContext = ((size_t)contextHandle & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; + CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; + if (!(instParamLookup.constLookup.accessType == IAT_VALUE && instParamLookup.constLookup.handle == nullptr)) + { + assert(needsMethodContext); + instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)contextHandle & ~CORINFO_CONTEXTFLAGS_MASK); + assert(instantiatingStub != NO_METHOD_HANDLE); + } + JITDUMP("Marking call [%06u] as guarded devirtualization candidate; will guess for %s %s\n", dspTreeID(call), classHandle != NO_CLASS_HANDLE ? "class" : "method", - classHandle != NO_CLASS_HANDLE ? eeGetClassName(classHandle) : eeGetMethodFullName(methodHandle)); + classHandle != NO_CLASS_HANDLE + ? eeGetClassName(classHandle) + : eeGetMethodFullName(instantiatingStub != NO_METHOD_HANDLE ? instantiatingStub : methodHandle)); + setMethodHasGuardedDevirtualization(); // Spill off any GT_RET_EXPR subtrees so we can clone the call. @@ -7946,20 +7959,11 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, // If the guarded method is an instantiating stub, find the instantiated method // - if (instantiatingStub) + if (instantiatingStub != NO_METHOD_HANDLE) { - JITDUMP(" ... method is an instantiating stub, looking for instantiated entry\n"); - CORINFO_CLASS_HANDLE ignoredClass = NO_CLASS_HANDLE; - CORINFO_METHOD_HANDLE ignoredMethod = NO_METHOD_HANDLE; - CORINFO_METHOD_HANDLE instantiatedMethod = - info.compCompHnd->getInstantiatedEntry(methodHandle, &ignoredMethod, &ignoredClass); - assert(ignoredClass == NO_CLASS_HANDLE); - - if (instantiatedMethod != NO_METHOD_HANDLE) - { - JITDUMP(" ... updating GDV candidate with instantiated entry info\n"); - pInfo->guardedMethodInstantiatedEntryHandle = instantiatedMethod; - } + JITDUMP(" ... updating GDV candidate with instantiated entry info\n"); + pInfo->guardedMethodHandle = instantiatingStub; + pInfo->guardedMethodInstantiatedEntryHandle = methodHandle; } // If the guarded class is a value class, look for an unboxed entry point. @@ -8780,25 +8784,23 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, const bool objClassIsFinal = (objClassAttribs & CORINFO_FLG_FINAL) != 0; #if defined(DEBUG) - const char* callKind = isInterface ? "interface" : "virtual"; - const char* objClassNote = "[?]"; - const char* objClassName = "?objClass"; - const char* baseClassName = "?baseClass"; - const char* baseMethodName = "?baseMethod"; + const char* callKind = isInterface ? "interface" : "virtual"; + const char* objClassNote = "[?]"; + const char* objClassName = "?objClass"; + const char* baseMethodFullName = "?baseMethod"; if (verbose || doPrint) { - objClassNote = isExact ? " [exact]" : objClassIsFinal ? " [final]" : ""; - objClassName = eeGetClassName(objClass); - baseClassName = eeGetClassName(baseClass); - baseMethodName = eeGetMethodName(baseMethod); + objClassNote = isExact ? " [exact]" : objClassIsFinal ? " [final]" : ""; + objClassName = eeGetClassName(objClass); + baseMethodFullName = eeGetMethodFullName(baseMethod); if (verbose) { printf("\nimpDevirtualizeCall: Trying to devirtualize %s call:\n" " class for 'this' is %s%s (attrib %08x)\n" - " base method is %s::%s\n", - callKind, objClassName, objClassNote, objClassAttribs, baseClassName, baseMethodName); + " base method is %s\n", + callKind, objClassName, objClassNote, objClassAttribs, baseMethodFullName); } } #endif // defined(DEBUG) @@ -8847,10 +8849,15 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, info.compCompHnd->resolveVirtualMethod(&dvInfo); - CORINFO_METHOD_HANDLE derivedMethod = dvInfo.devirtualizedMethod; - CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; - CORINFO_CLASS_HANDLE derivedClass = NO_CLASS_HANDLE; - CORINFO_RESOLVED_TOKEN* pDerivedResolvedToken = &dvInfo.resolvedTokenDevirtualizedMethod; + CORINFO_METHOD_HANDLE derivedMethod = dvInfo.devirtualizedMethod; + CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; + CORINFO_CLASS_HANDLE derivedClass = NO_CLASS_HANDLE; + CORINFO_RESOLVED_TOKEN* pDerivedResolvedToken = &dvInfo.resolvedTokenDevirtualizedMethod; + const bool needsRuntimeLookup = dvInfo.instParamLookup.lookupKind.needsRuntimeLookup; + const bool isArrayInterfaceDevirt = (objClassAttribs & CORINFO_FLG_ARRAY) != 0; + + const bool needsInstParam = needsRuntimeLookup || !(dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && + dvInfo.instParamLookup.constLookup.handle == nullptr); if (derivedMethod != nullptr) { @@ -8858,7 +8865,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS) { - assert(!dvInfo.needsMethodContext); derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); } else @@ -8866,7 +8872,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // Array interface devirt can return a nonvirtual generic method of the non-generic SZArrayHelper class. // Generic virtual method devirt also returns a generic method. // - assert(call->IsGenericVirtual(this) || dvInfo.needsMethodContext); assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); derivedClass = info.compCompHnd->getMethodClass(derivedMethod); } @@ -8877,54 +8882,36 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, bool canDevirtualize = false; #if defined(DEBUG) - const char* derivedClassName = "?derivedClass"; - const char* derivedMethodName = "?derivedMethod"; - const char* note = "inexact or not final"; - const char* instArg = ""; + const char* derivedMethodFullName = "?derivedMethod"; + const char* note = "inexact or not final"; + const char* instArg = ""; #endif - CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; + CORINFO_METHOD_HANDLE instParam = NO_METHOD_HANDLE; - if (dvInfo.isInstantiatingStub) + if (derivedMethod != nullptr && needsInstParam && !needsRuntimeLookup) { - // We should only end up with generic methods that needs a method context (eg. array interface, GVM). + // We should only end up with generic methods that need a method context (eg. array interface, GVM). // - assert(dvInfo.needsMethodContext); - - // We don't expect NAOT to end up here, since it has Array - // and normal devirtualization. - // - assert(!IsTargetAbi(CORINFO_NATIVEAOT_ABI)); - - // We don't expect R2R to end up here, since it does not (yet) support - // array interface devirtualization. - // - assert(!IsAot()); - - // We don't expect there to be an existing inst param arg. - // - CallArg* const instParam = call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam); - if (instParam != nullptr) - { - assert(!"unexpected inst param in virtual/interface call"); - return; - } + assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); // If we don't know the array type exactly we may have the wrong interface type here. // Bail out. // - if (!isExact) + if (isArrayInterfaceDevirt && !isExact) { JITDUMP("Array interface devirt: array type is inexact, sorry.\n"); return; } - // We want to inline the instantiating stub. Fetch the relevant info. - // - CORINFO_CLASS_HANDLE ignored = NO_CLASS_HANDLE; - derivedMethod = info.compCompHnd->getInstantiatedEntry(derivedMethod, &instantiatingStub, &ignored); - assert(ignored == NO_CLASS_HANDLE); - assert((derivedMethod == NO_METHOD_HANDLE) || (instantiatingStub != NO_METHOD_HANDLE)); + // We don't expect R2R/NAOT to end up here for array interface devirtualization. + // For NAOT, it has Array and normal devirtualization. + // For R2R, we don't (yet) support array interface devirtualization. + assert(call->IsGenericVirtual(this) || !IsAot()); + + instParam = (CORINFO_METHOD_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); + + assert(instParam != NO_METHOD_HANDLE); } // If we failed to get a method handle, we can't directly devirtualize. @@ -8955,18 +8942,18 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, { note = "final method"; } - if (dvInfo.isInstantiatingStub) + if (needsInstParam) { - instArg = " [instantiating stub]"; + instArg = needsRuntimeLookup ? "runtime lookup" : eeGetMethodFullName(instParam); } if (verbose || doPrint) { - derivedMethodName = eeGetMethodName(derivedMethod); - derivedClassName = eeGetClassName(derivedClass); + derivedMethodFullName = eeGetMethodFullName(derivedMethod); if (verbose) { - printf(" devirt to %s::%s -- %s%s\n", derivedClassName, derivedMethodName, note, instArg); + printf(" devirt to %s -- %s%s%s\n", derivedMethodFullName, note, + needsInstParam ? ", instantiation: " : "", instArg); gtDispTree(call); } } @@ -9005,6 +8992,29 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, return; } + // Insert the instantiation argument when necessary. + if (needsInstParam) + { + assert(call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam) == nullptr); + + CORINFO_METHOD_HANDLE compileTimeHandle = derivedMethod; + if (!needsRuntimeLookup) + { + compileTimeHandle = instParam; + } + + GenTree* instParam = getLookupTree(&dvInfo.instParamLookup, GTF_ICON_METHOD_HDL, compileTimeHandle); + + if (instParam == nullptr) + { + // If we're inlining, impLookupToTree can return nullptr after recording a fatal observation. + JITDUMP("Failed to produce the lookup for devirtualized call, sorry.\n"); + return; + } + + call->gtArgs.InsertInstParam(this, instParam); + } + // All checks done. Time to transform the call. // assert(canDevirtualize); @@ -9012,17 +9022,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, JITDUMP(" %s; can devirtualize\n", note); - 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 = gtNewIconEmbMethHndNode(instantiatingStub); - call->gtArgs.InsertInstParam(this, instParam); - } - // Make the updates. call->gtFlags &= ~GTF_CALL_VIRT_VTABLE; call->gtFlags &= ~GTF_CALL_VIRT_STUB; @@ -9052,8 +9051,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (doPrint) { - printf("Devirtualized %s call to %s:%s; now direct call to %s:%s [%s]\n", callKind, baseClassName, - baseMethodName, derivedClassName, derivedMethodName, note); + printf("Devirtualized %s call to %s; now direct call to %s [%s]%s%s\n", callKind, baseMethodFullName, + derivedMethodFullName, note, needsInstParam ? ", instantiation: " : "", instArg); } // If we successfully devirtualized based on an exact or final class, diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index e3e9250a9ff61b..5e839d13765c42 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5489,7 +5489,6 @@ GenTree* Compiler::fgCreateCallDispatcherAndGetResult(GenTreeCall* orig // getLookupTree: get a lookup tree // // Arguments: -// pResolvedToken - resolved token of the call // pLookup - the lookup to get the tree for // handleFlags - flags to set on the result node // compileTimeHandle - compile-time handle corresponding to the lookup @@ -5497,10 +5496,7 @@ GenTree* Compiler::fgCreateCallDispatcherAndGetResult(GenTreeCall* orig // Return Value: // A node representing the lookup tree // -GenTree* Compiler::getLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_LOOKUP* pLookup, - GenTreeFlags handleFlags, - void* compileTimeHandle) +GenTree* Compiler::getLookupTree(CORINFO_LOOKUP* pLookup, GenTreeFlags handleFlags, void* compileTimeHandle) { if (!pLookup->lookupKind.needsRuntimeLookup) { @@ -5523,26 +5519,21 @@ GenTree* Compiler::getLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, return gtNewIconEmbHndNode(handle, pIndirection, handleFlags, compileTimeHandle); } - return getRuntimeLookupTree(pResolvedToken, pLookup, compileTimeHandle); + return getRuntimeLookupTree(pLookup, compileTimeHandle); } //------------------------------------------------------------------------ // getRuntimeLookupTree: get a tree for a runtime lookup // // Arguments: -// pResolvedToken - resolved token of the call // pLookup - the lookup to get the tree for // compileTimeHandle - compile-time handle corresponding to the lookup // // Return Value: // A node representing the runtime lookup tree // -GenTree* Compiler::getRuntimeLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_LOOKUP* pLookup, - void* compileTimeHandle) +GenTree* Compiler::getRuntimeLookupTree(CORINFO_LOOKUP* pLookup, void* compileTimeHandle) { - assert(!compIsForInlining()); - CORINFO_RUNTIME_LOOKUP* pRuntimeLookup = &pLookup->runtimeLookup; // If pRuntimeLookup->indirections is equal to CORINFO_USEHELPER, it specifies that a run-time helper should be @@ -5663,8 +5654,8 @@ GenTree* Compiler::getTokenHandleTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, bo // info.compCompHnd->embedGenericHandle(pResolvedToken, parent, info.compMethodHnd, &embedInfo); - GenTree* result = getLookupTree(pResolvedToken, &embedInfo.lookup, gtTokenToIconFlags(pResolvedToken->token), - embedInfo.compileTimeHandle); + GenTree* result = + getLookupTree(&embedInfo.lookup, gtTokenToIconFlags(pResolvedToken->token), embedInfo.compileTimeHandle); // If we have a result and it requires runtime lookup, wrap it in a runtime lookup node. if ((result != nullptr) && embedInfo.lookup.lookupKind.needsRuntimeLookup) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index bcc358cb7a791c..3bcdaf5b2e91d0 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1346,8 +1346,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) info->devirtualizedMethod = null; info->exactContext = null; info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_UNKNOWN; - info->isInstantiatingStub = false; - info->needsMethodContext = false; + info->instParamLookup = default(CORINFO_LOOKUP); TypeDesc objType = HandleToObject(info->objClass); @@ -1478,6 +1477,29 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN); } + bool isArrayInterfaceDevirtualization = objType.IsArray && decl.OwningType.IsInterface; + bool isGenericVirtual = decl.HasInstantiation; + + if (isGenericVirtual && originalImpl.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + + if (!info->instParamLookup.lookupKind.needsRuntimeLookup && + (isArrayInterfaceDevirtualization || isGenericVirtual) && + impl.IsCanonicalMethod(CanonicalFormKind.Specific)) + { +#if READYTORUN + MethodWithToken originalImplWithToken = new MethodWithToken(originalImpl, methodWithTokenImpl.Token, null, false, null, null); + info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.MethodHandle, originalImplWithToken)); +#else + // TODO: Implement generic virtual method devirtualization constant lookup for NativeAOT + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; +#endif + } + #if READYTORUN // Testing has not shown that concerns about virtual matching are significant // Only generate verification for builds with the stress mode enabled @@ -1492,8 +1514,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) #endif info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_SUCCESS; info->devirtualizedMethod = ObjectToHandle(impl); - info->isInstantiatingStub = false; - info->exactContext = contextFromType(owningType); + info->exactContext = (isArrayInterfaceDevirtualization || isGenericVirtual) ? contextFromMethod(originalImpl) : contextFromType(owningType); return true; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index 4ba39d65fefc2c..40f78254be56e8 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1183,19 +1183,15 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO // invariant is `resolveVirtualMethod(...) == (devirtualizedMethod != nullptr)`. // - 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 - // - needsMethodContext is set TRUE if the devirtualized method may require a method context - // (in which case the method handle and context will be a generic method) + // - instParamLookup contains all the information necessary to pass the instantiation parameter for + // the devirtualized method. // public CORINFO_METHOD_STRUCT_* devirtualizedMethod; public CORINFO_CONTEXT_STRUCT* exactContext; public CORINFO_DEVIRTUALIZATION_DETAIL detail; public CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; public CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; - public byte _isInstantiatingStub; - public bool isInstantiatingStub { get { return _isInstantiatingStub != 0; } set { _isInstantiatingStub = value ? (byte)1 : (byte)0; } } - public byte _needsMethodContext; - public bool needsMethodContext { get { return _needsMethodContext != 0; } set { _needsMethodContext = value ? (byte)1 : (byte)0; } } + public CORINFO_LOOKUP instParamLookup; } //---------------------------------------------------------------------------- diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index 7dc7ca9fcf5211..dca2b5affd6ec7 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -686,12 +686,11 @@ struct Agnostic_ResolveVirtualMethodResult { bool returnValue; DWORDLONG devirtualizedMethod; - bool isInstantiatingStub; - bool needsMethodContext; DWORDLONG exactContext; DWORD detail; Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; + Agnostic_CORINFO_LOOKUP instParamLookup; }; struct Agnostic_GetInstantiatedEntryResult diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index fada9c7b16cbfa..197acfb04f57b4 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -3245,10 +3245,9 @@ void MethodContext::recResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info 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.needsMethodContext = info->needsMethodContext; + result.instParamLookup = SpmiRecordsHelper::StoreAgnostic_CORINFO_LOOKUP(&info->instParamLookup); if (returnValue) { @@ -3273,15 +3272,14 @@ 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, needsMethodContext-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}", + printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}, instParamLookup{%s}", result.returnValue ? "true" : "false", result.devirtualizedMethod, - result.isInstantiatingStub ? "true" : "false", - result.needsMethodContext ? "true" : "false", result.exactContext, result.detail, result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedMethod).c_str() : "???", - result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedUnboxedMethod).c_str() : "???"); + result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedUnboxedMethod).c_str() : "???", + SpmiDumpHelper::DumpAgnostic_CORINFO_LOOKUP(result.instParamLookup).c_str()); } bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info) @@ -3301,10 +3299,9 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info DEBUG_REP(dmpResolveVirtualMethod(key, result)); info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod; - info->isInstantiatingStub = result.isInstantiatingStub; - info->needsMethodContext = result.needsMethodContext; info->exactContext = (CORINFO_CONTEXT_HANDLE) result.exactContext; info->detail = (CORINFO_DEVIRTUALIZATION_DETAIL) result.detail; + info->instParamLookup = SpmiRecordsHelper::RestoreCORINFO_LOOKUP(result.instParamLookup); if (result.returnValue) { info->resolvedTokenDevirtualizedMethod = SpmiRecordsHelper::Restore_CORINFO_RESOLVED_TOKEN(&result.resolvedTokenDevirtualizedMethod, ResolveToken); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ea24d0728a173f..5cb35de41135b7 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3016,7 +3016,7 @@ void CEEInfo::ComputeRuntimeLookupForSharedGenericToken(DictionaryEntryKind entr MethodDesc* pContextMD = pCallerMD; MethodTable* pContextMT = pContextMD->GetMethodTable(); - // There is a pathological case where invalid IL refereces __Canon type directly, but there is no dictionary availabled to store the lookup. + // There is a pathological case where invalid IL references __Canon type directly, but there is no dictionary available to store the lookup. if (!pContextMD->IsSharedByGenericInstantiations()) COMPlusThrow(kInvalidProgramException); @@ -8590,8 +8590,9 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) info->detail = CORINFO_DEVIRTUALIZATION_UNKNOWN; memset(&info->resolvedTokenDevirtualizedMethod, 0, sizeof(info->resolvedTokenDevirtualizedMethod)); memset(&info->resolvedTokenDevirtualizedUnboxedMethod, 0, sizeof(info->resolvedTokenDevirtualizedUnboxedMethod)); - info->isInstantiatingStub = false; - info->needsMethodContext = false; + info->instParamLookup.lookupKind.needsRuntimeLookup = false; + info->instParamLookup.constLookup.accessType = IAT_VALUE; + info->instParamLookup.constLookup.handle = NULL; MethodDesc* pBaseMD = GetMethod(info->virtualMethod); MethodTable* pBaseMT = pBaseMD->GetMethodTable(); @@ -8827,12 +8828,9 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // This is generic virtual method devirtualization. if (!isArray && pBaseMD->HasMethodInstantiation()) { - pDevirtMD = pDevirtMD->FindOrCreateAssociatedMethodDesc( - pDevirtMD, pExactMT, pExactMT->IsValueType() && !pDevirtMD->IsStatic(), pBaseMD->GetMethodInstantiation(), true); - - // We still can't handle shared generic methods because we don't have - // the right generic context for runtime lookup. - // TODO: Remove this limitation. + MethodDesc* pPrimaryMD = pDevirtMD; + pDevirtMD = MethodDesc::FindOrCreateAssociatedMethodDesc( + pPrimaryMD, pExactMT, pExactMT->IsValueType() && !pPrimaryMD->IsStatic(), pBaseMD->GetMethodInstantiation(), true); if (pDevirtMD->IsSharedByGenericMethodInstantiations()) { info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; @@ -8842,32 +8840,24 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) isGenericVirtual = true; } - // Success! Pass back the results. - // - if (isArray) + if (!info->instParamLookup.lookupKind.needsRuntimeLookup && (isArray || isGenericVirtual) && pDevirtMD->IsInstantiatingStub()) { - // Note if array devirtualization produced an instantiation stub - // so jit can try and inline it. - // - info->isInstantiatingStub = pDevirtMD->IsInstantiatingStub(); - info->exactContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD); - info->needsMethodContext = true; + info->instParamLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE)pDevirtMD; + info->instParamLookup.constLookup.accessType = IAT_VALUE; } - else if (isGenericVirtual) + + if (isArray || isGenericVirtual) { - // We don't support shared generic methods yet so this should always be false - info->needsMethodContext = false; - // We don't produce an instantiating stub - info->isInstantiatingStub = false; info->exactContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD); + pDevirtMD = pDevirtMD->IsInstantiatingStub() ? pDevirtMD->GetWrappedMethodDesc() : pDevirtMD; } else { info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); - info->isInstantiatingStub = false; - info->needsMethodContext = false; } + // Success! Pass back the results. + // info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD; info->detail = CORINFO_DEVIRTUALIZATION_SUCCESS; diff --git a/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs b/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs index c971d07f273a4e..7aba5abe26bab3 100644 --- a/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs +++ b/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs @@ -3,6 +3,7 @@ // using System; +using System.Collections.Generic; using System.Runtime.CompilerServices; using Xunit; @@ -137,6 +138,12 @@ public static void GenericInterfaceBase_GenericStructDerived_NoInliningVariants( ValidateCaller("GenericInterfaceBase_GenericStructDerived_NoInlining_String_String", new GenericInterfaceBaseCaller(new GenericInterfaceBase_GenericStructDerived_NoInlining())); } + [Fact] + public static void RuntimeLookupDelegate() + { + RuntimeLookupDelegateGenericVirtual.Test(); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void ValidateCaller(string scenarioName, IBaseMethodCaller caller) { @@ -199,6 +206,45 @@ private static void Equal(T expected, T actual, [CallerArgumentExpression(nam } } +internal class RuntimeLookupDelegateGenericVirtual +{ + internal static readonly List s_list = new(); + + internal static void Test() + { + var test = new Base(); + test.Foo>(); + + var test2 = new Derived(); + Delegate m1 = test2.Foo>(); + Delegate m2 = test2.Foo>>; + Assert.Equal(m1, m2); + } +} + +internal class Base +{ + public virtual Delegate Foo() + { + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(U)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>>>)); + return Foo; + } +} + +internal class Derived : Base +{ + public override Delegate Foo() + { + return Foo>; + } +} + internal static class IconContextBridgeNonShared { public static TMethod SameMethodSameClass(IBaseMethodCaller caller, TMethod value) @@ -355,96 +401,164 @@ public static TMethod DifferentClassDifferentMethod(IBaseMethodCaller caller, TM internal static class RuntimeLookupDispatcher { + [MethodImpl(MethodImplOptions.NoInlining)] public static TMethod SameClassSameMethod(IBaseMethodCaller caller, TMethod value) { - return RuntimeLookupThunks.InvokeSameClassSameMethod(caller, value); + RuntimeLookupVirtualInvoker invoker = new RuntimeLookupVirtualStage(); + return invoker.SameClassSameMethod(caller, value); } + [MethodImpl(MethodImplOptions.NoInlining)] public static TMethod SameClassDifferentMethod(IBaseMethodCaller caller, TMethod value) { - return RuntimeLookupThunks.InvokeSameClassDifferentMethod(caller, value); + return SameClassDifferentMethodCore(caller, value); } + [MethodImpl(MethodImplOptions.NoInlining)] public static TMethod DifferentClassSameMethod(IBaseMethodCaller caller, TMethod value) { - return RuntimeLookupThunks.InvokeDifferentClassSameMethod(caller, value); + return RuntimeLookupDifferentClass.SameMethod(caller, value); } + [MethodImpl(MethodImplOptions.NoInlining)] public static TMethod DifferentClassDifferentMethod(IBaseMethodCaller caller, TMethod value) { - return RuntimeLookupThunks.InvokeDifferentClassDifferentMethod(caller, value); + return RuntimeLookupDifferentClass.DifferentMethod(caller, value); } -} -internal static class RuntimeLookupThunks -{ - public static T InvokeSameClassSameMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + private static TMethod SameClassDifferentMethodCore(IBaseMethodCaller caller, TMethod value) { - return RuntimeLookupHost.SameClassSameMethod(caller, value); + RuntimeLookupVirtualInvoker invoker = new RuntimeLookupVirtualStage(); + return invoker.SameClassDifferentMethod(caller, value); } +} - public static T InvokeDifferentClassSameMethod(IBaseMethodCaller caller, T value) +internal static class RuntimeLookupDifferentClass +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static T SameMethod(IBaseMethodCaller caller, T value) { - return RuntimeLookupHost.DifferentClassSameMethod(caller, value); + RuntimeLookupVirtualInvoker invoker = new RuntimeLookupVirtualStage(); + return invoker.DifferentClassSameMethod(caller, value); } - public static T InvokeSameClassDifferentMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public static T DifferentMethod(IBaseMethodCaller caller, T value) { - return RuntimeLookupHost.SameClassDifferentMethod(caller, value); + return DifferentMethodCore(caller, value); } - public static T InvokeDifferentClassDifferentMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + private static T DifferentMethodCore(IBaseMethodCaller caller, T value) { - return RuntimeLookupHost.DifferentClassDifferentMethod(caller, value); + RuntimeLookupVirtualInvoker invoker = new RuntimeLookupVirtualStage(); + return invoker.DifferentClassDifferentMethod(caller, value); } } -internal static class RuntimeLookupHost +internal abstract class RuntimeLookupVirtualInvoker +{ + public abstract T SameClassSameMethod(IBaseMethodCaller caller, T value); + public abstract T SameClassDifferentMethod(IBaseMethodCaller caller, T value); + public abstract T DifferentClassSameMethod(IBaseMethodCaller caller, T value); + public abstract T DifferentClassDifferentMethod(IBaseMethodCaller caller, T value); +} + +internal sealed class RuntimeLookupVirtualStage : RuntimeLookupVirtualInvoker { - public static T SameClassSameMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T SameClassSameMethod(IBaseMethodCaller caller, T value) { - return caller.Invoke(value); + RuntimeLookupVirtualInvoker invoker = RuntimeLookupTerminalFactory.CreateInvoker(); + return invoker.SameClassSameMethod(caller, value); } - public static T DifferentClassSameMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T SameClassDifferentMethod(IBaseMethodCaller caller, T value) { - return RuntimeLookupRemote.SameMethod(caller, value); + return SameClassDifferentMethodCore(caller, value); } - public static T SameClassDifferentMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T DifferentClassSameMethod(IBaseMethodCaller caller, T value) { - return SameClassDifferentMethodCore(caller, value); + RuntimeLookupVirtualInvoker invoker = RuntimeLookupTerminalFactory.CreateInvoker(); + return invoker.DifferentClassSameMethod(caller, value); } - public static T DifferentClassDifferentMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T DifferentClassDifferentMethod(IBaseMethodCaller caller, T value) { - return RuntimeLookupRemote.DifferentMethod(caller, value); + return DifferentClassDifferentMethodCore(caller, value); } + [MethodImpl(MethodImplOptions.NoInlining)] private static T SameClassDifferentMethodCore(IBaseMethodCaller caller, T value) { - return caller.Invoke(value); + RuntimeLookupVirtualInvoker invoker = RuntimeLookupTerminalFactory.CreateInvoker(); + return invoker.SameClassDifferentMethod(caller, value); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static T DifferentClassDifferentMethodCore(IBaseMethodCaller caller, T value) + { + RuntimeLookupVirtualInvoker invoker = RuntimeLookupTerminalFactory.CreateInvoker(); + return invoker.DifferentClassDifferentMethod(caller, value); + } +} + +internal static class RuntimeLookupTerminalFactory +{ + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static RuntimeLookupVirtualInvoker CreateInvoker() + { + return new RuntimeLookupTerminalInvoker(); } } -internal static class RuntimeLookupRemote +internal sealed class RuntimeLookupTerminalInvoker : RuntimeLookupVirtualInvoker { - public static T SameMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T SameClassSameMethod(IBaseMethodCaller caller, T value) { return caller.Invoke(value); } - public static T DifferentMethod(IBaseMethodCaller caller, T value) + [MethodImpl(MethodImplOptions.NoInlining)] + public override T SameClassDifferentMethod(IBaseMethodCaller caller, T value) { - return RemoteInner.Invoke(caller, value); + return SameClassDifferentMethodCore(caller, value); } - private static class RemoteInner + [MethodImpl(MethodImplOptions.NoInlining)] + public override T DifferentClassSameMethod(IBaseMethodCaller caller, T value) { - public static T Invoke(IBaseMethodCaller caller, T value) - { - return caller.Invoke(value); - } + return caller.Invoke(value); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public override T DifferentClassDifferentMethod(IBaseMethodCaller caller, T value) + { + return DifferentClassDifferentMethodCore(caller, value); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static T SameClassDifferentMethodCore(IBaseMethodCaller caller, T value) + { + return caller.Invoke(value); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static T DifferentClassDifferentMethodCore(IBaseMethodCaller caller, T value) + { + return DifferentClassDifferentMethodCoreInner.Invoke(caller, value); + } + + private static class DifferentClassDifferentMethodCoreInner + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static T Invoke(IBaseMethodCaller caller, T value) => caller.Invoke(value); } } From e7acb94820894c4b173544a18093cebb191f79b7 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 25 Apr 2026 21:27:59 +0900 Subject: [PATCH 2/5] Address review feedbacks --- src/coreclr/jit/importercalls.cpp | 6 +++--- src/coreclr/vm/jitinterface.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index aef12563545036..4d977ba50d93e6 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -9130,16 +9130,16 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, compileTimeHandle = instParam; } - GenTree* instParam = getLookupTree(&dvInfo.instParamLookup, GTF_ICON_METHOD_HDL, compileTimeHandle); + GenTree* instParamNode = getLookupTree(&dvInfo.instParamLookup, GTF_ICON_METHOD_HDL, compileTimeHandle); - if (instParam == nullptr) + if (instParamNode == nullptr) { // If we're inlining, impLookupToTree can return nullptr after recording a fatal observation. JITDUMP("Failed to produce the lookup for devirtualized call, sorry.\n"); return; } - call->gtArgs.InsertInstParam(this, instParam); + call->gtArgs.InsertInstParam(this, instParamNode); } // All checks done. Time to transform the call. diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2a7d001bdd29f1..7053ced5402660 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8615,6 +8615,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) info->detail = CORINFO_DEVIRTUALIZATION_UNKNOWN; memset(&info->resolvedTokenDevirtualizedMethod, 0, sizeof(info->resolvedTokenDevirtualizedMethod)); memset(&info->resolvedTokenDevirtualizedUnboxedMethod, 0, sizeof(info->resolvedTokenDevirtualizedUnboxedMethod)); + memset(&info->instParamLookup, 0, sizeof(info->instParamLookup)); info->instParamLookup.lookupKind.needsRuntimeLookup = false; info->instParamLookup.constLookup.accessType = IAT_VALUE; info->instParamLookup.constLookup.handle = NULL; From eeb97f82201130a14171693c7b1b249150c6b8ef Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 25 Apr 2026 21:59:06 +0900 Subject: [PATCH 3/5] Update src/coreclr/inc/corinfo.h Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/inc/corinfo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index c630b73409ab56..2b619dffe3bcc8 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1598,7 +1598,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO // - If pResolvedTokenDevirtualizedMethod is not set to NULL and targeting an R2R image // use it as the parameter to getCallInfo // - instParamLookup contains all the information necessary to pass the instantiation parameter for - // the devirtualized method. IAT_VALUE with a nullptr handle indicates no instantiation parameter is needed. + // the devirtualized method. A constant lookup with IAT_VALUE with a nullptr handle indicates no instantiation parameter is needed. // CORINFO_METHOD_HANDLE devirtualizedMethod; CORINFO_CONTEXT_HANDLE exactContext; From 7dfc4f39e7f7ebb8b05f10be14de59bae8609eac Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 25 Apr 2026 22:00:27 +0900 Subject: [PATCH 4/5] Use hasTypeArg instead of leaking CORINFO_LOOKUP --- src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/importercalls.cpp | 65 +++++++++++++-------- src/coreclr/jit/indirectcalltransformer.cpp | 2 +- src/coreclr/jit/inline.h | 2 +- 4 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index fa35c10846a60b..f6ea7e08b82a02 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7743,7 +7743,8 @@ class Compiler unsigned methodAttr, unsigned classAttr, unsigned likelihood, - CORINFO_LOOKUP instParamLookup, + bool hasTypeArg, + CORINFO_METHOD_HANDLE instantiatingStub, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 4d977ba50d93e6..d33b024f739471 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7742,6 +7742,17 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; CORINFO_METHOD_HANDLE exactMethod = dvInfo.devirtualizedMethod; uint32_t exactMethodAttrs = info.compCompHnd->getMethodAttribs(exactMethod); + assert(!dvInfo.instParamLookup.lookupKind.needsRuntimeLookup); + + const bool hasTypeArg = !(dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && + dvInfo.instParamLookup.constLookup.handle == nullptr); + CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; + if (hasTypeArg) + { + assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); + instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); + assert(instantiatingStub != NO_METHOD_HANDLE); + } // NOTE: This is currently used only with NativeAOT. In theory, we could also check if we // have static PGO data to decide which class to guess first. Presumably, this is a rare case. @@ -7756,7 +7767,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, } addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, - clsAttrs, likelyHood, dvInfo.instParamLookup, baseMethod, + clsAttrs, likelyHood, hasTypeArg, instantiatingStub, baseMethod, originalContext); } @@ -7780,14 +7791,11 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Iterate over the guesses for (int candidateId = 0; candidateId < candidatesCount; candidateId++) { - CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; - CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; - unsigned likelihood = likelihoods[candidateId]; - - CORINFO_LOOKUP instParamLookup{}; - instParamLookup.lookupKind.needsRuntimeLookup = false; - instParamLookup.constLookup.accessType = IAT_VALUE; - instParamLookup.constLookup.handle = nullptr; + CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; + CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; + unsigned likelihood = likelihoods[candidateId]; + bool hasTypeArg = false; + CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; CORINFO_CONTEXT_HANDLE likelyContext = originalContext; @@ -7830,9 +7838,18 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, break; } - likelyContext = dvInfo.exactContext; - likelyMethod = dvInfo.devirtualizedMethod; - instParamLookup = dvInfo.instParamLookup; + likelyContext = dvInfo.exactContext; + likelyMethod = dvInfo.devirtualizedMethod; + assert(!dvInfo.instParamLookup.lookupKind.needsRuntimeLookup); + + hasTypeArg = !(dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && + dvInfo.instParamLookup.constLookup.handle == nullptr); + if (hasTypeArg) + { + assert(((size_t)likelyContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); + instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)likelyContext & ~CORINFO_CONTEXTFLAGS_MASK); + assert(instantiatingStub != NO_METHOD_HANDLE); + } } else { @@ -7907,7 +7924,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Add this as a potential candidate. // addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs, - likelyClassAttribs, likelihood, instParamLookup, baseMethod, + likelyClassAttribs, likelihood, hasTypeArg, instantiatingStub, baseMethod, originalContext); } } @@ -7933,7 +7950,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // methodAttr - attributes of the method // classAttr - attributes of the class // likelihood - odds that this class is the class seen at runtime -// instParamLookup - the lookup information required by instantiation param +// hasTypeArg - devirtualized method requires an instantiation argument +// instantiatingStub - instantiating stub for the devirtualized method, if one is needed // originalMethodHandle - method handle of base method (before devirt) // originalContextHandle - context for the original call // @@ -7944,7 +7962,8 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, unsigned methodAttr, unsigned classAttr, unsigned likelihood, - CORINFO_LOOKUP instParamLookup, + bool hasTypeArg, + CORINFO_METHOD_HANDLE instantiatingStub, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle) { @@ -7999,15 +8018,15 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, // We're all set, proceed with candidate creation. // - assert(!instParamLookup.lookupKind.needsRuntimeLookup); - const bool needsMethodContext = ((size_t)contextHandle & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; - CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; - if (!(instParamLookup.constLookup.accessType == IAT_VALUE && instParamLookup.constLookup.handle == nullptr)) + if (hasTypeArg) { - assert(needsMethodContext); - instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)contextHandle & ~CORINFO_CONTEXTFLAGS_MASK); + assert(((size_t)contextHandle & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); assert(instantiatingStub != NO_METHOD_HANDLE); } + else + { + assert(instantiatingStub == NO_METHOD_HANDLE); + } JITDUMP("Marking call [%06u] as guarded devirtualization candidate; will guess for %s %s\n", dspTreeID(call), classHandle != NO_CLASS_HANDLE ? "class" : "method", @@ -8035,7 +8054,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, pInfo->originalContextHandle = originalContextHandle; pInfo->likelihood = likelihood; pInfo->exactContextHandle = contextHandle; - pInfo->needsMethodContext = needsMethodContext; + pInfo->hasTypeArg = hasTypeArg; // If the guarded method is an instantiating stub, find the instantiated method // @@ -10016,7 +10035,7 @@ void Compiler::impCheckCanInline(GenTreeCall* call, pInfo->originalMethodHandle = nullptr; pInfo->originalContextHandle = nullptr; pInfo->likelihood = 0; - pInfo->needsMethodContext = false; + pInfo->hasTypeArg = false; } pInfo->methInfo = methInfo; diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 27289b3aa4e6e9..818a6c3714fadc 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -974,7 +974,7 @@ class IndirectCallTransformer { // Pass the original method handle and original context handle to the devirtualizer if needed. // - if (inlineInfo->needsMethodContext) + if (inlineInfo->hasTypeArg) { methodHnd = inlineInfo->originalMethodHandle; context = inlineInfo->originalContextHandle; diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index c320f72fb5d7a1..d19e45fdb6af96 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -598,7 +598,7 @@ struct InlineCandidateInfo : public HandleHistogramProfileCandidateInfo CORINFO_METHOD_HANDLE guardedMethodUnboxedEntryHandle; CORINFO_METHOD_HANDLE guardedMethodInstantiatedEntryHandle; unsigned likelihood; - bool needsMethodContext; + bool hasTypeArg; CORINFO_METHOD_INFO methInfo; From 5942cb8ce6a2b023847b77905672c60ced9af5e8 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 25 Apr 2026 23:42:13 +0900 Subject: [PATCH 5/5] Switch back to needsMethodContext --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/importercalls.cpp | 55 +++++++++++---------- src/coreclr/jit/indirectcalltransformer.cpp | 2 +- src/coreclr/jit/inline.h | 2 +- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f6ea7e08b82a02..c274c2315232b4 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7743,7 +7743,7 @@ class Compiler unsigned methodAttr, unsigned classAttr, unsigned likelihood, - bool hasTypeArg, + bool needsMethodContext, CORINFO_METHOD_HANDLE instantiatingStub, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index d33b024f739471..991f09ec9bef52 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7744,12 +7744,14 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, uint32_t exactMethodAttrs = info.compCompHnd->getMethodAttribs(exactMethod); assert(!dvInfo.instParamLookup.lookupKind.needsRuntimeLookup); - const bool hasTypeArg = !(dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && - dvInfo.instParamLookup.constLookup.handle == nullptr); + const bool needsMethodContext = + ((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; + const bool needsInstParam = (dvInfo.instParamLookup.constLookup.accessType != IAT_VALUE) || + (dvInfo.instParamLookup.constLookup.handle != nullptr); CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; - if (hasTypeArg) + if (needsInstParam) { - assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); + assert(needsMethodContext); instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); assert(instantiatingStub != NO_METHOD_HANDLE); } @@ -7767,8 +7769,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, } addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, - clsAttrs, likelyHood, hasTypeArg, instantiatingStub, baseMethod, - originalContext); + clsAttrs, likelyHood, needsMethodContext, instantiatingStub, + baseMethod, originalContext); } if (call->GetInlineCandidatesCount() == numExactClasses) @@ -7791,11 +7793,11 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Iterate over the guesses for (int candidateId = 0; candidateId < candidatesCount; candidateId++) { - CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; - CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; - unsigned likelihood = likelihoods[candidateId]; - bool hasTypeArg = false; - CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; + CORINFO_CLASS_HANDLE likelyClass = likelyClasses[candidateId]; + CORINFO_METHOD_HANDLE likelyMethod = likelyMethods[candidateId]; + unsigned likelihood = likelihoods[candidateId]; + bool needsMethodContext = false; + CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; CORINFO_CONTEXT_HANDLE likelyContext = originalContext; @@ -7842,11 +7844,12 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, likelyMethod = dvInfo.devirtualizedMethod; assert(!dvInfo.instParamLookup.lookupKind.needsRuntimeLookup); - hasTypeArg = !(dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && - dvInfo.instParamLookup.constLookup.handle == nullptr); - if (hasTypeArg) + needsMethodContext = ((size_t)likelyContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; + const bool needsInstParam = (dvInfo.instParamLookup.constLookup.accessType != IAT_VALUE) || + (dvInfo.instParamLookup.constLookup.handle != nullptr); + if (needsInstParam) { - assert(((size_t)likelyContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); + assert(needsMethodContext); instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)likelyContext & ~CORINFO_CONTEXTFLAGS_MASK); assert(instantiatingStub != NO_METHOD_HANDLE); } @@ -7924,8 +7927,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Add this as a potential candidate. // addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs, - likelyClassAttribs, likelihood, hasTypeArg, instantiatingStub, baseMethod, - originalContext); + likelyClassAttribs, likelihood, needsMethodContext, instantiatingStub, + baseMethod, originalContext); } } @@ -7950,8 +7953,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // methodAttr - attributes of the method // classAttr - attributes of the class // likelihood - odds that this class is the class seen at runtime -// hasTypeArg - devirtualized method requires an instantiation argument -// instantiatingStub - instantiating stub for the devirtualized method, if one is needed +// needsMethodContext - devirtualized method's exact context is a method context +// instantiatingStub - instantiating stub to pass as an instantiation argument, if one is needed // originalMethodHandle - method handle of base method (before devirt) // originalContextHandle - context for the original call // @@ -7962,7 +7965,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, unsigned methodAttr, unsigned classAttr, unsigned likelihood, - bool hasTypeArg, + bool needsMethodContext, CORINFO_METHOD_HANDLE instantiatingStub, CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle) @@ -8018,14 +8021,14 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, // We're all set, proceed with candidate creation. // - if (hasTypeArg) + if (needsMethodContext) { assert(((size_t)contextHandle & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); - assert(instantiatingStub != NO_METHOD_HANDLE); } - else + + if (instantiatingStub != NO_METHOD_HANDLE) { - assert(instantiatingStub == NO_METHOD_HANDLE); + assert(needsMethodContext); } JITDUMP("Marking call [%06u] as guarded devirtualization candidate; will guess for %s %s\n", dspTreeID(call), @@ -8054,7 +8057,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, pInfo->originalContextHandle = originalContextHandle; pInfo->likelihood = likelihood; pInfo->exactContextHandle = contextHandle; - pInfo->hasTypeArg = hasTypeArg; + pInfo->needsMethodContext = needsMethodContext; // If the guarded method is an instantiating stub, find the instantiated method // @@ -10035,7 +10038,7 @@ void Compiler::impCheckCanInline(GenTreeCall* call, pInfo->originalMethodHandle = nullptr; pInfo->originalContextHandle = nullptr; pInfo->likelihood = 0; - pInfo->hasTypeArg = false; + pInfo->needsMethodContext = false; } pInfo->methInfo = methInfo; diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 818a6c3714fadc..27289b3aa4e6e9 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -974,7 +974,7 @@ class IndirectCallTransformer { // Pass the original method handle and original context handle to the devirtualizer if needed. // - if (inlineInfo->hasTypeArg) + if (inlineInfo->needsMethodContext) { methodHnd = inlineInfo->originalMethodHandle; context = inlineInfo->originalContextHandle; diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index d19e45fdb6af96..c320f72fb5d7a1 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -598,7 +598,7 @@ struct InlineCandidateInfo : public HandleHistogramProfileCandidateInfo CORINFO_METHOD_HANDLE guardedMethodUnboxedEntryHandle; CORINFO_METHOD_HANDLE guardedMethodInstantiatedEntryHandle; unsigned likelihood; - bool hasTypeArg; + bool needsMethodContext; CORINFO_METHOD_INFO methInfo;