Skip to content

MC: Simplify fragment reuse determination #149471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions llvm/include/llvm/MC/MCObjectStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 differs from the current fragment.
MCFragment *getOrCreateDataFragment(const MCSubtargetInfo *STI = nullptr);
MCFragment *getOrCreateDataFragment();

protected:
bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/MC/MCSection.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class LLVM_ABI MCSection {
// destructors.
class MCFragment {
friend class MCAssembler;
friend class MCStreamer;
friend class MCObjectStreamer;
friend class MCSection;

Expand Down
4 changes: 3 additions & 1 deletion llvm/include/llvm/MC/MCStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 ""; }
Expand Down
43 changes: 19 additions & 24 deletions llvm/lib/MC/MCObjectStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 difference
// 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);
}
Expand Down Expand Up @@ -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 difference
// 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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}

Expand Down
13 changes: 13 additions & 0 deletions llvm/lib/MC/MCStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions llvm/test/MC/RISCV/Relocations/mc-dump.s
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't the idea of this patch sequence to cut down on zero-sized data fragments? Is there something we can do here to get rid of this one?

Copy link
Member Author

@MaskRay MaskRay Jul 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#149030 (or another change) will remove this empty FT_Data. The fragment mechanism requires a large overhaul and I need to split the changes into pieces. The state after this patch does introduce an extra empty fragment for this corner case of RISC-V.


Intel's Jump Conditional Code Erratum https://reviews.llvm.org/D76475 made the improvement a bit complicated. I'll sort it out.

# 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
Expand Down
Loading