Rework GlobalValue::removeDeadConstantUsers to always remove dead constant

exprs hanging off a global, even if the global is not otherwise dead.  This
requires some tricky iterator gymnastics.

This implements Transforms/GlobalOpt/constantexpr-dangle.ll by deleting a
constantexpr that made it appear that the address of the function was taken.

llvm-svn: 34608

2 files changed, 29 insertions, 25 deletions

diff --git a/llvm/include/llvm/GlobalValue.h b/llvm/include/llvm/GlobalValue.h
index ca9149e6caf..576fbe6521a 100644
--- a/llvm/include/llvm/GlobalValue.h
+++ b/llvm/include/llvm/GlobalValue.h
@@ -128,10 +128,6 @@ public:
   /// off of this global value, remove them.  This method is useful for clients
   /// that want to check to see if a global is unused, but don't want to deal
   /// with potentially dead constants hanging off of the globals.
-  ///
-  /// This method tries to make the global dead.  If it detects a user that
-  /// would prevent it from becoming completely dead, it gives up early,
-  /// potentially leaving some dead constant users around.
   void removeDeadConstantUsers();
 
   // Methods for support type inquiry through isa, cast, and dyn_cast:
diff --git a/llvm/lib/VMCore/Globals.cpp b/llvm/lib/VMCore/Globals.cpp
index 327e2ad9aa2..198948d9028 100644
--- a/llvm/lib/VMCore/Globals.cpp
+++ b/llvm/lib/VMCore/Globals.cpp
@@ -22,20 +22,18 @@ using namespace llvm;
 //                            GlobalValue Class
 //===----------------------------------------------------------------------===//
 
-/// This could be named "SafeToDestroyGlobalValue". It just makes sure that
-/// there are no non-constant uses of this GlobalValue. If there aren't then
-/// this and the transitive closure of the constants can be deleted. See the
-/// destructor for details.
-static bool removeDeadConstantUsers(Constant* C) {
+/// removeDeadUsersOfConstant - If the specified constantexpr is dead, remove
+/// it.  This involves recursively eliminating any dead users of the
+/// constantexpr.
+static bool removeDeadUsersOfConstant(Constant *C) {
   if (isa<GlobalValue>(C)) return false; // Cannot remove this
 
-  while (!C->use_empty())
-    if (Constant *User = dyn_cast<Constant>(C->use_back())) {
-      if (!removeDeadConstantUsers(User))
-        return false; // Constant wasn't dead
-    } else {
-      return false; // Non-constant usage;
-    }
+  while (!C->use_empty()) {
+    Constant *User = dyn_cast<Constant>(C->use_back());
+    if (!User) return false; // Non-constant usage;
+    if (!removeDeadUsersOfConstant(User))
+      return false; // Constant wasn't dead
+  }
 
   C->destroyConstant();
   return true;
@@ -45,17 +43,27 @@ static bool removeDeadConstantUsers(Constant* C) {
 /// off of this global value, remove them.  This method is useful for clients
 /// that want to check to see if a global is unused, but don't want to deal
 /// with potentially dead constants hanging off of the globals.
-///
-/// This function returns true if the global value is now dead.  If all
-/// users of this global are not dead, this method may return false and
-/// leave some of them around.
 void GlobalValue::removeDeadConstantUsers() {
-  while(!use_empty()) {
-    if (Constant* User = dyn_cast<Constant>(use_back())) {
-      if (!::removeDeadConstantUsers(User))
-        return; // Constant wasn't dead
+  
+  Value::use_iterator I = use_begin(), E = use_end();
+  Value::use_iterator LastNonDeadUser = E;
+  for (; I != E; ++I) {
+    if (Constant *User = dyn_cast<Constant>(*I)) {
+      if (!removeDeadUsersOfConstant(User)) {
+        // If the constant wasn't dead, remember that this was the last live use
+        // and move on to the next constant.
+        LastNonDeadUser = I;
+      } else {
+        // If the constant was dead, then the iterator is invalidated.
+        if (LastNonDeadUser == E) {
+          I = use_begin();
+          if (I == E) break;
+        } else {
+          I = LastNonDeadUser;
+        }
+      }
     } else {
-      return; // Non-constant usage;
+      LastNonDeadUser = I;
     }
   }
 }