partial_apply support for coroutines
#71653
Conversation
asavonic
requested review from
kavon,
mikeash,
al45tair,
hborla,
slavapestov and
xedin
as code owners
asavonic
force-pushed
the
yield-diff-pa
branch
2 times, most recently
from
66c56f6 to
8f46ff7
Compare
|
@swift-ci please test |
|
Please test with swiftlang/llvm-project#8199 |
asavonic
force-pushed
the
yield-diff-pa
branch
from
8f46ff7 to
9e805a2
Compare
|
@swift-ci please test |
|
Please test with swiftlang/llvm-project#8199 |
|
@swift-ci please test |
|
A few things have changed since Feb:
Here is a WIP patchset that partially addresses (1) and (2), and triggers (3): |
|
@swift-ci please test |
|
@swift-ci please test linux |
|
@swift-ci please test linux |
|
@swift-ci please test windows |
|
@swift-ci please test macos |
|
@swift-ci please test |
|
@aschwaighofer @rjmccall Will you please take a look into this patch? It was reworked and now I believe much cleaner. In addition, all tests from So, to conclude: there was no support for Thanks! |
There was a problem hiding this comment.
Can you add a paragraph about partial-apply co-routines into docs/SIL.rst (under the section for partial_apply)? Like you wrote in the description of this PR. Please also add that this is (currently) only used for auto-diff.
Do escaping and non-escaping ([on_stack]) partial_applys support co-routines? If not, please document this as well (and add a check in the SIL verifier).
@eeckstein This is one of testcases (see https://github.com/swiftlang/swift/pull/71653/files#diff-2eac67a6f96297f40450a009472fdbb4cdb09445c4eb3f9a8b69550e19f2b9b3R600 and below) sil @partial_apply_class_on_stack : $@convention(thin) (@owned SwiftClass) -> () {
entry(%a : $SwiftClass):
%f = function_ref @partially_applyable_to_class : $@yield_once @convention(thin) (@guaranteed SwiftClass) -> (@yields SwiftClass)
%c = partial_apply [callee_guaranteed] [on_stack] %f(%a) : $@yield_once @convention(thin) (@guaranteed SwiftClass) -> (@yields SwiftClass)
%use = function_ref @use_closure : $@convention(thin) (@noescape @yield_once @callee_guaranteed () -> (@yields SwiftClass)) -> ()
apply %use(%c) : $@convention(thin) (@noescape @yield_once @callee_guaranteed () -> (@yields SwiftClass)) -> ()
dealloc_stack %c : $@noescape @yield_once @callee_guaranteed () -> (@yields SwiftClass)
strong_release %a : $SwiftClass
%t = tuple()
return %t : $()
}So it seems to be there |
|
@swift-ci please test |
|
@eeckstein I added the note to docs in 962777f Please let me know if this is enough or something else should be added |
|
preset=buildbot,tools=RA,stdlib=RD,test=non_executable |
Perfect, thanks! |
|
@swift-ci please test |
|
preset=buildbot,tools=RA,stdlib=RD,test=non_executable |
The patch adds lowering of partial_apply instructions for coroutines. This pattern seems to trigger a lot of type mismatch errors in IRGen, because coroutine functions are not substituted in the same way as regular functions (see the patch 07f03bd "Use pattern substitutions to consistently abstract yields" for more details). The odd type conversions in the patch are related to this issue, and these should be checked carefully. Perhaps it is better to enable substitutions for coroutine functions instead (at least for some cases). Other than that, lowering of partial_apply for coroutines is straightforward: we generate another coroutine that captures arguments passed to the partial_apply instructions. It calls the original coroutine for yields (first return) and yields the resulting values. Then it calls the original function's continuation for return or unwind, and forwards them to the caller as well. After IRGen, LLVM's Coroutine pass transforms the generated coroutine (along with all other coroutines) and eliminates llvm.coro.* intrinsics. LIT tests check LLVM IR after this transformation.
A placeholder goes instead of the context in case of error results, so that thin functions have the same number of arguments as thick ones. Coroutine lowering adds it as part of getCallEmission (via setFromCallee), so we don't need to do it in emitPartialApplicationForwarder.
Fix tests on arm64e
Co-authored-by: Arnold Schwaighofer <[email protected]>
|
@swift-ci please test |
|
preset=buildbot,tools=RA,stdlib=RD,test=non_executable |
|
Buildbot configiration fails due |
The patch adds lowering of partial_apply instructions for coroutines.
This pattern seems to trigger a lot of type mismatch errors in IRGen, because coroutine functions are not substituted in the same way as regular functions (see the patch 07f03bd "Use pattern substitutions to consistently abstract yields" for more details). The odd type conversions in the patch are related to this issue, and these should be checked carefully. Perhaps it is better to enable substitutions for coroutine functions instead (at least for some cases).
Other than that, lowering of partial_apply for coroutines is straightforward: we generate another coroutine that captures arguments passed to the partial_apply instructions. It calls the original coroutine for yields (first return) and yields the resulting values. Then it calls the original function's continuation for return or unwind, and forwards them to the caller as well.
After IRGen, LLVM's Coroutine pass transforms the generated coroutine (along with all other coroutines) and eliminates llvm.coro.* intrinsics. LIT tests check LLVM IR after this transformation.
Test cases are taken from partial_apply.sil test, and adapted to use coroutines.