Skip to content

Commit fdaca77

Browse files
ellishgellishg
committed
order correctly and add more tests
1 parent 3c3233c commit fdaca77

File tree

8 files changed

+285
-80
lines changed

8 files changed

+285
-80
lines changed

‎lld/MachO/Config.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ struct Configuration {
223223
bool warnThinArchiveMissingMembers;
224224
bool disableVerify;
225225
bool separateCstringLiteralSections;
226+
bool tailMergeStrings;
226227

227228
bool callGraphProfileSort = false;
228229
llvm::StringRef printSymbolOrder;

‎lld/MachO/Driver.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,6 +1986,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
19861986
config->separateCstringLiteralSections =
19871987
args.hasFlag(OPT_separate_cstring_literal_sections,
19881988
OPT_no_separate_cstring_literal_sections, false);
1989+
config->tailMergeStrings =
1990+
args.hasFlag(OPT_tail_merge_strings, OPT_no_tail_merge_strings, false);
19891991

19901992
auto IncompatWithCGSort = [&](StringRef firstArgStr) {
19911993
// Throw an error only if --call-graph-profile-sort is explicitly specified

‎lld/MachO/Options.td‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,10 @@ defm separate_cstring_literal_sections
10911091
"Emit all cstring literals into the __cstring section. As a special "
10921092
"case, the __objc_methname section will still be emitted. (default)">,
10931093
Group<grp_rare>;
1094+
defm tail_merge_strings
1095+
: BB<"tail-merge-strings", "Enable string tail merging",
1096+
"Disable string tail merging to improve link-time performance">,
1097+
Group<grp_rare>;
10941098

10951099
def grp_deprecated : OptionGroup<"deprecated">, HelpText<"DEPRECATED">;
10961100

‎lld/MachO/SyntheticSections.cpp‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,7 @@ void CStringSection::finalizeContents() {
17461746
void DeduplicatedCStringSection::finalizeContents() {
17471747
// Find the largest alignment required for each string.
17481748
DenseMap<CachedHashStringRef, Align> strToAlignment;
1749+
// Used for tail merging only
17491750
std::vector<CachedHashStringRef> deduplicatedStrs;
17501751
for (const CStringInputSection *isec : inputs) {
17511752
for (const auto &[i, piece] : llvm::enumerate(isec->pieces)) {
@@ -1755,7 +1756,7 @@ void DeduplicatedCStringSection::finalizeContents() {
17551756
assert(isec->align != 0);
17561757
auto align = getStringPieceAlignment(isec, piece);
17571758
auto [it, wasInserted] = strToAlignment.try_emplace(s, align);
1758-
if (wasInserted)
1759+
if (config->tailMergeStrings && wasInserted)
17591760
deduplicatedStrs.push_back(s);
17601761
if (!wasInserted && it->second < align)
17611762
it->second = align;
@@ -1784,17 +1785,17 @@ void DeduplicatedCStringSection::finalizeContents() {
17841785
mergeCandidate = s;
17851786
continue;
17861787
}
1787-
uint64_t tailOffset = mergeCandidate->size() - s.size();
1788+
uint64_t tailMergeOffset = mergeCandidate->size() - s.size();
17881789
// TODO: If the tail offset is incompatible with this string's alignment, we
17891790
// might be able to find another superstring with a compatible tail offset.
17901791
// The difficulty is how to do this efficiently
17911792
const auto &align = strToAlignment.at(s);
1792-
if (!isAligned(align, tailOffset))
1793+
if (!isAligned(align, tailMergeOffset))
17931794
continue;
17941795
auto &mergeCandidateAlign = strToAlignment[*mergeCandidate];
17951796
if (align > mergeCandidateAlign)
17961797
mergeCandidateAlign = align;
1797-
tailMergeMap.try_emplace(s, *mergeCandidate, tailOffset);
1798+
tailMergeMap.try_emplace(s, *mergeCandidate, tailMergeOffset);
17981799
}
17991800

18001801
// Sort the strings for performance and compression size win, and then
@@ -1803,38 +1804,34 @@ void DeduplicatedCStringSection::finalizeContents() {
18031804
for (auto &[isec, i] : priorityBuilder.buildCStringPriorities(inputs)) {
18041805
auto &piece = isec->pieces[i];
18051806
auto s = isec->getCachedHashStringRef(i);
1806-
// Skip tail merged strings until their superstring offsets are resolved
1807-
if (tailMergeMap.count(s))
1808-
continue;
1807+
// Any string can be tail merged with itself with an offset of zero
1808+
uint64_t tailMergeOffset = 0;
1809+
auto mergeIt =
1810+
config->tailMergeStrings ? tailMergeMap.find(s) : tailMergeMap.end();
1811+
if (mergeIt != tailMergeMap.end()) {
1812+
auto &[superString, offset] = mergeIt->second;
1813+
// s can be tail merged with superString. Do not layout s. Instead layout
1814+
// superString if we haven't already
1815+
assert(superString.val().ends_with(s.val()));
1816+
s = superString;
1817+
tailMergeOffset = offset;
1818+
}
18091819
auto [it, wasInserted] = stringOffsetMap.try_emplace(s, /*placeholder*/ 0);
18101820
if (wasInserted) {
18111821
// Avoid computing the offset until we are sure we will need to
18121822
uint64_t offset = alignTo(size, strToAlignment.at(s));
18131823
it->second = offset;
18141824
size = offset + s.size() + 1; // account for null terminator
18151825
}
1816-
// If the string was already in stringOffsetMap, it is a duplicate and we
1817-
// only need to assign the offset.
1818-
piece.outSecOff = it->second;
1819-
}
1820-
for (CStringInputSection *isec : inputs) {
1821-
for (const auto &[i, piece] : llvm::enumerate(isec->pieces)) {
1822-
if (!piece.live)
1823-
continue;
1824-
auto s = isec->getCachedHashStringRef(i);
1825-
auto it = tailMergeMap.find(s);
1826-
if (it == tailMergeMap.end())
1827-
continue;
1828-
const auto &[superString, tailOffset] = it->second;
1829-
assert(superString.val().ends_with(s.val()));
1830-
assert(!tailMergeMap.count(superString));
1831-
auto &outSecOff = stringOffsetMap[s];
1832-
outSecOff = stringOffsetMap.at(superString) + tailOffset;
1833-
piece.outSecOff = outSecOff;
1834-
assert(isAligned(strToAlignment.at(s), piece.outSecOff));
1826+
piece.outSecOff = it->second + tailMergeOffset;
1827+
if (mergeIt != tailMergeMap.end()) {
1828+
auto &tailMergedString = mergeIt->first;
1829+
stringOffsetMap[tailMergedString] = piece.outSecOff;
1830+
assert(isAligned(strToAlignment.at(tailMergedString), piece.outSecOff));
18351831
}
1836-
isec->isFinal = true;
18371832
}
1833+
for (CStringInputSection *isec : inputs)
1834+
isec->isFinal = true;
18381835
}
18391836

18401837
void DeduplicatedCStringSection::writeTo(uint8_t *buf) const {

‎lld/test/MachO/cstring-dedup.s‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
# RUN: llvm-objdump --macho --section="__DATA,ptrs" --syms %t/test | FileCheck %s
99
# RUN: llvm-readobj --section-headers %t/test | FileCheck %s --check-prefix=HEADER
1010

11-
## Make sure we only have 2 deduplicated strings in __cstring.
11+
## Make sure we only have 3 deduplicated strings in __cstring.
1212
# STR: Contents of (__TEXT,__cstring) section
1313
# STR: {{[[:xdigit:]]+}} foo
1414
# STR: {{[[:xdigit:]]+}} barbaz
15+
# STR: {{[[:xdigit:]]+}} {{$}}
1516

1617
## Make sure both symbol and section relocations point to the right thing.
1718
# CHECK: Contents of (__DATA,ptrs) section

‎lld/test/MachO/cstring-tailmerge-objc.s‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
; REQUIRES: aarch64
2+
; RUN: rm -rf %t && split-file %s %t
3+
4+
; Test that ObjC method names are tail merged and
5+
; ObjCSelRefsHelper::makeSelRef() still works correctly
6+
7+
; RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/a.s -o %t/a.o
8+
; RUN: %lld -dylib -arch arm64 --deduplicate-strings --tail-merge-strings %t/a.o -o %t/a
9+
; RUN: llvm-objdump --macho --section="__TEXT,__objc_methname" %t/a | FileCheck %s --implicit-check-not=error
10+
11+
; RUN: %lld -dylib -arch arm64 --deduplicate-strings --no-tail-merge-strings %t/a.o -o %t/nomerge
12+
; RUN: llvm-objdump --macho --section="__TEXT,__objc_methname" %t/nomerge | FileCheck %s --check-prefixes=CHECK,NOMERGE --implicit-check-not=error
13+
14+
; CHECK: withBar:error:
15+
; NOMERGE: error:
16+
17+
;--- a.mm
18+
__attribute__((objc_root_class))
19+
@interface Foo
20+
- (void)withBar:(int)bar error:(int)error;
21+
- (void)error:(int)error;
22+
@end
23+
24+
@implementation Foo
25+
- (void)withBar:(int)bar error:(int)error {}
26+
- (void)error:(int)error {}
27+
@end
28+
29+
void *_objc_empty_cache;
30+
void *_objc_empty_vtable;
31+
;--- gen
32+
clang -Oz -target arm64-apple-darwin a.mm -S -o -
33+
;--- a.s
34+
.build_version macos, 11, 0
35+
.section __TEXT,__text,regular,pure_instructions
36+
.p2align 2 ; -- Begin function -[Foo withBar:error:]
37+
"-[Foo withBar:error:]": ; @"\01-[Foo withBar:error:]"
38+
.cfi_startproc
39+
; %bb.0:
40+
ret
41+
.cfi_endproc
42+
; -- End function
43+
.p2align 2 ; -- Begin function -[Foo error:]
44+
"-[Foo error:]": ; @"\01-[Foo error:]"
45+
.cfi_startproc
46+
; %bb.0:
47+
ret
48+
.cfi_endproc
49+
; -- End function
50+
.globl __objc_empty_vtable ; @_objc_empty_vtable
51+
.zerofill __DATA,__common,__objc_empty_vtable,8,3
52+
.section __DATA,__objc_data
53+
.globl _OBJC_CLASS_$_Foo ; @"OBJC_CLASS_$_Foo"
54+
.p2align 3, 0x0
55+
_OBJC_CLASS_$_Foo:
56+
.quad _OBJC_METACLASS_$_Foo
57+
.quad 0
58+
.quad __objc_empty_cache
59+
.quad __objc_empty_vtable
60+
.quad __OBJC_CLASS_RO_$_Foo
61+
62+
.globl _OBJC_METACLASS_$_Foo ; @"OBJC_METACLASS_$_Foo"
63+
.p2align 3, 0x0
64+
_OBJC_METACLASS_$_Foo:
65+
.quad _OBJC_METACLASS_$_Foo
66+
.quad _OBJC_CLASS_$_Foo
67+
.quad __objc_empty_cache
68+
.quad __objc_empty_vtable
69+
.quad __OBJC_METACLASS_RO_$_Foo
70+
71+
.section __TEXT,__objc_classname,cstring_literals
72+
l_OBJC_CLASS_NAME_: ; @OBJC_CLASS_NAME_
73+
.asciz "Foo"
74+
75+
.section __DATA,__objc_const
76+
.p2align 3, 0x0 ; @"_OBJC_METACLASS_RO_$_Foo"
77+
__OBJC_METACLASS_RO_$_Foo:
78+
.long 3 ; 0x3
79+
.long 40 ; 0x28
80+
.long 40 ; 0x28
81+
.space 4
82+
.quad 0
83+
.quad l_OBJC_CLASS_NAME_
84+
.quad 0
85+
.quad 0
86+
.quad 0
87+
.quad 0
88+
.quad 0
89+
90+
.section __TEXT,__objc_methname,cstring_literals
91+
l_OBJC_METH_VAR_NAME_: ; @OBJC_METH_VAR_NAME_
92+
.asciz "withBar:error:"
93+
94+
.section __TEXT,__objc_methtype,cstring_literals
95+
l_OBJC_METH_VAR_TYPE_: ; @OBJC_METH_VAR_TYPE_
96+
.asciz "v24@0:8i16i20"
97+
98+
.section __TEXT,__objc_methname,cstring_literals
99+
l_OBJC_METH_VAR_NAME_.1: ; @OBJC_METH_VAR_NAME_.1
100+
.asciz "error:"
101+
102+
.section __TEXT,__objc_methtype,cstring_literals
103+
l_OBJC_METH_VAR_TYPE_.2: ; @OBJC_METH_VAR_TYPE_.2
104+
.asciz "v20@0:8i16"
105+
106+
.section __DATA,__objc_const
107+
.p2align 3, 0x0 ; @"_OBJC_$_INSTANCE_METHODS_Foo"
108+
__OBJC_$_INSTANCE_METHODS_Foo:
109+
.long 24 ; 0x18
110+
.long 2 ; 0x2
111+
.quad l_OBJC_METH_VAR_NAME_
112+
.quad l_OBJC_METH_VAR_TYPE_
113+
.quad "-[Foo withBar:error:]"
114+
.quad l_OBJC_METH_VAR_NAME_.1
115+
.quad l_OBJC_METH_VAR_TYPE_.2
116+
.quad "-[Foo error:]"
117+
118+
.p2align 3, 0x0 ; @"_OBJC_CLASS_RO_$_Foo"
119+
__OBJC_CLASS_RO_$_Foo:
120+
.long 2 ; 0x2
121+
.long 0 ; 0x0
122+
.long 0 ; 0x0
123+
.space 4
124+
.quad 0
125+
.quad l_OBJC_CLASS_NAME_
126+
.quad __OBJC_$_INSTANCE_METHODS_Foo
127+
.quad 0
128+
.quad 0
129+
.quad 0
130+
.quad 0
131+
132+
.globl __objc_empty_cache ; @_objc_empty_cache
133+
.zerofill __DATA,__common,__objc_empty_cache,8,3
134+
.section __DATA,__objc_classlist,regular,no_dead_strip
135+
.p2align 3, 0x0 ; @"OBJC_LABEL_CLASS_$"
136+
l_OBJC_LABEL_CLASS_$:
137+
.quad _OBJC_CLASS_$_Foo
138+
139+
.section __DATA,__objc_imageinfo,regular,no_dead_strip
140+
L_OBJC_IMAGE_INFO:
141+
.long 0
142+
.long 64
143+
144+
.subsections_via_symbols

0 commit comments

Comments
 (0)