RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 12:41:49 +01:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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

llvm-svn: 296535
This commit is contained in:
Daniel Berlin 2017-02-28 22:57:50 +00:00
parent 50bcef8d40
commit df7ca1ee6d
16 changed files with 198 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
include/llvm/Support/GenericDomTree.h
View File

@ -770,22 +770,12 @@ 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);
}
}

89
include/llvm/Support/GenericDomTreeConstruction.h
View File

@ -25,6 +25,7 @@
#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"
@ -39,8 +40,10 @@ public:
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) {}
@ -55,7 +58,6 @@ 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;
@ -67,11 +69,6 @@ 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;
}
@ -142,34 +139,78 @@ 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 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
@ -234,13 +275,11 @@ 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 = !MultipleRoots ? DT.Roots[0] : nullptr;
typename GraphT::NodeRef Root = NeedFakeRoot ? nullptr : DT.Roots[0];
DT.RootNode =
(DT.DomTreeNodes[Root] =

51
lib/Transforms/Scalar/ADCE.cpp
View File

@ -253,46 +253,23 @@ void AggressiveDeadCodeElimination::initialize() {
}
}
// 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

18
test/Analysis/PostDominators/pr24415.ll Normal file
View File

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

7
test/Analysis/PostDominators/pr6047_a.ll
View File

@ -12,4 +12,9 @@ bb35.loopexit3:
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}

8
test/Analysis/PostDominators/pr6047_b.ll
View File

@ -16,4 +16,10 @@ bb35.loopexit3:
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}

51
test/Analysis/PostDominators/pr6047_c.ll
View File

@ -144,4 +144,53 @@ bb35.loopexit3:
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}

10
test/Analysis/PostDominators/pr6047_d.ll
View File

@ -21,4 +21,12 @@ bb35.loopexit3:
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}

4
test/Analysis/RegionInfo/infinite_loop.ll
View File

@ -16,6 +16,4 @@ define void @normal_condition() nounwind {
}
; 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

11
test/Analysis/RegionInfo/infinite_loop_2.ll
View File

@ -26,12 +26,11 @@ define void @normal_condition() nounwind {
}
; 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,

19
test/Analysis/RegionInfo/infinite_loop_3.ll
View File

@ -38,16 +38,15 @@ define void @normal_condition() nounwind {
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,

14
test/Analysis/RegionInfo/infinite_loop_4.ll
View File

@ -38,12 +38,14 @@ define void @normal_condition() nounwind {
}
; 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,

1
test/Analysis/RegionInfo/infinite_loop_5_a.ll
View File

@ -19,6 +19,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

1
test/Analysis/RegionInfo/infinite_loop_5_b.ll
View File

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

9
test/Transforms/StructurizeCFG/branch-on-argument.ll
View File

@ -3,14 +3,17 @@
; 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

1
test/Transforms/StructurizeCFG/no-branch-to-entry.ll
View File

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