Merge pull request #10192 from AndyAyersMS/InterfaceDevirt

Interface devirt
dotnet · Mar 17, 2017 · 8208cb5 · 8208cb5
2 parents e93885e + 4e02195
commit 8208cb5
Show file tree

Hide file tree

Showing 24 changed files with 582 additions and 73 deletions.
diff --git a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h
@@ -129,7 +129,8 @@
     // Return null if devirtualization is not possible.
     CORINFO_METHOD_HANDLE resolveVirtualMethod(
         CORINFO_METHOD_HANDLE virtualMethod,
-        CORINFO_CLASS_HANDLE implementingClass
+        CORINFO_CLASS_HANDLE implementingClass,
+        CORINFO_CONTEXT_HANDLE ownerType
         );
 
     // If a method's attributes have (getMethodAttribs) CORINFO_FLG_INTRINSIC set,

diff --git a/src/ToolBox/superpmi/superpmi-shared/lwmlist.h b/src/ToolBox/superpmi/superpmi-shared/lwmlist.h
@@ -141,7 +141,7 @@ LWM(IsWriteBarrierHelperRequired, DWORDLONG, DWORD)
 LWM(MergeClasses, DLDL, DWORDLONG)
 LWM(PInvokeMarshalingRequired, Agnostic_PInvokeMarshalingRequired, DWORD)
 LWM(ResolveToken, Agnostic_CORINFO_RESOLVED_TOKENin, Agnostic_CORINFO_RESOLVED_TOKENout)
-LWM(ResolveVirtualMethod, DLDL, DWORDLONG)
+LWM(ResolveVirtualMethod, Agnostic_ResolveVirtualMethod, DWORDLONG)
 LWM(TryResolveToken, Agnostic_CORINFO_RESOLVED_TOKENin, Agnostic_CORINFO_RESOLVED_TOKENout)
 LWM(SatisfiesClassConstraints, DWORDLONG, DWORD)
 LWM(SatisfiesMethodConstraints, DLDL, DWORD)

diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp
@@ -3296,33 +3296,40 @@ void MethodContext::repGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsig
     DEBUG_REP(dmpGetMethodVTableOffset((DWORDLONG)method, value));
 }
 
-void MethodContext::recResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass, CORINFO_METHOD_HANDLE result)
+void MethodContext::recResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass,
+    CORINFO_CONTEXT_HANDLE ownerType, CORINFO_METHOD_HANDLE result)
 {
     if (ResolveVirtualMethod == nullptr)
     {
-        ResolveVirtualMethod = new LightWeightMap<DLDL, DWORDLONG>();
+        ResolveVirtualMethod = new LightWeightMap<Agnostic_ResolveVirtualMethod, DWORDLONG>();
     }
 
-    DLDL key;
-    key.A = (DWORDLONG)virtMethod;
-    key.B = (DWORDLONG)implClass;
+    Agnostic_ResolveVirtualMethod key;
+    key.virtualMethod = (DWORDLONG)virtMethod;
+    key.implementingClass = (DWORDLONG)implClass;
+    key.ownerType = (DWORDLONG)ownerType;
     ResolveVirtualMethod->Add(key, (DWORDLONG) result);
     DEBUG_REC(dmpResolveVirtualMethod(key, result));
 }
 
-void MethodContext::dmpResolveVirtualMethod(DLDL key, DWORDLONG value)
+void MethodContext::dmpResolveVirtualMethod(const Agnostic_ResolveVirtualMethod& key, DWORDLONG value)
 {
-    printf("ResolveVirtualMethod virtMethod-%016llX, implClass-%016llX, result-%016llX", key.A, key.B, value);
+    printf("ResolveVirtualMethod virtMethod-%016llX, implClass-%016llX, ownerType--%01611X, result-%016llX",
+        key.virtualMethod, key.implementingClass, key.ownerType, value);
 }
 
-CORINFO_METHOD_HANDLE MethodContext::repResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass)
+CORINFO_METHOD_HANDLE MethodContext::repResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass,
+    CORINFO_CONTEXT_HANDLE ownerType)
 {
-    DLDL key;
-    key.A = (DWORDLONG)virtMethod;
-    key.B = (DWORDLONG)implClass;
+    Agnostic_ResolveVirtualMethod key;
+    key.virtualMethod = (DWORDLONG)virtMethod;
+    key.implementingClass = (DWORDLONG)implClass;
+    key.ownerType = (DWORDLONG)ownerType;
 
-    AssertCodeMsg(ResolveVirtualMethod != nullptr, EXCEPTIONCODE_MC, "No ResolveVirtualMap map for %016llX-%016llX", key.A, key.B);
-    AssertCodeMsg(ResolveVirtualMethod->GetIndex(key) != -1, EXCEPTIONCODE_MC, "Didn't find %016llX-%016llx", key.A, key.B);
+    AssertCodeMsg(ResolveVirtualMethod != nullptr, EXCEPTIONCODE_MC, "No ResolveVirtualMap map for %016llX-%016llX-%016llX",
+        key.virtualMethod, key.implementingClass, key.ownerType);
+    AssertCodeMsg(ResolveVirtualMethod->GetIndex(key) != -1, EXCEPTIONCODE_MC, "Didn't find %016llX-%016llx-%016llX",
+        key.virtualMethod, key.implementingClass, key.ownerType);
     DWORDLONG result = ResolveVirtualMethod->Get(key);
 
     DEBUG_REP(dmpResolveVirtualMethod(key, result));

diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/ToolBox/superpmi/superpmi-shared/methodcontext.h
@@ -436,6 +436,13 @@ class MethodContext
         DWORD   result;
     };
 
+    struct Agnostic_ResolveVirtualMethod
+    {
+        DWORDLONG virtualMethod;
+        DWORDLONG implementingClass;
+        DWORDLONG ownerType;
+    };
+
 #pragma pack(pop)
 
     MethodContext();
@@ -698,9 +705,11 @@ class MethodContext
     void dmpGetMethodVTableOffset(DWORDLONG key, DD value);
     void repGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned *offsetOfIndirection, unsigned* offsetAfterIndirection);
 
-    void recResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass, CORINFO_METHOD_HANDLE result);
-    void dmpResolveVirtualMethod(DLDL key, DWORDLONG value);
-    CORINFO_METHOD_HANDLE repResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass);
+    void recResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass,
+        CORINFO_CONTEXT_HANDLE ownerType, CORINFO_METHOD_HANDLE result);
+    void dmpResolveVirtualMethod(const Agnostic_ResolveVirtualMethod& key, DWORDLONG value);
+    CORINFO_METHOD_HANDLE repResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass,
+        CORINFO_CONTEXT_HANDLE ownerType);
 
     void recGetTokenTypeAsHandle(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_CLASS_HANDLE result);
     void dmpGetTokenTypeAsHandle(const Agnostic_CORINFO_RESOLVED_TOKEN& key, DWORDLONG value);

diff --git a/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp
@@ -240,12 +240,13 @@ void interceptor_ICJI::getMethodVTableOffset (
 // Return null if devirtualization is not possible.
 CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(
     CORINFO_METHOD_HANDLE virtualMethod,
-    CORINFO_CLASS_HANDLE implementingClass
+    CORINFO_CLASS_HANDLE implementingClass,
+    CORINFO_CONTEXT_HANDLE ownerType
     )
 {
     mc->cr->AddCall("resolveVirtualMethod");
-    CORINFO_METHOD_HANDLE result = original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass);
-    mc->recResolveVirtualMethod(virtualMethod, implementingClass, result);
+    CORINFO_METHOD_HANDLE result = original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
+    mc->recResolveVirtualMethod(virtualMethod, implementingClass, ownerType, result);
     return result;
 }
 

diff --git a/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp
@@ -169,11 +169,12 @@ void interceptor_ICJI::getMethodVTableOffset (
 // Return null if devirtualization is not possible.
 CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(
     CORINFO_METHOD_HANDLE virtualMethod,
-    CORINFO_CLASS_HANDLE implementingClass
+    CORINFO_CLASS_HANDLE implementingClass,
+    CORINFO_CONTEXT_HANDLE ownerType
     )
 {
     mcs->AddCall("resolveVirtualMethod");
-    return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass);
+    return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
 }
 
 // If a method's attributes have (getMethodAttribs) CORINFO_FLG_INTRINSIC set,

diff --git a/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp
@@ -157,10 +157,11 @@ void interceptor_ICJI::getMethodVTableOffset (
 // Return null if devirtualization is not possible.
 CORINFO_METHOD_HANDLE interceptor_ICJI::resolveVirtualMethod(
     CORINFO_METHOD_HANDLE virtualMethod,
-    CORINFO_CLASS_HANDLE implementingClass
+    CORINFO_CLASS_HANDLE implementingClass,
+    CORINFO_CONTEXT_HANDLE ownerType
     )
 {
-    return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass);
+    return original_ICorJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
 }
 
 // If a method's attributes have (getMethodAttribs) CORINFO_FLG_INTRINSIC set,

diff --git a/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
@@ -189,11 +189,12 @@ void MyICJI::getMethodVTableOffset (
 // Return null if devirtualization is not possible.
 CORINFO_METHOD_HANDLE MyICJI::resolveVirtualMethod(
     CORINFO_METHOD_HANDLE virtualMethod,
-    CORINFO_CLASS_HANDLE implementingClass
+    CORINFO_CLASS_HANDLE implementingClass,
+    CORINFO_CONTEXT_HANDLE ownerType
     )
 {
     jitInstance->mc->cr->AddCall("resolveVirtualMethod");
-    CORINFO_METHOD_HANDLE result = jitInstance->mc->repResolveVirtualMethod(virtualMethod, implementingClass);
+    CORINFO_METHOD_HANDLE result = jitInstance->mc->repResolveVirtualMethod(virtualMethod, implementingClass, ownerType);
     return result;
 }
 

diff --git a/src/inc/corinfo.h b/src/inc/corinfo.h
@@ -231,11 +231,11 @@ TODO: Talk about initializing strutures before use
 #if COR_JIT_EE_VERSION > 460
 
 // Update this one
-SELECTANY const GUID JITEEVersionIdentifier = { /* cda334f7-0020-4622-a4a5-8b8ac71ee5cf */
-    0xcda334f7,
-    0x0020,
-    0x4622,
-    {0xa4, 0xa5, 0x8b, 0x8a, 0xc7, 0x1e, 0xe5, 0xcf}
+SELECTANY const GUID JITEEVersionIdentifier = { /* 3d43decb-a611-4413-a0af-a24278a00e2d */
+    0x3d43decb,
+    0xa611,
+    0x4413,
+    {0xa0, 0xaf, 0xa2, 0x42, 0x78, 0xa0, 0x0e, 0x2d}
   };
 
 #else
@@ -2117,12 +2117,17 @@ class ICorStaticInfo
             ) = 0;
 
 #if COR_JIT_EE_VERSION > 460
-    // Find the virtual method in implementingClass that overrides virtualMethod.
-    // Return null if devirtualization is not possible.
+    // Find the virtual method in implementingClass that overrides virtualMethod,
+    // or the method in implementingClass that implements the interface method
+    // represented by virtualMethod.
+    //
+    // Return null if devirtualization is not possible. Owner type is optional
+    // and provides additional context for shared interface devirtualization.
     virtual CORINFO_METHOD_HANDLE resolveVirtualMethod(
-        CORINFO_METHOD_HANDLE virtualMethod,        /* IN */
-        CORINFO_CLASS_HANDLE implementingClass      /* IN */
-        ) = 0;
+            CORINFO_METHOD_HANDLE       virtualMethod,          /* IN */
+            CORINFO_CLASS_HANDLE        implementingClass,      /* IN */
+            CORINFO_CONTEXT_HANDLE      ownerType = NULL        /* IN */
+            ) = 0;
 #endif
 
     // If a method's attributes have (getMethodAttribs) CORINFO_FLG_INTRINSIC set,

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
@@ -18488,16 +18488,15 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
         return;
     }
 
-    // Bail (for now) if base class is an interface.
     if (isInterface)
     {
         assert(call->IsVirtualStub());
-        JITDUMP("--- base class is interface, sorry\n");
-        return;
+        JITDUMP("--- base class is interface\n");
     }
 
     // Fetch the method that would be called based on the declared type of 'this'
-    CORINFO_METHOD_HANDLE derivedMethod = info.compCompHnd->resolveVirtualMethod(baseMethod, objClass);
+    CORINFO_CONTEXT_HANDLE ownerType     = callInfo->contextHandle;
+    CORINFO_METHOD_HANDLE  derivedMethod = info.compCompHnd->resolveVirtualMethod(baseMethod, objClass, ownerType);
 
     // If we failed to get a handle, we can't devirtualize.  This can
     // happen when prejitting, if the devirtualization crosses

diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp
@@ -8716,7 +8716,8 @@ void CEEInfo::getMethodVTableOffset (CORINFO_METHOD_HANDLE methodHnd,
 /*********************************************************************/
 static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod,
                                                         CORINFO_METHOD_HANDLE baseMethod,
-                                                        CORINFO_CLASS_HANDLE derivedClass)
+                                                        CORINFO_CLASS_HANDLE derivedClass,
+                                                        CORINFO_CONTEXT_HANDLE ownerType)
 {
     STANDARD_VM_CONTRACT;
 
@@ -8729,15 +8730,11 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
     //@GENERICS: shouldn't be doing this for instantiated methods as they live elsewhere
     _ASSERTE(!pBaseMD->HasMethodInstantiation());
 
-    // Interface call devirtualization is not yet supported.
-    if (pBaseMT->IsInterface())
-    {
-        return nullptr;
-    }
-
     // Method better be virtual
     _ASSERTE(pBaseMD->IsVirtual());
 
+    MethodDesc* pDevirtMD = nullptr;
+
     TypeHandle DerivedClsHnd(derivedClass);
     MethodTable* pDerivedMT = DerivedClsHnd.GetMethodTable();
     _ASSERTE(pDerivedMT->IsRestored() && pDerivedMT->IsFullyLoaded());
@@ -8748,33 +8745,65 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
         return nullptr;
     }
 
-    // The derived class should be a subclass of the the base class.
-    MethodTable* pCheckMT = pDerivedMT;
-
-    while (pCheckMT != nullptr)
+    if (pBaseMT->IsInterface())
     {
-        if (pCheckMT->HasSameTypeDefAs(pBaseMT))
+        // Interface call devirtualization.
+        //
+        // We must ensure that pDerivedMT actually implements the
+        // interface corresponding to pBaseMD.
+        if (!pDerivedMT->CanCastToInterface(pBaseMT))
         {
-            break;
+            return nullptr;
         }
 
-        pCheckMT = pCheckMT->GetParentMethodTable();
+        // For generic interface methods we must have an ownerType to
+        // safely devirtualize.
+        if (ownerType != nullptr)
+        {
+            pDevirtMD = pDerivedMT->GetMethodDescForInterfaceMethod(GetTypeFromContext(ownerType), pBaseMD);
+        }
+        else if (!pBaseMD->HasClassOrMethodInstantiation())
+        {
+            pDevirtMD = pDerivedMT->GetMethodDescForInterfaceMethod(pBaseMD);
+        }
+        else
+        {
+            return nullptr;
+        }
     }
-
-    if (pCheckMT == nullptr)
+    else
     {
-        return nullptr;
+        // Virtual call devirtualization.
+        // 
+        // The derived class should be a subclass of the the base class.
+        MethodTable* pCheckMT = pDerivedMT;
+
+        while (pCheckMT != nullptr)
+        {
+            if (pCheckMT->HasSameTypeDefAs(pBaseMT))
+            {
+                break;
+            }
+
+            pCheckMT = pCheckMT->GetParentMethodTable();
+        }
+
+        if (pCheckMT == nullptr)
+        {
+            return nullptr;
+        }
+
+        // The base method should be in the base vtable
+        WORD slot = pBaseMD->GetSlot();
+        _ASSERTE(slot < pBaseMT->GetNumVirtuals());
+        _ASSERTE(pBaseMD == pBaseMT->GetMethodDescForSlot(slot));
+
+        // Fetch the method that would be invoked if the class were
+        // exactly derived class. It is up to the jit to determine whether
+        // directly calling this method is correct.
+        pDevirtMD = pDerivedMT->GetMethodDescForSlot(slot);
     }
 
-    // The base method should be in the base vtable
-    WORD slot = pBaseMD->GetSlot();
-    _ASSERTE(slot < pBaseMT->GetNumVirtuals());
-    _ASSERTE(pBaseMD == pBaseMT->GetMethodDescForSlot(slot));
-
-    // Fetch the method that would be invoked if the class were
-    // exactly derived class. It is up to the jit to determine whether
-    // directly calling this method is correct.
-    MethodDesc* pDevirtMD = pDerivedMT->GetMethodDescForSlot(slot);
     _ASSERTE(pDevirtMD->IsRestored());
 
 #ifdef FEATURE_READYTORUN_COMPILER
@@ -8798,15 +8827,16 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
 }
 
 CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethod(CORINFO_METHOD_HANDLE methodHnd,
-                                                    CORINFO_CLASS_HANDLE derivedClass)
+                                                    CORINFO_CLASS_HANDLE derivedClass,
+                                                    CORINFO_CONTEXT_HANDLE ownerType)
 {
     STANDARD_VM_CONTRACT;
 
     CORINFO_METHOD_HANDLE result = nullptr;
 
     JIT_TO_EE_TRANSITION();
 
-    result = resolveVirtualMethodHelper(m_pMethodBeingCompiled, methodHnd, derivedClass);
+    result = resolveVirtualMethodHelper(m_pMethodBeingCompiled, methodHnd, derivedClass, ownerType);
 
     EE_TO_JIT_TRANSITION();
 

diff --git a/src/vm/jitinterface.h b/src/vm/jitinterface.h
@@ -731,7 +731,8 @@ class CEEInfo : public ICorJitInfo
 
     CORINFO_METHOD_HANDLE resolveVirtualMethod(
         CORINFO_METHOD_HANDLE virtualMethod,
-        CORINFO_CLASS_HANDLE implementingClass
+        CORINFO_CLASS_HANDLE implementingClass,
+        CORINFO_CONTEXT_HANDLE ownerType
         );
 
     CorInfoIntrinsics getIntrinsicID(CORINFO_METHOD_HANDLE method,

diff --git a/src/zap/zapinfo.cpp b/src/zap/zapinfo.cpp
@@ -3729,10 +3729,11 @@ void ZapInfo::getMethodVTableOffset(CORINFO_METHOD_HANDLE method,
 
 CORINFO_METHOD_HANDLE ZapInfo::resolveVirtualMethod(
         CORINFO_METHOD_HANDLE virtualMethod,
-        CORINFO_CLASS_HANDLE implementingClass
+        CORINFO_CLASS_HANDLE implementingClass,
+        CORINFO_CONTEXT_HANDLE ownerType
         )
 {
-    return m_pEEJitInfo->resolveVirtualMethod(virtualMethod, implementingClass);
+    return m_pEEJitInfo->resolveVirtualMethod(virtualMethod, implementingClass, ownerType);
 }
 
 CorInfoIntrinsics ZapInfo::getIntrinsicID(CORINFO_METHOD_HANDLE method,

diff --git a/src/zap/zapinfo.h b/src/zap/zapinfo.h
@@ -667,7 +667,8 @@ class ZapInfo
 
     CORINFO_METHOD_HANDLE resolveVirtualMethod(
         CORINFO_METHOD_HANDLE virtualMethod,
-        CORINFO_CLASS_HANDLE implementingClass
+        CORINFO_CLASS_HANDLE implementingClass,
+        CORINFO_CONTEXT_HANDLE ownerType
         );
 
     CorInfoIntrinsics getIntrinsicID(CORINFO_METHOD_HANDLE method,