Skip to content

Conversation

@jakobbotsch
Copy link
Member

getStaticFieldCurrentClass has very different behavior depending on whether pIsSpeculative is passed or not, and we need to record the resulting class handle in both cases.

This fixes a case of replays not working after recording that I hit while tracking down a separate issue.

getStaticFieldCurrentClass has very different behavior depending on
whether pIsSpeculative is passed or not, and we need to record the
resulting class handle in both cases.

This fixes a case of replays not working after recording that I hit
while tracking down a separate issue.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 23, 2023
@ghost ghost assigned jakobbotsch Mar 23, 2023
@ghost
Copy link

ghost commented Mar 23, 2023

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

Issue Details

getStaticFieldCurrentClass has very different behavior depending on whether pIsSpeculative is passed or not, and we need to record the resulting class handle in both cases.

This fixes a case of replays not working after recording that I hit while tracking down a separate issue.

Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib. Will wait with merging this until the weekend.

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

It's also one of those methods that potentially can give different answers on successive calls, since it's querying the class initialization state kept by the runtime.

I assume SPMI will just overwrite and record the last answer it sees, which means the replay might not be entirely faithful and/or might not even work.

We could perhaps consider caching answers on the jit interface side -- I believe we do this for things like profile data that is also possibly volatile.

@jakobbotsch jakobbotsch merged commit 922c75c into dotnet:main Mar 26, 2023
@jakobbotsch jakobbotsch deleted the spmi-fix-recGetStaticFieldCurrentClass branch March 26, 2023 13:21
0x928e,
0x4281,
{0xab, 0x68, 0xcd, 0x40, 0x17, 0xc1, 0x14, 0x1b}
constexpr GUID JITEEVersionIdentifier = { /* 95c688c7-28cf-4b1f-922a-11bf3947e56f */
Copy link
Member

Choose a reason for hiding this comment

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

Side question: now that runtime and corert are in a monorepo, can we replace JITEEVersionIdentifier with computed hash without needing to hardcode in the sources?

$ git ls-files -s src/coreclr/vm/jitinterface.h | while read _ hash __; do
    printf 'a "more" universal JIT interface identifier: %s' "$hash"
done

a "more" universal JIT interface identifier: cc2cccca72a0961f75ecb50fa79b07888ebec164

@ghost ghost locked as resolved and limited conversation to collaborators Apr 26, 2023
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.

3 participants