-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Move async variant resolution into getCallInfo
#124550
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?
Changes from all commits
a2713ef
5164966
1d0de49
c85622b
f9224d4
eb135f9
29ba4aa
88be40f
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 | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1434,14 +1434,14 @@ enum CORINFO_THIS_TRANSFORM | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| enum CORINFO_CALLINFO_FLAGS | ||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_NONE = 0x0000, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_ALLOWINSTPARAM = 0x0001, // Can the compiler generate code to pass an instantiation parameters? Simple compilers should not use this flag | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_CALLVIRT = 0x0002, // Is it a virtual call? | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // UNUSED = 0x0004, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_DISALLOW_STUB = 0x0008, // Do not use a stub for this call, even if it is a virtual call. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_SECURITYCHECKS = 0x0010, // Perform security checks. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_LDFTN = 0x0020, // Resolving target of LDFTN | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // UNUSED = 0x0040, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_NONE = 0x0000, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_ALLOWINSTPARAM = 0x0001, // Can the compiler generate code to pass an instantiation parameters? Simple compilers should not use this flag | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_CALLVIRT = 0x0002, // Is it a virtual call? | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_ALLOWASYNCVARIANT = 0x0004, // allow resolution to an async variant | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_DISALLOW_STUB = 0x0008, // Do not use a stub for this call, even if it is a virtual call. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_SECURITYCHECKS = 0x0010, // Perform security checks. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CALLINFO_LDFTN = 0x0020, // Resolving target of LDFTN | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // UNUSED = 0x0040, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| enum CorInfoIsAccessAllowedResult | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1484,10 +1484,6 @@ enum CorInfoTokenKind | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // token comes from devirtualizing a method | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_TOKENKIND_DevirtualizedMethod = 0x800 | CORINFO_TOKENKIND_Method, | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // token comes from runtime async awaiting pattern | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_TOKENKIND_Await = 0x2000 | CORINFO_TOKENKIND_Method, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_TOKENKIND_AwaitVirtual = 0x4000 | CORINFO_TOKENKIND_Method, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| struct CORINFO_RESOLVED_TOKEN | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1560,6 +1556,12 @@ struct CORINFO_CALL_INFO | |||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_CONST_LOOKUP instParamLookup; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| bool wrapperDelegateInvoke; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // If CORINFO_CALLINFO_ALLOWASYNCVARIANT was passed, this is the resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // async variant or NULL if no async variant was resolved. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // This is the async variant of the token's method and differs from hMethod | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // of this class in cases of sharing, constrained resolution etc. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CORINFO_METHOD_HANDLE resolvedAsyncVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a separate The idea with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure what the EE sides end up using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose the primary one is that we call runtime/src/coreclr/jit/importercalls.cpp Lines 805 to 810 in 4eebedb
This works now because the JIT updates the token's Perhaps a cleaner way would be if runtime/src/coreclr/vm/jitinterface.cpp Lines 5252 to 5269 in 4eebedb
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about we introduce That way we'll keep clearly marking the token as "async variant token". It is weird that the EE side would change tokenType, but also JIT side changing hMethod is not exactly clean.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would work but it will be a new contract for It's probably a bit better but I am leaning towards making it so that we do not need the token anymore for non-trivial things after |
||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| enum CORINFO_DEVIRTUALIZATION_DETAIL | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6917,11 +6917,6 @@ void Compiler::impSetupAsyncCall(GenTreeCall* call, OPCODE opcode, unsigned pref | |
| JITDUMP(" Continuation continues on thread pool\n"); | ||
| } | ||
| } | ||
| else if (opcode == CEE_CALLI) | ||
| { | ||
| // Used for unboxing/instantiating stubs | ||
| JITDUMP("Call is an async calli\n"); | ||
| } | ||
|
Comment on lines
-6920
to
-6924
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated cleanup, this is no longer needed. |
||
| else | ||
| { | ||
| JITDUMP("Call is an async non-task await\n"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1763,7 +1763,7 @@ private static object ResolveTokenInScope(MethodILScope methodIL, object typeOrM | |||||
| return result; | ||||||
| } | ||||||
|
|
||||||
| private object GetRuntimeDeterminedObjectForToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken) | ||||||
| private object GetRuntimeDeterminedObjectForToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken, bool isAsyncVariant = false) | ||||||
| { | ||||||
| // Since RyuJIT operates on canonical types (as opposed to runtime determined ones), but the | ||||||
| // dependency analysis operates on runtime determined ones, we convert the resolved token | ||||||
|
|
@@ -1777,8 +1777,7 @@ private object GetRuntimeDeterminedObjectForToken(ref CORINFO_RESOLVED_TOKEN pRe | |||||
| object result = GetRuntimeDeterminedObjectForToken(methodIL, typeOrMethodContext, pResolvedToken.token); | ||||||
| if (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Newarr) | ||||||
| result = ((TypeDesc)result).MakeArrayType(); | ||||||
|
|
||||||
| if (pResolvedToken.tokenType is CorInfoTokenKind.CORINFO_TOKENKIND_Await or CorInfoTokenKind.CORINFO_TOKENKIND_AwaitVirtual) | ||||||
| if (isAsyncVariant && !((MethodDesc)result).IsAsyncVariant()) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this part necessary?
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is needed since the token may represent async variants directly for internal compiler-generated IL (like the thunks), so the result may already be the async variant for those cases. |
||||||
| result = _compilation.TypeSystemContext.GetAsyncVariantMethod((MethodDesc)result); | ||||||
|
|
||||||
| return result; | ||||||
|
|
@@ -1868,45 +1867,8 @@ private void resolveToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken) | |||||
| _compilation.NodeFactory.MetadataManager.GetDependenciesDueToAccess(ref _additionalDependencies, _compilation.NodeFactory, (MethodIL)methodIL, method); | ||||||
| #endif | ||||||
|
|
||||||
| if (pResolvedToken.tokenType is CorInfoTokenKind.CORINFO_TOKENKIND_Await or CorInfoTokenKind.CORINFO_TOKENKIND_AwaitVirtual) | ||||||
| { | ||||||
| // in rare cases a method that returns Task is not actually TaskReturning (i.e. returns T). | ||||||
| // we cannot resolve to an Async variant in such case. | ||||||
| // return NULL, so that caller would re-resolve as a regular method call | ||||||
| bool allowAsyncVariant = method.GetTypicalMethodDefinition().Signature.ReturnsTaskOrValueTask(); | ||||||
|
|
||||||
| // Don't get async variant of Delegate.Invoke method; the pointed to method is not an async variant either. | ||||||
| allowAsyncVariant = allowAsyncVariant && !method.OwningType.IsDelegate; | ||||||
|
|
||||||
| #if !READYTORUN | ||||||
| if (allowAsyncVariant) | ||||||
| { | ||||||
| bool isDirect = pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Await || method.IsCallEffectivelyDirect(); | ||||||
| if (isDirect && !method.IsAsync) | ||||||
| { | ||||||
| // Async variant would be a thunk. Do not resolve direct calls | ||||||
| // to async thunks. That just creates and JITs unnecessary | ||||||
| // thunks, and the thunks are harder for the JIT to optimize. | ||||||
| allowAsyncVariant = false; | ||||||
| } | ||||||
| } | ||||||
| #endif | ||||||
|
|
||||||
| method = allowAsyncVariant | ||||||
| ? _compilation.TypeSystemContext.GetAsyncVariantMethod(method) | ||||||
| : null; | ||||||
| } | ||||||
|
|
||||||
| if (method != null) | ||||||
| { | ||||||
| pResolvedToken.hMethod = ObjectToHandle(method); | ||||||
| pResolvedToken.hClass = ObjectToHandle(method.OwningType); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| pResolvedToken.hMethod = null; | ||||||
| pResolvedToken.hClass = null; | ||||||
| } | ||||||
| pResolvedToken.hMethod = ObjectToHandle(method); | ||||||
| pResolvedToken.hClass = ObjectToHandle(method.OwningType); | ||||||
| } | ||||||
| else | ||||||
| if (result is FieldDesc) | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.