[PATCH] D80106: Fix several places that were calling verifyFunction or verifyModule without checking the return value.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: efriedma, jdoerfert, rjmccall.
Herald added subscribers: modocache, hiraditya.
Herald added a project: LLVM.

verifyFunction/verifyModule don't assert or error internally. They
also don't print anything if you don't pass a raw_ostream to them.
So the caller needs to check the result and ideally pass a stream
to get the messages. Otherwise they're just really expensive no-ops.

I've replaced calls inside LLVM_DEBUG with assert. The call in
CoroSplit should probably be in an assert, but I don't know
enough about the coroutine pipeline health.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80106

Files:
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/tools/llvm-as-fuzzer/llvm-as-fuzzer.cpp
  llvm/tools/llvm-split/llvm-split.cpp


Index: llvm/tools/llvm-split/llvm-split.cpp
===================================================================
--- llvm/tools/llvm-split/llvm-split.cpp
+++ llvm/tools/llvm-split/llvm-split.cpp
@@ -61,7 +61,11 @@
       exit(1);
     }
 
-    verifyModule(*MPart);
+    if (verifyModule(*MPart, &errs())) {
+      errs() << "Broken module!\n";
+      exit(1);
+    }
+
     WriteBitcodeToFile(*MPart, Out->os());
 
     // Declare success.
Index: llvm/tools/llvm-as-fuzzer/llvm-as-fuzzer.cpp
===================================================================
--- llvm/tools/llvm-as-fuzzer/llvm-as-fuzzer.cpp
+++ llvm/tools/llvm-as-fuzzer/llvm-as-fuzzer.cpp
@@ -70,6 +70,7 @@
   if (!M.get())
     return 0;
 
-  verifyModule(*M.get());
+  if (verifyModule(*M.get(), &errs()))
+    report_fatal_error("Broken module");
   return 0;
 }
Index: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7669,7 +7669,7 @@
   // Mark the loop as already vectorized to avoid vectorizing again.
   Hints.setAlreadyVectorized();
 
-  LLVM_DEBUG(verifyFunction(*L->getHeader()->getParent()));
+  assert(!verifyFunction(*L->getHeader()->getParent(), &dbgs()));
   return true;
 }
 
@@ -7971,7 +7971,7 @@
     Hints.setAlreadyVectorized();
   }
 
-  LLVM_DEBUG(verifyFunction(*L->getHeader()->getParent()));
+  assert(!verifyFunction(*L->getHeader()->getParent()));
   return true;
 }
 
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===================================================================
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -894,7 +894,8 @@
   // For now, we do a mandatory verification step because we don't
   // entirely trust this pass.  Note that we don't want to add a verifier
   // pass to FPM below because it will also verify all the global data.
-  verifyFunction(F);
+  if (verifyFunction(F, &errs()))
+    report_fatal_error("Broken function");
 
   legacy::FunctionPassManager FPM(F.getParent());
 
Index: llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
===================================================================
--- llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
+++ llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
@@ -1292,7 +1292,8 @@
 
 #ifdef EXPENSIVE_CHECKS
   // Run this only when expensive checks are enabled.
-  verifyFunction(F);
+  if (verifyFunction(F, &dbgs()))
+    report_fatal_error("Broken function");
 #endif
   return true;
 }
Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -1071,10 +1071,10 @@
                                 DemoteCatchSwitchPHIOnlyOpt);
 
   if (!DisableCleanups) {
-    LLVM_DEBUG(verifyFunction(F));
+    assert(!verifyFunction(F, &dbgs()));
     removeImplausibleInstructions(F);
 
-    LLVM_DEBUG(verifyFunction(F));
+    assert(!verifyFunction(F, &dbgs()));
     cleanupPreparedFunclets(F);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80106.264545.patch
Type: text/x-patch
Size: 3140 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200518/e4f01359/attachment.bin>