[lld][macho] Ignore cstrings in bp orderer

[ellishg]

cstring sections are laid out by {Deduplicated}CStringSection::finalizeContents() and ignores the order determined by runBalancedPartitioning().

llvm-project/lld/MachO/SyntheticSections.cpp

Lines 1729 to 1746 in e63f0f5

	void CStringSection::finalizeContents() {
	size = 0;
	// TODO: Call buildCStringPriorities() to support cstring ordering when
	// deduplication is off, although this may negatively impact build
	// performance.
	for (CStringInputSection *isec : inputs) {
	for (const auto &[i, piece] : llvm::enumerate(isec->pieces)) {
	if (!piece.live)
	continue;
	piece.outSecOff = alignTo(size, getStringPieceAlignment(isec, piece));
	StringRef string = isec->getStringRef(i);
	size = piece.outSecOff + string.size() + 1; // account for null terminator
	}
	isec->isFinal = true;
	}
	}
	void DeduplicatedCStringSection::finalizeContents() {

Do not consider cstring sections or any section already finalized in runBalancedPartitioning(). This may result in fewer sections passed to BP, which could result in faster linktime and better results. In some large binaries, I didn't not see a meaningful difference in compressed size.

[llvmbot]

@llvm/pr-subscribers-lld-macho

@llvm/pr-subscribers-lld

Author: Ellis Hoag (ellishg)

Changes

cstring sections are laid out by {Deduplicated}CStringSection::finalizeContents() and ignores the order determined by runBalancedPartitioning().

llvm-project/lld/MachO/SyntheticSections.cpp

Lines 1729 to 1746 in e63f0f5

	void CStringSection::finalizeContents() {
	size = 0;
	// TODO: Call buildCStringPriorities() to support cstring ordering when
	// deduplication is off, although this may negatively impact build
	// performance.
	for (CStringInputSection *isec : inputs) {
	for (const auto &[i, piece] : llvm::enumerate(isec->pieces)) {
	if (!piece.live)
	continue;
	piece.outSecOff = alignTo(size, getStringPieceAlignment(isec, piece));
	StringRef string = isec->getStringRef(i);
	size = piece.outSecOff + string.size() + 1; // account for null terminator
	}
	isec->isFinal = true;
	}
	}
	void DeduplicatedCStringSection::finalizeContents() {

Do not consider cstring sections or any section already finalized in runBalancedPartitioning(). This may result in fewer sections passed to BP, which could result in faster linktime and better results. In some large binaries, I didn't not see a meaningful difference in compressed size.

---

Full diff: https://github.com/llvm/llvm-project/pull/165757.diff

2 Files Affected:

• (modified) lld/MachO/BPSectionOrderer.cpp (+4)
• (modified) lld/test/MachO/bp-section-orderer.s (+5)

diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index d50abc22fc6c1..328c33e6cfb65 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -118,6 +118,10 @@ DenseMap<const InputSection *, int> lld::macho::runBalancedPartitioning(
         auto *isec = subsec.isec;
         if (!isec || isec->data.empty() || !isec->data.data())
           continue;
+        // CString section order is handled by
+        // {Deduplicated}CStringSection::finalizeContents()
+        if (isa<CStringInputSection>(isec) || isec->isFinal)
+          continue;
         // ConcatInputSections are entirely live or dead, so the offset is
         // irrelevant.
         if (isa<ConcatInputSection>(isec) && !isec->isLive(0))
diff --git a/lld/test/MachO/bp-section-orderer.s b/lld/test/MachO/bp-section-orderer.s
index 90924e5797b64..d7de90d6cd7b3 100644
--- a/lld/test/MachO/bp-section-orderer.s
+++ b/lld/test/MachO/bp-section-orderer.s
@@ -106,6 +106,11 @@ r3:
 r4:
   .quad s2
 
+# cstrings are ignored by runBalancedPartitioning()
+.cstring
+cstr:
+  .asciz "this is cstr"
+
 .bss
 bss0:
   .zero 10

[ellishg]

CC @Colibrow

[Colibrow]

LGTM.