[lld] 557e1fa - [lld-macho] Extend ICF to literal sections
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 28 11:49:53 PDT 2021
Author: Jez Ng
Date: 2021-06-28T14:49:39-04:00
New Revision: 557e1fa02f470bd4f14b7aa4060430007332895a
URL: https://github.com/llvm/llvm-project/commit/557e1fa02f470bd4f14b7aa4060430007332895a
DIFF: https://github.com/llvm/llvm-project/commit/557e1fa02f470bd4f14b7aa4060430007332895a.diff
LOG: [lld-macho] Extend ICF to literal sections
Literal sections can be deduplicated before running ICF. That makes it
easy to compare them during ICF: we can tell if two literals are
constant-equal by comparing their offsets in their OutputSection.
LLD-ELF takes a similar approach.
Reviewed By: #lld-macho, gkm
Differential Revision: https://reviews.llvm.org/D104671
Added:
lld/test/MachO/icf-literals.s
Modified:
lld/MachO/Driver.cpp
lld/MachO/ICF.cpp
lld/MachO/InputSection.cpp
lld/MachO/Options.td
lld/MachO/SyntheticSections.cpp
lld/MachO/SyntheticSections.h
lld/MachO/Writer.cpp
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index f91fca0999042..1d06f19311c1b 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1080,7 +1080,9 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
config->emitBitcodeBundle = args.hasArg(OPT_bitcode_bundle);
config->emitDataInCodeInfo =
args.hasFlag(OPT_data_in_code_info, OPT_no_data_in_code_info, true);
- config->dedupLiterals = args.hasArg(OPT_deduplicate_literals);
+ config->icfLevel = getICFLevel(args);
+ config->dedupLiterals = args.hasArg(OPT_deduplicate_literals) ||
+ config->icfLevel != ICFLevel::none;
// FIXME: Add a commandline flag for this too.
config->zeroModTime = getenv("ZERO_AR_DATE");
@@ -1123,8 +1125,6 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
config->undefinedSymbolTreatment = getUndefinedSymbolTreatment(args);
- config->icfLevel = getICFLevel(args);
-
if (config->outputType == MH_EXECUTE)
config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
/*file=*/nullptr,
diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp
index ce49dc903d4bc..4ff8c578d56c2 100644
--- a/lld/MachO/ICF.cpp
+++ b/lld/MachO/ICF.cpp
@@ -104,23 +104,22 @@ static bool equalsVariable(const ConcatInputSection *ia,
if (isa<Defined>(sa)) {
const auto *da = dyn_cast<Defined>(sa);
const auto *db = dyn_cast<Defined>(sb);
- if (da->value != db->value)
- return false;
- if (da->isAbsolute() != db->isAbsolute())
- return false;
- if (da->isec) {
+ if (da->isec && db->isec) {
if (da->isec->kind() != db->isec->kind())
return false;
if (const auto *isecA = dyn_cast<ConcatInputSection>(da->isec)) {
const auto *isecB = cast<ConcatInputSection>(db->isec);
- if (isecA->icfEqClass[icfPass % 2] !=
- isecB->icfEqClass[icfPass % 2])
- return false;
- } else {
- // FIXME: implement ICF for other InputSection kinds
- return false;
+ return da->value == db->value && isecA->icfEqClass[icfPass % 2] ==
+ isecB->icfEqClass[icfPass % 2];
}
+ // Else we have two literal sections. References to them are
+ // constant-equal if their offsets in the output section are equal.
+ return da->isec->parent == db->isec->parent &&
+ da->isec->getOffset(da->value) ==
+ db->isec->getOffset(db->value);
}
+ assert(da->isAbsolute() && db->isAbsolute());
+ return da->value == db->value;
} else if (isa<DylibSymbol>(sa)) {
// There is one DylibSymbol per gotIndex and we already checked for
// symbol equality, thus we know that these must be
diff erent.
@@ -135,14 +134,13 @@ static bool equalsVariable(const ConcatInputSection *ia,
return false;
if (const auto *isecA = dyn_cast<ConcatInputSection>(sa)) {
const auto *isecB = cast<ConcatInputSection>(sb);
- if (isecA->icfEqClass[icfPass % 2] != isecB->icfEqClass[icfPass % 2])
- return false;
+ return isecA->icfEqClass[icfPass % 2] == isecB->icfEqClass[icfPass % 2];
} else {
- // FIXME: implement ICF for other InputSection kinds
- return false;
+ assert(isa<CStringInputSection>(sa) ||
+ isa<WordLiteralInputSection>(sa));
+ return sa->getOffset(ra.addend) == sb->getOffset(rb.addend);
}
}
- return true;
};
return std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(),
f);
@@ -207,11 +205,15 @@ void ICF::run() {
if (auto *dylibSym = dyn_cast<DylibSymbol>(sym))
hash += dylibSym->stubsHelperIndex;
else if (auto *defined = dyn_cast<Defined>(sym)) {
- hash += defined->value;
- if (defined->isec)
- if (auto *isec = cast<ConcatInputSection>(defined->isec))
- hash += isec->icfEqClass[icfPass % 2];
- // FIXME: implement ICF for other InputSection kinds
+ if (defined->isec) {
+ if (auto isec = dyn_cast<ConcatInputSection>(defined->isec))
+ hash += defined->value + isec->icfEqClass[icfPass % 2];
+ else
+ hash += defined->isec->kind() +
+ defined->isec->getOffset(defined->value);
+ } else {
+ hash += defined->value;
+ }
} else
llvm_unreachable("foldIdenticalSections symbol kind");
}
diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index 78a7f00a18c5c..a961807abd230 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -62,9 +62,7 @@ bool ConcatInputSection::isHashableForICF(bool isText) const {
case S_8BYTE_LITERALS:
case S_16BYTE_LITERALS:
case S_LITERAL_POINTERS:
- // FIXME(jezng): We should not have any ConcatInputSections of these types
- // when running ICF.
- return false;
+ llvm_unreachable("found unexpected literal type in ConcatInputSection");
case S_ZEROFILL:
case S_GB_ZEROFILL:
case S_NON_LAZY_SYMBOL_POINTERS:
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index b115f43594f31..ebff0d5813a02 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -55,7 +55,7 @@ def time_trace_file_eq: Joined<["--"], "time-trace-file=">,
HelpText<"Specify time trace output file">,
Group<grp_lld>;
def deduplicate_literals: Flag<["--"], "deduplicate-literals">,
- HelpText<"Enable literal deduplication">,
+ HelpText<"Enable literal deduplication. This is implied by --icf={safe,all}">,
Group<grp_lld>;
def print_dylib_search: Flag<["--"], "print-dylib-search">,
HelpText<"Print which paths lld searched when trying to find dylibs">,
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 563b6e2ab605a..81fec04275295 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -1188,7 +1188,7 @@ void CStringSection::addInput(CStringInputSection *isec) {
inputs.push_back(isec);
}
-void CStringSection::finalize() {
+void CStringSection::finalizeContents() {
// Add all string pieces to the string table builder to create section
// contents.
for (const CStringInputSection *isec : inputs)
diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 3b2605871009a..a5f6ea9a6e1f4 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -523,7 +523,7 @@ class CStringSection final : public SyntheticSection {
CStringSection();
void addInput(CStringInputSection *);
uint64_t getSize() const override { return builder.getSize(); }
- void finalize() override;
+ void finalizeContents();
bool isNeeded() const override { return !inputs.empty(); }
void writeTo(uint8_t *buf) const override { builder.write(buf); }
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 5dab4d1aa3145..9dca3416875b6 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -52,6 +52,7 @@ class Writer {
void scanSymbols();
template <class LP> void createOutputSections();
template <class LP> void createLoadCommands();
+ void foldIdenticalLiterals();
void foldIdenticalSections();
void finalizeAddresses();
void finalizeLinkEditSegment();
@@ -942,6 +943,12 @@ template <class LP> void Writer::createOutputSections() {
linkEditSegment = getOrCreateOutputSegment(segment_names::linkEdit);
}
+void Writer::foldIdenticalLiterals() {
+ if (in.cStringSection)
+ in.cStringSection->finalizeContents();
+ // TODO: WordLiteralSection & CFStringSection should be finalized here too
+}
+
void Writer::foldIdenticalSections() {
if (config->icfLevel == ICFLevel::none)
return;
@@ -973,8 +980,8 @@ void Writer::foldIdenticalSections() {
else
concatIsec->icfEqClass[0] = ++icfUniqueID;
}
- // FIXME: hash literal sections here?
}
+ // FIXME: hash literal sections here too?
parallelForEach(hashable,
[](ConcatInputSection *isec) { isec->hashForICF(); });
// Now that every input section is either hashed or marked as unique,
@@ -1118,6 +1125,9 @@ template <class LP> void Writer::run() {
in.stubHelper->setup();
scanSymbols();
createOutputSections<LP>();
+ // ICF assumes that all literals have been folded already, so we must run
+ // foldIdenticalLiterals before foldIdenticalSections.
+ foldIdenticalLiterals();
foldIdenticalSections();
// After this point, we create no new segments; HOWEVER, we might
// yet create branch-range extension thunks for architectures whose
diff --git a/lld/test/MachO/icf-literals.s b/lld/test/MachO/icf-literals.s
new file mode 100644
index 0000000000000..dbe0490dd6848
--- /dev/null
+++ b/lld/test/MachO/icf-literals.s
@@ -0,0 +1,86 @@
+# REQUIRES: x86
+# RUN: rm -rf %t; mkdir %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -lSystem --icf=all -o %t/test %t/test.o
+# RUN: llvm-objdump --macho --syms -d %t/test | FileCheck %s
+
+# CHECK: _main:
+# CHECK-NEXT: callq _foo2_ref
+# CHECK-NEXT: callq _foo2_ref
+# CHECK-NEXT: callq _bar2_ref
+# CHECK-NEXT: callq _bar2_ref
+# CHECK-NEXT: callq _baz2_ref
+# CHECK-NEXT: callq _baz2_ref
+# CHECK-NEXT: callq _qux2_ref
+# CHECK-NEXT: callq _qux2_ref
+
+# CHECK: [[#%.16x,FOO:]] l O __TEXT,__cstring _foo1
+# CHECK-NEXT: [[#%.16x,FOO:]] l O __TEXT,__cstring _foo2
+# CHECK-NEXT: [[#%.16x,BAR:]] l O __TEXT,__cstring _bar1
+# CHECK-NEXT: [[#%.16x,BAR:]] l O __TEXT,__cstring _bar2
+# CHECK-NEXT: [[#%.16x,BAZ:]] l O __TEXT,__literals _baz1
+# CHECK-NEXT: [[#%.16x,BAZ:]] l O __TEXT,__literals _baz2
+# CHECK-NEXT: [[#%.16x,QUX:]] l O __TEXT,__literals _qux1
+# CHECK-NEXT: [[#%.16x,QUX:]] l O __TEXT,__literals _qux2
+# CHECK-NEXT: [[#%.16x,FOO_REF:]] l F __TEXT,__text _foo1_ref
+# CHECK-NEXT: [[#%.16x,FOO_REF:]] l F __TEXT,__text _foo2_ref
+# CHECK-NEXT: [[#%.16x,BAR_REF:]] l F __TEXT,__text _bar1_ref
+# CHECK-NEXT: [[#%.16x,BAR_REF:]] l F __TEXT,__text _bar2_ref
+# CHECK-NEXT: [[#%.16x,BAZ_REF:]] l F __TEXT,__text _baz1_ref
+# CHECK-NEXT: [[#%.16x,BAZ_REF:]] l F __TEXT,__text _baz2_ref
+# CHECK-NEXT: [[#%.16x,QUX_REF:]] l F __TEXT,__text _qux1_ref
+# CHECK-NEXT: [[#%.16x,QUX_REF:]] l F __TEXT,__text _qux2_ref
+
+## _foo1 vs _bar1: same section,
diff erent offsets
+## _foo1 vs _baz1: same offset,
diff erent sections
+
+.cstring
+_foo1:
+ .asciz "foo"
+_foo2:
+ .asciz "foo"
+_bar1:
+ .asciz "bar"
+_bar2:
+ .asciz "bar"
+
+.literal8
+_baz1:
+ .quad 0xdead
+_baz2:
+ .quad 0xdead
+_qux1:
+ .quad 0xbeef
+_qux2:
+ .quad 0xbeef
+
+.text
+_foo1_ref:
+ mov _foo1 at GOTPCREL(%rip), %rax
+_foo2_ref:
+ mov _foo2 at GOTPCREL(%rip), %rax
+_bar1_ref:
+ mov _bar1 at GOTPCREL(%rip), %rax
+_bar2_ref:
+ mov _bar2 at GOTPCREL(%rip), %rax
+_baz1_ref:
+ mov _baz1 at GOTPCREL(%rip), %rax
+_baz2_ref:
+ mov _baz2 at GOTPCREL(%rip), %rax
+_qux1_ref:
+ mov _qux1 at GOTPCREL(%rip), %rax
+_qux2_ref:
+ mov _qux2 at GOTPCREL(%rip), %rax
+
+.globl _main
+_main:
+ callq _foo1_ref
+ callq _foo2_ref
+ callq _bar1_ref
+ callq _bar2_ref
+ callq _baz1_ref
+ callq _baz2_ref
+ callq _qux1_ref
+ callq _qux2_ref
+
+.subsections_via_symbols
More information about the llvm-commits
mailing list