diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1080,7 +1080,9 @@ 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 @@ 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 --- a/lld/MachO/ICF.cpp +++ b/lld/MachO/ICF.cpp @@ -104,23 +104,22 @@ if (isa(sa)) { const auto *da = dyn_cast(sa); const auto *db = dyn_cast(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(da->isec)) { const auto *isecB = cast(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(sa)) { // There is one DylibSymbol per gotIndex and we already checked for // symbol equality, thus we know that these must be different. @@ -135,14 +134,13 @@ return false; if (const auto *isecA = dyn_cast(sa)) { const auto *isecB = cast(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(sa) || + isa(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 @@ if (auto *dylibSym = dyn_cast(sym)) hash += dylibSym->stubsHelperIndex; else if (auto *defined = dyn_cast(sym)) { - hash += defined->value; - if (defined->isec) - if (auto *isec = cast(defined->isec)) - hash += isec->icfEqClass[icfPass % 2]; - // FIXME: implement ICF for other InputSection kinds + if (defined->isec) { + if (auto isec = dyn_cast(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 --- a/lld/MachO/InputSection.cpp +++ b/lld/MachO/InputSection.cpp @@ -62,9 +62,7 @@ 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 --- a/lld/MachO/Options.td +++ b/lld/MachO/Options.td @@ -55,7 +55,7 @@ HelpText<"Specify time trace output file">, Group; def deduplicate_literals: Flag<["--"], "deduplicate-literals">, - HelpText<"Enable literal deduplication">, + HelpText<"Enable literal deduplication. This is implied by --icf={safe,all}">, Group; 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.h b/lld/MachO/SyntheticSections.h --- a/lld/MachO/SyntheticSections.h +++ b/lld/MachO/SyntheticSections.h @@ -523,7 +523,7 @@ 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/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp --- a/lld/MachO/SyntheticSections.cpp +++ b/lld/MachO/SyntheticSections.cpp @@ -1188,7 +1188,7 @@ 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/Writer.cpp b/lld/MachO/Writer.cpp --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -52,6 +52,7 @@ void scanSymbols(); template void createOutputSections(); template void createLoadCommands(); + void foldIdenticalLiterals(); void foldIdenticalSections(); void finalizeAddresses(); void finalizeLinkEditSegment(); @@ -942,6 +943,12 @@ 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 @@ 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 @@ in.stubHelper->setup(); scanSymbols(); createOutputSections(); + // 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 --- /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, different offsets +## _foo1 vs _baz1: same offset, different 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@GOTPCREL(%rip), %rax +_foo2_ref: + mov _foo2@GOTPCREL(%rip), %rax +_bar1_ref: + mov _bar1@GOTPCREL(%rip), %rax +_bar2_ref: + mov _bar2@GOTPCREL(%rip), %rax +_baz1_ref: + mov _baz1@GOTPCREL(%rip), %rax +_baz2_ref: + mov _baz2@GOTPCREL(%rip), %rax +_qux1_ref: + mov _qux1@GOTPCREL(%rip), %rax +_qux2_ref: + mov _qux2@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