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

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 30 12:36:28 PST 2019


wenlei added a comment.

My understanding is devirt trigger is only a trick used with Legacy PM to run optimizations on coroutine funclets after coro-split. With NewPM's `CGSCCUpdateResult` infra, can we communicate that change and request passes directly with `ModuleToPostOrderCGSCCPassAdaptor`, without artificially introducing the indirect call and devirt?

I would imaging any form of outlining would require this communication if optimization is needed for the outlined region. How is that handled for outlining in general with New PM and can we follow that without the devirt trick?



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1393
+    LazyCallGraph::Node &N = CG.get(*Clone);
+    N.populate();
+  }
----------------
Seems `LazyCallGraph` wanted every possible (direct or indirect) references to be modeled upfront, and there's check/asserts for that. This change effectively bypasses that. Is slightly less optimal CGSCC order the only consequence of not following that rule?

With coro-split, we do alter the underlying CG, is it safe to populate CG Node without other bookkeeping like informing `CGSCCUpdateResult`? I'm not familiar with the assumptions and constraints about how CG should be updated, but asking because this `populate()` is never called outside of call graph construction, and I'm guessing it's a public function only because `LazyCallGraphTest` needed it..



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1466
+  LazyCallGraph::Node &N = CG.get(*DevirtFn);
+  N.populate();
+}
----------------
This doesn't seem necessary as DevirtFn doesn't contain any calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71899





More information about the llvm-commits mailing list