Support devirtualization for virtual methods that require runtime lookups

src/coreclr/jit/compiler.h

@@ -8096,7 +8096,8 @@ class Compiler
                                          bool                    isInterface,
                                          CORINFO_METHOD_HANDLE   baseMethod,
                                          CORINFO_CLASS_HANDLE    baseClass,
-                                         CORINFO_CONTEXT_HANDLE* pContextHandle);
+                                         CORINFO_CONTEXT_HANDLE* pContextHandle,
+                                         CORINFO_RESOLVED_TOKEN* pResolvedToken);
 
     bool isCompatibleMethodGDV(GenTreeCall* call, CORINFO_METHOD_HANDLE gdvTarget);
 

src/coreclr/jit/fginline.cpp

@@ -608,16 +608,17 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                 }
 #endif // DEBUG
 
-                CORINFO_METHOD_HANDLE  method                 = call->gtLateDevirtualizationInfo->methodHnd;
-                CORINFO_CONTEXT_HANDLE context                = call->gtLateDevirtualizationInfo->exactContextHnd;
-                InlineContext*         inlinersContext        = call->gtInlineContext;
-                unsigned               methodFlags            = 0;
-                const bool             isLateDevirtualization = true;
-                const bool             explicitTailCall       = call->IsTailPrefixedCall();
+                CORINFO_METHOD_HANDLE   method                 = call->gtLateDevirtualizationInfo->methodHnd;
+                CORINFO_CONTEXT_HANDLE  context                = call->gtLateDevirtualizationInfo->exactContextHnd;
+                InlineContext*          inlinersContext        = call->gtInlineContext;
+                CORINFO_RESOLVED_TOKEN* pResolvedToken         = &call->gtLateDevirtualizationInfo->resolvedToken;
+                unsigned                methodFlags            = 0;
+                const bool              isLateDevirtualization = true;
+                const bool              explicitTailCall       = call->IsTailPrefixedCall();
 
                 CORINFO_CONTEXT_HANDLE contextInput = context;
                 context                             = nullptr;
-                m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
+                m_compiler->impDevirtualizeCall(call, pResolvedToken, &method, &methodFlags, &contextInput, &context,
                                                 isLateDevirtualization, explicitTailCall);
 
                 if (!call->IsDevirtualizationCandidate(m_compiler))

src/coreclr/jit/importercalls.cpp

@@ -1021,7 +1021,7 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
         else if (call->AsCall()->IsDelegateInvoke())
         {
             considerGuardedDevirtualization(call->AsCall(), rawILOffset, false, call->AsCall()->gtCallMethHnd,
-                                            NO_CLASS_HANDLE, nullptr);
+                                            NO_CLASS_HANDLE, nullptr, pResolvedToken);
         }
     }
 
@@ -1291,6 +1291,7 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
             info->methodHnd                            = callInfo->hMethod;
             info->exactContextHnd                      = exactContextHnd;
             info->ilLocation                           = impCurStmtDI.GetLocation();
+            info->resolvedToken                        = *pResolvedToken;
             call->AsCall()->gtLateDevirtualizationInfo = info;
         }
     }
@@ -7865,7 +7866,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall*            call,
                                                bool                    isInterface,
                                                CORINFO_METHOD_HANDLE   baseMethod,
                                                CORINFO_CLASS_HANDLE    baseClass,
-                                               CORINFO_CONTEXT_HANDLE* pContextHandle)
+                                               CORINFO_CONTEXT_HANDLE* pContextHandle,
+                                               CORINFO_RESOLVED_TOKEN* pResolvedToken)
 {
     JITDUMP("Considering guarded devirtualization at IL offset %u (0x%x)\n", ilOffset, ilOffset);
 
@@ -7946,7 +7948,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall*            call,
                 dvInfo.virtualMethod               = baseMethod;
                 dvInfo.objClass                    = exactCls;
                 dvInfo.context                     = originalContext;
-                dvInfo.pResolvedTokenVirtualMethod = nullptr;
+                dvInfo.pResolvedTokenVirtualMethod = pResolvedToken;
 
                 JITDUMP("GDV exact: resolveVirtualMethod (method %p class %p context %p)\n", dvInfo.virtualMethod,
                         dvInfo.objClass, dvInfo.context);
@@ -9088,7 +9090,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             return;
         }
 
-        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle);
+        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle,
+                                        pResolvedToken);
 
         return;
     }
@@ -9132,7 +9135,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             return;
         }
 
-        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle);
+        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle,
+                                        pResolvedToken);
         return;
     }
 
@@ -9254,7 +9258,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             return;
         }
 
-        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, objClass, pContextHandle);
+        considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, objClass, pContextHandle,
+                                        pResolvedToken);
         return;
     }
 

src/coreclr/jit/inline.h

@@ -635,6 +635,7 @@ struct LateDevirtualizationInfo
     CORINFO_METHOD_HANDLE  methodHnd;
     CORINFO_CONTEXT_HANDLE exactContextHnd;
     ILLocation             ilLocation;
+    CORINFO_RESOLVED_TOKEN resolvedToken;
 };
 
 // InlArgInfo describes inline candidate argument properties.

src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs

@@ -148,19 +148,19 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType
                                 return null;
 
                             case DefaultInterfaceMethodResolution.DefaultImplementation:
-                                if (dimMethod.OwningType.HasInstantiation || (declMethod != defaultInterfaceDispatchDeclMethod))
+                                if (declMethod != defaultInterfaceDispatchDeclMethod)
                                 {
-                                    // If we devirtualized into a default interface method on a generic type, we should actually return an
-                                    // instantiating stub but this is not happening.
-                                    // Making this work is tracked by https://github.com/dotnet/runtime/issues/9588
-
-                                    // In addition, we fail here for variant default interface dispatch
+                                    // Fail for variant default interface dispatch
                                     devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_DIM;
                                     return null;
                                 }
                                 else
                                 {
                                     impl = dimMethod;
+                                    if (originalDeclMethod.HasInstantiation)
+                                    {
+                                        impl = impl.GetMethodDefinition().MakeInstantiatedMethod(originalDeclMethod.Instantiation);
+                                    }
                                 }
                                 break;
                         }
@@ -219,13 +219,6 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType
                 }
             }
 
-            if (impl != null && impl.HasInstantiation && impl.GetCanonMethodTarget(CanonicalFormKind.Specific).IsCanonicalMethod(CanonicalFormKind.Specific))
-            {
-                // We don't support devirtualization of shared generic virtual methods yet.
-                devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
-                impl = null;
-            }
-
             return impl;
         }
 

src/coreclr/tools/Common/Compiler/GenericCycleDetection/GraphBuilder.ForEach.cs

@@ -59,7 +59,11 @@ private static void ForEachEmbeddedGenericFormalWorker(TypeDesc type, Instantiat
                         TypeDesc genericTypeDefinition = type.GetTypeDefinition();
                         Instantiation genericTypeParameters = genericTypeDefinition.Instantiation;
                         Instantiation genericTypeArguments = type.Instantiation;
-                        for (int i = 0; i < genericTypeArguments.Length; i++)
+
+                        // We have some negative IL tests that reference a generic type with more type arguments
+                        // than the type definition has type parameters.
+                        // Tolerate them as such a type can never load and therefore cannot form a cycle.
+                        for (int i = 0; i < genericTypeArguments.Length && i < genericTypeParameters.Length; i++)
                         {
                             var genericTypeParameter = (EcmaGenericParameter)genericTypeParameters[i];
                             TypeDesc genericTypeArgument = genericTypeArguments[i];

src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs

@@ -124,6 +124,7 @@ public enum DictionaryEntryKind
         DispatchStubAddrSlot = 5,
         FieldDescSlot = 6,
         DeclaringTypeHandleSlot = 7,
+        DevirtualizedMethodDescSlot = 8,
     }
 
     public enum ReadyToRunFixupKind

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

@@ -1483,6 +1483,90 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
                 info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN);
             }
 
+            bool isArray = decl.OwningType.IsInterface && objType.IsArray;
+            bool isGenericVirtual = !isArray && decl.HasInstantiation;
+            MethodDesc instArgTarget = unboxingStub ? nonUnboxingImpl : impl;
+            bool requiresInstMethodDescArg = instArgTarget.RequiresInstMethodDescArg();
+            bool requiresInstMethodTableArg = instArgTarget.RequiresInstMethodTableArg();
+
+            // For unboxing stubs whose unboxed entry needs a MethodTable inst arg, the boxed object supplies the exact MT.
+            // For MethodDesc cases we always need to supply the exact MD.
+            if (requiresInstMethodDescArg || (requiresInstMethodTableArg && !unboxingStub))
+            {
+                if (originalImpl.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any))
+                {
+                    // If we end up with a shared MethodTable that is not exact,
+                    // we can't devirtualize since it's not possible to compute the instantiation argument even as a runtime lookup.
+                    info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                    return false;
+                }
+
+                if (originalImpl.IsRuntimeDeterminedExactMethod || originalImpl.IsSharedByGenericInstantiations)
+                {
+                    if (info->pResolvedTokenVirtualMethod == null || unboxingStub)
+                    {
+                        info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                        return false;
+                    }
+
+#if READYTORUN
+                    ComputeRuntimeLookupForSharedGenericToken(
+                        Internal.ReadyToRunConstants.DictionaryEntryKind.DevirtualizedMethodDescSlot,
+                        ref *info->pResolvedTokenVirtualMethod,
+                        null,
+                        originalImpl,
+                        MethodBeingCompiled,
+                        ref info->instParamLookup);
+#else
+                    // TODO: Implement generic virtual method devirtualization runtime lookup for NativeAOT
+                    info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                    return false;
+#endif
+                }
+            }
+
+            if (isArray)
+            {
+#if READYTORUN
+                // Array interface devirt is not yet supported by R2R.
+                info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                return false;
+#else
+                // NativeAOT handles arrays in a different way that doesn't require an instantiating stub.
+#endif
+            }
+
+            if (!info->instParamLookup.lookupKind.needsRuntimeLookup)
+            {
+                if (requiresInstMethodDescArg)
+                {
+                    if (unboxingStub)
+                    {
+                        // Bail out for now. We need an unboxing stub that points to an instantiated method.
+                        info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                        return false;
+                    }
+#if READYTORUN
+                    MethodWithToken originalImplWithToken = new MethodWithToken(originalImpl, methodWithTokenImpl.Token, null, false, null, null);
+                    info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.MethodDictionary, originalImplWithToken));
+#else
+                    info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.NodeFactory.MethodGenericDictionary(originalImpl));
+#endif
+                }
+                else if (requiresInstMethodTableArg)
+                {
+                    if (!unboxingStub)
+                    {
+#if READYTORUN
+                        info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.TypeDictionary, originalImpl.OwningType));
+
+#else
+                        info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.NodeFactory.ConstructedTypeSymbol(originalImpl.OwningType));
+#endif
+                    }
+                }
+            }
+
 #if READYTORUN
             // Testing has not shown that concerns about virtual matching are significant
             // Only generate verification for builds with the stress mode enabled
@@ -1497,7 +1581,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
 #endif
             info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_SUCCESS;
             info->devirtualizedMethod = ObjectToHandle(impl);
-            info->tokenLookupContext = contextFromType(owningType);
+            info->tokenLookupContext = (isArray || isGenericVirtual) ? contextFromMethod(originalImpl) : contextFromType(owningType);
 
             return true;
 

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DevirtualizationManager.cs

@@ -45,8 +45,7 @@ protected override MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefTyp
             //
             // Result method checking
             // 1. Ensure that the resolved result versions with the code, or is the decl method
-            // 2. Devirtualizing to a default interface method is not currently considered to be useful, and how to check for version
-            //    resilience has not yet been analyzed.
+            // 2. When devirtualizing to a default interface method, the resolved result method must version with the code.
             // 3. When checking that the resolved result versions with the code, validate that all of the types
             //    From implType to the owning type of resolved result method also version with the code.
 
@@ -163,6 +162,17 @@ protected override MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefTyp
 
             if (resolvedVirtualMethod != null)
             {
+                if (resolvedVirtualMethod.OwningType.IsInterface)
+                {
+                    if (_compilationModuleGroup.VersionsWithMethodBody(resolvedVirtualMethod))
+                    {
+                        return resolvedVirtualMethod;
+                    }
+
+                    devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE;
+                    return null;
+                }
+
                 // Validate that the inheritance chain for resolution is within version bubble
                 // The rule is somewhat tricky here.
                 // If the resolved method is the declMethod, then only types which derive from the