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

[Gor Nishanov via llvm-commits]

GorNishanov marked 7 inline comments as done.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:33
@@ +32,3 @@
+// The "coro-suspend-crossing" flag is very noisy. There is another debug type,
+// "coro-frame", which has results in leaner debug spew.
+#define DEBUG_TYPE "coro-suspend-crossing"
----------------
remove 'has'

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:402-403
@@ +401,4 @@
+  for (auto const &E : Spills) {
+    // If we have not seen the value, generate a spill.
+    if (CurrentValue != E.def()) {
+      CurrentValue = E.def();
----------------
majnemer wrote:
> Maybe:
>   if (CurrentValue == E.def())
>     continue;
> 
> This will let you reduce the indentation.
I cannot do that. We need to finish processing the reloads for the CurrentValue. Spills datastructure work like multiset:
<def, use1>
<def, use2>
<def, use3>
etc.
Thus, if (CurrentValue == E.def()), I don't have to generate a spill (i.e. store),
But I do have to load the value for every use.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:483-484
@@ +482,4 @@
+
+  SmallVector<BasicBlock *, 8> Preds(pred_begin(&BB), pred_end(&BB));
+  for (BasicBlock *Pred : Preds) {
+    auto *IncomingBB = SplitEdge(Pred, &BB);
----------------
majnemer wrote:
> What prevents you from visiting the same pred twice?
Hmm... Need to think about it a little bit more.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:487-488
@@ +486,4 @@
+    IncomingBB->setName(BB.getName() + Twine(".from.") + Pred->getName());
+    auto *PN = cast<PHINode>(&BB.front());
+    do {
+      int Index = PN->getBasicBlockIndex(IncomingBB);
----------------
majnemer wrote:
> I think you could structure this like a normal for loop:
>   for (auto *PN = cast<PHINode>(&BB.front()); PN != nullptr; PN = dyn_cast<PHINode>(PN->getNextNode()))
PN cannot be null on the first entry. So, used do-while instead. Saves one compare :)


https://reviews.llvm.org/D23586