[bugpoint] Simplify the global initializer reducer, NFC

[vsk]

Summary

Fix the comments, use early exits, use unique_ptr, and use ranged for
loops.

This is in preparation for a global *variable* reducer, which, with any
luck will help us clean up test cases.

[MatzeB]

LGTM
...and except maybe for the renaming of the class this all looks like trivial changes that are fine with post-commit review.

llvm/trunk/tools/bugpoint/CrashDebugger.cpp

if (OrigProgram) {
     BD.Program = OrigProgram;
   }
   return NoFailure;
 }
 
 using BugTester = bool (*)(const BugDriver &, Module *);
 
 namespace {
-/// ReduceCrashingGlobalVariables - This works by removing the global
-/// variable's initializer and seeing if the program still crashes. If it
-/// does, then we keep that program and try again.
-///
-class ReduceCrashingGlobalVariables : public ListReducer<GlobalVariable *> {
+/// ReduceCrashingGlobalInitializers - This works by removing global variable
+/// initializers and seeing if the program still crashes. If it does, then we
+/// keep that program and try again.
+class ReduceCrashingGlobalInitializers : public ListReducer<GlobalVariable *> {
   BugDriver &BD;
   BugTester TestFn;
 
 public:
-  ReduceCrashingGlobalVariables(BugDriver &bd, BugTester testFn)
+  ReduceCrashingGlobalInitializers(BugDriver &bd, BugTester testFn)
       : BD(bd), TestFn(testFn) {}
 
   Expected<TestResult> doTest(std::vector<GlobalVariable *> &Prefix,
                               std::vector<GlobalVariable *> &Kept) override {
     if (!Kept.empty() && TestGlobalVariables(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestGlobalVariables(Prefix))
       return KeepPrefix;
     return NoFailure;
   }
 
   bool TestGlobalVariables(std::vector<GlobalVariable *> &GVs);
 };
 }
 
-bool ReduceCrashingGlobalVariables::TestGlobalVariables(
+bool ReduceCrashingGlobalInitializers::TestGlobalVariables(
     std::vector<GlobalVariable *> &GVs) {
   // Clone the program to try hacking it apart...
   ValueToValueMapTy VMap;
-  Module *M = CloneModule(BD.getProgram(), VMap).release();
+  std::unique_ptr<Module> M = CloneModule(BD.getProgram(), VMap);
 
   // Convert list to set for fast lookup...
   std::set<GlobalVariable *> GVSet;
 
   for (unsigned i = 0, e = GVs.size(); i != e; ++i) {
     GlobalVariable *CMGV = cast<GlobalVariable>(VMap[GVs[i]]);
     assert(CMGV && "Global Variable not in module?!");
     GVSet.insert(CMGV);
   }
 
   outs() << "Checking for crash with only these global variables: ";
   PrintGlobalVariableList(GVs);
   outs() << ": ";
 
   // Loop over and delete any global variables which we aren't supposed to be
   // playing with...
   for (GlobalVariable &I : M->globals())
     if (I.hasInitializer() && !GVSet.count(&I)) {
       DeleteGlobalInitializer(&I);
       I.setLinkage(GlobalValue::ExternalLinkage);
       I.setComdat(nullptr);
     }
 
   // Try running the hacked up program...
-  if (TestFn(BD, M)) {
-    BD.setNewProgram(M); // It crashed, keep the trimmed version...
+  if (TestFn(BD, M.get())) {
+    BD.setNewProgram(M.release()); // It crashed, keep the trimmed version...
 
     // Make sure to use global variable pointers that point into the now-current
     // module.
     GVs.assign(GVSet.begin(), GVSet.end());
     return true;
   }
 
-  delete M;
   return false;
 }
 
 namespace {
 /// ReduceCrashingFunctions reducer - This works by removing functions and
 /// seeing if the program still crashes. If it does, then keep the newer,
 /// smaller program.
 ///
if (TestFn(BD, M)) {
 
     BD.setNewProgram(M); // It crashed, keep the trimmed version...
     return true;
   }
   delete M; // It didn't crash, try something else.
   return false;
 }
 
+/// Attempt to eliminate as many global initializers as possible.
 static Error ReduceGlobalInitializers(BugDriver &BD, BugTester TestFn) {
-  if (BD.getProgram()->global_begin() != BD.getProgram()->global_end()) {
-    // Now try to reduce the number of global variable initializers in the
-    // module to something small.
-    Module *M = CloneModule(BD.getProgram()).release();
-    bool DeletedInit = false;
+  Module *OrigM = BD.getProgram();
+  if (OrigM->global_empty())
+    return Error::success();
+
+  std::unique_ptr<Module> M = CloneModule(OrigM);
 
-    for (Module::global_iterator I = M->global_begin(), E = M->global_end();
-         I != E; ++I)
-      if (I->hasInitializer()) {
-        DeleteGlobalInitializer(&*I);
-        I->setLinkage(GlobalValue::ExternalLinkage);
-        I->setComdat(nullptr);
-        DeletedInit = true;
-      }
+  for (GlobalVariable &GV : M->globals()) {
+    if (GV.hasInitializer()) {
+      DeleteGlobalInitializer(&GV);
+      GV.setLinkage(GlobalValue::ExternalLinkage);
+      GV.setComdat(nullptr);
+  }
+
+  if (!DeletedInit)
+    return Error::success();
 
-    if (!DeletedInit) {
-      delete M; // No change made...
-    } else {
-      // See if the program still causes a crash...
-      outs() << "\nChecking to see if we can delete global inits: ";
 
-      if (TestFn(BD, M)) { // Still crashes?
-        BD.setNewProgram(M);
-        outs() << "\n*** Able to remove all global initializers!\n";
-      } else { // No longer crashes?
-        outs() << "  - Removing all global inits hides problem!\n";
-        delete M;
+  if (TestFn(BD, M.get())) { // Still crashes?
+    BD.setNewProgram(M.release());
+    return Error::success();
+  }
+
+  // No longer crashes.
 
-        std::vector<GlobalVariable *> GVs;
-
-        for (Module::global_iterator I = BD.getProgram()->global_begin(),
-                                     E = BD.getProgram()->global_end();
-             I != E; ++I)
-          if (I->hasInitializer())
-            GVs.push_back(&*I);
+  for (GlobalVariable &GV : OrigM->globals())
+    if (GV.hasInitializer())
+      GVs.push_back(&GV);
 
-        if (GVs.size() > 1 && !BugpointIsInterrupted) {
-          outs() << "\n*** Attempting to reduce the number of global "
-                 << "variables in the testcase\n";
+    outs() << "\n*** Attempting to reduce the number of global initializers "
+           << "in the testcase\n";
 
-          unsigned OldSize = GVs.size();
-          Expected<bool> Result =
-              ReduceCrashingGlobalVariables(BD, TestFn).reduceList(GVs);
-          if (Error E = Result.takeError())
-            return E;
+        ReduceCrashingGlobalInitializers(BD, TestFn).reduceList(GVs);
 
-          if (GVs.size() < OldSize)
-            BD.EmitProgressBitcode(BD.getProgram(), "reduced-global-variables");
-        }
-      }
-    }
-  }
   return Error::success();
 }
 
 static Error ReduceInsts(BugDriver &BD, BugTester TestFn) {
   // Attempt to delete instructions using bisection. This should help out nasty
   // cases with large basic blocks where the problem is at one end.
   if (!BugpointIsInterrupted) {
     std::vector<const Instruction *> Insts;