-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[clr-interp] Cause compilation to be skipped when using function only used by tail-call helper #123508
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
[clr-interp] Cause compilation to be skipped when using function only used by tail-call helper #123508
Conversation
… used by tail-call helper
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
|
/azp run runtime-interpreter |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for skipping interpreter compilation when encountering the NextCallReturnAddress intrinsic that is not followed by a POP instruction. This intrinsic is used in tail-call helper functions which are currently only supported by the JIT, not the interpreter.
Changes:
- Introduces a new
SKIPCODEerror handling function that throwsCORJIT_SKIPPEDexception to skip compilation - Adds handling for
NI_System_StubHelpers_NextCallReturnAddressinEmitNamedIntrinsicCallto skip compilation when the intrinsic is not followed by POP
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/interpreter/failures.h | Added declaration for new SKIPCODE function |
| src/coreclr/interpreter/eeinterp.cpp | Implemented SKIPCODE function that throws CORJIT_SKIPPED exception |
| src/coreclr/interpreter/compiler.cpp | Added case handler for NextCallReturnAddress intrinsic to skip compilation when not followed by POP |
janvorli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
|
/ba-g known infra issues |
The NextCallReturnAddress intrinsic is only actually used in a case inside of the tail-call helper function, and in some of our interp modes we occasionally try to interpreter this tail-call helper. Instead of allowing that to happen, just skip the compilation.