[PATCH] D23586: [Coroutines] Part 8: Coroutine Frame Building algorithm

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:116
@@ +115,3 @@
+
+    // We rewritten PHINodes, so that only the ones with exactly one incoming
+    // value need to be analyzed.
----------------
Perhaps "We've rewritten" or "We rewrote".

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:440-443
@@ +439,6 @@
+
+    // Replace all uses of CurrentValue in the current instruction with reload.
+    for (Use &U : E.user()->operands())
+      if (U.get() == CurrentValue)
+        U.set(CurrentReload);
+  }
----------------
I think this case be `E.user()->replaceUsesOfWith(CurrentValue, CurrentReload);`

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:457-459
@@ +456,5 @@
+        Builder.CreateConstInBoundsGEP2_32(FrameTy, FramePtr, 0, P.second);
+    G->takeName(P.first);
+    P.first->replaceAllUsesWith(G);
+    P.first->eraseFromParent();
+  }
----------------
Could this be `ReplaceInstWithInst(P.first, G);`?

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:563-565
@@ +562,5 @@
+    // CurrentMaterialization for the block.
+    for (Use &U : E.user()->operands())
+      if (U.get() == CurrentDef)
+        U.set(CurrentMaterialization);
+  }
----------------
I think `replaceUsesOfWith` could be used here.


Repository:
  rL LLVM

https://reviews.llvm.org/D23586