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

SPMI: Gracefully handle managed exceptions during replay #89654

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jul 28, 2023

Various JIT-EE APIs are expected to be able to throw exceptions, which SPMI will record and rethrow during replay. This change makes us handled managed exception that escapes from CILJit::compileMethod gracefully by not considering them as replay failures.
The expectation is that an exception with this exception code will never be thrown due to a JIT bug, and is always thrown as a result of a recorded exception that the JIT itself did not want to handle.

Also do a small amount of cleanup for the EH in SPMI.

Contributes to #47540 and #47546.

Various JIT-EE APIs are expected to be able to throw exceptions, which
SPMI will record and rethrow during replay. This change rethrows these
exceptions using a known exception code so that we can gracefully handle
these exceptions during replay; this makes us no longer consider these
as replay failures.

Also do a small amount of cleanup for the EH in SPMI.
@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 Jul 28, 2023
@ghost ghost assigned jakobbotsch Jul 28, 2023
@ghost
Copy link

ghost commented Jul 28, 2023

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

Issue Details

Various JIT-EE APIs are expected to be able to throw exceptions, which SPMI will record and rethrow during replay. This change rethrows these exceptions using a known exception code so that we can gracefully handle these exceptions during replay; this makes us no longer consider these as replay failures.

Also do a small amount of cleanup for the EH in SPMI.

Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @BruceForstall

@BruceForstall
Copy link
Member

So before, replay would raise the original captured exception. Now, it raises a new "distinguished" SPMI replay exception. Does the JIT ever capture this and continue compilation? E.g., I think the only case JIT compilation continues after a caught exception is during inlining. Before, if an exception was not thrown in an inlinee compile, would the JIT previously clean up and report a compilation failure. And now it doesn't catch the exception and spmi replay does, and doesn't consider it a replay failure?

Do we end up with spmi asm diffs due to this change?

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jul 29, 2023

So before, replay would raise the original captured exception. Now, it raises a new "distinguished" SPMI replay exception. Does the JIT ever capture this and continue compilation? E.g., I think the only case JIT compilation continues after a caught exception is during inlining. Before, if an exception was not thrown in an inlinee compile, would the JIT previously clean up and report a compilation failure. And now it doesn't catch the exception and spmi replay does, and doesn't consider it a replay failure?

Do we end up with spmi asm diffs due to this change?

Hmm, good point about inlining -- I hadn't considered that. It works out with this change because the error trap is not picky about what exceptions it catches, but I don't think it's ideal.

I'm going to revise the change to consider the managed exception code as a "non JIT" exception, and thus allow those exceptions without signaling a replay failure. The assumption will then be that this exception code is never a JIT bug.
For the cases I was looking at that should be good enough -- those cases were instances of MissingFieldException, MissingMethodException that the EE throws as part of resolveToken.

@jakobbotsch jakobbotsch changed the title SPMI: Gracefully handle recorded EE-side exceptions during replay SPMI: Gracefully handle managed exceptions during replay Jul 29, 2023
@jakobbotsch
Copy link
Member Author

Ping @BruceForstall, does this look ok to you now?

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM

@jakobbotsch jakobbotsch merged commit 3567ee0 into dotnet:main Aug 2, 2023
111 checks passed
@jakobbotsch jakobbotsch deleted the spmi-immediate-replay-fixes-2 branch August 2, 2023 08:39
@jakobbotsch
Copy link
Member Author

Hmm, I think I'll have to go back to the other approach again, or also allow the C++ exception code. This method isn't sufficient for crossgen2/ilc since they throw exceptions across the JIT-EE interface using C++ exceptions.

I think the original solution might be fine since the JIT is not expected to directly interact with exceptions, but rather go through JIT-EE APIs like runWithErrorTrap -- so during SPMI replay we can unpack the "distinguished" exception type and handle it as if it was the underlying exception (though it doesn't look we actually need to handle it specially).

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

2 participants