-
-
Save japaric/ef95ae39fc0891dc4268b9d322a3b6bf to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp | |
index e0bb0eb42b5..794e502d468 100644 | |
--- a/lib/Transforms/IPO/MergeFunctions.cpp | |
+++ b/lib/Transforms/IPO/MergeFunctions.cpp | |
@@ -27,7 +27,7 @@ | |
// -- We define Function* container class with custom "operator<" (FunctionPtr). | |
// -- "FunctionPtr" instances are stored in std::set collection, so every | |
// std::set::insert operation will give you result in log(N) time. | |
-// | |
+// | |
// As an optimization, a hash of the function structure is calculated first, and | |
// two functions are only compared if they have the same hash. This hash is | |
// cheap to compute, and has the property that if function F == G according to | |
@@ -333,7 +333,7 @@ bool MergeFunctions::runOnModule(Module &M) { | |
for (Function &Func : M) { | |
if (!Func.isDeclaration() && !Func.hasAvailableExternallyLinkage()) { | |
HashedFuncs.push_back({FunctionComparator::functionHash(Func), &Func}); | |
- } | |
+ } | |
} | |
std::stable_sort( | |
@@ -352,7 +352,7 @@ bool MergeFunctions::runOnModule(Module &M) { | |
Deferred.push_back(WeakVH(I->second)); | |
} | |
} | |
- | |
+ | |
do { | |
std::vector<WeakVH> Worklist; | |
Deferred.swap(Worklist); | |
@@ -465,8 +465,15 @@ static Value *createCast(IRBuilder<> &Builder, Value *V, Type *DestTy) { | |
// of G with bitcast(F). Deletes G. | |
void MergeFunctions::writeThunk(Function *F, Function *G) { | |
if (!G->isInterposable()) { | |
- // Redirect direct callers of G to F. | |
- replaceDirectCallers(G, F); | |
+ if (G->hasGlobalUnnamedAddr()) { | |
+ // If G's address is not significant, replace it entirely. | |
+ Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType()); | |
+ G->replaceAllUsesWith(BitcastF); | |
+ } else { | |
+ // Redirect direct callers of G to F. (See note on MergeFunctionsPDI | |
+ // above). | |
+ replaceDirectCallers(G, F); | |
+ } | |
} | |
// If G was internal then we may have replaced all uses of G with F. If so, | |
@@ -476,6 +483,16 @@ void MergeFunctions::writeThunk(Function *F, Function *G) { | |
return; | |
} | |
+ // Don't merge tiny functions using a thunk, since it can just end up | |
+ // making the function larger. | |
+ if (F->size() == 1) { | |
+ if (F->front().size() <= 2) { | |
+ DEBUG(dbgs() << "writeThunk: " << F->getName() | |
+ << " is too small to bother creating a thunk for\n"); | |
+ return; | |
+ } | |
+ } | |
+ | |
Function *NewG = Function::Create(G->getFunctionType(), G->getLinkage(), "", | |
G->getParent()); | |
BasicBlock *BB = BasicBlock::Create(F->getContext(), "", NewG); | |
@@ -562,11 +579,11 @@ void MergeFunctions::replaceFunctionInTree(const FunctionNode &FN, | |
Function *F = FN.getFunc(); | |
assert(FunctionComparator(F, G, &GlobalNumbers).compare() == 0 && | |
"The two functions must be equal"); | |
- | |
+ | |
auto I = FNodesInTree.find(F); | |
assert(I != FNodesInTree.end() && "F should be in FNodesInTree"); | |
assert(FNodesInTree.count(G) == 0 && "FNodesInTree should not contain G"); | |
- | |
+ | |
FnTreeType::iterator IterToFNInFnTree = I->second; | |
assert(&(*IterToFNInFnTree) == &FN && "F should map to FN in FNodesInTree."); | |
// Remove F -> FN and insert G -> FN | |
@@ -591,18 +608,6 @@ bool MergeFunctions::insert(Function *NewFunction) { | |
const FunctionNode &OldF = *Result.first; | |
- // Don't merge tiny functions, since it can just end up making the function | |
- // larger. | |
- // FIXME: Should still merge them if they are unnamed_addr and produce an | |
- // alias. | |
- if (NewFunction->size() == 1) { | |
- if (NewFunction->front().size() <= 2) { | |
- DEBUG(dbgs() << NewFunction->getName() | |
- << " is to small to bother merging\n"); | |
- return false; | |
- } | |
- } | |
- | |
// Impose a total order (by name) on the replacement of functions. This is | |
// important when operating on more than one module independently to prevent | |
// cycles of thunks calling each other when the modules are linked together. | |
diff --git a/test/Transforms/MergeFunc/merge-small-unnamed-addr.ll b/test/Transforms/MergeFunc/merge-small-unnamed-addr.ll | |
new file mode 100644 | |
index 00000000000..256f6864761 | |
--- /dev/null | |
+++ b/test/Transforms/MergeFunc/merge-small-unnamed-addr.ll | |
@@ -0,0 +1,14 @@ | |
+; RUN: opt -S -mergefunc < %s | FileCheck %s | |
+ | |
+; CHECK-NOT: @b | |
+ | |
+@x = constant { void ()*, void ()* } { void ()* @a, void ()* @b } | |
+; CHECK: { void ()* @a, void ()* @a } | |
+ | |
+define internal void @a() unnamed_addr { | |
+ ret void | |
+} | |
+ | |
+define internal void @b() unnamed_addr { | |
+ ret void | |
+} | |
diff --git a/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll b/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll | |
new file mode 100644 | |
index 00000000000..8a9a9e67551 | |
--- /dev/null | |
+++ b/test/Transforms/MergeFunc/merge-unnamed-addr-bitcast.ll | |
@@ -0,0 +1,30 @@ | |
+; RUN: opt -S -mergefunc < %s | FileCheck %s | |
+ | |
+%A = type { i32 } | |
+%B = type { i32 } | |
+ | |
+; CHECK-NOT: @b | |
+ | |
+@x = constant { i32 (i32)*, i32 (i32)* } | |
+ { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*), | |
+ i32 (i32)* bitcast (i32 (%B)* @b to i32 (i32)*) } | |
+; CHECK: { i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*), i32 (i32)* bitcast (i32 (%A)* @a to i32 (i32)*) } | |
+ | |
+define internal i32 @a(%A) unnamed_addr { | |
+ extractvalue %A %0, 0 | |
+ xor i32 %2, 0 | |
+ ret i32 %3 | |
+} | |
+ | |
+define internal i32 @b(%B) unnamed_addr { | |
+ extractvalue %B %0, 0 | |
+ xor i32 %2, 0 | |
+ ret i32 %3 | |
+} | |
+ | |
+define i32 @c(i32) { | |
+ insertvalue %B undef, i32 %0, 0 | |
+ call i32 @b(%B %2) | |
+; CHECK: call i32 bitcast (i32 (%A)* @a to i32 (%B)*)(%B %2) | |
+ ret i32 %3 | |
+} | |
diff --git a/test/Transforms/MergeFunc/merge-unnamed-addr.ll b/test/Transforms/MergeFunc/merge-unnamed-addr.ll | |
new file mode 100644 | |
index 00000000000..cb34d43c08f | |
--- /dev/null | |
+++ b/test/Transforms/MergeFunc/merge-unnamed-addr.ll | |
@@ -0,0 +1,18 @@ | |
+; RUN: opt -S -mergefunc < %s | FileCheck %s | |
+ | |
+; CHECK-NOT: @b | |
+ | |
+@x = constant { i32 (i32)*, i32 (i32)* } { i32 (i32)* @a, i32 (i32)* @b } | |
+; CHECK: { i32 (i32)* @a, i32 (i32)* @a } | |
+ | |
+define internal i32 @a(i32 %a) unnamed_addr { | |
+ %b = xor i32 %a, 0 | |
+ %c = xor i32 %b, 0 | |
+ ret i32 %c | |
+} | |
+ | |
+define internal i32 @b(i32 %a) unnamed_addr { | |
+ %b = xor i32 %a, 0 | |
+ %c = xor i32 %b, 0 | |
+ ret i32 %c | |
+} | |
diff --git a/test/Transforms/MergeFunc/no-merge-small.ll b/test/Transforms/MergeFunc/no-merge-small.ll | |
new file mode 100644 | |
index 00000000000..888d42caeb8 | |
--- /dev/null | |
+++ b/test/Transforms/MergeFunc/no-merge-small.ll | |
@@ -0,0 +1,11 @@ | |
+; RUN: opt -S -mergefunc < %s | FileCheck %s | |
+ | |
+; CHECK: @a | |
+define void @a() { | |
+ ret void | |
+} | |
+ | |
+; CHECK: @b | |
+define void @b() { | |
+ ret void | |
+} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment