This is an archive of the discontinued LLVM Phabricator instance.

[Coroutines] Part13: Handle single edge PHINodes across suspends
ClosedPublic

Authored by GorNishanov on Sep 5 2016, 6:19 PM.

Details

Summary

If one of the uses of the value is a single edge PHINode, handle it.

Original:

%val = something
<suspend>   
%p = PHINode [%val]

After Spill + Part13:

%val = something
%slot = gep val.spill.slot
store %val, %slot
<suspend>
%p = load %slot

Plus tiny fixes/changes:

  • use correct index for coro.free in CoroCleanup
  • fixup id parameter in coro.free to allow authoring coroutine in plain C with __builtins

Diff Detail

Event Timeline

GorNishanov updated this revision to Diff 70351.Sep 5 2016, 6:19 PM
GorNishanov retitled this revision from to [Coroutines] Part13: Handle single edge PHINodes across suspends.
GorNishanov updated this object.
GorNishanov added a reviewer: majnemer.
GorNishanov added a subscriber: llvm-commits.
majnemer added inline comments.Sep 5 2016, 8:45 PM
lib/Transforms/Coroutines/CoroEarly.cpp
169–174

Can't the FE do this?

GorNishanov added inline comments.Sep 5 2016, 9:08 PM
lib/Transforms/Coroutines/CoroEarly.cpp
169–174

Coroutine await C++ FE, sure.

This particular helper allows construct coroutines in plain C using __builtins mapping to llvm.coro.* intrinsics.
coro.id creates trouble, since it returns tokens and those do not have a good mapping in the builtin type system.

The workaround is to allow __builtin_coro_free(mem) output llvm.coro.free(token none, %mem) and then fixup in CoroEarly to point to an appropriate coro.id.

GorNishanov updated this revision to Diff 70356.Sep 5 2016, 9:29 PM

Take out coro.free fixup code. We can do it in the frontend. Alphabetize the list of interesting coro intrinsics in coro.early

GorNishanov marked 2 inline comments as done.Sep 5 2016, 9:29 PM
GorNishanov added inline comments.Sep 6 2016, 8:03 AM
lib/Transforms/Coroutines/CoroEarly.cpp
175–180

After I took out the fixup code, I am having second thoughts on it and want to put it back.

CoroEarly purpose is to cleanup after frontend. The fixup for coro.frees fits nicely there. Having nearly identical code that looks like LLVM pass at the end of CodeGenFunction::FinishFunction seems awkward. Having it in CoroEarly puts all of the cleanup logic in one place.

majnemer added inline comments.Sep 8 2016, 3:23 PM
lib/Transforms/Coroutines/CoroEarly.cpp
175–180

Ok, let's bring it back.

GorNishanov updated this revision to Diff 70767.Sep 8 2016, 5:04 PM

Put coro.free fixup code back

GorNishanov marked 2 inline comments as done.Sep 8 2016, 5:05 PM
majnemer accepted this revision.Sep 8 2016, 9:54 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.Sep 8 2016, 9:54 PM
This revision was automatically updated to reflect the committed changes.