llvm-reduce: Don't delete instructions in global variable reduction

[arsenm]

Summary

For some reason the global variable reduction was trying to delete
use instructions. This broke the verifier if the user was a terminator,
since the block now no longer has one. It doesn't make sense for this
reduction to delete the users, so just stop doing that.

[regehr]

LGTM. I agree this is a good change, delta passes shouldn't try to be too clever.

[arsenm]

a50cec5bf9ede33881988e4b9c3bb9b5ee5f7178

llvm/test/tools/llvm-reduce/reduce-global-variable-terminator-user.ll

+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=global-variables --test FileCheck --test-arg --check-prefixes=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefixes=RESULT %s < %t
+
+; The global variable reduction was trying to delete use instructions
+; of globals for some reason, and breaking the basic blocks that had
+; global uses in the terminator
+
+; RESULT-NOT: @zed
+@zed = global i32 0
+
+; INTERESTING: @bar
+; RESULT: @bar
+@bar = global i32 1
+
+; RESULT: define ptr @zed_user() {
+; RESULT-NEXT: ret ptr null
+define ptr @zed_user() {
+  ret ptr @zed
+}

llvm/test/tools/llvm-reduce/remove-global-vars.ll

 ; Test that llvm-reduce can remove uninteresting Global Variables as well as
 ; their direct uses (which in turn are replaced with '0').
 
-; RUN: llvm-reduce --delta-passes=global-variables,global-initializers --test FileCheck --test-arg --check-prefixes=CHECK-ALL,CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
-; RUN: cat %t | FileCheck --check-prefixes=CHECK-ALL,CHECK-FINAL --implicit-check-not=uninteresting %s
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=global-variables,global-initializers --test FileCheck --test-arg --check-prefixes=CHECK-ALL,CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefixes=CHECK-ALL,CHECK-FINAL --implicit-check-not=uninteresting %s < %t
 
 $interesting5 = comdat any
 
 ; CHECK-INTERESTINGNESS: @interesting = {{.*}}global i32{{.*}}, align 4
 ; CHECK-INTERESTINGNESS: @interesting2 = global i32 0, align 4
 ; CHECK-INTERESTINGNESS: @interesting3 = {{.*}}global i32{{.*}}, align 4
 ; CHECK-INTERESTINGNESS: @interesting4 = {{.*}}constant i32{{.*}}, align 4
 ; CHECK-INTERESTINGNESS: @interesting5 = {{.*}}global i32{{.*}}, align 4

 @uninteresting = global i32 1, align 4
 @uninteresting2 = external global i32, align 4
 
 define i32 @main() {
 entry:
   %0 = load i32, ptr @uninteresting, align 4
 
   ; CHECK-INTERESTINGNESS: store i32 {{.*}}, ptr @interesting, align 4
-  ; CHECK-FINAL: store i32 0, ptr @interesting, align 4
+  ; CHECK-FINAL: store i32 %0, ptr @interesting, align 4
   store i32 %0, ptr @interesting, align 4
 
   ; CHECK-INTERESTINGNESS: store i32 {{.*}}, ptr @interesting3, align 4
-  ; CHECK-FINAL: store i32 0, ptr @interesting3, align 4
+  ; CHECK-FINAL: store i32 %0, ptr @interesting3, align 4
   store i32 %0, ptr @interesting3, align 4
 
   ; CHECK-ALL: load i32, ptr @interesting, align 4
   %1 = load i32, ptr @interesting, align 4
   store i32 %1, ptr @uninteresting, align 4
 
   ; CHECK-ALL: load i32, ptr @interesting3, align 4
   %2 = load i32, ptr @interesting3, align 4

llvm/tools/llvm-reduce/deltas/ReduceGlobalVars.cpp

 // to reduce Global Variables in the provided Module.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ReduceGlobalVars.h"
 #include "Utils.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
-#include <set>
 
 using namespace llvm;
 
+static bool shouldAlwaysKeep(const GlobalVariable &GV) {
+  return GV.getName() == "llvm.used" || GV.getName() == "llvm.compiler.used";
+}
+
 /// Removes all the GVs that aren't inside the desired Chunks.
 static void extractGVsFromModule(Oracle &O, Module &Program) {
   // Get GVs inside desired chunks
   std::vector<Constant *> InitGVsToKeep;
   for (auto &GV : Program.globals()) {
-    if ((GV.getName() == "llvm.used" || GV.getName() == "llvm.compiler.used") ||
-        O.shouldKeep())
+    if (shouldAlwaysKeep(GV) || O.shouldKeep())
       InitGVsToKeep.push_back(&GV);
   }
 
   // We create a vector first, then convert it to a set, so that we don't have
   // to pay the cost of rebalancing the set frequently if the order we insert
   // the elements doesn't match the order they should appear inside the set.
-  std::set<Constant *> GVsToKeep(InitGVsToKeep.begin(), InitGVsToKeep.end());
+  DenseSet<Constant *> GVsToKeep(InitGVsToKeep.begin(), InitGVsToKeep.end());
 
   // Delete out-of-chunk GVs and their uses
   DenseSet<Constant *> ToRemove;
-  std::vector<WeakVH> InstToRemove;
   for (auto &GV : Program.globals()) {
-    if (!GVsToKeep.count(&GV)) {
-      for (auto *U : GV.users())
-        if (auto *Inst = dyn_cast<Instruction>(U))
-          InstToRemove.push_back(Inst);
-
+    if (!GVsToKeep.count(&GV))
       ToRemove.insert(&GV);
-    }
-  }
 
   removeFromUsedLists(Program,
                       [&ToRemove](Constant *C) { return ToRemove.count(C); });
 
-  // Delete (unique) Instruction uses of unwanted GVs
-  for (Value *V : InstToRemove) {
-    if (!V)
-      continue;
-    auto *Inst = cast<Instruction>(V);
-    Inst->replaceAllUsesWith(getDefaultValue(Inst->getType()));
-    Inst->eraseFromParent();
-  }
-
   for (auto *GV : ToRemove) {
     GV->replaceAllUsesWith(getDefaultValue(GV->getType()));
     cast<GlobalVariable>(GV)->eraseFromParent();
   }
 }
 
 void llvm::reduceGlobalsDeltaPass(TestRunner &Test) {
   runDeltaPass(Test, extractGVsFromModule, "Reducing GlobalVariables");
 }