[PATCH] D41298: [PDT] Add verifyDomTree and verifyAnalysis for Post Dom Trees

[Dave Green via Phabricator via llvm-commits]

dmgreen updated this revision to Diff 130639.
dmgreen marked an inline comment as done.
dmgreen added a subscriber: kuhar.
dmgreen added a comment.

Hello. Sorry for the delay.

I have left this as verifyDomTree and using VerifyDomInfo to keep things consistent between the two dom trees. I can change this if you think it is best, but personally like the idea of treating the two trees similarly. Let me know what you think.


https://reviews.llvm.org/D41298

Files:
  include/llvm/Analysis/PostDominators.h
  lib/Analysis/PostDominators.cpp
  lib/IR/Dominators.cpp


Index: lib/IR/Dominators.cpp
===================================================================
--- lib/IR/Dominators.cpp
+++ lib/IR/Dominators.cpp
@@ -382,8 +382,8 @@
 }
 
 void DominatorTreeWrapperPass::verifyAnalysis() const {
-    if (VerifyDomInfo)
-      DT.verifyDomTree();
+  if (VerifyDomInfo)
+    DT.verifyDomTree();
 }
 
 void DominatorTreeWrapperPass::print(raw_ostream &OS, const Module *) const {
Index: lib/Analysis/PostDominators.cpp
===================================================================
--- lib/Analysis/PostDominators.cpp
+++ lib/Analysis/PostDominators.cpp
@@ -39,11 +39,42 @@
            PAC.preservedSet<CFGAnalyses>());
 }
 
+void PostDominatorTree::verifyDomTree() const {
+  // Perform the expensive checks only when VerifyDomInfo is set.
+  if (VerifyDomInfo && !verify()) {
+    errs() << "\n~~~~~~~~~~~\n\t\tPostDomTree verification failed!\n~~~~~~~~~~~\n";
+    print(errs());
+    abort();
+  }
+
+  assert(getRoots().size() >= 1);
+  Function &F = *getRoots()[0]->getParent();
+
+  PostDominatorTree OtherDT;
+  OtherDT.recalculate(F);
+  if (compare(OtherDT)) {
+    errs() << "PostDominatorTree for function " << F.getName()
+           << " is not up to date!\nExisting:\n";
+    print(errs());
+    errs() << "\nRecalculated:\n";
+    OtherDT.print(errs());
+    errs() << "\nCFG:\n";
+    F.print(errs());
+    errs().flush();
+    abort();
+  }
+}
+
 bool PostDominatorTreeWrapperPass::runOnFunction(Function &F) {
   DT.recalculate(F);
   return false;
 }
 
+void PostDominatorTreeWrapperPass::verifyAnalysis() const {
+  if (VerifyDomInfo)
+    DT.verifyDomTree();
+}
+
 void PostDominatorTreeWrapperPass::print(raw_ostream &OS, const Module *) const {
   DT.print(OS);
 }
Index: include/llvm/Analysis/PostDominators.h
===================================================================
--- include/llvm/Analysis/PostDominators.h
+++ include/llvm/Analysis/PostDominators.h
@@ -32,6 +32,12 @@
   /// Handle invalidation explicitly.
   bool invalidate(Function &F, const PreservedAnalyses &PA,
                   FunctionAnalysisManager::Invalidator &);
+
+  /// \brief Verify the correctness of the postdomtree by re-computing it.
+  ///
+  /// This should only be used for debugging as it aborts the program if the
+  /// verification fails.
+  void verifyDomTree() const;
 };
 
 /// \brief Analysis pass which computes a \c PostDominatorTree.
@@ -75,6 +81,8 @@
 
   bool runOnFunction(Function &F) override;
 
+  void verifyAnalysis() const override;
+
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41298.130639.patch
Type: text/x-patch
Size: 2601 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180119/443ae20b/attachment.bin>