Skip to content

Commit 1e131df

Browse files
fhahnfhahn
authored
Merge pull request #10112 from fhahn/fix-merge-func-link0nce
[MergeFuncs] Don't introduce calls to (linkonce,weak)_odr functions.
2 parents c07da4b + 6f1edfc commit 1e131df

File tree

9 files changed

+588
-16
lines changed

9 files changed

+588
-16
lines changed

‎clang/test/CodeGenCXX/merge-functions.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// REQUIRES: x86-registered-target
2-
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O0 -fmerge-functions -emit-llvm -o - -x c++ < %s | FileCheck %s -implicit-check-not=_ZN1A1gEiPi
3-
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O1 -fmerge-functions -emit-llvm -o - -x c++ < %s | FileCheck %s -implicit-check-not=_ZN1A1gEiPi
2+
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O0 -fmerge-functions -emit-llvm -o - -x c++ < %s | FileCheck %s
3+
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O1 -fmerge-functions -emit-llvm -o - -x c++ < %s | FileCheck %s
44

55
// Basic functionality test. Function merging doesn't kick in on functions that
66
// are too simple.
@@ -10,4 +10,8 @@ struct A {
1010
virtual int g(int x, int *p) { return x ? *p : 1; }
1111
} a;
1212

13-
// CHECK: define {{.*}} @_ZN1A1fEiPi
13+
// CHECK: define linkonce_odr noundef i32 @_ZN1A1gEiPi(
14+
// CHECK: tail call noundef i32 @0(
15+
16+
// CHECK: define linkonce_odr noundef i32 @_ZN1A1fEiPi(
17+
// CHECK: tail call noundef i32 @0(

‎llvm/lib/Transforms/IPO/MergeFunctions.cpp‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,10 +862,20 @@ bool MergeFunctions::writeThunkOrAlias(Function *F, Function *G) {
862862
return false;
863863
}
864864

865+
/// Returns true if \p F is either weak_odr or linkonce_odr.
866+
static bool isODR(const Function *F) {
867+
return F->hasWeakODRLinkage() || F->hasLinkOnceODRLinkage();
868+
}
869+
865870
// Merge two equivalent functions. Upon completion, Function G is deleted.
866871
void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
867-
if (F->isInterposable()) {
868-
assert(G->isInterposable());
872+
873+
// Create a new thunk that both F and G can call, if F cannot call G directly.
874+
// That is the case if F is either interposable or if G is either weak_odr or
875+
// linkonce_odr.
876+
if (F->isInterposable() || (isODR(F) && isODR(G))) {
877+
assert((!isODR(G) || isODR(F)) &&
878+
"if G is ODR, F must also be ODR due to ordering");
869879

870880
// Both writeThunkOrAlias() calls below must succeed, either because we can
871881
// create aliases for G and NewF, or because a thunk for F is profitable.
@@ -879,13 +889,22 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
879889
F->getAddressSpace(), "", F->getParent());
880890
NewF->copyAttributesFrom(F);
881891
NewF->takeName(F);
892+
NewF->setComdat(F->getComdat());
893+
F->setComdat(nullptr);
882894
NewF->IsNewDbgInfoFormat = F->IsNewDbgInfoFormat;
883895
// Ensure CFI type metadata is propagated to the new function.
884896
copyMetadataIfPresent(F, NewF, "type");
885897
copyMetadataIfPresent(F, NewF, "kcfi_type");
886898
removeUsers(F);
887899
F->replaceAllUsesWith(NewF);
888900

901+
// If G or NewF are (weak|linkonce)_odr, update all callers to call the
902+
// thunk.
903+
if (isODR(G))
904+
replaceDirectCallers(G, F);
905+
if (isODR(F))
906+
replaceDirectCallers(NewF, F);
907+
889908
// We collect alignment before writeThunkOrAlias that overwrites NewF and
890909
// G's content.
891910
const MaybeAlign NewFAlign = NewF->getAlign();
@@ -959,16 +978,24 @@ void MergeFunctions::replaceFunctionInTree(const FunctionNode &FN,
959978

960979
// Ordering for functions that are equal under FunctionComparator
961980
static bool isFuncOrderCorrect(const Function *F, const Function *G) {
981+
if (isODR(F) != isODR(G)) {
982+
// ODR functions before non-ODR functions. A ODR function can call a non-ODR
983+
// function if it is not interposable, but not the other way around.
984+
return isODR(G);
985+
}
986+
962987
if (F->isInterposable() != G->isInterposable()) {
963988
// Strong before weak, because the weak function may call the strong
964989
// one, but not the other way around.
965990
return !F->isInterposable();
966991
}
992+
967993
if (F->hasLocalLinkage() != G->hasLocalLinkage()) {
968994
// External before local, because we definitely have to keep the external
969995
// function, but may be able to drop the local one.
970996
return !F->hasLocalLinkage();
971997
}
998+
972999
// Impose a total order (by name) on the replacement of functions. This is
9731000
// important when operating on more than one module independently to prevent
9741001
// cycles of thunks calling each other when the modules are linked together.

‎llvm/test/Transforms/MergeFunc/comdat.ll‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ define linkonce_odr hidden i32 @g(i32 %x, i32 %y) comdat {
1919
ret i32 %sum3
2020
}
2121

22-
; CHECK-DAG: define linkonce_odr hidden i32 @f(i32 %x, i32 %y) comdat
23-
; CHECK-DAG: define linkonce_odr hidden i32 @g(i32 %0, i32 %1) comdat
22+
; CHECK-DAG: define private i32 @0(i32 %x, i32 %y) comdat($f)
23+
; CHECK-DAG: define linkonce_odr hidden i32 @g(i32 %0, i32 %1) comdat {
24+
; CHECK-DAG: define linkonce_odr hidden i32 @f(i32 %0, i32 %1) {
2425

‎llvm/test/Transforms/MergeFunc/linkonce_odr.ll‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,12 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
12
; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funC
23

34
; Replacments should be totally ordered on the function name.
45
; If we don't do this we can end up with one module defining a thunk for @funA
56
; and another module defining a thunk for @funB.
6-
;
77
; The problem with this is that the linker could then choose these two stubs
88
; each of the two modules and we end up with two stubs calling each other.
99

10-
; CHECK-LABEL: define linkonce_odr i32 @funA
11-
; CHECK-NEXT: add
12-
; CHECK: ret
13-
14-
; CHECK-LABEL: define linkonce_odr i32 @funB
15-
; CHECK-NEXT: tail call i32 @funA(i32 %0, i32 %1)
16-
; CHECK-NEXT: ret
17-
1810
define linkonce_odr i32 @funC(i32 %x, i32 %y) {
1911
%sum = add i32 %x, %y
2012
%sum2 = add i32 %x, %sum
@@ -40,3 +32,25 @@ define linkonce_odr i32 @funA(i32 %x, i32 %y) {
4032
; @funC, however, can safely be deleted as it has no uses, and is discardable
4133
; if unused.
4234
@take_addr_of_funB = global ptr @funB
35+
;.
36+
; CHECK: @take_addr_of_funB = global ptr @funB
37+
;.
38+
; CHECK-LABEL: define private i32 @0(
39+
; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]]) {
40+
; CHECK-NEXT: [[SUM:%.*]] = add i32 [[X]], [[Y]]
41+
; CHECK-NEXT: [[SUM2:%.*]] = add i32 [[X]], [[SUM]]
42+
; CHECK-NEXT: [[SUM3:%.*]] = add i32 [[X]], [[SUM2]]
43+
; CHECK-NEXT: ret i32 [[SUM3]]
44+
;
45+
;
46+
; CHECK-LABEL: define linkonce_odr i32 @funC(
47+
; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
48+
; CHECK-NEXT: [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
49+
; CHECK-NEXT: ret i32 [[TMP3]]
50+
;
51+
;
52+
; CHECK-LABEL: define linkonce_odr i32 @funB(
53+
; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
54+
; CHECK-NEXT: [[TMP3:%.*]] = tail call i32 @[[GLOB0]](i32 [[TMP0]], i32 [[TMP1]])
55+
; CHECK-NEXT: ret i32 [[TMP3]]
56+
;

‎llvm/test/Transforms/MergeFunc/merge-linkonce-odr-used.ll‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
2+
; RUN: opt -p mergefunc -S %s | FileCheck %s
3+
4+
@llvm.used = appending global [3 x ptr] [ptr @linkonce_odr_caller_of_foo_1, ptr @linkonce_odr_caller_of_foo_2, ptr @linkonce_odr_caller_of_foo_3]
5+
6+
define void @caller_of_callers(ptr %p) {
7+
call void @linkonce_odr_caller_of_foo_1(ptr %p)
8+
call void @linkonce_odr_caller_of_foo_2(ptr %p)
9+
call void @linkonce_odr_caller_of_foo_3(ptr %p)
10+
ret void
11+
}
12+
13+
define linkonce_odr void @linkonce_odr_caller_of_foo_1(ptr %p) {
14+
entry:
15+
tail call void @foo(ptr %p)
16+
tail call void @foo(ptr %p)
17+
tail call void @foo(ptr %p)
18+
ret void
19+
}
20+
21+
define linkonce_odr void @linkonce_odr_caller_of_foo_2(ptr %p) {
22+
entry:
23+
tail call void @foo(ptr %p)
24+
tail call void @foo(ptr %p)
25+
tail call void @foo(ptr %p)
26+
ret void
27+
}
28+
29+
define linkonce_odr void @linkonce_odr_caller_of_foo_3(ptr %p) {
30+
entry:
31+
tail call void @foo(ptr %p)
32+
tail call void @foo(ptr %p)
33+
tail call void @foo(ptr %p)
34+
ret void
35+
}
36+
37+
declare void @foo(ptr)
38+
39+
;.
40+
; CHECK: @llvm.used = appending global [3 x ptr] [ptr @linkonce_odr_caller_of_foo_1, ptr @linkonce_odr_caller_of_foo_2, ptr @linkonce_odr_caller_of_foo_3]
41+
;.
42+
; CHECK-LABEL: define void @caller_of_callers(
43+
; CHECK-SAME: ptr [[P:%.*]]) {
44+
; CHECK-NEXT: call void @[[GLOB0:[0-9]+]](ptr [[P]])
45+
; CHECK-NEXT: call void @[[GLOB0]](ptr [[P]])
46+
; CHECK-NEXT: call void @[[GLOB0]](ptr [[P]])
47+
; CHECK-NEXT: ret void
48+
;
49+
;
50+
; CHECK-LABEL: define private void @0(
51+
; CHECK-SAME: ptr [[P:%.*]]) {
52+
; CHECK-NEXT: [[ENTRY:.*:]]
53+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
54+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
55+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
56+
; CHECK-NEXT: ret void
57+
;
58+
;
59+
; CHECK-LABEL: define linkonce_odr void @linkonce_odr_caller_of_foo_2(
60+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
61+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
62+
; CHECK-NEXT: ret void
63+
;
64+
;
65+
; CHECK-LABEL: define linkonce_odr void @linkonce_odr_caller_of_foo_1(
66+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
67+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
68+
; CHECK-NEXT: ret void
69+
;
70+
;
71+
; CHECK-LABEL: define linkonce_odr void @linkonce_odr_caller_of_foo_3(
72+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
73+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
74+
; CHECK-NEXT: ret void
75+
;

‎llvm/test/Transforms/MergeFunc/merge-linkonce-odr-weak-odr-mixed-used.ll‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
2+
; RUN: opt -p mergefunc -S %s | FileCheck %s
3+
4+
@llvm.used = appending global [3 x ptr] [ptr @weak_odr_caller_of_foo_1, ptr @linkonce_odr_caller_of_foo_2, ptr @weak_odr_caller_of_foo_3]
5+
6+
define void @caller_of_callers(ptr %p) {
7+
call void @weak_odr_caller_of_foo_1(ptr %p)
8+
call void @linkonce_odr_caller_of_foo_2(ptr %p)
9+
call void @weak_odr_caller_of_foo_3(ptr %p)
10+
ret void
11+
}
12+
13+
define weak_odr void @weak_odr_caller_of_foo_1(ptr %p) {
14+
entry:
15+
tail call void @foo(ptr %p)
16+
tail call void @foo(ptr %p)
17+
tail call void @foo(ptr %p)
18+
ret void
19+
}
20+
21+
define linkonce_odr void @linkonce_odr_caller_of_foo_2(ptr %p) {
22+
entry:
23+
tail call void @foo(ptr %p)
24+
tail call void @foo(ptr %p)
25+
tail call void @foo(ptr %p)
26+
ret void
27+
}
28+
29+
define weak_odr void @weak_odr_caller_of_foo_3(ptr %p) {
30+
entry:
31+
tail call void @foo(ptr %p)
32+
tail call void @foo(ptr %p)
33+
tail call void @foo(ptr %p)
34+
ret void
35+
}
36+
37+
declare void @foo(ptr)
38+
39+
;.
40+
; CHECK: @llvm.used = appending global [3 x ptr] [ptr @weak_odr_caller_of_foo_1, ptr @linkonce_odr_caller_of_foo_2, ptr @weak_odr_caller_of_foo_3]
41+
;.
42+
; CHECK-LABEL: define void @caller_of_callers(
43+
; CHECK-SAME: ptr [[P:%.*]]) {
44+
; CHECK-NEXT: call void @[[GLOB0:[0-9]+]](ptr [[P]])
45+
; CHECK-NEXT: call void @[[GLOB0]](ptr [[P]])
46+
; CHECK-NEXT: call void @[[GLOB0]](ptr [[P]])
47+
; CHECK-NEXT: ret void
48+
;
49+
;
50+
; CHECK-LABEL: define private void @0(
51+
; CHECK-SAME: ptr [[P:%.*]]) {
52+
; CHECK-NEXT: [[ENTRY:.*:]]
53+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
54+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
55+
; CHECK-NEXT: tail call void @foo(ptr [[P]])
56+
; CHECK-NEXT: ret void
57+
;
58+
;
59+
; CHECK-LABEL: define weak_odr void @weak_odr_caller_of_foo_1(
60+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
61+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
62+
; CHECK-NEXT: ret void
63+
;
64+
;
65+
; CHECK-LABEL: define linkonce_odr void @linkonce_odr_caller_of_foo_2(
66+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
67+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
68+
; CHECK-NEXT: ret void
69+
;
70+
;
71+
; CHECK-LABEL: define weak_odr void @weak_odr_caller_of_foo_3(
72+
; CHECK-SAME: ptr [[TMP0:%.*]]) {
73+
; CHECK-NEXT: tail call void @[[GLOB0]](ptr [[TMP0]])
74+
; CHECK-NEXT: ret void
75+
;

0 commit comments

Comments
 (0)