[PATCH] D23229: Part 4c: Coroutine Devirtualization: Devirtualize coro.resume and coro.destroy.

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Coroutines/CoroElide.cpp:66-74
@@ +65,11 @@
+
+  // Now the value type matches the type of the intrinsic. Go ahead and replace.
+  for (CoroSubFnInst *I : Users) {
+    I->replaceAllUsesWith(Value);
+    I->eraseFromParent();
+  }
+
+  // Propagate the constant to turn the indirect call into a direct call, so
+  // that CGPassManager recognizes the change as devirtualization.
+  coro::constantFoldUsers(Value);
+}
----------------
This can do a lot of unnecessary work because there can be lots of users of the Constant.

Instead, could we use `replaceAndRecursivelySimplify`?  It is quite powerful.

I imagine something like:
  for (CoroSubFnInst *I : Users)
    replaceAndRecursivelySimplify(I, Value);

================
Comment at: lib/Transforms/Coroutines/CoroElide.cpp:106-109
@@ +105,6 @@
+                     "of coroutine subfunctions");
+  auto *ResumeAddrConstant =
+      ConstantFolder().CreateExtractValue(Resumers, CoroSubFnInst::ResumeIndex);
+  auto *DestroyAddrConstant = ConstantFolder().CreateExtractValue(
+      Resumers, CoroSubFnInst::DestroyIndex);
+
----------------
Any reason why you need the constant folder here? I'd have used `ConstantExpr::getExtractValue`.

================
Comment at: lib/Transforms/Coroutines/CoroInstr.h:105
@@ +104,3 @@
+
+    Result.Resumers = cast<ConstantArray>(Initializer);
+    return Result;
----------------
It seems like the natural thing to do would be to mirror the logic for `OutlinedParts`.

If you wish to cast<> here, we need a verifier check to ensure that the `GlobalVariable`'s initializer is either a `ConstantArray` or a `ConstantStruct`.


Repository:
  rL LLVM

https://reviews.llvm.org/D23229