-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Emit IL for AsyncResumptionStub #121456
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
Emit IL for AsyncResumptionStub #121456
Conversation
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
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 implements the async resumption stub for native AOT compilation by porting the CoreCLR VM implementation from jitinterface.cpp to C#. The stub is used to resume async methods after suspension.
- Replaces the placeholder
ThrowNotSupportedExceptionimplementation with a complete IL emission that matches CoreCLR'sgetAsyncResumptionStub - Adds proper caching of the generated IL and comparison logic for the stub method
- Updates the constructor assertion to use a simplified
IsAsyncCall()method
Comments suppressed due to low confidence (2)
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs:148
- This CompareToImpl method duplicates the existing implementation in AsyncResumptionStub.Sorting.cs (lines 13-16). Both implementations are identical and will cause a compilation error due to duplicate method definitions in the same partial class.
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs:127 - The
brtrueinstruction requires an address on the stack, butEmitLdLocaloads the address of the local variable. This should beEmitLdLocinstead to load the value of the continuation for the branch condition. The CoreCLR reference implementation usesEmitLDLOC(line 14772 in jitinterface.cpp).
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
…mplementations of AsyncResumptionStub
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs
Outdated
Show resolved
Hide resolved
|
Created #121458 to keep the corelib changes in a separate commit. |
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
…ub.cs Co-authored-by: Vladimir Sadov <vsadov@microsoft.com>
Optimizations are currently broken. Make sure not to run ilc with |
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
…r different MethodDescs Ran into this as I was trying something with #121456. That PR introduces a situation where two MethodDescs could map to the same EntryPoint. We didn't previously have that since MethodEntrypointOrTentativeMethod doesn't support unboxing thunks (that do a similar trick).
MichalStrehovsky
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.
Looks like this would work otherwise!
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
…r different MethodDescs (#121703) Ran into this as I was trying something with #121456. That PR introduces a situation where two MethodDescs could map to the same EntryPoint. We didn't previously have that since MethodEntrypointOrTentativeMethod doesn't support unboxing thunks (that do a similar trick). Without this fix we would end up with two `TentativeInstanceMethodNode` that point to the same EntryPoint and that doesn't lead to anything good. These need to be 1:1 with entrypoints. Cc @dotnet/ilc-contrib
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs
Outdated
Show resolved
Hide resolved
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 5 out of 5 changed files in this pull request and generated 2 comments.
| if (_signature is null) | ||
| return InitializeSignature(); | ||
|
|
||
| return _signature; |
Copilot
AI
Nov 18, 2025
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.
The Signature property should follow the null-coalescing pattern like other properties in the codebase. Use return _signature ??= InitializeSignature(); instead of explicit null check and separate assignment.
| if (_signature is null) | |
| return InitializeSignature(); | |
| return _signature; | |
| return _signature ??= InitializeSignature(); |
| /// This method should be marked IsAsync=false and HasInstantiation=false. These are defaults | ||
| /// for MethodDesc and so aren't explicitly set in the code below. | ||
| /// </summary> | ||
| internal sealed partial class ExplicitContinuationAsyncMethod : MethodDesc |
Copilot
AI
Nov 18, 2025
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.
The ExplicitContinuationAsyncMethod class lacks a mangling partial class file. Based on the codebase pattern (AsyncResumptionStub, AsyncMethodVariant, etc.), internal call methods that can be referenced should implement IPrefixMangledMethod. Consider adding an AsyncResumptionStub.Mangling.cs partial class for ExplicitContinuationAsyncMethod to provide proper name mangling support.
Updates the AsyncResumptionStub to emit IL. The implementation was ported from the runtime's getAsyncResumptionStub
runtime/src/coreclr/vm/jitinterface.cpp
Lines 14646 to 14833 in 5668ab9