[PATCH] D55160: [coroutines] Improve suspend point simplification

[Brian Gesiak via Phabricator via llvm-commits]

modocache requested changes to this revision.
modocache added a comment.
This revision now requires changes to proceed.

Sorry for the wait! Mostly nits, but also I think the `while` condition might be inverted? It seems like, as is, it would never execute the body of the loop...?



================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:544
+// the coroutine and if that is the case we cannot eliminate the suspend point.
+static bool hasCallsInBlockBetween(Instruction* From, Instruction *To) {
+  for (Instruction *I = From; I != To; I = I->getNextNode()) {
----------------
nit: Run clang-format on this; I believe it should be `Instruction *From`, but in any case `From` and `To` should be consistent.


================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:566
+  // will have to eventually hit SaveBB when going backwards from ResDesBB.
+  while (Worklist.empty()) {
+    auto *BB = Worklist.pop_back_val();
----------------
Is this condition inverted because of a typo? I'd expect this to be `while (!Worklist.empty()) { ... }`.


================
Comment at: test/Transforms/Coroutines/no-suspend.ll:62
   %save = call token @llvm.coro.save(i8* %hdl)
+  ; memcpy intrinsics should be not prevent simlification.
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 1, i1 false)
----------------
nit: Just some typos, should be: "should not prevent simplification" -- note the extra "be" and the missing "p".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55160/new/

https://reviews.llvm.org/D55160