Summary:
Currently, our post-dom tree tries to ignore and remove the effects of
infinite loops. It fails miserably at this, because it tries to do it
ahead of time, and thus can only detect self-loops, and any other type
of infinite loop, it pretends doesn't exist at all.
This can, in a bunch of cases, lead to wrong answers and a completely
empty post-dom tree.
Wrong answer:
```
declare void foo()
define internal void @f() {
entry:
br i1 undef, label %bb35, label %bb3.i
bb3.i:
call void @foo()
br label %bb3.i
bb35.loopexit3:
br label %bb35
bb35:
ret void
}
```
We get:
```
Inorder PostDominator Tree:
[1] <<exit node>> {0,7}
[2] %bb35 {1,6}
[3] %bb35.loopexit3 {2,3}
[3] %entry {4,5}
```
This is a trivial modification of the testcase for PR 6047
Note that we pretend bb3.i doesn't exist.
We also pretend that bb35 post-dominates entry.
While it's true that it does not exit in a theoretical sense, it's not
really helpful to try to ignore the effect and pretend that bb35
post-dominates entry. Worse, we pretend the infinite loop does
nothing (it's usually considered a side-effect), and doesn't even
exist, even when it calls a function. Sadly, this makes it impossible
to use when you are trying to move code safely. All compilers also
create virtual or real single exit nodes (including us), and connect
infinite loops there (which this patch does). In fact, others have
worked around our behavior here, to the point of building their own
post-dom trees:
https://zneak.github.io/fcd/2016/02/17/structuring.html and pointing
out the region infrastructure is near-useless for them with postdom in
this state :(
Completely empty post-dom tree:
```
define void @spam() #0 {
bb:
br label %bb1
bb1: ; preds = %bb1, %bb
br label %bb1
bb2: ; No predecessors!
ret void
}
```
Printing analysis 'Post-Dominator Tree Construction' for function 'foo':
=============================--------------------------------
Inorder PostDominator Tree:
[1] <<exit node>> {0,1}
:(
(note that even if you ignore the effects of infinite loops, bb2
should be present as an exit node that post-dominates nothing).
This patch changes post-dom to properly handle infinite loops and does
root finding during calculation to prevent empty tress in such cases.
We match gcc's (and the canonical theoretical) behavior for infinite
loops (find the backedge, connect it to the exit block).
Testcases coming as soon as i finish running this on a ton of random graphs :)
Reviewers: chandlerc, davide
Subscribers: bryant, llvm-commits
Differential Revision: https://reviews.llvm.org/D29705
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@296535
91177308-0d34-0410-b5e6-
96231b3b80d8
/// recalculate - compute a dominator tree for the given function
template <class FT> void recalculate(FT &F) {
- typedef GraphTraits<FT *> TraitsTy;
reset();
this->Vertex.push_back(nullptr);
if (!this->IsPostDominators) {
- // Initialize root
- NodeT *entry = TraitsTy::getEntryNode(&F);
- addRoot(entry);
-
Calculate<FT, NodeT *>(*this, F);
} else {
- // Initialize the roots list
- for (auto *Node : nodes(&F))
- if (TraitsTy::child_begin(Node) == TraitsTy::child_end(Node))
- addRoot(Node);
-
Calculate<FT, Inverse<NodeT *>>(*this, F);
}
}
#define LLVM_SUPPORT_GENERICDOMTREECONSTRUCTION_H
#include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Support/GenericDomTree.h"
df_iterator_dom_storage(BaseSet &Storage) : Storage(Storage) {}
typedef typename BaseSet::iterator iterator;
- std::pair<iterator, bool> insert(NodeRef N) {
- return Storage.insert({N, InfoType()});
+ std::pair<iterator, bool> insert(NodeRef To) {
+ auto Result = Storage.insert({To, InfoType()});
+
+ return Result;
}
void completed(NodeRef) {}
typename GraphT::NodeRef,
typename DominatorTreeBaseByGraphTraits<GraphT>::InfoRec>
DFStorage(DT.Info);
- bool IsChildOfArtificialExit = (N != 0);
for (auto I = idf_ext_begin(V, DFStorage), E = idf_ext_end(V, DFStorage);
I != E; ++I) {
typename GraphT::NodeRef BB = *I;
if (I.getPathLength() > 1)
BBInfo.Parent = DT.Info[I.getPath(I.getPathLength() - 2)].DFSNum;
DT.Vertex.push_back(BB); // Vertex[n] = V;
-
- if (IsChildOfArtificialExit)
- BBInfo.Parent = 1;
-
- IsChildOfArtificialExit = false;
}
return N;
}
void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<NodeT>> &DT,
FuncT &F) {
typedef GraphTraits<NodeT> GraphT;
+ typedef GraphTraits<FuncT *> FuncGraphT;
static_assert(std::is_pointer<typename GraphT::NodeRef>::value,
"NodeRef should be pointer type");
typedef typename std::remove_pointer<typename GraphT::NodeRef>::type NodeType;
unsigned N = 0;
- bool MultipleRoots = (DT.Roots.size() > 1);
- if (MultipleRoots) {
+ bool NeedFakeRoot = DT.isPostDominator();
+ // If this is post dominators, push a fake node to start
+ if (NeedFakeRoot) {
auto &BBInfo = DT.Info[nullptr];
BBInfo.DFSNum = BBInfo.Semi = ++N;
BBInfo.Label = nullptr;
-
- DT.Vertex.push_back(nullptr); // Vertex[n] = V;
+ DT.Vertex.push_back(nullptr); // Vertex[n] = V;
+ } else {
+ // The root is the entry block of the CFG
+ DT.addRoot(FuncGraphT::getEntryNode(&F));
}
// Step #1: Number blocks in depth-first order and initialize variables used
// in later stages of the algorithm.
- if (DT.isPostDominator()){
- for (unsigned i = 0, e = static_cast<unsigned>(DT.Roots.size());
- i != e; ++i)
- N = ReverseDFSPass<GraphT>(DT, DT.Roots[i], N);
- } else {
- N = DFSPass<GraphT>(DT, DT.Roots[0], N);
+ if (DT.isPostDominator()) {
+ unsigned Total = 0;
+ for (auto I : nodes(&F)) {
+ ++Total;
+ // If it has no *successors*, it is definitely a root.
+ if (FuncGraphT::child_begin(I) == FuncGraphT::child_end(I)) {
+ N = ReverseDFSPass<GraphT>(DT, I, N);
+ DT.Info[I].Parent = 1;
+ DT.addRoot(I);
+ }
+ }
+ // Accounting for the virtual exit, see if we had any unreachable nodes
+ if (Total + 1 != N ) {
+ // Make another DFS pass over all other nodes to find the unreachable
+ // blocks, and find the furthest paths we'll be able to make.
+ // Note that this looks N^2, but it's really 2N worst case, if every node
+ // is unreachable. This is because we are still going to only visit each
+ // unreachable node once, we may just visit it in two directions,
+ // depending on how lucky we get.
+ SmallPtrSet<NodeType *, 4> ConnectToExitBlock;
+ for (auto I : nodes(&F))
+ if (!DT.Info.count(I)) {
+ // Find the furthest away we can get by following successors, then
+ // follow them in reverse. This gives us some reasonable answer about
+ // the post-dom tree inside any infinite loop. In particular, it
+ // guarantees we get to the farthest away point along *some*
+ // path. This also matches GCC behavior. If we really wanted a
+ // totally complete picture of dominance inside this infinite loop, we
+ // could do it with SCC-like algorithms to find the lowest and highest
+ // points in the infinite loop. In theory, it would be nice to give
+ // the canonical backedge for the loop, but it's expensive.
+ auto *FurthestAway = *po_begin(I);
+ ConnectToExitBlock.insert(FurthestAway);
+ N = ReverseDFSPass<GraphT>(DT, FurthestAway, N);
+ }
+ // Finally, now everything should be visited, and anything with parent
+ // ==
+ // 0 should be connected to virtual exit.
+ for (auto *Node : ConnectToExitBlock) {
+ auto FindResult = DT.Info.find(Node);
+ assert(FindResult != DT.Info.end() &&
+ "Everything should have been visited by now");
+ if (FindResult->second.Parent == 0) {
+ FindResult->second.Parent = 1;
+ DT.addRoot(Node);
+ }
+ }
+ }
+ } else {
+ N = DFSPass<GraphT>(DT, GraphTraits<FuncT *>::getEntryNode(&F), N);
}
- // it might be that some blocks did not get a DFS number (e.g., blocks of
- // infinite loops). In these cases an artificial exit node is required.
- MultipleRoots |= (DT.isPostDominator() && N != GraphTraits<FuncT*>::size(&F));
-
// When naively implemented, the Lengauer-Tarjan algorithm requires a separate
// bucket for each vertex. However, this is unnecessary, because each vertex
// is only placed into a single bucket (that of its semidominator), and each
WIDom = DT.IDoms[WIDom];
}
- if (DT.Roots.empty()) return;
-
// Add a node for the root. This node might be the actual root, if there is
// one exit block, or it may be the virtual exit (denoted by (BasicBlock *)0)
// which postdominates all real exits if there are multiple exit blocks, or
// an infinite loop.
- typename GraphT::NodeRef Root = !MultipleRoots ? DT.Roots[0] : nullptr;
+ typename GraphT::NodeRef Root = NeedFakeRoot ? nullptr : DT.Roots[0];
DT.RootNode =
(DT.DomTreeNodes[Root] =
}
}
- // Mark blocks live if there is no path from the block to the
- // return of the function or a successor for which this is true.
- // This protects IDFCalculator which cannot handle such blocks.
- for (auto &BBInfoPair : BlockInfo) {
- auto &BBInfo = BBInfoPair.second;
- if (BBInfo.terminatorIsLive())
- continue;
- auto *BB = BBInfo.BB;
- if (!PDT.getNode(BB)) {
- markLive(BBInfo.Terminator);
- continue;
- }
- for (auto *Succ : successors(BB))
- if (!PDT.getNode(Succ)) {
- markLive(BBInfo.Terminator);
- break;
- }
- }
-
- // Mark blocks live if there is no path from the block to the
- // return of the function or a successor for which this is true.
- // This protects IDFCalculator which cannot handle such blocks.
- for (auto &BBInfoPair : BlockInfo) {
- auto &BBInfo = BBInfoPair.second;
- if (BBInfo.terminatorIsLive())
- continue;
- auto *BB = BBInfo.BB;
- if (!PDT.getNode(BB)) {
- DEBUG(dbgs() << "Not post-dominated by return: " << BB->getName()
+ // Mark blocks live if there is no path from the block to a
+ // return of the function.
+ // We do this by seeing which of the postdomtree root children exit the
+ // program, and for all others, mark the subtree live.
+ for (auto &PDTChild : children<DomTreeNode *>(PDT.getRootNode())) {
+ auto *BB = PDTChild->getBlock();
+ auto &Info = BlockInfo[BB];
+ // Real function return
+ if (isa<ReturnInst>(Info.Terminator)) {
+ DEBUG(dbgs() << "post-dom root child is not a return: " << BB->getName()
<< '\n';);
- markLive(BBInfo.Terminator);
continue;
}
- for (auto *Succ : successors(BB))
- if (!PDT.getNode(Succ)) {
- DEBUG(dbgs() << "Successor not post-dominated by return: "
- << BB->getName() << '\n';);
- markLive(BBInfo.Terminator);
- break;
- }
+
+ // This child is something else, like an infinite loop.
+ for (auto DFNode : depth_first(PDTChild))
+ markLive(BlockInfo[DFNode->getBlock()].Terminator);
}
// Treat the entry block as always live
--- /dev/null
+; RUN: opt < %s -postdomtree -analyze | FileCheck %s
+; RUN: opt < %s -passes='print<postdomtree>' 2>&1 | FileCheck %s
+
+; Function Attrs: nounwind ssp uwtable
+define void @foo() {
+ br label %1
+
+; <label>:1 ; preds = %0, %1
+ br label %1
+ ; No predecessors!
+ ret void
+}
+
+; CHECK: Inorder PostDominator Tree:
+; CHECK-NEXT: [1] <<exit node>> {0,7}
+; CHECK-NEXT: [2] %2 {1,2}
+; CHECK-NEXT: [2] %1 {3,6}
+; CHECK-NEXT: [3] %0 {4,5}
bb35:
ret void
}
-; CHECK: [3] %entry
+;CHECK:Inorder PostDominator Tree:
+;CHECK-NEXT: [1] <<exit node>> {0,9}
+;CHECK-NEXT: [2] %bb35 {1,4}
+;CHECK-NEXT: [3] %bb35.loopexit3 {2,3}
+;CHECK-NEXT: [2] %entry {5,6}
+;CHECK-NEXT: [2] %bb3.i {7,8}
bb35:
ret void
}
-; CHECK: [4] %entry
+; CHECK: Inorder PostDominator Tree:
+; CHECK-NEXT: [1] <<exit node>> {0,11}
+; CHECK-NEXT: [2] %bb35 {1,4}
+; CHECK-NEXT: [3] %bb35.loopexit3 {2,3}
+; CHECK-NEXT: [2] %a {5,6}
+; CHECK-NEXT: [2] %entry {7,8}
+; CHECK-NEXT: [2] %bb3.i {9,10}
bb35:
ret void
}
-; CHECK: [3] %entry
+; CHECK: Inorder PostDominator Tree:
+; CHECK-NEXT: [1] <<exit node>> {0,97}
+; CHECK-NEXT: [2] %bb35 {1,92}
+; CHECK-NEXT: [3] %bb35.loopexit3 {2,3}
+; CHECK-NEXT: [3] %bb35.loopexit {4,5}
+; CHECK-NEXT: [3] %bb31 {6,7}
+; CHECK-NEXT: [3] %bb30 {8,9}
+; CHECK-NEXT: [3] %bb30.loopexit1 {10,11}
+; CHECK-NEXT: [3] %bb30.loopexit {12,13}
+; CHECK-NEXT: [3] %bb23 {14,15}
+; CHECK-NEXT: [3] %bb23.us {16,17}
+; CHECK-NEXT: [3] %bb23.preheader {18,19}
+; CHECK-NEXT: [3] %bb23.us.preheader {20,21}
+; CHECK-NEXT: [3] %bb.nph {22,23}
+; CHECK-NEXT: [3] %bb29.preheader {24,25}
+; CHECK-NEXT: [3] %bb20 {26,27}
+; CHECK-NEXT: [3] %bb19 {28,29}
+; CHECK-NEXT: [3] %bb.nph14 {30,31}
+; CHECK-NEXT: [3] %bb17.loopexit.split {32,33}
+; CHECK-NEXT: [3] %bb16 {34,35}
+; CHECK-NEXT: [3] %bb15 {36,37}
+; CHECK-NEXT: [3] %bb15.loopexit2 {38,39}
+; CHECK-NEXT: [3] %bb15.loopexit {40,41}
+; CHECK-NEXT: [3] %bb8 {42,43}
+; CHECK-NEXT: [3] %bb8.us {44,45}
+; CHECK-NEXT: [3] %bb8.preheader {46,47}
+; CHECK-NEXT: [3] %bb8.us.preheader {48,49}
+; CHECK-NEXT: [3] %bb.nph18 {50,51}
+; CHECK-NEXT: [3] %bb14.preheader {52,53}
+; CHECK-NEXT: [3] %bb5 {54,55}
+; CHECK-NEXT: [3] %bb4 {56,57}
+; CHECK-NEXT: [3] %bb.nph21 {58,59}
+; CHECK-NEXT: [3] %bb3.i.loopexit.us {60,61}
+; CHECK-NEXT: [3] %bb8.i.us {62,63}
+; CHECK-NEXT: [3] %bb4.i.us {64,65}
+; CHECK-NEXT: [3] %bb6.i.us {66,67}
+; CHECK-NEXT: [3] %bb1.i.us {68,69}
+; CHECK-NEXT: [3] %bb.i4.us.backedge {70,71}
+; CHECK-NEXT: [3] %bb7.i.us {72,73}
+; CHECK-NEXT: [3] %bb.i4.us {74,75}
+; CHECK-NEXT: [3] %bb3.split.us {76,77}
+; CHECK-NEXT: [3] %bb3 {78,79}
+; CHECK-NEXT: [3] %bb32.preheader {80,81}
+; CHECK-NEXT: [3] %_float32_unpack.exit8 {82,83}
+; CHECK-NEXT: [3] %bb.i5 {84,85}
+; CHECK-NEXT: [3] %_float32_unpack.exit {86,87}
+; CHECK-NEXT: [3] %bb.i {88,89}
+; CHECK-NEXT: [3] %bb {90,91}
+; CHECK-NEXT: [2] %entry {93,94}
+; CHECK-NEXT: [2] %bb3.i {95,96}
bb35:
ret void
}
-; CHECK: [4] %entry
+; CHECK: Inorder PostDominator Tree:
+; CHECK-NEXT: [1] <<exit node>> {0,15}
+; CHECK-NEXT: [2] %bb35 {1,4}
+; CHECK-NEXT: [3] %bb35.loopexit3 {2,3}
+; CHECK-NEXT: [2] %c {5,12}
+; CHECK-NEXT: [3] %b {6,7}
+; CHECK-NEXT: [3] %entry {8,9}
+; CHECK-NEXT: [3] %a {10,11}
+; CHECK-NEXT: [2] %bb3.i {13,14}
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
-; CHECK: [1] 1 => 4
-; STAT: 2 region - The # of regions
-; STAT: 1 region - The # of simple regions
+; STAT: 1 region - The # of regions
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
-; CHECK: [1] 1 => 3
+; CHECK: [1] 5 => 6
; STAT: 2 region - The # of regions
-; STAT: 1 region - The # of simple regions
-; BBIT: 0, 1, 2, 5, 11, 6, 12, 3, 4,
-; BBIT: 1, 2, 5, 11, 6, 12,
+; BBIT: 0, 1, 2, 5, 11, 6, 12, 3, 4,
+; BBIT: 5, 11, 12,
-; RNIT: 0, 1 => 3, 3, 4,
-; RNIT: 1, 2, 5, 11, 6, 12,
+; RNIT: 0, 1, 2, 5 => 6, 6, 3, 4,
+; RNIT: 5, 11, 12,
ret void
}
; CHECK-NOT: =>
-; CHECK: [0] 0 => <Function Return>
-; CHECK-NEXT: [1] 1 => 3
-; CHECK-NEXT: [1] 7 => 1
+; CHECK:[0] 0 => <Function Return>
+; CHECK-NEXT: [1] 5 => 6
+; CHECK-NEXT: [1] 9 => 10
; STAT: 3 region - The # of regions
-; STAT: 2 region - The # of simple regions
-; BBIT: 0, 7, 1, 2, 5, 11, 6, 12, 3, 4, 8, 9, 13, 10, 14,
-; BBIT: 7, 8, 9, 13, 10, 14,
-; BBIT: 1, 2, 5, 11, 6, 12,
+; BBIT: 0, 7, 1, 2, 5, 11, 6, 12, 3, 4, 8, 9, 13, 10, 14,
+; BBIT: 5, 11, 12,
+; BBIT: 9, 13, 14,
-; RNIT: 0, 7 => 1, 1 => 3, 3, 4,
-; RNIT: 7, 8, 9, 13, 10, 14,
-; RNIT: 1, 2, 5, 11, 6, 12,
+; RNIT: 0, 7, 1, 2, 5 => 6, 6, 3, 4, 8, 9 => 10, 10,
+; RNIT: 5, 11, 12,
+; RNIT: 9, 13, 14,
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
-; CHECK-NEXT: [1] 7 => 3
-; STAT: 2 region - The # of regions
+; CHECK-NEXT: [1] 2 => 10
+; CHECK_NEXT: [2] 5 => 6
+; STAT: 3 region - The # of regions
; STAT: 1 region - The # of simple regions
; BBIT: 0, 7, 1, 2, 5, 11, 6, 10, 8, 9, 13, 14, 12, 3, 4,
-; BBIT: 7, 1, 2, 5, 11, 6, 10, 8, 9, 13, 14, 12,
-
-; RNIT: 0, 7 => 3, 3, 4,
-; RNIT: 7, 1, 2, 5, 11, 6, 10, 8, 9, 13, 14, 12,
+; BBIT: 2, 5, 11, 6, 12,
+; BBIT: 5, 11, 12,
+; RNIT: 0, 7, 1, 2 => 10, 10, 8, 9, 13, 14, 3, 4,
+; RNIT: 2, 5 => 6, 6,
+; RNIT: 5, 11, 12,
; CHECK: Region tree:
; CHECK-NEXT: [0] 0 => <Function Return>
-; CHECK-NEXT: [1] 7 => 3
; CHECK-NEXT: End region tree
; CHECK: Region tree:
; CHECK-NEXT: [0] 0 => <Function Return>
-; CHECK-NEXT: [1] 7 => 3
; CHECK-NEXT: End region tree
; CHECK-LABEL: @invert_branch_on_arg_inf_loop(
; CHECK: entry:
; CHECK: %arg.inv = xor i1 %arg, true
-; CHECK: phi i1 [ false, %Flow1 ], [ %arg.inv, %entry ]
define void @invert_branch_on_arg_inf_loop(i32 addrspace(1)* %out, i1 %arg) {
entry:
- br i1 %arg, label %for.end, label %for.body
+ br i1 %arg, label %for.end, label %sesestart
+sesestart:
+ br label %for.body
for.body: ; preds = %entry, %for.body
store i32 999, i32 addrspace(1)* %out, align 4
- br label %for.body
+ br i1 %arg, label %for.body, label %seseend
+seseend:
+ ret void
for.end: ; preds = %Flow
ret void
+; XFAIL: *
; RUN: opt -S -o - -structurizecfg -verify-dom-info < %s | FileCheck %s
; CHECK-LABEL: @no_branch_to_entry_undef(
projects / llvm / commitdiff