Skip to content

Commit

Permalink
[release/7.0] JIT: Simplify JitDisasm matching behavior and support n…
Browse files Browse the repository at this point in the history 
…amespaces/generics in release (#75260)

* 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.

* Update JIT-EE GUID

Avoid using the same JIT-EE GUID as the main branch.
  • Loading branch information
@jakobbotsch
jakobbotsch authored Sep 8, 2022
1 parent 5d71eeb commit 3db9d17
Show file tree
Hide file tree
Showing 18 changed files with 585 additions and 631 deletions.
54 changes: 42 additions & 12 deletions docs/design/coreclr/jit/viewing-jit-dumps.md
View file
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
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
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 = { /* 6be47e5d-a92b-4d16-9280-f63df646ada4 */
0x6be47e5d,
0xa92b,
0x4d16,
{0x92, 0x80, 0xf6, 0x3d, 0xf6, 0x46, 0xad, 0xa4}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
80 changes: 33 additions & 47 deletions src/coreclr/jit/compiler.cpp
View file
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

0 comments on commit 3db9d17

Please sign in to comment.