-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Emit IL for AsyncResumptionStub #121456
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?
Emit IL for AsyncResumptionStub #121456
Changes from 1 commit
2abb220
6449d07
a9aead4
c18bc5d
85ff8c7
3da6325
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -15,11 +15,11 @@ public partial class AsyncResumptionStub : ILStubMethod | |||
| { | ||||
| private readonly MethodDesc _owningMethod; | ||||
| private MethodSignature _signature; | ||||
| private MethodIL _methodIL; | ||||
|
|
||||
| public AsyncResumptionStub(MethodDesc owningMethod) | ||||
| { | ||||
| Debug.Assert(owningMethod.IsAsyncVariant() | ||||
| || (owningMethod.IsAsync && !owningMethod.Signature.ReturnsTaskOrValueTask())); | ||||
| Debug.Assert(owningMethod.IsAsyncCall()); | ||||
| _owningMethod = owningMethod; | ||||
| } | ||||
|
|
||||
|
|
@@ -32,22 +32,119 @@ public AsyncResumptionStub(MethodDesc owningMethod) | |||
|
|
||||
| public override TypeSystemContext Context => _owningMethod.Context; | ||||
|
|
||||
| protected override int ClassCode => unchecked((int)0xa91ac565); | ||||
|
|
||||
|
||||
| protected override int ClassCode => unchecked((int)0xa91ac565); |
Outdated
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.
I do not think this comment useful. You can easily tell what the IL is from the actual emitter below. It can only get out of sync overtime.
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.
On the JIT side here, between this and other arguments, we also put:
- generic context arg, if present.
It is set to 0, and will not be used by the calee. Continuation has captured the real value and that will be used instead. But we need to match what the calee expects, so we need to push 0 for the context. - continuation.
it is our arg0. Per the ABI of Async* calls it goes right before the other arguments.
(unless this is x86)
On x86, as usual, there is a difference and the hidden args go after formal ones and in reverse order.
--
*Note - the resume stub is an ordinary function, but what we are resuming is an Async method, thus we shuffle the continuation from our arg0 to its predefined/hidden position in the calee signature.
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.
Not sure. Maybe this is correct in NAOT, but ldftn is a bit suspicious.
If this is a metadata token for the method that we are resuming, you may end up calling the Task-returning variant.
In the JIT counterpart we load the actual address of the JITed target method.
As in:
pCode->EmitLDC((DWORD_PTR)m_finalCodeAddressSlot);
pCode->EmitLDIND_I();cc: @MichalStrehovsky - will ldftn do the right thing here?
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 should be emitted as a regular call and the JIT/EE interface and/or the JIT itself should make sure that the call does the right thing.
(Once we figure out what to do here, we may want to switch the JIT counterpart to the same plan.)
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.
Yes, this should be a regular call. The call will be to a thing with RuntimeAsync calling convention; we can make tokens for that and we will make such token here. We probably don't need to pass generic context explicitly either.
But the question is what this will do in the JIT. My expectation is that it will take similar paths as a AsyncExplicitImpl method call.
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.
But the question is what this will do in the JIT. My expectation is that it will take similar paths as a AsyncExplicitImpl method call.
Runtime async method calls pass a null continuation in the JIT. This needs to pass an actual continuation.
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.
"Resume" in a way is the opposite of "Call". In Call we pass formal arguments, generic context, etc.., but continuation is null. In Resume only continuation is what is not default/null.
Doing CALLI with a special-crafted signature is the technique to do a low level "unsafe cast" through call conventions in IL. I have seen it in other places (like instantiating stubs?).
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.
My expectation is that it will take similar paths as a AsyncExplicitImpl method call.
Note - what we are resuming may actually be AsyncExplicitImpl.
Anything with Async call conv may need resuming. Whether it has a variant with non-async call convention is unimportant here. Just the part is that it is Async (i.e. emitted as a state machine, has async call conv).
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.
I have seen it in other places (like instantiating stubs?).
I do not think that this trick is used anywhere in AOT compilers (just looked through all Emit(ILOpcode.calli - none of them look like that).
AsyncCallContinuation() intrinsic gets the async continuation from the last call. Can we have a counterpart that sets the async continuation for the next call?
We can try going the ldftn route, but I expect that it will hit problems with generics and the generated code won't be the best at the end if we manage to make it work. Managed function pointers in NAOT are tagged pointers. Regular calli of a managed function pointer compiles into if (ptr & tag_bit) { ptr[0](ptr[1] /* instantiating arg */, regular args); } else { ptr(regular args) }. We would probably need to suppress this for this ldftn + calli pair.
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.
Could CALLI trick work the same as in JIT case if we clear the tag after LDFTN?
(This all assumes that we can know if the calee/resumee has a generic context parameter and can push 0 to its position)
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.
As another wild thought - I think this IL is never exposed outside of internals of VM/JIT/AOT. Perhaps we could use an IL prefix to tag a call as a resuming call?
Ex:
// load continuation
ldarg 0
// the prefix means "consume one arg as a continuation argument, pass everything else as default"
// could be applied to calli as well
// callee must have async callconv
resume.
call <token> 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.
Could CALLI trick work the same as in JIT case if we clear the tag after LDFTN?
We can certainly try to make LDFTN + CALLI work and special-case it in enough places to make it do what we want. My gut feeling is that it won't be pretty.
Perhaps we could use an IL prefix to tag a call as a resuming call?
I think it is equivalent to AsyncCallContinuation setter intrinsic, just with a different encoding. We have prior art to use calls to encode internal custom modifiers of call instructions (e.g. NextCallReturnAddress or AsyncCallContinuation), so I would continue to do so here as well.
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.
is this.Signature the signature of the stub or the method that we are resuming?
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.Signature is the stub signature. This should be changed to _owningMethod.Signature.
Outdated
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.
Can we move this to AsyncHelpers so that we do not have to add StubHelpers.cs to NativeAOT?
StubHelpers in CoreCLR is everything-in-the-kitchen-sink type. It is not pretty.
jtschuster marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Nov 7, 2025
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.
The logic for returnsVoid is inverted. It should be == instead of !=. The variable is true when the return type is NOT void, which means the variable name and logic are contradictory. This will cause incorrect IL generation - the result handling code will execute when the method returns void and will be skipped when it doesn't return void.
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.
We do not store IL on the ILStubMethods. There is a cache in the ILProvider.