Remove call to canInline

Instead check for version resilience over on the VM side, and
return a null handle if the method can't be devirtualized because it
would build in a fragile dependence.

Author: AndyAyersMS

src/jit/importer.cpp

@@ -18480,13 +18480,12 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     // Fetch the method that would be called based on the declared type of 'this'
     CORINFO_METHOD_HANDLE derivedMethod = info.compCompHnd->resolveVirtualMethod(baseMethod, objClass);
 
-    // Bail if we can't get method info for the derived method
-    CORINFO_METHOD_INFO methInfo;
-    const bool          hasInfo = info.compCompHnd->getMethodInfo(derivedMethod, &methInfo);
-
-    if (!hasInfo)
+    // If we failed to get a handle, we can't devirtualize.  This can
+    // happen when prejitting, if the devirtualization crosses
+    // servicing bubble boundaries.
+    if (derivedMethod == nullptr)
     {
-        JITDUMP("--- no method info, sorry\n");
+        JITDUMP("--- no derived method, sorry\n");
         return;
     }
 
@@ -18507,88 +18506,66 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
     }
 #endif // defined(DEBUG)
 
-    // Verify we can inline -- both because we'll want to try and
-    // inline if we devirtualize, and also because this appropriately
-    // safeguards servicing bubbles.
-    //
-    // Note we may want to devirtualize even if we can't inline, but
-    // unfortunately today we can't distinguish between inlines that
-    // are marked noinline for perf reasons versus non-perf reasons.
-    // So we have to assume the worst.
-    //
-    // For instance when crossgenning corelib, we initially allow
-    // devirt of StringBuilder.ToString, but once we generate code for
-    // it we think it is a poor inline candidate, and mark it as
-    // noinline, and after that we refuse to devirtualize calls.
-    DWORD         restrictions = 0;
-    CorInfoInline vmResult     = info.compCompHnd->canInline(info.compMethodHnd, derivedMethod, &restrictions);
-
-    if ((vmResult == INLINE_PASS) && (restrictions == 0))
+    if (objClassIsFinal || derivedMethodIsFinal)
     {
-        if (objClassIsFinal || derivedMethodIsFinal)
-        {
-            JITDUMP("!!! Inlining ok, no restrictions, final; can devirtualize\n");
+        JITDUMP("!!! Inlining ok, no restrictions, final; can devirtualize\n");
+        printf("!!! Inlining ok, no restrictions, final; can devirtualize\n");
 
-            // Make the updates.
-            call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
-            call->gtFlags &= ~GTF_CALL_VIRT_STUB;
-            call->gtCallMethHnd = derivedMethod;
+        // Make the updates.
+        call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
+        call->gtFlags &= ~GTF_CALL_VIRT_STUB;
+        call->gtCallMethHnd = derivedMethod;
 
-            // Need to update call info too. This is fragile
-            // but hopefully the derived method conforms to
-            // the base in most other ways.
-            callInfo->hMethod     = derivedMethod;
-            callInfo->methodFlags = derivedMethodAttribs;
+        // Need to update call info too. This is fragile
+        // but hopefully the derived method conforms to
+        // the base in most other ways.
+        callInfo->hMethod     = derivedMethod;
+        callInfo->methodFlags = derivedMethodAttribs;
 
-            // Clear the inline candidate info (may be non-null since
-            // it's a union field used for other things by virtual
-            // stubs)
-            call->gtInlineCandidateInfo = nullptr;
+        // Clear the inline candidate info (may be non-null since
+        // it's a union field used for other things by virtual
+        // stubs)
+        call->gtInlineCandidateInfo = nullptr;
 
 #ifdef FEATURE_READYTORUN_COMPILER
-            if (opts.IsReadyToRun())
-            {
-                // We should refetch the lookup value from the VM.
-                call->setEntryPoint(callInfo->codePointerLookup.constLookup);
-                call->gtCallMoreFlags &= ~GTF_CALL_M_VIRTSTUB_REL_INDIRECT;
-            }
+        if (opts.IsReadyToRun())
+        {
+            // We should refetch the lookup value from the VM.
+            call->setEntryPoint(callInfo->codePointerLookup.constLookup);
+            call->gtCallMoreFlags &= ~GTF_CALL_M_VIRTSTUB_REL_INDIRECT;
+        }
 #endif
 
-            // Note this may not equal objClass, if there is a
-            // final method that objClass inherits.
-            CORINFO_CLASS_HANDLE derivedClass = info.compCompHnd->getMethodClass(derivedMethod);
+        // Note this may not equal objClass, if there is a
+        // final method that objClass inherits.
+        CORINFO_CLASS_HANDLE derivedClass = info.compCompHnd->getMethodClass(derivedMethod);
 
-            // Really should just re-invoke getCallInfo...
-            callInfo->contextHandle = MAKE_CLASSCONTEXT(derivedClass);
+        // Really should just re-invoke getCallInfo...
+        callInfo->contextHandle = MAKE_CLASSCONTEXT(derivedClass);
 
-            // Update context handle too...
-            *exactContextHandle = MAKE_CLASSCONTEXT(derivedClass);
+        // Update context handle too...
+        *exactContextHandle = MAKE_CLASSCONTEXT(derivedClass);
 
 #if defined(DEBUG)
-            if (verbose)
-            {
-                printf("... after devirt...\n");
-                gtDispTree(call);
-            }
-#endif // defined(DEBUG)
-        }
-        else
+        if (verbose)
         {
-            // Neither class or method is final.
-            //
-            // We could speculatively devirtualize, but there's no
-            // reason to believe the derived method is the one that
-            // is likely to be invoked.
-            //
-            // If there's currently no further overriding (that is, at
-            // the time of jitting, objClass has no subclasses that
-            // override this method), then perhaps we'd be willing to
-            // make a bet...?
-            JITDUMP("??? Inlining ok, no restrictions, NOT final; speculative\n");
+            printf("... after devirt...\n");
+            gtDispTree(call);
         }
+#endif // defined(DEBUG)
     }
     else
     {
-        JITDUMP("--- can't inline (%d) or has restrictions (%d), sorry\n", vmResult, restrictions);
+        // Neither class or method is final.
+        //
+        // We could speculatively devirtualize, but there's no
+        // reason to believe the derived method is the one that
+        // is likely to be invoked.
+        //
+        // If there's currently no further overriding (that is, at
+        // the time of jitting, objClass has no subclasses that
+        // override this method), then perhaps we'd be willing to
+        // make a bet...?
+        JITDUMP("??? NOT final; speculative devirt?\n");
     }
 }

src/vm/jitinterface.cpp

@@ -8937,7 +8937,7 @@ CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethodExact(CORINFO_METHOD_HANDLE m
         MODE_PREEMPTIVE;
     } CONTRACTL_END;
 
-    CORINFO_METHOD_HANDLE result;
+    CORINFO_METHOD_HANDLE result = nullptr;
 
     JIT_TO_EE_TRANSITION();
 
@@ -8966,11 +8966,51 @@ CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethodExact(CORINFO_METHOD_HANDLE m
     // MethodDescs returned to JIT at runtime are always fully loaded. Verify that it is the case.
     _ASSERTE(pMT->IsRestored() && pMT->IsFullyLoaded());
 
-    MethodDesc* pMD = pMT->GetMethodDescForSlot(slot);
+    MethodDesc* pDevirtMD = pMT->GetMethodDescForSlot(slot);
 
-    _ASSERTE(pMD->IsRestored());
+    _ASSERTE(pDevirtMD->IsRestored());
 
-    result = (CORINFO_METHOD_HANDLE) pMD;
+    // Allow devirtialization if jitting, or if prejitting and the
+    // method being jitted and the devirtualized method are in the
+    // same versioning bubble.
+    //
+    // Mirrors logic in canInline.
+    bool allowDevirt = false;
+    if (IsCompilingForNGen())
+    {
+        MethodDesc* pCallerMD = m_pMethodBeingCompiled;
+        Module* pCallerModule = pCallerMD->GetLoaderModule();
+        Module* pDevirtModule = pDevirtMD->GetModule();
+        Assembly* pCallerAssembly = pCallerModule->GetAssembly();
+        Assembly* pDevirtAssembly = pDevirtModule->GetAssembly();
+
+        if (pCallerAssembly == pDevirtAssembly)
+        {
+            // Ngen or R2R, same assembly
+            allowDevirt = true;
+        }
+        else
+        {
+#ifdef FEATURE_READYTORUN_COMPILER
+            if (IsReadyToRunCompilation() &&
+                IsInSameVersionBubble(pCallerMD, pDevirtMD))
+            {
+                // R2R, same version bubble
+                allowDevirt = true;
+            }
+#endif
+        }
+    }
+    else
+    {
+        // Jitting
+        allowDevirt = true;
+    }
+
+    if (allowDevirt)
+    {
+        result = (CORINFO_METHOD_HANDLE) pDevirtMD;
+    }
 
     EE_TO_JIT_TRANSITION();