-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: Handle async calls implemented via ldvirtftn
properly
#118726
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
Conversation
Handling was missing to mark these as async and to insert async continuations.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
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 PR adds proper async call handling for virtual method calls that are implemented via ldvirtftn
in the JIT compiler. The changes ensure that these calls are correctly marked as async and have the necessary async continuation arguments inserted.
Key changes:
- Extracts async call setup logic into reusable helper methods
- Adds async handling for ldvirtftn-based virtual calls
- Includes test infrastructure updates to enable async testing
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/coreclr/jit/importercalls.cpp | Refactors async call handling into helper methods and adds async support for ldvirtftn calls |
src/coreclr/jit/compiler.h | Adds declarations for new async call helper methods |
src/tests/async/Directory.Build.targets | Enables async test builds by uncommenting DisableProjectBuild |
src/tests/async/async-gvm/async-gvm.csproj | Creates new test project for async generic virtual method testing |
src/tests/async/async-gvm/async-gvm.cs | Implements test cases for async generic virtual methods |
Opened #118735 to fix the failing test (flakey). |
All other failures are known, cc @dotnet/jit-contrib @VSadov PTAL @AndyAyersMS |
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. Thanks!
Handling was missing to mark these as async and to insert async continuations.
Fix #118714