From 00f13f2fb04c21fc1675ce264ef7291cbcec67a1 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 16 Mar 2017 08:30:17 -0700 Subject: [PATCH 1/3] Interface call devirtualization: VM side of changes Extend resolveVirtualMethod to take in the owner type needed to resolve shared generic interface calls, and implement VM support for interface call devirtualization. Add a check that the class presented by the jit implements the interface, to catch cases where the IL is missing type casts. Update jit GUID, SPMI and zap to reflect the changed signature. --- .../superpmi-shared/icorjitinfoimpl.h | 3 +- .../superpmi/superpmi-shared/lwmlist.h | 2 +- .../superpmi-shared/methodcontext.cpp | 33 ++++--- .../superpmi/superpmi-shared/methodcontext.h | 15 +++- .../superpmi-shim-collector/icorjitinfo.cpp | 7 +- .../superpmi-shim-counter/icorjitinfo.cpp | 5 +- .../superpmi-shim-simple/icorjitinfo.cpp | 5 +- src/ToolBox/superpmi/superpmi/icorjitinfo.cpp | 5 +- src/inc/corinfo.h | 25 +++--- src/vm/jitinterface.cpp | 86 +++++++++++++------ src/vm/jitinterface.h | 3 +- src/zap/zapinfo.cpp | 5 +- src/zap/zapinfo.h | 3 +- 13 files changed, 128 insertions(+), 69 deletions(-) diff --git a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h index 4c1aa3d72d52..1e83343d2da5 100644 --- 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 index 8e19656f188d..6e5f0168fd26 100644 --- 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 index c60b593ae0e0..39bcefea5916 100644 --- 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(); + ResolveVirtualMethod = new LightWeightMap(); } - 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 index ee2d4ac7c857..5827fff00cb0 100644 --- 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 index bb19cab45547..01e98cd7fdb4 100644 --- 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 index beff1f4e4bea..705b1e89395c 100644 --- 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 index 9e229eeeafa3..9783983af956 100644 --- 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 index ae4ba9917cff..91ebb4ddeb4f 100644 --- 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 index c094c94c7a4c..7473de88cd68 100644 --- 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/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index bdccdefd2489..1639e2df199a 100644 --- 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,7 +8827,8 @@ 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; @@ -8806,7 +8836,7 @@ CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethod(CORINFO_METHOD_HANDLE method 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 index c0df0cf34fba..a432b59c9a68 100644 --- 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 index e817e4459e24..4cc7e9fe1571 100644 --- 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 index c0846d104402..973170f09d28 100644 --- 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, From 2c8b3ae4c1786968f387af9c78a343e0a561869b Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 16 Mar 2017 08:30:58 -0700 Subject: [PATCH 2/3] Interface call devirtualization: jit side of changes Unblock VM queries where the base class is an interface (queries where the implementing class is also an interface are still blocked as no resolution is possible). Pass along the context handle that the jit got from getCallInfo. --- src/jit/importer.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index e285fed5ae9e..9d1daac04b44 100644 --- 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 From 4e021953d8421ed6d53d8d0ff5bef2d09a738c5e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 15 Mar 2017 08:16:59 -0700 Subject: [PATCH 3/3] Add some test cases --- .../JIT/opt/Devirtualization/comparable.cs | 67 +++++++++++++++++++ .../opt/Devirtualization/comparable.csproj | 41 ++++++++++++ .../opt/Devirtualization/contravariance.cs | 38 +++++++++++ .../Devirtualization/contravariance.csproj | 41 ++++++++++++ .../JIT/opt/Devirtualization/covariance.cs | 35 ++++++++++ .../opt/Devirtualization/covariance.csproj | 41 ++++++++++++ tests/src/JIT/opt/Devirtualization/late.cs | 48 +++++++++++++ .../src/JIT/opt/Devirtualization/late.csproj | 41 ++++++++++++ tests/src/JIT/opt/Devirtualization/simple.cs | 58 ++++++++++++++++ .../JIT/opt/Devirtualization/simple.csproj | 41 ++++++++++++ 10 files changed, 451 insertions(+) create mode 100644 tests/src/JIT/opt/Devirtualization/comparable.cs create mode 100644 tests/src/JIT/opt/Devirtualization/comparable.csproj create mode 100644 tests/src/JIT/opt/Devirtualization/contravariance.cs create mode 100644 tests/src/JIT/opt/Devirtualization/contravariance.csproj create mode 100644 tests/src/JIT/opt/Devirtualization/covariance.cs create mode 100644 tests/src/JIT/opt/Devirtualization/covariance.csproj create mode 100644 tests/src/JIT/opt/Devirtualization/late.cs create mode 100644 tests/src/JIT/opt/Devirtualization/late.csproj create mode 100644 tests/src/JIT/opt/Devirtualization/simple.cs create mode 100644 tests/src/JIT/opt/Devirtualization/simple.csproj diff --git a/tests/src/JIT/opt/Devirtualization/comparable.cs b/tests/src/JIT/opt/Devirtualization/comparable.cs new file mode 100644 index 000000000000..2322395bf2d0 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/comparable.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +public sealed class X: IComparable +{ + int ival; + + public X(int i) + { + ival = i; + } + + public int CompareTo(X x) + { + return ival - x.ival; + } + + public bool Equals(X x) + { + return ival == x.ival; + } +} + +public class Y where T : IComparable +{ + public static int C(T x, T y) + { + // IL here is + // ldarga 0 + // ldarg 1 + // constrained ... callvirt ... + // + // The ldarga blocks both caller-arg direct sub and type + // propagation since the jit thinks arg0 might be redefined. + // + // For ref types the ldarga is undone in codegen just before + // the call so we end up with *(&arg0) and we know this is + // arg0. Ideally we'd also understand that this pattern can't + // lead to reassignment, but our view of the callee and what + // it does with address-taken args is quite limited. + // + // Even if we can't propagate the caller's value or type, we + // might be able to retype the generic __Canon for arg0 as the + // more specific type that the caller is using (here, X). + // + // An interesting variant on this would be to derive from X + // (say with XD) and have the caller pass instances of XD + // instead of instances of X. We'd need to make sure we retype + // arg0 as X and not XD. + return x.CompareTo(y); + } +} + +public class Z +{ + public static int Main() + { + // Ideally inlining Y.C would enable the interface call in Y + // to be devirtualized, since we know the exact type of the + // first argument. We can't get this yet. + int result = Y.C(new X(103), new X(3)); + return result; + } +} diff --git a/tests/src/JIT/opt/Devirtualization/comparable.csproj b/tests/src/JIT/opt/Devirtualization/comparable.csproj new file mode 100644 index 000000000000..780cd910cad6 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/comparable.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + PdbOnly + True + + + + + + + + + + + diff --git a/tests/src/JIT/opt/Devirtualization/contravariance.cs b/tests/src/JIT/opt/Devirtualization/contravariance.cs new file mode 100644 index 000000000000..0594772c9afd --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/contravariance.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +interface I +{ + T A(); +} + +class X : I where T: class +{ + T I.A() + { + return (T)(object)"X"; + } +} + +class T +{ + static object F(I i) + { + return i.A(); + } + + public static int Main() + { + // Jit should inline F and then devirtualize the call to A. + // (inlining A blocked by runtime lookup) + object j = F(new X()); + if (j is string) + { + return ((string)j)[0] + 12; + } + return -1; + } +} diff --git a/tests/src/JIT/opt/Devirtualization/contravariance.csproj b/tests/src/JIT/opt/Devirtualization/contravariance.csproj new file mode 100644 index 000000000000..287c2009f13d --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/contravariance.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + PdbOnly + True + + + + + + + + + + + diff --git a/tests/src/JIT/opt/Devirtualization/covariance.cs b/tests/src/JIT/opt/Devirtualization/covariance.cs new file mode 100644 index 000000000000..2ed7d2bdd83a --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/covariance.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +interface I +{ + int A(T t); +} + +class X : I +{ + int c = 0; + int I.A(T t) + { + return ++c; + } +} + +class T +{ + static int F(I i) + { + return i.A("A"); + } + + public static int Main() + { + // Jit should inline F and then devirtualize + // and inline the call to A. + int j = F(new X()); + return j + 99; + } +} diff --git a/tests/src/JIT/opt/Devirtualization/covariance.csproj b/tests/src/JIT/opt/Devirtualization/covariance.csproj new file mode 100644 index 000000000000..b94309945ab7 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/covariance.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + PdbOnly + True + + + + + + + + + + + diff --git a/tests/src/JIT/opt/Devirtualization/late.cs b/tests/src/JIT/opt/Devirtualization/late.cs new file mode 100644 index 000000000000..3991e7df43b6 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/late.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +interface Ix where T : class +{ + T F(); +} + +class Base : Ix +{ + public virtual string F() { return "B"; } +} + +class Derived : Base +{ + public override string F() { return "D"; } +} + +class Bx +{ + public Ix Get() { return new Derived(); } +} + +public class Z +{ + static string X(Base b) + { + return b.F(); + } + + public static int Main() + { + // Would like to be able to late devirtualize the call to F + // here after inlining Get exposes the exact type of the + // object, but since the return type of Get is a (shared) + // interface type, we need the exact context for F to do so + // safely. + // + // Unfortunately we lose track of that context, because when + // we import the call to F, it is not an inline candidate. + string s = new Bx().Get().F(); + return (int) s[0] + 32; + } +} + diff --git a/tests/src/JIT/opt/Devirtualization/late.csproj b/tests/src/JIT/opt/Devirtualization/late.csproj new file mode 100644 index 000000000000..6af10b47f605 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/late.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + PdbOnly + True + + + + + + + + + + + diff --git a/tests/src/JIT/opt/Devirtualization/simple.cs b/tests/src/JIT/opt/Devirtualization/simple.cs new file mode 100644 index 000000000000..46b039ce15a4 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/simple.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +// Some simple interface call devirtualization cases + +interface Ix +{ + int F(); +} + +interface Iy +{ + int G(); +} + +interface Iz +{ + int H(); + int I(); +} + +public class B : Iy, Ix, Iz +{ + public int F() { return 3; } + virtual public int G() { return 5; } + int Iz.H() { return 7; } + int Iz.I() { return 11; } +} + +public class Z : B, Iz +{ + new public int F() { return 13; } + override public int G() { return 17; } + int Iz.H() { return 19; } + + static int Fx(Ix x) { return x.F(); } + static int Gy(Iy y) { return y.G(); } + static int Hz(Iz z) { return z.H(); } + static int Hi(Iz z) { return z.I(); } + + public static int Main() + { + int callsBF = Fx(new Z()) + Fx(new B()) + ((Ix) new Z()).F() + ((Ix) new B()).F(); + int callsBG = Gy(new B()) + ((Iy) new B()).G() + (new B()).G(); + int callsBH = Hz(new B()) + ((Iz) new B()).H(); + int callsBI = Hi(new Z()) + Hi(new B()) + ((Iz) new Z()).I() + ((Iz) new B()).I(); + int callsZG = Gy(new Z()) + ((Iy) new Z()).G() + (new Z()).G(); + int callsZH = Hz(new Z()) + ((Iz) new Z()).H(); + + int expected = 4 * 3 + 3 * 5 + 2 * 7 + 4 * 11 + 3 * 17 + 2 * 19; + + return callsBF + callsBG + callsBI + callsBH + callsZG + callsZH - expected + 100; + } +} + diff --git a/tests/src/JIT/opt/Devirtualization/simple.csproj b/tests/src/JIT/opt/Devirtualization/simple.csproj new file mode 100644 index 000000000000..cb5b5e33e0a4 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/simple.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + PdbOnly + True + + + + + + + + + + +