Instantiating stub for devirtualized default interface method on a generic type

src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp

@@ -1273,7 +1273,7 @@ void MethodContext::recResolveToken(CORINFO_RESOLVED_TOKEN* pResolvedToken, DWOR
     key = SpmiRecordsHelper::CreateAgnostic_CORINFO_RESOLVED_TOKENin(pResolvedToken);
 
     ResolveTokenValue value;
-    value.tokenOut      = SpmiRecordsHelper::StoreAgnostic_CORINFO_RESOLVED_TOKENout(pResolvedToken, ResolveToken);
+    value.tokenOut      = SpmiRecordsHelper::StoreAgnostic_CORINFO_RESOLVED_TOKENout(pResolvedToken, TryResolveToken);
     value.exceptionCode = (DWORD)exceptionCode;
 
     ResolveToken->Add(key, value);
@@ -1296,7 +1296,7 @@ void MethodContext::repResolveToken(CORINFO_RESOLVED_TOKEN* pResolvedToken, DWOR
 
     ResolveTokenValue value = ResolveToken->Get(key);
 
-    SpmiRecordsHelper::Restore_CORINFO_RESOLVED_TOKENout(pResolvedToken, value.tokenOut, ResolveToken);
+    SpmiRecordsHelper::Restore_CORINFO_RESOLVED_TOKENout(pResolvedToken, value.tokenOut, TryResolveToken);
     *exceptionCode = (DWORD)value.exceptionCode;
     DEBUG_REP(dmpResolveToken(key, value));
 }

src/coreclr/jit/importer.cpp

@@ -8592,13 +8592,18 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
     // (b) To shared-code instance methods in generic structs; the extra parameter
     //     is the struct's type handle (a vtable ptr)
     // (c) To shared-code per-instantiation non-generic static methods in generic
-    //     classes and structs; the extra parameter is the type handle
+    //     classes, structs and default interface methods; the extra parameter is the type handle
     // (d) To shared-code generic methods; the extra parameter is an
     //     exact-instantiation MethodDesc
     //
     // We also set the exact type context associated with the call so we can
     // inline the call correctly later on.
 
+    // after devirtulization it may appear that devirtualized method
+    // would require extra type handle arg
+    // it's tempting to reorder this branch two "ifs" below
+    // and generalize its logic for devirtualized method case
+    // but I'm afraid to break stuff
     if (sig->callConv & CORINFO_CALLCONV_PARAMTYPE)
     {
         assert(call->AsCall()->gtCallType == CT_USER_FUNC);
@@ -20915,7 +20920,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             return;
         }
 
-        // Ask the runtime to determine the method that would be called based on the likely type.
+        // Ask the runtime to determine the method that would be called based on the guessed-for type.
         //
         CORINFO_DEVIRTUALIZATION_INFO dvInfo;
         dvInfo.virtualMethod = baseMethod;
@@ -21160,20 +21165,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     // stubs)
     call->gtInlineCandidateInfo = nullptr;
 
-#if defined(DEBUG)
-    if (verbose)
-    {
-        printf("... after devirt...\n");
-        gtDispTree(call);
-    }
-
-    if (doPrint)
-    {
-        printf("Devirtualized %s call to %s:%s; now direct call to %s:%s [%s]\n", callKind, baseClassName,
-               baseMethodName, derivedClassName, derivedMethodName, note);
-    }
-#endif // defined(DEBUG)
-
+    bool passExtraArgForValueType = false;
     // If the 'this' object is a box, see if we can find the unboxed entry point for the call.
     if (thisObj->IsBoxedValue())
     {
@@ -21186,9 +21178,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
         }
 
         // Note for some shared methods the unboxed entry point requires an extra parameter.
-        bool                  requiresInstMethodTableArg = false;
+        bool                  requiresInstMethodTableArgForUnboxedEntry = false;
         CORINFO_METHOD_HANDLE unboxedEntryMethod =
-            info.compCompHnd->getUnboxedEntry(derivedMethod, &requiresInstMethodTableArg);
+            info.compCompHnd->getUnboxedEntry(derivedMethod, &requiresInstMethodTableArgForUnboxedEntry);
 
         if (unboxedEntryMethod != nullptr)
         {
@@ -21200,7 +21192,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             //
             // Ideally, we then inline the boxed method and and if it turns out not to modify
             // the copy, we can undo the copy too.
-            if (requiresInstMethodTableArg)
+            if (requiresInstMethodTableArgForUnboxedEntry)
             {
                 // Perform a trial box removal and ask for the type handle tree.
                 JITDUMP("Unboxed entry needs method table arg...\n");
@@ -21286,6 +21278,11 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
                 }
             }
         }
+        // there is no unboxed entry when we got devirtualized to DIM
+        else if (dvInfo.requiresInstMethodTableArg)
+        {
+            passExtraArgForValueType = true;
+        }
         else
         {
             // Many of the low-level methods on value classes won't have unboxed entries,
@@ -21296,6 +21293,50 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
             JITDUMP("Sorry, failed to find unboxed entry point\n");
         }
     }
+    // check wheter we have returned an instantiating stub for generic DIM
+    if ((isInterface && dvInfo.requiresInstMethodTableArg) || passExtraArgForValueType)
+    {
+        assert(((SIZE_T)dvInfo.context & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS);
+        CORINFO_CLASS_HANDLE exactClassHandle = eeGetClassFromContext(dvInfo.context);
+        GenTree*             instParam        = gtNewIconEmbClsHndNode(exactClassHandle);
+        call->gtCallMethHnd                   = derivedMethod;
+        if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr))
+        {
+            call->gtCallArgs = gtPrependNewCallArg(instParam, call->gtCallArgs);
+        }
+        // Append for non-empty L2R
+        else
+        {
+            GenTreeCall::Use* beforeArg = call->gtCallArgs;
+            while (beforeArg->GetNext() != nullptr)
+            {
+                beforeArg = beforeArg->GetNext();
+            }
+
+            beforeArg->SetNext(gtNewCallArgs(instParam));
+        }
+        // do we need this?
+        for (GenTreeCall::Use& use : call->AsCall()->Args())
+        {
+            call->gtFlags |= use.GetNode()->gtFlags & GTF_GLOB_EFFECT;
+        }
+
+        // should we patch call->gtCallMoreFlags ?
+    }
+
+#if defined(DEBUG)
+    if (verbose)
+    {
+        printf("... after devirt...\n");
+        gtDispTree(call);
+    }
+
+    if (doPrint)
+    {
+        printf("Devirtualized %s call to %s:%s; now direct call to %s:%s [%s]\n", callKind, baseClassName,
+               baseMethodName, derivedClassName, derivedMethodName, note);
+    }
+#endif // defined(DEBUG)
 
     // Need to update call info too.
     //

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

@@ -969,6 +969,8 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
                 {
                     Debug.Assert(ownerTypeDesc is InstantiatedType);
                     decl = _compilation.TypeSystemContext.GetMethodForInstantiatedType(decl.GetTypicalMethodDefinition(), (InstantiatedType)ownerTypeDesc);
+                    info->exactContext = contextFromType(decl.OwningType);
+                    info->requiresInstMethodTableArg = true;
                 }
             }
 
@@ -984,9 +986,11 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
                 impl = getUnboxingThunk(impl);
             }
 
+            MethodDesc exactImpl = TypeSystemHelpers.FindMethodOnTypeWithMatchingTypicalMethod(objType, impl);
+
             info->devirtualizedMethod = ObjectToHandle(impl);
             info->requiresInstMethodTableArg = false;
-            info->exactContext = contextFromType(impl.OwningType);
+            info->exactContext = contextFromType(exactImpl.OwningType);
 
             return true;
         }

src/coreclr/vm/jitinterface.cpp

@@ -7762,7 +7762,7 @@ getMethodInfoHelper(
         ftn,
         false);
 
-    // Shared generic or static per-inst methods and shared methods on generic structs
+    // Shared generic or static per-inst methods, shared methods on generic structs and default interface methods
     // take an extra argument representing their instantiation
     if (ftn->RequiresInstArg())
         methInfo->args.callConv = (CorInfoCallConv)(methInfo->args.callConv | CORINFO_CALLCONV_PARAMTYPE);
@@ -8898,25 +8898,28 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
         // Don't try and devirtualize com interface calls.
         if (pObjMT->IsComObjectType())
         {
-            return nullptr;
+            return false;
         }
 #endif // FEATURE_COMINTEROP
 
         // Interface call devirtualization.
         //
         // We must ensure that pObjMT actually implements the
         // interface corresponding to pBaseMD.
-        if (!pObjMT->CanCastToInterface(pBaseMT))
-        {
-            return false;
-        }
+        bool canCastStraightForward = pObjMT->CanCastToInterface(pBaseMT);
 
         // For generic interface methods we must have context to 
         // safely devirtualize.
+        MethodTable* pOwnerMT = nullptr;
         if (info->context != nullptr)
         {
             TypeHandle OwnerClsHnd = GetTypeFromContext(info->context);
-            MethodTable* pOwnerMT = OwnerClsHnd.GetMethodTable();
+            pOwnerMT = OwnerClsHnd.GetMethodTable();
+
+            if (!canCastStraightForward && !(pOwnerMT->IsInterface() && pObjMT->CanCastToInterface(pOwnerMT)))
+            {
+                return false;
+            }
 
             // If the derived class is a shared class, make sure the
             // owner class is too.
@@ -8927,6 +8930,10 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
 
             pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(TypeHandle(pOwnerMT), pBaseMD, FALSE /* throwOnConflict */);
         }
+        else if (!canCastStraightForward)
+        {
+            return false;
+        }
         else if (!pBaseMD->HasClassOrMethodInstantiation())
         {
             pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(pBaseMD, FALSE /* throwOnConflict */);
@@ -8937,12 +8944,27 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
             return false;
         }
 
-        // 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
+        // default interface method
+        // generics over value types do not share code, so we should do nothing on caller site
+        // when `requiresInstMethodTableArg == false`
         if (pDevirtMD->GetMethodTable()->IsInterface() && pDevirtMD->HasClassInstantiation())
         {
-            return false;
+            // since we are in DIM branch, that means
+            // call MethodTable::GetMethodDescForInterfaceMethod above has returned
+            // either instantiating stub ie `pDevirtMD->IsWrapperStub() == true`
+            // or non shared generic instantiation ie <T> is <Int32>
+            _ASSERTE(pDevirtMD->IsWrapperStub() || !(pDevirtMD->GetMethodTable()->IsSharedByGenericInstantiations() || pDevirtMD->IsSharedByGenericMethodInstantiations()));
+            info->exactContext = MAKE_CLASSCONTEXT(pDevirtMD->GetMethodTable());
+            if (pDevirtMD->IsWrapperStub())
+            {
+                info->requiresInstMethodTableArg = true;
+                pDevirtMD = pDevirtMD->GetExistingWrappedMethodDesc();
+            }
+            else
+            {
+                info->requiresInstMethodTableArg = false;
+            }
+            _ASSERTE(pDevirtMD->IsRestored() && pDevirtMD->GetMethodTable()->IsFullyLoaded());
         }
     }
     else
@@ -8999,18 +9021,24 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
     // interface method. If so, we'll use the method's class.
     //
     MethodTable* pApproxMT = pDevirtMD->GetMethodTable();
-    MethodTable* pExactMT = pApproxMT;
 
     if (pApproxMT->IsInterface())
     {
-        // As noted above, we can't yet handle generic interfaces
-        // with default methods.
-        _ASSERTE(!pDevirtMD->HasClassInstantiation());
+        if (pDevirtMD->HasClassInstantiation())
+        {
+            // DIMs are handled above
+            _ASSERTE(info->exactContext != NULL);
+        }
+        else
+        {
+            info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pApproxMT);
+        }
 
     }
     else
     {
-        pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT);
+        MethodTable* pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT);
+        info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT);
     }
 
 #ifdef FEATURE_READYTORUN_COMPILER
@@ -9034,8 +9062,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
     // Success! Pass back the results.
     //
     info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD;
-    info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT);
-    info->requiresInstMethodTableArg = false;
 
     return true;
 }

src/coreclr/vm/method.hpp

@@ -581,7 +581,7 @@ class MethodDesc
     //
     // RequiresInstMethodDescArg()
     //    The method is itself generic and is shared between generic
-    // instantiations but is not itself generic.  Furthermore
+    // instantiations but is not itself generic. WTF LOL. Furthermore
     // no "this" pointer is given (e.g. a value type method), so we pass in the
     // exact-instantiation method table as an extra argument.
     //   i.e. shared-code instantiated generic methods

src/coreclr/vm/methodtable.h

@@ -2094,6 +2094,7 @@ class MethodTable
 
     MethodDesc *GetMethodDescForInterfaceMethod(TypeHandle ownerType, MethodDesc *pInterfaceMD, BOOL throwOnConflict);
     MethodDesc *GetMethodDescForInterfaceMethod(MethodDesc *pInterfaceMD, BOOL throwOnConflict); // You can only use this one for non-generic interfaces
+    // ^-- I don't believe this is correct statement, we have PRECONDITION(!pInterfaceMD->HasClassOrMethodInstantiation()); which implies it can be used with generic interfaces
 
     //-------------------------------------------------------------------
     // INTERFACE MAP.

src/tests/Loader/classloader/DefaultInterfaceMethods/devirttest/devirttest.cs

@@ -0,0 +1,31 @@
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading.Tasks;
+
+interface I<T>
+{
+    string DefaultTypeOf() => typeof(T).Name;
+}
+
+class Dummy { }
+
+class C : I<string>, I<object>, I<Dummy>
+{
+    string I<Dummy>.DefaultTypeOf() => "C.Dummy";
+}
+
+public static class Program
+{
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    static int Main()
+    {
+        var c = new C();
+        var dcs = ((I<string>)c).DefaultTypeOf();
+        if (dcs != "String") return 200;
+        var dos = ((I<object>)c).DefaultTypeOf();
+        if (dos != "Object") return 300;
+        var dds = ((I<Dummy>)c).DefaultTypeOf();
+        if (dds != "C.Dummy") return 300;
+        return 100;
+    }
+}

src/tests/Loader/classloader/DefaultInterfaceMethods/devirttest/devirttest.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="devirttest.cs" />
+  </ItemGroup>
+</Project>

src/tests/Loader/classloader/DefaultInterfaceMethods/devirttest39419/devirttest39419.cs

@@ -0,0 +1,35 @@
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading.Tasks;
+
+interface IM<T>
+{
+    bool UseDefaultM { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => true; }
+    ValueTask M(T instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false");
+    static ValueTask DefaultM(T instance)
+    {
+        Console.WriteLine("Default Behaviour");
+        return default;
+    }
+}
+
+struct M : IM<int> { }
+
+public static class Program
+{
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    static int Main()
+    {
+        var m = new M();
+        if (((IM<int>)m).UseDefaultM)
+        {
+            IM<int>.DefaultM(42);
+            return 100;
+        }
+        else
+        {
+            ((IM<int>)m).M(42);
+        }
+        return 200;
+    }
+}