[llvm] 73e4b58 - MC: Simplify fragment reuse determination
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 18 09:51:24 PDT 2025
Author: Fangrui Song
Date: 2025-07-18T09:51:21-07:00
New Revision: 73e4b589ba9526c72f495ca6898ed18d730d2db4
URL: https://github.com/llvm/llvm-project/commit/73e4b589ba9526c72f495ca6898ed18d730d2db4
DIFF: https://github.com/llvm/llvm-project/commit/73e4b589ba9526c72f495ca6898ed18d730d2db4.diff
LOG: 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: https://github.com/llvm/llvm-project/pull/149471
Added:
Modified:
llvm/include/llvm/MC/MCObjectStreamer.h
llvm/include/llvm/MC/MCSection.h
llvm/include/llvm/MC/MCStreamer.h
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
llvm/lib/MC/MCStreamer.cpp
llvm/test/MC/RISCV/Relocations/mc-dump.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index a55fd4a14675f..319e131999d48 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -73,20 +73,9 @@ class MCObjectStreamer : public MCStreamer {
MCSymbol *emitCFILabel() override;
void emitCFISections(bool EH, bool Debug) override;
- void insert(MCFragment *F) {
- auto *Sec = CurFrag->getParent();
- F->setParent(Sec);
- F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
- CurFrag->Next = F;
- CurFrag = F;
- Sec->curFragList()->Tail = F;
- }
-
/// Get a data fragment to write into, creating a new one if the current
/// fragment is not FT_Data.
- /// Optionally a \p STI can be passed in so that a new fragment is created
- /// if the Subtarget
diff ers from the current fragment.
- MCFragment *getOrCreateDataFragment(const MCSubtargetInfo *STI = nullptr);
+ MCFragment *getOrCreateDataFragment();
protected:
bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 296fdd8af0d14..313071ec75033 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -188,6 +188,7 @@ class LLVM_ABI MCSection {
// destructors.
class MCFragment {
friend class MCAssembler;
+ friend class MCStreamer;
friend class MCObjectStreamer;
friend class MCSection;
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index b3a9aabd6ece5..4b91dbc794682 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -429,7 +429,6 @@ class LLVM_ABI MCStreamer {
CurFrag->getParent() == getCurrentSection().first);
return CurFrag;
}
-
/// Save the current and previous section on the section stack.
void pushSection() {
SectionStack.push_back(
@@ -457,6 +456,9 @@ class LLVM_ABI MCStreamer {
MCSymbol *endSection(MCSection *Section);
+ void insert(MCFragment *F);
+ void newFragment();
+
/// Returns the mnemonic for \p MI, if the streamer has access to a
/// instruction printer and returns an empty string otherwise.
virtual StringRef getMnemonic(const MCInst &MI) const { return ""; }
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 67433f2b265e5..d5b8f22463894 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -106,26 +106,12 @@ void MCObjectStreamer::emitFrames(MCAsmBackend *MAB) {
MCDwarfFrameEmitter::Emit(*this, MAB, false);
}
-static bool canReuseDataFragment(const MCFragment &F,
- const MCAssembler &Assembler,
- const MCSubtargetInfo *STI) {
- if (!F.hasInstructions())
- return true;
- // Do not add data after a linker-relaxable instruction. The
diff erence
- // between a new label and a label at or before the linker-relaxable
- // instruction cannot be resolved at assemble-time.
- if (F.isLinkerRelaxable())
- return false;
- // If the subtarget is changed mid fragment we start a new fragment to record
- // the new STI.
- return !STI || F.getSubtargetInfo() == STI;
-}
-
-MCFragment *
-MCObjectStreamer::getOrCreateDataFragment(const MCSubtargetInfo *STI) {
+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 ||
- !canReuseDataFragment(*F, *Assembler, STI)) {
+ if (F->getKind() != MCFragment::FT_Data) {
F = getContext().allocFragment<MCFragment>();
insert(F);
}
@@ -363,16 +349,23 @@ void MCObjectStreamer::emitInstToData(const MCInst &Inst,
F->doneAppending();
if (!Fixups.empty())
F->appendFixups(Fixups);
+ F->setHasInstructions(STI);
+ bool MarkedLinkerRelaxable = false;
for (auto &Fixup : MutableArrayRef(F->getFixups()).slice(FixupStartIndex)) {
Fixup.setOffset(Fixup.getOffset() + CodeOffset);
- if (Fixup.isLinkerRelaxable()) {
- F->setLinkerRelaxable();
+ if (!Fixup.isLinkerRelaxable())
+ continue;
+ F->setLinkerRelaxable();
+ // Do not add data after a linker-relaxable instruction. The
diff erence
+ // between a new label and a label at or before the linker-relaxable
+ // instruction cannot be resolved at assemble-time.
+ if (!MarkedLinkerRelaxable) {
+ MarkedLinkerRelaxable = true;
getCurrentSectionOnly()->setLinkerRelaxable();
+ newFragment();
}
}
-
- F->setHasInstructions(STI);
}
void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
@@ -568,8 +561,10 @@ void MCObjectStreamer::emitCodeAlignment(Align Alignment,
// if the alignment is larger than the minimum NOP size.
unsigned Size;
if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
- Size))
+ Size)) {
getCurrentSectionOnly()->setLinkerRelaxable();
+ newFragment();
+ }
}
void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,
diff --git a/llvm/lib/MC/MCParser/MCTargetAsmParser.cpp b/llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
index 665d92eb9a21c..7f0934971b27c 100644
--- a/llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
+++ b/llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
@@ -9,6 +9,7 @@
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCRegister.h"
+#include "llvm/MC/MCStreamer.h"
using namespace llvm;
@@ -22,6 +23,10 @@ MCTargetAsmParser::~MCTargetAsmParser() = default;
MCSubtargetInfo &MCTargetAsmParser::copySTI() {
MCSubtargetInfo &STICopy = getContext().getSubtargetCopy(getSTI());
STI = &STICopy;
+ // The returned STI will likely be modified. Create a new fragment to prevent
+ // mixing STI values within a fragment.
+ if (getStreamer().getCurrentFragment())
+ getStreamer().newFragment();
return STICopy;
}
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index d814ab8880500..c3ecf8fc717f5 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1404,6 +1404,19 @@ MCSymbol *MCStreamer::endSection(MCSection *Section) {
return Sym;
}
+void MCStreamer::insert(MCFragment *F) {
+ auto *Sec = CurFrag->getParent();
+ F->setParent(Sec);
+ F->setLayoutOrder(CurFrag->getLayoutOrder() + 1);
+ CurFrag->Next = F;
+ CurFrag = F;
+ Sec->curFragList()->Tail = F;
+}
+
+void MCStreamer::newFragment() {
+ insert(getContext().allocFragment<MCFragment>());
+}
+
static VersionTuple
targetVersionOrMinimumSupportedOSVersion(const Triple &Target,
VersionTuple TargetVersion) {
diff --git a/llvm/test/MC/RISCV/Relocations/mc-dump.s b/llvm/test/MC/RISCV/Relocations/mc-dump.s
index 24f3e67ebbdda..f72258498169f 100644
--- a/llvm/test/MC/RISCV/Relocations/mc-dump.s
+++ b/llvm/test/MC/RISCV/Relocations/mc-dump.s
@@ -9,6 +9,7 @@
# CHECK-NEXT:0 Data LinkerRelaxable Size:8 [97,00,00,00,e7,80,00,00]
# CHECK-NEXT: Fixup @0 Value:specifier(19,ext) Kind:4023
# CHECK-NEXT: Symbol @0 $x
+# CHECK-NEXT:8 Data Size:0 []
# CHECK-NEXT:8 Align Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
# CHECK-NEXT:12 Data Size:4 [13,05,30,00]
# CHECK-NEXT:16 Align Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
More information about the llvm-commits
mailing list