PGO: Instrument cold blocks with profile validators

src/coreclr/inc/clrconfigvalues.h

@@ -289,7 +289,7 @@ RETAIL_CONFIG_STRING_INFO(EXTERNAL_GCName, W("GCName"), "")
 RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_ConvertIbcData, W("ConvertIbcData"), 1, "Converts between v1 and v2 IBC data")
 RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_DisableIBC, W("DisableIBC"), 0, "Disables the use of IBC data")
 RETAIL_CONFIG_DWORD_INFO(EXTERNAL_UseIBCFile, W("UseIBCFile"), 0, "")
-
+RETAIL_CONFIG_STRING_INFO(UNSUPPORTED_ProfileValidationPath, W("ProfileValidationPath"), "Path to save list of methods with polluted profiles to.")
 
 ///
 /// JIT

src/coreclr/inc/corinfo.h

@@ -608,6 +608,8 @@ enum CorInfoHelpFunc
     CORINFO_HELP_CLASSPROFILE32,            // Update 32-bit class profile for a call site
     CORINFO_HELP_CLASSPROFILE64,            // Update 64-bit class profile for a call site
 
+    CORINFO_HELP_PROFILE_VALIDATOR,         // Notify runtime if a cold block is called
+
     CORINFO_HELP_COUNT,
 };
 

src/coreclr/inc/corjitflags.h

@@ -103,7 +103,7 @@ class CORJIT_FLAGS
         CORJIT_FLAG_UNUSED16                = 43,
 #endif // !defined(TARGET_ARM)
 
-        CORJIT_FLAG_UNUSED17                = 44,
+        CORJIT_FLAG_VALIDATE_PROFILE        = 44,
         CORJIT_FLAG_UNUSED18                = 45,
         CORJIT_FLAG_UNUSED19                = 46,
         CORJIT_FLAG_UNUSED20                = 47,

src/coreclr/inc/crsttypes.h

@@ -134,7 +134,8 @@ enum CrstType
     CrstUnwindInfoTableLock = 116,
     CrstVSDIndirectionCellLock = 117,
     CrstWrapperTemplate = 118,
-    kNumberOfCrstTypes = 119
+    CrstMethodsWithPollutedProfiles = 119,
+    kNumberOfCrstTypes = 120
 };
 
 #endif // __CRST_TYPES_INCLUDED

src/coreclr/inc/jiteeversionguid.h

@@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
 #define GUID_DEFINED
 #endif // !GUID_DEFINED
 
-constexpr GUID JITEEVersionIdentifier = { /* 1052f490-cad7-4610-99bb-6f2bd91a1d19 */
-    0x1052f490,
-    0xcad7,
-    0x4610,
-    {0x99, 0xbb, 0x6f, 0x2b, 0xd9, 0x1a, 0x1d, 0x19}
+constexpr GUID JITEEVersionIdentifier = { /* 64303781-0337-40D2-BFE2-698DF0D110F1 */
+    0x64303781,
+    0x337,
+    0x40d2,
+    {0xbf, 0xe2, 0x69, 0x8d, 0xf0, 0xd1, 0x10, 0xf1}
   };
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////

src/coreclr/inc/jithelpers.h

@@ -359,6 +359,7 @@
     JITHELPER(CORINFO_HELP_PATCHPOINT, JIT_Patchpoint, CORINFO_HELP_SIG_REG_ONLY)
     JITHELPER(CORINFO_HELP_CLASSPROFILE32, JIT_ClassProfile32, CORINFO_HELP_SIG_REG_ONLY)
     JITHELPER(CORINFO_HELP_CLASSPROFILE64, JIT_ClassProfile64, CORINFO_HELP_SIG_REG_ONLY)
+    JITHELPER(CORINFO_HELP_PROFILE_VALIDATOR, JIT_ProfileValidator, CORINFO_HELP_SIG_REG_ONLY)
 
 #undef JITHELPER
 #undef DYNAMICJITHELPER

src/coreclr/jit/compiler.cpp

@@ -5095,6 +5095,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     // Insert GC Polls
     DoPhase(this, PHASE_INSERT_GC_POLLS, &Compiler::fgInsertGCPolls);
 
+    // Instrument cold blocks in order to validate profiles
+    if (!compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && compileFlags->IsSet(JitFlags::JIT_FLAG_VALIDATE_PROFILE) &&
+        opts.OptimizationEnabled() && fgFirstBB->hasProfileWeight() && !opts.IsReadyToRun())
+    {
+        DoPhase(this, PHASE_IBCINSTR, &Compiler::fgInsertProfileValidators);
+    }
+
     // Determine start of cold region if we are hot/cold splitting
     //
     DoPhase(this, PHASE_DETERMINE_FIRST_COLD_BLOCK, &Compiler::fgDetermineFirstColdBlock);

src/coreclr/jit/compiler.h

@@ -5728,6 +5728,7 @@ class Compiler
 
     PhaseStatus fgPrepareToInstrumentMethod();
     PhaseStatus fgInstrumentMethod();
+    PhaseStatus fgInsertProfileValidators();
     PhaseStatus fgIncorporateProfileData();
     void        fgIncorporateBlockCounts();
     void        fgIncorporateEdgeCounts();

src/coreclr/jit/fgprofile.cpp

@@ -1560,6 +1560,36 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod()
     return PhaseStatus::MODIFIED_NOTHING;
 }
 
+PhaseStatus Compiler::fgInsertProfileValidators()
+{
+    bool modified = false;
+    for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->bbNext)
+    {
+        if (block->hasProfileWeight() && (block->getBBWeight(this) == BB_ZERO_WEIGHT))
+        {
+            // TODO: don't emit validator if the block is dominated by other cold blocks
+
+            // Pass CORINFO_METHOD_HANDLE
+            GenTreeCall::Use* const args =
+                gtNewCallArgs(gtNewIconNode(reinterpret_cast<ssize_t>(info.compMethodHnd), TYP_I_IMPL),
+                              gtNewIconNode(block->bbNum, TYP_INT));
+
+            GenTree* call = fgMorphCall(gtNewHelperCallNode(CORINFO_HELP_PROFILE_VALIDATOR, TYP_VOID, args));
+            gtSetEvalOrder(call);
+            block->bbFlags |= BBF_HAS_CALL;
+
+            Statement* newStmt = fgNewStmtAtBeg(block, call);
+            if (fgStmtListThreaded)
+            {
+                gtSetStmtInfo(newStmt);
+                fgSetStmtSeq(newStmt);
+            }
+            modified = true;
+        }
+    }
+    return modified ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
+}
+
 //------------------------------------------------------------------------
 // fgInstrumentMethod: add instrumentation probes to the method
 //

src/coreclr/jit/jitee.h

@@ -87,7 +87,7 @@ class JitFlags
         JIT_FLAG_UNUSED16                = 43,
 #endif // !defined(TARGET_ARM)
 
-        JIT_FLAG_UNUSED17                = 44,
+        JIT_FLAG_VALIDATE_PROFILE        = 44,
         JIT_FLAG_UNUSED18                = 45,
         JIT_FLAG_UNUSED19                = 46,
         JIT_FLAG_UNUSED20                = 47,
@@ -212,6 +212,7 @@ class JitFlags
         FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE, JIT_FLAG_REVERSE_PINVOKE);
         FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_TIER0, JIT_FLAG_TIER0);
         FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_TIER1, JIT_FLAG_TIER1);
+        FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_VALIDATE_PROFILE, JIT_FLAG_VALIDATE_PROFILE);
 
 #if defined(TARGET_ARM)
 

src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs

@@ -297,6 +297,7 @@ which is the right helper to use to allocate an object of a given type. */
 
         CORINFO_HELP_STACK_PROBE,               // Probes each page of the allocated stack frame
 
+        CORINFO_HELP_PROFILE_VALIDATOR,
         CORINFO_HELP_COUNT,
     }
 }

src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs

@@ -1378,6 +1378,7 @@ public enum CorJitFlag : uint
         CORJIT_FLAG_RELATIVE_CODE_RELOCS = 41, // JIT should generate PC-relative address computations instead of EE relocation records
         CORJIT_FLAG_NO_INLINING = 42, // JIT should not inline any called method into this method
         CORJIT_FLAG_SOFTFP_ABI = 43, // On ARM should enable armel calling convention
+        CORJIT_FLAG_VALIDATE_PROFILE = 44, // Instrument cold blocks with counters to validate profiles
     }
 
     public struct CORJIT_FLAGS

src/coreclr/vm/appdomain.cpp

@@ -2133,6 +2133,7 @@ AppDomain::AppDomain()
 #endif
 
     m_pRefClassFactHash = NULL;
+    s_methodsWithPollutedProfiles = NULL;
 
     m_ForceTrivialWaitOperations = false;
     m_Stage=STAGE_CREATING;
@@ -2230,6 +2231,7 @@ void AppDomain::Init()
 
     m_ReflectionCrst.Init(CrstReflection, CRST_UNSAFE_ANYMODE);
     m_RefClassFactCrst.Init(CrstClassFactInfoHash);
+    m_MethodsWithPollutedProfileCrst.Init(CrstMethodsWithPollutedProfiles, CRST_UNSAFE_ANYMODE);
 
     SetStage(STAGE_READYFORMANAGEDCODE);
 
@@ -2392,6 +2394,82 @@ EEClassFactoryInfoHashTable* AppDomain::SetupClassFactHash()
     return m_pRefClassFactHash;
 }
 
+void AppDomain::LogMethodWithPollutedProfile(CORINFO_METHOD_HANDLE method, unsigned bbNum)
+{
+    CrstHolder ch(&m_MethodsWithPollutedProfileCrst);
+
+    if (s_methodsWithPollutedProfiles == nullptr)
+    {
+        NewHolder<EEPtrHashTable> pCache(new EEPtrHashTable());
+        if (!pCache->Init(20, nullptr, nullptr, false))
+            return;
+        s_methodsWithPollutedProfiles = pCache.Extract();
+    }
+
+    PTR_VOID key = method;
+    HashDatum value;
+    if (s_methodsWithPollutedProfiles->GetValue(key, &value))
+    {
+        // Increment the counter
+        value = reinterpret_cast<HashDatum>(reinterpret_cast<UINT64>(value) + 1);
+        s_methodsWithPollutedProfiles->ReplaceValue(key, value);
+    }
+    else
+    {
+        s_methodsWithPollutedProfiles->InsertValue(key, reinterpret_cast<HashDatum>(1));
+    }
+}
+
+void AppDomain::ListMethodsWithPollutedProfile()
+{
+    if (s_methodsWithPollutedProfiles == nullptr)
+    {
+        return;
+    }
+
+    LPWSTR filePath = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_ProfileValidationPath);
+    if (filePath == nullptr)
+    {
+        return;
+    }
+
+    FILE* file = _wfopen(filePath, W("a"));
+    if (file == nullptr)
+    {
+        return;
+    }
+
+    CrstHolder ch(&m_MethodsWithPollutedProfileCrst);
+
+    // CSV Format
+    fprintf(file, "Method name, Cold block hits\n");
+
+    EEHashTableIteration iter;
+    s_methodsWithPollutedProfiles->IterateStart(&iter);
+    BOOL keepGoing = s_methodsWithPollutedProfiles->IterateNext(&iter);
+    while (keepGoing)
+    {
+        auto key = static_cast<CORINFO_METHOD_HANDLE>(s_methodsWithPollutedProfiles->IterateGetKey(&iter));
+        if (key != nullptr)
+        {
+            MethodDesc* method = GetMethod(key);
+            if (method != nullptr)
+            {
+                HashDatum value = s_methodsWithPollutedProfiles->IterateGetValue(&iter);
+                const UINT64 counter = reinterpret_cast<UINT64>(value);
+
+                LPCUTF8 nameSpace;
+                LPCUTF8 scope = method->GetMethodTable()->GetFullyQualifiedNameInfo(&nameSpace);
+                fprintf(file, "%s::%s, %llu\n", scope, method->GetName(), counter);
+            }
+        }
+        keepGoing = s_methodsWithPollutedProfiles->IterateNext(&iter);
+    }
+    delete s_methodsWithPollutedProfiles;
+    s_methodsWithPollutedProfiles = nullptr;
+    fclose(file);
+}
+
 #ifdef FEATURE_COMINTEROP
 DispIDCache* AppDomain::SetupRefDispIDCache()
 {

src/coreclr/vm/appdomain.hpp

@@ -2074,8 +2074,9 @@ class AppDomain : public BaseDomain
 private:
     CrstExplicitInit    m_ReflectionCrst;
     CrstExplicitInit    m_RefClassFactCrst;
+    CrstExplicitInit    m_MethodsWithPollutedProfileCrst;
 
-
+    EEPtrHashTable *s_methodsWithPollutedProfiles;
     EEClassFactoryInfoHashTable *m_pRefClassFactHash;   // Hash table that maps a class factory info to a COM comp.
 #ifdef FEATURE_COMINTEROP
     DispIDCache *m_pRefDispIDCache;
@@ -2091,6 +2092,9 @@ class AppDomain : public BaseDomain
         return &m_RefClassFactCrst;
     }
 
+    void LogMethodWithPollutedProfile(CORINFO_METHOD_HANDLE method, unsigned bbNum);
+    void ListMethodsWithPollutedProfile();
+
 #ifndef DACCESS_COMPILE
     EEClassFactoryInfoHashTable* GetClassFactHash()
     {

src/coreclr/vm/ceemain.cpp

@@ -1295,6 +1295,8 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
     {
         ClrFlsSetThreadType(ThreadType_Shutdown);
 
+        AppDomain::GetCurrentDomain()->ListMethodsWithPollutedProfile();
+
         if (fIsDllUnloading && g_fEEShutDown)
         {
             // I'm in the final shutdown and the first part has already been run.

src/coreclr/vm/jithelpers.cpp

@@ -5322,6 +5322,15 @@ HCIMPL2(void, JIT_ClassProfile32, Object *obj, void* tableAddress)
 }
 HCIMPLEND
 
+HCIMPL2(void, JIT_ProfileValidator, CORINFO_METHOD_HANDLE methodHnd, unsigned bbNum)
+{
+    FCALL_CONTRACT;
+    FC_GC_POLL_NOT_NEEDED();
+
+    AppDomain::GetCurrentDomain()->LogMethodWithPollutedProfile(methodHnd, bbNum);
+}
+HCIMPLEND
+
 // Version of helper above used when the count is 64-bit
 HCIMPL2(void, JIT_ClassProfile64, Object *obj, void* tableAddress)
 {

src/coreclr/vm/jitinterface.cpp

@@ -12947,6 +12947,12 @@ CORJIT_FLAGS GetCompileFlags(MethodDesc * ftn, CORJIT_FLAGS flags, CORINFO_METHO
         flags.Set(CORJIT_FLAGS::CORJIT_FLAG_BBOPT);
     }
 
+    // Instrument cold blocks in order to validate profiles.
+    if (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_ProfileValidationPath) != nullptr)
+    {
+        flags.Set(CORJIT_FLAGS::CORJIT_FLAG_VALIDATE_PROFILE);
+    }
+
 #endif
 
     return flags;