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

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: docs/Coroutines.rst:117-121
@@ -116,8 +116,7 @@
 
 The `entry` block establishes the coroutine frame. The `coro.size`_ intrinsic is
 lowered to a constant representing the size required for the coroutine frame. 
-The `coro.begin`_ intrinsic initializes the coroutine frame and returns the a
-token that is used to obtain the coroutine handle via `coro.frame` intrinsic.
-The first parameter of `coro.begin` is given a block of memory to be used if the
-coroutine frame needs to be allocated dynamically.
+The `coro.begin`_ intrinsic initializes the coroutine frame and returns the 
+coroutine handle. The second parameter of `coro.begin` is given a block of memory 
+to be used if the coroutine frame needs to be allocated dynamically.
 
----------------
We should probably say something about `coro.id` here.

================
Comment at: docs/Coroutines.rst:426
@@ -429,1 +425,3 @@
+    %phi = phi i8* [ null, %entry ], [ %alloc, %dyn.alloc ]
+    %hdl = call noalias i8* @llvm.coro.begin(i8* %phi)
     br label %loop
----------------
Missing the token.

================
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:
----------------
Did you intend to send this part out for review?

================
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) {
----------------
This could just be `auto *False = ConstantInt::getFalse(C)`

================
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()) {
----------------
Hmm... Is it possible for a coro.subfn to use a coro.begin via something weird like a phi?


https://reviews.llvm.org/D23412