[PATCH] D139872: [llvm][CallBrPrepare] split critical edges
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 20 15:13:21 PST 2022
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:
> > 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 )
> Oh, hmm, I see what you mean. Yes, that's what the code in SafeStackLegacyPass involving the `std::optional<DominatorTree>` is doing: if an existing domtree is available, it uses it, otherwise it only computes the domtree if it's needed.
So it looks like there's a tradeoff (FWICT) with that pattern:
```
+ // It's highly likely that most programs do not contain CallBrInsts. Follow a
+ // similar pattern from SafeStackLegacyPass::runOnFunction to reuse previous
+ // domtree analysis if available, otherwise compute it lazily. This avoids
+ // forcing Dominator Tree Construction at -O0 for programs that likely do not
+ // contain CallBrInsts. It does pessimize programs with callbr at higher
+ // optimization levels, as the DominatorTree created here is not reused by
+ // subsequent passes.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139872/new/
https://reviews.llvm.org/D139872
More information about the llvm-commits
mailing list