[IRGen] Fix ABI for thick async functions. #36284
Conversation
|
Can we ban partial applies of non-constant functions in SIL? They should only be emitted in a few narrow cases and it would simplify IRGen to not have to deal with this. |
|
To be safe, we probably don't want to change the behavior for sync functions at the moment. Does SILGen ever emit partial applies of non-constant async functions? If not, it would make sense to add a check in the verifier that no partial apply is of a function that is non-constant and async. Until we have more async code to test on though, it may be tough to be entirely confident that no partial applies of non-constant async functions will ever appear. Creating a thunk if one does will serve as a backup against that possibility. Moving partial apply forwarder creation out of IRGen would address the root issue of IRGen complexity around partial applies, including the case of partial applies of non-constant functions. |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
2 times, most recently
from
25dd43b to
19b25dd
Compare
|
@swift-ci please test |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
19b25dd to
c7e67e4
Compare
|
@swift-ci please test |
|
Build failed |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
c7e67e4 to
f4aca56
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
f4aca56 to
62e1c8e
Compare
|
@swift-ci please test |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
62e1c8e to
566557e
Compare
|
Build failed |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
566557e to
d3548ec
Compare
|
@swift-ci please test |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
2 times, most recently
from
57c44e5 to
b29fb94
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
b29fb94 to
8b64d12
Compare
|
@swift-ci please test |
|
Build failed |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
2 times, most recently
from
968dcc0 to
c668ff0
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
|
The lldb failure should get fixed when swiftlang/llvm-project#2666 merges |
|
Build failed |
|
@swift-ci Please clean test os x |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
3470888 to
453c356
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
1 similar comment
|
@swift-ci please test windows platform |
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
2 times, most recently
from
7c17325 to
0d611c9
Compare
|
@swift-ci please test |
Up to now, there had been no need to define a LinkEntity for a partial apply forwarder. Now that async partial apply forwarders will each have their own async function pointer, an entity is needed to pass to the code that generates the async function pointers. No demangling or remangling changes are required because that code has existed for as long as partial apply forwarders to support demangling their symbols.
The new AFP entity follows the pattern set by previous AFP entities.
Previously, the partially applied functions were not defined and as such had no sizes. That resulted in downstream issues as an attempt was made to replace the size in the AsyncFunctionPointer for the functions' forwarders with the size in the AsyncFunctionPointer for functions because the latter did not exist.
Now that thunks are introduced for async dynamic functions which are partially applied, the application forwarders of those thunks will have different names.
Previously, the test verified the behavior the partial application of of dynamic async @convention(witness_method) functions which produced @callee_owned thick functions. That callee convention is no longer used, so here that test is changed to verify the behavior of producing @callee_guaranteed thick functions which are in fact still used.
Previously, thick async functions were represented sometimes as a pair of (AsyncFunctionPointer, nullptr)--when the thick function was produced via a thin_to_thick_function, e.g.--and sometimes as a pair of (FunctionPointer, ThickContext)--when the thick function was produced by a partial_apply--with the size stored in the slot of the ThickContext. That optimized for the wrong case: partial applies of dynamic async functions; in that case, there is no appropriate AsyncFunctionPointer to form when lowering the partial_apply instruction. The far more common case is to know exactly which function is being partially applied. In that case, we can form the appropriate AsyncFunctionPointer. Furthermore, the previous representation made calling a thick function more complex: it was always necessary to check whether the context was in fact null and then proceed along two different paths depending. Here, that behavior is corrected by creating a thunk in a mandatory IRGen SIL pass in the case that the function that is being partially applied is dynamic. That new thunk is then partially applied in place of the original partial_apply of the dynamic function.
nate-chandler
force-pushed
the
concurrency/irgen/rdar72105841
branch
from
0d611c9 to
ee63777
Compare
|
@swift-ci please test |
|
@swift-ci Please test Apple Silicon macOS Platform |
Previously, thick async functions were represented sometimes as a pair of (AsyncFunctionPointer, nullptr)--when the thick function was produced via a thin_to_thick_function, e.g.--and sometimes as a pair of (FunctionPointer, ThickContext)--when the thick function was produced by a partial_apply--with the size stored in the slot of the ThickContext.
That optimized for the wrong case: partial applies of dynamic async functions; in that case, there is no appropriate AsyncFunctionPointer to form when lowering the partial_apply instruction. The far more common case is to know exactly which function is being partially applied. In that case, we can form the appropriate AsyncFunctionPointer.
Furthermore, the previous representation made calling a thick function more complex: it was always necessary to check whether the context was in fact null and then proceed along two different paths depending.
Here, that behavior is corrected by creating a thunk in a mandatory IRGen SIL pass in the case that the function that is being partially applied is dynamic. That new thunk is then partially applied in place of the original partial_apply of the dynamic function.