Skip to content

Commit 73e4b58

Browse files
authored
MC: Simplify fragment reuse determination
First, avoid checking MCSubtargetInfo by reducing unnecessary overhead introduced in https://reviews.llvm.org/D44928 . That change passed STI to both FT_Data and FT_Relaxable fragments, but STI is only necessary for FT_Relaxable. The use of STI in FT_Data was added for: * Bundle alignment mode, which has been removed (#148781). * ARM, which inappropriately uses STI in `ARMAsmBackend::applyFixup` due to tech debt, unlike other targets. All tests passed even without the `copySTI` change. To ensure safety, `copySTI` now starts a new fragment to prevent mixed STI values. Second, avoid checking LinkerRelaxable by eagerly starting a new fragment when a FT_Data/FT_Align fragment is marked linker-relaxable. There is currently an extra empty FT_Data if an alignment immediately follows a linker-relaxable fragment, which will be improved in the future when FT_Align information is moved to the variable-tail. Pull Request: #149471
1 parent 0121314 commit 73e4b58

File tree

7 files changed

+43
-37
lines changed

7 files changed

+43
-37
lines changed

llvm/include/llvm/MC/MCObjectStreamer.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,9 @@ class MCObjectStreamer : public MCStreamer {
7373
MCSymbol *emitCFILabel() override;
7474
void emitCFISections(bool EH, bool Debug) override;
7575

76-
void insert(MCFragment *F) {
77-
auto *Sec = CurFrag->getParent();
78-
F->setParent(Sec);
79-
F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
80-
CurFrag->Next = F;
81-
CurFrag = F;
82-
Sec->curFragList()->Tail = F;
83-
}
84-
8576
/// Get a data fragment to write into, creating a new one if the current
8677
/// fragment is not FT_Data.
87-
/// Optionally a \p STI can be passed in so that a new fragment is created
88-
/// if the Subtarget differs from the current fragment.
89-
MCFragment *getOrCreateDataFragment(const MCSubtargetInfo *STI = nullptr);
78+
MCFragment *getOrCreateDataFragment();
9079

9180
protected:
9281
bool changeSectionImpl(MCSection *Section, uint32_t Subsection);

llvm/include/llvm/MC/MCSection.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ class LLVM_ABI MCSection {
188188
// destructors.
189189
class MCFragment {
190190
friend class MCAssembler;
191+
friend class MCStreamer;
191192
friend class MCObjectStreamer;
192193
friend class MCSection;
193194

llvm/include/llvm/MC/MCStreamer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,6 @@ class LLVM_ABI MCStreamer {
429429
CurFrag->getParent() == getCurrentSection().first);
430430
return CurFrag;
431431
}
432-
433432
/// Save the current and previous section on the section stack.
434433
void pushSection() {
435434
SectionStack.push_back(
@@ -457,6 +456,9 @@ class LLVM_ABI MCStreamer {
457456

458457
MCSymbol *endSection(MCSection *Section);
459458

459+
void insert(MCFragment *F);
460+
void newFragment();
461+
460462
/// Returns the mnemonic for \p MI, if the streamer has access to a
461463
/// instruction printer and returns an empty string otherwise.
462464
virtual StringRef getMnemonic(const MCInst &MI) const { return ""; }

llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -106,26 +106,12 @@ void MCObjectStreamer::emitFrames(MCAsmBackend *MAB) {
106106
MCDwarfFrameEmitter::Emit(*this, MAB, false);
107107
}
108108

109-
static bool canReuseDataFragment(const MCFragment &F,
110-
const MCAssembler &Assembler,
111-
const MCSubtargetInfo *STI) {
112-
if (!F.hasInstructions())
113-
return true;
114-
// Do not add data after a linker-relaxable instruction. The difference
115-
// between a new label and a label at or before the linker-relaxable
116-
// instruction cannot be resolved at assemble-time.
117-
if (F.isLinkerRelaxable())
118-
return false;
119-
// If the subtarget is changed mid fragment we start a new fragment to record
120-
// the new STI.
121-
return !STI || F.getSubtargetInfo() == STI;
122-
}
123-
124-
MCFragment *
125-
MCObjectStreamer::getOrCreateDataFragment(const MCSubtargetInfo *STI) {
109+
MCFragment *MCObjectStreamer::getOrCreateDataFragment() {
110+
// TODO: Start a new fragment whenever finalizing the variable-size tail of a
111+
// previous one, so that all getOrCreateDataFragment calls can be replaced
112+
// with getCurrentFragment
126113
auto *F = getCurrentFragment();
127-
if (F->getKind() != MCFragment::FT_Data ||
128-
!canReuseDataFragment(*F, *Assembler, STI)) {
114+
if (F->getKind() != MCFragment::FT_Data) {
129115
F = getContext().allocFragment<MCFragment>();
130116
insert(F);
131117
}
@@ -363,16 +349,23 @@ void MCObjectStreamer::emitInstToData(const MCInst &Inst,
363349
F->doneAppending();
364350
if (!Fixups.empty())
365351
F->appendFixups(Fixups);
352+
F->setHasInstructions(STI);
366353

354+
bool MarkedLinkerRelaxable = false;
367355
for (auto &Fixup : MutableArrayRef(F->getFixups()).slice(FixupStartIndex)) {
368356
Fixup.setOffset(Fixup.getOffset() + CodeOffset);
369-
if (Fixup.isLinkerRelaxable()) {
370-
F->setLinkerRelaxable();
357+
if (!Fixup.isLinkerRelaxable())
358+
continue;
359+
F->setLinkerRelaxable();
360+
// Do not add data after a linker-relaxable instruction. The difference
361+
// between a new label and a label at or before the linker-relaxable
362+
// instruction cannot be resolved at assemble-time.
363+
if (!MarkedLinkerRelaxable) {
364+
MarkedLinkerRelaxable = true;
371365
getCurrentSectionOnly()->setLinkerRelaxable();
366+
newFragment();
372367
}
373368
}
374-
375-
F->setHasInstructions(STI);
376369
}
377370

378371
void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
@@ -568,8 +561,10 @@ void MCObjectStreamer::emitCodeAlignment(Align Alignment,
568561
// if the alignment is larger than the minimum NOP size.
569562
unsigned Size;
570563
if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
571-
Size))
564+
Size)) {
572565
getCurrentSectionOnly()->setLinkerRelaxable();
566+
newFragment();
567+
}
573568
}
574569

575570
void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,

llvm/lib/MC/MCParser/MCTargetAsmParser.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
1010
#include "llvm/MC/MCContext.h"
1111
#include "llvm/MC/MCRegister.h"
12+
#include "llvm/MC/MCStreamer.h"
1213

1314
using namespace llvm;
1415

@@ -22,6 +23,10 @@ MCTargetAsmParser::~MCTargetAsmParser() = default;
2223
MCSubtargetInfo &MCTargetAsmParser::copySTI() {
2324
MCSubtargetInfo &STICopy = getContext().getSubtargetCopy(getSTI());
2425
STI = &STICopy;
26+
// The returned STI will likely be modified. Create a new fragment to prevent
27+
// mixing STI values within a fragment.
28+
if (getStreamer().getCurrentFragment())
29+
getStreamer().newFragment();
2530
return STICopy;
2631
}
2732

llvm/lib/MC/MCStreamer.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,6 +1404,19 @@ MCSymbol *MCStreamer::endSection(MCSection *Section) {
14041404
return Sym;
14051405
}
14061406

1407+
void MCStreamer::insert(MCFragment *F) {
1408+
auto *Sec = CurFrag->getParent();
1409+
F->setParent(Sec);
1410+
F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
1411+
CurFrag->Next = F;
1412+
CurFrag = F;
1413+
Sec->curFragList()->Tail = F;
1414+
}
1415+
1416+
void MCStreamer::newFragment() {
1417+
insert(getContext().allocFragment<MCFragment>());
1418+
}
1419+
14071420
static VersionTuple
14081421
targetVersionOrMinimumSupportedOSVersion(const Triple &Target,
14091422
VersionTuple TargetVersion) {

llvm/test/MC/RISCV/Relocations/mc-dump.s

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
# CHECK-NEXT:0 Data LinkerRelaxable Size:8 [97,00,00,00,e7,80,00,00]
1010
# CHECK-NEXT: Fixup @0 Value:specifier(19,ext) Kind:4023
1111
# CHECK-NEXT: Symbol @0 $x
12+
# CHECK-NEXT:8 Data Size:0 []
1213
# CHECK-NEXT:8 Align Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
1314
# CHECK-NEXT:12 Data Size:4 [13,05,30,00]
1415
# CHECK-NEXT:16 Align Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops

0 commit comments

Comments
 (0)