[llvm-commits] [llvm] r112595 - /llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp

[Nick Lewycky]

Author: nicholas
Date: Tue Aug 31 03:29:37 2010
New Revision: 112595

URL: http://llvm.org/viewvc/llvm-project?rev=112595&view=rev
Log:
Fix an infinite loop; merging two functions will create a new function (if the
two are weak, we make them thunks to a new strong function) so don't iterate
through the function list as we're modifying it.

Also add back the outermost loop which got removed during the cleanups.

Modified:
    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp

Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=112595&r1=112594&r2=112595&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Tue Aug 31 03:29:37 2010
@@ -63,6 +63,7 @@
 #include "llvm/Support/ValueHandle.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetData.h"
+#include <vector>
 using namespace llvm;
 
 STATISTIC(NumFunctionsMerged, "Number of functions merged");
@@ -82,7 +83,8 @@
 
   private:
     /// MergeTwoFunctions - Merge two equivalent functions. Upon completion, G
-    /// is deleted.
+    /// may be deleted, or may be converted into a thunk. In either case, it
+    /// should never be visited again.
     void MergeTwoFunctions(Function *F, Function *G) const;
 
     /// WriteThunk - Replace G with a simple tail call to bitcast(F). Also
@@ -598,41 +600,53 @@
 };
 
 bool MergeFunctions::runOnModule(Module &M) {
-  bool Changed = false;
+  typedef DenseSet<ComparableFunction *, MergeFunctionsEqualityInfo> FnSetType;
 
+  bool Changed = false;
   TD = getAnalysisIfAvailable<TargetData>();
 
-  typedef DenseSet<ComparableFunction *, MergeFunctionsEqualityInfo> FnSetType;
-  FnSetType FnSet;
-  for (Module::iterator F = M.begin(), E = M.end(); F != E;) {
-    if (F->isDeclaration() || F->hasAvailableExternallyLinkage()) {
-      ++F;
-      continue;
-    }
+  std::vector<Function *> Funcs;
+  for (Module::iterator F = M.begin(), E = M.end(); F != E; ++F) {
+    if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage())
+      Funcs.push_back(F);
+  }
+
+  bool LocalChanged;
+  do {
+    LocalChanged = false;
 
-    ComparableFunction *NewF = new ComparableFunction(F, TD);
-    ++F;
-    std::pair<FnSetType::iterator, bool> Result = FnSet.insert(NewF);
-    if (!Result.second) {
-      ComparableFunction *&OldF = *Result.first;
-      assert(OldF && "Expected a hash collision");
-
-      // NewF will be deleted in favour of OldF unless NewF is strong and OldF
-      // is weak in which case swap them to keep the strong definition.
-
-      if (OldF->Func->isWeakForLinker() && !NewF->Func->isWeakForLinker())
-        std::swap(OldF, NewF);
-
-      DEBUG(dbgs() << "  " << OldF->Func->getName() << " == "
-                   << NewF->Func->getName() << '\n');
-
-      Changed = true;
-      Function *DeleteF = NewF->Func;
-      delete NewF;
-      MergeTwoFunctions(OldF->Func, DeleteF);
+    FnSetType FnSet;
+    for (unsigned i = 0, e = Funcs.size(); i != e;) {
+      Function *F = Funcs[i];
+      ComparableFunction *NewF = new ComparableFunction(F, TD);
+      std::pair<FnSetType::iterator, bool> Result = FnSet.insert(NewF);
+      if (!Result.second) {
+        ComparableFunction *&OldF = *Result.first;
+        assert(OldF && "Expected a hash collision");
+
+        // NewF will be deleted in favour of OldF unless NewF is strong and
+        // OldF is weak in which case swap them to keep the strong definition.
+
+        if (OldF->Func->isWeakForLinker() && !NewF->Func->isWeakForLinker())
+          std::swap(OldF, NewF);
+
+        DEBUG(dbgs() << "  " << OldF->Func->getName() << " == "
+                     << NewF->Func->getName() << '\n');
+
+	Funcs.erase(Funcs.begin() + i);
+	--e;
+
+        Function *DeleteF = NewF->Func;
+        delete NewF;
+        MergeTwoFunctions(OldF->Func, DeleteF);
+	LocalChanged = true;
+        Changed = true;
+      } else {
+	++i;
+      }
     }
-  }
+    DeleteContainerPointers(FnSet);
+  } while (LocalChanged);
 
-  DeleteContainerPointers(FnSet);
   return Changed;
 }