[lld] [lld] Merge equivalent symbols found during ICF (PR #134342)

Pranav Kant via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 18 15:53:59 PDT 2025


https://github.com/pranavk updated https://github.com/llvm/llvm-project/pull/134342

>From b78a951bfb497c91de0bea04fca36c4ec68583e5 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Fri, 4 Apr 2025 03:44:16 +0000
Subject: [PATCH 1/7] [lld] Merge equivalent symbols found during ICF

---
 lld/ELF/ICF.cpp                        | 51 ++++++++++++++++++-
 lld/ELF/SymbolTable.cpp                |  7 +++
 lld/ELF/SymbolTable.h                  |  1 +
 lld/test/ELF/aarch64-got-merging-icf.s | 68 ++++++++++++++++++++++++++
 lld/test/ELF/icf-preemptible.s         |  3 ++
 5 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 lld/test/ELF/aarch64-got-merging-icf.s

diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index 1cdcf6be9d8a9..467487e10f310 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -333,6 +333,28 @@ bool ICF<ELFT>::equalsConstant(const InputSection *a, const InputSection *b) {
              : constantEq(a, ra.relas, b, rb.relas);
 }
 
+template <class RelTy>
+static SmallVector<Symbol *> getReloc(const InputSection *sec,
+                                      Relocs<RelTy> relocs) {
+  SmallVector<Symbol *> syms;
+  for (auto ri = relocs.begin(), re = relocs.end(); ri != re; ++ri) {
+    Symbol &sym = sec->file->getRelocTargetSym(*ri);
+    syms.push_back(&sym);
+  }
+  return syms;
+}
+
+template <class ELFT>
+static SmallVector<Symbol *> getRelocTargetSyms(const InputSection *sec) {
+  const RelsOrRelas<ELFT> rel = sec->template relsOrRelas<ELFT>();
+  if (rel.areRelocsCrel())
+    return getReloc(sec, rel.crels);
+  if (rel.areRelocsRel())
+    return getReloc(sec, rel.rels);
+
+  return getReloc(sec, rel.relas);
+}
+
 // Compare two lists of relocations. Returns true if all pairs of
 // relocations point to the same section in terms of ICF.
 template <class ELFT>
@@ -535,14 +557,35 @@ template <class ELFT> void ICF<ELFT>::run() {
   auto print = [&ctx = ctx]() -> ELFSyncStream {
     return {ctx, ctx.arg.printIcfSections ? DiagLevel::Msg : DiagLevel::None};
   };
+
+  DenseMap<Symbol *, Symbol *> symbolMap;
   // Merge sections by the equivalence class.
+  // Merge symbols identified as equivalent during ICF
   forEachClassRange(0, sections.size(), [&](size_t begin, size_t end) {
     if (end - begin == 1)
       return;
     print() << "selected section " << sections[begin];
+    SmallVector<Symbol *> syms = getRelocTargetSyms<ELFT>(sections[begin]);
     for (size_t i = begin + 1; i < end; ++i) {
       print() << "  removing identical section " << sections[i];
       sections[begin]->replace(sections[i]);
+      SmallVector<Symbol *> replacedSyms =
+          getRelocTargetSyms<ELFT>(sections[i]);
+      assert(syms.size() == replacedSyms.size() &&
+             "Should have same number of syms!");
+      for (size_t i = 0; i < syms.size(); i++) {
+        if (syms[i] == replacedSyms[i] || !syms[i]->isGlobal() ||
+            !replacedSyms[i]->isGlobal())
+          continue;
+        auto [it, inserted] =
+            symbolMap.insert(std::make_pair(replacedSyms[i], syms[i]));
+        print() << "  selected symbol: " << syms[i]->getName().data()
+                << "; replaced symbol: " << replacedSyms[i]->getName().data();
+        if (!inserted) {
+          print() << "    replacement already exists: "
+                  << it->getSecond()->getName().data();
+        }
+      }
 
       // At this point we know sections merged are fully identical and hence
       // we want to remove duplicate implicit dependencies such as link order
@@ -561,11 +604,17 @@ template <class ELFT> void ICF<ELFT>::run() {
           d->folded = true;
         }
   };
-  for (Symbol *sym : ctx.symtab->getSymbols())
+  for (Symbol *sym : ctx.symtab->getSymbols()) {
     fold(sym);
+    if (Symbol *s = symbolMap.lookup(sym))
+      ctx.symtab->redirect(sym, s);
+  }
   parallelForEach(ctx.objectFiles, [&](ELFFileBase *file) {
     for (Symbol *sym : file->getLocalSymbols())
       fold(sym);
+    for (Symbol *&sym : file->getMutableGlobalSymbols())
+      if (Symbol *s = symbolMap.lookup(sym))
+        sym = s;
   });
 
   // InputSectionDescription::sections is populated by processSectionCommands().
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index b8a70d4e898fc..2199f159692b0 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -29,6 +29,13 @@ using namespace llvm::ELF;
 using namespace lld;
 using namespace lld::elf;
 
+void SymbolTable::redirect(Symbol *from, Symbol *to) {
+  int &fromIdx = symMap[CachedHashStringRef(from->getName())];
+  const int toIdx = symMap[CachedHashStringRef(to->getName())];
+
+  fromIdx = toIdx;
+}
+
 void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
   // Redirect __real_foo to the original foo and foo to the original __wrap_foo.
   int &idx1 = symMap[CachedHashStringRef(sym->getName())];
diff --git a/lld/ELF/SymbolTable.h b/lld/ELF/SymbolTable.h
index d6443742f7baa..e3a39bac85f97 100644
--- a/lld/ELF/SymbolTable.h
+++ b/lld/ELF/SymbolTable.h
@@ -41,6 +41,7 @@ class SymbolTable {
   SymbolTable(Ctx &ctx) : ctx(ctx) {}
   ArrayRef<Symbol *> getSymbols() const { return symVector; }
 
+  void redirect(Symbol *from, Symbol *to);
   void wrap(Symbol *sym, Symbol *real, Symbol *wrap);
 
   Symbol *insert(StringRef name);
diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
new file mode 100644
index 0000000000000..9f359cbf3a0a9
--- /dev/null
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -0,0 +1,68 @@
+// REQUIRES: aarch64
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t
+# RUN: ld.lld %t -o %t2 --icf=all
+# RUN: llvm-objdump --section-headers %t2 | FileCheck %s --check-prefix=EXE
+
+# RUN: ld.lld -shared %t -o %t3 --icf=all
+# RUN: llvm-objdump --section-headers %t3 | FileCheck %s --check-prefix=DSO
+
+## All .rodata.* sections should merge into a single GOT entry
+# EXE: {{.*}}.got 00000008{{.*}}
+
+## When symbols are preemptible in DSO mode, GOT entries wouldn't be merged
+# DSO: {{.*}}.got 00000020{{.*}}
+
+.addrsig
+
+callee:
+ret
+
+.section .rodata.dummy1,"a", at progbits
+sym1:
+.long 111
+.long 122
+.byte 123
+
+.section .rodata.dummy2,"a", at progbits
+sym2:
+.long 111
+.long 122
+sym3:
+.byte 123
+
+.macro f, index
+
+.section .text.f1_\index,"ax", at progbits
+f1_\index:
+adrp x0, :got:g\index
+mov x1, #\index
+b f2_\index
+
+.section .text.f2_\index,"ax", at progbits
+f2_\index:
+ldr x0, [x0, :got_lo12:g\index] 
+b callee
+
+.globl g\index
+.section .rodata.g\index,"a", at progbits
+g_\index:
+.long 111
+.long 122
+
+g\index:
+.byte 123
+
+.section .text._start,"ax", at progbits
+bl f1_\index
+
+.endm
+
+.section .text._start,"ax", at progbits
+.globl _start
+_start:
+
+f 0
+f 1
+f 2
+f 3
diff --git a/lld/test/ELF/icf-preemptible.s b/lld/test/ELF/icf-preemptible.s
index 4bd1eca438b19..f79cf73b911ba 100644
--- a/lld/test/ELF/icf-preemptible.s
+++ b/lld/test/ELF/icf-preemptible.s
@@ -11,12 +11,15 @@
 # EXE-NOT:  {{.}}
 # EXE:      selected section {{.*}}:(.text.g1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.g2)
+# EXE-NEXT:   selected symbol: f1; replaced symbol: f2
 # EXE-NEXT:   removing identical section {{.*}}:(.text.g3)
 # EXE-NEXT: selected section {{.*}}:(.text.f1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.f2)
 # EXE-NEXT: selected section {{.*}}:(.text.h1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h2)
+# EXE-NEXT:   selected symbol: g1; replaced symbol: g2
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h3)
+# EXE-NEXT:   selected symbol: g1; replaced symbol: g3
 # EXE-NOT:  {{.}}
 
 ## Definitions are preemptible in a DSO. Only leaf functions can be folded.

>From df987d52fd2cf139896163b483b0db831ad0055a Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Fri, 4 Apr 2025 04:06:17 +0000
Subject: [PATCH 2/7] add comments to test case

---
 lld/test/ELF/aarch64-got-merging-icf.s | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index 9f359cbf3a0a9..c5717106d582a 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -33,17 +33,20 @@ sym3:
 
 .macro f, index
 
+# (Kept unique) first instruction of the GOT code sequence
 .section .text.f1_\index,"ax", at progbits
 f1_\index:
 adrp x0, :got:g\index
 mov x1, #\index
 b f2_\index
 
+# Folded, second instruction of the GOT code sequence
 .section .text.f2_\index,"ax", at progbits
 f2_\index:
 ldr x0, [x0, :got_lo12:g\index] 
 b callee
 
+# Folded
 .globl g\index
 .section .rodata.g\index,"a", at progbits
 g_\index:

>From 5800242e4248f3a9476c8db76e88c55bd7d91ccd Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Mon, 14 Apr 2025 23:16:07 +0000
Subject: [PATCH 3/7] modify test case to introduce local symbols

---
 lld/test/ELF/aarch64-got-merging-icf.s | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index c5717106d582a..dff026be80f26 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -18,20 +18,7 @@
 callee:
 ret
 
-.section .rodata.dummy1,"a", at progbits
-sym1:
-.long 111
-.long 122
-.byte 123
-
-.section .rodata.dummy2,"a", at progbits
-sym2:
-.long 111
-.long 122
-sym3:
-.byte 123
-
-.macro f, index
+.macro f, index, isglobal
 
 # (Kept unique) first instruction of the GOT code sequence
 .section .text.f1_\index,"ax", at progbits
@@ -47,7 +34,9 @@ ldr x0, [x0, :got_lo12:g\index]
 b callee
 
 # Folded
+.ifnb \isglobal
 .globl g\index
+.endif
 .section .rodata.g\index,"a", at progbits
 g_\index:
 .long 111
@@ -65,7 +54,7 @@ bl f1_\index
 .globl _start
 _start:
 
-f 0
-f 1
-f 2
-f 3
+f 0 1
+f 1 1
+f 2 1
+f 3
\ No newline at end of file

>From 35489bfad31907c90e8fe139e91ae45a0e59c61d Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Mon, 14 Apr 2025 23:39:32 +0000
Subject: [PATCH 4/7] modify test

---
 lld/test/ELF/aarch64-got-merging-icf.s | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index dff026be80f26..7623b766a7b11 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -8,7 +8,7 @@
 # RUN: llvm-objdump --section-headers %t3 | FileCheck %s --check-prefix=DSO
 
 ## All .rodata.* sections should merge into a single GOT entry
-# EXE: {{.*}}.got 00000008{{.*}}
+# EXE: {{.*}}.got 00000010{{.*}}
 
 ## When symbols are preemptible in DSO mode, GOT entries wouldn't be merged
 # DSO: {{.*}}.got 00000020{{.*}}

>From 635d348f07b1438f9cb7c117da5004f14b154697 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Tue, 15 Apr 2025 22:49:41 +0000
Subject: [PATCH 5/7] use equivalence classes to simplify and handle more cases

---
 lld/ELF/ICF.cpp                        | 30 ++++++++++++++------------
 lld/ELF/SymbolTable.cpp                |  1 -
 lld/test/ELF/aarch64-got-merging-icf.s | 29 +++++++++++++++++++++++--
 lld/test/ELF/icf-preemptible.s         |  7 +++---
 4 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index 467487e10f310..b220d916fc3ce 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -80,6 +80,7 @@
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
+#include "llvm/ADT/EquivalenceClasses.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Parallel.h"
@@ -558,7 +559,7 @@ template <class ELFT> void ICF<ELFT>::run() {
     return {ctx, ctx.arg.printIcfSections ? DiagLevel::Msg : DiagLevel::None};
   };
 
-  DenseMap<Symbol *, Symbol *> symbolMap;
+  EquivalenceClasses<Symbol *> symbolEquivalence;
   // Merge sections by the equivalence class.
   // Merge symbols identified as equivalent during ICF
   forEachClassRange(0, sections.size(), [&](size_t begin, size_t end) {
@@ -577,14 +578,7 @@ template <class ELFT> void ICF<ELFT>::run() {
         if (syms[i] == replacedSyms[i] || !syms[i]->isGlobal() ||
             !replacedSyms[i]->isGlobal())
           continue;
-        auto [it, inserted] =
-            symbolMap.insert(std::make_pair(replacedSyms[i], syms[i]));
-        print() << "  selected symbol: " << syms[i]->getName().data()
-                << "; replaced symbol: " << replacedSyms[i]->getName().data();
-        if (!inserted) {
-          print() << "    replacement already exists: "
-                  << it->getSecond()->getName().data();
-        }
+        symbolEquivalence.unionSets(syms[i], replacedSyms[i]);
       }
 
       // At this point we know sections merged are fully identical and hence
@@ -606,15 +600,23 @@ template <class ELFT> void ICF<ELFT>::run() {
   };
   for (Symbol *sym : ctx.symtab->getSymbols()) {
     fold(sym);
-    if (Symbol *s = symbolMap.lookup(sym))
-      ctx.symtab->redirect(sym, s);
+    auto it = symbolEquivalence.findLeader(sym);
+    if (it != symbolEquivalence.member_end() && *it != sym) {
+      print() << "Redirecting " << sym->getName() << " to " << (*it)->getName();
+      ctx.symtab->redirect(sym, *it);
+    }
   }
   parallelForEach(ctx.objectFiles, [&](ELFFileBase *file) {
     for (Symbol *sym : file->getLocalSymbols())
       fold(sym);
-    for (Symbol *&sym : file->getMutableGlobalSymbols())
-      if (Symbol *s = symbolMap.lookup(sym))
-        sym = s;
+    for (Symbol *&sym : file->getMutableGlobalSymbols()) {
+      auto it = symbolEquivalence.findLeader(sym);
+      if (it != symbolEquivalence.member_end() && *it != sym) {
+        print() << "Redirecting " << sym->getName() << " to "
+                << (*it)->getName();
+        sym = *it;
+      }
+    }
   });
 
   // InputSectionDescription::sections is populated by processSectionCommands().
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 2199f159692b0..91e47e15b01a4 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -32,7 +32,6 @@ using namespace lld::elf;
 void SymbolTable::redirect(Symbol *from, Symbol *to) {
   int &fromIdx = symMap[CachedHashStringRef(from->getName())];
   const int toIdx = symMap[CachedHashStringRef(to->getName())];
-
   fromIdx = toIdx;
 }
 
diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index 7623b766a7b11..7c26b836873b8 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -11,7 +11,7 @@
 # EXE: {{.*}}.got 00000010{{.*}}
 
 ## When symbols are preemptible in DSO mode, GOT entries wouldn't be merged
-# DSO: {{.*}}.got 00000020{{.*}}
+# DSO: {{.*}}.got 00000028{{.*}}
 
 .addrsig
 
@@ -50,6 +50,30 @@ bl f1_\index
 
 .endm
 
+# another set of sections merging: g1 <- g2
+
+.section .text.t1_0,"ax", at progbits
+t1_0:
+adrp x2, :got:g1
+mov x3, #1
+b t2_0
+
+.section .text.t2_0,"ax", at progbits
+t2_0:
+ldr x2, [x2, :got_lo12:g1]
+b callee
+
+.section .text.t1_1,"ax", at progbits
+t1_1:
+adrp x2, :got:g2
+mov x3, #2
+b t2_1
+
+.section .text.t2_1,"ax", at progbits
+t2_1:
+ldr x2, [x2, :got_lo12:g2]
+b callee
+
 .section .text._start,"ax", at progbits
 .globl _start
 _start:
@@ -57,4 +81,5 @@ _start:
 f 0 1
 f 1 1
 f 2 1
-f 3
\ No newline at end of file
+f 3 1
+f 4
diff --git a/lld/test/ELF/icf-preemptible.s b/lld/test/ELF/icf-preemptible.s
index f79cf73b911ba..46a4399808d1f 100644
--- a/lld/test/ELF/icf-preemptible.s
+++ b/lld/test/ELF/icf-preemptible.s
@@ -11,16 +11,15 @@
 # EXE-NOT:  {{.}}
 # EXE:      selected section {{.*}}:(.text.g1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.g2)
-# EXE-NEXT:   selected symbol: f1; replaced symbol: f2
 # EXE-NEXT:   removing identical section {{.*}}:(.text.g3)
 # EXE-NEXT: selected section {{.*}}:(.text.f1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.f2)
 # EXE-NEXT: selected section {{.*}}:(.text.h1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h2)
-# EXE-NEXT:   selected symbol: g1; replaced symbol: g2
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h3)
-# EXE-NEXT:   selected symbol: g1; replaced symbol: g3
-# EXE-NOT:  {{.}}
+# EXE-NEXT: Redirecting f2 to f1
+# EXE-NEXT: Redirecting g2 to g1
+# EXE-NEXT: Redirecting g3 to g1
 
 ## Definitions are preemptible in a DSO. Only leaf functions can be folded.
 # DSO-NOT:  {{.}}

>From 12b28ca892ec5de487c271213e4439c5c6dbd123 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Wed, 16 Apr 2025 21:04:19 +0000
Subject: [PATCH 6/7] address maskray review comments

---
 lld/ELF/ICF.cpp                        |  7 ++++---
 lld/test/ELF/aarch64-got-merging-icf.s | 13 +++++++++++--
 lld/test/ELF/icf-preemptible.s         | 10 +++++++---
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index b220d916fc3ce..330fdba912e08 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -602,7 +602,8 @@ template <class ELFT> void ICF<ELFT>::run() {
     fold(sym);
     auto it = symbolEquivalence.findLeader(sym);
     if (it != symbolEquivalence.member_end() && *it != sym) {
-      print() << "Redirecting " << sym->getName() << " to " << (*it)->getName();
+      print() << "redirecting '" << sym->getName() << "' in symtab to '"
+              << (*it)->getName() << "'";
       ctx.symtab->redirect(sym, *it);
     }
   }
@@ -612,8 +613,8 @@ template <class ELFT> void ICF<ELFT>::run() {
     for (Symbol *&sym : file->getMutableGlobalSymbols()) {
       auto it = symbolEquivalence.findLeader(sym);
       if (it != symbolEquivalence.member_end() && *it != sym) {
-        print() << "Redirecting " << sym->getName() << " to "
-                << (*it)->getName();
+        print() << "redirecting '" << sym->getName() << "' to '"
+                << (*it)->getName() << "'";
         sym = *it;
       }
     }
diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index 7c26b836873b8..b37177a99e256 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -1,13 +1,21 @@
 // REQUIRES: aarch64
 
 # RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t
+# RUN: llvm-mc -filetype=obj -crel -triple=aarch64 %s -o %tcrel
 # RUN: ld.lld %t -o %t2 --icf=all
+# RUN: ld.lld %tcrel -o %tcrel2 --icf=all
+
 # RUN: llvm-objdump --section-headers %t2 | FileCheck %s --check-prefix=EXE
+# RUN: llvm-objdump --section-headers %tcrel2 | FileCheck %s --check-prefix=EXE
 
 # RUN: ld.lld -shared %t -o %t3 --icf=all
+# RUN: ld.lld -shared %tcrel -o %tcrel3 --icf=all
+
 # RUN: llvm-objdump --section-headers %t3 | FileCheck %s --check-prefix=DSO
+# RUN: llvm-objdump --section-headers %tcrel3 | FileCheck %s --check-prefix=DSO
 
-## All .rodata.* sections should merge into a single GOT entry
+## All global g* symbols should merge into a single GOT entry while non-global
+## gets its own GOT entry.
 # EXE: {{.*}}.got 00000010{{.*}}
 
 ## When symbols are preemptible in DSO mode, GOT entries wouldn't be merged
@@ -50,7 +58,8 @@ bl f1_\index
 
 .endm
 
-# another set of sections merging: g1 <- g2
+## Another set of sections merging: g1 <- g2. Linker should be able to
+## resolve both g1 and g2 to g0 based on ICF on previous sections.
 
 .section .text.t1_0,"ax", at progbits
 t1_0:
diff --git a/lld/test/ELF/icf-preemptible.s b/lld/test/ELF/icf-preemptible.s
index 46a4399808d1f..9352493600695 100644
--- a/lld/test/ELF/icf-preemptible.s
+++ b/lld/test/ELF/icf-preemptible.s
@@ -17,9 +17,13 @@
 # EXE-NEXT: selected section {{.*}}:(.text.h1)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h2)
 # EXE-NEXT:   removing identical section {{.*}}:(.text.h3)
-# EXE-NEXT: Redirecting f2 to f1
-# EXE-NEXT: Redirecting g2 to g1
-# EXE-NEXT: Redirecting g3 to g1
+# EXE-NEXT: redirecting 'f2' in symtab to 'f1'
+# EXE-NEXT: redirecting 'g2' in symtab to 'g1'
+# EXE-NEXT: redirecting 'g3' in symtab to 'g1'
+# EXE-NEXT: redirecting 'f2' to 'f1'
+# EXE-NEXT: redirecting 'g2' to 'g1'
+# EXE-NEXT: redirecting 'g3' to 'g1'
+# EXE-NOT:  {{.}}
 
 ## Definitions are preemptible in a DSO. Only leaf functions can be folded.
 # DSO-NOT:  {{.}}

>From d6847466b9a5648f1f1e51867743d758cb0f42a2 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Fri, 18 Apr 2025 22:53:42 +0000
Subject: [PATCH 7/7] maskray review round2

---
 lld/ELF/ICF.cpp                        | 2 +-
 lld/test/ELF/aarch64-got-merging-icf.s | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index 330fdba912e08..849f6bdd445f9 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -561,7 +561,7 @@ template <class ELFT> void ICF<ELFT>::run() {
 
   EquivalenceClasses<Symbol *> symbolEquivalence;
   // Merge sections by the equivalence class.
-  // Merge symbols identified as equivalent during ICF
+  // Merge symbols identified as equivalent during ICF.
   forEachClassRange(0, sections.size(), [&](size_t begin, size_t end) {
     if (end - begin == 1)
       return;
diff --git a/lld/test/ELF/aarch64-got-merging-icf.s b/lld/test/ELF/aarch64-got-merging-icf.s
index b37177a99e256..01d7bdfd75185 100644
--- a/lld/test/ELF/aarch64-got-merging-icf.s
+++ b/lld/test/ELF/aarch64-got-merging-icf.s
@@ -1,4 +1,5 @@
-// REQUIRES: aarch64
+## REQUIRES: aarch64
+## Check that symbols that ICF assumes to be the same get a single GOT entry
 
 # RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t
 # RUN: llvm-mc -filetype=obj -crel -triple=aarch64 %s -o %tcrel



More information about the llvm-commits mailing list