From e27e718637e2b5eb2b832cd23737333689192cfa Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 23 Aug 2022 01:56:42 +0200 Subject: [PATCH 1/2] [release/7.0-rc1] Improve DOTNET_JitDisasm and introduce DOTNET_JitDisasmSummary --- .../design/features/OsrDetailsAndDebugging.md | 2 +- src/coreclr/jit/codegencommon.cpp | 28 +------ src/coreclr/jit/compiler.cpp | 82 ++++++++++++------ src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/jitconfig.cpp | 83 +++++++++++++------ src/coreclr/jit/jitconfig.h | 1 + src/coreclr/jit/jitconfigvalues.h | 19 +++-- 7 files changed, 128 insertions(+), 90 deletions(-) diff --git a/docs/design/features/OsrDetailsAndDebugging.md b/docs/design/features/OsrDetailsAndDebugging.md index 1b1303eccb402..60d697473c147 100644 --- a/docs/design/features/OsrDetailsAndDebugging.md +++ b/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: diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index f4c0a37ce224b..45cbd8ac6c073 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/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) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index e73052a780f60..dd0e41fe9852d 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/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 @@ -5106,9 +5130,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)); @@ -6710,24 +6756,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; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 3291aca767c38..d1b52ab79e947 100644 --- a/src/coreclr/jit/compiler.h +++ b/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; @@ -9472,6 +9470,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX } const char* compGetTieringName(bool wantShortName = false) const; + const char* compGetPgoSourceName() const; const char* compGetStressMessage() const; codeOptimize compCodeOpt() const diff --git a/src/coreclr/jit/jitconfig.cpp b/src/coreclr/jit/jitconfig.cpp index 6fdbad427fe8a..b74291ea8373f 100644 --- a/src/coreclr/jit/jitconfig.cpp +++ b/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; } diff --git a/src/coreclr/jit/jitconfig.h b/src/coreclr/jit/jitconfig.h index 12d327d292b39..90f3daf9f6317 100644 --- a/src/coreclr/jit/jitconfig.h +++ b/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; diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 3e6df61b1f71f..efd5adf017c7d 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/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 From 2c0836dd79f0f19f787fb6de3d3d4f7837fd301d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 23 Aug 2022 02:49:29 +0200 Subject: [PATCH 2/2] Update jitconfig.cpp --- src/coreclr/jit/jitconfig.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/jit/jitconfig.cpp b/src/coreclr/jit/jitconfig.cpp index b74291ea8373f..cd65af36f106e 100644 --- a/src/coreclr/jit/jitconfig.cpp +++ b/src/coreclr/jit/jitconfig.cpp @@ -323,6 +323,11 @@ void JitConfigValues::MethodSet::destroy(ICorJitHost* host) // 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) { + if (srcSize < needleSize) + { + return -1; + } + for (size_t srcPos = 0; srcPos <= srcSize - needleSize; srcPos++) { if (strncmp(pSrc + srcPos, needle, needleSize) == 0)