[PATCH] D43113: [bugpoint] Don't stop reducing when verification fails, keep trying

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: davide, MatzeB.

Instead of calling exit() when a verification step fails we should keep
reducing the input. This is in line with what we do in other cases when
opt fails (see performFinalCleanups(), extractLoop(), etc).

I have a test case that provides coverage for this change, but it's
large and (for now) relies on an uncommitted assertion in SelectionDAG.

I don't think it's worthwhile to test this change in-tree because a
bugfix to a bugpoint reducer could break the test. (It would be fair to
say that the real problem I'm having is that some reducer is buggy, and
that I could just try to fix that instead, but unless we instantly fix
all reducer bugs having this "keep going" behavior sgtm. Thoughts?)


https://reviews.llvm.org/D43113

Files:
  tools/bugpoint/BugDriver.h
  tools/bugpoint/CrashDebugger.cpp
  tools/bugpoint/ExtractFunction.cpp


Index: tools/bugpoint/ExtractFunction.cpp
===================================================================
--- tools/bugpoint/ExtractFunction.cpp
+++ tools/bugpoint/ExtractFunction.cpp
@@ -121,7 +121,7 @@
   std::unique_ptr<Module> New = runPassesOn(Clone.get(), Passes);
   if (!New) {
     errs() << "Instruction removal failed.  Sorry. :(  Please report a bug!\n";
-    exit(1);
+    return nullptr;
   }
   return New;
 }
Index: tools/bugpoint/CrashDebugger.cpp
===================================================================
--- tools/bugpoint/CrashDebugger.cpp
+++ tools/bugpoint/CrashDebugger.cpp
@@ -451,7 +451,7 @@
   std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
   if (!New) {
     errs() << "verify failed!\n";
-    exit(1);
+    return false;
   }
   M = std::move(New);
 
@@ -558,7 +558,7 @@
   std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
   if (!New) {
     errs() << "verify failed!\n";
-    exit(1);
+    return false;
   }
   M = std::move(New);
 
@@ -650,7 +650,7 @@
   std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
   if (!New) {
     errs() << "verify failed!\n";
-    exit(1);
+    return false;
   }
   M = std::move(New);
 
@@ -1010,6 +1010,8 @@
               outs() << "Checking instruction: " << *I;
               std::unique_ptr<Module> M =
                   BD.deleteInstructionFromProgram(&*I, Simplification);
+              if (!M) // We couldn't delete this instruction. Keep trying.
+                continue;
 
               // Find out if the pass still crashes on this pass...
               if (TestFn(BD, M.get())) {
Index: tools/bugpoint/BugDriver.h
===================================================================
--- tools/bugpoint/BugDriver.h
+++ tools/bugpoint/BugDriver.h
@@ -200,7 +200,8 @@
   /// This method clones the current Program and deletes the specified
   /// instruction from the cloned module.  It then runs a series of cleanup
   /// passes (ADCE and SimplifyCFG) to eliminate any code which depends on the
-  /// value. The modified module is then returned.
+  /// value. The modified module is then returned. If deletion causes module
+  /// verification to fail, nullptr is returned.
   ///
   std::unique_ptr<Module> deleteInstructionFromProgram(const Instruction *I,
                                                        unsigned Simp);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43113.133560.patch
Type: text/x-patch
Size: 2369 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/dc73102f/attachment.bin>