[llvm] r306711 - [Dominators] Add parent and sibling property verification (non-hacky)
Jakub Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 10:45:51 PDT 2017
Author: kuhar
Date: Thu Jun 29 10:45:51 2017
New Revision: 306711
URL: http://llvm.org/viewvc/llvm-project?rev=306711&view=rev
Log:
[Dominators] Add parent and sibling property verification (non-hacky)
Summary:
This patch adds an additional level of verification - it checks parent and sibling properties of a tree. By definition, every tree with these two properties is a dominator tree.
It is possible to run those check by running llvm with `-verify-dom-info=1`.
Bootstrapping clang and building the llvm test suite with this option enabled doesn't yield any errors.
Reviewers: dberlin, sanjoy, chandlerc
Reviewed By: dberlin
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D34482
Modified:
llvm/trunk/include/llvm/IR/Dominators.h
llvm/trunk/include/llvm/Support/GenericDomTree.h
llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
llvm/trunk/lib/IR/Dominators.cpp
Modified: llvm/trunk/include/llvm/IR/Dominators.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Dominators.h?rev=306711&r1=306710&r2=306711&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Dominators.h (original)
+++ llvm/trunk/include/llvm/IR/Dominators.h Thu Jun 29 10:45:51 2017
@@ -36,12 +36,22 @@ class raw_ostream;
extern template class DomTreeNodeBase<BasicBlock>;
extern template class DominatorTreeBase<BasicBlock>;
+namespace DomTreeBuilder {
extern template void Calculate<Function, BasicBlock *>(
DominatorTreeBaseByGraphTraits<GraphTraits<BasicBlock *>> &DT, Function &F);
+
extern template void Calculate<Function, Inverse<BasicBlock *>>(
DominatorTreeBaseByGraphTraits<GraphTraits<Inverse<BasicBlock *>>> &DT,
Function &F);
+extern template bool Verify<BasicBlock *>(
+ const DominatorTreeBaseByGraphTraits<GraphTraits<BasicBlock *>> &DT);
+
+extern template bool Verify<Inverse<BasicBlock *>>(
+ const DominatorTreeBaseByGraphTraits<GraphTraits<Inverse<BasicBlock *>>>
+ &DT);
+} // namespace DomTreeBuilder
+
using DomTreeNode = DomTreeNodeBase<BasicBlock>;
class BasicBlockEdge {
Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=306711&r1=306710&r2=306711&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)
+++ llvm/trunk/include/llvm/Support/GenericDomTree.h Thu Jun 29 10:45:51 2017
@@ -204,12 +204,16 @@ void PrintDomTree(const DomTreeNodeBase<
namespace DomTreeBuilder {
template <class NodeT>
struct SemiNCAInfo;
-} // namespace DomTreeBuilder
// The calculate routine is provided in a separate header but referenced here.
template <class FuncT, class N>
void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<N>> &DT, FuncT &F);
+// The verify function is provided in a separate header but referenced here.
+template <class N>
+bool Verify(const DominatorTreeBaseByGraphTraits<GraphTraits<N>> &DT);
+} // namespace DomTreeBuilder
+
/// \brief Core dominator tree base class.
///
/// This class is a generic template over graph nodes. It is instantiated for
@@ -723,16 +727,23 @@ public:
NodeT *entry = TraitsTy::getEntryNode(&F);
addRoot(entry);
- Calculate<FT, NodeT *>(*this, F);
+ DomTreeBuilder::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);
+ DomTreeBuilder::Calculate<FT, Inverse<NodeT *>>(*this, F);
}
}
+
+ /// verify - check parent and sibling property
+ bool verify() const {
+ return this->isPostDominator()
+ ? DomTreeBuilder::Verify<Inverse<NodeT *>>(*this)
+ : DomTreeBuilder::Verify<NodeT *>(*this);
+ }
};
// These two functions are declared out of line as a workaround for building
Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=306711&r1=306710&r2=306711&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)
+++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Thu Jun 29 10:45:51 2017
@@ -30,8 +30,8 @@
#include "llvm/Support/GenericDomTree.h"
namespace llvm {
-
namespace DomTreeBuilder {
+
// Information record used by Semi-NCA during tree construction.
template <typename NodeT>
struct SemiNCAInfo {
@@ -47,11 +47,13 @@ struct SemiNCAInfo {
NodePtr IDom = nullptr;
};
- DomTreeT &DT;
std::vector<NodePtr> NumToNode;
DenseMap<NodePtr, InfoRec> NodeToInfo;
- SemiNCAInfo(DomTreeT &DT) : DT(DT) {}
+ void clear() {
+ NumToNode.clear();
+ NodeToInfo.clear();
+ }
NodePtr getIDom(NodePtr BB) const {
auto InfoIt = NodeToInfo.find(BB);
@@ -60,7 +62,7 @@ struct SemiNCAInfo {
return InfoIt->second.IDom;
}
- TreeNodePtr getNodeForBlock(NodePtr BB) {
+ TreeNodePtr getNodeForBlock(NodePtr BB, DomTreeT &DT) {
if (TreeNodePtr Node = DT.getNode(BB)) return Node;
// Haven't calculated this node yet? Get or calculate the node for the
@@ -68,7 +70,7 @@ struct SemiNCAInfo {
NodePtr IDom = getIDom(BB);
assert(IDom || DT.DomTreeNodes[nullptr]);
- TreeNodePtr IDomNode = getNodeForBlock(IDom);
+ TreeNodePtr IDomNode = getNodeForBlock(IDom, DT);
// Add a new tree node for this NodeT, and link it as a child of
// IDomNode
@@ -121,7 +123,7 @@ struct SemiNCAInfo {
return N;
}
- unsigned runDFS(NodePtr V, unsigned N) {
+ unsigned runForwardDFS(NodePtr V, unsigned N) {
auto DFStorage = getStorage();
for (auto I = df_ext_begin(V, DFStorage), E = df_ext_end(V, DFStorage);
@@ -178,7 +180,7 @@ struct SemiNCAInfo {
}
template <typename NodeType>
- void runSemiNCA(unsigned NumBlocks) {
+ void runSemiNCA(DomTreeT &DT, unsigned NumBlocks) {
unsigned N = 0;
NumToNode.push_back(nullptr);
@@ -198,7 +200,7 @@ struct SemiNCAInfo {
i != e; ++i)
N = runReverseDFS(DT.Roots[i], N);
} else {
- N = runDFS(DT.Roots[0], N);
+ N = runForwardDFS(DT.Roots[0], N);
}
// It might be that some blocks did not get a DFS number (e.g., blocks of
@@ -268,7 +270,7 @@ struct SemiNCAInfo {
assert(ImmDom || DT.DomTreeNodes[nullptr]);
// Get or calculate the node for the immediate dominator
- TreeNodePtr IDomNode = getNodeForBlock(ImmDom);
+ TreeNodePtr IDomNode = getNodeForBlock(ImmDom, DT);
// Add a new tree node for this BasicBlock, and link it as a child of
// IDomNode
@@ -278,8 +280,115 @@ struct SemiNCAInfo {
DT.updateDFSNumbers();
}
+
+ void doFullDFSWalk(const DomTreeT &DT) {
+ unsigned Num = 0;
+ for (auto *Root : DT.Roots)
+ if (!DT.isPostDominator())
+ Num = runForwardDFS(Root, Num);
+ else
+ Num = runReverseDFS(Root, Num);
+ }
+
+ static void PrintBlockOrNullptr(raw_ostream &O, NodePtr Obj) {
+ if (!Obj)
+ O << "nullptr";
+ else
+ Obj->printAsOperand(O, false);
+ }
+
+ // Checks if the tree contains all reachable nodes in the input graph.
+ bool verifyReachability(const DomTreeT &DT) {
+ clear();
+ doFullDFSWalk(DT);
+
+ for (auto &NodeToTN : DT.DomTreeNodes) {
+ const TreeNodePtr TN = NodeToTN.second.get();
+ const NodePtr BB = TN->getBlock();
+ if (!BB) continue;
+
+ if (NodeToInfo.count(BB) == 0) {
+ errs() << "DomTree node ";
+ PrintBlockOrNullptr(errs(), BB);
+ errs() << " not found by DFS walk!\n";
+ errs().flush();
+
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ // Checks if the tree has the parent property: if for all edges from V to W in
+ // the input graph, such that V is reachable, the parent of W in the tree is
+ // an ancestor of V in the tree.
+ //
+ // This means that if a node gets disconnected from the graph, then all of
+ // the nodes it dominated previously will now become unreachable.
+ bool verifyParentProperty(const DomTreeT &DT) {
+ for (auto &NodeToTN : DT.DomTreeNodes) {
+ const TreeNodePtr TN = NodeToTN.second.get();
+ const NodePtr BB = TN->getBlock();
+ if (!BB || TN->getChildren().empty()) continue;
+
+ clear();
+ NodeToInfo.insert({BB, {}});
+ doFullDFSWalk(DT);
+
+ for (TreeNodePtr Child : TN->getChildren())
+ if (NodeToInfo.count(Child->getBlock()) != 0) {
+ errs() << "Child ";
+ PrintBlockOrNullptr(errs(), Child->getBlock());
+ errs() << " reachable after its parent ";
+ PrintBlockOrNullptr(errs(), BB);
+ errs() << " is removed!\n";
+ errs().flush();
+
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ // Check if the tree has sibling property: if a node V does not dominate a
+ // node W for all siblings V and W in the tree.
+ //
+ // This means that if a node gets disconnected from the graph, then all of its
+ // siblings will now still be reachable.
+ bool verifySiblingProperty(const DomTreeT &DT) {
+ for (auto &NodeToTN : DT.DomTreeNodes) {
+ const TreeNodePtr TN = NodeToTN.second.get();
+ const NodePtr BB = TN->getBlock();
+ if (!BB || TN->getChildren().empty()) continue;
+
+ const auto &Siblings = TN->getChildren();
+ for (const TreeNodePtr N : Siblings) {
+ clear();
+ NodeToInfo.insert({N->getBlock(), {}});
+ doFullDFSWalk(DT);
+
+ for (const TreeNodePtr S : Siblings) {
+ if (S == N) continue;
+
+ if (NodeToInfo.count(S->getBlock()) == 0) {
+ errs() << "Node ";
+ PrintBlockOrNullptr(errs(), S->getBlock());
+ errs() << " not reachable when its sibling ";
+ PrintBlockOrNullptr(errs(), N->getBlock());
+ errs() << " is removed!\n";
+ errs().flush();
+
+ return false;
+ }
+ }
+ }
+ }
+
+ return true;
+ }
};
-} // namespace DomTreeBuilder
template <class FuncT, class NodeT>
void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<NodeT>> &DT,
@@ -287,11 +396,23 @@ void Calculate(DominatorTreeBaseByGraphT
using NodePtr = typename GraphTraits<NodeT>::NodeRef;
static_assert(std::is_pointer<NodePtr>::value,
"NodePtr should be a pointer type");
+ SemiNCAInfo<typename std::remove_pointer<NodePtr>::type> SNCA;
+ SNCA.template runSemiNCA<NodeT>(DT, GraphTraits<FuncT *>::size(&F));
+}
+
+template <class NodeT>
+bool Verify(const DominatorTreeBaseByGraphTraits<GraphTraits<NodeT>> &DT) {
+ using NodePtr = typename GraphTraits<NodeT>::NodeRef;
+ static_assert(std::is_pointer<NodePtr>::value,
+ "NodePtr should be a pointer type");
+ SemiNCAInfo<typename std::remove_pointer<NodePtr>::type> SNCA;
+
+ return SNCA.verifyReachability(DT) && SNCA.verifyParentProperty(DT) &&
+ SNCA.verifySiblingProperty(DT);
- DomTreeBuilder::SemiNCAInfo<typename std::remove_pointer<NodePtr>::type>
- SNCA(DT);
- SNCA.template runSemiNCA<NodeT>(GraphTraits<FuncT *>::size(&F));
}
+
+} // namespace DomTreeBuilder
} // namespace llvm
#endif
Modified: llvm/trunk/lib/IR/Dominators.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Dominators.cpp?rev=306711&r1=306710&r2=306711&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Dominators.cpp (original)
+++ llvm/trunk/lib/IR/Dominators.cpp Thu Jun 29 10:45:51 2017
@@ -63,15 +63,22 @@ bool BasicBlockEdge::isSingleEdge() cons
template class llvm::DomTreeNodeBase<BasicBlock>;
template class llvm::DominatorTreeBase<BasicBlock>;
-template void llvm::Calculate<Function, BasicBlock *>(
+template void llvm::DomTreeBuilder::Calculate<Function, BasicBlock *>(
DominatorTreeBase<
typename std::remove_pointer<GraphTraits<BasicBlock *>::NodeRef>::type>
&DT,
Function &F);
-template void llvm::Calculate<Function, Inverse<BasicBlock *>>(
+template void llvm::DomTreeBuilder::Calculate<Function, Inverse<BasicBlock *>>(
DominatorTreeBase<typename std::remove_pointer<
GraphTraits<Inverse<BasicBlock *>>::NodeRef>::type> &DT,
Function &F);
+template bool llvm::DomTreeBuilder::Verify<BasicBlock *>(
+ const DominatorTreeBase<
+ typename std::remove_pointer<GraphTraits<BasicBlock *>::NodeRef>::type>
+ &DT);
+template bool llvm::DomTreeBuilder::Verify<Inverse<BasicBlock *>>(
+ const DominatorTreeBase<typename std::remove_pointer<
+ GraphTraits<Inverse<BasicBlock *>>::NodeRef>::type> &DT);
bool DominatorTree::invalidate(Function &F, const PreservedAnalyses &PA,
FunctionAnalysisManager::Invalidator &) {
@@ -285,6 +292,12 @@ bool DominatorTree::isReachableFromEntry
}
void DominatorTree::verifyDomTree() const {
+ if (!verify()) {
+ errs() << "\n~~~~~~~~~~~\n\t\tDomTree verification failed!\n~~~~~~~~~~~\n";
+ print(errs());
+ abort();
+ }
+
Function &F = *getRoot()->getParent();
DominatorTree OtherDT;
More information about the llvm-commits
mailing list