[PATCH] D21042: Verifier: Simplify and fix issue where we were not verifying unmaterialized functions.

[Peter Collingbourne via llvm-commits]

pcc created this revision.
pcc added a reviewer: aprantl.
pcc added a subscriber: llvm-commits.

- Arrange to call verify(Function &) on each function, followed by
  verify(Module &), whether the verifier is being used from the pass or from
  verifyModule(). As a side effect, this fixes an issue that caused us not
  to call verify(Function &) on unmaterialized functions from verifyModule().

- Remove previously unreachable code that verifies that a function definition
  has an entry block. By definition, a function definition has at least one block.

http://reviews.llvm.org/D21042

Files:
  lib/IR/Verifier.cpp

Index: lib/IR/Verifier.cpp
===================================================================
--- lib/IR/Verifier.cpp
+++ lib/IR/Verifier.cpp
@@ -276,13 +276,14 @@
     Context = &M->getContext();
 
     // First ensure the function is well-enough formed to compute dominance
-    // information.
-    if (F.empty()) {
-      if (OS)
-        *OS << "Function '" << F.getName()
-            << "' does not contain an entry block!\n";
-      return false;
-    }
+    // information, and 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.
+    // FIXME: It's really gross that we have to cast away constness here.
+    if (!F.empty())
+      DT.recalculate(const_cast<Function &>(F));
+
     for (const BasicBlock &BB : F) {
       if (!BB.empty() && BB.back().isTerminator())
         continue;
@@ -296,13 +297,6 @@
       return false;
     }
 
-    // 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.
-    // FIXME: It's really gross that we have to cast away constness here.
-    DT.recalculate(const_cast<Function &>(F));
-
     Broken = false;
     // FIXME: We strip const here because the inst visitor strips const.
     visit(const_cast<Function &>(F));
@@ -320,17 +314,10 @@
     Context = &M.getContext();
     Broken = false;
 
-    // Scan through, checking all of the external function's linkage now...
-    for (const Function &F : M) {
-      visitGlobalValue(F);
-
-      // Check to make sure function prototypes are okay.
-      if (F.isDeclaration()) {
-        visitFunction(F);
-        if (F.getIntrinsicID() == Intrinsic::experimental_deoptimize)
-          DeoptimizeDeclarations.push_back(&F);
-      }
-    }
+    // Collect all declarations of the llvm.experimental.deoptimize intrinsic.
+    for (const Function &F : M)
+      if (F.getIntrinsicID() == Intrinsic::experimental_deoptimize)
+        DeoptimizeDeclarations.push_back(&F);
 
     // Now that we've visited every function, verify that we never asked to
     // recover a frame index that wasn't escaped.
@@ -1872,6 +1859,8 @@
 // visitFunction - Verify that a function is ok.
 //
 void Verifier::visitFunction(const Function &F) {
+  visitGlobalValue(F);
+
   // Check function arguments.
   FunctionType *FT = F.getFunctionType();
   unsigned NumArgs = F.arg_size();
@@ -4450,8 +4439,7 @@
 
   bool Broken = false;
   for (const Function &F : M)
-    if (!F.isDeclaration() && !F.isMaterializable())
-      Broken |= !V.verify(F);
+    Broken |= !V.verify(F);
 
   Broken |= !V.verify(M);
   if (BrokenDebugInfo)
@@ -4488,7 +4476,12 @@
   }
 
   bool doFinalization(Module &M) override {
-    bool HasErrors = !V.verify(M);
+    bool HasErrors = false;
+    for (Function &F : M)
+      if (F.isDeclaration())
+        HasErrors |= !V.verify(F);
+
+    HasErrors |= !V.verify(M);
     if (FatalErrors) {
       if (HasErrors)
         report_fatal_error("Broken module found, compilation aborted!");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21042.59786.patch
Type: text/x-patch
Size: 3310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160606/b927cbda/attachment.bin>