This is an archive of the discontinued LLVM Phabricator instance.

[coroutines] Add handling for unwind coro.ends
AbandonedPublic

Authored by GorNishanov on Oct 10 2016, 12:16 PM.

Details

Reviewers
majnemer
Summary

Hi David:

This is an implementation of a hybrid idea, where for WinEH I use bundles and replace the coro.end with cleanupret and for landing pad model I use explicit control flow that jumps to EHresume block that clang set up for me.

clang part: https://reviews.llvm.org/D25444

Diff Detail

Event Timeline

GorNishanov retitled this revision from to [coroutines] Add handling for unwind coro.ends.
GorNishanov updated this object.
majnemer edited edge metadata.Oct 11 2016, 12:08 AM

Can we have IR tests for this?

lib/Transforms/Coroutines/CoroSplit.cpp
165

Does FromPad have any CleanupReturnInsts at this point?

Can we have IR tests for this?

Absolutely. I am working on them and will send an official CR with llvm-commits added on creation. I just wanted to make sure that the approach is reasonable before investing more time in this direction.

I think its fine but int_coro_end should probably be marked as may_throw so we don't sink after it. We could append nounwind after we do the slicing.

GorNishanov marked an inline comment as done.Oct 12 2016, 1:35 PM
GorNishanov added inline comments.
lib/Transforms/Coroutines/CoroSplit.cpp
165

By construction, it should not have any between cleanuppad and coro.end. It might have afterwards, but, those will get cleaned up by simplify cfg.

I will update the documentation for coroutine intrinsics and will indicate that expectation is that for WinEH exception hanlding, frontend should not emit any cleanuprets between cleanuppad and coro.end.

majnemer added inline comments.Oct 12 2016, 1:39 PM
lib/Transforms/Coroutines/CoroSplit.cpp
165

I wasn't thinking of cleanuprets between the pad and the coro.end but rets on paths which never reach coro.end.

GorNishanov abandoned this revision.Oct 13 2016, 7:07 AM
GorNishanov marked an inline comment as done.

Opened official CR here https://reviews.llvm.org/D25543