Skip to content

Commit 39c8cfb

Browse files
committed
MC: Optimize getOrCreateDataFragment
... by eagerly allocating an empty fragment when adding a fragment with a variable-size tail. X86AsmBackend, The JCC erratum mitigation and x86-pad-for-align set a flag for FT_Relaxable, which needs to be moved to emitInstructionBegin. ``` if (CF->getKind() == MCFragment::FT_Relaxable) CF->setAllowAutoPadding(canPadInst(Inst, OS)); ``` Follow-up to #148544
1 parent 54492c2 commit 39c8cfb

File tree

6 files changed

+52
-40
lines changed

6 files changed

+52
-40
lines changed

llvm/include/llvm/MC/MCObjectStreamer.h

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

76-
/// Get a data fragment to write into, creating a new one if the current
77-
/// fragment is not FT_Data.
78-
MCFragment *getOrCreateDataFragment();
76+
// TODO: Change callers to use getCurrentFragment instead.
77+
MCFragment *getOrCreateDataFragment() { return getCurrentFragment(); }
7978

8079
protected:
8180
bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
81+
MCAlignFragment *createAlignFragment(Align Alignment, int64_t Fill,
82+
uint8_t FillLen,
83+
unsigned MaxBytesToEmit);
8284

8385
public:
8486
void visitUsedSymbol(const MCSymbol &Sym) override;

llvm/include/llvm/MC/MCStreamer.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,8 @@ class LLVM_ABI MCStreamer {
270270
/// section changes.
271271
virtual void changeSection(MCSection *, uint32_t);
272272

273+
void addFragment(MCFragment *F);
274+
273275
virtual void emitCFIStartProcImpl(MCDwarfFrameInfo &Frame);
274276
virtual void emitCFIEndProcImpl(MCDwarfFrameInfo &CurFrame);
275277

@@ -456,8 +458,10 @@ class LLVM_ABI MCStreamer {
456458

457459
MCSymbol *endSection(MCSection *Section);
458460

459-
void insert(MCFragment *F);
461+
/// Add a new fragment to the current section without a variable-size tail.
460462
void newFragment();
463+
/// Add a fragment with a variable-size tail and start a new empty fragment.
464+
void insert(MCFragment *F);
461465

462466
/// Returns the mnemonic for \p MI, if the streamer has access to a
463467
/// instruction printer and returns an empty string otherwise.

llvm/lib/MC/MCCodeView.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ void CodeViewContext::emitStringTable(MCObjectStreamer &OS) {
166166
// somewhere else. If somebody wants two string tables in their .s file, one
167167
// will just be empty.
168168
if (!StrTabFragment) {
169-
StrTabFragment = Ctx.allocFragment<MCFragment>();
170-
OS.insert(StrTabFragment);
169+
OS.newFragment();
170+
StrTabFragment = OS.getCurrentFragment();
171171
}
172172

173173
OS.emitValueToAlignment(Align(4), 0);

llvm/lib/MC/MCObjectStreamer.cpp

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

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
113-
auto *F = getCurrentFragment();
114-
if (F->getKind() != MCFragment::FT_Data) {
115-
F = getContext().allocFragment<MCFragment>();
116-
insert(F);
117-
}
118-
return F;
119-
}
120-
121109
void MCObjectStreamer::visitUsedSymbol(const MCSymbol &Sym) {
122110
Assembler->registerSymbol(Sym);
123111
}
@@ -379,6 +367,7 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
379367
F->setVarContents(Data);
380368
F->setVarFixups(Fixups);
381369
F->setInst(Inst);
370+
newFragment();
382371
}
383372

384373
void MCObjectStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
@@ -444,6 +433,7 @@ void MCObjectStreamer::emitDwarfAdvanceLineAddr(int64_t LineDelta,
444433
F->Kind = MCFragment::FT_Dwarf;
445434
F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, SMLoc()));
446435
F->setDwarfLineDelta(LineDelta);
436+
newFragment();
447437
}
448438

449439
void MCObjectStreamer::emitDwarfLineEndEntry(MCSection *Section,
@@ -474,6 +464,7 @@ void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
474464
auto *F = getOrCreateDataFragment();
475465
F->Kind = MCFragment::FT_DwarfFrame;
476466
F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, Loc));
467+
newFragment();
477468
}
478469

479470
void MCObjectStreamer::emitCVLocDirective(unsigned FunctionId, unsigned FileNo,
@@ -536,32 +527,38 @@ void MCObjectStreamer::emitBytes(StringRef Data) {
536527
DF->appendContents(ArrayRef(Data.data(), Data.size()));
537528
}
538529

539-
void MCObjectStreamer::emitValueToAlignment(Align Alignment, int64_t Fill,
540-
uint8_t FillLen,
541-
unsigned MaxBytesToEmit) {
530+
MCAlignFragment *MCObjectStreamer::createAlignFragment(
531+
Align Alignment, int64_t Fill, uint8_t FillLen, unsigned MaxBytesToEmit) {
542532
if (MaxBytesToEmit == 0)
543533
MaxBytesToEmit = Alignment.value();
544-
insert(getContext().allocFragment<MCAlignFragment>(Alignment, Fill, FillLen,
545-
MaxBytesToEmit));
534+
return getContext().allocFragment<MCAlignFragment>(Alignment, Fill, FillLen,
535+
MaxBytesToEmit);
536+
}
546537

538+
void MCObjectStreamer::emitValueToAlignment(Align Alignment, int64_t Fill,
539+
uint8_t FillLen,
540+
unsigned MaxBytesToEmit) {
541+
auto *F = createAlignFragment(Alignment, Fill, FillLen, MaxBytesToEmit);
542+
insert(F);
547543
// Update the maximum alignment on the current section if necessary.
548-
MCSection *CurSec = getCurrentSectionOnly();
549-
CurSec->ensureMinAlignment(Alignment);
544+
F->getParent()->ensureMinAlignment(Alignment);
550545
}
551546

552547
void MCObjectStreamer::emitCodeAlignment(Align Alignment,
553548
const MCSubtargetInfo *STI,
554549
unsigned MaxBytesToEmit) {
555-
emitValueToAlignment(Alignment, 0, 1, MaxBytesToEmit);
556-
auto *F = cast<MCAlignFragment>(getCurrentFragment());
550+
auto *F = createAlignFragment(Alignment, 0, 1, MaxBytesToEmit);
557551
F->setEmitNops(true, STI);
552+
insert(F);
553+
// Update the maximum alignment on the current section if necessary.
554+
F->getParent()->ensureMinAlignment(Alignment);
555+
558556
// With RISC-V style linker relaxation, mark the section as linker-relaxable
559557
// if the alignment is larger than the minimum NOP size.
560558
unsigned Size;
561559
if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
562560
Size)) {
563-
getCurrentSectionOnly()->setLinkerRelaxable();
564-
newFragment();
561+
F->getParent()->setLinkerRelaxable();
565562
}
566563
}
567564

llvm/lib/MC/MCStreamer.cpp

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

1407-
void MCStreamer::insert(MCFragment *F) {
1407+
void MCStreamer::addFragment(MCFragment *F) {
14081408
auto *Sec = CurFrag->getParent();
14091409
F->setParent(Sec);
14101410
F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
@@ -1414,7 +1414,14 @@ void MCStreamer::insert(MCFragment *F) {
14141414
}
14151415

14161416
void MCStreamer::newFragment() {
1417-
insert(getContext().allocFragment<MCFragment>());
1417+
addFragment(getContext().allocFragment<MCFragment>());
1418+
}
1419+
1420+
void MCStreamer::insert(MCFragment *F) {
1421+
assert(F->getKind() != MCFragment::FT_Data &&
1422+
"F should have a variable-size tail");
1423+
addFragment(F);
1424+
newFragment();
14181425
}
14191426

14201427
static VersionTuple

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,6 @@ bool X86AsmBackend::canPadInst(const MCInst &Inst, MCObjectStreamer &OS) const {
456456
}
457457

458458
bool X86AsmBackend::canPadBranches(MCObjectStreamer &OS) const {
459-
if (!OS.getAllowAutoPadding())
460-
return false;
461459
assert(allowAutoPadding() && "incorrect initialization!");
462460

463461
// We only pad in text section.
@@ -491,8 +489,15 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
491489
// fragment will have changed.
492490
IsRightAfterData =
493491
isRightAfterData(OS.getCurrentFragment(), PrevInstPosition);
492+
bool CanPadInst = false;
493+
bool AutoPadding = OS.getAllowAutoPadding();
494+
if (LLVM_UNLIKELY(AutoPadding || X86PadForAlign)) {
495+
CanPadInst = canPadInst(Inst, OS);
496+
if (CanPadInst)
497+
OS.getCurrentFragment()->setAllowAutoPadding(true);
498+
}
494499

495-
if (!canPadBranches(OS))
500+
if (!AutoPadding || !canPadBranches(OS))
496501
return;
497502

498503
// NB: PrevInst only valid if canPadBranches is true.
@@ -504,7 +509,7 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
504509
// we call canPadInst (not cheap) twice. However, in the common case, we can
505510
// avoid unnecessary calls to that, as this is otherwise only used for
506511
// relaxable fragments.
507-
if (!canPadInst(Inst, OS))
512+
if (!CanPadInst)
508513
return;
509514

510515
if (PendingBA && PendingBA->getNext() == OS.getCurrentFragment()) {
@@ -542,15 +547,12 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
542547
/// Set the last fragment to be aligned for the BoundaryAlignFragment.
543548
void X86AsmBackend::emitInstructionEnd(MCObjectStreamer &OS,
544549
const MCInst &Inst) {
545-
MCFragment *CF = OS.getCurrentFragment();
546-
if (CF->getKind() == MCFragment::FT_Relaxable)
547-
CF->setAllowAutoPadding(canPadInst(Inst, OS));
548-
549550
// Update PrevInstOpcode here, canPadInst() reads that.
551+
MCFragment *CF = OS.getCurrentFragment();
550552
PrevInstOpcode = Inst.getOpcode();
551553
PrevInstPosition = std::make_pair(CF, getSizeForInstFragment(CF));
552554

553-
if (!canPadBranches(OS))
555+
if (!OS.getAllowAutoPadding() || !canPadBranches(OS))
554556
return;
555557

556558
// PrevInst is only needed if canPadBranches. Copying an MCInst isn't cheap.

0 commit comments

Comments
 (0)