[PATCH] D23234: [Coroutines] Part 5: Add CGSCC restart trigger

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:24
@@ +23,3 @@
+// split.
+static void makeReadyForSplit(Function &F, CallGraph &CG) {
+  Module &M = *F.getParent();
----------------
Would you consider `prepare` instead of `makeReady`?

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:32-35
@@ +31,6 @@
+  // Find a spot in the entry block to insert a call to devirt trigger function.
+  BasicBlock &EntryBlock = F.getEntryBlock();
+  BasicBlock::iterator It = EntryBlock.begin();
+  while (isa<AllocaInst>(*It) || isa<DbgInfoIntrinsic>(*It))
+    ++It;
+
----------------
Any reason why you don't just use `EntryBlock.getFirstInsertionPt()`?

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:68
@@ +67,3 @@
+  DevirtFn->addFnAttr(Attribute::AlwaysInline);
+  auto Entry = BasicBlock::Create(C, "entry", DevirtFn);
+  ReturnInst::Create(C, Entry);
----------------
`auto *`?

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:71
@@ +70,3 @@
+
+  auto Node = CG.getOrInsertFunction(DevirtFn);
+
----------------
Ditto.

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:75
@@ +74,3 @@
+  Nodes.push_back(Node);
+  SCC.initialize(&*Nodes.begin(), &*Nodes.end());
+}
----------------
I've updated this API to be a little more reasonable, just pass in `Nodes`.

================
Comment at: lib/Transforms/Coroutines/CoroSplit.cpp:104
@@ +103,3 @@
+    for (CallGraphNode *CGN : SCC)
+      if (auto F = CGN->getFunction())
+        if (F->hasFnAttribute(CORO_ATTR_STR))
----------------
Can this be `auto *`?


https://reviews.llvm.org/D23234