Skip to content

[release/7.0] JIT: Simplify JitDisasm matching behavior and support namespaces/generics in release #75260

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

Merged
carlossanlop merged 2 commits into from
Sep 8, 2022
Merged

[release/7.0] JIT: Simplify JitDisasm matching behavior and support namespaces/generics in release #75260

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd1385f
JIT: Simplify JitDisasm matching behavior (#74430)
jakobbotsch Sep 4, 2022
2ea9307
Update JIT-EE GUID
jakobbotsch Sep 8, 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
Next Next commit
JIT: Simplify JitDisasm matching behavior (#74430) 
This changes how the JIT matches method names and signatures for method
sets (e.g. JitDisasm). It also starts printing method instantiations for full method names
and makes references to types consistent in generic instantiations and the signature.
In addition it starts supporting generic instantiations in release too.
To do this, most of the type printing is moved to the JIT, which also aligns the output
between crossgen2 and the VM (there were subtle differences here, like spaces between generic type arguments).
More importantly, we (for the most part) stop relying on JIT-EE methods that are documented to only be for debug purposes.

The new behavior of the matching is the following:

* The matching behavior is always string based.
* The JitDisasm string is a space-separated list of patterns. Patterns can arbitrarily
   contain both '*' (match any characters) and '?' (match any 1 character).
* The string matched against depends on characters in the pattern:
    + If the pattern contains a ':' character, the string matched against is prefixed by the class name and a colon
    + If the pattern contains a '(' character, the string matched against is suffixed by the signature
    + If the class name (part before colon) contains a '[', the class contains its generic instantiation
    + If the method name (part between colon and '(') contains a '[', the method contains its generic instantiation

For example, consider

```
namespace MyNamespace
{
    public class C<T1, T2>
    {
        [MethodImpl(MethodImplOptions.NoInlining)]
        public void M<T3, T4>(T1 arg1, T2 arg2, T3 arg3, T4 arg4)
        {
        }
    }
}

new C<sbyte, string>().M<int, object>(default, default, default, default); // compilation 1
new C<int, int>().M<int, int>(default, default, default, default); // compilation 2
```
The full strings are:
Before the change:

```
MyNamespace.C`2[SByte,__Canon][System.SByte,System.__Canon]:M(byte,System.__Canon,int,System.__Canon)
MyNamespace.C`2[Int32,Int32][System.Int32,System.Int32]:M(int,int,int,int)
```
Notice no method instantiation and the double class instantiation, which seems like an EE bug. Also two different names are used for sbyte: System.SByte and byte.

After the change the strings are:

```
MyNamespace.C`2[byte,System.__Canon]:M[int,System.__Canon](byte,System.__Canon,int,System.__Canon)
MyNamespace.C`2[int,int]:M[int,int](int,int,int,int)
```

The following strings will match both compilations:

```
M
*C`2:M
*C`2[*]:M[*](*)
MyNamespace.C`2:M
```

The following will match only the first one:

```
M[int,*Canon]
MyNamespace.C`2[byte,*]:M
M(*Canon)
```

There is one significant change in behavior here, which is that I have removed the special case that allows matching class names without namespaces. In particular, today Console:WriteLine would match all overloads of System.Console.WriteLine, while after this change it will not match. However, with generalized wild cards the replacement is simple in *Console:WriteLine.
  • Loading branch information
@jakobbotsch
jakobbotsch committed Sep 8, 2022
commit fd1385f586dfb344223e472681fa24db8ab549de
54 changes: 42 additions & 12 deletions docs/design/coreclr/jit/viewing-jit-dumps.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,29 +120,59 @@ These can be set in one of three ways:

## Specifying method names

The complete syntax for specifying a single method name (for a flag that takes a method name, such as `COMPlus_JitDump`) is:

Some environment variables such as `COMPlus_JitDump` take a set of patterns specifying method names. The matching works in the following way:
* A method set string is a space-separated list of patterns. Patterns can arbitrarily contain both '*' (match any characters) and '?' (match any 1 character).
* The string matched against depends on characters in the pattern:
+ If the pattern contains a ':' character, the string matched against is prefixed by the class name and a colon
+ If the pattern contains a '(' character, the string matched against is suffixed by the signature
+ If the class name (part before colon) contains a '[', the class contains its generic instantiation
+ If the method name (part between colon and '(') contains a '[', the method contains its generic instantiation

In particular, the matching is done against strings of the following format which coincides with how the JIT displays method signatures (so these can be copy pasted into the environment variable).
```
[[<Namespace>.]<ClassName>::]<MethodName>[([<types>)]
[ClassName[Instantiation]:]MethodName[Instantiation][(<types>)]
```

For example
For example, consider the following:
```csharp
namespace MyNamespace
{
public class C<T1, T2>
{
[MethodImpl(MethodImplOptions.NoInlining)]
public void M<T3, T4>(T1 arg1, T2 arg2, T3 arg3, T4 arg4)
{
}
}
}

```
System.Object::ToString(System.Object)
new C<sbyte, string>().M<int, object>(default, default, default, default); // compilation 1
new C<int, int>().M<int, int>(default, default, default, default); // compilation 2
```

The namespace, class name, and argument types are optional, and if they are not present, default to a wildcard. Thus stating:
The full names of these instantiations are the following, as printed by `COMPlus_JitDisasmSummary`:

```
Main
MyNamespace.C`2[byte,System.__Canon]:M[int,System.__Canon](byte,System.__Canon,int,System.__Canon)
MyNamespace.C`2[int,int]:M[int,int](int,int,int,int)
```
Note that ``C`2`` here is the name put into metadata by Roslyn; the suffix is not added by RyuJIT.
For Powershell users keep in mind that backtick is the escape character and itself has to be escaped via double backtick.

will match all methods named Main from any class and any number of arguments.

`<types>` is a comma separated list of type names. Note that presently only the number of arguments and not the types themselves are used to distinguish methods. Thus, `Main(Foo, Bar)` and `Main(int, int)` will both match any main method with two arguments.
The following strings will match both compilations:
```
M
*C`2:M
*C`2[*]:M[*](*)
MyNamespace.C`2:M
```

The wildcard character `*` can be used for `<ClassName>` and `<MethodName>`. In particular `*` by itself indicates every method.
The following match only the first compilation:
```
M[int,*Canon]
MyNamespace.C`2[byte,*]:M
M(*Canon)
```

## Useful COMPlus variables

Expand Down
12 changes: 9 additions & 3 deletions src/coreclr/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2893,9 +2893,15 @@ class ICorStaticInfo
CORINFO_METHOD_HANDLE hMethod
) = 0;

// this function is for debugging only. It returns the method name
// and if 'moduleName' is non-null, it sets it to something that will
// says which method (a class name, or a module name)
// This function returns the method name and if 'moduleName' is non-null,
// it sets it to something that contains the method (a class
// name, or a module name). Note that the moduleName parameter is for
// diagnostics only.
//
// The method name returned is the same as getMethodNameFromMetadata except
// in the case of functions without metadata (e.g. IL stubs), where this
// function still returns a reasonable name while getMethodNameFromMetadata
// returns null.
virtual const char* getMethodName (
CORINFO_METHOD_HANDLE ftn, /* IN */
const char **moduleName /* OUT */
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* 1b9551b8-21f4-4233-9c90-f3eabd6a322b */
0x1b9551b8,
0x21f4,
0x4233,
{0x9c, 0x90, 0xf3, 0xea, 0xbd, 0x6a, 0x32, 0x2b}
constexpr GUID JITEEVersionIdentifier = { /* 0cd8b9d4-04f4-45a7-b16b-7f24b7c0a454 */
0x0cd8b9d4,
0x04f4,
0x45a7,
{0xb1, 0x6b, 0x7f, 0x24, 0xb7, 0xc0, 0xa4, 0x54}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
80 changes: 33 additions & 47 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,7 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
info.compCompHnd = compHnd;
info.compMethodHnd = methodHnd;
info.compMethodInfo = methodInfo;
info.compClassHnd = compHnd->getMethodClass(methodHnd);

#ifdef DEBUG
bRangeAllowStress = false;
Expand All @@ -1788,17 +1789,10 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
info.compClassName = nullptr;
info.compFullName = nullptr;

const char* classNamePtr;
const char* methodName;

methodName = eeGetMethodName(methodHnd, &classNamePtr);
unsigned len = (unsigned)roundUp(strlen(classNamePtr) + 1);
info.compClassName = getAllocator(CMK_DebugOnly).allocate<char>(len);
info.compMethodName = methodName;
strcpy_s((char*)info.compClassName, len, classNamePtr);

info.compFullName = eeGetMethodFullName(methodHnd);
info.compPerfScore = 0.0;
info.compMethodName = eeGetMethodName(methodHnd, nullptr);
info.compClassName = eeGetClassName(info.compClassHnd);
info.compFullName = eeGetMethodFullName(methodHnd);
info.compPerfScore = 0.0;

info.compMethodSuperPMIIndex = g_jitHost->getIntConfigValue(W("SuperPMIMethodContextNumber"), -1);
#endif // defined(DEBUG) || defined(LATE_DISASM) || DUMP_FLOWGRAPHS
Expand Down Expand Up @@ -2537,7 +2531,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)

if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
if (pfAltJit->contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (pfAltJit->contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.altJit = true;
}
Expand Down Expand Up @@ -2618,7 +2612,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
//
if (compIsForImportOnly() && (!altJitConfig || opts.altJit))
{
if (JitConfig.JitImportBreak().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitImportBreak().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
assert(!"JitImportBreak reached");
}
Expand All @@ -2633,7 +2627,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
//
if (!compIsForInlining())
{
if (JitConfig.JitDump().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitDump().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
verboseDump = true;
}
Expand Down Expand Up @@ -2868,32 +2862,32 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
opts.dspOrder = true;
}

if (JitConfig.JitGCDump().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitGCDump().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.dspGCtbls = true;
}

if (JitConfig.JitDisasm().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitDisasm().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.disAsm = true;
}

if (JitConfig.JitDisasm().contains("SPILLED", nullptr, nullptr))
if (JitConfig.JitDisasmSpilled())
{
opts.disAsmSpilled = true;
}

if (JitConfig.JitUnwindDump().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitUnwindDump().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.dspUnwind = true;
}

if (JitConfig.JitEHDump().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitEHDump().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.dspEHTable = true;
}

if (JitConfig.JitDebugDump().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitDebugDump().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.dspDebugInfo = true;
}
Expand Down Expand Up @@ -2931,7 +2925,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
opts.compLongAddress = true;
}

if (JitConfig.JitOptRepeat().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitOptRepeat().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.optRepeat = true;
}
Expand All @@ -2941,7 +2935,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
// JitEarlyExpandMDArraysFilter.
if (JitConfig.JitEarlyExpandMDArrays() == 0)
{
if (JitConfig.JitEarlyExpandMDArraysFilter().contains(info.compMethodName, info.compClassName,
if (JitConfig.JitEarlyExpandMDArraysFilter().contains(info.compMethodHnd, info.compClassHnd,
&info.compMethodInfo->args))
{
opts.compJitEarlyExpandMDArrays = true;
Expand Down Expand Up @@ -2982,7 +2976,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
printf(""); // in our logic this causes a flush
}

if (JitConfig.JitBreak().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitBreak().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
assert(!"JitBreak reached");
}
Expand All @@ -2994,8 +2988,8 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
}

if (verbose ||
JitConfig.JitDebugBreak().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args) ||
JitConfig.JitBreak().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
JitConfig.JitDebugBreak().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args) ||
JitConfig.JitBreak().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
compDebugBreak = true;
}
Expand All @@ -3014,14 +3008,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
s_pJitFunctionFileInitialized = true;
}
#else // DEBUG
if (!JitConfig.JitDisasm().isEmpty())
if (JitConfig.JitDisasm().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
const char* methodName = info.compCompHnd->getMethodName(info.compMethodHnd, nullptr);
const char* className = info.compCompHnd->getClassName(info.compClassHnd);
if (JitConfig.JitDisasm().contains(methodName, className, &info.compMethodInfo->args))
{
opts.disAsm = true;
}
opts.disAsm = true;
}
#endif // !DEBUG

Expand Down Expand Up @@ -3189,21 +3178,21 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
// JitForceProcedureSplitting is used to force procedure splitting on checked assemblies.
// This is useful for debugging on a checked build. Note that we still only do procedure
// splitting in the zapper.
if (JitConfig.JitForceProcedureSplitting().contains(info.compMethodName, info.compClassName,
if (JitConfig.JitForceProcedureSplitting().contains(info.compMethodHnd, info.compClassHnd,
&info.compMethodInfo->args))
{
opts.compProcedureSplitting = true;
}

// JitNoProcedureSplitting will always disable procedure splitting.
if (JitConfig.JitNoProcedureSplitting().contains(info.compMethodName, info.compClassName,
if (JitConfig.JitNoProcedureSplitting().contains(info.compMethodHnd, info.compClassHnd,
&info.compMethodInfo->args))
{
opts.compProcedureSplitting = false;
}
//
// JitNoProcedureSplittingEH will disable procedure splitting in functions with EH.
if (JitConfig.JitNoProcedureSplittingEH().contains(info.compMethodName, info.compClassName,
if (JitConfig.JitNoProcedureSplittingEH().contains(info.compMethodHnd, info.compClassHnd,
&info.compMethodInfo->args))
{
opts.compProcedureSplittingEH = false;
Expand Down Expand Up @@ -3319,7 +3308,7 @@ bool Compiler::compJitHaltMethod()
/* This method returns true when we use an INS_BREAKPOINT to allow us to step into the generated native code */
/* Note that this these two "Jit" environment variables also work for ngen images */

if (JitConfig.JitHalt().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitHalt().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
return true;
}
Expand Down Expand Up @@ -3423,7 +3412,7 @@ bool Compiler::compStressCompileHelper(compStressArea stressArea, unsigned weigh
}

if (!JitConfig.JitStressOnly().isEmpty() &&
!JitConfig.JitStressOnly().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
!JitConfig.JitStressOnly().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
return false;
}
Expand Down Expand Up @@ -3706,7 +3695,7 @@ void Compiler::compSetOptimizationLevel()

if (!theMinOptsValue)
{
if (JitConfig.JitMinOptsName().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitMinOptsName().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
theMinOptsValue = true;
}
Expand Down Expand Up @@ -4210,8 +4199,7 @@ const char* Compiler::compGetStressMessage() const
{
// Or is it excluded via name?
if (!JitConfig.JitStressOnly().isEmpty() ||
!JitConfig.JitStressOnly().contains(info.compMethodName, info.compClassName,
&info.compMethodInfo->args))
!JitConfig.JitStressOnly().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
// Not excluded -- stress can happen
stressMessage = " JitStress";
Expand Down Expand Up @@ -5159,7 +5147,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
#ifdef DEBUG
const char* fullName = info.compFullName;
#else
const char* fullName = eeGetMethodFullName(info.compMethodHnd);
const char* fullName =
eeGetMethodFullName(info.compMethodHnd, /* includeReturnType */ false, /* includeThisSpecifier */ false);
#endif

char debugPart[128] = {0};
Expand Down Expand Up @@ -5522,13 +5511,13 @@ bool Compiler::skipMethod()
return true;
}

if (JitConfig.JitExclude().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (JitConfig.JitExclude().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
return true;
}

if (!JitConfig.JitInclude().isEmpty() &&
!JitConfig.JitInclude().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
!JitConfig.JitInclude().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
return true;
}
Expand Down Expand Up @@ -5834,9 +5823,7 @@ int Compiler::compCompile(CORINFO_MODULE_HANDLE classPtr,
{
impTokenLookupContextHandle = impInlineInfo->tokenLookupContextHandle;

assert(impInlineInfo->inlineCandidateInfo->clsHandle == info.compCompHnd->getMethodClass(info.compMethodHnd));
info.compClassHnd = impInlineInfo->inlineCandidateInfo->clsHandle;

assert(impInlineInfo->inlineCandidateInfo->clsHandle == info.compClassHnd);
assert(impInlineInfo->inlineCandidateInfo->clsAttr == info.compCompHnd->getClassAttribs(info.compClassHnd));
// printf("%x != %x\n", impInlineInfo->inlineCandidateInfo->clsAttr,
// info.compCompHnd->getClassAttribs(info.compClassHnd));
Expand All @@ -5846,7 +5833,6 @@ int Compiler::compCompile(CORINFO_MODULE_HANDLE classPtr,
{
impTokenLookupContextHandle = METHOD_BEING_COMPILED_CONTEXT();

info.compClassHnd = info.compCompHnd->getMethodClass(info.compMethodHnd);
info.compClassAttr = info.compCompHnd->getClassAttribs(info.compClassHnd);
}

Expand Down
Loading