[lld] 18a9b18 - [COFF] Simplify ICF associated comdat handling
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 14 10:40:22 PDT 2021
Author: Reid Kleckner
Date: 2021-04-14T10:40:16-07:00
New Revision: 18a9b180870fcfc98da10ba9027f8eb5d0e7aba5
URL: https://github.com/llvm/llvm-project/commit/18a9b180870fcfc98da10ba9027f8eb5d0e7aba5
DIFF: https://github.com/llvm/llvm-project/commit/18a9b180870fcfc98da10ba9027f8eb5d0e7aba5.diff
LOG: [COFF] Simplify ICF associated comdat handling
This is a different approach from D98993 that should achieve most of the
same benefit. The two changes are:
1. Sort the list of associated child sections by section name
2. Do not consider associated sections to have children themselves
This fixes the main issue, which was that we sometimes considered an
.xdata section to have a child .pdata section. That lead to slow links
and larger binaries (less xdata folding).
Otherwise, this should be NFC: we go back to ignoring .debug/.gljmp and
other metadata sections rather than only looking at pdata/xdata. We
discovered that we do care about other associated sections, like ASan
global registration metadata.
Added:
lld/test/COFF/icf-assoc-order.s
lld/test/COFF/icf-xdata-last.s
Modified:
lld/COFF/Chunks.cpp
lld/COFF/Chunks.h
Removed:
################################################################################
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index b91398f1e34ed..35fe61f18fa82 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -454,11 +454,21 @@ void SectionChunk::writeAndRelocateSubsection(ArrayRef<uint8_t> sec,
}
void SectionChunk::addAssociative(SectionChunk *child) {
- // Insert this child at the head of the list.
+ // Insert the child section into the list of associated children. Keep the
+ // list ordered by section name so that ICF does not depend on section order.
assert(child->assocChildren == nullptr &&
"associated sections cannot have their own associated children");
- child->assocChildren = assocChildren;
- assocChildren = child;
+ SectionChunk *prev = this;
+ SectionChunk *next = assocChildren;
+ for (; next != nullptr; prev = next, next = next->assocChildren) {
+ if (next->getSectionName() <= child->getSectionName())
+ break;
+ }
+
+ // Insert child between prev and next.
+ assert(prev->assocChildren == next);
+ prev->assocChildren = child;
+ child->assocChildren = next;
}
static uint8_t getBaserelType(const coff_relocation &rel) {
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 49ee8b2a86294..4ecb371189a7a 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -293,8 +293,12 @@ class SectionChunk final : public Chunk {
// Allow iteration over the associated child chunks for this section.
llvm::iterator_range<AssociatedIterator> children() const {
- return llvm::make_range(AssociatedIterator(assocChildren),
- AssociatedIterator(nullptr));
+ // Associated sections do not have children. The assocChildren field is
+ // part of the parent's list of children.
+ bool isAssoc = selection == llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE;
+ return llvm::make_range(
+ AssociatedIterator(isAssoc ? nullptr : assocChildren),
+ AssociatedIterator(nullptr));
}
// The section ID this chunk belongs to in its Obj.
diff --git a/lld/test/COFF/icf-assoc-order.s b/lld/test/COFF/icf-assoc-order.s
new file mode 100644
index 0000000000000..45ddc0c6e7f70
--- /dev/null
+++ b/lld/test/COFF/icf-assoc-order.s
@@ -0,0 +1,52 @@
+# REQUIRES: x86
+# RUN: llvm-mc %s -filetype=obj -triple=x86_64-windows-msvc -o %t.obj
+# RUN: lld-link %t.obj -export:foo -export:bar -dll -noentry -out:%t.dll -verbose 2>&1 | FileCheck %s
+# RUN: llvm-readobj --sections %t.dll | FileCheck %s --check-prefix=TEXT
+
+# The order of the pdata and xdata sections here shouldn't matter. We should
+# still replace bar with foo.
+
+# CHECK: ICF needed {{.*}} iterations
+# CHECK: Selected foo
+# CHECK: Removed bar
+
+# We should only have five bytes of text.
+# TEXT: Name: .text
+# TEXT-NEXT: Size: 0x5
+
+ .section .text,"xr",discard,foo
+ .globl foo
+foo:
+ pushq %rbx
+ pushq %rdi
+ popq %rdi
+ popq %rbx
+ retq
+
+
+.section .pdata,"r",associative,foo
+.long foo
+.long 5
+.long foo_xdata at IMGREL
+
+.section .xdata,"r",associative,foo
+foo_xdata:
+.long 42
+
+ .section .text,"xr",discard,bar
+ .globl bar
+bar:
+ pushq %rbx
+ pushq %rdi
+ popq %rdi
+ popq %rbx
+ retq
+
+.section .xdata,"r",associative,bar
+bar_xdata:
+.long 42
+
+.section .pdata,"r",associative,bar
+.long bar
+.long 5
+.long bar_xdata at IMGREL
diff --git a/lld/test/COFF/icf-xdata-last.s b/lld/test/COFF/icf-xdata-last.s
new file mode 100644
index 0000000000000..d8ad7d394fb37
--- /dev/null
+++ b/lld/test/COFF/icf-xdata-last.s
@@ -0,0 +1,48 @@
+# REQUIRES: x86
+# RUN: llvm-mc %s -filetype=obj -triple=x86_64-windows-msvc -o %t.obj
+# RUN: lld-link %t.obj -export:foo -export:bar -dll -noentry -out:%t.dll -merge:.xdata=.xdata -verbose 2>&1 | FileCheck %s
+# RUN: llvm-readobj --sections %t.dll | FileCheck %s --check-prefix=XDATA
+
+# Test xdata can be merged when text and pdata
diff er. This test is structured
+# so that xdata comes after pdata, which makes xdata come before pdata in the
+# assocChildren linked list.
+
+# CHECK: ICF needed {{.*}} iterations
+# CHECK: Selected
+# CHECK: Removed
+
+# XDATA: Name: .xdata
+# XDATA-NEXT: VirtualSize: 0x4
+
+ .section .text,"xr",discard,foo
+ .globl foo
+foo:
+ pushq %rax
+ popq %rax
+ retq
+
+.section .pdata,"r",associative,foo
+.long foo
+.long 5
+.long foo_xdata at IMGREL
+
+
+.section .xdata,"r",associative,foo
+foo_xdata:
+.long 42
+
+ .section .text,"xr",discard,bar
+ .globl bar
+bar:
+ pushq %rcx
+ popq %rcx
+ retq
+
+.section .pdata,"r",associative,bar
+.long bar
+.long 5
+.long bar_xdata at IMGREL
+
+.section .xdata,"r",associative,bar
+bar_xdata:
+.long 42
More information about the llvm-commits
mailing list