[PATCH] D46829: [BreakCriticalEdges] Require DominatorTree when using the old pass manager

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

LoopInfo has a dependency on DominatorTree: when a pass requests the LoopInfo analysis, DominatorTree gets implicitly added to the pass pipeline.  This is fine so far, but doesn't take into account what happens when a pass preserves LoopInfo or DominatorTree.  When the CFG is mutated, we need to update LoopInfo, and at that point, we again need the DominatorTree.  Normally it should be available: passes which require LoopInfo usually also require DomTree, and passes which preserve LoopInfo also usually preserve DomTree.

But BranchProbabilityInfo is weird: it requires LoopInfo, but not DomTree.  In the simple case without any preserved analysis passes, this works because the pass manager implicitly adds the DomTree run anyway.  But in the more complicated case, where there's some pass in between the LoopInfo run and the BranchProbabilityInfo run, it doesn't work.  The DomTree gets computed, then dropped after LoopInfo, because no future pass in the pipeline requires it.  But we actually need it to preserve LoopInfo.

One way to solve this problem is, like you're suggesting, to make any pass which uses a DomTree to update LoopInfo explicitly require a DomTree.  This works, but adds unnecessary computation to the pass pipeline: we compute the domtree even if nothing actually needs it.

The other way to solve the issue is to always require/preserve domtree anywhere that requires/preseves LoopInfo.  This way, we always have a domtree if we have loopinfo, and we only compute the domtree in cases where we actually need it.


https://reviews.llvm.org/D46829