[lld] 7ce9a3e - [COFF] Only consider associated EH sections during ICF
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 22 15:36:32 PDT 2021
Author: Reid Kleckner
Date: 2021-03-22T15:36:26-07:00
New Revision: 7ce9a3e9a91bb0c71cd3560079ff4c31d5dade1b
URL: https://github.com/llvm/llvm-project/commit/7ce9a3e9a91bb0c71cd3560079ff4c31d5dade1b
DIFF: https://github.com/llvm/llvm-project/commit/7ce9a3e9a91bb0c71cd3560079ff4c31d5dade1b.diff
LOG: [COFF] Only consider associated EH sections during ICF
The only known reason why ICF should not merge otherwise identical
sections with differing associated sections has to do with exception
handling tables. It's not clear what ICF should do when there are other
kinds of associated sections. In every other case when this has come up,
debug info and CF guard metadata, we have opted to make ICF ignore the
associated sections.
For comparison, ELF doesn't do anything for comdat groups. Instead,
.eh_frame is parsed to figure out if a section has an LSDA, and if so,
ICF is disabled.
Another issue is that the order of associated sections is not defined.
We have had issues in the past (crbug.com/1144476) where changing the
order of the .xdata/.pdata sections in the object file lead to large ICF
slowdowns.
To address these issues, I decided it would be best to explicitly
consider only .pdata and .xdata sections during ICF. This makes it easy
to ignore the object file order, and I think it makes the intention of
the code clearer.
I've also made the children() accessor return an empty list for
associated sections. This mostly only affects ICF and GC. This was the
behavior before I made this a linked list, so the behavior change should
be good. This had positive effects on chrome.dll: more .xdata sections
were merged that previously could not be merged because they were
associated with distinct .pdata sections.
Reviewed By: mstorsjo
Differential Revision: https://reviews.llvm.org/D98993
Added:
lld/test/COFF/icf-assoc-order.s
lld/test/COFF/icf-xdata-last.s
Modified:
lld/COFF/Chunks.h
lld/COFF/ICF.cpp
Removed:
################################################################################
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index e076d8e71109..6e7af0babe58 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/COFF/ICF.cpp b/lld/COFF/ICF.cpp
index 732646967296..8e6b1a0bfcba 100644
--- a/lld/COFF/ICF.cpp
+++ b/lld/COFF/ICF.cpp
@@ -46,7 +46,7 @@ class ICF {
private:
void segregate(size_t begin, size_t end, bool constant);
- bool assocEquals(const SectionChunk *a, const SectionChunk *b);
+ bool equalsEHData(const SectionChunk *a, const SectionChunk *b);
bool equalsConstant(const SectionChunk *a, const SectionChunk *b);
bool equalsVariable(const SectionChunk *a, const SectionChunk *b);
@@ -127,21 +127,31 @@ void ICF::segregate(size_t begin, size_t end, bool constant) {
}
}
-// Returns true if two sections' associative children are equal.
-bool ICF::assocEquals(const SectionChunk *a, const SectionChunk *b) {
- // Ignore associated metadata sections that don't participate in ICF, such as
- // debug info and CFGuard metadata.
- auto considerForICF = [](const SectionChunk &assoc) {
- StringRef Name = assoc.getSectionName();
- return !(Name.startswith(".debug") || Name == ".gfids$y" ||
- Name == ".giats$y" || Name == ".gljmp$y");
+// Returns true if two sections have equivalent associated .pdata/.xdata
+// sections.
+bool ICF::equalsEHData(const SectionChunk *a, const SectionChunk *b) {
+ auto findEHData = [](const SectionChunk *s) {
+ const SectionChunk *pdata = nullptr;
+ const SectionChunk *xdata = nullptr;
+ for (const SectionChunk &assoc : s->children()) {
+ StringRef name = assoc.getSectionName();
+ if (name.startswith(".pdata") && (name.size() == 6 || name[6] == '$'))
+ pdata = &assoc;
+ else if (name.startswith(".xdata") &&
+ (name.size() == 6 || name[6] == '$'))
+ xdata = &assoc;
+ }
+ return std::make_pair(pdata, xdata);
+ };
+ auto aData = findEHData(a);
+ auto bData = findEHData(b);
+ auto considerEqual = [cnt = cnt](const SectionChunk *l,
+ const SectionChunk *r) {
+ return l == r || (l->getContents() == r->getContents() &&
+ l->eqClass[cnt % 2] == r->eqClass[cnt % 2]);
};
- auto ra = make_filter_range(a->children(), considerForICF);
- auto rb = make_filter_range(b->children(), considerForICF);
- return std::equal(ra.begin(), ra.end(), rb.begin(), rb.end(),
- [&](const SectionChunk &ia, const SectionChunk &ib) {
- return ia.eqClass[cnt % 2] == ib.eqClass[cnt % 2];
- });
+ return considerEqual(aData.first, bData.first) &&
+ considerEqual(aData.second, bData.second);
}
// Compare "non-moving" part of two sections, namely everything
@@ -175,7 +185,7 @@ bool ICF::equalsConstant(const SectionChunk *a, const SectionChunk *b) {
a->getSectionName() == b->getSectionName() &&
a->header->SizeOfRawData == b->header->SizeOfRawData &&
a->checksum == b->checksum && a->getContents() == b->getContents() &&
- assocEquals(a, b);
+ equalsEHData(a, b);
}
// Compare "moving" part of two sections, namely relocation targets.
@@ -193,7 +203,7 @@ bool ICF::equalsVariable(const SectionChunk *a, const SectionChunk *b) {
};
return std::equal(a->getRelocs().begin(), a->getRelocs().end(),
b->getRelocs().begin(), eq) &&
- assocEquals(a, b);
+ equalsEHData(a, b);
}
// Find the first Chunk after Begin that has a
diff erent class from Begin.
diff --git a/lld/test/COFF/icf-assoc-order.s b/lld/test/COFF/icf-assoc-order.s
new file mode 100644
index 000000000000..45ddc0c6e7f7
--- /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 000000000000..d8ad7d394fb3
--- /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