[PATCH] D23412: [Coroutines]: Part6b: Add coro.id intrinsic.

[Gor Nishanov via llvm-commits]

GorNishanov marked 2 inline comments as done.

================
Comment at: lib/Transforms/Coroutines/CoroEarly.cpp:55-58
@@ -54,2 +54,6 @@
         continue;
-      case Intrinsic::coro_begin:
+      // case Intrinsic::coro_alloc:
+      // case Intrinsic::coro_begin:
+      //   CS.setCannotDuplicate();
+      //   break;
+      case Intrinsic::coro_id:
----------------
majnemer wrote:
> Did you intend to send this part out for review?
No. I'll take it out. It is for the next patch.

================
Comment at: lib/Transforms/Coroutines/CoroElide.cpp:117
@@ +116,3 @@
+  //   coro.begin(id, mem)
+  auto *False = ConstantInt::get(Type::getInt1Ty(C), 0);
+  for (auto *CA : CoroAllocs) {
----------------
majnemer wrote:
> This could just be `auto *False = ConstantInt::getFalse(C)`
Cool! I'll switch to that.

================
Comment at: lib/Transforms/Coroutines/CoroElide.cpp:164-166
@@ -177,1 +163,5 @@
+  // Collect all coro.subfn.addrs associated with coro.begin.
+  for (CoroBeginInst *CB : CoroBegins) {
+    for (User *U : CB->users())
+      if (auto *II = dyn_cast<CoroSubFnInst>(U))
         switch (II->getIndex()) {
----------------
majnemer wrote:
> Hmm... Is it possible for a coro.subfn to use a coro.begin via something weird like a phi?
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.


https://reviews.llvm.org/D23412