Switch to DenseSet, simplifying much more code. We now have a single iteration

where we hash, compare and fold, instead of one iteration where we build up
the hash buckets and a second one to fold.


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

lib/Transforms/IPO/MergeFunctions.cpp

@@ -45,10 +45,11 @@
 
 #define DEBUG_TYPE "mergefunc"
 #include "llvm/Transforms/IPO.h"
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Constants.h"
 #include "llvm/InlineAsm.h"
 #include "llvm/Instructions.h"
@@ -59,10 +60,9 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/IRBuilder.h"
+#include "llvm/Support/ValueHandle.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetData.h"
-#include <map>
-#include <vector>
 using namespace llvm;
 
 STATISTIC(NumFunctionsMerged, "Number of functions merged");
@@ -81,14 +81,9 @@ namespace {
     bool runOnModule(Module &M);
 
   private:
-    /// PairwiseCompareAndMerge - Given a list of functions, compare each pair
+    /// MergeTwoFunctions - Merge two equivalent functions. Upon completion, G
-    /// and merge the pairs of equivalent functions.
+    /// is deleted.
-    bool PairwiseCompareAndMerge(std::vector<Function *> &FnVec);
+    void MergeTwoFunctions(Function *F, Function *G) const;
-
-    /// MergeTwoFunctions - Merge two equivalent functions. Upon completion,
-    /// FnVec[j] should never be visited again.
-    void MergeTwoFunctions(std::vector<Function *> &FnVec,
-                           unsigned i, unsigned j) const;
 
     /// WriteThunk - Replace G with a simple tail call to bitcast(F). Also
     /// replace direct uses of G with bitcast(F).
@@ -112,7 +107,8 @@ namespace {
 /// side of claiming that two functions are different).
 class FunctionComparator {
 public:
-  FunctionComparator(TargetData *TD, Function *F1, Function *F2)
+  FunctionComparator(const TargetData *TD, const Function *F1,
+                     const Function *F2)
     : F1(F1), F2(F2), TD(TD), IDMap1Count(0), IDMap2Count(0) {}
 
   /// Compare - test whether the two functions have equivalent behaviour.
@@ -144,9 +140,9 @@ private:
   bool isEquivalentType(const Type *Ty1, const Type *Ty2) const;
 
   // The two functions undergoing comparison.
-  Function *F1, *F2;
+  const Function *F1, *F2;
 
-  TargetData *TD;
+  const TargetData *TD;
 
   typedef DenseMap<const Value *, unsigned long> IDMap;
   IDMap Map1, Map2;
@@ -154,22 +150,6 @@ private:
 };
 }
 
-/// Compute a hash guaranteed to be equal for two equivalent functions, but
-/// very likely to be different for different functions.
-static unsigned long ProfileFunction(const Function *F) {
-  const FunctionType *FTy = F->getFunctionType();
-
-  FoldingSetNodeID ID;
-  ID.AddInteger(F->size());
-  ID.AddInteger(F->getCallingConv());
-  ID.AddBoolean(F->hasGC());
-  ID.AddBoolean(FTy->isVarArg());
-  ID.AddInteger(FTy->getReturnType()->getTypeID());
-  for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i)
-    ID.AddInteger(FTy->getParamType(i)->getTypeID());
-  return ID.ComputeHash();
-}
-
 /// isEquivalentType - any two pointers in the same address space are
 /// equivalent. Otherwise, standard type equivalence rules apply.
 bool FunctionComparator::isEquivalentType(const Type *Ty1,
@@ -545,17 +525,8 @@ void MergeFunctions::WriteThunk(Function *F, Function *G) const {
 }
 
 /// MergeTwoFunctions - Merge two equivalent functions. Upon completion,
-/// FnVec[j] is deleted but not removed from the vector.
+/// Function G is deleted.
-void MergeFunctions::MergeTwoFunctions(std::vector<Function *> &FnVec,
+void MergeFunctions::MergeTwoFunctions(Function *F, Function *G) const {
-                                       unsigned i, unsigned j) const {
-  Function *F = FnVec[i];
-  Function *G = FnVec[j];
-
-  if (F->isWeakForLinker() && !G->isWeakForLinker()) {
-    std::swap(FnVec[i], FnVec[j]);
-    std::swap(F, G);
-  }
-
   if (F->isWeakForLinker()) {
     assert(G->isWeakForLinker());
 
@@ -580,54 +551,88 @@ void MergeFunctions::MergeTwoFunctions(std::vector<Function *> &FnVec,
   ++NumFunctionsMerged;
 }
 
-/// PairwiseCompareAndMerge - Given a list of functions, compare each pair and
+static unsigned ProfileFunction(const Function *F) {
-/// merge the pairs of equivalent functions.
+  const FunctionType *FTy = F->getFunctionType();
-bool MergeFunctions::PairwiseCompareAndMerge(std::vector<Function *> &FnVec) {
-  bool Changed = false;
-  for (int i = 0, e = FnVec.size(); i != e; ++i) {
-    for (int j = i + 1; j != e; ++j) {
-      bool isEqual = FunctionComparator(TD, FnVec[i], FnVec[j]).Compare();
 
-      DEBUG(dbgs() << "  " << FnVec[i]->getName()
+  FoldingSetNodeID ID;
-            << (isEqual ? " == " : " != ") << FnVec[j]->getName() << "\n");
+  ID.AddInteger(F->size());
-
+  ID.AddInteger(F->getCallingConv());
-      if (isEqual) {
+  ID.AddBoolean(F->hasGC());
-        MergeTwoFunctions(FnVec, i, j);
+  ID.AddBoolean(FTy->isVarArg());
-        Changed = true;
+  ID.AddInteger(FTy->getReturnType()->getTypeID());
-        FnVec.erase(FnVec.begin() + j);
+  for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i)
-        --j, --e;
+    ID.AddInteger(FTy->getParamType(i)->getTypeID());
-      }
+  return ID.ComputeHash();
-    }
-  }
-  return Changed;
 }
 
+class ComparableFunction {
+public:
+  ComparableFunction(Function *Func, TargetData *TD)
+    : Func(Func), Hash(ProfileFunction(Func)), TD(TD) {}
+
+  AssertingVH<Function> const Func;
+  const unsigned Hash;
+  TargetData * const TD;
+};
+
+struct MergeFunctionsEqualityInfo {
+  static ComparableFunction *getEmptyKey() {
+    return reinterpret_cast<ComparableFunction*>(0);
+  }
+  static ComparableFunction *getTombstoneKey() {
+    return reinterpret_cast<ComparableFunction*>(-1);
+  }
+  static unsigned getHashValue(const ComparableFunction *CF) {
+    return CF->Hash;
+  }
+  static bool isEqual(const ComparableFunction *LHS,
+                      const ComparableFunction *RHS) {
+    if (LHS == RHS)
+      return true;
+    if (LHS == getEmptyKey() || LHS == getTombstoneKey() ||
+        RHS == getEmptyKey() || RHS == getTombstoneKey())
+      return false;
+    assert(LHS->TD == RHS->TD && "Comparing functions for different targets");
+    return FunctionComparator(LHS->TD, LHS->Func, RHS->Func).Compare();
+  }
+};
+
 bool MergeFunctions::runOnModule(Module &M) {
   bool Changed = false;
 
-  std::map<unsigned long, std::vector<Function *> > FnMap;
-
-  for (Module::iterator F = M.begin(), E = M.end(); F != E; ++F) {
-    if (F->isDeclaration() || F->hasAvailableExternallyLinkage())
-      continue;
-
-    FnMap[ProfileFunction(F)].push_back(F);
-  }
-
   TD = getAnalysisIfAvailable<TargetData>();
 
-  bool LocalChanged;
+  typedef DenseSet<ComparableFunction *, MergeFunctionsEqualityInfo> FnSetType;
-  do {
+  FnSetType FnSet;
-    LocalChanged = false;
+  for (Module::iterator F = M.begin(), E = M.end(); F != E;) {
-    DEBUG(dbgs() << "size: " << FnMap.size() << "\n");
+    if (F->isDeclaration() || F->hasAvailableExternallyLinkage()) {
-    for (std::map<unsigned long, std::vector<Function *> >::iterator
+      ++F;
-           I = FnMap.begin(), E = FnMap.end(); I != E; ++I) {
+      continue;
-      std::vector<Function *> &FnVec = I->second;
-      DEBUG(dbgs() << "hash (" << I->first << "): " << FnVec.size() << "\n");
-      LocalChanged |= PairwiseCompareAndMerge(FnVec);
     }
-    Changed |= LocalChanged;
-  } while (LocalChanged);
 
+    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);
+    }
+  }
+
+  DeleteContainerPointers(FnSet);
   return Changed;
 }