From 92a42eead2ffd1c71a0a6486560b9773b9a582fb Mon Sep 17 00:00:00 2001
From: Arnold Schwaighofer <aschwaighofer@apple.com>
Date: Tue, 9 Jun 2015 18:19:17 +0000
Subject: [PATCH] MergeFunctions: Don't replace a weak function use by another
 equivalent weak function

We don't know whether the weak functions definition is the definitive definition.

rdar://21303727

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239422 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/IPO/MergeFunctions.cpp  | 30 ++++++++--------
 test/Transforms/MergeFunc/fold-weak.ll | 48 +++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp
index 052f1b4b132..2e3519eac6a 100644
--- a/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/lib/Transforms/IPO/MergeFunctions.cpp
@@ -1397,28 +1397,26 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
   if (F->mayBeOverridden()) {
     assert(G->mayBeOverridden());
 
+    // Make them both thunks to the same internal function.
+    Function *H = Function::Create(F->getFunctionType(), F->getLinkage(), "",
+                                   F->getParent());
+    H->copyAttributesFrom(F);
+    H->takeName(F);
+    removeUsers(F);
+    F->replaceAllUsesWith(H);
+
+    unsigned MaxAlignment = std::max(G->getAlignment(), H->getAlignment());
+
     if (HasGlobalAliases) {
-      // Make them both thunks to the same internal function.
-      Function *H = Function::Create(F->getFunctionType(), F->getLinkage(), "",
-                                     F->getParent());
-      H->copyAttributesFrom(F);
-      H->takeName(F);
-      removeUsers(F);
-      F->replaceAllUsesWith(H);
-
-      unsigned MaxAlignment = std::max(G->getAlignment(), H->getAlignment());
-
       writeAlias(F, G);
       writeAlias(F, H);
-
-      F->setAlignment(MaxAlignment);
-      F->setLinkage(GlobalValue::PrivateLinkage);
     } else {
-      // We can't merge them. Instead, pick one and update all direct callers
-      // to call it and hope that we improve the instruction cache hit rate.
-      replaceDirectCallers(G, F);
+      writeThunk(F, G);
+      writeThunk(F, H);
     }
 
+    F->setAlignment(MaxAlignment);
+    F->setLinkage(GlobalValue::PrivateLinkage);
     ++NumDoubleWeak;
   } else {
     writeThunkOrAlias(F, G);
diff --git a/test/Transforms/MergeFunc/fold-weak.ll b/test/Transforms/MergeFunc/fold-weak.ll
index 4df6e39c125..f8a18887890 100644
--- a/test/Transforms/MergeFunc/fold-weak.ll
+++ b/test/Transforms/MergeFunc/fold-weak.ll
@@ -1,17 +1,47 @@
-; RUN: opt < %s -mergefunc -S > %t
-; RUN: grep "define weak" %t | count 2
-; RUN: grep "call" %t | count 2
-; XFAIL: *
-
-; This test is off for a bit as we change this particular sort of folding to
-; only apply on ELF systems and not Mach-O systems.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
 
 define weak i32 @sum(i32 %x, i32 %y) {
   %sum = add i32 %x, %y
-  ret i32 %sum
+  %sum2 = add i32 %sum, %y
+  %sum3 = add i32 %sum2, %y
+  ret i32 %sum3
 }
 
 define weak i32 @add(i32 %x, i32 %y) {
   %sum = add i32 %x, %y
-  ret i32 %sum
+  %sum2 = add i32 %sum, %y
+  %sum3 = add i32 %sum2, %y
+  ret i32 %sum3
+}
+
+; Don't replace a weak function use by another equivalent function. We don't
+; know whether the symbol that will ulitmately be linked is equivalent - we
+; don't know that the weak definition is the definitive definition or whether it
+; will be overriden by a stronger definition).
+
+; CHECK-LABEL: define private i32 @0
+; CHECK: add i32
+; CHECK: add i32
+; CHECK: add i32
+; CHECK: ret
+
+; CHECK-LABEL: define i32 @use_weak
+; CHECK: call i32 @add
+; CHECK: call i32 @sum
+; CHECK: ret
+
+; CHECK-LABEL: define weak i32 @sum
+; CHECK:  tail call i32 @0
+; CHECK:  ret
+
+; CHECK-LABEL: define weak i32 @add
+; CHECK:  tail call i32 @0
+; CHECK:  ret
+
+
+define i32 @use_weak(i32 %a, i32 %b) {
+  %res = call i32 @add(i32 %a, i32 %b)
+  %res2 = call i32 @sum(i32 %a, i32 %b)
+  %res3 = add i32 %res, %res2
+  ret i32 %res3
 }