Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 17 commits into from
Aug 22, 2022
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
caf1dcf
Improve JitDisasm for top-level statements, nested functions and expo…
EgorBo Aug 17, 2022
bc37fd9
Wildcards support
EgorBo Aug 17, 2022
859209c
Address feedback
EgorBo Aug 17, 2022
cbcadf5
Address feedback
EgorBo Aug 18, 2022
02b97e5
Update compiler.cpp
EgorBo Aug 18, 2022
d49f397
fix gcc
EgorBo Aug 18, 2022
b52364a
Clean up
EgorBo Aug 18, 2022
08ae661
Update src/coreclr/jit/compiler.cpp
EgorBo Aug 19, 2022
8767829
Clean up
EgorBo Aug 19, 2022
6a397f3
Merge branch 'main' of github.com:dotnet/runtime into improve-jitdisasm
EgorBo Aug 19, 2022
aadcc1e
add comma for release
EgorBo Aug 19, 2022
4ec9cb1
Address feedback
EgorBo Aug 20, 2022
f6a6ffd
Merge branch 'main' of github.com:dotnet/runtime into improve-jitdisasm
EgorBo Aug 20, 2022
77e1e89
Update src/coreclr/jit/compiler.cpp
EgorBo Aug 22, 2022
691d7d3
Add PGO/Instrumentation notes
EgorBo Aug 22, 2022
9658c2d
add "with"
EgorBo Aug 22, 2022
2c6c231
address feedback
EgorBo Aug 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -5106,9 +5104,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
compJitTelemetry.NotifyEndOfCompilation();
#endif

#if defined(DEBUG)
++Compiler::jitTotalMethodCompiled;
#endif // defined(DEBUG)
InterlockedIncrement(&Compiler::jitTotalMethodCompiled);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

compFunctionTraceEnd(*methodCodePtr, *methodCodeSize, false);
JITDUMP("Method code size: %d\n", (unsigned)(*methodCodeSize));
Expand Down Expand Up @@ -6709,25 +6705,29 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
}
}

#ifdef DEBUG
if ((JitConfig.DumpJittedMethods() == 1) && !compIsForInlining())
if (JitConfig.DumpJittedMethods() && !compIsForInlining())
{
enum
{
BUFSIZE = 20
};
char osrBuffer[BUFSIZE] = {0};
#ifdef DEBUG
const int 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());
printf("JIT Compiling %4d [%s]%s, ILsize=%u, hash=0x%08x %s%s\n", Compiler::jitTotalMethodCompiled,
compGetTieringName(), info.compFullName, info.compILCodeSize, info.compMethodHash(), osrBuffer,
compGetStressMessage());
#else
const char* methodName = info.compCompHnd->getMethodName(info.compMethodHnd, nullptr);
const char* className = info.compCompHnd->getClassName(info.compClassHnd);
printf("JIT compiling %4d [%s]%s:%s, ILsize=%u\n", Compiler::jitTotalMethodCompiled, compGetTieringName(),
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
className, methodName, info.compILCodeSize);
#endif
}

#ifdef DEBUG
if (compIsForInlining())
{
compGenTreeID = impInlineInfo->InlinerCompiler->compGenTreeID;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4141,9 +4141,7 @@ class Compiler
regNumber getCallArgIntRegister(regNumber floatReg);
regNumber getCallArgFloatRegister(regNumber intReg);

#if defined(DEBUG)
static unsigned jitTotalMethodCompiled;
#endif

#ifdef DEBUG
static LONG jitNestingLevel;
Expand Down
84 changes: 58 additions & 26 deletions src/coreclr/jit/jitconfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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 starts 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?
Expand Down Expand Up @@ -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++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(srcSize >= needleSize)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if needleSize is bigger it will just return "not found" because it actually can be bigger

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in that case won't the srcSize - needleSize way bigger? You might just fast return "not found" in such cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this is still unaddressed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kunalspathak sorry didn't notice your comment - I am not sure I understand, let's say
srcSize is 10 and needleSize is 20, then srcSize - needleSize is -10 so for-loop immediately exits and returns "not found" - am I wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked in depth here, but srcSize - needleSize can never be negative, since size_t is unsigned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah oops, it supposed to be ssize_t

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked in depth here, but srcSize - needleSize can never be negative, since size_t is unsigned.

^ That.

{
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 && 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;
}

Expand Down Expand Up @@ -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 ||
Expand All @@ -367,8 +399,8 @@ 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, name->m_methodNameWildcardAtStart,
name->m_classNameWildcardAtEnd, className))
{
return true;
}
Expand All @@ -379,8 +411,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;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/jitconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 10 additions & 9 deletions src/coreclr/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -257,6 +256,8 @@ CONFIG_INTEGER(EnableIncompleteISAClass, W("EnableIncompleteISAClass"), 0) // En
CONFIG_METHODSET(JitDisasm, W("JitDisasm"))
#endif // !defined(DEBUG)

CONFIG_INTEGER(DumpJittedMethods, W("DumpJittedMethods"), 0) // Prints all jitted methods to the console
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be awesome


CONFIG_INTEGER(RichDebugInfo, W("RichDebugInfo"), 0) // If 1, keep rich debug info and report it back to the EE

#ifdef DEBUG
Expand Down