Skip to content

Commit 742ecfc

Browse files
MaskRayMaskRay
committed
[MC] Relax MCFillFragment and compute fragment offsets eagerly
This builds on top of commit 9d0754a ("[MC] Relax fragments eagerly") and relaxes fragments eagerly to eliminate MCSection::HasLayout and `getFragmentOffset` overhead. Relands 1a47f3f 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 38b12d4. 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.
1 parent af2f8a8 commit 742ecfc

File tree

10 files changed

+90
-69
lines changed

10 files changed

+90
-69
lines changed

‎llvm/include/llvm/MC/MCAsmBackend.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,9 @@ class MCAsmBackend {
198198
virtual bool writeNopData(raw_ostream &OS, uint64_t Count,
199199
const MCSubtargetInfo *STI) const = 0;
200200

201-
/// Give backend an opportunity to finish layout after relaxation
202-
virtual void finishLayout(MCAssembler const &Asm) const {}
201+
// Return true if fragment offsets have been adjusted and an extra layout
202+
// iteration is needed.
203+
virtual bool finishLayout(const MCAssembler &Asm) const { return false; }
203204

204205
/// Generate the compact unwind encoding for the CFI instructions.
205206
virtual uint64_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,

‎llvm/include/llvm/MC/MCAssembler.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,12 @@ class MCAssembler {
111111
/// Check whether the given fragment needs relaxation.
112112
bool fragmentNeedsRelaxation(const MCRelaxableFragment *IF) const;
113113

114+
void layoutSection(MCSection &Sec);
114115
/// Perform one layout iteration and return true if any offsets
115116
/// were adjusted.
116-
bool layoutOnce();
117+
bool relaxOnce();
117118

118-
/// Perform relaxation on a single fragment - returns true if the fragment
119-
/// changes as a result of relaxation.
119+
/// Perform relaxation on a single fragment.
120120
bool relaxFragment(MCFragment &F);
121121
bool relaxInstruction(MCRelaxableFragment &IF);
122122
bool relaxLEB(MCLEBFragment &IF);
@@ -125,6 +125,7 @@ class MCAssembler {
125125
bool relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF);
126126
bool relaxCVInlineLineTable(MCCVInlineLineTableFragment &DF);
127127
bool relaxCVDefRange(MCCVDefRangeFragment &DF);
128+
bool relaxFill(MCFillFragment &F);
128129
bool relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &DF);
129130

130131
public:
@@ -144,10 +145,9 @@ class MCAssembler {
144145
uint64_t computeFragmentSize(const MCFragment &F) const;
145146

146147
void layoutBundle(MCFragment *Prev, MCFragment *F) const;
147-
void ensureValid(MCSection &Sec) const;
148148

149149
// Get the offset of the given fragment inside its containing section.
150-
uint64_t getFragmentOffset(const MCFragment &F) const;
150+
uint64_t getFragmentOffset(const MCFragment &F) const { return F.Offset; }
151151

152152
uint64_t getSectionAddressSize(const MCSection &Sec) const;
153153
uint64_t getSectionFileSize(const MCSection &Sec) const;

‎llvm/include/llvm/MC/MCFragment.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ class MCFillFragment : public MCFragment {
293293
uint64_t Value;
294294
/// The number of bytes to insert.
295295
const MCExpr &NumValues;
296+
uint64_t Size = 0;
296297

297298
/// Source location of the directive that this fragment was created for.
298299
SMLoc Loc;
@@ -306,6 +307,8 @@ class MCFillFragment : public MCFragment {
306307
uint64_t getValue() const { return Value; }
307308
uint8_t getValueSize() const { return ValueSize; }
308309
const MCExpr &getNumValues() const { return NumValues; }
310+
uint64_t getSize() const { return Size; }
311+
void setSize(uint64_t Value) { Size = Value; }
309312

310313
SMLoc getLoc() const { return Loc; }
311314

‎llvm/include/llvm/MC/MCSection.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ class MCSection {
9999
/// Whether this section has had instructions emitted into it.
100100
bool HasInstructions : 1;
101101

102-
bool HasLayout : 1;
103-
104102
bool IsRegistered : 1;
105103

106104
bool IsText : 1;
@@ -171,9 +169,6 @@ class MCSection {
171169
bool hasInstructions() const { return HasInstructions; }
172170
void setHasInstructions(bool Value) { HasInstructions = Value; }
173171

174-
bool hasLayout() const { return HasLayout; }
175-
void setHasLayout(bool Value) { HasLayout = Value; }
176-
177172
bool isRegistered() const { return IsRegistered; }
178173
void setIsRegistered(bool Value) { IsRegistered = Value; }
179174

‎llvm/lib/MC/MCAssembler.cpp

Lines changed: 56 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -388,28 +388,6 @@ void MCAssembler::layoutBundle(MCFragment *Prev, MCFragment *F) const {
388388
DF->Offset = EF->Offset;
389389
}
390390

391-
void MCAssembler::ensureValid(MCSection &Sec) const {
392-
if (Sec.hasLayout())
393-
return;
394-
Sec.setHasLayout(true);
395-
MCFragment *Prev = nullptr;
396-
uint64_t Offset = 0;
397-
for (MCFragment &F : Sec) {
398-
F.Offset = Offset;
399-
if (isBundlingEnabled() && F.hasInstructions()) {
400-
layoutBundle(Prev, &F);
401-
Offset = F.Offset;
402-
}
403-
Offset += computeFragmentSize(F);
404-
Prev = &F;
405-
}
406-
}
407-
408-
uint64_t MCAssembler::getFragmentOffset(const MCFragment &F) const {
409-
ensureValid(*F.getParent());
410-
return F.Offset;
411-
}
412-
413391
// Simple getSymbolOffset helper for the non-variable case.
414392
static bool getLabelOffset(const MCAssembler &Asm, const MCSymbol &S,
415393
bool ReportError, uint64_t &Val) {
@@ -864,22 +842,21 @@ void MCAssembler::layout() {
864842

865843
// Layout until everything fits.
866844
this->HasLayout = true;
867-
while (layoutOnce()) {
845+
for (MCSection &Sec : *this)
846+
layoutSection(Sec);
847+
while (relaxOnce())
868848
if (getContext().hadError())
869849
return;
870-
// Size of fragments in one section can depend on the size of fragments in
871-
// another. If any fragment has changed size, we have to re-layout (and
872-
// as a result possibly further relax) all.
873-
for (MCSection &Sec : *this)
874-
Sec.setHasLayout(false);
875-
}
876850

877851
DEBUG_WITH_TYPE("mc-dump", {
878852
errs() << "assembler backend - post-relaxation\n--\n";
879853
dump(); });
880854

881-
// Finalize the layout, including fragment lowering.
882-
getBackend().finishLayout(*this);
855+
// Some targets might want to adjust fragment offsets. If so, perform another
856+
// layout iteration.
857+
if (getBackend().finishLayout(*this))
858+
for (MCSection &Sec : *this)
859+
layoutSection(Sec);
883860

884861
flushPendingErrors();
885862

@@ -1185,6 +1162,14 @@ bool MCAssembler::relaxCVDefRange(MCCVDefRangeFragment &F) {
11851162
return OldSize != F.getContents().size();
11861163
}
11871164

1165+
bool MCAssembler::relaxFill(MCFillFragment &F) {
1166+
uint64_t Size = computeFragmentSize(F);
1167+
if (F.getSize() == Size)
1168+
return false;
1169+
F.setSize(Size);
1170+
return true;
1171+
}
1172+
11881173
bool MCAssembler::relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &PF) {
11891174
uint64_t OldSize = PF.getContents().size();
11901175
int64_t AddrDelta;
@@ -1221,21 +1206,54 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
12211206
return relaxCVInlineLineTable(cast<MCCVInlineLineTableFragment>(F));
12221207
case MCFragment::FT_CVDefRange:
12231208
return relaxCVDefRange(cast<MCCVDefRangeFragment>(F));
1209+
case MCFragment::FT_Fill:
1210+
return relaxFill(cast<MCFillFragment>(F));
12241211
case MCFragment::FT_PseudoProbe:
12251212
return relaxPseudoProbeAddr(cast<MCPseudoProbeAddrFragment>(F));
12261213
}
12271214
}
12281215

1229-
bool MCAssembler::layoutOnce() {
1216+
void MCAssembler::layoutSection(MCSection &Sec) {
1217+
MCFragment *Prev = nullptr;
1218+
uint64_t Offset = 0;
1219+
for (MCFragment &F : Sec) {
1220+
F.Offset = Offset;
1221+
if (LLVM_UNLIKELY(isBundlingEnabled())) {
1222+
if (F.hasInstructions()) {
1223+
layoutBundle(Prev, &F);
1224+
Offset = F.Offset;
1225+
}
1226+
Prev = &F;
1227+
}
1228+
Offset += computeFragmentSize(F);
1229+
}
1230+
}
1231+
1232+
bool MCAssembler::relaxOnce() {
12301233
++stats::RelaxationSteps;
12311234
PendingErrors.clear();
12321235

1233-
bool Changed = false;
1234-
for (MCSection &Sec : *this)
1235-
for (MCFragment &Frag : Sec)
1236-
if (relaxFragment(Frag))
1237-
Changed = true;
1238-
return Changed;
1236+
// Size of fragments in one section can depend on the size of fragments in
1237+
// another. If any fragment has changed size, we have to re-layout (and
1238+
// as a result possibly further relax) all sections.
1239+
bool ChangedAny = false;
1240+
for (MCSection &Sec : *this) {
1241+
// Assume each iteration finalizes at least one extra fragment. If the
1242+
// layout does not converge after N+1 iterations, bail out.
1243+
auto MaxIter = Sec.curFragList()->Tail->getLayoutOrder() + 1;
1244+
for (;;) {
1245+
bool Changed = false;
1246+
for (MCFragment &F : Sec)
1247+
if (relaxFragment(F))
1248+
Changed = true;
1249+
1250+
ChangedAny |= Changed;
1251+
if (!Changed || --MaxIter == 0)
1252+
break;
1253+
layoutSection(Sec);
1254+
}
1255+
}
1256+
return ChangedAny;
12391257
}
12401258

12411259
void MCAssembler::reportError(SMLoc L, const Twine &Msg) const {

‎llvm/lib/MC/MCSection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ using namespace llvm;
2222
MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText,
2323
bool IsVirtual, MCSymbol *Begin)
2424
: Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
25-
HasLayout(false), IsRegistered(false), IsText(IsText),
26-
IsVirtual(IsVirtual), LinkerRelaxable(false), Name(Name), Variant(V) {
25+
IsRegistered(false), IsText(IsText), IsVirtual(IsVirtual),
26+
LinkerRelaxable(false), Name(Name), Variant(V) {
2727
// The initial subsection number is 0. Create a fragment list.
2828
CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
2929
}

‎llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ class HexagonAsmBackend : public MCAsmBackend {
697697
return true;
698698
}
699699

700-
void finishLayout(MCAssembler const &Asm) const override {
700+
bool finishLayout(const MCAssembler &Asm) const override {
701701
SmallVector<MCFragment *> Frags;
702702
for (MCSection &Sec : Asm) {
703703
Frags.clear();
@@ -760,7 +760,6 @@ class HexagonAsmBackend : public MCAsmBackend {
760760
//assert(!Error);
761761
(void)Error;
762762
ReplaceInstruction(Asm.getEmitter(), RF, Inst);
763-
Sec.setHasLayout(false);
764763
Size = 0; // Only look back one instruction
765764
break;
766765
}
@@ -770,6 +769,7 @@ class HexagonAsmBackend : public MCAsmBackend {
770769
}
771770
}
772771
}
772+
return true;
773773
}
774774
}; // class HexagonAsmBackend
775775

‎llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ class X86AsmBackend : public MCAsmBackend {
194194
bool padInstructionEncoding(MCRelaxableFragment &RF, MCCodeEmitter &Emitter,
195195
unsigned &RemainingSize) const;
196196

197-
void finishLayout(const MCAssembler &Asm) const override;
197+
bool finishLayout(const MCAssembler &Asm) const override;
198198

199199
unsigned getMaximumNopSize(const MCSubtargetInfo &STI) const override;
200200

@@ -863,15 +863,15 @@ bool X86AsmBackend::padInstructionEncoding(MCRelaxableFragment &RF,
863863
return Changed;
864864
}
865865

866-
void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
866+
bool X86AsmBackend::finishLayout(const MCAssembler &Asm) const {
867867
// See if we can further relax some instructions to cut down on the number of
868868
// nop bytes required for code alignment. The actual win is in reducing
869869
// instruction count, not number of bytes. Modern X86-64 can easily end up
870870
// decode limited. It is often better to reduce the number of instructions
871871
// (i.e. eliminate nops) even at the cost of increasing the size and
872872
// complexity of others.
873873
if (!X86PadForAlign && !X86PadForBranchAlign)
874-
return;
874+
return false;
875875

876876
// The processed regions are delimitered by LabeledFragments. -g may have more
877877
// MCSymbols and therefore different relaxation results. X86PadForAlign is
@@ -880,6 +880,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
880880
for (const MCSymbol &S : Asm.symbols())
881881
LabeledFragments.insert(S.getFragment());
882882

883+
bool Changed = false;
883884
for (MCSection &Sec : Asm) {
884885
if (!Sec.isText())
885886
continue;
@@ -928,8 +929,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
928929
// Give the backend a chance to play any tricks it wishes to increase
929930
// the encoding size of the given instruction. Target independent code
930931
// will try further relaxation, but target's may play further tricks.
931-
if (padInstructionEncoding(RF, Asm.getEmitter(), RemainingSize))
932-
Sec.setHasLayout(false);
932+
Changed |= padInstructionEncoding(RF, Asm.getEmitter(), RemainingSize);
933933

934934
// If we have an instruction which hasn't been fully relaxed, we can't
935935
// skip past it and insert bytes before it. Changing its starting
@@ -942,14 +942,12 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
942942
}
943943
Relaxable.clear();
944944

945-
// BoundaryAlign explicitly tracks it's size (unlike align)
946-
if (F.getKind() == MCFragment::FT_BoundaryAlign)
947-
cast<MCBoundaryAlignFragment>(F).setSize(RemainingSize);
948-
949945
// If we're looking at a boundary align, make sure we don't try to pad
950946
// its target instructions for some following directive. Doing so would
951947
// break the alignment of the current boundary align.
952948
if (auto *BF = dyn_cast<MCBoundaryAlignFragment>(&F)) {
949+
cast<MCBoundaryAlignFragment>(F).setSize(RemainingSize);
950+
Changed = true;
953951
const MCFragment *LastFragment = BF->getLastFragment();
954952
if (!LastFragment)
955953
continue;
@@ -959,9 +957,7 @@ void X86AsmBackend::finishLayout(MCAssembler const &Asm) const {
959957
}
960958
}
961959

962-
// The layout is done. Mark every fragment as valid.
963-
for (MCSection &Section : Asm)
964-
Asm.getFragmentOffset(*Section.curFragList()->Tail);
960+
return Changed;
965961
}
966962

967963
unsigned X86AsmBackend::getMaximumNopSize(const MCSubtargetInfo &STI) const {

‎llvm/test/MC/AArch64/label-arithmetic-diags-elf.s

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
// RUN: not llvm-mc -triple aarch64-elf -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
1+
// RUN: rm -rf %t && split-file %s %t && cd %t
2+
// RUN: not llvm-mc -triple aarch64-elf -filetype=obj a.s -o /dev/null 2>&1 | FileCheck a.s
3+
// RUN: not llvm-mc -triple aarch64-elf -filetype=obj b.s -o /dev/null 2>&1 | FileCheck b.s
24

5+
//--- a.s
36
.data
47
b:
58
.fill 300
@@ -9,6 +12,7 @@ e:
912
// CHECK-NEXT: .byte e - b
1013
// CHECK-NEXT: ^
1114

15+
//--- b.s
1216
.section sec_x
1317
start:
1418
.space 5000

‎llvm/test/MC/ELF/layout-interdependency3.s

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
## This contrived .space example previously triggered "invalid number of bytes" error.
22
## https://github.com/llvm/llvm-project/issues/123402
3-
# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s
3+
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
4+
# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s
45

5-
# CHECK: error: invalid number of bytes
6+
# CHECK-LABEL: <p_1st>:
7+
# CHECK: e: cli
8+
# CHECK-LABEL: <q_1st>:
9+
# CHECK: 25: nop
610

711
.section .p,"ax"
812
p_1st:

0 commit comments

Comments
 (0)