JIT: Optimize always suspending helpers#130493
Conversation
Several async helpers are known to always suspend. We can skip the "returned a continuation" check for those.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR teaches the CoreCLR JIT’s runtime-async transformation to treat certain async helper calls as always suspending, allowing it to skip generating the “did the call return a continuation?” null check and instead unconditionally branch to the suspension path. To enable this, the relevant helpers are marked [Intrinsic] and recognized by lookupNamedIntrinsic, and a new AsyncCallInfo::AlwaysSuspends bit flows from import to async transformation.
Changes:
- Mark
AsyncHelpers.AwaitAwaiter/UnsafeAwaitAwaiterand CoreCLR’sSuspend*/TransparentSuspend*helpers as[Intrinsic]so the JIT can identify them reliably. - Add new named intrinsics for these helpers and plumb
NamedIntrinsicintoimpSetupAsyncCallto setAsyncCallInfo.AlwaysSuspends. - Update
AsyncTransformation::CreateCheckAndSuspendAfterCallto omit the null-continuation check whenAlwaysSuspendsis set.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs | Marks AwaitAwaiter / UnsafeAwaitAwaiter as [Intrinsic] to enable JIT recognition. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | Marks Suspend* / TransparentSuspend* helpers as [Intrinsic] for JIT recognition. |
| src/coreclr/jit/namedintrinsiclist.h | Adds new NamedIntrinsic IDs for always-suspending async helpers. |
| src/coreclr/jit/importercalls.cpp | Recognizes new helper intrinsics and plumbs NamedIntrinsic into async-call setup to set AlwaysSuspends. |
| src/coreclr/jit/gentree.h | Extends AsyncCallInfo with AlwaysSuspends. |
| src/coreclr/jit/compiler.h | Updates impSetupAsyncCall signature to accept the callee NamedIntrinsic. |
| src/coreclr/jit/async.cpp | Skips null-continuation check and changes CFG wiring for AlwaysSuspends calls. |
| if (alwaysSuspends) | ||
| { | ||
| // Unconditionally branch to the suspension. The remainder is only | ||
| // reachable via resumption (or unreachable for tail awaits). | ||
| m_compiler->fgRemoveRefPred(block->GetTargetEdge()); | ||
| FlowEdge* retBBEdge = m_compiler->fgAddRefPred(suspendBB, block); | ||
| block->SetTargetEdge(retBBEdge); | ||
|
|
||
| // We normally assume awaits complete synchronously, but these always | ||
| // suspend ones do not. Thus the weight in the target actually came |
| // from the resumption path. But we have already computed weights in | ||
| // the front end under the view that all the async calls returned | ||
| // normally, and we cannot reconcile that locally. | ||
| if (m_compiler->fgPgoConsistent) | ||
| { | ||
| JITDUMP("Marking profile inconsistent due to always-suspend helper [%06u]\n", Compiler::dspTreeID(call)); | ||
| m_compiler->fgPgoConsistent = false; | ||
| } |
There was a problem hiding this comment.
We don't actually record resumption/suspension in PGO information. We add the probes at the time where there is no resumption/suspension, and then later when we add suspension/resumption everything is added so that these paths do not hit any probes, so except for the suspension/resumption blocks the counts are consistent (when we suspend at a call, we will resume at that call again).
The problem here is that we remove the non-suspending edge from a call. That edge would usually have all the count and keep things consistent. So now things end up wildly inconsistent.
For example, for
private static async Task PgoExample()
{
for (int i = 0; i < 10; i++)
{
await Task.Yield();
}
Console.WriteLine("Foo");
}The flow graph before async looks like:
So BB02 is the loop.
After async with this change the flow graph looks like:
Block counts are very inconsistent now. I am not sure it is feasible to keep it fully consistent. It seems we would need to add some count going into the resumption and then also add count to the suspension and propagate that through. Hopefully this inconsistency won't cause problems for layout.
There was a problem hiding this comment.
There may be some tricks we can play to get instrumentation to add extra probes, but we probably don't know where to place them until after importation.
You could drop back to using block probes, thought I am not sure if those go through the same kind of smoothing we do for edge probes.
There was a problem hiding this comment.
I do think we (at some point) want probes to figure out how many times a call suspends. I think that's the only information we need to fill in the rest of the weights properly. Perhaps we can just allocate one of the normal count probes schema entries for the async call's IL offset, ignore that in the frontend later and get ahold of that when we transform async calls in the backend.
|
Azure Pipelines: Successfully started running 5 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS Sadly cannot see diffs here because it requires the APIs to be marked |
Several async helpers are known to always suspend. We can skip the "returned a continuation?" check for those.
Fix #128324