-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Long-lived AsyncMethodVariant in MethodWithGCInfo/MethodWithToken #121357
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
Long-lived AsyncMethodVariant in MethodWithGCInfo/MethodWithToken #121357
Conversation
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 async method variants in the ReadyToRun compiler. The changes introduce a new AsyncMethodVariant type system abstraction that represents special calling convention variants for async methods.
Key Changes:
- Added new
AsyncMethodVariantinfrastructure to represent async method implementations and thunks with special calling conventions - Extended the type system with async-related properties (
IsAsync,AsyncMethodKind) across multiple method descriptor types - Updated ReadyToRun signatures and metadata to track async variant methods
- Modified code generation to create async variants for Task-returning methods
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ILCompiler.TypeSystem.csproj | Added new async-related partial class files to the build |
| ILCompiler.ReadyToRun.csproj | Removed obsolete async method descriptor files |
| CorInfoImpl.ReadyToRun.cs | Added asyncVariant parameter handling and async method factory |
| ReadyToRunILProvider.cs | Added early return for async methods that aren't yet implemented |
| ReadyToRunTableManager.cs | Added IsAsyncThunk condition for generic methods tracking |
| ReadyToRunCodegenNodeFactory.cs | Fixed whitespace formatting |
| SignatureBuilder.cs | Added async thunk variant flag to method signatures |
| MethodWithGCInfo.cs | Added debug assertion and AsyncVariant property |
| MethodFixupSignature.cs | Added task-returning check to optimization condition |
| InstanceEntryPointTableNode.cs | Added asyncVariant parameter to signature building |
| AllMethodsOnTypeNode.cs | Added automatic async variant dependencies for Task-returning methods |
| ReadyToRunObjectWriter.cs | Removed unused using statement |
| MethodForRuntimeDeterminedType.cs | Added IsAsync and AsyncMethodKind property forwarding |
| EcmaMethod.cs | Reorganized flags and added formatting pragma |
| EcmaMethod.Async.cs | New file implementing async method kind logic |
| TypeSystemContext.cs | Changed return type to be more specific |
| TypeSystemContext.Async.cs | New file adding GetAsyncVariant factory method |
| MethodForInstantiatedType.cs | Refactored signature initialization |
| MethodForInstantiatedType.Async.cs | New file forwarding AsyncMethodKind from typical method |
| MethodDesc.cs | Renamed AsyncCallConv flag for consistency |
| MethodDesc.Async.cs | New file with core async method infrastructure |
| MethodDelegator.Async.cs | New file making AsyncMethodKind abstract |
| InstantiatedMethod.Async.cs | New file forwarding AsyncMethodKind from method definition |
| AsyncMethodVariant.cs | New file implementing the async variant wrapper type |
| UnboxingMethodDesc.cs | Added AsyncMethodKind property forwarding |
| CorInfoTypes.cs | Added CORINFO_CALLCONV_ASYNCCALL constant |
| CorInfoImpl.cs | Added async calling convention handling |
| ReadyToRunConstants.cs | Changed flags enum to ushort and added async thunk flag |
src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.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.
This looks good to me otherwise. We could merge with this feedback addressed.
src/coreclr/tools/Common/TypeSystem/Common/AsyncMethodVariant.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj
Show resolved
Hide resolved
src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/Common/AsyncMethodVariant.cs
Outdated
Show resolved
Hide resolved
...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.cs
Outdated
Show resolved
Hide resolved
|
/ba-g android timeouts |
Incorporates feedback from #121218, creating a long-lived async MethodDesc. There will be more warts that need to be worked out once we actually compile these, in particular lots of
(EcmaMethod)methodDesc.GetMethodDefinition(), which doesn't work withAsyncMethodVariant.