JIT: Devirtualize generic virtual methods

src/coreclr/inc/corinfo.h

@@ -1556,6 +1556,7 @@ enum CORINFO_DEVIRTUALIZATION_DETAIL
     CORINFO_DEVIRTUALIZATION_FAILED_DUPLICATE_INTERFACE,           // crossgen2 virtual method algorithm and runtime algorithm differ in the presence of duplicate interface implementations
     CORINFO_DEVIRTUALIZATION_FAILED_DECL_NOT_REPRESENTABLE,        // Decl method cannot be represented in R2R image
     CORINFO_DEVIRTUALIZATION_FAILED_TYPE_EQUIVALENCE,              // Support for type equivalence in devirtualization is not yet implemented in crossgen2
+    CORINFO_DEVIRTUALIZATION_FAILED_GENERIC_VIRTUAL,               // Devirtualization of generic virtual methods is not yet implemented in crossgen2
     CORINFO_DEVIRTUALIZATION_COUNT,                                // sentinel for maximum value
 };
 
@@ -1578,7 +1579,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO
     // - If pResolvedTokenDevirtualizedMethod is not set to NULL and targeting an R2R image
     //   use it as the parameter to getCallInfo
     // - isInstantiatingStub is set to TRUE if the devirtualized method is a generic method instantiating stub
-    // - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization
+    // - mayNeedMethodContext 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)
     //
     CORINFO_METHOD_HANDLE           devirtualizedMethod;
@@ -1587,7 +1588,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO
     CORINFO_RESOLVED_TOKEN          resolvedTokenDevirtualizedMethod;
     CORINFO_RESOLVED_TOKEN          resolvedTokenDevirtualizedUnboxedMethod;
     bool                            isInstantiatingStub;
-    bool                            wasArrayInterfaceDevirt;
+    bool                            mayNeedMethodContext;
 };
 
 //----------------------------------------------------------------------------

src/coreclr/jit/compiler.cpp

@@ -10598,6 +10598,8 @@ const char* Compiler::devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DE
             return "Decl method cannot be represented in R2R image";
         case CORINFO_DEVIRTUALIZATION_FAILED_TYPE_EQUIVALENCE:
             return "Support for type equivalence in devirtualization is not yet implemented in crossgen2";
+        case CORINFO_DEVIRTUALIZATION_FAILED_GENERIC_VIRTUAL:
+            return "Devirtualization of generic virtual methods is not yet implemented in crossgen2";
         default:
             return "undefined";
     }

src/coreclr/jit/fginline.cpp

@@ -551,7 +551,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
         {
             return call->gtCallMethHnd;
         }
-        else
+        else if (call->IsGenericVirtual(m_compiler))
         {
             GenTree* runtimeMethHndNode =
                 call->gtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetNode();
@@ -563,11 +563,12 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                 case GT_CNS_INT:
                     return CORINFO_METHOD_HANDLE(runtimeMethHndNode->AsIntCon()->IconValue());
                 default:
-                    assert(!"Unexpected type in RuntimeMethodHandle arg.");
+                    // Unable to get method handle for devirtualization.
+                    // This can happen if the method handle is not an RUNTIMELOOKUP or CNS_INT for generic virtuals,
                     return nullptr;
             }
-            return nullptr;
         }
+        return nullptr;
     }
 
     //------------------------------------------------------------------------
@@ -612,9 +613,13 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
 
         if (tree->OperIs(GT_CALL))
         {
-            GenTreeCall* call = tree->AsCall();
-            // TODO-CQ: Drop `call->gtCallType == CT_USER_FUNC` once we have GVM devirtualization
-            bool tryLateDevirt = call->IsDevirtualizationCandidate(m_compiler) && (call->gtCallType == CT_USER_FUNC);
+            GenTreeCall* call          = tree->AsCall();
+            bool         tryLateDevirt = call->IsDevirtualizationCandidate(m_compiler);
+            if (tryLateDevirt && call->gtCallType == CT_INDIRECT)
+            {
+                // For indirect calls, we can only late devirt if it's a generic virtual method for now.
+                tryLateDevirt = call->IsGenericVirtual(m_compiler);
+            }
 
 #ifdef DEBUG
             tryLateDevirt = tryLateDevirt && (JitConfig.JitEnableLateDevirtualization() == 1);
@@ -637,6 +642,16 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                 const bool             isLateDevirtualization = true;
                 const bool             explicitTailCall       = call->IsTailPrefixedCall();
 
+                if (method == nullptr)
+                {
+                    assert(!call->IsVirtual());
+                    // Unable to get method handle for devirtualization.
+                    // This can happen if the method handle is not an RUNTIMELOOKUP or CNS_INT for generic virtuals,
+                    // e.g. a local variable holding the method handle.
+                    // Just bail out.
+                    return;
+                }
+
                 CORINFO_CONTEXT_HANDLE contextInput = context;
                 context                             = nullptr;
                 m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,

src/coreclr/jit/gentree.cpp

@@ -2374,9 +2374,7 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const
 //
 bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const
 {
-    return IsVirtual() ||
-           (gtCallType == CT_INDIRECT && (gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) ||
-                                          gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT)));
+    return IsVirtual() || IsGenericVirtual(compiler);
 }
 
 //-------------------------------------------------------------------------

src/coreclr/jit/gentree.h

@@ -5260,6 +5260,11 @@ struct GenTreeCall final : public GenTree
     {
         return (gtFlags & GTF_CALL_VIRT_KIND_MASK) == GTF_CALL_VIRT_VTABLE;
     }
+    bool IsGenericVirtual(Compiler* compiler) const
+    {
+        return (gtCallType == CT_INDIRECT && (gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) ||
+                                              gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT)));
+    }
 
     bool IsDevirtualizationCandidate(Compiler* compiler) const;
 

src/coreclr/jit/importercalls.cpp

@@ -417,11 +417,20 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
                 thisPtr          = impTransformThis(thisPtr, pConstrainedResolvedToken, callInfo->thisTransform);
                 assert(thisPtr != nullptr);
 
+                GenTree* origThisPtr = thisPtr;
+
                 // Clone the (possibly transformed) "this" pointer
                 GenTree* thisPtrCopy;
                 thisPtr =
                     impCloneExpr(thisPtr, &thisPtrCopy, CHECK_SPILL_ALL, nullptr DEBUGARG("LDVIRTFTN this pointer"));
 
+                // We cloned the "this" pointer, mark it as a single def and set the class for it
+                if (thisPtr->OperIsLocal() && thisPtr->TypeIs(TYP_REF) && (origThisPtr != thisPtr))
+                {
+                    lvaGetDesc(thisPtr->AsLclVarCommon())->lvSingleDef = 1;
+                    lvaSetClass(thisPtr->AsLclVarCommon()->GetLclNum(), origThisPtr);
+                }
+
                 GenTree* fptr = impImportLdvirtftn(thisPtr, pResolvedToken, callInfo);
                 assert(fptr != nullptr);
 
@@ -448,10 +457,11 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
                 }
 #endif
 
-                // Sine we are jumping over some code, check that its OK to skip that code
+                // Since we are jumping over some code, check that its OK to skip that code
                 assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
                        (sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);
-                goto DONE;
+
+                goto DEVIRT;
             }
 
             case CORINFO_CALL:
@@ -968,6 +978,8 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
         }
     }
 
+DEVIRT:
+
     bool probing;
     probing = impConsiderCallProbe(call->AsCall(), rawILOffset);
 
@@ -7603,7 +7615,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall*            call,
                 }
 
                 addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs,
-                                                    clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt,
+                                                    clsAttrs, likelyHood, dvInfo.mayNeedMethodContext,
                                                     dvInfo.isInstantiatingStub, baseMethod, originalContext);
             }
 
@@ -7627,11 +7639,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                  arrayInterface    = false;
-        bool                  instantiatingStub = false;
+        CORINFO_CLASS_HANDLE  likelyClass          = likelyClasses[candidateId];
+        CORINFO_METHOD_HANDLE likelyMethod         = likelyMethods[candidateId];
+        unsigned              likelihood           = likelihoods[candidateId];
+        bool                  mayNeedMethodContext = false;
+        bool                  instantiatingStub    = false;
 
         CORINFO_CONTEXT_HANDLE likelyContext = originalContext;
 
@@ -7674,10 +7686,10 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall*            call,
                 break;
             }
 
-            likelyContext     = dvInfo.exactContext;
-            likelyMethod      = dvInfo.devirtualizedMethod;
-            arrayInterface    = dvInfo.wasArrayInterfaceDevirt;
-            instantiatingStub = dvInfo.isInstantiatingStub;
+            likelyContext        = dvInfo.exactContext;
+            likelyMethod         = dvInfo.devirtualizedMethod;
+            mayNeedMethodContext = dvInfo.mayNeedMethodContext;
+            instantiatingStub    = dvInfo.isInstantiatingStub;
         }
         else
         {
@@ -7752,7 +7764,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall*            call,
         // Add this as a potential candidate.
         //
         addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs,
-                                            likelyClassAttribs, likelihood, arrayInterface, instantiatingStub,
+                                            likelyClassAttribs, likelihood, mayNeedMethodContext, instantiatingStub,
                                             baseMethod, originalContext);
     }
 }
@@ -7778,7 +7790,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
-//    arrayInterface - devirtualization of an array interface call
+//    mayNeedMethodContext - devirtualized method may need generic method context (e.g. array interfaces)
 //    instantiatingStub - devirtualized method in an instantiating stub
 //    originalMethodHandle - method handle of base method (before devirt)
 //    originalContextHandle - context for the original call
@@ -7790,7 +7802,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall*           call,
                                                    unsigned               methodAttr,
                                                    unsigned               classAttr,
                                                    unsigned               likelihood,
-                                                   bool                   arrayInterface,
+                                                   bool                   mayNeedMethodContext,
                                                    bool                   instantiatingStub,
                                                    CORINFO_METHOD_HANDLE  originalMethodHandle,
                                                    CORINFO_CONTEXT_HANDLE originalContextHandle)
@@ -7869,7 +7881,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall*           call,
     pInfo->originalContextHandle                = originalContextHandle;
     pInfo->likelihood                           = likelihood;
     pInfo->exactContextHandle                   = contextHandle;
-    pInfo->arrayInterface                       = arrayInterface;
+    pInfo->mayNeedMethodContext                 = mayNeedMethodContext;
 
     // If the guarded method is an instantiating stub, find the instantiated method
     //
@@ -8603,6 +8615,12 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
                                                                                      &rootCompiler->info.compMethodInfo->args);
 #endif // DEBUG
 
+    if (JitConfig.JitEnableGenericVirtualDevirtualization() == 0 && call->IsGenericVirtual(this))
+    {
+        JITDUMP("\nimpDevirtualizeCall: generic virtual devirtualization disabled\n");
+        return;
+    }
+
     // Fetch information about the virtual method we're calling.
     CORINFO_METHOD_HANDLE baseMethod        = *method;
     unsigned              baseMethodAttribs = *methodFlags;
@@ -8634,6 +8652,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
 #endif // DEBUG
     }
 
+    // Is the call a generic virtual method call?
+    const bool isGenericVirtual = call->IsGenericVirtual(this);
+
     // In R2R mode, we might see virtual stub calls to
     // non-virtuals. For instance cases where the non-virtual method
     // is in a different assembly but is called via CALLVIRT. For
@@ -8643,7 +8664,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     // In non-R2R modes CALLVIRT <nonvirtual> will be turned into a
     // regular call+nullcheck by normal call importation.
     //
-    if ((baseMethodAttribs & CORINFO_FLG_VIRTUAL) == 0)
+    if (!isGenericVirtual && (baseMethodAttribs & CORINFO_FLG_VIRTUAL) == 0)
     {
         assert(call->IsVirtualStub());
         assert(IsAot());
@@ -8737,7 +8758,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     // It may or may not know enough to devirtualize...
     if (isInterface)
     {
-        assert(call->IsVirtualStub());
+        assert(call->IsVirtualStub() || isGenericVirtual);
         JITDUMP("--- base class is interface\n");
     }
 
@@ -8767,14 +8788,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
 
         if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS)
         {
-            assert(!dvInfo.wasArrayInterfaceDevirt);
+            assert(!dvInfo.mayNeedMethodContext);
             derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK);
         }
         else
         {
             // Array interface devirt can return a nonvirtual generic method of the non-generic SZArrayHelper class.
             //
-            assert(dvInfo.wasArrayInterfaceDevirt);
+            assert(isGenericVirtual || dvInfo.mayNeedMethodContext);
             assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD);
             derivedClass = info.compCompHnd->getMethodClass(derivedMethod);
         }
@@ -8795,9 +8816,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
 
     if (dvInfo.isInstantiatingStub)
     {
-        // We should only end up with generic methods that needs a method context (eg. array interface).
+        // We should only end up with generic methods that needs a method context (eg. array interface, GVM).
         //
-        assert(dvInfo.wasArrayInterfaceDevirt);
+        assert(dvInfo.mayNeedMethodContext);
 
         // We don't expect NAOT to end up here, since it has Array<T>
         // and normal devirtualization.
@@ -8920,6 +8941,42 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
 
     JITDUMP("    %s; can devirtualize\n", note);
 
+    if (dvInfo.isInstantiatingStub)
+    {
+        if (isGenericVirtual)
+        {
+            // This generic method call requires a generic method context.
+            // Pass the method handle as the inst param arg.
+            //
+            assert(call->gtCallAddr->IsCall());
+            CallArg* const methHndArg =
+                call->gtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle);
+            assert(methHndArg != nullptr);
+            GenTree* methHnd = methHndArg->GetEarlyNode();
+            if (methHnd->OperIs(GT_RUNTIMELOOKUP))
+            {
+                // We need to do runtime lookup instead of using the instantiating stub directly.
+                call->gtArgs.InsertInstParam(this, methHnd);
+            }
+            else
+            {
+                // We can pass the instantiating stub directly as we don't need a runtime lookup.
+                GenTree* const instParam = gtNewIconEmbMethHndNode(instantiatingStub);
+                call->gtArgs.InsertInstParam(this, instParam);
+            }
+        }
+        else
+        {
+            // 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;
@@ -8928,17 +8985,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     call->gtControlExpr = nullptr;
     INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_DEVIRTUALIZED);
 
-    if (dvInfo.isInstantiatingStub)
-    {
-        // Pass the instantiating stub method desc as the inst param arg.
-        //
-        // Note different embedding would be needed for NAOT/R2R,
-        // but we have ruled those out above.
-        //
-        GenTree* const instParam = gtNewIconEmbMethHndNode(instantiatingStub);
-        call->gtArgs.InsertInstParam(this, instParam);
-    }
-
     // Virtual calls include an implicit null check, which we may
     // now need to make explicit.
     if (!objIsNonNull)
@@ -9798,7 +9844,7 @@ void Compiler::impCheckCanInline(GenTreeCall*           call,
             pInfo->originalMethodHandle                 = nullptr;
             pInfo->originalContextHandle                = nullptr;
             pInfo->likelihood                           = 0;
-            pInfo->arrayInterface                       = false;
+            pInfo->mayNeedMethodContext                 = false;
         }
 
         pInfo->methInfo                       = methInfo;

src/coreclr/jit/indirectcalltransformer.cpp

@@ -970,10 +970,9 @@ class IndirectCallTransformer
             CORINFO_CONTEXT_HANDLE context   = inlineInfo->exactContextHandle;
             if (clsHnd != NO_CLASS_HANDLE)
             {
-                // If we devirtualized an array interface call,
-                // pass the original method handle and original context handle to the devirtualizer.
+                // Pass the original method handle and original context handle to the devirtualizer if needed.
                 //
-                if (inlineInfo->arrayInterface)
+                if (inlineInfo->mayNeedMethodContext)
                 {
                     methodHnd = inlineInfo->originalMethodHandle;
                     context   = inlineInfo->originalContextHandle;