From 360ae851516342c67acf9829373c8996e1f7e2d9 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 21 May 2021 18:18:44 -0700 Subject: [PATCH 1/3] JIT: pgo/devirt diagnostic improvements Several changes to help better diagnose PGO and devirtualization issues: * Report the source of the PGO data to the jit * Report the reason for a devirtualization failure to the jit * Add checking mode that compares result of devirtualzation to class profile * Add reporting mode to assess overall rates of devirtualization failure when the jit has exact type information. Also fix a loophole where in some case we'd still devirtualize if not optimizing. Note crossgen2 does not yet set devirtualization failure reasons. --- .../ToolBox/superpmi/mcs/verbdumpmap.cpp | 13 ++- .../ToolBox/superpmi/mcs/verbjitflags.cpp | 13 ++- .../superpmi/superpmi-shared/agnostic.h | 1 + .../superpmi-shared/methodcontext.cpp | 17 ++- .../superpmi/superpmi-shared/methodcontext.h | 12 +- .../superpmi-shim-collector/icorjitinfo.cpp | 7 +- .../superpmi-shim-counter/icorjitinfo.cpp | 5 +- .../superpmi-shim-simple/icorjitinfo.cpp | 5 +- .../ToolBox/superpmi/superpmi/icorjitinfo.cpp | 6 +- src/coreclr/inc/corinfo.h | 2 + src/coreclr/inc/corjit.h | 14 ++- src/coreclr/inc/icorjitinfoimpl_generated.h | 3 +- src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/jit/ICorJitInfo_API_wrapper.hpp | 5 +- src/coreclr/jit/compiler.cpp | 5 +- src/coreclr/jit/compiler.h | 3 + src/coreclr/jit/fgprofile.cpp | 38 ++++++- src/coreclr/jit/importer.cpp | 105 +++++++++++++++--- src/coreclr/jit/jitconfigvalues.h | 4 +- .../tools/Common/JitInterface/CorInfoBase.cs | 6 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 4 +- .../tools/Common/JitInterface/CorInfoTypes.cs | 13 +++ .../ThunkGenerator/ThunkInput.txt | 5 +- .../tools/aot/jitinterface/jitinterface.h | 7 +- src/coreclr/vm/jitinterface.cpp | 24 +++- src/coreclr/vm/jitinterface.h | 4 +- src/coreclr/vm/pgo.cpp | 13 ++- src/coreclr/vm/pgo.h | 5 +- src/coreclr/zap/zapinfo.cpp | 10 +- 29 files changed, 284 insertions(+), 75 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/mcs/verbdumpmap.cpp b/src/coreclr/ToolBox/superpmi/mcs/verbdumpmap.cpp index 4880d1b63629c..a86b85be14ff4 100644 --- a/src/coreclr/ToolBox/superpmi/mcs/verbdumpmap.cpp +++ b/src/coreclr/ToolBox/superpmi/mcs/verbdumpmap.cpp @@ -90,7 +90,8 @@ void DumpMap(int index, MethodContext* mc) bool hasEdgeProfile = false; bool hasClassProfile = false; bool hasLikelyClass = false; - if (mc->hasPgoData(hasEdgeProfile, hasClassProfile, hasLikelyClass)) + ICorJitInfo::PgoSource pgoSource = ICorJitInfo::PgoSource::Unknown; + if (mc->hasPgoData(hasEdgeProfile, hasClassProfile, hasLikelyClass, pgoSource)) { rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_PGO); @@ -108,6 +109,16 @@ void DumpMap(int index, MethodContext* mc) { rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_LIKELY_CLASS); } + + if (pgoSource == ICorJitInfo::PgoSource::Static) + { + rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_STATIC_PROFILE); + } + + if (pgoSource == ICorJitInfo::PgoSource::Dynamic) + { + rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_DYNAMIC_PROFILE); + } } printf(", %s\n", SpmiDumpHelper::DumpJitFlags(rawFlags).c_str()); diff --git a/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp b/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp index 8c0f855d5c515..eaf208508b7dd 100644 --- a/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp +++ b/src/coreclr/ToolBox/superpmi/mcs/verbjitflags.cpp @@ -30,7 +30,8 @@ int verbJitFlags::DoWork(const char* nameOfInput) bool hasEdgeProfile = false; bool hasClassProfile = false; bool hasLikelyClass = false; - if (mc->hasPgoData(hasEdgeProfile, hasClassProfile, hasLikelyClass)) + ICorJitInfo::PgoSource pgoSource = ICorJitInfo::PgoSource::Unknown; + if (mc->hasPgoData(hasEdgeProfile, hasClassProfile, hasLikelyClass, pgoSource)) { rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_PGO); @@ -48,6 +49,16 @@ int verbJitFlags::DoWork(const char* nameOfInput) { rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_LIKELY_CLASS); } + + if (pgoSource == ICorJitInfo::PgoSource::Static) + { + rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_STATIC_PROFILE); + } + + if (pgoSource == ICorJitInfo::PgoSource::Dynamic) + { + rawFlags |= 1ULL << (EXTRA_JIT_FLAGS::HAS_DYNAMIC_PROFILE); + } } int index = flagMap.GetIndex(rawFlags); diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h index 59cc7857f4c03..ad6bad793cd7b 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h @@ -491,6 +491,7 @@ struct Agnostic_GetPgoInstrumentationResults DWORD data_index; DWORD dataByteCount; DWORD result; + DWORD pgoSource; }; struct Agnostic_GetProfilingHandle diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 60eb84d931481..567537ef62481 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -5543,6 +5543,7 @@ void MethodContext::recGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, BYTE** pInstrumentationData, + ICorJitInfo::PgoSource* pPgoSource, HRESULT result) { if (GetPgoInstrumentationResults == nullptr) @@ -5571,6 +5572,7 @@ void MethodContext::recGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd value.data_index = GetPgoInstrumentationResults->AddBuffer((unsigned char*)*pInstrumentationData, (unsigned)maxOffset); value.dataByteCount = (unsigned)maxOffset; value.result = (DWORD)result; + value.pgoSource = (DWORD)*pPgoSource; DWORDLONG key = CastHandle(ftnHnd); GetPgoInstrumentationResults->Add(key, value); @@ -5578,8 +5580,8 @@ void MethodContext::recGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd } void MethodContext::dmpGetPgoInstrumentationResults(DWORDLONG key, const Agnostic_GetPgoInstrumentationResults& value) { - printf("GetPgoInstrumentationResults key ftn-%016llX, value res-%08X schemaCnt-%u profileBufSize-%u schema{", - key, value.result, value.countSchemaItems, value.dataByteCount); + printf("GetPgoInstrumentationResults key ftn-%016llX, value res-%08X schemaCnt-%u profileBufSize-%u source-%u schema{", + key, value.result, value.countSchemaItems, value.dataByteCount, value.pgoSource); if (value.countSchemaItems > 0) { @@ -5636,7 +5638,8 @@ void MethodContext::dmpGetPgoInstrumentationResults(DWORDLONG key, const Agnosti HRESULT MethodContext::repGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, - BYTE** pInstrumentationData) + BYTE** pInstrumentationData, + ICorJitInfo::PgoSource* pPgoSource) { DWORDLONG key = CastHandle(ftnHnd); AssertMapAndKeyExist(GetPgoInstrumentationResults, key, ": key %016llX", key); @@ -5646,6 +5649,7 @@ HRESULT MethodContext::repGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftn *pCountSchemaItems = (UINT32)tempValue.countSchemaItems; *pInstrumentationData = (BYTE*)GetPgoInstrumentationResults->GetBuffer(tempValue.data_index); + *pPgoSource = (ICorJitInfo::PgoSource)tempValue.pgoSource; ICorJitInfo::PgoInstrumentationSchema* pOutSchema = (ICorJitInfo::PgoInstrumentationSchema*)AllocJitTempBuffer(tempValue.countSchemaItems * sizeof(ICorJitInfo::PgoInstrumentationSchema)); @@ -6814,7 +6818,8 @@ int MethodContext::dumpMethodIdentityInfoToBuffer(char* buff, int len, bool igno ICorJitInfo::PgoInstrumentationSchema* schema = nullptr; UINT32 schemaCount = 0; BYTE* schemaData = nullptr; - HRESULT pgoHR = repGetPgoInstrumentationResults(pInfo->ftn, &schema, &schemaCount, &schemaData); + ICorJitInfo::PgoSource pgoSource = ICorJitInfo::PgoSource::Unknown; + HRESULT pgoHR = repGetPgoInstrumentationResults(pInfo->ftn, &schema, &schemaCount, &schemaData, &pgoSource); size_t minOffset = (size_t) ~0; size_t maxOffset = 0; @@ -6902,7 +6907,7 @@ int MethodContext::dumpMD5HashToBuffer(BYTE* pBuffer, int bufLen, char* hash, in return m_hash.HashBuffer(pBuffer, bufLen, hash, hashLen); } -bool MethodContext::hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile, bool& hasLikelyClass) +bool MethodContext::hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile, bool& hasLikelyClass, ICorJitInfo::PgoSource& pgoSource) { hasEdgeProfile = false; hasClassProfile = false; @@ -6919,7 +6924,7 @@ bool MethodContext::hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile, bool ICorJitInfo::PgoInstrumentationSchema* schema = nullptr; UINT32 schemaCount = 0; BYTE* schemaData = nullptr; - HRESULT pgoHR = repGetPgoInstrumentationResults(info.ftn, &schema, &schemaCount, &schemaData); + HRESULT pgoHR = repGetPgoInstrumentationResults(info.ftn, &schema, &schemaCount, &schemaData, &pgoSource); if (SUCCEEDED(pgoHR)) { diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h index c9cfddd11673a..6970b163cdec8 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -50,7 +50,9 @@ enum EXTRA_JIT_FLAGS HAS_PGO = 63, HAS_EDGE_PROFILE = 62, HAS_CLASS_PROFILE = 61, - HAS_LIKELY_CLASS = 60 + HAS_LIKELY_CLASS = 60, + HAS_STATIC_PROFILE = 59, + HAS_DYNAMIC_PROFILE = 58 }; // Asserts to catch changes in corjit flags definitions. @@ -59,6 +61,8 @@ static_assert((int)EXTRA_JIT_FLAGS::HAS_PGO == (int)CORJIT_FLAGS::CorJitFlag::CO static_assert((int)EXTRA_JIT_FLAGS::HAS_EDGE_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED35, "Jit Flags Mismatch"); static_assert((int)EXTRA_JIT_FLAGS::HAS_CLASS_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED34, "Jit Flags Mismatch"); static_assert((int)EXTRA_JIT_FLAGS::HAS_LIKELY_CLASS == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED33, "Jit Flags Mismatch"); +static_assert((int)EXTRA_JIT_FLAGS::HAS_STATIC_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED32, "Jit Flags Mismatch"); +static_assert((int)EXTRA_JIT_FLAGS::HAS_DYNAMIC_PROFILE == (int)CORJIT_FLAGS::CorJitFlag::CORJIT_FLAG_UNUSED31, "Jit Flags Mismatch"); class MethodContext { @@ -99,7 +103,7 @@ class MethodContext int dumpMethodIdentityInfoToBuffer(char* buff, int len, bool ignoreMethodName = false, CORINFO_METHOD_INFO* optInfo = nullptr, unsigned optFlags = 0); int dumpMethodMD5HashToBuffer(char* buff, int len, bool ignoreMethodName = false, CORINFO_METHOD_INFO* optInfo = nullptr, unsigned optFlags = 0); - bool hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile, bool& hasLikelyClass); + bool hasPgoData(bool& hasEdgeProfile, bool& hasClassProfile, bool& hasLikelyClass, ICorJitInfo::PgoSource& pgoSource); void recGlobalContext(const MethodContext& other); @@ -699,9 +703,9 @@ class MethodContext void dmpAllocPgoInstrumentationBySchema(DWORDLONG key, const Agnostic_AllocPgoInstrumentationBySchema& value); HRESULT repAllocPgoInstrumentationBySchema(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema* pSchema, UINT32 countSchemaItems, BYTE** pInstrumentationData); - void recGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, BYTE** pInstrumentationData, HRESULT result); + void recGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, BYTE** pInstrumentationData, ICorJitInfo::PgoSource* pPgoSource, HRESULT result); void dmpGetPgoInstrumentationResults(DWORDLONG key, const Agnostic_GetPgoInstrumentationResults& value); - HRESULT repGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, BYTE** pInstrumentationData); + HRESULT repGetPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, UINT32* pCountSchemaItems, BYTE** pInstrumentationData, ICorJitInfo::PgoSource* pPgoSource); void recMergeClasses(CORINFO_CLASS_HANDLE cls1, CORINFO_CLASS_HANDLE cls2, CORINFO_CLASS_HANDLE result); void dmpMergeClasses(DLDL key, DWORDLONG value); diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp index d7a3b48fdacf5..cd601a2621333 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -2067,11 +2067,12 @@ HRESULT interceptor_ICJI::allocPgoInstrumentationBySchema(CORINFO_METHOD_HANDLE HRESULT interceptor_ICJI::getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, PgoInstrumentationSchema **pSchema, // pointer to the schema table which describes the instrumentation results (pointer will not remain valid after jit completes) uint32_t * pCountSchemaItems, // pointer to the count schema items - uint8_t ** pInstrumentationData) // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + uint8_t ** pInstrumentationData, // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + PgoSource* pPgoSource) { mc->cr->AddCall("getPgoInstrumentationResults"); - HRESULT temp = original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); - mc->recGetPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, temp); + HRESULT temp = original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pPgoSource); + mc->recGetPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pPgoSource, temp); return temp; } diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp index 46ab7cb848915..0c9a83911481b 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp @@ -1329,10 +1329,11 @@ JITINTERFACE_HRESULT interceptor_ICJI::getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, - uint8_t** pInstrumentationData) + uint8_t** pInstrumentationData, + ICorJitInfo::PgoSource* pgoSource) { mcs->AddCall("getPgoInstrumentationResults"); - return original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); + return original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pgoSource); } JITINTERFACE_HRESULT interceptor_ICJI::allocPgoInstrumentationBySchema( diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp index 1ab397077f06c..1fd27c307ac8b 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp @@ -1164,9 +1164,10 @@ JITINTERFACE_HRESULT interceptor_ICJI::getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, - uint8_t** pInstrumentationData) + uint8_t** pInstrumentationData, + ICorJitInfo::PgoSource* pgoSource) { - return original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); + return original_ICorJitInfo->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pgoSource); } JITINTERFACE_HRESULT interceptor_ICJI::allocPgoInstrumentationBySchema( diff --git a/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp index ce20edd00be1d..f1eb73b8915c9 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp @@ -1815,10 +1815,12 @@ HRESULT MyICJI::allocPgoInstrumentationBySchema(CORINFO_METHOD_HANDLE ftnHnd, HRESULT MyICJI::getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, PgoInstrumentationSchema **pSchema, // pointer to the schema table which describes the instrumentation results (pointer will not remain valid after jit completes) uint32_t * pCountSchemaItems, // pointer to the count schema items - uint8_t ** pInstrumentationData) // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + uint8_t ** pInstrumentationData, // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + PgoSource* pPgoSource) + { jitInstance->mc->cr->AddCall("getPgoInstrumentationResults"); - return jitInstance->mc->repGetPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); + return jitInstance->mc->repGetPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pPgoSource); } // Associates a native call site, identified by its offset in the native code stream, with diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 8ae242aedf928..c132f4c232c50 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1616,10 +1616,12 @@ struct CORINFO_DEVIRTUALIZATION_INFO // invariant is `resolveVirtualMethod(...) == (devirtualizedMethod != nullptr)`. // - requiresInstMethodTableArg is set to TRUE if the devirtualized method requires a type handle arg. // - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table. + // - failure reason set if devirtualization fails. // CORINFO_METHOD_HANDLE devirtualizedMethod; bool requiresInstMethodTableArg; CORINFO_CONTEXT_HANDLE exactContext; + const char* failureReason; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/inc/corjit.h b/src/coreclr/inc/corjit.h index 51fe431bbb8be..f133cf5167b2b 100644 --- a/src/coreclr/inc/corjit.h +++ b/src/coreclr/inc/corjit.h @@ -388,6 +388,17 @@ class ICorJitInfo : public ICorDynamicInfo int32_t Other; }; + enum class PgoSource + { + Unknown = 0, // PGO data source unknown + Static = 1, // PGO data comes from embedded R2R profile data + Dynamic = 2, // PGO data comes from current run + Blend = 3, // PGO data comes from blend of prior runs and current run + Text = 4, // PGO data comes from text file + IBC = 5, // PGO data from classic IBC + Sampling= 6, // PGO data derived from sampling + }; + #define DEFAULT_UNKNOWN_TYPEHANDLE 1 #define UNKNOWN_TYPEHANDLE_MIN 1 #define UNKNOWN_TYPEHANDLE_MAX 33 @@ -404,8 +415,9 @@ class ICorJitInfo : public ICorDynamicInfo PgoInstrumentationSchema **pSchema, // OUT: pointer to the schema table (array) which describes the instrumentation results // (pointer will not remain valid after jit completes). uint32_t * pCountSchemaItems, // OUT: pointer to the count of schema items in `pSchema` array. - uint8_t ** pInstrumentationData // OUT: `*pInstrumentationData` is set to the address of the instrumentation data + uint8_t ** pInstrumentationData, // OUT: `*pInstrumentationData` is set to the address of the instrumentation data // (pointer will not remain valid after jit completes). + PgoSource * pPgoSource // OUT: value describing source of pgo data ) = 0; // Allocate a profile buffer for use in the current process diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h index ebd9813e2c4fd..c65a0840d6f64 100644 --- a/src/coreclr/inc/icorjitinfoimpl_generated.h +++ b/src/coreclr/inc/icorjitinfoimpl_generated.h @@ -678,7 +678,8 @@ JITINTERFACE_HRESULT getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, - uint8_t** pInstrumentationData) override; + uint8_t** pInstrumentationData, + ICorJitInfo::PgoSource* pgoSource) override; JITINTERFACE_HRESULT allocPgoInstrumentationBySchema( CORINFO_METHOD_HANDLE ftnHnd, diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 54ed6978c7ad5..548fcf5f7f398 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 895f5d24-eb01-4aff-ad6c-1efc6a91498a */ - 0x895f5d24, - 0xeb01, - 0x4aff, - {0xad, 0x6c, 0x1e, 0xfc, 0x6a, 0x91, 0x49, 0x8a} +constexpr GUID JITEEVersionIdentifier = { /* 81a5e384-8ca5-4947-8b2e-1d76556728fd */ + 0x81a5e384, + 0x8ca5, + 0x4947, + {0x8b, 0x2e, 0x1d, 0x76, 0x55, 0x67, 0x28, 0xfd} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp b/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp index 33f33056ff639..fb79dc1844422 100644 --- a/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp +++ b/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp @@ -1617,10 +1617,11 @@ JITINTERFACE_HRESULT WrapICorJitInfo::getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, - uint8_t** pInstrumentationData) + uint8_t** pInstrumentationData, + ICorJitInfo::PgoSource* pgoSource) { API_ENTER(getPgoInstrumentationResults); - JITINTERFACE_HRESULT temp = wrapHnd->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); + JITINTERFACE_HRESULT temp = wrapHnd->getPgoInstrumentationResults(ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pgoSource); API_LEAVE(getPgoInstrumentationResults); return temp; } diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index b92b0c1977b61..046bfd7673ee1 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2865,11 +2865,12 @@ void Compiler::compInitOptions(JitFlags* jitFlags) fgPgoSchemaCount = 0; fgPgoQueryResult = E_FAIL; fgPgoFailReason = nullptr; + fgPgoSource = ICorJitInfo::PgoSource::Unknown; if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT)) { fgPgoQueryResult = info.compCompHnd->getPgoInstrumentationResults(info.compMethodHnd, &fgPgoSchema, - &fgPgoSchemaCount, &fgPgoData); + &fgPgoSchemaCount, &fgPgoData, &fgPgoSource); // a failed result that also has a non-NULL fgPgoSchema // indicates that the ILSize for the method no longer matches @@ -3423,7 +3424,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags) if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) && fgHaveProfileData()) { - printf("OPTIONS: optimized using profile data\n"); + printf("OPTIONS: optimized using %s profile data\n", pgoSourceToString(fgPgoSource)); } if (fgPgoFailReason != nullptr) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b867331ff2110..a4fcb4fbcbb70 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5752,6 +5752,7 @@ class Compiler public: const char* fgPgoFailReason; bool fgPgoDisabled; + ICorJitInfo::PgoSource fgPgoSource; ICorJitInfo::PgoInstrumentationSchema* fgPgoSchema; BYTE* fgPgoData; UINT32 fgPgoSchemaCount; @@ -9392,6 +9393,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX } } + const char* pgoSourceToString(ICorJitInfo::PgoSource p); + #endif // DEBUG // clang-format off diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index ccd12f4fcc9da..6ee94d7f3ce11 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -1638,8 +1638,6 @@ PhaseStatus Compiler::fgInstrumentMethod() HRESULT res = info.compCompHnd->allocPgoInstrumentationBySchema(info.compMethodHnd, schema.data(), (UINT32)schema.size(), &profileMemory); - JITDUMP("Instrumentation data base address is %p\n", dspPtr(profileMemory)); - // Deal with allocation failures. // if (!SUCCEEDED(res)) @@ -1661,6 +1659,8 @@ PhaseStatus Compiler::fgInstrumentMethod() return PhaseStatus::MODIFIED_NOTHING; } + JITDUMP("Instrumentation data base address is %p\n", dspPtr(profileMemory)); + // Add the instrumentation code // for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->bbNext) @@ -1733,8 +1733,8 @@ PhaseStatus Compiler::fgIncorporateProfileData() // Summarize profile data // - JITDUMP("Have profile data: %d schema records (schema at %p, data at %p)\n", fgPgoSchemaCount, dspPtr(fgPgoSchema), - dspPtr(fgPgoData)); + JITDUMP("Have %s profile data: %d schema records (schema at %p, data at %p)\n", pgoSourceToString(fgPgoSource), + fgPgoSchemaCount, dspPtr(fgPgoSchema), dspPtr(fgPgoData)); fgNumProfileRuns = 0; unsigned otherRecords = 0; @@ -3926,4 +3926,34 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block) return missingEdges == 0; } +const char* Compiler::pgoSourceToString(ICorJitInfo::PgoSource p) +{ + const char* pgoSource = "unknown"; + switch (fgPgoSource) + { + case ICorJitInfo::PgoSource::Dynamic: + pgoSource = "dynamic"; + break; + case ICorJitInfo::PgoSource::Static: + pgoSource = "static"; + break; + case ICorJitInfo::PgoSource::Text: + pgoSource = "text"; + break; + case ICorJitInfo::PgoSource::Blend: + pgoSource = "static+dynamic"; + break; + case ICorJitInfo::PgoSource::IBC: + pgoSource = "IBC"; + break; + case ICorJitInfo::PgoSource::Sampling: + pgoSource = "Sampling"; + break; + default: + break; + } + + return pgoSource; +} + #endif // DEBUG diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index b929424c56089..d3e81101ad76b 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1,3 +1,4 @@ + // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. @@ -20719,19 +20720,21 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, assert(pContextHandle != nullptr); // This should be a virtual vtable or virtual stub call. + // assert(call->IsVirtual()); // Possibly instrument, if not optimizing. // - if (opts.OptimizationDisabled() && (call->gtCallType != CT_INDIRECT)) + if (opts.OptimizationDisabled()) { // During importation, optionally flag this block as one that // contains calls requiring class profiling. Ideally perhaps // we'd just keep track of the calls themselves, so we don't // have to search for them later. // - if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && !opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT) && - (JitConfig.JitClassProfiling() > 0) && !isLateDevirtualization) + if ((call->gtCallType != CT_INDIRECT) && opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && + !opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT) && (JitConfig.JitClassProfiling() > 0) && + !isLateDevirtualization) { JITDUMP("\n ... marking [%06u] in " FMT_BB " for class profile instrumentation\n", dspTreeID(call), compCurBB->bbNum); @@ -20769,7 +20772,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, const bool doPrint = JitConfig.JitPrintDevirtualizedMethods().contains(rootCompiler->info.compMethodName, rootCompiler->info.compClassName, &rootCompiler->info.compMethodInfo->args); - #endif // DEBUG // Fetch information about the virtual method we're calling. @@ -20905,6 +20907,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, dvInfo.virtualMethod = baseMethod; dvInfo.objClass = objClass; dvInfo.context = *pContextHandle; + dvInfo.failureReason = "unspecified"; info.compCompHnd->resolveVirtualMethod(&dvInfo); @@ -20912,9 +20915,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; CORINFO_CLASS_HANDLE derivedClass = NO_CLASS_HANDLE; - if (exactContext != nullptr) + if (derivedMethod != nullptr) { - // We currently expect the context to always be a class context. + assert(exactContext != nullptr); assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS); derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); } @@ -20936,7 +20939,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // if (derivedMethod == nullptr) { - JITDUMP("--- no derived method\n"); + JITDUMP("--- no derived method: %s\n", dvInfo.failureReason); } else { @@ -20980,6 +20983,17 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, { JITDUMP(" Class not final or exact%s\n", isInterface ? "" : ", and method not final"); +#if defined(DEBUG) + // If we know the object type exactly, we generally expect we can devirtualize. + // (don't when doing late devirt as we won't have an owner type (yet)) + // + if (!isLateDevirtualization && (isExact || objClassIsFinal) && JitConfig.JitNoteFailedExactDevirtualization()) + { + printf("@@@ Exact/Final devirt failure in %s at [%06u] $ %s\n", info.compFullName, dspTreeID(call), + dvInfo.failureReason); + } +#endif + // Don't try guarded devirtualiztion if we're doing late devirtualization. // if (isLateDevirtualization) @@ -20994,18 +21008,16 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, } // All checks done. Time to transform the call. + // + // We should always have an exact class context. + // + // Note that wouldnt' be true if the runtime side supported array interface devirt, + // the resulting method would be a generic method of the non-generic SZArrayHelper class. + // assert(canDevirtualize); JITDUMP(" %s; can devirtualize\n", note); - // See if the method we're devirtualizing to is an intrinsic. - // - if (derivedMethodAttribs & (CORINFO_FLG_JIT_INTRINSIC | CORINFO_FLG_INTRINSIC)) - { - JITDUMP("!!! Devirt to intrinsic in %s, calling %s::%s\n", impInlineRoot()->info.compFullName, derivedClassName, - derivedMethodName); - } - // Make the updates. call->gtFlags &= ~GTF_CALL_VIRT_VTABLE; call->gtFlags &= ~GTF_CALL_VIRT_STUB; @@ -21037,6 +21049,67 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, printf("Devirtualized %s call to %s:%s; now direct call to %s:%s [%s]\n", callKind, baseClassName, baseMethodName, derivedClassName, derivedMethodName, note); } + + // If we successfully devirtualized based on an exact or final class, + // and we have dynamic PGO data describing the likely class, make sure they agree. + // + // If pgo source is not dynamic we may see likely classes from other versions of this code + // where types had different properties. + // + // If method is an inlinee we may be specializing to a class that wasn't seen at runtime. + // + const bool canSensiblyCheck = + (isExact || objClassIsFinal) && (fgPgoSource == ICorJitInfo::PgoSource::Dynamic) && !compIsForInlining(); + if (JitConfig.JitCrossCheckDevirtualizationAndPGO() && canSensiblyCheck) + { + unsigned likelihood = 0; + unsigned numberOfClasses = 0; + + CORINFO_CLASS_HANDLE likelyClass = + getLikelyClass(fgPgoSchema, fgPgoSchemaCount, fgPgoData, ilOffset, &likelihood, &numberOfClasses); + + if (likelyClass != NO_CLASS_HANDLE) + { + // PGO had better agree the class we devirtualized to is plausible. + // + if (likelyClass != derivedClass) + { + // Managed type system may report different addresses for a class handle + // at different times....? + // + // Also, AOT may have a more nuanced notion of class equality. + // + if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT)) + { + bool mismatch = true; + + // derivedClass will be the introducer of derived method, so it's possible + // likelyClass is a non-overriding subclass. Check up the hierarchy. + // + CORINFO_CLASS_HANDLE parentClass = likelyClass; + while (parentClass != NO_CLASS_HANDLE) + { + if (parentClass == derivedClass) + { + mismatch = false; + break; + } + + parentClass = info.compCompHnd->getParentType(parentClass); + } + + if (mismatch || (numberOfClasses != 1) || (likelihood != 100)) + { + printf("@@@ Likely %p (%s) != Derived %p (%s) [n=%u, l=%u, il=%u] in %s \n", likelyClass, + eeGetClassName(likelyClass), derivedClass, eeGetClassName(derivedClass), numberOfClasses, + likelihood, ilOffset, info.compFullName); + } + + assert(!(mismatch || (numberOfClasses != 1) || (likelihood != 100))); + } + } + } + } #endif // defined(DEBUG) // If the 'this' object is a value class, see if we can rework the call to invoke the @@ -21046,6 +21119,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // We won't optimize explicit tail calls, as ensuring we get the right tail call info // is tricky (we'd need to pass an updated sig and resolved token back to some callers). // + // Note we may not have a derived class in some cases (eg interface call on an array) + // if (info.compCompHnd->isValueClass(derivedClass)) { if (isExplicitTailCall) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index ba3ab23b8bdd8..7c5c84772a665 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -486,7 +486,9 @@ CONFIG_INTEGER(JitCollect64BitCounts, W("JitCollect64BitCounts"), 0) // Collect CONFIG_INTEGER(JitDisablePgo, W("JitDisablePgo"), 0) // Ignore pgo data for all methods #if defined(DEBUG) CONFIG_STRING(JitEnablePgoRange, W("JitEnablePgoRange")) // Enable pgo data for only some methods -#endif // debug +CONFIG_INTEGER(JitCrossCheckDevirtualizationAndPGO, W("JitCrossCheckDevirtualizationAndPGO"), 0) +CONFIG_INTEGER(JitNoteFailedExactDevirtualization, W("JitNoteFailedExactDevirtualization"), 0) +#endif // debug // Control when Virtual Calls are expanded CONFIG_INTEGER(JitExpandCallsEarly, W("JitExpandCallsEarly"), 1) // Expand Call targets early (in the global morph diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs b/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs index 7125dfc59b904..d450ac74e6b03 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs @@ -2446,12 +2446,12 @@ static void _reportFatalError(IntPtr thisHandle, IntPtr* ppException, CorJitResu } [UnmanagedCallersOnly] - static HRESULT _getPgoInstrumentationResults(IntPtr thisHandle, IntPtr* ppException, CORINFO_METHOD_STRUCT_* ftnHnd, PgoInstrumentationSchema** pSchema, uint* pCountSchemaItems, byte** pInstrumentationData) + static HRESULT _getPgoInstrumentationResults(IntPtr thisHandle, IntPtr* ppException, CORINFO_METHOD_STRUCT_* ftnHnd, PgoInstrumentationSchema** pSchema, uint* pCountSchemaItems, byte** pInstrumentationData, PgoSource* pgoSource) { var _this = GetThis(thisHandle); try { - return _this.getPgoInstrumentationResults(ftnHnd, ref *pSchema, ref *pCountSchemaItems, pInstrumentationData); + return _this.getPgoInstrumentationResults(ftnHnd, ref *pSchema, ref *pCountSchemaItems, pInstrumentationData, ref *pgoSource); } catch (Exception ex) { @@ -2718,7 +2718,7 @@ static IntPtr GetUnmanagedCallbacks() callbacks[162] = (delegate* unmanaged)&_logMsg; callbacks[163] = (delegate* unmanaged)&_doAssert; callbacks[164] = (delegate* unmanaged)&_reportFatalError; - callbacks[165] = (delegate* unmanaged)&_getPgoInstrumentationResults; + callbacks[165] = (delegate* unmanaged)&_getPgoInstrumentationResults; callbacks[166] = (delegate* unmanaged)&_allocPgoInstrumentationBySchema; callbacks[167] = (delegate* unmanaged)&_recordCallSite; callbacks[168] = (delegate* unmanaged)&_recordRelocation; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 65137fad8c029..d3e414f417922 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3542,7 +3542,8 @@ public static void ComputeJitPgoInstrumentationSchema(Func objec instrumentationData = msInstrumentationData.ToArray(); } - private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref PgoInstrumentationSchema* pSchema, ref uint countSchemaItems, byte** pInstrumentationData) + private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref PgoInstrumentationSchema* pSchema, ref uint countSchemaItems, byte** pInstrumentationData, + ref PgoSource pPgoSource) { MethodDesc methodDesc = HandleToObject(ftnHnd); @@ -3569,6 +3570,7 @@ private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref pSchema = pgoResults.pSchema; countSchemaItems = pgoResults.countSchemaItems; *pInstrumentationData = pgoResults.pInstrumentationData; + pPgoSource = PgoSource.Static; return pgoResults.hr; } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index f779093898227..59750ea800cce 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -313,6 +313,17 @@ public struct PgoInstrumentationSchema public int Other; } + public enum PgoSource + { + Unknown = 0, + Static = 1, + Dynamic = 2, + Blend = 3, + Text = 4, + IBC = 5, + Sampling = 6, + } + [StructLayout(LayoutKind.Sequential)] public unsafe struct AllocMemArgs { @@ -1074,11 +1085,13 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO // invariant is `resolveVirtualMethod(...) == (devirtualizedMethod != nullptr)`. // - requiresInstMethodTableArg is set to TRUE if the devirtualized method requires a type handle arg. // - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table. + // - failureReason is optionally set to asciiz string describing why devirtualization failed // public CORINFO_METHOD_STRUCT_* devirtualizedMethod; public byte _requiresInstMethodTableArg; public bool requiresInstMethodTableArg { get { return _requiresInstMethodTableArg != 0; } set { _requiresInstMethodTableArg = value ? (byte)1 : (byte)0; } } public CORINFO_CONTEXT_STRUCT* exactContext; + public byte* failureReason; } //---------------------------------------------------------------------------- diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt index 18ac90d0b459e..a15819e7569e3 100644 --- a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt +++ b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt @@ -100,7 +100,6 @@ CORINFO_JUST_MY_CODE_HANDLE**,ref CORINFO_JUST_MY_CODE_HANDLE_* ICorJitInfo::PgoInstrumentationSchema**,ref PgoInstrumentationSchema* ICorJitInfo::PgoInstrumentationSchema*,PgoInstrumentationSchema* - AllocMemArgs*, ref AllocMemArgs ; Enums @@ -128,6 +127,8 @@ CorJitFuncKind CorJitResult TypeCompareState CORINFO_InstructionSet,InstructionSet +ICorJitInfo::PgoSource, PgoSource +ICorJitInfo::PgoSource*, ref PgoSource ; Handle types CORINFO_MODULE_HANDLE,CORINFO_MODULE_STRUCT_* @@ -317,7 +318,7 @@ FUNCTIONS bool logMsg(unsigned level, const char* fmt, va_list args) int doAssert(const char* szFile, int iLine, const char* szExpr) void reportFatalError(CorJitResult result) - JITINTERFACE_HRESULT getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, uint8_t**pInstrumentationData) + JITINTERFACE_HRESULT getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, uint8_t**pInstrumentationData, ICorJitInfo::PgoSource* pgoSource) JITINTERFACE_HRESULT allocPgoInstrumentationBySchema(CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema* pSchema, uint32_t countSchemaItems, uint8_t** pInstrumentationData) void recordCallSite(uint32_t instrOffset, CORINFO_SIG_INFO* callSig, CORINFO_METHOD_HANDLE methodHandle) void recordRelocation(void* location, void* locationRW, void* target, uint16_t fRelocType, uint16_t slotNum, int32_t addlDelta) diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface.h b/src/coreclr/tools/aot/jitinterface/jitinterface.h index 78daddda066b5..19e6073afacf4 100644 --- a/src/coreclr/tools/aot/jitinterface/jitinterface.h +++ b/src/coreclr/tools/aot/jitinterface/jitinterface.h @@ -176,7 +176,7 @@ struct JitInterfaceCallbacks bool (* logMsg)(void * thisHandle, CorInfoExceptionClass** ppException, unsigned level, const char* fmt, va_list args); int (* doAssert)(void * thisHandle, CorInfoExceptionClass** ppException, const char* szFile, int iLine, const char* szExpr); void (* reportFatalError)(void * thisHandle, CorInfoExceptionClass** ppException, CorJitResult result); - JITINTERFACE_HRESULT (* getPgoInstrumentationResults)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, uint8_t** pInstrumentationData); + JITINTERFACE_HRESULT (* getPgoInstrumentationResults)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, uint8_t** pInstrumentationData, ICorJitInfo::PgoSource* pgoSource); JITINTERFACE_HRESULT (* allocPgoInstrumentationBySchema)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema* pSchema, uint32_t countSchemaItems, uint8_t** pInstrumentationData); void (* recordCallSite)(void * thisHandle, CorInfoExceptionClass** ppException, uint32_t instrOffset, CORINFO_SIG_INFO* callSig, CORINFO_METHOD_HANDLE methodHandle); void (* recordRelocation)(void * thisHandle, CorInfoExceptionClass** ppException, void* location, void* locationRW, void* target, uint16_t fRelocType, uint16_t slotNum, int32_t addlDelta); @@ -1786,10 +1786,11 @@ class JitInterfaceWrapper : public ICorJitInfo CORINFO_METHOD_HANDLE ftnHnd, ICorJitInfo::PgoInstrumentationSchema** pSchema, uint32_t* pCountSchemaItems, - uint8_t** pInstrumentationData) + uint8_t** pInstrumentationData, + ICorJitInfo::PgoSource* pgoSource) { CorInfoExceptionClass* pException = nullptr; - JITINTERFACE_HRESULT temp = _callbacks->getPgoInstrumentationResults(_thisHandle, &pException, ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData); + JITINTERFACE_HRESULT temp = _callbacks->getPgoInstrumentationResults(_thisHandle, &pException, ftnHnd, pSchema, pCountSchemaItems, pInstrumentationData, pgoSource); if (pException != nullptr) throw pException; return temp; } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index a8b5981d5adf1..ff729d9118c96 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8926,6 +8926,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Can't devirtualize from __Canon. if (ObjClassHnd == TypeHandle(g_pCanonMethodTableClass)) { + info->failureReason = "object class is canonical"; return false; } @@ -8936,7 +8937,8 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Don't try and devirtualize com interface calls. if (pObjMT->IsComObjectType()) { - return nullptr; + info->failureReason = "object class is Com"; + return false; } #endif // FEATURE_COMINTEROP @@ -8946,6 +8948,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // interface corresponding to pBaseMD. if (!pObjMT->CanCastToInterface(pBaseMT)) { + info->failureReason = "can't cast obj to interface type"; return false; } @@ -8972,6 +8975,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (pDevirtMD == nullptr) { + info->failureReason = "can't find interface method"; return false; } @@ -8980,6 +8984,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Making this work is tracked by https://github.com/dotnet/runtime/issues/9588 if (pDevirtMD->GetMethodTable()->IsInterface() && pDevirtMD->HasClassInstantiation()) { + info->failureReason = "default interface method"; return false; } } @@ -9002,6 +9007,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (pCheckMT == nullptr) { + info->failureReason = "object class not subclass"; return false; } @@ -9025,6 +9031,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (dslot != slot) { + info->failureReason = "explicit slot overeride"; return false; } } @@ -9064,6 +9071,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (!allowDevirt) { + info->failureReason = "crosses version bubble"; return false; } } @@ -9074,6 +9082,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD; info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->requiresInstMethodTableArg = false; + info->failureReason = "success"; return true; } @@ -12122,8 +12131,9 @@ HRESULT CEEJitInfo::allocPgoInstrumentationBySchema( HRESULT CEEJitInfo::getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, PgoInstrumentationSchema **pSchema, // pointer to the schema table which describes the instrumentation results (pointer will not remain valid after jit completes) - uint32_t * pCountSchemaItems, // pointer to the count schema items - uint8_t ** pInstrumentationData // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + uint32_t * pCountSchemaItems, // pointer to the count schema items + uint8_t ** pInstrumentationData, // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + PgoSource * pPgoSource // source of pgo data ) { CONTRACTL { @@ -12135,6 +12145,7 @@ HRESULT CEEJitInfo::getPgoInstrumentationResults( HRESULT hr = E_FAIL; *pCountSchemaItems = 0; *pInstrumentationData = NULL; + *pPgoSource = PgoSource::Unknown; JIT_TO_EE_TRANSITION(); @@ -12160,13 +12171,15 @@ HRESULT CEEJitInfo::getPgoInstrumentationResults( m_foundPgoData = newPgoData; newPgoData.SuppressRelease(); - newPgoData->m_hr = PgoManager::getPgoInstrumentationResults(pMD, &newPgoData->m_allocatedData, &newPgoData->m_schema, &newPgoData->m_cSchemaElems, &newPgoData->m_pInstrumentationData); + newPgoData->m_hr = PgoManager::getPgoInstrumentationResults(pMD, &newPgoData->m_allocatedData, &newPgoData->m_schema, + &newPgoData->m_cSchemaElems, &newPgoData->m_pInstrumentationData, &newPgoData->m_pgoSource); pDataCur = m_foundPgoData; } *pSchema = pDataCur->m_schema; *pCountSchemaItems = pDataCur->m_cSchemaElems; *pInstrumentationData = pDataCur->m_pInstrumentationData; + *pPgoSource = pDataCur->m_pgoSource; hr = pDataCur->m_hr; #else _ASSERTE(!"getPgoInstrumentationResults not implemented on CEEJitInfo!"); @@ -14408,7 +14421,8 @@ HRESULT CEEInfo::getPgoInstrumentationResults( CORINFO_METHOD_HANDLE ftnHnd, PgoInstrumentationSchema **pSchema, // pointer to the schema table which describes the instrumentation results (pointer will not remain valid after jit completes) uint32_t * pCountSchemaItems, // pointer to the count schema items - uint8_t ** pInstrumentationData // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + uint8_t ** pInstrumentationData, // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + PgoSource * pPgoSource ) { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 56cb5befac401..21cd4597c711f 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -676,7 +676,8 @@ class CEEJitInfo : public CEEInfo CORINFO_METHOD_HANDLE ftnHnd, /* IN */ PgoInstrumentationSchema** pSchema, /* OUT */ uint32_t* pCountSchemaItems, /* OUT */ - uint8_t**pInstrumentationData /* OUT */ + uint8_t**pInstrumentationData, /* OUT */ + PgoSource *pPgoSource /* OUT */ ) override final; void recordCallSite( @@ -927,6 +928,7 @@ protected : UINT32 m_cSchemaElems; BYTE *m_pInstrumentationData = nullptr; HRESULT m_hr = E_NOTIMPL; + PgoSource m_pgoSource = PgoSource::Unknown; }; ComputedPgoData* m_foundPgoData = nullptr; #endif diff --git a/src/coreclr/vm/pgo.cpp b/src/coreclr/vm/pgo.cpp index 747b4cb1b0d19..2ba24960f09ac 100644 --- a/src/coreclr/vm/pgo.cpp +++ b/src/coreclr/vm/pgo.cpp @@ -685,8 +685,10 @@ HRESULT PgoManager::allocPgoInstrumentationBySchemaInstance(MethodDesc* pMD, { if (!ComparePgoSchemaEquals(existingData->header.GetData(), existingData->header.countsOffset, pSchema, countSchemaItems)) { + printf("@@@ Alloc failing, existing schema clash...\n"); return E_NOTIMPL; } + printf("@@@ Alloc returning existing schema + data...\n"); *pInstrumentationData = existingData->header.GetData(); return S_OK; } @@ -710,12 +712,13 @@ HRESULT PgoManager::allocPgoInstrumentationBySchemaInstance(MethodDesc* pMD, } #ifndef DACCESS_COMPILE -HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData) +HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData, ICorJitInfo::PgoSource *pPgoSource) { // Initialize our out params *pAllocatedData = NULL; *pInstrumentationData = NULL; *pCountSchemaItems = 0; + *pPgoSource = ICorJitInfo::PgoSource::Unknown; PgoManager *mgr; if (!pMD->IsDynamicMethod()) @@ -730,7 +733,7 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca HRESULT hr = E_NOTIMPL; if (mgr != NULL) { - hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData); + hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData, pPgoSource); } // If not found in the data from the current run, look in the data from the text file @@ -799,6 +802,7 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca *pCountSchemaItems = schemaArray.GetCount(); *pInstrumentationData = found->GetData(); + *pPgoSource = ICorJitInfo::PgoSource::Text; hr = S_OK; } EX_CATCH @@ -951,12 +955,13 @@ HRESULT PgoManager::getPgoInstrumentationResultsFromR2RFormat(ReadyToRunInfo *pR } #endif // DACCESS_COMPILE -HRESULT PgoManager::getPgoInstrumentationResultsInstance(MethodDesc* pMD, BYTE** pAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData) +HRESULT PgoManager::getPgoInstrumentationResultsInstance(MethodDesc* pMD, BYTE** pAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData, ICorJitInfo::PgoSource* pPgoSource) { // Initialize our out params *pAllocatedData = NULL; *pInstrumentationData = NULL; *pCountSchemaItems = 0; + *pPgoSource = ICorJitInfo::PgoSource::Unknown; HeaderList *found; @@ -977,6 +982,7 @@ HRESULT PgoManager::getPgoInstrumentationResultsInstance(MethodDesc* pMD, BYTE** // Consider merging this data with the live data instead in the future if (pMD->GetModule()->IsReadyToRun() && pMD->GetModule()->GetReadyToRunInfo()->GetPgoInstrumentationData(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData)) { + *pPgoSource = ICorJitInfo::PgoSource::Static; return S_OK; } @@ -1010,6 +1016,7 @@ HRESULT PgoManager::getPgoInstrumentationResultsInstance(MethodDesc* pMD, BYTE** { *pInstrumentationDataDst = *pSrc; } + *pPgoSource = ICorJitInfo::PgoSource::Dynamic; return S_OK; } else diff --git a/src/coreclr/vm/pgo.h b/src/coreclr/vm/pgo.h index c7c5e1cb754f7..38e6f4b89f33f 100644 --- a/src/coreclr/vm/pgo.h +++ b/src/coreclr/vm/pgo.h @@ -22,7 +22,7 @@ class PgoManager public: - static HRESULT getPgoInstrumentationResults(MethodDesc* pMD, BYTE **pAllocatedDatapAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData); + static HRESULT getPgoInstrumentationResults(MethodDesc* pMD, BYTE **pAllocatedDatapAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, BYTE**pInstrumentationData, ICorJitInfo::PgoSource* pPgoSource); static HRESULT allocPgoInstrumentationBySchema(MethodDesc* pMD, ICorJitInfo::PgoInstrumentationSchema* pSchema, UINT32 countSchemaItems, BYTE** pInstrumentationData); static HRESULT getPgoInstrumentationResultsFromR2RFormat(ReadyToRunInfo *pReadyToRunInfo, Module* pModule, @@ -147,7 +147,8 @@ class PgoManager BYTE** pAllocatedData, ICorJitInfo::PgoInstrumentationSchema** ppSchema, UINT32 *pCountSchemaItems, - BYTE**pInstrumentationData); + BYTE**pInstrumentationData, + ICorJitInfo::PgoSource* pPgoSource); HRESULT allocPgoInstrumentationBySchemaInstance(MethodDesc* pMD, ICorJitInfo::PgoInstrumentationSchema* pSchema, diff --git a/src/coreclr/zap/zapinfo.cpp b/src/coreclr/zap/zapinfo.cpp index 347f357c7de36..34ff7fb19d374 100644 --- a/src/coreclr/zap/zapinfo.cpp +++ b/src/coreclr/zap/zapinfo.cpp @@ -1000,8 +1000,9 @@ HRESULT ZapInfo::allocPgoInstrumentationBySchema(CORINFO_METHOD_HANDLE ftnHnd, HRESULT ZapInfo::getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, PgoInstrumentationSchema **pSchema, // pointer to the schema table which describes the instrumentation results (pointer will not remain valid after jit completes) - uint32_t * pCountSchemaItems, // pointer to the count schema items - uint8_t ** pInstrumentationData) // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + uint32_t * pCountSchemaItems, // pointer to the count schema items + uint8_t ** pInstrumentationData, // pointer to the actual instrumentation data (pointer will not remain valid after jit completes) + PgoSource* pPgoSource) { _ASSERTE(pCountSchemaItems != nullptr); _ASSERTE(pInstrumentationData != nullptr); @@ -1013,6 +1014,7 @@ HRESULT ZapInfo::getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, *pCountSchemaItems = 0; *pSchema = nullptr; *pInstrumentationData = nullptr; + *pPgoSource = PgoSource::Unknown; int32_t numRuns = 0; @@ -1134,6 +1136,7 @@ HRESULT ZapInfo::getPgoInstrumentationResults(CORINFO_METHOD_HANDLE ftnHnd, *pCountSchemaItems = pgoResults->m_schema.GetCount(); *pSchema = pgoResults->m_schema.GetElements(); *pInstrumentationData = pgoResults->pInstrumentationData; + *pPgoSource = PgoSource::IBC; return pgoResults->m_hr; } @@ -4281,6 +4284,7 @@ BOOL ZapInfo::CurrentMethodHasProfileData() UINT32 size; ICorJitInfo::PgoInstrumentationSchema * pSchema; BYTE* pData; - return SUCCEEDED(getPgoInstrumentationResults(m_currentMethodHandle, &pSchema, &size, &pData)); + ICorJitInfo::PgoSource pgoSource; + return SUCCEEDED(getPgoInstrumentationResults(m_currentMethodHandle, &pSchema, &size, &pData, &pgoSource)); } From 8899b1143936901dbb96dd0cc262c89f6d1b9b04 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 25 May 2021 17:41:30 -0700 Subject: [PATCH 2/3] change failure reason to description enum --- .../superpmi/superpmi-shared/agnostic.h | 1 + .../superpmi-shared/methodcontext.cpp | 6 ++-- src/coreclr/inc/corinfo.h | 25 ++++++++++++---- src/coreclr/jit/compiler.cpp | 30 +++++++++++++++++++ src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/fgprofile.cpp | 9 ++++++ src/coreclr/jit/importer.cpp | 6 ++-- .../tools/Common/JitInterface/CorInfoTypes.cs | 19 ++++++++++-- src/coreclr/vm/jitinterface.cpp | 18 +++++------ 9 files changed, 94 insertions(+), 21 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h index ad6bad793cd7b..4b47339b352f6 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/agnostic.h @@ -568,6 +568,7 @@ struct Agnostic_ResolveVirtualMethodResult DWORDLONG devirtualizedMethod; bool requiresInstMethodTableArg; DWORDLONG exactContext; + DWORD detail; }; struct ResolveTokenValue diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 567537ef62481..e0ab600eb3d10 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -3175,14 +3175,15 @@ void MethodContext::recResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info result.devirtualizedMethod = CastHandle(info->devirtualizedMethod); result.requiresInstMethodTableArg = info->requiresInstMethodTableArg; result.exactContext = CastHandle(info->exactContext); + result.detail = (DWORD) info->detail; ResolveVirtualMethod->Add(key, result); DEBUG_REC(dmpResolveVirtualMethod(key, result)); } void MethodContext::dmpResolveVirtualMethod(const Agnostic_ResolveVirtualMethodKey& key, const Agnostic_ResolveVirtualMethodResult& result) { - printf("ResolveVirtualMethod virtMethod-%016llX, objClass-%016llX, context-%016llX :: returnValue-%d, devirtMethod-%016llX, requiresInstArg-%d, exactContext-%016llX", - key.virtualMethod, key.objClass, key.context, result.returnValue, result.devirtualizedMethod, result.requiresInstMethodTableArg, result.exactContext); + printf("ResolveVirtualMethod virtMethod-%016llX, objClass-%016llX, context-%016llX :: returnValue-%d, devirtMethod-%016llX, requiresInstArg-%d, exactContext-%016llX, detail-%d", + key.virtualMethod, key.objClass, key.context, result.returnValue, result.devirtualizedMethod, result.requiresInstMethodTableArg, result.exactContext, result.detail); } bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info) @@ -3201,6 +3202,7 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod; info->requiresInstMethodTableArg = result.requiresInstMethodTableArg; info->exactContext = (CORINFO_CONTEXT_HANDLE) result.exactContext; + info->detail = (CORINFO_DEVIRTUALIZATION_DETAIL) result.detail; return result.returnValue; } diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index c132f4c232c50..c1d8ff98c5833 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1601,6 +1601,21 @@ struct CORINFO_CALL_INFO bool wrapperDelegateInvoke; }; +enum CORINFO_DEVIRTUALIZATION_DETAIL +{ + CORINFO_DEVIRTUALIZATION_UNKNOWN, // no details available + CORINFO_DEVIRTUALIZATION_SUCCESS, // devirtualization was successful + CORINFO_DEVIRTUALIZATION_FAILED_CANON, // object class was canonical + CORINFO_DEVIRTUALIZATION_FAILED_COM, // object class was com + CORINFO_DEVIRTUALIZATION_FAILED_CAST, // object class could not be cast to interface class + CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP, // interface method could not be found + CORINFO_DEVIRTUALIZATION_FAILED_DIM, // interface method was default interface method + CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS, // object not subclass of base class + CORINFO_DEVIRTUALIZATION_FAILED_SLOT, // virtual method installed via explicit override + CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE, // devirtualization crossed version bubble + CORINFO_DEVIRTUALIZATION_COUNT, // sentinel for maximum value +}; + struct CORINFO_DEVIRTUALIZATION_INFO { // @@ -1616,12 +1631,12 @@ struct CORINFO_DEVIRTUALIZATION_INFO // invariant is `resolveVirtualMethod(...) == (devirtualizedMethod != nullptr)`. // - requiresInstMethodTableArg is set to TRUE if the devirtualized method requires a type handle arg. // - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table. - // - failure reason set if devirtualization fails. + // - details on the computation done by the jit host // - CORINFO_METHOD_HANDLE devirtualizedMethod; - bool requiresInstMethodTableArg; - CORINFO_CONTEXT_HANDLE exactContext; - const char* failureReason; + CORINFO_METHOD_HANDLE devirtualizedMethod; + bool requiresInstMethodTableArg; + CORINFO_CONTEXT_HANDLE exactContext; + CORINFO_DEVIRTUALIZATION_DETAIL detail; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 046bfd7673ee1..c67b2f0f9d32e 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9679,3 +9679,33 @@ void Compiler::gtChangeOperToNullCheck(GenTree* tree, BasicBlock* block) block->bbFlags |= BBF_HAS_NULLCHECK; optMethodFlags |= OMF_HAS_NULLCHECK; } + +#if defined(DEBUG) +//------------------------------------------------------------------------------ +// devirtualizationDetailToString: describe the detailed devirtualization reason +// +// Arguments: +// detail - detail to describe +// +// Returns: +// descriptive string +// +const char* Compiler::devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DETAIL detail) +{ + switch (detail) + { + case CORINFO_DEVIRTUALIZATION_UNKNOWN: return "unknown"; + case CORINFO_DEVIRTUALIZATION_SUCCESS: return "success"; + case CORINFO_DEVIRTUALIZATION_FAILED_CANON: return "object class was canonical"; + case CORINFO_DEVIRTUALIZATION_FAILED_COM: return "object class was com"; + case CORINFO_DEVIRTUALIZATION_FAILED_CAST: return "object class could not be cast to interface class"; + case CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP: return "interface method could not be found"; + case CORINFO_DEVIRTUALIZATION_FAILED_DIM: return "interface method was default interface method"; + case CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS: return "object not subclass of base class"; + case CORINFO_DEVIRTUALIZATION_FAILED_SLOT: return "virtual method installed via explicit override"; + case CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE: return "devirtualization crossed version bubble"; + default: return "undefined"; + } +} +#endif // defined(DEBUG) + diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a4fcb4fbcbb70..6ddf49ce55352 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9394,6 +9394,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX } const char* pgoSourceToString(ICorJitInfo::PgoSource p); + const char* devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DETAIL detail); #endif // DEBUG diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 6ee94d7f3ce11..a43e2c686fce5 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -3926,6 +3926,15 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block) return missingEdges == 0; } +//------------------------------------------------------------------------------ +// pgoSourceToString: describe source of pgo data +// +// Arguments: +// r - source enum to describe +// +// Returns: +// descriptive string +// const char* Compiler::pgoSourceToString(ICorJitInfo::PgoSource p) { const char* pgoSource = "unknown"; diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d3e81101ad76b..0fe8910002e41 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20907,7 +20907,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, dvInfo.virtualMethod = baseMethod; dvInfo.objClass = objClass; dvInfo.context = *pContextHandle; - dvInfo.failureReason = "unspecified"; + dvInfo.detail = CORINFO_DEVIRTUALIZATION_UNKNOWN; info.compCompHnd->resolveVirtualMethod(&dvInfo); @@ -20939,7 +20939,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // if (derivedMethod == nullptr) { - JITDUMP("--- no derived method: %s\n", dvInfo.failureReason); + JITDUMP("--- no derived method: %s\n", devirtualizationDetailToString(dvInfo.detail)); } else { @@ -20990,7 +20990,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (!isLateDevirtualization && (isExact || objClassIsFinal) && JitConfig.JitNoteFailedExactDevirtualization()) { printf("@@@ Exact/Final devirt failure in %s at [%06u] $ %s\n", info.compFullName, dspTreeID(call), - dvInfo.failureReason); + devirtualizationDetailToString(dvInfo.detail)); } #endif diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index 59750ea800cce..91114435396e7 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1070,6 +1070,21 @@ public unsafe struct CORINFO_CALL_INFO public bool wrapperDelegateInvoke { get { return _wrapperDelegateInvoke != 0; } set { _wrapperDelegateInvoke = value ? (byte)1 : (byte)0; } } } + public enum CORINFO_DEVIRTUALIZATION_DETAIL + { + CORINFO_DEVIRTUALIZATION_UNKNOWN, // no details available + CORINFO_DEVIRTUALIZATION_SUCCESS, // devirtualization was successful + CORINFO_DEVIRTUALIZATION_FAILED_CANON, // object class was canonical + CORINFO_DEVIRTUALIZATION_FAILED_COM, // object class was com + CORINFO_DEVIRTUALIZATION_FAILED_CAST, // object class could not be cast to interface class + CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP, // interface method could not be found + CORINFO_DEVIRTUALIZATION_FAILED_DIM, // interface method was default interface method + CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS, // object not subclass of base class + CORINFO_DEVIRTUALIZATION_FAILED_SLOT, // virtual method installed via explicit override + CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE, // devirtualization crossed version bubble + CORINFO_DEVIRTUALIZATION_COUNT, // sentinel for maximum value + } + public unsafe struct CORINFO_DEVIRTUALIZATION_INFO { // @@ -1085,13 +1100,13 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO // invariant is `resolveVirtualMethod(...) == (devirtualizedMethod != nullptr)`. // - requiresInstMethodTableArg is set to TRUE if the devirtualized method requires a type handle arg. // - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table. - // - failureReason is optionally set to asciiz string describing why devirtualization failed + // - detail describes the computation done by the jit host // public CORINFO_METHOD_STRUCT_* devirtualizedMethod; public byte _requiresInstMethodTableArg; public bool requiresInstMethodTableArg { get { return _requiresInstMethodTableArg != 0; } set { _requiresInstMethodTableArg = value ? (byte)1 : (byte)0; } } public CORINFO_CONTEXT_STRUCT* exactContext; - public byte* failureReason; + public CORINFO_DEVIRTUALIZATION_DETAIL detail; } //---------------------------------------------------------------------------- diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ff729d9118c96..e0382cbfc23a4 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8926,7 +8926,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Can't devirtualize from __Canon. if (ObjClassHnd == TypeHandle(g_pCanonMethodTableClass)) { - info->failureReason = "object class is canonical"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; return false; } @@ -8937,7 +8937,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Don't try and devirtualize com interface calls. if (pObjMT->IsComObjectType()) { - info->failureReason = "object class is Com"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_COM; return false; } #endif // FEATURE_COMINTEROP @@ -8948,7 +8948,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // interface corresponding to pBaseMD. if (!pObjMT->CanCastToInterface(pBaseMT)) { - info->failureReason = "can't cast obj to interface type"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; return false; } @@ -8975,7 +8975,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (pDevirtMD == nullptr) { - info->failureReason = "can't find interface method"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP; return false; } @@ -8984,7 +8984,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Making this work is tracked by https://github.com/dotnet/runtime/issues/9588 if (pDevirtMD->GetMethodTable()->IsInterface() && pDevirtMD->HasClassInstantiation()) { - info->failureReason = "default interface method"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_DIM; return false; } } @@ -9007,7 +9007,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (pCheckMT == nullptr) { - info->failureReason = "object class not subclass"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS; return false; } @@ -9031,7 +9031,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (dslot != slot) { - info->failureReason = "explicit slot overeride"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_SLOT; return false; } } @@ -9071,7 +9071,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (!allowDevirt) { - info->failureReason = "crosses version bubble"; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE; return false; } } @@ -9082,7 +9082,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD; info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->requiresInstMethodTableArg = false; - info->failureReason = "success"; + info->detail = CORINFO_DEVIRTUALIZATION_SUCCESS; return true; } From 24551182147f00fd4f9fa104c604135446753969 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 25 May 2021 17:43:13 -0700 Subject: [PATCH 3/3] format --- src/coreclr/jit/compiler.cpp | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index c67b2f0f9d32e..af16742179908 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9694,18 +9694,28 @@ const char* Compiler::devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DE { switch (detail) { - case CORINFO_DEVIRTUALIZATION_UNKNOWN: return "unknown"; - case CORINFO_DEVIRTUALIZATION_SUCCESS: return "success"; - case CORINFO_DEVIRTUALIZATION_FAILED_CANON: return "object class was canonical"; - case CORINFO_DEVIRTUALIZATION_FAILED_COM: return "object class was com"; - case CORINFO_DEVIRTUALIZATION_FAILED_CAST: return "object class could not be cast to interface class"; - case CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP: return "interface method could not be found"; - case CORINFO_DEVIRTUALIZATION_FAILED_DIM: return "interface method was default interface method"; - case CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS: return "object not subclass of base class"; - case CORINFO_DEVIRTUALIZATION_FAILED_SLOT: return "virtual method installed via explicit override"; - case CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE: return "devirtualization crossed version bubble"; - default: return "undefined"; + case CORINFO_DEVIRTUALIZATION_UNKNOWN: + return "unknown"; + case CORINFO_DEVIRTUALIZATION_SUCCESS: + return "success"; + case CORINFO_DEVIRTUALIZATION_FAILED_CANON: + return "object class was canonical"; + case CORINFO_DEVIRTUALIZATION_FAILED_COM: + return "object class was com"; + case CORINFO_DEVIRTUALIZATION_FAILED_CAST: + return "object class could not be cast to interface class"; + case CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP: + return "interface method could not be found"; + case CORINFO_DEVIRTUALIZATION_FAILED_DIM: + return "interface method was default interface method"; + case CORINFO_DEVIRTUALIZATION_FAILED_SUBCLASS: + return "object not subclass of base class"; + case CORINFO_DEVIRTUALIZATION_FAILED_SLOT: + return "virtual method installed via explicit override"; + case CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE: + return "devirtualization crossed version bubble"; + default: + return "undefined"; } } #endif // defined(DEBUG) -