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.

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

Display names of handles in dumps #97573

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

BruceForstall
Copy link
Member

For class/method/field handles, display their name in dumps in addition to their handle value.

Also fixes a problem in assertion prop dumping where 64-bit class handle constants were truncated to 32-bit in dump.

For class/method/field handles, display their name in dumps
in addition to their handle value.

Also fixes a problem in assertion prop dumping where 64-bit class
handle constants were truncated to 32-bit in dump.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 26, 2024
@ghost ghost assigned BruceForstall Jan 26, 2024
@ghost
Copy link

ghost commented Jan 26, 2024

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

For class/method/field handles, display their name in dumps in addition to their handle value.

Also fixes a problem in assertion prop dumping where 64-bit class handle constants were truncated to 32-bit in dump.

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

Examples:

In BB08 New Global Type     Assertion: ($82,$202) V13.01 is Exact Type MT(0x00007FFD2A2DD8F0 System.Text.RegularExpressions.RegexNode), index = #08

[000007] H---------- gctx                                +--*  CNS_INT(h) long   0x7ffd29fe7f38 class System.Span`1[System.Object]

[000143] H---------- gctx                    +--*  CNS_INT(h) long   0x7ffd2a4b3268 method System.Threading.StackHelper:CallOnEmptyStack[System.Text.RegularExpressions.RegexNode,System.Text.RegularExpressions.RegexNode,ubyte](System.Action`3[System.Text.RegularExpressions.RegexNode,System.Text.RegularExpressions.RegexNode,ubyte],System.Text.RegularExpressions.RegexNode,System.Text.RegularExpressions.RegexNode,ubyte)

@BruceForstall BruceForstall requested a review from EgorBo January 26, 2024 20:52
@BruceForstall
Copy link
Member Author

@EgorBo @dotnet/jit-contrib PTAL

@ryujit-bot
Copy link

Diff results for #97573

Throughput diffs

Throughput diffs for windows/arm64 ran on linux/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.windows.arm64.checked.mch +0.01%

Details here


@ryujit-bot
Copy link

Diff results for #97573

Throughput diffs

Throughput diffs for osx/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.osx.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on windows/x64

MinOpts (-0.01% to +0.00%)
Collection PDIFF
libraries.pmi.windows.arm64.checked.mch -0.01%

Details here


@BruceForstall BruceForstall merged commit b1d7ad6 into dotnet:main Feb 12, 2024
127 of 129 checks passed
@BruceForstall BruceForstall deleted the DisplayHandlesInDumps branch February 12, 2024 21:52
@MichalStrehovsky
Copy link
Member

This broke JitDump in native AOT (and probably crossgen too). It's casting things to CORINFO_METHOD_HANDLE and CORINFO_CLASS_HANDLE that were not CORINFO_METHOD_HANDLE and CORINFO_CLASS_HANDLE. The distinction matters in prejit.

@jakobbotsch
Copy link
Member

jakobbotsch commented Feb 16, 2024

Yeah, the handle that we should be using here is different (e.g. we have a debug-only GenTreeIntCon::gtTargetHandle to use if you have a GenTreeIntCon, but its maintenance is best-effort). #77789, #70437, #78382 and #70452 were related.

@EgorBo
Copy link
Member

EgorBo commented Feb 16, 2024

A VN can be converted to CORINFO_CLASS_HANDLE like this:

CORINFO_CLASS_HANDLE  clsHandle;
if (vnStore->IsVNTypeHandle(clsVN) && 
    vnStore->EmbeddedHandleMapLookup(vnStore->ConstantValue<ssize_t>(clsVN), (ssize_t*)&clsHandle))
{
    ...
}

(probably, can be a bit relaxed for non-aot)

@BruceForstall
Copy link
Member Author

BruceForstall commented Feb 16, 2024

This broke JitDump in native AOT (and probably crossgen too). It's casting things to CORINFO_METHOD_HANDLE and CORINFO_CLASS_HANDLE that were not CORINFO_METHOD_HANDLE and CORINFO_CLASS_HANDLE. The distinction matters in prejit.

What's the failure mode? In crossgen2 I see a bunch of <unknown class> / <unknown method>.

@BruceForstall
Copy link
Member Author

I don't understand: we have a handle constant, but it's not a handle constant? What is it?

@jakobbotsch
Copy link
Member

What's the failure mode? In crossgen2 I see a bunch of <unknown class> / <unknown method>.

This is probably because the SPMI exception handler around the printers is catching the exception thrown on the EE side.

@SingleAccretion
Copy link
Contributor

I don't understand: we have a handle constant, but it's not a handle constant? What is it?

CORINFO_CLASS_HANDLE's meaning is overloaded: it means both a compile-time handle, and an "embedded" handle. In IR and VN, handles are always "embedded", so you need something like Egor's code to get back to the compile-time handle.

(This is the case for all other handle types too, FWIW)

@BruceForstall
Copy link
Member Author

So a "compile-time handle" is a value the JIT can use to interrogate properties on the VM interface, and an "embedded handle" is the constant that the JIT writes into the code stream?

@SingleAccretion
Copy link
Contributor

So a "compile-time handle" is a value the JIT can use to interrogate properties on the VM interface, and an "embedded handle" is the constant that the JIT writes into the code stream?

Yep, that is exactly correct.

@EgorBo
Copy link
Member

EgorBo commented Feb 16, 2024

(This is the case for all other handle types too, FWIW)

I don't think it's needed for GTF_ICON_OBJECT_HANDLE?

@SingleAccretion
Copy link
Contributor

I don't think it's needed for GTF_ICON_OBJECT_HANDLE?

I suppose so. That is a handle type which only has the embedded form.

@BruceForstall
Copy link
Member Author

It seems odd we don't use/store the compile handle everywhere and look up the embedded handle only at codegen time when deciding what constant to embed.

BruceForstall added a commit to BruceForstall/runtime that referenced this pull request Feb 21, 2024
Revert dotnet#97573 to previous behavior (not dumping handle strings) for
NativeAOT and R2R compiles; those require more work to find the
handle to use.
BruceForstall added a commit that referenced this pull request Feb 21, 2024
Revert #97573 to previous behavior (not dumping handle strings) for
NativeAOT and R2R compiles; those require more work to find the
handle to use.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants