Compile runtime async versions of synchronous task-returning methods#128384
Compile runtime async versions of synchronous task-returning methods#128384jakobbotsch wants to merge 16 commits into
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR extends the JIT↔EE async infrastructure so the JIT can compile a dedicated “runtime async version” of synchronous Task/ValueTask-returning methods, including a tail-position optimization that turns eligible tail calls/returns into runtime-async awaits.
Changes:
- Adds a new JIT↔EE interface API (
getAwaitReturnCall) and plumbs it through SuperPMI and generated wrappers/shims. - Updates CoreCLR async thunk IL generation and
AsyncHelpersto separate “suspend” helpers from typedTransparentAwait(...)helpers used by the JIT. - Updates the JIT importer to recognize “async-version tail await” patterns and to wrap async-version returns in an await via the new EE callback.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/async/reflection/reflection.cs | Disables several assertions related to reflection/stack behavior for await-on-task-returning paths. |
| src/coreclr/vm/prestub.cpp | Adjusts IL header retrieval logic for async variant methods. |
| src/coreclr/vm/method.hpp | Changes IL-header eligibility rules for async/thunk/return-dropping methods; exposes GetAsyncThunkResultTypeSig. |
| src/coreclr/vm/metasig.h | Adds metasig entries for Task/ValueTask TransparentAwait helper signatures. |
| src/coreclr/vm/jitinterface.h | Adds helper declarations related to runtime lookup computation for new await-return support. |
| src/coreclr/vm/jitinterface.cpp | Implements getAwaitReturnCall and runtime-lookup computation for generic await helpers; sets CORINFO_ASYNC_VERSION for applicable methods. |
| src/coreclr/vm/corelib.h | Adds/updates CoreLibBinder entries for new suspend/await AsyncHelpers methods with proper signatures. |
| src/coreclr/vm/asyncthunks.cpp | Switches thunk-emitted calls from TransparentAwait* to TransparentSuspendFor* helpers. |
| src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp | Records/replays the new getAwaitReturnCall API for SuperPMI. |
| src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp | Forwards the new API in the simple shim. |
| src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp | Counts/forwards the new API in the counter shim. |
| src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp | Records the new API in the collector shim. |
| src/coreclr/tools/superpmi/superpmi-shared/spmirecordhelper.h | Makes lookup restore helpers take const& and updates corresponding implementations. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h | Adds recording/replay plumbing for getAwaitReturnCall. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | Implements record/dump/replay for getAwaitReturnCall. |
| src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h | Registers the new lightweight-map packet for GetAwaitReturnCall. |
| src/coreclr/tools/superpmi/superpmi-shared/agnostic.h | Adds agnostic structs for CORINFO_LOOKUP* and the await-return result payload. |
| src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncThunks.cs | Updates IL stub emitter to use TransparentSuspendFor* helper names. |
| src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt | Adds the new interface method to thunk generation input. |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Adds a stub managed implementation of getAwaitReturnCall for the managed JIT interface. |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs | Adds unmanaged callback plumbing for getAwaitReturnCall. |
| src/coreclr/tools/aot/jitinterface/jitinterface_generated.h | Adds the new callback and wrapper method to the AOT jitinterface wrapper. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | Renames “suspend” helpers and adds typed TransparentAwait(...) overloads used by the JIT. |
| src/coreclr/jit/importercalls.cpp | Adds tail-await handling for async-version tail-await prefix and blocks inlining of async-version callees. |
| src/coreclr/jit/importer.cpp | Adds async-version tail-call recognition, wraps async-version returns in an await via getAwaitReturnCall, and introduces impWrapTopOfStackInAwait. |
| src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp | Adds wrapper forwarding for getAwaitReturnCall. |
| src/coreclr/jit/ICorJitInfo_names_generated.h | Adds name entry for getAwaitReturnCall. |
| src/coreclr/jit/fginline.cpp | Minor whitespace change near async flag handling for inlinee compilation. |
| src/coreclr/jit/compiler.h | Introduces PREFIX_IS_ASYNC_VERSION_TAIL_AWAIT, impWrapTopOfStackInAwait, and compIsAsyncVersion(). |
| src/coreclr/jit/compiler.cpp | Adds verbose printing when compiling an async-version method. |
| src/coreclr/inc/jiteeversionguid.h | Updates JIT↔EE version GUID due to interface change. |
| src/coreclr/inc/icorjitinfoimpl_generated.h | Adds getAwaitReturnCall override to the generated ICorJitInfo impl header. |
| src/coreclr/inc/corinfo.h | Adds CORINFO_ASYNC_VERSION and the new ICorStaticInfo::getAwaitReturnCall method. |
| // TODO: crossgen2 cannot handle us removing this | ||
| if (isAwait && IsReadyToRun() && (callInfo.kind == CORINFO_CALL)) |
There was a problem hiding this comment.
@jtschuster When I remove this crossgen2 fails to compile e.g. async-inline-thunks.
Example console log: https://helixr1107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-128384-merge-8f95d8483b3d47da86/readytorun/1/console.ddc105bc.log
C:\h\w\A9C20992\w\B36909F7\e>call readytorun\readytorun\readytorun.cmd -usewatcher
BEGIN EXECUTION
"C:\h\w\A9C20992\p\watchdog.exe" 29 "C:\h\w\A9C20992\p\corerun.exe" -p "System.Reflection.Metadata.MetadataUpdater.IsSupported=false" -p "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization=true" readytorun.dll
23:43:15.210 Running test: readytorun/async-inline-thunks/async-inline-thunks/async-inline-thunks.cmd
Unhandled exception. ILCompiler.CodeGenerationFailedException: Code generation failed for method 'Async variant: [async-inline-thunks]InlineThunks.SmallNonRuntimeValueTaskAsync(int32)'
---> System.NotImplementedException: [S.P.CoreLib]System.Threading.Tasks.ValueTask`1.get_IsCompleted()
at ILCompiler.DependencyAnalysis.ReadyToRun.ModuleTokenResolver.GetModuleTokenForMethod(MethodDesc, Boolean, Boolean) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleTokenResolver.cs:line 125
at Internal.JitInterface.CorInfoImpl.HandleToModuleToken(CORINFO_RESOLVED_TOKEN&, Boolean&) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs:line 1496
at Internal.JitInterface.CorInfoImpl.ComputeMethodWithToken(MethodDesc, CORINFO_RESOLVED_TOKEN&, TypeDesc, Boolean) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs:line 1416
at Internal.JitInterface.CorInfoImpl.getCallInfo(CORINFO_RESOLVED_TOKEN&, CORINFO_RESOLVED_TOKEN*, CORINFO_METHOD_STRUCT_*, CORINFO_CALLINFO_FLAGS, CORINFO_CALL_INFO*) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs:line 2647
at Internal.JitInterface.CorInfoImpl._getCallInfo(IntPtr, IntPtr*, CORINFO_RESOLVED_TOKEN*, CORINFO_RESOLVED_TOKEN*, CORINFO_METHOD_STRUCT_*, CORINFO_CALLINFO_FLAGS, CORINFO_CALL_INFO*) in /_/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs:line 2568
What seems to happen is that while compiling AwaitNonRuntimeValueTaskAsync JIT ends up with a call to the runtime async callable thunk of SmallNonRuntimeValueTaskAsync. But nothing has added this thunk to dependencies, and then while trying to inline the thunk we ask about ValueTask.get_IsCompleted which hasn't been added to the mutable module. Can you help with figuring out the right fix?
There was a problem hiding this comment.
Looks like this might be all that's needed:
diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ExternalReferenceTokenManager.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ExternalReferenceTokenManager.cs
index 1edea14b3f1..f046893f12f 100644
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ExternalReferenceTokenManager.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ExternalReferenceTokenManager.cs
@@ -122,7 +122,7 @@ private void EnsureMethodDefTokensAreAvailableInVersionBubble(MethodDesc methodD
AddTokenToMutableModule(ecmaMethod);
return;
}
- if (methodDesc.HasInstantiation)
+ if (methodDesc.HasInstantiation || methodDesc.OwningType.HasInstantiation)
{
EnsureTypeDefTokensAreAvailableInVersionBubble(methodDesc.GetMethodDefinition().OwningType);
foreach (TypeDesc instParam in methodDesc.Instantiation)There was a problem hiding this comment.
Thanks! Seems to do the trick!
| @@ -582,7 +582,7 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig | |||
| // No, tail await to TransparentAwaitValueTask | |||
|
Another pattern we may want to recognize: |
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] GetCurrentMethodAsync()", GetCurrentMethodAwait().Result); | ||
|
|
||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] GetCurrentMethodTask()", GetCurrentMethodTask().Result); | ||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] GetCurrentMethodTask()", GetCurrentMethodAwaitTask().Result); | ||
| //Assert.Equal("System.Threading.Tasks.Task`1[System.String] GetCurrentMethodTask()", GetCurrentMethodAwaitTask().Result); | ||
| } |
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackAsync(Int32)", FromStackAsync(0).Result); | ||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackAsync(Int32)", FromStackAwait(0).Result); | ||
|
|
||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackTask(Int32)", FromStackTask(0).Result); | ||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackTask(Int32)", FromStackAwaitTask(0).Result); | ||
| //Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackTask(Int32)", FromStackAwaitTask(0).Result); | ||
| } |
| Assert.Equal("Void FromStack(Int32)", FromStackTask(1).Result); | ||
| // Note: we do not go through suspend/resume, that is why we see the actual caller. | ||
| // we do not see the async->Task thunk though. | ||
| Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackAwaitTask(Int32)", FromStackAwaitTask(1).Result); | ||
| //Assert.Equal("System.Threading.Tasks.Task`1[System.String] FromStackAwaitTask(Int32)", FromStackAwaitTask(1).Result); | ||
| } |
| Assert.Equal("FromStackDMIAsync", FromStackDMIAsync(0).Result); | ||
| Assert.Equal("FromStackDMIAsync", FromStackDMIAwait(0).Result); | ||
|
|
||
| Assert.Equal("FromStackDMITask", FromStackDMITask(0).Result); | ||
| Assert.Equal("FromStackDMITask", FromStackDMIAwaitTask(0).Result); | ||
| //Assert.Equal("FromStackDMITask", FromStackDMIAwaitTask(0).Result); | ||
| } |
| Assert.Equal("FromStackDMI", FromStackDMITask(1).Result); | ||
| // Note: we do not go through suspend/resume, that is why we see the actual caller. | ||
| // we do not see the async->Task thunk though. | ||
| Assert.Equal("FromStackDMIAwaitTask", FromStackDMIAwaitTask(1).Result); | ||
| //Assert.Equal("FromStackDMIAwaitTask", FromStackDMIAwaitTask(1).Result); | ||
| } |
| if (compIsAsyncVersion()) | ||
| { | ||
| if ((codeAddr + sz < codeEndp) && (getU1LittleEndian(codeAddr + sz) == CEE_RET)) | ||
| { | ||
| JITDUMP("\nRecognized tail-call in async version\n"); | ||
| awaitOffset = (IL_OFFSET)(codeAddr - info.compCode); | ||
| isAwait = true; | ||
| prefixFlags |= PREFIX_IS_ASYNC_VERSION_TAIL_AWAIT; | ||
|
|
||
| // Consume the ret, but leave sz that will be consumed when we loop around. | ||
| codeAddrAfterMatch = codeAddr + sz + 1 - sz; | ||
| } |
|
@steveisok @noahfalk @tommcdon What do you think something like this would take on the diagnostics side? The async variant for a non-async method no longer delegates to the non-async method; rather, it is compiled with the same IL as the non-async variant. So now placing a breakpoint in a task-returning function may need to place it in both the async and non-async variant, call stacks should not ignore the async variant anymore, etc. It becomes much closer to having multiple generic instantiations of the same method. What would it take for diagnostics to make something like this work? |
Hi @jakobbotsch! Few questions to help gauge the diagnostics impact:
|
In this prototype they aren't, but I am not sure if we will want to keep it as a possible strategy or not. In some cases delegation to the original method does not come with any downsides compared to compiling a separate version.
Currently in this prototype
Similarly to generic instantiations the set of sequence points will be different between different compilations-- the sequence points will refer to the original IL, so the IL offsets will be the same, but the native offsets will be different.
Yes, it does.
When an async method awaits a task-returning method it will generally be calling into the async version of the task-returning method. Or, it might call |
Instead of delegating from runtime async callable thunks to the original task returning methods this PR compiles a fully separate runtime async version. It then adds a guaranteed optimization to make tail calls in the synchronous task-returning methods into runtime async calls.
This is one potential approach to fix #115771.