[lld] 118bfde - [lld-macho] Have ICF dedup explicitly-defined selrefs

Jez Ng via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 14:59:37 PDT 2022


Author: Jez Ng
Date: 2022-09-14T17:59:22-04:00
New Revision: 118bfde90aed6fa4012cfd3167c033a78382350d

URL: https://github.com/llvm/llvm-project/commit/118bfde90aed6fa4012cfd3167c033a78382350d
DIFF: https://github.com/llvm/llvm-project/commit/118bfde90aed6fa4012cfd3167c033a78382350d.diff

LOG: [lld-macho] Have ICF dedup explicitly-defined selrefs

This is what ld64 does (though it doesn't use ICF to do this; instead it
always dedups selrefs by default).

We'll want to dedup implicitly-defined selrefs as well, but I will leave
that for future work.

Additionally, I'm not *super* happy with the current LLD implementation
because I think it is rather janky and inefficient. But at least it
moves us toward the goal of closing the size gap with ld64. I've
described ideas for cleaning up our implementation here:
https://github.com/llvm/llvm-project/issues/57714

Differential Revision: https://reviews.llvm.org/D133780

Added: 
    

Modified: 
    lld/MachO/ICF.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputSection.cpp
    lld/MachO/InputSection.h
    lld/test/MachO/objc-selrefs.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp
index e782260434254..4f4b8fd8e2a4f 100644
--- a/lld/MachO/ICF.cpp
+++ b/lld/MachO/ICF.cpp
@@ -414,25 +414,30 @@ void macho::foldIdenticalSections(bool onlyCfStrings) {
   std::vector<ConcatInputSection *> foldable;
   uint64_t icfUniqueID = inputSections.size();
   for (ConcatInputSection *isec : inputSections) {
-    bool isFoldableWithAddendsRemoved =
-        isCfStringSection(isec) || isClassRefsSection(isec);
+    bool isFoldableWithAddendsRemoved = isCfStringSection(isec) ||
+                                        isClassRefsSection(isec) ||
+                                        isSelRefsSection(isec);
+    // NOTE: __objc_selrefs is typically marked as no_dead_strip by MC, but we
+    // can still fold it.
+    bool hasFoldableFlags = (isSelRefsSection(isec) ||
+                             sectionType(isec->getFlags()) == MachO::S_REGULAR);
     // FIXME: consider non-code __text sections as foldable?
     bool isFoldable = (!onlyCfStrings || isCfStringSection(isec)) &&
-                      (isCodeSection(isec) || isGccExceptTabSection(isec) ||
-                       isFoldableWithAddendsRemoved) &&
+                      (isCodeSection(isec) || isFoldableWithAddendsRemoved ||
+                       isGccExceptTabSection(isec)) &&
                       !isec->keepUnique && !isec->shouldOmitFromOutput() &&
-                      sectionType(isec->getFlags()) == MachO::S_REGULAR;
+                      hasFoldableFlags;
     if (isFoldable) {
       foldable.push_back(isec);
       for (Defined *d : isec->symbols)
         if (d->unwindEntry)
           foldable.push_back(d->unwindEntry);
 
-      // __cfstring has embedded addends that foil ICF's hashing / equality
+      // Some sections have embedded addends that foil ICF's hashing / equality
       // checks. (We can ignore embedded addends when doing ICF because the same
       // information gets recorded in our Reloc structs.) We therefore create a
-      // mutable copy of the CFString and zero out the embedded addends before
-      // performing any hashing / equality checks.
+      // mutable copy of the the section data and zero out the embedded addends
+      // before performing any hashing / equality checks.
       if (isFoldableWithAddendsRemoved) {
         // We have to do this copying serially as the BumpPtrAllocator is not
         // thread-safe. FIXME: Make a thread-safe allocator.
@@ -443,7 +448,7 @@ void macho::foldIdenticalSections(bool onlyCfStrings) {
         isec->data = copy;
       }
     } else if (!isEhFrameSection(isec)) {
-      // EH frames are gathered as foldable from unwindEntry above; give a
+      // EH frames are gathered as foldables from unwindEntry above; give a
       // unique ID to everything else.
       isec->icfEqClass[0] = ++icfUniqueID;
     }

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 16f0b651b76d2..f68985f99b47c 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -274,6 +274,9 @@ static Optional<size_t> getRecordSize(StringRef segname, StringRef name) {
 
   if (name == section_names::objcClassRefs && segname == segment_names::data)
     return target->wordSize;
+
+  if (name == section_names::objcSelrefs && segname == segment_names::data)
+    return target->wordSize;
   return {};
 }
 

diff  --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index fb1b9291e30a5..f6a03649985e0 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -332,6 +332,11 @@ bool macho::isClassRefsSection(const InputSection *isec) {
          isec->getSegName() == segment_names::data;
 }
 
+bool macho::isSelRefsSection(const InputSection *isec) {
+  return isec->getName() == section_names::objcSelrefs &&
+         isec->getSegName() == segment_names::data;
+}
+
 bool macho::isEhFrameSection(const InputSection *isec) {
   return isec->getName() == section_names::ehFrame &&
          isec->getSegName() == segment_names::text;

diff  --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 58e75b156b558..2b8369142b802 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -285,6 +285,7 @@ inline bool isWordLiteralSection(uint32_t flags) {
 bool isCodeSection(const InputSection *);
 bool isCfStringSection(const InputSection *);
 bool isClassRefsSection(const InputSection *);
+bool isSelRefsSection(const InputSection *);
 bool isEhFrameSection(const InputSection *);
 bool isGccExceptTabSection(const InputSection *);
 

diff  --git a/lld/test/MachO/objc-selrefs.s b/lld/test/MachO/objc-selrefs.s
index e9ec6ca483e00..9dff440727ac3 100644
--- a/lld/test/MachO/objc-selrefs.s
+++ b/lld/test/MachO/objc-selrefs.s
@@ -1,11 +1,21 @@
 # REQUIRES: aarch64
 # RUN: rm -rf %t; split-file %s %t
 
-# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/existing.s -o %t/existing.o
-# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/main.s -o %t/main.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/explicit-selrefs-1.s -o %t/explicit-selrefs-1.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/explicit-selrefs-2.s -o %t/explicit-selrefs-2.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/implicit-selrefs.s -o %t/implicit-selrefs.o
 
-# RUN: %lld -arch arm64 -lSystem -o %t/out %t/existing.o %t/main.o
-# RUN: llvm-otool -vs __DATA __objc_selrefs %t/out | FileCheck %s --check-prefix=SELREFS
+# RUN: %lld -dylib -arch arm64 -lSystem -o %t/explicit-only-no-icf \
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o
+# RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-only-no-icf | \
+# RUN:   FileCheck %s --check-prefix=EXPLICIT-NO-ICF
+
+## NOTE: ld64 always dedups the selrefs unconditionally, but we only do it when
+## ICF is enabled.
+# RUN: %lld -dylib -arch arm64 -lSystem -o %t/explicit-only-with-icf \
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o
+# RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-only-with-icf \
+# RUN:   | FileCheck %s --check-prefix=EXPLICIT-WITH-ICF
 
 # SELREFS: Contents of (__DATA,__objc_selrefs) section
 # SELREFS-NEXT: __TEXT:__objc_methname:foo
@@ -14,18 +24,30 @@
 # SELREFS-NEXT: __TEXT:__objc_methname:length
 # SELREFS-EMPTY:
 
-# RUN: %lld -arch arm64 -lSystem -o %t/out %t/existing.o %t/main.o --deduplicate-literals
-# RUN: llvm-otool -vs __DATA __objc_selrefs %t/out | FileCheck %s --check-prefix=DEDUP
+## We don't yet support dedup'ing implicitly-defined selrefs.
+# RUN: %lld -dylib -arch arm64 -lSystem --icf=all -o %t/explicit-and-implicit \
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o %t/implicit-selrefs.o
+# RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-and-implicit \
+# RUN:   | FileCheck %s --check-prefix=EXPLICIT-AND-IMPLICIT
+
+# EXPLICIT-NO-ICF:       Contents of (__DATA,__objc_selrefs) section
+# EXPLICIT-NO-ICF-NEXT:  __TEXT:__objc_methname:foo
+# EXPLICIT-NO-ICF-NEXT:  __TEXT:__objc_methname:bar
+# EXPLICIT-NO-ICF-NEXT:  __TEXT:__objc_methname:bar
+# EXPLICIT-NO-ICF-NEXT:  __TEXT:__objc_methname:foo
+
+# EXPLICIT-WITH-ICF:      Contents of (__DATA,__objc_selrefs) section
+# EXPLICIT-WITH-ICF-NEXT: __TEXT:__objc_methname:foo
+# EXPLICIT-WITH-ICF-NEXT: __TEXT:__objc_methname:bar
 
-# DEDUP: Contents of (__DATA,__objc_selrefs) section
-# DEDUP-NEXT: __TEXT:__objc_methname:foo
-# DEDUP-NEXT: __TEXT:__objc_methname:bar
+# EXPLICIT-AND-IMPLICIT:      Contents of (__DATA,__objc_selrefs) section
+# EXPLICIT-AND-IMPLICIT-NEXT: __TEXT:__objc_methname:foo
+# EXPLICIT-AND-IMPLICIT-NEXT: __TEXT:__objc_methname:bar
 # NOTE: Ideally this wouldn't exist, but while it does it needs to point to the deduplicated string
-# DEDUP-NEXT: __TEXT:__objc_methname:foo
-# DEDUP-NEXT: __TEXT:__objc_methname:length
-# DEDUP-EMPTY:
+# EXPLICIT-AND-IMPLICIT-NEXT: __TEXT:__objc_methname:foo
+# EXPLICIT-AND-IMPLICIT-NEXT: __TEXT:__objc_methname:length
 
-#--- existing.s
+#--- explicit-selrefs-1.s
 .section  __TEXT,__objc_methname,cstring_literals
 lselref1:
   .asciz  "foo"
@@ -34,17 +56,28 @@ lselref2:
 
 .section  __DATA,__objc_selrefs,literal_pointers,no_dead_strip
 .p2align  3
-.quad lselref1
-.quad lselref2
+  .quad lselref1
+  .quad lselref2
+  .quad lselref2
+
+#--- explicit-selrefs-2.s
+.section  __TEXT,__objc_methname,cstring_literals
+lselref1:
+  .asciz  "foo"
+
+.section  __DATA,__objc_selrefs,literal_pointers,no_dead_strip
+.p2align  3
+  .quad lselref1
 
-#--- main.s
+#--- implicit-selrefs.s
 .text
 .globl _objc_msgSend
+.p2align 2
 _objc_msgSend:
   ret
 
-.globl _main
-_main:
-  bl  _objc_msgSend$length
-  bl  _objc_msgSend$foo
+.p2align 2
+_sender:
+  bl _objc_msgSend$length
+  bl _objc_msgSend$foo
   ret


        


More information about the llvm-commits mailing list