From 75dfadbaf9b7a3c171b1a9542355e38c8d977d8c Mon Sep 17 00:00:00 2001 From: Nick Lewycky Date: Tue, 31 Aug 2010 05:53:05 +0000 Subject: [PATCH] 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. llvm-svn: 112582 --- lib/Transforms/IPO/MergeFunctions.cpp | 165 +++++++++++++------------- 1 file changed, 85 insertions(+), 80 deletions(-) diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp index cac824b2e89..cefed1a059a 100644 --- a/lib/Transforms/IPO/MergeFunctions.cpp +++ b/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 -#include 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 - /// and merge the pairs of equivalent functions. - bool PairwiseCompareAndMerge(std::vector &FnVec); - - /// MergeTwoFunctions - Merge two equivalent functions. Upon completion, - /// FnVec[j] should never be visited again. - void MergeTwoFunctions(std::vector &FnVec, - unsigned i, unsigned j) const; + /// MergeTwoFunctions - Merge two equivalent functions. Upon completion, G + /// is deleted. + void MergeTwoFunctions(Function *F, Function *G) 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 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. -void MergeFunctions::MergeTwoFunctions(std::vector &FnVec, - 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); - } - +/// Function G is deleted. +void MergeFunctions::MergeTwoFunctions(Function *F, Function *G) const { if (F->isWeakForLinker()) { assert(G->isWeakForLinker()); @@ -580,54 +551,88 @@ void MergeFunctions::MergeTwoFunctions(std::vector &FnVec, ++NumFunctionsMerged; } -/// PairwiseCompareAndMerge - Given a list of functions, compare each pair and -/// merge the pairs of equivalent functions. -bool MergeFunctions::PairwiseCompareAndMerge(std::vector &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(); +static unsigned ProfileFunction(const Function *F) { + const FunctionType *FTy = F->getFunctionType(); - DEBUG(dbgs() << " " << FnVec[i]->getName() - << (isEqual ? " == " : " != ") << FnVec[j]->getName() << "\n"); - - if (isEqual) { - MergeTwoFunctions(FnVec, i, j); - Changed = true; - FnVec.erase(FnVec.begin() + j); - --j, --e; - } - } - } - return Changed; + 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(); } +class ComparableFunction { +public: + ComparableFunction(Function *Func, TargetData *TD) + : Func(Func), Hash(ProfileFunction(Func)), TD(TD) {} + + AssertingVH const Func; + const unsigned Hash; + TargetData * const TD; +}; + +struct MergeFunctionsEqualityInfo { + static ComparableFunction *getEmptyKey() { + return reinterpret_cast(0); + } + static ComparableFunction *getTombstoneKey() { + return reinterpret_cast(-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 > 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(); - bool LocalChanged; - do { - LocalChanged = false; - DEBUG(dbgs() << "size: " << FnMap.size() << "\n"); - for (std::map >::iterator - I = FnMap.begin(), E = FnMap.end(); I != E; ++I) { - std::vector &FnVec = I->second; - DEBUG(dbgs() << "hash (" << I->first << "): " << FnVec.size() << "\n"); - LocalChanged |= PairwiseCompareAndMerge(FnVec); + typedef DenseSet FnSetType; + FnSetType FnSet; + for (Module::iterator F = M.begin(), E = M.end(); F != E;) { + if (F->isDeclaration() || F->hasAvailableExternallyLinkage()) { + ++F; + continue; } - Changed |= LocalChanged; - } while (LocalChanged); + ComparableFunction *NewF = new ComparableFunction(F, TD); + ++F; + std::pair 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; }