rewrite OptimizeAwayTrappingUsesOfLoads to 1) avoid a temporary

vector and extraneous loop over it, 2) not delete globals used by
phis/selects etc which could actually be useful.  This fixes PR3321.
Many thanks to Duncan for narrowing this down.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@62201 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 61 insertions, 24 deletions

diff --git a/lib/Transforms/IPO/GlobalOpt.cpp b/lib/Transforms/IPO/GlobalOpt.cpp
index 53aa03d..b4a5634 100644
--- a/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/lib/Transforms/IPO/GlobalOpt.cpp
@@ -733,44 +733,46 @@ static bool OptimizeAwayTrappingUsesOfValue(Value *V, Constant *NewV) {
 /// if the loaded value is dynamically null, then we know that they cannot be
 /// reachable with a null optimize away the load.
 static bool OptimizeAwayTrappingUsesOfLoads(GlobalVariable *GV, Constant *LV) {
-  std::vector<LoadInst*> Loads;
   bool Changed = false;
 
+  // Keep track of whether we are able to remove all the uses of the global
+  // other than the store that defines it.
+  bool AllNonStoreUsesGone = true;
+  
   // Replace all uses of loads with uses of uses of the stored value.
-  for (Value::use_iterator GUI = GV->use_begin(), E = GV->use_end();
-       GUI != E; ++GUI)
-    if (LoadInst *LI = dyn_cast<LoadInst>(*GUI)) {
-      Loads.push_back(LI);
+  for (Value::use_iterator GUI = GV->use_begin(), E = GV->use_end(); GUI != E;){
+    User *GlobalUser = *GUI++;
+    if (LoadInst *LI = dyn_cast<LoadInst>(GlobalUser)) {
       Changed |= OptimizeAwayTrappingUsesOfValue(LI, LV);
+      // If we were able to delete all uses of the loads
+      if (LI->use_empty()) {
+        LI->eraseFromParent();
+        Changed = true;
+      } else {
+        AllNonStoreUsesGone = false;
+      }
+    } else if (isa<StoreInst>(GlobalUser)) {
+      // Ignore the store that stores "LV" to the global.
+      assert(GlobalUser->getOperand(1) == GV &&
+             "Must be storing *to* the global");
     } else {
-      // If we get here we could have stores, selects, or phi nodes whose values
-      // are loaded.
-      assert((isa<StoreInst>(*GUI) || isa<PHINode>(*GUI) ||
-              isa<SelectInst>(*GUI) || isa<ConstantExpr>(*GUI)) &&
-             "Only expect load and stores!");
+      AllNonStoreUsesGone = false;
+
+      // If we get here we could have other crazy uses that are transitively
+      // loaded.
+      assert((isa<PHINode>(GlobalUser) || isa<SelectInst>(GlobalUser) ||
+              isa<ConstantExpr>(GlobalUser)) && "Only expect load and stores!");
     }
+  }
 
   if (Changed) {
     DOUT << "OPTIMIZED LOADS FROM STORED ONCE POINTER: " << *GV;
     ++NumGlobUses;
   }
 
-  // Delete all of the loads we can, keeping track of whether we nuked them all!
-  bool AllLoadsGone = true;
-  while (!Loads.empty()) {
-    LoadInst *L = Loads.back();
-    if (L->use_empty()) {
-      L->eraseFromParent();
-      Changed = true;
-    } else {
-      AllLoadsGone = false;
-    }
-    Loads.pop_back();
-  }
-
   // If we nuked all of the loads, then none of the stores are needed either,
   // nor is the global.
-  if (AllLoadsGone) {
+  if (AllNonStoreUsesGone) {
     DOUT << "  *** GLOBAL NOW DEAD!\n";
     CleanupConstantGlobalUsers(GV, 0);
     if (GV->use_empty()) {
diff --git a/test/Transforms/GlobalOpt/2009-01-13-phi-user.ll b/test/Transforms/GlobalOpt/2009-01-13-phi-user.ll
new file mode 100644
index 0000000..03ec3b6
--- /dev/null
+++ b/test/Transforms/GlobalOpt/2009-01-13-phi-user.ll
@@ -0,0 +1,35 @@
+; RUN: llvm-as < %s | opt -globalopt | llvm-dis | grep {phi.*@head}
+; PR3321
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
+target triple = "x86_64-unknown-linux-gnu"
+	%struct.node = type { %struct.node*, i32 }
+@head = internal global %struct.node* null		; <%struct.node**> [#uses=2]
+@node = internal global %struct.node { %struct.node* null, i32 42 }, align 16		; <%struct.node*> [#uses=1]
+
+define i32 @f() nounwind {
+entry:
+	store %struct.node* @node, %struct.node** @head, align 8
+	br label %bb1
+
+bb:		; preds = %bb1
+	%0 = getelementptr %struct.node* %t.0, i64 0, i32 1		; <i32*> [#uses=1]
+	%1 = load i32* %0, align 4		; <i32> [#uses=1]
+	%2 = getelementptr %struct.node* %t.0, i64 0, i32 0		; <%struct.node**> [#uses=1]
+	br label %bb1
+
+bb1:		; preds = %bb, %entry
+	%value.0 = phi i32 [ undef, %entry ], [ %1, %bb ]		; <i32> [#uses=1]
+	%t.0.in = phi %struct.node** [ @head, %entry ], [ %2, %bb ]		; <%struct.node**> [#uses=1]
+	%t.0 = load %struct.node** %t.0.in		; <%struct.node*> [#uses=3]
+	%3 = icmp eq %struct.node* %t.0, null		; <i1> [#uses=1]
+	br i1 %3, label %bb2, label %bb
+
+bb2:		; preds = %bb1
+	ret i32 %value.0
+}
+
+define i32 @main() nounwind {
+entry:
+	%0 = call i32 @f() nounwind		; <i32> [#uses=1]
+	ret i32 %0
+}