[PATCH] D139872: [llvm][CallBrPrepare] split critical edges

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added subscribers: davide, ab, pcc, lebedev.ri.
nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:128-129
+
+  auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
+  DominatorTree *DT = DTWP ? &DTWP->getDomTree() : nullptr;
+
----------------
efriedma wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > aeubanks wrote:
> > > > nickdesaulniers wrote:
> > > > > @aeubanks does this look correct?
> > > > yup, lg
> > > Oh, I guess later on (in a later commit) I will need to query `DT.dominates`.  If DOM tree info isn't available, wat do?
> > Perhaps I would manually need to construct a DomTreeUpdater or something?
> > 
> > I guess there's prior art in SafeStackLegacyPass::runOnFunction
> If you need a domtree, just explicitly request one.  With LegacyPM, something like `AU.addRequired<DominatorTreeWrapperPass>();` in CallBrPrepare::getAnalysisUsage
So I think the issue with `AU.addRequired<DominatorTreeWrapperPass>();` is that it might now introduce a `Dominator Tree Construction` into `-O0` pipelines pessimistically.

The idea being that if we scanned the IR for `callbr`s (which are highly unlikely to exist in most programs outside of the Linux kernel and tcmalloc), we could lazily compute the DOMTree only if we needed (I //think// that's why `SafeStackLegacyPass::runOnFunction` has that pattern? cc @ab @pcc @davide @lebedev.ri )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139872