Index: include/llvm/IR/Dominators.h =================================================================== --- include/llvm/IR/Dominators.h +++ include/llvm/IR/Dominators.h @@ -36,12 +36,22 @@ extern template class DomTreeNodeBase; extern template class DominatorTreeBase; +namespace DomTreeBuilder { extern template void Calculate( DominatorTreeBaseByGraphTraits> &DT, Function &F); + extern template void Calculate>( DominatorTreeBaseByGraphTraits>> &DT, Function &F); +extern template bool Verify( + const DominatorTreeBaseByGraphTraits> &DT); + +extern template bool Verify>( + const DominatorTreeBaseByGraphTraits>> + &DT); +} // namespace DomTreeBuilder + using DomTreeNode = DomTreeNodeBase; class BasicBlockEdge { Index: include/llvm/Support/GenericDomTree.h =================================================================== --- include/llvm/Support/GenericDomTree.h +++ include/llvm/Support/GenericDomTree.h @@ -204,12 +204,16 @@ namespace DomTreeBuilder { template struct SemiNCAInfo; -} // namespace DomTreeBuilder // The calculate routine is provided in a separate header but referenced here. template void Calculate(DominatorTreeBaseByGraphTraits> &DT, FuncT &F); +// The verify function is provided in a separate header but referenced here. +template +bool Verify(const DominatorTreeBaseByGraphTraits> &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 @@ NodeT *entry = TraitsTy::getEntryNode(&F); addRoot(entry); - Calculate(*this, F); + DomTreeBuilder::Calculate(*this, F); } else { // Initialize the roots list for (auto *Node : nodes(&F)) if (TraitsTy::child_begin(Node) == TraitsTy::child_end(Node)) addRoot(Node); - Calculate>(*this, F); + DomTreeBuilder::Calculate>(*this, F); } } + + /// verify - check parent and sibling property + bool verify() const { + return this->isPostDominator() + ? DomTreeBuilder::Verify>(*this) + : DomTreeBuilder::Verify(*this); + } }; // These two functions are declared out of line as a workaround for building Index: include/llvm/Support/GenericDomTreeConstruction.h =================================================================== --- include/llvm/Support/GenericDomTreeConstruction.h +++ include/llvm/Support/GenericDomTreeConstruction.h @@ -30,8 +30,8 @@ #include "llvm/Support/GenericDomTree.h" namespace llvm { - namespace DomTreeBuilder { + // Information record used by Semi-NCA during tree construction. template struct SemiNCAInfo { @@ -47,11 +47,13 @@ NodePtr IDom = nullptr; }; - DomTreeT &DT; std::vector NumToNode; DenseMap 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 @@ 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 @@ 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 @@ 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 @@ } template - void runSemiNCA(unsigned NumBlocks) { + void runSemiNCA(DomTreeT &DT, unsigned NumBlocks) { unsigned N = 0; NumToNode.push_back(nullptr); @@ -198,7 +200,7 @@ 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 @@ 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 @@ 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 void Calculate(DominatorTreeBaseByGraphTraits> &DT, @@ -287,11 +396,22 @@ using NodePtr = typename GraphTraits::NodeRef; static_assert(std::is_pointer::value, "NodePtr should be a pointer type"); + SemiNCAInfo::type> SNCA; + SNCA.template runSemiNCA(DT, GraphTraits::size(&F)); +} + +template +bool Verify(const DominatorTreeBaseByGraphTraits> &DT) { + using NodePtr = typename GraphTraits::NodeRef; + static_assert(std::is_pointer::value, + "NodePtr should be a pointer type"); + SemiNCAInfo::type> SNCA; - DomTreeBuilder::SemiNCAInfo::type> - SNCA(DT); - SNCA.template runSemiNCA(GraphTraits::size(&F)); + return SNCA.verifyReachability(DT) && SNCA.verifyParentProperty(DT) && + SNCA.verifySiblingProperty(DT); } -} // namespace DomTreeBuilder + +} // namespace DomTreeBuilder +} // namespace llvm #endif Index: lib/IR/Dominators.cpp =================================================================== --- lib/IR/Dominators.cpp +++ lib/IR/Dominators.cpp @@ -63,15 +63,22 @@ template class llvm::DomTreeNodeBase; template class llvm::DominatorTreeBase; -template void llvm::Calculate( +template void llvm::DomTreeBuilder::Calculate( DominatorTreeBase< typename std::remove_pointer::NodeRef>::type> &DT, Function &F); -template void llvm::Calculate>( +template void llvm::DomTreeBuilder::Calculate>( DominatorTreeBase>::NodeRef>::type> &DT, Function &F); +template bool llvm::DomTreeBuilder::Verify( + const DominatorTreeBase< + typename std::remove_pointer::NodeRef>::type> + &DT); +template bool llvm::DomTreeBuilder::Verify>( + const DominatorTreeBase>::NodeRef>::type> &DT); bool DominatorTree::invalidate(Function &F, const PreservedAnalyses &PA, FunctionAnalysisManager::Invalidator &) { @@ -285,6 +292,12 @@ } void DominatorTree::verifyDomTree() const { + if (!verify()) { + errs() << "\n~~~~~~~~~~~\n\t\tDomTree verification failed!\n~~~~~~~~~~~\n"; + print(errs()); + abort(); + } + Function &F = *getRoot()->getParent(); DominatorTree OtherDT;