Page MenuHomePhabricator

[Coroutines]: Part6b: Add intrinsic.

Authored by GorNishanov on Aug 11 2016, 10:54 AM.


  1. Make coroutine representation more robust against optimization that may duplicate instruction by introducing intrinsics that returns a token that will get fed into coro.alloc and coro.begin. Due to returning a token, it won't get duplicated and can be used as reliable indicator of coroutine identify when a particular coroutine call gets inlined.
  2. Move last three arguments of coro.begin into as they will be shared if coro.begin will get duplicated.
  3. doc + test + code updated to support the new intrinsic.

Diff Detail


Event Timeline

GorNishanov retitled this revision from to Coroutine: part6 preview.
GorNishanov updated this object.
GorNishanov added reviewers: mehdi_amini, majnemer.
majnemer accepted this revision.Aug 11 2016, 12:00 PM
majnemer edited edge metadata.

LGTM with nits addressed

937 ↗(On Diff #67709)


This revision is now accepted and ready to land.Aug 11 2016, 12:00 PM
majnemer edited edge metadata.Aug 11 2016, 12:32 PM
majnemer added a subscriber: llvm-commits.
GorNishanov retitled this revision from Coroutine: part6 preview to [Coroutines]: Part6b: Add intrinsic..
GorNishanov updated this object.
GorNishanov requested a review of this revision.Aug 11 2016, 9:27 PM
GorNishanov edited edge metadata.

A lot of churn, needs a review.

GorNishanov edited edge metadata.


majnemer added inline comments.Aug 11 2016, 9:49 PM
117–121 ↗(On Diff #67796)

We should probably say something about here.

426 ↗(On Diff #67796)

Missing the token.

55–58 ↗(On Diff #67796)

Did you intend to send this part out for review?

117 ↗(On Diff #67796)

This could just be auto *False = ConstantInt::getFalse(C)

164–166 ↗(On Diff #67796)

Hmm... Is it possible for a coro.subfn to use a coro.begin via something weird like a phi?

GorNishanov marked 2 inline comments as done.Aug 11 2016, 9:52 PM
GorNishanov added inline comments.
55–58 ↗(On Diff #67796)

No. I'll take it out. It is for the next patch.

117 ↗(On Diff #67796)

Cool! I'll switch to that.

164–166 ↗(On Diff #67796)

Sure, but, in that case, it would mean that the handle to coroutine escaped, moved out, was zero out and thus is not eligible for devirtualization.

GorNishanov marked 2 inline comments as done.

Feedback addressed. Preparing for landing.

  1. Fixes in Coroutines.rst
  2. use ConstantInt::getFalse
  3. remove commented out code
  4. add clarification comment about coro.subfn.addr
GorNishanov marked 6 inline comments as done.Aug 11 2016, 10:19 PM
This revision was automatically updated to reflect the committed changes.