Revert "Fix PR 24415 (at least), by making our post-dominator tree behavior sane."

and also "clang-format GenericDomTreeConstruction.h, since the current
formatting makes it look like their is a bug in the loop indentation, and there
is not"

This reverts commit r296535.

There are still some open design questions which I would like to discuss. I
revert this for Daniel (who gave the OK), as he is on vacation.

llvm-svn: 296812
This commit is contained in:
Tobias Grosser 2017-03-02 21:08:37 +00:00
parent ed28e742ee
commit f818c3300b
16 changed files with 105 additions and 196 deletions

View File

@ -778,12 +778,22 @@ public:
/// 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);
}
}

View File

@ -25,7 +25,6 @@
#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"
@ -40,10 +39,8 @@ public:
df_iterator_dom_storage(BaseSet &Storage) : Storage(Storage) {}
typedef typename BaseSet::iterator iterator;
std::pair<iterator, bool> insert(NodeRef To) {
auto Result = Storage.insert({To, InfoType()});
return Result;
std::pair<iterator, bool> insert(NodeRef N) {
return Storage.insert({N, InfoType()});
}
void completed(NodeRef) {}
@ -58,6 +55,7 @@ unsigned ReverseDFSPass(DominatorTreeBaseByGraphTraits<GraphT> &DT,
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;
@ -69,6 +67,11 @@ unsigned ReverseDFSPass(DominatorTreeBaseByGraphTraits<GraphT> &DT,
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;
}
@ -139,77 +142,34 @@ template <class FuncT, class NodeT>
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 NeedFakeRoot = DT.isPostDominator();
// If this is post dominators, push a fake node to start
if (NeedFakeRoot) {
bool MultipleRoots = (DT.Roots.size() > 1);
if (MultipleRoots) {
auto &BBInfo = DT.Info[nullptr];
BBInfo.DFSNum = BBInfo.Semi = ++N;
BBInfo.Label = nullptr;
DT.Vertex.push_back(nullptr); // Vertex[n] = V;
} else {
// The root is the entry block of the CFG
DT.addRoot(FuncGraphT::getEntryNode(&F));
DT.Vertex.push_back(nullptr); // Vertex[n] = V;
}
// Step #1: Number blocks in depth-first order and initialize variables used
// in later stages of the algorithm.
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);
}
}
}
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, GraphTraits<FuncT *>::getEntryNode(&F), N);
N = DFSPass<GraphT>(DT, DT.Roots[0], 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
@ -274,11 +234,13 @@ void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<NodeT>> &DT,
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 = NeedFakeRoot ? nullptr : DT.Roots[0];
typename GraphT::NodeRef Root = !MultipleRoots ? DT.Roots[0] : nullptr;
DT.RootNode =
(DT.DomTreeNodes[Root] =

View File

@ -253,23 +253,46 @@ void AggressiveDeadCodeElimination::initialize() {
}
}
// 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';);
// 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;
}
}
// This child is something else, like an infinite loop.
for (auto DFNode : depth_first(PDTChild))
markLive(BlockInfo[DFNode->getBlock()].Terminator);
// 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()
<< '\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;
}
}
// Treat the entry block as always live

View File

@ -1,18 +0,0 @@
; 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}

View File

@ -12,9 +12,4 @@ bb35.loopexit3:
bb35:
ret void
}
;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}
; CHECK: [3] %entry

View File

@ -16,10 +16,4 @@ bb35.loopexit3:
bb35:
ret void
}
; 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}
; CHECK: [4] %entry

View File

@ -144,53 +144,4 @@ bb35.loopexit3:
bb35:
ret void
}
; 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}
; CHECK: [3] %entry

View File

@ -21,12 +21,4 @@ bb35.loopexit3:
bb35:
ret void
}
; 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: [4] %entry

View File

@ -16,4 +16,6 @@ define void @normal_condition() nounwind {
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
; STAT: 1 region - The # of regions
; CHECK: [1] 1 => 4
; STAT: 2 region - The # of regions
; STAT: 1 region - The # of simple regions

View File

@ -26,11 +26,12 @@ define void @normal_condition() nounwind {
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
; CHECK: [1] 5 => 6
; CHECK: [1] 1 => 3
; STAT: 2 region - The # of regions
; STAT: 1 region - The # of simple regions
; BBIT: 0, 1, 2, 5, 11, 6, 12, 3, 4,
; BBIT: 5, 11, 12,
; BBIT: 0, 1, 2, 5, 11, 6, 12, 3, 4,
; BBIT: 1, 2, 5, 11, 6, 12,
; RNIT: 0, 1, 2, 5 => 6, 6, 3, 4,
; RNIT: 5, 11, 12,
; RNIT: 0, 1 => 3, 3, 4,
; RNIT: 1, 2, 5, 11, 6, 12,

View File

@ -38,15 +38,16 @@ define void @normal_condition() nounwind {
ret void
}
; CHECK-NOT: =>
; CHECK:[0] 0 => <Function Return>
; CHECK-NEXT: [1] 5 => 6
; CHECK-NEXT: [1] 9 => 10
; CHECK: [0] 0 => <Function Return>
; CHECK-NEXT: [1] 1 => 3
; CHECK-NEXT: [1] 7 => 1
; 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: 5, 11, 12,
; BBIT: 9, 13, 14,
; 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,
; RNIT: 0, 7, 1, 2, 5 => 6, 6, 3, 4, 8, 9 => 10, 10,
; RNIT: 5, 11, 12,
; RNIT: 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,

View File

@ -38,14 +38,12 @@ define void @normal_condition() nounwind {
}
; CHECK-NOT: =>
; CHECK: [0] 0 => <Function Return>
; CHECK-NEXT: [1] 2 => 10
; CHECK_NEXT: [2] 5 => 6
; STAT: 3 region - The # of regions
; CHECK-NEXT: [1] 7 => 3
; STAT: 2 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: 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,
; 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,

View File

@ -19,5 +19,6 @@ define void @normal_condition() nounwind {
; CHECK: Region tree:
; CHECK-NEXT: [0] 0 => <Function Return>
; CHECK-NEXT: [1] 7 => 3
; CHECK-NEXT: End region tree

View File

@ -21,4 +21,5 @@ define void @normal_condition() nounwind {
; CHECK: Region tree:
; CHECK-NEXT: [0] 0 => <Function Return>
; CHECK-NEXT: [1] 7 => 3
; CHECK-NEXT: End region tree

View File

@ -3,17 +3,14 @@
; 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 %sesestart
sesestart:
br label %for.body
br i1 %arg, label %for.end, label %for.body
for.body: ; preds = %entry, %for.body
store i32 999, i32 addrspace(1)* %out, align 4
br i1 %arg, label %for.body, label %seseend
seseend:
ret void
br label %for.body
for.end: ; preds = %Flow
ret void

View File

@ -1,4 +1,3 @@
; XFAIL: *
; RUN: opt -S -o - -structurizecfg -verify-dom-info < %s | FileCheck %s
; CHECK-LABEL: @no_branch_to_entry_undef(