[PM] Remove the preverifier and directly compute the DominatorTree for

the verifier after ensuring the CFG is at least usefully formed.

This fixes a number of problems:
1) The PreVerifier was missing the controls the Verifier provides over
   *how* an invalid module is handled -- it just aborted the program!
   Now it uses the same logic as the Verifier which is significantly
   more library-friendly.
2) The DominatorTree used previously could have been cached and not
   updated due to bugs in prior passes and we would silently use the
   stale tree. This could cause dominance errors to not be as quickly
   diagnosed.
3) We can now (in the next patch) pull the functionality of the verifier
   apart from the pass infrastructure so that you can verify IR without
   having any form of pass manager. This in turn frees the code to share
   logic between old and new pass manager variants.

Along the way I fixed at least one annoying bug -- the state for
'Broken' wasn't being cleared from run to run causing all functions
visited after the first broken function to be marked as broken
regardless of whether *they* were a problem. Fortunately, I don't really
know much of a way to observe this peculiarity.

In case folks are worried about the runtime cost, its negligible.
I looked at running the entire regression test suite (which should be
a relatively good use of the verifier) before and after but was unable
to even measure the time spent on the verifier and there was no
regresion from before to after. I checked both with debug builds and
optimized builds.

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

diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h
index 2e4e1029373f7899958101d40c8ea34315f28734..a88d94945a821b9fd5eb976233f1db68be46a050 100644 (file)
--- a/include/llvm/InitializePasses.h
+++ b/include/llvm/InitializePasses.h
@@ -211,7 +211,6 @@ void initializePostDomViewerPass(PassRegistry&);
 void initializePostDominatorTreePass(PassRegistry&);
 void initializePostRASchedulerPass(PassRegistry&);
 void initializePostMachineSchedulerPass(PassRegistry&);
-void initializePreVerifierPass(PassRegistry&);
 void initializePrintFunctionPassWrapperPass(PassRegistry&);
 void initializePrintModulePassWrapperPass(PassRegistry&);
 void initializePrintBasicBlockPassPass(PassRegistry&);

diff --git a/lib/IR/Core.cpp b/lib/IR/Core.cpp
index 6e920a2daa4b8a93ace7306c1706e43d837c8ab5..1417e5dc4fd960c909b518eea8eda94efc385845 100644 (file)
--- a/lib/IR/Core.cpp
+++ b/lib/IR/Core.cpp
@@ -45,7 +45,6 @@ void llvm::initializeCore(PassRegistry &Registry) {
   initializePrintFunctionPassWrapperPass(Registry);
   initializePrintBasicBlockPassPass(Registry);
   initializeVerifierPass(Registry);
-  initializePreVerifierPass(Registry);
 }
 
 void LLVMInitializeCore(LLVMPassRegistryRef R) {

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index 002460e53d434e466c3d5436333fe9a8cc36e668..0868ef8fab2fca41dc184237d3a6aa9250f4ab00 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -79,46 +79,6 @@ using namespace llvm;
 static cl::opt<bool> DisableDebugInfoVerifier("disable-debug-info-verifier",
                                               cl::init(true));
 
-namespace {  // Anonymous namespace for class
-  struct PreVerifier : public FunctionPass {
-    static char ID; // Pass ID, replacement for typeid
-
-    PreVerifier() : FunctionPass(ID) {
-      initializePreVerifierPass(*PassRegistry::getPassRegistry());
-    }
-
-    virtual void getAnalysisUsage(AnalysisUsage &AU) const {
-      AU.setPreservesAll();
-    }
-
-    // Check that the prerequisites for successful DominatorTree construction
-    // are satisfied.
-    bool runOnFunction(Function &F) {
-      bool Broken = false;
-
-      for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
-        if (I->empty() || !I->back().isTerminator()) {
-          dbgs() << "Basic Block in function '" << F.getName()
-                 << "' does not have terminator!\n";
-          I->printAsOperand(dbgs(), true);
-          dbgs() << "\n";
-          Broken = true;
-        }
-      }
-
-      if (Broken)
-        report_fatal_error("Broken module, no Basic Block terminator!");
-
-      return false;
-    }
-  };
-}
-
-char PreVerifier::ID = 0;
-INITIALIZE_PASS(PreVerifier, "preverify", "Preliminary module verification",
-                false, false)
-static char &PreVerifyID = PreVerifier::ID;
-
 namespace {
   struct Verifier : public FunctionPass, public InstVisitor<Verifier> {
     static char ID; // Pass ID, replacement for typeid
@@ -127,7 +87,7 @@ namespace {
                           // What to do if verification fails.
     Module *Mod;          // Module we are verifying right now
     LLVMContext *Context; // Context within which we are verifying
-    DominatorTree *DT;    // Dominator Tree, caution can be null!
+    DominatorTree DT;
     const DataLayout *DL;
 
     std::string Messages;
@@ -153,13 +113,13 @@ namespace {
 
     Verifier()
       : FunctionPass(ID), Broken(false),
-        action(AbortProcessAction), Mod(0), Context(0), DT(0), DL(0),
+        action(AbortProcessAction), Mod(0), Context(0), DL(0),
         MessagesStr(Messages), PersonalityFn(0) {
       initializeVerifierPass(*PassRegistry::getPassRegistry());
     }
     explicit Verifier(VerifierFailureAction ctn)
       : FunctionPass(ID), Broken(false), action(ctn), Mod(0),
-        Context(0), DT(0), DL(0), MessagesStr(Messages), PersonalityFn(0) {
+        Context(0), DL(0), MessagesStr(Messages), PersonalityFn(0) {
       initializeVerifierPass(*PassRegistry::getPassRegistry());
     }
 
@@ -175,8 +135,27 @@ namespace {
     }
 
     bool runOnFunction(Function &F) {
-      // Get dominator information if we are being run by PassManager
-      DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
+      Broken = false;
+
+      // First ensure the function is well-enough formed to compute dominance
+      // information.
+      for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
+        if (I->empty() || !I->back().isTerminator()) {
+          dbgs() << "Basic Block in function '" << F.getName()
+                 << "' does not have terminator!\n";
+          I->printAsOperand(dbgs(), true);
+          dbgs() << "\n";
+          Broken = true;
+        }
+      }
+      if (Broken)
+        return abortIfBroken();
+
+      // Now directly compute a dominance tree. We don't rely on the pass
+      // manager to provide this as it isolates us from a potentially
+      // out-of-date dominator tree and makes it significantly more complex to
+      // run this code outside of a pass manager.
+      DT.recalculate(F);
 
       Mod = F.getParent();
       if (!Context) Context = &F.getContext();
@@ -232,8 +211,6 @@ namespace {
 
     virtual void getAnalysisUsage(AnalysisUsage &AU) const {
       AU.setPreservesAll();
-      AU.addRequiredID(PreVerifyID);
-      AU.addRequired<DominatorTreeWrapperPass>();
     }
 
     /// abortIfBroken - If the module is broken and we are supposed to abort on
@@ -393,10 +370,7 @@ namespace {
 } // End anonymous namespace
 
 char Verifier::ID = 0;
-INITIALIZE_PASS_BEGIN(Verifier, "verify", "Module Verifier", false, false)
-INITIALIZE_PASS_DEPENDENCY(PreVerifier)
-INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_END(Verifier, "verify", "Module Verifier", false, false)
+INITIALIZE_PASS(Verifier, "verify", "Module Verifier", false, false)
 
 // Assert - We know that cond should be true, if not print an error message.
 #define Assert(C, M) \
@@ -2026,7 +2000,7 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
   }
 
   const Use &U = I.getOperandUse(i);
-  Assert2(InstsInThisBlock.count(Op) || DT->dominates(Op, U),
+  Assert2(InstsInThisBlock.count(Op) || DT.dominates(Op, U),
           "Instruction does not dominate all uses!", Op, &I);
 }
 
@@ -2039,7 +2013,7 @@ void Verifier::visitInstruction(Instruction &I) {
   if (!isa<PHINode>(I)) {   // Check that non-phi nodes are not self referential
     for (Value::use_iterator UI = I.use_begin(), UE = I.use_end();
          UI != UE; ++UI)
-      Assert1(*UI != (User*)&I || !DT->isReachableFromEntry(BB),
+      Assert1(*UI != (User*)&I || !DT.isReachableFromEntry(BB),
               "Only PHI nodes may reference their own value!", &I);
   }