[llvm] 742ecfc - [MC] Relax MCFillFragment and compute fragment offsets eagerly
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 2 00:29:32 PDT 2025
Author: Fangrui Song
Date: 2025-06-02T00:29:27-07:00
New Revision: 742ecfc13e8aa34cfff2900e31838f657fcafe30
URL: https://github.com/llvm/llvm-project/commit/742ecfc13e8aa34cfff2900e31838f657fcafe30
DIFF: https://github.com/llvm/llvm-project/commit/742ecfc13e8aa34cfff2900e31838f657fcafe30.diff
LOG: [MC] Relax MCFillFragment and compute fragment offsets eagerly
This builds on top of commit 9d0754ada5dbbc0c009bcc2f7824488419cc5530
("[MC] Relax fragments eagerly") and relaxes fragments eagerly to
eliminate MCSection::HasLayout and `getFragmentOffset` overhead.
Relands 1a47f3f3db66589c11f8ddacfeaecc03fb80c510
Builds with many text sections (e.g. full LTO) shall observe a decrease
in compile time.
---
In addition, ensure `.fill` and `.space` directives with expressions are
re-evaluated during fragment relaxation, as their sizes may change.
Continue iteration to prevent stale, incorrect sizes.
This change has to be coupled with the fragment algorithm change
as otherwise the test test/MC/ELF/layout-interdependency.s would not
converge.
Fixes #123402 and resolves the root cause of #100283, building on error
postponing from commit 38b12d4a7c219b46d1cb52580cbacbdb931262f2.
For AArch64/label-arithmetic-diags-elf.s, the extra iteration
reports a .fill error early and suppresses the fixup/relocation errors.
Just split the tests.
Added:
Modified:
llvm/include/llvm/MC/MCAsmBackend.h
llvm/include/llvm/MC/MCAssembler.h
llvm/include/llvm/MC/MCFragment.h
llvm/include/llvm/MC/MCSection.h
llvm/lib/MC/MCAssembler.cpp
llvm/lib/MC/MCSection.cpp
llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
llvm/test/MC/AArch64/label-arithmetic-diags-elf.s
llvm/test/MC/ELF/layout-interdependency3.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 3b095772bd144..f6d776b839fe5 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -198,8 +198,9 @@ class MCAsmBackend {
virtual bool writeNopData(raw_ostream &OS, uint64_t Count,
const MCSubtargetInfo *STI) const = 0;
- /// Give backend an opportunity to finish layout after relaxation
- virtual void finishLayout(MCAssembler const &Asm) const {}
+ // Return true if fragment offsets have been adjusted and an extra layout
+ // iteration is needed.
+ virtual bool finishLayout(const MCAssembler &Asm) const { return false; }
/// Generate the compact unwind encoding for the CFI instructions.
virtual uint64_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index 3e141734ceac2..988677193d6d9 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -111,12 +111,12 @@ class MCAssembler {
/// Check whether the given fragment needs relaxation.
bool fragmentNeedsRelaxation(const MCRelaxableFragment *IF) const;
+ void layoutSection(MCSection &Sec);
/// Perform one layout iteration and return true if any offsets
/// were adjusted.
- bool layoutOnce();
+ bool relaxOnce();
- /// Perform relaxation on a single fragment - returns true if the fragment
- /// changes as a result of relaxation.
+ /// Perform relaxation on a single fragment.
bool relaxFragment(MCFragment &F);
bool relaxInstruction(MCRelaxableFragment &IF);
bool relaxLEB(MCLEBFragment &IF);
@@ -125,6 +125,7 @@ class MCAssembler {
bool relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF);
bool relaxCVInlineLineTable(MCCVInlineLineTableFragment &DF);
bool relaxCVDefRange(MCCVDefRangeFragment &DF);
+ bool relaxFill(MCFillFragment &F);
bool relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &DF);
public:
@@ -144,10 +145,9 @@ class MCAssembler {
uint64_t computeFragmentSize(const MCFragment &F) const;
void layoutBundle(MCFragment *Prev, MCFragment *F) const;
- void ensureValid(MCSection &Sec) const;
// Get the offset of the given fragment inside its containing section.
- uint64_t getFragmentOffset(const MCFragment &F) const;
+ uint64_t getFragmentOffset(const MCFragment &F) const { return F.Offset; }
uint64_t getSectionAddressSize(const MCSection &Sec) const;
uint64_t getSectionFileSize(const MCSection &Sec) const;
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index fdbfef28088d1..3d40e59c69849 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -293,6 +293,7 @@ class MCFillFragment : public MCFragment {
uint64_t Value;
/// The number of bytes to insert.
const MCExpr &NumValues;
+ uint64_t Size = 0;
/// Source location of the directive that this fragment was created for.
SMLoc Loc;
@@ -306,6 +307,8 @@ class MCFillFragment : public MCFragment {
uint64_t getValue() const { return Value; }
uint8_t getValueSize() const { return ValueSize; }
const MCExpr &getNumValues() const { return NumValues; }
+ uint64_t getSize() const { return Size; }
+ void setSize(uint64_t Value) { Size = Value; }
SMLoc getLoc() const { return Loc; }
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 942d09bad5949..fb23e256d0aab 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -99,8 +99,6 @@ class MCSection {
/// Whether this section has had instructions emitted into it.
bool HasInstructions : 1;
- bool HasLayout : 1;
-
bool IsRegistered : 1;
bool IsText : 1;
@@ -171,9 +169,6 @@ class MCSection {
bool hasInstructions() const { return HasInstructions; }
void setHasInstructions(bool Value) { HasInstructions = Value; }
- bool hasLayout() const { return HasLayout; }
- void setHasLayout(bool Value) { HasLayout = Value; }
-
bool isRegistered() const { return IsRegistered; }
void setIsRegistered(bool Value) { IsRegistered = Value; }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 5898f7fb0812a..1866c5b9e0e81 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -388,28 +388,6 @@ void MCAssembler::layoutBundle(MCFragment *Prev, MCFragment *F) const {
DF->Offset = EF->Offset;
}
-void MCAssembler::ensureValid(MCSection &Sec) const {
- if (Sec.hasLayout())
- return;
- Sec.setHasLayout(true);
- MCFragment *Prev = nullptr;
- uint64_t Offset = 0;
- for (MCFragment &F : Sec) {
- F.Offset = Offset;
- if (isBundlingEnabled() && F.hasInstructions()) {
- layoutBundle(Prev, &F);
- Offset = F.Offset;
- }
- Offset += computeFragmentSize(F);
- Prev = &F;
- }
-}
-
-uint64_t MCAssembler::getFragmentOffset(const MCFragment &F) const {
- ensureValid(*F.getParent());
- return F.Offset;
-}
-
// Simple getSymbolOffset helper for the non-variable case.
static bool getLabelOffset(const MCAssembler &Asm, const MCSymbol &S,
bool ReportError, uint64_t &Val) {
@@ -864,22 +842,21 @@ void MCAssembler::layout() {
// Layout until everything fits.
this->HasLayout = true;
- while (layoutOnce()) {
+ for (MCSection &Sec : *this)
+ layoutSection(Sec);
+ while (relaxOnce())
if (getContext().hadError())
return;
- // Size of fragments in one section can depend on the size of fragments in
- // another. If any fragment has changed size, we have to re-layout (and
- // as a result possibly further relax) all.
- for (MCSection &Sec : *this)
- Sec.setHasLayout(false);
- }
DEBUG_WITH_TYPE("mc-dump", {
errs() << "assembler backend - post-relaxation\n--\n";
dump(); });
- // Finalize the layout, including fragment lowering.
- getBackend().finishLayout(*this);
+ // Some targets might want to adjust fragment offsets. If so, perform another
+ // layout iteration.
+ if (getBackend().finishLayout(*this))
+ for (MCSection &Sec : *this)
+ layoutSection(Sec);
flushPendingErrors();
@@ -1185,6 +1162,14 @@ bool MCAssembler::relaxCVDefRange(MCCVDefRangeFragment &F) {
return OldSize != F.getContents().size();
}
+bool MCAssembler::relaxFill(MCFillFragment &F) {
+ uint64_t Size = computeFragmentSize(F);
+ if (F.getSize() == Size)
+ return false;
+ F.setSize(Size);
+ return true;
+}
+
bool MCAssembler::relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &PF) {
uint64_t OldSize = PF.getContents().size();
int64_t AddrDelta;
@@ -1221,21 +1206,54 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
return relaxCVInlineLineTable(cast<MCCVInlineLineTableFragment>(F));
case MCFragment::FT_CVDefRange:
return relaxCVDefRange(cast<MCCVDefRangeFragment>(F));
+ case MCFragment::FT_Fill:
+ return relaxFill(cast<MCFillFragment>(F));
case MCFragment::FT_PseudoProbe:
return relaxPseudoProbeAddr(cast<MCPseudoProbeAddrFragment>(F));
}
}
-bool MCAssembler::layoutOnce() {
+void MCAssembler::layoutSection(MCSection &Sec) {
+ MCFragment *Prev = nullptr;
+ uint64_t Offset = 0;
+ for (MCFragment &F : Sec) {
+ F.Offset = Offset;
+ if (LLVM_UNLIKELY(isBundlingEnabled())) {
+ if (F.hasInstructions()) {
+ layoutBundle(Prev, &F);
+ Offset = F.Offset;
+ }
+ Prev = &F;
+ }
+ Offset += computeFragmentSize(F);
+ }
+}
+
+bool MCAssembler::relaxOnce() {
++stats::RelaxationSteps;
PendingErrors.clear();
- bool Changed = false;
- for (MCSection &Sec : *this)
- for (MCFragment &Frag : Sec)
- if (relaxFragment(Frag))
- Changed = true;
- return Changed;
+ // Size of fragments in one section can depend on the size of fragments in
+ // another. If any fragment has changed size, we have to re-layout (and
+ // as a result possibly further relax) all sections.
+ bool ChangedAny = false;
+ for (MCSection &Sec : *this) {
+ // Assume each iteration finalizes at least one extra fragment. If the
+ // layout does not converge after N+1 iterations, bail out.
+ auto MaxIter = Sec.curFragList()->Tail->getLayoutOrder() + 1;
+ for (;;) {
+ bool Changed = false;
+ for (MCFragment &F : Sec)
+ if (relaxFragment(F))
+ Changed = true;
+
+ ChangedAny |= Changed;
+ if (!Changed || --MaxIter == 0)
+ break;
+ layoutSection(Sec);
+ }
+ }
+ return ChangedAny;
}
void MCAssembler::reportError(SMLoc L, const Twine &Msg) const {
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 0e64c887e91b9..b61738d8837a2 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -22,8 +22,8 @@ using namespace llvm;
MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText,
bool IsVirtual, MCSymbol *Begin)
: Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
- HasLayout(false), IsRegistered(false), IsText(IsText),
- IsVirtual(IsVirtual), LinkerRelaxable(false), Name(Name), Variant(V) {
+ IsRegistered(false), IsText(IsText), IsVirtual(IsVirtual),
+ LinkerRelaxable(false), Name(Name), Variant(V) {
// The initial subsection number is 0. Create a fragment list.
CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
}
diff --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
index 624610a66ee94..45bb82c945a7a 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
@@ -697,7 +697,7 @@ class HexagonAsmBackend : public MCAsmBackend {
return true;
}
- void finishLayout(MCAssembler const &Asm) const override {
+ bool finishLayout(const MCAssembler &Asm) const override {
SmallVector<MCFragment *> Frags;
for (MCSection &Sec : Asm) {
Frags.clear();
@@ -760,7 +760,6 @@ class HexagonAsmBackend : public MCAsmBackend {
//assert(!Error);
(void)Error;
ReplaceInstruction(Asm.getEmitter(), RF, Inst);
- Sec.setHasLayout(false);
Size = 0; // Only look back one instruction
break;
}
@@ -770,6 +769,7 @@ class HexagonAsmBackend : public MCAsmBackend {
}
}
}
+ return true;
}
}; // class HexagonAsmBackend
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index ac60543e50eff..2d0f021eac5a8 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -194,7 +194,7 @@ class X86AsmBackend : public MCAsmBackend {
bool padInstructionEncoding(MCRelaxableFragment &RF, MCCodeEmitter &Emitter,
unsigned &RemainingSize) const;
- void finishLayout(const MCAssembler &Asm) const override;
+ bool finishLayout(const MCAssembler &Asm) const override;
unsigned getMaximumNopSize(const MCSubtargetInfo &STI) const override;
@@ -863,7 +863,7 @@ bool X86AsmBackend::padInstructionEncoding(MCRelaxableFragment &RF,
return Changed;
}
-void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
+bool X86AsmBackend::finishLayout(const MCAssembler &Asm) const {
// See if we can further relax some instructions to cut down on the number of
// nop bytes required for code alignment. The actual win is in reducing
// instruction count, not number of bytes. Modern X86-64 can easily end up
@@ -871,7 +871,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
// (i.e. eliminate nops) even at the cost of increasing the size and
// complexity of others.
if (!X86PadForAlign && !X86PadForBranchAlign)
- return;
+ return false;
// The processed regions are delimitered by LabeledFragments. -g may have more
// MCSymbols and therefore
diff erent relaxation results. X86PadForAlign is
@@ -880,6 +880,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
for (const MCSymbol &S : Asm.symbols())
LabeledFragments.insert(S.getFragment());
+ bool Changed = false;
for (MCSection &Sec : Asm) {
if (!Sec.isText())
continue;
@@ -928,8 +929,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
// Give the backend a chance to play any tricks it wishes to increase
// the encoding size of the given instruction. Target independent code
// will try further relaxation, but target's may play further tricks.
- if (padInstructionEncoding(RF, Asm.getEmitter(), RemainingSize))
- Sec.setHasLayout(false);
+ Changed |= padInstructionEncoding(RF, Asm.getEmitter(), RemainingSize);
// If we have an instruction which hasn't been fully relaxed, we can't
// skip past it and insert bytes before it. Changing its starting
@@ -942,14 +942,12 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
}
Relaxable.clear();
- // BoundaryAlign explicitly tracks it's size (unlike align)
- if (F.getKind() == MCFragment::FT_BoundaryAlign)
- cast<MCBoundaryAlignFragment>(F).setSize(RemainingSize);
-
// If we're looking at a boundary align, make sure we don't try to pad
// its target instructions for some following directive. Doing so would
// break the alignment of the current boundary align.
if (auto *BF = dyn_cast<MCBoundaryAlignFragment>(&F)) {
+ cast<MCBoundaryAlignFragment>(F).setSize(RemainingSize);
+ Changed = true;
const MCFragment *LastFragment = BF->getLastFragment();
if (!LastFragment)
continue;
@@ -959,9 +957,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
}
}
- // The layout is done. Mark every fragment as valid.
- for (MCSection &Section : Asm)
- Asm.getFragmentOffset(*Section.curFragList()->Tail);
+ return Changed;
}
unsigned X86AsmBackend::getMaximumNopSize(const MCSubtargetInfo &STI) const {
diff --git a/llvm/test/MC/AArch64/label-arithmetic-diags-elf.s b/llvm/test/MC/AArch64/label-arithmetic-diags-elf.s
index 2ef67fafb2ea5..ff53d01e5feda 100644
--- a/llvm/test/MC/AArch64/label-arithmetic-diags-elf.s
+++ b/llvm/test/MC/AArch64/label-arithmetic-diags-elf.s
@@ -1,5 +1,8 @@
-// RUN: not llvm-mc -triple aarch64-elf -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: rm -rf %t && split-file %s %t && cd %t
+// RUN: not llvm-mc -triple aarch64-elf -filetype=obj a.s -o /dev/null 2>&1 | FileCheck a.s
+// RUN: not llvm-mc -triple aarch64-elf -filetype=obj b.s -o /dev/null 2>&1 | FileCheck b.s
+//--- a.s
.data
b:
.fill 300
@@ -9,6 +12,7 @@ e:
// CHECK-NEXT: .byte e - b
// CHECK-NEXT: ^
+//--- b.s
.section sec_x
start:
.space 5000
diff --git a/llvm/test/MC/ELF/layout-interdependency3.s b/llvm/test/MC/ELF/layout-interdependency3.s
index 85b90078b1b2a..bd73890192eea 100644
--- a/llvm/test/MC/ELF/layout-interdependency3.s
+++ b/llvm/test/MC/ELF/layout-interdependency3.s
@@ -1,8 +1,12 @@
## This contrived .space example previously triggered "invalid number of bytes" error.
## https://github.com/llvm/llvm-project/issues/123402
-# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
+# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s
-# CHECK: error: invalid number of bytes
+# CHECK-LABEL: <p_1st>:
+# CHECK: e: cli
+# CHECK-LABEL: <q_1st>:
+# CHECK: 25: nop
.section .p,"ax"
p_1st:
More information about the llvm-commits
mailing list