[llvm] 39c8cfb - MC: Optimize getOrCreateDataFragment
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 19 16:55:35 PDT 2025
Author: Fangrui Song
Date: 2025-07-19T16:55:30-07:00
New Revision: 39c8cfb70d203439e3296dfdfe3d41f1cb2ec551
URL: https://github.com/llvm/llvm-project/commit/39c8cfb70d203439e3296dfdfe3d41f1cb2ec551
DIFF: https://github.com/llvm/llvm-project/commit/39c8cfb70d203439e3296dfdfe3d41f1cb2ec551.diff
LOG: 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
Added:
Modified:
llvm/include/llvm/MC/MCObjectStreamer.h
llvm/include/llvm/MC/MCStreamer.h
llvm/lib/MC/MCCodeView.cpp
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MCStreamer.cpp
llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 319e131999d48..3ccc1e30222a9 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -73,12 +73,14 @@ class MCObjectStreamer : public MCStreamer {
MCSymbol *emitCFILabel() override;
void emitCFISections(bool EH, bool Debug) override;
- /// Get a data fragment to write into, creating a new one if the current
- /// fragment is not FT_Data.
- MCFragment *getOrCreateDataFragment();
+ // TODO: Change callers to use getCurrentFragment instead.
+ MCFragment *getOrCreateDataFragment() { return getCurrentFragment(); }
protected:
bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
+ MCAlignFragment *createAlignFragment(Align Alignment, int64_t Fill,
+ uint8_t FillLen,
+ unsigned MaxBytesToEmit);
public:
void visitUsedSymbol(const MCSymbol &Sym) override;
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 4b91dbc794682..0f87cf8f13a59 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -270,6 +270,8 @@ class LLVM_ABI MCStreamer {
/// section changes.
virtual void changeSection(MCSection *, uint32_t);
+ void addFragment(MCFragment *F);
+
virtual void emitCFIStartProcImpl(MCDwarfFrameInfo &Frame);
virtual void emitCFIEndProcImpl(MCDwarfFrameInfo &CurFrame);
@@ -456,8 +458,10 @@ class LLVM_ABI MCStreamer {
MCSymbol *endSection(MCSection *Section);
- void insert(MCFragment *F);
+ /// Add a new fragment to the current section without a variable-size tail.
void newFragment();
+ /// Add a fragment with a variable-size tail and start a new empty fragment.
+ void insert(MCFragment *F);
/// Returns the mnemonic for \p MI, if the streamer has access to a
/// instruction printer and returns an empty string otherwise.
diff --git a/llvm/lib/MC/MCCodeView.cpp b/llvm/lib/MC/MCCodeView.cpp
index 1f9825185175a..b151a2545ddb2 100644
--- a/llvm/lib/MC/MCCodeView.cpp
+++ b/llvm/lib/MC/MCCodeView.cpp
@@ -166,8 +166,8 @@ void CodeViewContext::emitStringTable(MCObjectStreamer &OS) {
// somewhere else. If somebody wants two string tables in their .s file, one
// will just be empty.
if (!StrTabFragment) {
- StrTabFragment = Ctx.allocFragment<MCFragment>();
- OS.insert(StrTabFragment);
+ OS.newFragment();
+ StrTabFragment = OS.getCurrentFragment();
}
OS.emitValueToAlignment(Align(4), 0);
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index f61dda6d248b5..285c0f29ca4cc 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -106,18 +106,6 @@ void MCObjectStreamer::emitFrames(MCAsmBackend *MAB) {
MCDwarfFrameEmitter::Emit(*this, MAB, false);
}
-MCFragment *MCObjectStreamer::getOrCreateDataFragment() {
- // TODO: Start a new fragment whenever finalizing the variable-size tail of a
- // previous one, so that all getOrCreateDataFragment calls can be replaced
- // with getCurrentFragment
- auto *F = getCurrentFragment();
- if (F->getKind() != MCFragment::FT_Data) {
- F = getContext().allocFragment<MCFragment>();
- insert(F);
- }
- return F;
-}
-
void MCObjectStreamer::visitUsedSymbol(const MCSymbol &Sym) {
Assembler->registerSymbol(Sym);
}
@@ -379,6 +367,7 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
F->setVarContents(Data);
F->setVarFixups(Fixups);
F->setInst(Inst);
+ newFragment();
}
void MCObjectStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
@@ -444,6 +433,7 @@ void MCObjectStreamer::emitDwarfAdvanceLineAddr(int64_t LineDelta,
F->Kind = MCFragment::FT_Dwarf;
F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, SMLoc()));
F->setDwarfLineDelta(LineDelta);
+ newFragment();
}
void MCObjectStreamer::emitDwarfLineEndEntry(MCSection *Section,
@@ -474,6 +464,7 @@ void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
auto *F = getOrCreateDataFragment();
F->Kind = MCFragment::FT_DwarfFrame;
F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, Loc));
+ newFragment();
}
void MCObjectStreamer::emitCVLocDirective(unsigned FunctionId, unsigned FileNo,
@@ -536,32 +527,38 @@ void MCObjectStreamer::emitBytes(StringRef Data) {
DF->appendContents(ArrayRef(Data.data(), Data.size()));
}
-void MCObjectStreamer::emitValueToAlignment(Align Alignment, int64_t Fill,
- uint8_t FillLen,
- unsigned MaxBytesToEmit) {
+MCAlignFragment *MCObjectStreamer::createAlignFragment(
+ Align Alignment, int64_t Fill, uint8_t FillLen, unsigned MaxBytesToEmit) {
if (MaxBytesToEmit == 0)
MaxBytesToEmit = Alignment.value();
- insert(getContext().allocFragment<MCAlignFragment>(Alignment, Fill, FillLen,
- MaxBytesToEmit));
+ return getContext().allocFragment<MCAlignFragment>(Alignment, Fill, FillLen,
+ MaxBytesToEmit);
+}
+void MCObjectStreamer::emitValueToAlignment(Align Alignment, int64_t Fill,
+ uint8_t FillLen,
+ unsigned MaxBytesToEmit) {
+ auto *F = createAlignFragment(Alignment, Fill, FillLen, MaxBytesToEmit);
+ insert(F);
// Update the maximum alignment on the current section if necessary.
- MCSection *CurSec = getCurrentSectionOnly();
- CurSec->ensureMinAlignment(Alignment);
+ F->getParent()->ensureMinAlignment(Alignment);
}
void MCObjectStreamer::emitCodeAlignment(Align Alignment,
const MCSubtargetInfo *STI,
unsigned MaxBytesToEmit) {
- emitValueToAlignment(Alignment, 0, 1, MaxBytesToEmit);
- auto *F = cast<MCAlignFragment>(getCurrentFragment());
+ auto *F = createAlignFragment(Alignment, 0, 1, MaxBytesToEmit);
F->setEmitNops(true, STI);
+ insert(F);
+ // Update the maximum alignment on the current section if necessary.
+ F->getParent()->ensureMinAlignment(Alignment);
+
// With RISC-V style linker relaxation, mark the section as linker-relaxable
// if the alignment is larger than the minimum NOP size.
unsigned Size;
if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
Size)) {
- getCurrentSectionOnly()->setLinkerRelaxable();
- newFragment();
+ F->getParent()->setLinkerRelaxable();
}
}
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index c3ecf8fc717f5..b0e52bf511ba4 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1404,7 +1404,7 @@ MCSymbol *MCStreamer::endSection(MCSection *Section) {
return Sym;
}
-void MCStreamer::insert(MCFragment *F) {
+void MCStreamer::addFragment(MCFragment *F) {
auto *Sec = CurFrag->getParent();
F->setParent(Sec);
F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
@@ -1414,7 +1414,14 @@ void MCStreamer::insert(MCFragment *F) {
}
void MCStreamer::newFragment() {
- insert(getContext().allocFragment<MCFragment>());
+ addFragment(getContext().allocFragment<MCFragment>());
+}
+
+void MCStreamer::insert(MCFragment *F) {
+ assert(F->getKind() != MCFragment::FT_Data &&
+ "F should have a variable-size tail");
+ addFragment(F);
+ newFragment();
}
static VersionTuple
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index 387d289e36f8a..16de664d97f75 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -456,8 +456,6 @@ bool X86AsmBackend::canPadInst(const MCInst &Inst, MCObjectStreamer &OS) const {
}
bool X86AsmBackend::canPadBranches(MCObjectStreamer &OS) const {
- if (!OS.getAllowAutoPadding())
- return false;
assert(allowAutoPadding() && "incorrect initialization!");
// We only pad in text section.
@@ -491,8 +489,15 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
// fragment will have changed.
IsRightAfterData =
isRightAfterData(OS.getCurrentFragment(), PrevInstPosition);
+ bool CanPadInst = false;
+ bool AutoPadding = OS.getAllowAutoPadding();
+ if (LLVM_UNLIKELY(AutoPadding || X86PadForAlign)) {
+ CanPadInst = canPadInst(Inst, OS);
+ if (CanPadInst)
+ OS.getCurrentFragment()->setAllowAutoPadding(true);
+ }
- if (!canPadBranches(OS))
+ if (!AutoPadding || !canPadBranches(OS))
return;
// NB: PrevInst only valid if canPadBranches is true.
@@ -504,7 +509,7 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
// we call canPadInst (not cheap) twice. However, in the common case, we can
// avoid unnecessary calls to that, as this is otherwise only used for
// relaxable fragments.
- if (!canPadInst(Inst, OS))
+ if (!CanPadInst)
return;
if (PendingBA && PendingBA->getNext() == OS.getCurrentFragment()) {
@@ -542,15 +547,12 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
/// Set the last fragment to be aligned for the BoundaryAlignFragment.
void X86AsmBackend::emitInstructionEnd(MCObjectStreamer &OS,
const MCInst &Inst) {
- MCFragment *CF = OS.getCurrentFragment();
- if (CF->getKind() == MCFragment::FT_Relaxable)
- CF->setAllowAutoPadding(canPadInst(Inst, OS));
-
// Update PrevInstOpcode here, canPadInst() reads that.
+ MCFragment *CF = OS.getCurrentFragment();
PrevInstOpcode = Inst.getOpcode();
PrevInstPosition = std::make_pair(CF, getSizeForInstFragment(CF));
- if (!canPadBranches(OS))
+ if (!OS.getAllowAutoPadding() || !canPadBranches(OS))
return;
// PrevInst is only needed if canPadBranches. Copying an MCInst isn't cheap.
More information about the llvm-commits
mailing list