Improve JitDisasm for top-level statements, expose DumpJittedMethods in Release

docs/design/features/OsrDetailsAndDebugging.md

@@ -338,7 +338,7 @@ Note if a Tier0 method is recursive and has loops there can be some interesting
 
 ### Seeing which OSR methods are created
 
-* `DOTNET_DumpJittedMethods=1` will specially mark OSR methods with the inspiring IL offsets.
+* `DOTNET_JitDisasmSummary=1` will specially mark OSR methods with the inspiring IL offsets.
 
 For example, running a libraries test with some stressful OSR settings, there ended up being 699 OSR methods jitted out of 160675 total methods. Grepping for OSR in the dump output, the last few lines were:
 

src/coreclr/jit/codegencommon.cpp

@@ -1887,31 +1887,9 @@ void CodeGen::genGenerateMachineCode()
 
         if (compiler->fgHaveProfileData())
         {
-            const char* pgoKind;
-            switch (compiler->fgPgoSource)
-            {
-                case ICorJitInfo::PgoSource::Static:
-                    pgoKind = "Static";
-                    break;
-                case ICorJitInfo::PgoSource::Dynamic:
-                    pgoKind = "Dynamic";
-                    break;
-                case ICorJitInfo::PgoSource::Blend:
-                    pgoKind = "Blend";
-                    break;
-                case ICorJitInfo::PgoSource::Text:
-                    pgoKind = "Textual";
-                    break;
-                case ICorJitInfo::PgoSource::Sampling:
-                    pgoKind = "Sample-based";
-                    break;
-                default:
-                    pgoKind = "Unknown";
-                    break;
-            }
-
-            printf("; with %s PGO: edge weights are %s, and fgCalledCount is " FMT_WT "\n", pgoKind,
-                   compiler->fgHaveValidEdgeWeights ? "valid" : "invalid", compiler->fgCalledCount);
+            printf("; with %s: edge weights are %s, and fgCalledCount is " FMT_WT "\n",
+                   compiler->compGetPgoSourceName(), compiler->fgHaveValidEdgeWeights ? "valid" : "invalid",
+                   compiler->fgCalledCount);
         }
 
         if (compiler->fgPgoFailReason != nullptr)

src/coreclr/jit/compiler.cpp

@@ -34,9 +34,7 @@ extern ICorJitHost* g_jitHost;
 #define COLUMN_FLAGS (COLUMN_KINDS + 32)
 #endif
 
-#if defined(DEBUG)
 unsigned Compiler::jitTotalMethodCompiled = 0;
-#endif // defined(DEBUG)
 
 #if defined(DEBUG)
 LONG Compiler::jitNestingLevel = 0;
@@ -4082,8 +4080,9 @@ bool Compiler::compRsvdRegCheck(FrameLayoutState curState)
 //
 const char* Compiler::compGetTieringName(bool wantShortName) const
 {
-    const bool tier0 = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0);
-    const bool tier1 = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER1);
+    const bool tier0         = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0);
+    const bool tier1         = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER1);
+    const bool instrumenting = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR);
 
     if (!opts.compMinOptsIsSet)
     {
@@ -4097,13 +4096,13 @@ const char* Compiler::compGetTieringName(bool wantShortName) const
 
     if (tier0)
     {
-        return "Tier0";
+        return instrumenting ? "Instrumented Tier0" : "Tier0";
     }
     else if (tier1)
     {
         if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_OSR))
         {
-            return "Tier1-OSR";
+            return instrumenting ? "Instrumented Tier1-OSR" : "Tier1-OSR";
         }
         else
         {
@@ -4149,6 +4148,31 @@ const char* Compiler::compGetTieringName(bool wantShortName) const
     }
 }
 
+//------------------------------------------------------------------------
+// compGetPgoSourceName: get a string describing PGO source
+//
+// Returns:
+//   String describing describing PGO source (e.g. Dynamic, Static, etc)
+//
+const char* Compiler::compGetPgoSourceName() const
+{
+    switch (fgPgoSource)
+    {
+        case ICorJitInfo::PgoSource::Static:
+            return "Static PGO";
+        case ICorJitInfo::PgoSource::Dynamic:
+            return "Dynamic PGO";
+        case ICorJitInfo::PgoSource::Blend:
+            return "Blend PGO";
+        case ICorJitInfo::PgoSource::Text:
+            return "Textual PGO";
+        case ICorJitInfo::PgoSource::Sampling:
+            return "Sample-based PGO";
+        default:
+            return "";
+    }
+}
+
 //------------------------------------------------------------------------
 // compGetStressMessage: get a string describing jitstress capability
 //   for this method
@@ -4953,9 +4977,31 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     compJitTelemetry.NotifyEndOfCompilation();
 #endif
 
-#if defined(DEBUG)
-    ++Compiler::jitTotalMethodCompiled;
-#endif // defined(DEBUG)
+    unsigned methodsCompiled = (unsigned)InterlockedIncrement((LONG*)&Compiler::jitTotalMethodCompiled);
+
+    if (JitConfig.JitDisasmSummary() && !compIsForInlining())
+    {
+        char osrBuffer[20] = {0};
+        if (opts.IsOSR())
+        {
+            // Tiering name already includes "OSR", we just want the IL offset
+            sprintf_s(osrBuffer, 20, " @0x%x", info.compILEntry);
+        }
+
+#ifdef DEBUG
+        const char* fullName = info.compFullName;
+#else
+        const char* fullName  = eeGetMethodFullName(info.compMethodHnd);
+#endif
+
+        char debugPart[128] = {0};
+        INDEBUG(sprintf_s(debugPart, 128, ", hash=0x%08x%s", info.compMethodHash(), compGetStressMessage()));
+
+        const bool hasProf = fgHaveProfileData();
+        printf("%4d: JIT compiled %s [%s%s%s%s, IL size=%u, code size=%u%s]\n", methodsCompiled, fullName,
+               compGetTieringName(), osrBuffer, hasProf ? " with " : "", hasProf ? compGetPgoSourceName() : "",
+               info.compILCodeSize, *methodCodeSize, debugPart);
+    }
 
     compFunctionTraceEnd(*methodCodePtr, *methodCodeSize, false);
     JITDUMP("Method code size: %d\n", (unsigned)(*methodCodeSize));
@@ -6557,24 +6603,6 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
     }
 
 #ifdef DEBUG
-    if ((JitConfig.DumpJittedMethods() == 1) && !compIsForInlining())
-    {
-        enum
-        {
-            BUFSIZE = 20
-        };
-        char osrBuffer[BUFSIZE] = {0};
-        if (opts.IsOSR())
-        {
-            // Tiering name already includes "OSR", we just want the IL offset
-            //
-            sprintf_s(osrBuffer, BUFSIZE, " @0x%x", info.compILEntry);
-        }
-
-        printf("Compiling %4d %s::%s, IL size = %u, hash=0x%08x %s%s%s\n", Compiler::jitTotalMethodCompiled,
-               info.compClassName, info.compMethodName, info.compILCodeSize, info.compMethodHash(),
-               compGetTieringName(), osrBuffer, compGetStressMessage());
-    }
     if (compIsForInlining())
     {
         compGenTreeID   = impInlineInfo->InlinerCompiler->compGenTreeID;

src/coreclr/jit/compiler.h

@@ -4145,9 +4145,7 @@ class Compiler
     regNumber getCallArgIntRegister(regNumber floatReg);
     regNumber getCallArgFloatRegister(regNumber intReg);
 
-#if defined(DEBUG)
     static unsigned jitTotalMethodCompiled;
-#endif
 
 #ifdef DEBUG
     static LONG jitNestingLevel;
@@ -9505,6 +9503,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     }
 
     const char* compGetTieringName(bool wantShortName = false) const;
+    const char* compGetPgoSourceName() const;
     const char* compGetStressMessage() const;
 
     codeOptimize compCodeOpt() const

src/coreclr/jit/jitconfig.cpp

@@ -44,14 +44,15 @@ void JitConfigValues::MethodSet::initialize(const WCHAR* list, ICorJitHost* host
     bool         isQuoted  = false;    // true while parsing inside a quote "this-is-a-quoted-region"
     MethodName   currentName;          // Buffer used while parsing the current entry
 
-    currentName.m_next                    = nullptr;
-    currentName.m_methodNameStart         = -1;
-    currentName.m_methodNameLen           = -1;
-    currentName.m_methodNameWildcardAtEnd = false;
-    currentName.m_classNameStart          = -1;
-    currentName.m_classNameLen            = -1;
-    currentName.m_classNameWildcardAtEnd  = false;
-    currentName.m_numArgs                 = -1;
+    currentName.m_next                      = nullptr;
+    currentName.m_methodNameStart           = -1;
+    currentName.m_methodNameLen             = -1;
+    currentName.m_methodNameWildcardAtStart = false;
+    currentName.m_methodNameWildcardAtEnd   = false;
+    currentName.m_classNameStart            = -1;
+    currentName.m_classNameLen              = -1;
+    currentName.m_classNameWildcardAtEnd    = false;
+    currentName.m_numArgs                   = -1;
 
     enum State
     {
@@ -202,21 +203,31 @@ void JitConfigValues::MethodSet::initialize(const WCHAR* list, ICorJitHost* host
                     }
 
                     // Is the first character a wildcard?
-                    if (m_list[currentName.m_methodNameStart] == WILD_CHAR)
+                    if (m_list[currentName.m_methodNameStart] == WILD_CHAR && currentName.m_methodNameLen == 1)
                     {
                         // The method name is a full wildcard; mark it as such.
                         currentName.m_methodNameStart = -1;
                         currentName.m_methodNameLen   = -1;
                     }
-                    // Is there a wildcard at the end of the method name?
-                    //
-                    else if (m_list[currentName.m_methodNameStart + currentName.m_methodNameLen - 1] == WILD_CHAR)
+                    else
                     {
-                        // i.e. class:foo*, will match any method that starts with "foo"
-
-                        // Remove the trailing WILD_CHAR from method name
-                        currentName.m_methodNameLen--; // backup for WILD_CHAR
-                        currentName.m_methodNameWildcardAtEnd = true;
+                        // Is there a wildcard at the start of the method name?
+                        if (m_list[currentName.m_methodNameStart] == WILD_CHAR)
+                        {
+                            // i.e. class:*foo, will match any method that ends with "foo"
+                            // Remove the leading WILD_CHAR from method name
+                            currentName.m_methodNameStart++;
+                            currentName.m_methodNameLen--;
+                            currentName.m_methodNameWildcardAtStart = true;
+                        }
+                        // Is there a wildcard at the end of the method name?
+                        if (m_list[currentName.m_methodNameStart + currentName.m_methodNameLen - 1] == WILD_CHAR)
+                        {
+                            // i.e. class:foo*, will match any method that starts with "foo"
+                            // Remove the trailing WILD_CHAR from method name
+                            currentName.m_methodNameLen--; // backup for WILD_CHAR
+                            currentName.m_methodNameWildcardAtEnd = true;
+                        }
                     }
 
                     // should we expect an ARG_LIST?
@@ -309,11 +320,30 @@ void JitConfigValues::MethodSet::destroy(ICorJitHost* host)
     m_names = nullptr;
 }
 
-static bool matchesName(const char* const name, int nameLen, bool wildcardAtEnd, const char* const s2)
+// strstr that is length-limited, this implementation is not intended to be used on hot paths
+static size_t strnstr(const char* pSrc, size_t srcSize, const char* needle, size_t needleSize)
 {
-    // 's2' must start with 'nameLen' characters of 'name'
-    if (strncmp(name, s2, nameLen) != 0)
+    for (size_t srcPos = 0; srcPos <= srcSize - needleSize; srcPos++)
+    {
+        if (strncmp(pSrc + srcPos, needle, needleSize) == 0)
+        {
+            return srcPos;
+        }
+    }
+    return -1;
+}
+
+static bool matchesName(
+    const char* const name, int nameLen, bool wildcardAtStart, bool wildcardAtEnd, const char* const s2)
+{
+    if (wildcardAtStart && (int)strnstr(s2, strlen(s2), name, nameLen) == -1)
+    {
+        return false;
+    }
+
+    if (!wildcardAtStart && strncmp(name, s2, nameLen) != 0)
     {
+        // 's2' must start with 'nameLen' characters of 'name'
         return false;
     }
 
@@ -346,12 +376,14 @@ bool JitConfigValues::MethodSet::contains(const char*       methodName,
         if (name->m_methodNameStart != -1)
         {
             const char* expectedMethodName = &m_list[name->m_methodNameStart];
-            if (!matchesName(expectedMethodName, name->m_methodNameLen, name->m_methodNameWildcardAtEnd, methodName))
+            if (!matchesName(expectedMethodName, name->m_methodNameLen, name->m_methodNameWildcardAtStart,
+                             name->m_methodNameWildcardAtEnd, methodName))
             {
                 // C++ embeds the class name into the method name; deal with that here.
                 const char* colon = strchr(methodName, ':');
                 if (colon != nullptr && colon[1] == ':' &&
-                    matchesName(expectedMethodName, name->m_methodNameLen, name->m_methodNameWildcardAtEnd, methodName))
+                    matchesName(expectedMethodName, name->m_methodNameLen, name->m_methodNameWildcardAtStart,
+                                name->m_methodNameWildcardAtEnd, methodName))
                 {
                     int classLen = (int)(colon - methodName);
                     if (name->m_classNameStart == -1 ||
@@ -367,8 +399,7 @@ bool JitConfigValues::MethodSet::contains(const char*       methodName,
 
         // If m_classNameStart is valid, check for a mismatch
         if (className == nullptr || name->m_classNameStart == -1 ||
-            matchesName(&m_list[name->m_classNameStart], name->m_classNameLen, name->m_classNameWildcardAtEnd,
-                        className))
+            matchesName(&m_list[name->m_classNameStart], name->m_classNameLen, false, false, className))
         {
             return true;
         }
@@ -379,8 +410,8 @@ bool JitConfigValues::MethodSet::contains(const char*       methodName,
         if (nsSep != nullptr && nsSep != className)
         {
             const char* onlyClass = nsSep[-1] == '.' ? nsSep : &nsSep[1];
-            if (matchesName(&m_list[name->m_classNameStart], name->m_classNameLen, name->m_classNameWildcardAtEnd,
-                            onlyClass))
+            if (matchesName(&m_list[name->m_classNameStart], name->m_classNameLen, false,
+                            name->m_classNameWildcardAtEnd, onlyClass))
             {
                 return true;
             }

src/coreclr/jit/jitconfig.h

@@ -20,6 +20,7 @@ class JitConfigValues
             MethodName* m_next;
             int         m_methodNameStart;
             int         m_methodNameLen;
+            bool        m_methodNameWildcardAtStart;
             bool        m_methodNameWildcardAtEnd;
             int         m_classNameStart;
             int         m_classNameLen;

src/coreclr/jit/jitconfigvalues.h

@@ -26,15 +26,14 @@ CONFIG_INTEGER(DiffableDasm, W("JitDiffableDasm"), 0)          // Make the disas
 CONFIG_INTEGER(JitDasmWithAddress, W("JitDasmWithAddress"), 0) // Print the process address next to each instruction of
                                                                // the disassembly
 CONFIG_INTEGER(DisplayLoopHoistStats, W("JitLoopHoistStats"), 0) // Display JIT loop hoisting statistics
-CONFIG_INTEGER(DisplayLsraStats, W("JitLsraStats"), 0)       // Display JIT Linear Scan Register Allocator statistics
-                                                             // If set to "1", display the stats in textual format.
-                                                             // If set to "2", display the stats in csv format.
-                                                             // If set to "3", display the stats in summarize format.
-                                                             // Recommended to use with JitStdOutFile flag.
-CONFIG_STRING(JitLsraOrdering, W("JitLsraOrdering"))         // LSRA heuristics ordering
-CONFIG_INTEGER(DumpJittedMethods, W("DumpJittedMethods"), 0) // Prints all jitted methods to the console
-CONFIG_INTEGER(EnablePCRelAddr, W("JitEnablePCRelAddr"), 1)  // Whether absolute addr be encoded as PC-rel offset by
-                                                             // RyuJIT where possible
+CONFIG_INTEGER(DisplayLsraStats, W("JitLsraStats"), 0)      // Display JIT Linear Scan Register Allocator statistics
+                                                            // If set to "1", display the stats in textual format.
+                                                            // If set to "2", display the stats in csv format.
+                                                            // If set to "3", display the stats in summarize format.
+                                                            // Recommended to use with JitStdOutFile flag.
+CONFIG_STRING(JitLsraOrdering, W("JitLsraOrdering"))        // LSRA heuristics ordering
+CONFIG_INTEGER(EnablePCRelAddr, W("JitEnablePCRelAddr"), 1) // Whether absolute addr be encoded as PC-rel offset by
+                                                            // RyuJIT where possible
 CONFIG_INTEGER(JitAssertOnMaxRAPasses, W("JitAssertOnMaxRAPasses"), 0)
 CONFIG_INTEGER(JitBreakEmitOutputInstr, W("JitBreakEmitOutputInstr"), -1)
 CONFIG_INTEGER(JitBreakMorphTree, W("JitBreakMorphTree"), 0xffffffff)
@@ -257,6 +256,8 @@ CONFIG_INTEGER(EnableIncompleteISAClass, W("EnableIncompleteISAClass"), 0) // En
 CONFIG_METHODSET(JitDisasm, W("JitDisasm"))
 #endif // !defined(DEBUG)
 
+CONFIG_INTEGER(JitDisasmSummary, W("JitDisasmSummary"), 0) // Prints all jitted methods to the console
+
 CONFIG_INTEGER(RichDebugInfo, W("RichDebugInfo"), 0) // If 1, keep rich debug info and report it back to the EE
 
 #ifdef DEBUG