Move the dominator verification code out of special code embedded within

the PassManager code into a regular verifyAnalysis method.

Also, reorganize loop verification. Make the LoopPass infrastructure
call verifyLoop as needed instead of having LoopInfo::verifyAnalysis
check every loop in the function after each looop pass. Add a new
command-line argument, -verify-loop-info, to enable the expensive
full checking.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@82952 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Analysis/Dominators.h b/include/llvm/Analysis/Dominators.h
index e8aceaa6226fde2d5d03c9b6ff4958e4566f565a..f63e31c36bab3e69c7d21dbf3879071418111605 100644 (file)
--- a/include/llvm/Analysis/Dominators.h
+++ b/include/llvm/Analysis/Dominators.h
@@ -734,6 +734,8 @@ public:
 
   virtual bool runOnFunction(Function &F);
 
+  virtual void verifyAnalysis() const;
+
   virtual void getAnalysisUsage(AnalysisUsage &AU) const {
     AU.setPreservesAll();
   }
@@ -991,6 +993,8 @@ public:
     return false;
   }
 
+  virtual void verifyAnalysis() const;
+
   virtual void getAnalysisUsage(AnalysisUsage &AU) const {
     AU.setPreservesAll();
     AU.addRequired<DominatorTree>();

diff --git a/include/llvm/PassManagers.h b/include/llvm/PassManagers.h
index a067294e9fa4748668a641ed942b43f18f3ab8fe..5a8f55570a822034e37d0b7d1440b0802c4b1d42 100644 (file)
--- a/include/llvm/PassManagers.h
+++ b/include/llvm/PassManagers.h
@@ -280,9 +280,6 @@ public:
   /// verifyPreservedAnalysis -- Verify analysis presreved by pass P.
   void verifyPreservedAnalysis(Pass *P);
 
-  /// verifyDomInfo -- Verify dominator information if it is available.
-  void verifyDomInfo(Pass &P, Function &F);
-
   /// Remove Analysis that is not preserved by the pass
   void removeNotPreservedAnalysis(Pass *P);
   

diff --git a/lib/Analysis/LoopInfo.cpp b/lib/Analysis/LoopInfo.cpp
index 665e53df6dcc1c46797d774206658e050e8ca0ef..ce2d29f331b6b40073d215fe14b74ee2803a7e5f 100644 (file)
--- a/lib/Analysis/LoopInfo.cpp
+++ b/lib/Analysis/LoopInfo.cpp
@@ -20,11 +20,22 @@
 #include "llvm/Analysis/Dominators.h"
 #include "llvm/Assembly/Writer.h"
 #include "llvm/Support/CFG.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include <algorithm>
 using namespace llvm;
 
+// Always verify loopinfo if expensive checking is enabled.
+#ifdef XDEBUG
+bool VerifyLoopInfo = true;
+#else
+bool VerifyLoopInfo = false;
+#endif
+static cl::opt<bool,true>
+VerifyLoopInfoX("verify-loop-info", cl::location(VerifyLoopInfo),
+                cl::desc("Verify loop info (time consuming)"));
+
 char LoopInfo::ID = 0;
 static RegisterPass<LoopInfo>
 X("loops", "Natural Loop Information", true, true);
@@ -375,10 +386,20 @@ bool LoopInfo::runOnFunction(Function &) {
 }
 
 void LoopInfo::verifyAnalysis() const {
+  // LoopInfo is a FunctionPass, but verifying every loop in the function
+  // each time verifyAnalysis is called is very expensive. The
+  // -verify-loop-info option can enable this. In order to perform some
+  // checking by default, LoopPass has been taught to call verifyLoop
+  // manually during loop pass sequences.
+
+  if (!VerifyLoopInfo) return;
+
   for (iterator I = begin(), E = end(); I != E; ++I) {
     assert(!(*I)->getParentLoop() && "Top-level loop has a parent!");
     (*I)->verifyLoopNest();
   }
+
+  // TODO: check BBMap consistency.
 }
 
 void LoopInfo::getAnalysisUsage(AnalysisUsage &AU) const {

diff --git a/lib/Analysis/LoopPass.cpp b/lib/Analysis/LoopPass.cpp
index e297ab420731f7976c5aad9270603c710a2ed205..22466ae3d061f1f563f0fa1a413c110b3af22463 100644 (file)
--- a/lib/Analysis/LoopPass.cpp
+++ b/lib/Analysis/LoopPass.cpp
@@ -242,16 +242,24 @@ bool LPPassManager::runOnFunction(Function &F) {
         dumpPassInfo(P, MODIFICATION_MSG, ON_LOOP_MSG, "");
       dumpPreservedSet(P);
 
-      if (!skipThisLoop)
+      if (!skipThisLoop) {
+        // Manually check that this loop is still healthy. This is done
+        // instead of relying on LoopInfo::verifyLoop since LoopInfo
+        // is a function pass and it's really expensive to verify every
+        // loop in the function every time. That level of checking can be
+        // enabled with the -verify-loop-info option.
+        Timer *T = StartPassTimer(LI);
+        CurrentLoop->verifyLoop();
+        StopPassTimer(LI, T);
+
+        // Then call the regular verifyAnalysis functions.
         verifyPreservedAnalysis(LP);
+      }
 
       removeNotPreservedAnalysis(P);
       recordAvailableAnalysis(P);
       removeDeadPasses(P, "", ON_LOOP_MSG);
 
-      // If dominator information is available then verify the info if requested.
-      verifyDomInfo(*LP, F);
-
       if (skipThisLoop)
         // Do not run other passes on this loop.
         break;

diff --git a/lib/VMCore/Dominators.cpp b/lib/VMCore/Dominators.cpp
index 1f172145c3b1ff6518a5b5f2f56ca6844d763c07..40e4c4bc4c8d24226d90506925b94ce5a304f4b0 100644 (file)
--- a/lib/VMCore/Dominators.cpp
+++ b/lib/VMCore/Dominators.cpp
@@ -24,9 +24,20 @@
 #include "llvm/Analysis/DominatorInternals.h"
 #include "llvm/Instructions.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/CommandLine.h"
 #include <algorithm>
 using namespace llvm;
 
+// Always verify dominfo if expensive checking is enabled.
+#ifdef XDEBUG
+bool VerifyDomInfo = true;
+#else
+bool VerifyDomInfo = false;
+#endif
+static cl::opt<bool,true>
+VerifyDomInfoX("verify-dom-info", cl::location(VerifyDomInfo),
+               cl::desc("Verify dominator info (time consuming)"));
+
 //===----------------------------------------------------------------------===//
 //  DominatorTree Implementation
 //===----------------------------------------------------------------------===//
@@ -48,6 +59,16 @@ bool DominatorTree::runOnFunction(Function &F) {
   return false;
 }
 
+void DominatorTree::verifyAnalysis() const {
+  if (!VerifyDomInfo || true /* fixme */) return;
+
+  Function &F = *getRoot()->getParent();
+
+  DominatorTree OtherDT;
+  OtherDT.getBase().recalculate(F);
+  assert(!compare(OtherDT) && "Invalid DominatorTree info!");
+}
+
 void DominatorTree::print(raw_ostream &OS, const Module *) const {
   DT->print(OS);
 }
@@ -87,6 +108,17 @@ char DominanceFrontier::ID = 0;
 static RegisterPass<DominanceFrontier>
 G("domfrontier", "Dominance Frontier Construction", true, true);
 
+void DominanceFrontier::verifyAnalysis() const {
+  if (!VerifyDomInfo) return;
+
+  DominatorTree &DT = getAnalysis<DominatorTree>();
+
+  DominanceFrontier OtherDF;
+  const std::vector<BasicBlock*> &DTRoots = DT.getRoots();
+  OtherDF.calculate(DT, DT.getNode(DTRoots[0]));
+  assert(!compare(OtherDF) && "Invalid DominanceFrontier info!");
+}
+
 // NewBB is split and now it has one successor. Update dominace frontier to
 // reflect this change.
 void DominanceFrontier::splitBlock(BasicBlock *NewBB) {

diff --git a/lib/VMCore/PassManager.cpp b/lib/VMCore/PassManager.cpp
index a3496ed9b4f93629021a849810a38f58efc85b1a..f10bc6f5ef6f7744c367cf056a9c841b04873405 100644 (file)
--- a/lib/VMCore/PassManager.cpp
+++ b/lib/VMCore/PassManager.cpp
@@ -13,6 +13,7 @@
 
 
 #include "llvm/PassManagers.h"
+#include "llvm/Assembly/Writer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Module.h"
@@ -22,7 +23,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/System/Mutex.h"
 #include "llvm/System/Threading.h"
-#include "llvm/Analysis/Dominators.h"
 #include "llvm-c/Core.h"
 #include <algorithm>
 #include <cstdio>
@@ -45,16 +45,6 @@ enum PassDebugLevel {
   None, Arguments, Structure, Executions, Details
 };
 
-// Always verify dominfo if expensive checking is enabled.
-#ifdef XDEBUG
-bool VerifyDomInfo = true;
-#else
-bool VerifyDomInfo = false;
-#endif
-static cl::opt<bool,true>
-VerifyDomInfoX("verify-dom-info", cl::location(VerifyDomInfo),
-               cl::desc("Verify dominator info (time consuming)"));
-
 static cl::opt<enum PassDebugLevel>
 PassDebugging("debug-pass", cl::Hidden,
                   cl::desc("Print PassManager debugging information"),
@@ -703,47 +693,13 @@ void PMDataManager::verifyPreservedAnalysis(Pass *P) {
   for (AnalysisUsage::VectorType::const_iterator I = PreservedSet.begin(),
          E = PreservedSet.end(); I != E; ++I) {
     AnalysisID AID = *I;
-    if (Pass *AP = findAnalysisPass(AID, true))
-      AP->verifyAnalysis();
-  }
-}
-
-/// verifyDomInfo - Verify dominator information if it is available.
-void PMDataManager::verifyDomInfo(Pass &P, Function &F) {
-  if (!VerifyDomInfo || !P.getResolver())
-    return;
+    if (Pass *AP = findAnalysisPass(AID, true)) {
 
-  DominatorTree *DT = P.getAnalysisIfAvailable<DominatorTree>();
-  if (!DT)
-    return;
-
-  DominatorTree OtherDT;
-  OtherDT.getBase().recalculate(F);
-  if (DT->compare(OtherDT)) {
-    errs() << "Dominator Information for " << F.getName() << "\n";
-    errs() << "Pass '" << P.getPassName() << "'\n";
-    errs() << "----- Valid -----\n";
-    OtherDT.dump();
-    errs() << "----- Invalid -----\n";
-    DT->dump();
-    llvm_unreachable("Invalid dominator info");
-  }
-
-  DominanceFrontier *DF = P.getAnalysisIfAvailable<DominanceFrontier>();
-  if (!DF) 
-    return;
-
-  DominanceFrontier OtherDF;
-  std::vector<BasicBlock*> DTRoots = DT->getRoots();
-  OtherDF.calculate(*DT, DT->getNode(DTRoots[0]));
-  if (DF->compare(OtherDF)) {
-    errs() << "Dominator Information for " << F.getName() << "\n";
-    errs() << "Pass '" << P.getPassName() << "'\n";
-    errs() << "----- Valid -----\n";
-    OtherDF.dump();
-    errs() << "----- Invalid -----\n";
-    DF->dump();
-    llvm_unreachable("Invalid dominator info");
+      Timer *T = 0;
+      if (TheTimeInfo) T = TheTimeInfo->passStarted(AP);
+      AP->verifyAnalysis();
+      if (T) T->stopTimer();
+    }
   }
 }
 
@@ -1384,9 +1340,6 @@ bool FPPassManager::runOnFunction(Function &F) {
     removeNotPreservedAnalysis(FP);
     recordAvailableAnalysis(FP);
     removeDeadPasses(FP, F.getName(), ON_FUNCTION_MSG);
-
-    // If dominator information is available then verify the info if requested.
-    verifyDomInfo(*FP, F);
   }
   return Changed;
 }

diff --git a/test/DebugInfo/2009-01-15-RecordVariableCrash.ll b/test/DebugInfo/2009-01-15-RecordVariableCrash.ll
index 42fb54ce7c8fb9af77dcb182e0092e09cc18a4fd..cee4d724bf4beeebf82f3d8a498648f4dd974d13 100644 (file)
--- a/test/DebugInfo/2009-01-15-RecordVariableCrash.ll
+++ b/test/DebugInfo/2009-01-15-RecordVariableCrash.ll
@@ -1,4 +1,4 @@
-; RUN: llc %s -o /dev/null -verify-dom-info
+; RUN: llc %s -o /dev/null -verify-dom-info -verify-loop-info
        %llvm.dbg.anchor.type = type { i32, i32 }
        %llvm.dbg.basictype.type = type { i32, { }*, i8*, { }*, i32, i64, i64, i64, i32, i32, i8*, i8* }
        %llvm.dbg.compile_unit.type = type { i32, { }*, i32, i8*, i8*, i8* }

diff --git a/test/Transforms/LCSSA/2006-10-31-UnreachableBlock-2.ll b/test/Transforms/LCSSA/2006-10-31-UnreachableBlock-2.ll
index 6f583f47ce89230c69af9e9da22dda26472f5a6e..3ba8d18b00cf79b60d1f20d3ecebfb0a5a7f9818 100644 (file)
--- a/test/Transforms/LCSSA/2006-10-31-UnreachableBlock-2.ll
+++ b/test/Transforms/LCSSA/2006-10-31-UnreachableBlock-2.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -lcssa -disable-output -verify-dom-info
+; RUN: opt < %s -lcssa -disable-output -verify-dom-info -verify-loop-info
 ; PR977
 ; END.
 declare i32 @opost_block()

diff --git a/test/Transforms/LoopSimplify/2004-04-13-LoopSimplifyUpdateDomFrontier.ll b/test/Transforms/LoopSimplify/2004-04-13-LoopSimplifyUpdateDomFrontier.ll
index f4ecbea3ab075739628a765b535391b6b929a8b4..4fe6e2156f972652d797fda8255a759976427f38 100644 (file)
--- a/test/Transforms/LoopSimplify/2004-04-13-LoopSimplifyUpdateDomFrontier.ll
+++ b/test/Transforms/LoopSimplify/2004-04-13-LoopSimplifyUpdateDomFrontier.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -scalarrepl -loopsimplify -licm -disable-output -verify-dom-info
+; RUN: opt < %s -scalarrepl -loopsimplify -licm -disable-output -verify-dom-info -verify-loop-info
 
 define void @inflate() {
 entry:

diff --git a/test/Transforms/LoopUnswitch/preserve-analyses.ll b/test/Transforms/LoopUnswitch/preserve-analyses.ll
index 8d3f1382f3cc3f5cbee0b526a10acf7c700bcbbb..3364fb274109642e074a036ac54d48fa3ac28812 100644 (file)
--- a/test/Transforms/LoopUnswitch/preserve-analyses.ll
+++ b/test/Transforms/LoopUnswitch/preserve-analyses.ll
@@ -1,4 +1,4 @@
-; RUN: opt -loop-unswitch %s -disable-output
+; RUN: opt -loop-unswitch -verify-loop-info -verify-dom-info %s -disable-output
 
 ; Loop unswitch should be able to unswitch these loops and
 ; preserve LCSSA and LoopSimplify forms.