-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Emit task returning thunks in crossgen2 #122651
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
base: main
Are you sure you want to change the base?
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 PR implements support for emitting Task-returning async method thunks in crossgen2 by encoding them as MethodSignatures with the AsyncVariant flag set. The changes introduce a new GetPrimaryMethodDesc helper method to navigate between different MethodDesc variants (async, unboxing, P/Invoke) and their primary metadata representations. The GC reference map emission is updated to account for the additional async continuation parameter.
Key changes:
- Adds
AsyncVariantflag to ReadyToRunMethodSigFlags enum (requiring uint instead of byte to accommodate 0x100 value) - Implements
GetPrimaryMethodDescextension method to resolve various MethodDesc wrappers to their primary ECMA method - Updates ArgIterator and GCRefMapBuilder to handle async continuation parameter in both managed and native code
- Modifies ReadyToRunILProvider to emit Task-returning thunks for async methods
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/frames.cpp | Adds async continuation parameter handling in FakeGcScanRoots for GC scanning |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs | Adds display support for ASYNC flag in method signatures |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.cs | New file introducing GetPrimaryMethodDesc helper to unwrap MethodDesc variants |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | Updates token resolution to use GetPrimaryMethodDesc and removes version bubble checks |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Adds new MethodDescExtensions.cs file to project |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.cs | Implements Task-returning async thunk emission for async methods |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.cs | Updates module resolution to use GetPrimaryMethodDesc |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Updates async variant assertion to use new helper method |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs | Adds IsPrimaryMethodDesc check in debug code and minor formatting fix |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs | Adds AsyncVariant flag emission in method signatures |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs | Replaces unboxing check with IsPrimaryMethodDesc for fixup optimization |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs | Adds async continuation parameter to GC reference map generation |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs | Implements async continuation argument offset calculation and allocation logic |
| src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncResumptionStub.cs | Exposes TargetMethod property for GetPrimaryMethodDesc navigation |
| src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs | Changes ReadyToRunMethodSigFlags from byte to uint and adds AsyncVariant flag |
| src/coreclr/tools/Common/Compiler/AsyncMethodVariant.cs | Adds GetAsyncVariant and GetTargetOfAsyncVariant extension methods |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
Outdated
Show resolved
Hide resolved
...lr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
54e7d22 to
b779849
Compare
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 16 out of 16 changed files in this pull request and generated no new comments.
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
Outdated
Show resolved
Hide resolved
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The generated IL thunk does not have valid tokens, so the tokens emitted in the EH handler info are invalid. It's not an invariant that there is always a reference to System.Exception or System.Object in the assembly that we are generating thunks for, so we can't always get a valid Token for the exception type token field. Instead, we can create a filter that does a type check against System.Exception. The reference to System.Exception will be filled in with a Fixup with a module override if we don't have a reference to System.Exception in the assembly.
|
/azp run runtime-coreclr crossgen2 outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| codestream.EndTry(tryFilterRegion); | ||
| } | ||
| // Catch | ||
| // Filter - check if the caught object is a System.Exception |
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.
Should we make this workaround for R2R encoding limitation specific to R2R to avoid leaving perf on the table in NAOT?
What would it take to fix the encoding limitation in R2R?
Emits Task-returning async method thunks in crossgen2 as MethodSignatures with the AsyncVariant flag set
Adds
GetPrimaryMethodDescto get the most natural "primary" method desc for MethodDesc's that point to the same Ecma/Instantiated method. This is used in the places where we cast to EcmaMethod to get relevant metadata information.GCRefMap emission also needed to be updated to account for the continuation parameter. Runtime's
FakeGcScanRoots(used to verify the ReadyToRun images GCRefMap in debug config) also needed to updated to handle the continuation parameter.