[PATCH] D71899: [Coroutines][2/6] New pass manager: coro-split

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

Thanks for making the changes to schedule 2nd coro-split so funclets get fully optimized. A few comments inline, looks good otherwise.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1411
+  // pass would be misattributed to that unrelated function pass.
+  updateCGAndAnalysisManagerForCGSCCPass(CG, C, N, AM, UR);
+}
----------------
Now that we have CallGraphUpdater, can we use `CGUpdater.reanalyzeFunction(N.getFunction())` instead?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1575
+  if (Coroutines.empty())
+    llvm_unreachable("experimental pass manager cannot yet handle "
+                     "'llvm.coro.prepare.retcon'");
----------------
nit: I would say "new pass manager" instead of "experimental pass manager".. it's not that experimental at this moment. 


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1597
+      // worklist.
+      UR.RCWorklist.insert(&C.getOuterRefSCC());
+      F.addFnAttr(CORO_PRESPLIT_ATTR, PREPARED_FOR_SPLIT);
----------------
Is it necessary to request RefSCC to be reprocessed? I thought `UR.CWorklist.insert(&C)` should be enough..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71899