JIT: Create suspensions/resumptions separately from processing calls in async transformation#125497
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR refactors CoreCLR JIT async transformation so it first collects continuation requirements (live locals, optional fields, return shapes) and then materializes suspension/resumption blocks and continuation layouts afterwards, enabling future sharing/reuse of continuation layouts across multiple suspension points.
Changes:
- Add a
constoverload forjitstd::vector::data()to support const access patterns introduced by the refactor. - Introduce
ContinuationLayoutBuilder,AsyncState, and new return-shape tracking (ReturnTypeInfo/ReturnInfo) to decouple “what’s needed” from “how it’s laid out/encoded”. - Delay creation of suspension/resumption IR until all states are recorded (
CreateResumptionsAndSuspensions), and update layout/materialization logic to use per-state “sub-layout” membership checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/coreclr/jit/jitstd/vector.h |
Adds const T* data() const needed for new const usage patterns (e.g., binary search over const vectors). |
src/coreclr/jit/async.h |
Introduces builder/state/layout refactor types (ContinuationLayoutBuilder, AsyncState, return info structs) and updates AsyncTransformation plumbing accordingly. |
src/coreclr/jit/async.cpp |
Implements the new builder-based workflow, delayed IR creation, return-slot handling, and layout finalization (including GC bitmap/type allocation). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/coreclr/jit/async.cpp:1
ContinuationLayoutBuilder::Create()declares a localClassLayout* layout;inside the locals loop, which shadows the outerContinuationLayout* layoutvariable. This is very error-prone and (depending on the exact code as compiled) can lead to passing the wronglayoutpointer type/value intobitmapBuilder.SetType(...), producing an incorrect GC bitmap (or even a compile-time type mismatch if overloads differ). Rename one of these variables (e.g.,ContinuationLayout* contLayoutfor the outer one andClassLayout* structLayoutfor the per-local layout), and ensureSetTypereceives the per-localClassLayout*for structs (andnullptrotherwise).
// Licensed to the .NET Foundation under one or more agreements.
src/coreclr/jit/async.cpp:1
FillInDataOnSuspensiongates OSR IL offset emission on the compiler OSR/patchpoint checks instead of the already-computedsubLayout.NeedsOSRILOffset()(used elsewhere for flags). UsingsubLayout.NeedsOSRILOffset()here would keep the logic consistent with the builder’s decisions and reduce the chance of future divergence as layout-sharing strategies evolve.
// Licensed to the .NET Foundation under one or more agreements.
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS Minor diffs. Only because some things are happening in different orders and that results in different local allocation order, leading to different stack frame encodings. Otherwise this shouldn't have functional changes. Looks like superpmi-diffs is broken. I can take a look tomorrow. |
|
/ba-g Deadletter and missing SPMI collection due to azdo pool issue |
This refactors the async transformation to collect all continuation information before generating the suspensions/resumptions themselves. This allows reusing continuation layouts across multiple suspension points and will allow experimenting with strategies where we only allocate one continuation, and then reuse it for all future suspensions.
Some minor diffs expected where locals are allocated in different orders, leading to different stack frame encodings.