Skip to content

Commit b1932b8

Browse files
MaskRayMaskRay
authored
[MC] Aligned bundling: remove special handling for RelaxAll
When both aligned bundling and RelaxAll are enabled, bundle padding is directly written into fragments (https://reviews.llvm.org/D8072). (The original motivation was memory usage, which has been achieved from different angles with recent assembler improvement). The code presents challenges with the work to replace fragment representation (e.g. #94950 #95077). This patch removes the special handling. RelaxAll still works but the behavior seems slightly different as revealed by 2 changed tests. However, most `-mc-relax-all` tests are unchanged. RelaxAll used to be the default for clang -O0. This mode has significant code size drawbacks and newer Clang doesn't use it (#90013). --- flushPendingLabels: The FOffset parameter can be removed: pending labels will be assigned to the incoming fragment at offset 0. Pull Request: #95188
1 parent 72b841d commit b1932b8

File tree

10 files changed

+13
-141
lines changed

10 files changed

+13
-141
lines changed

‎llvm/include/llvm/MC/MCELFStreamer.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ class MCELFStreamer : public MCObjectStreamer {
3939
/// state management
4040
void reset() override {
4141
SeenIdent = false;
42-
BundleGroups.clear();
4342
MCObjectStreamer::reset();
4443
}
4544

@@ -142,14 +141,7 @@ class MCELFStreamer : public MCObjectStreamer {
142141
void finalizeCGProfileEntry(const MCSymbolRefExpr *&S, uint64_t Offset);
143142
void finalizeCGProfile();
144143

145-
/// Merge the content of the fragment \p EF into the fragment \p DF.
146-
void mergeFragment(MCDataFragment *, MCDataFragment *);
147-
148144
bool SeenIdent = false;
149-
150-
/// BundleGroups - The stack of fragments holding the bundle-locked
151-
/// instructions.
152-
SmallVector<MCDataFragment *, 4> BundleGroups;
153145
};
154146

155147
MCELFStreamer *createARMELFStreamer(MCContext &Context,

‎llvm/include/llvm/MC/MCSection.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,7 @@ class MCSection {
227227
void addPendingLabel(MCSymbol* label, unsigned Subsection = 0);
228228

229229
/// Associate all pending labels in a subsection with a fragment.
230-
void flushPendingLabels(MCFragment *F, uint64_t FOffset = 0,
231-
unsigned Subsection = 0);
230+
void flushPendingLabels(MCFragment *F, unsigned Subsection);
232231

233232
/// Associate all pending labels with empty data fragments. One fragment
234233
/// will be created for each subsection as necessary.

‎llvm/include/llvm/MC/MCWasmStreamer.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,6 @@ class MCWasmStreamer : public MCObjectStreamer {
7373

7474
void fixSymbolsInTLSFixups(const MCExpr *expr);
7575

76-
/// Merge the content of the fragment \p EF into the fragment \p DF.
77-
void mergeFragment(MCDataFragment *, MCDataFragment *);
78-
7976
bool SeenIdent;
8077
};
8178

‎llvm/lib/MC/MCAssembler.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -420,12 +420,6 @@ void MCAsmLayout::layoutBundle(MCFragment *F) {
420420
// The fragment's offset will point to after the padding, and its computed
421421
// size won't include the padding.
422422
//
423-
// When the -mc-relax-all flag is used, we optimize bundling by writting the
424-
// padding directly into fragments when the instructions are emitted inside
425-
// the streamer. When the fragment is larger than the bundle size, we need to
426-
// ensure that it's bundle aligned. This means that if we end up with
427-
// multiple fragments, we must emit bundle padding between fragments.
428-
//
429423
// ".align N" is an example of a directive that introduces multiple
430424
// fragments. We could add a special case to handle ".align N" by emitting
431425
// within-fragment padding (which would produce less padding when N is less
@@ -436,7 +430,7 @@ void MCAsmLayout::layoutBundle(MCFragment *F) {
436430
MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
437431
uint64_t FSize = Assembler.computeFragmentSize(*this, *EF);
438432

439-
if (!Assembler.getRelaxAll() && FSize > Assembler.getBundleAlignSize())
433+
if (FSize > Assembler.getBundleAlignSize())
440434
report_fatal_error("Fragment can't be larger than a bundle size");
441435

442436
uint64_t RequiredBundlePadding =

‎llvm/lib/MC/MCELFStreamer.cpp

Lines changed: 3 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -50,44 +50,6 @@ bool MCELFStreamer::isBundleLocked() const {
5050
return getCurrentSectionOnly()->isBundleLocked();
5151
}
5252

53-
void MCELFStreamer::mergeFragment(MCDataFragment *DF,
54-
MCDataFragment *EF) {
55-
MCAssembler &Assembler = getAssembler();
56-
57-
if (Assembler.isBundlingEnabled() && Assembler.getRelaxAll()) {
58-
uint64_t FSize = EF->getContents().size();
59-
60-
if (FSize > Assembler.getBundleAlignSize())
61-
report_fatal_error("Fragment can't be larger than a bundle size");
62-
63-
uint64_t RequiredBundlePadding = computeBundlePadding(
64-
Assembler, EF, DF->getContents().size(), FSize);
65-
66-
if (RequiredBundlePadding > UINT8_MAX)
67-
report_fatal_error("Padding cannot exceed 255 bytes");
68-
69-
if (RequiredBundlePadding > 0) {
70-
SmallString<256> Code;
71-
raw_svector_ostream VecOS(Code);
72-
EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
73-
Assembler.writeFragmentPadding(VecOS, *EF, FSize);
74-
75-
DF->getContents().append(Code.begin(), Code.end());
76-
}
77-
}
78-
79-
flushPendingLabels(DF, DF->getContents().size());
80-
81-
for (unsigned i = 0, e = EF->getFixups().size(); i != e; ++i) {
82-
EF->getFixups()[i].setOffset(EF->getFixups()[i].getOffset() +
83-
DF->getContents().size());
84-
DF->getFixups().push_back(EF->getFixups()[i]);
85-
}
86-
if (DF->getSubtargetInfo() == nullptr && EF->getSubtargetInfo())
87-
DF->setHasInstructions(*EF->getSubtargetInfo());
88-
DF->getContents().append(EF->getContents().begin(), EF->getContents().end());
89-
}
90-
9153
void MCELFStreamer::initSections(bool NoExecStack, const MCSubtargetInfo &STI) {
9254
MCContext &Ctx = getContext();
9355
switchSection(Ctx.getObjectFileInfo()->getTextSection());
@@ -575,24 +537,12 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
575537

576538
if (Assembler.isBundlingEnabled()) {
577539
MCSection &Sec = *getCurrentSectionOnly();
578-
if (Assembler.getRelaxAll() && isBundleLocked()) {
579-
// If the -mc-relax-all flag is used and we are bundle-locked, we re-use
580-
// the current bundle group.
581-
DF = BundleGroups.back();
582-
CheckBundleSubtargets(DF->getSubtargetInfo(), &STI);
583-
}
584-
else if (Assembler.getRelaxAll() && !isBundleLocked())
585-
// When not in a bundle-locked group and the -mc-relax-all flag is used,
586-
// we create a new temporary fragment which will be later merged into
587-
// the current fragment.
588-
DF = getContext().allocFragment<MCDataFragment>();
589-
else if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
540+
if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
590541
// If we are bundle-locked, we re-use the current fragment.
591542
// The bundle-locking directive ensures this is a new data fragment.
592543
DF = cast<MCDataFragment>(getCurrentFragment());
593544
CheckBundleSubtargets(DF->getSubtargetInfo(), &STI);
594-
}
595-
else if (!isBundleLocked() && Fixups.size() == 0) {
545+
} else if (!isBundleLocked() && Fixups.size() == 0) {
596546
// Optimize memory usage by emitting the instruction to a
597547
// MCCompactEncodedInstFragment when not in a bundle-locked group and
598548
// there are no fixups registered.
@@ -632,13 +582,6 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
632582
getAssembler().getBackend().RelaxFixupKind)
633583
DF->setLinkerRelaxable();
634584
DF->getContents().append(Code.begin(), Code.end());
635-
636-
if (Assembler.isBundlingEnabled() && Assembler.getRelaxAll()) {
637-
if (!isBundleLocked()) {
638-
mergeFragment(getOrCreateDataFragment(&STI), DF);
639-
delete DF;
640-
}
641-
}
642585
}
643586

644587
void MCELFStreamer::emitBundleAlignMode(Align Alignment) {
@@ -660,12 +603,6 @@ void MCELFStreamer::emitBundleLock(bool AlignToEnd) {
660603
if (!isBundleLocked())
661604
Sec.setBundleGroupBeforeFirstInst(true);
662605

663-
if (getAssembler().getRelaxAll() && !isBundleLocked()) {
664-
// TODO: drop the lock state and set directly in the fragment
665-
MCDataFragment *DF = getContext().allocFragment<MCDataFragment>();
666-
BundleGroups.push_back(DF);
667-
}
668-
669606
Sec.setBundleLockState(AlignToEnd ? MCSection::BundleLockedAlignToEnd
670607
: MCSection::BundleLocked);
671608
}
@@ -680,27 +617,7 @@ void MCELFStreamer::emitBundleUnlock() {
680617
else if (Sec.isBundleGroupBeforeFirstInst())
681618
report_fatal_error("Empty bundle-locked group is forbidden");
682619

683-
// When the -mc-relax-all flag is used, we emit instructions to fragments
684-
// stored on a stack. When the bundle unlock is emitted, we pop a fragment
685-
// from the stack a merge it to the one below.
686-
if (getAssembler().getRelaxAll()) {
687-
assert(!BundleGroups.empty() && "There are no bundle groups");
688-
MCDataFragment *DF = BundleGroups.back();
689-
690-
// FIXME: Use BundleGroups to track the lock state instead.
691-
Sec.setBundleLockState(MCSection::NotBundleLocked);
692-
693-
// FIXME: Use more separate fragments for nested groups.
694-
if (!isBundleLocked()) {
695-
mergeFragment(getOrCreateDataFragment(DF->getSubtargetInfo()), DF);
696-
BundleGroups.pop_back();
697-
delete DF;
698-
}
699-
700-
if (Sec.getBundleLockState() != MCSection::BundleLockedAlignToEnd)
701-
getOrCreateDataFragment()->setAlignToBundleEnd(false);
702-
} else
703-
Sec.setBundleLockState(MCSection::NotBundleLocked);
620+
Sec.setBundleLockState(MCSection::NotBundleLocked);
704621
}
705622

706623
void MCELFStreamer::finishImpl() {

‎llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ void MCObjectStreamer::flushPendingLabels(MCFragment *F, uint64_t FOffset) {
8181
}
8282

8383
// Associate the labels with F.
84-
CurSection->flushPendingLabels(F, FOffset, CurSubsectionIdx);
84+
CurSection->flushPendingLabels(F, CurSubsectionIdx);
8585
}
8686

8787
void MCObjectStreamer::flushPendingLabels() {
@@ -215,7 +215,7 @@ static bool canReuseDataFragment(const MCDataFragment &F,
215215
// When bundling is enabled, we don't want to add data to a fragment that
216216
// already has instructions (see MCELFStreamer::emitInstToData for details)
217217
if (Assembler.isBundlingEnabled())
218-
return Assembler.getRelaxAll();
218+
return false;
219219
// If the subtarget is changed mid fragment we start a new fragment to record
220220
// the new STI.
221221
return !STI || F.getSubtargetInfo() == STI;
@@ -292,8 +292,7 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
292292
// Otherwise queue the label and set its fragment pointer when we emit the
293293
// next fragment.
294294
auto *F = dyn_cast_or_null<MCDataFragment>(getCurrentFragment());
295-
if (F && !(getAssembler().isBundlingEnabled() &&
296-
getAssembler().getRelaxAll())) {
295+
if (F) {
297296
Symbol->setFragment(F);
298297
Symbol->setOffset(F->getContents().size());
299298
} else {
@@ -465,9 +464,6 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
465464

466465
void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
467466
const MCSubtargetInfo &STI) {
468-
if (getAssembler().getRelaxAll() && getAssembler().isBundlingEnabled())
469-
llvm_unreachable("All instructions should have already been relaxed");
470-
471467
// Always create a new, separate fragment here, because its size can change
472468
// during relaxation.
473469
MCRelaxableFragment *IF =

‎llvm/lib/MC/MCSection.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,14 @@ void MCSection::addPendingLabel(MCSymbol *label, unsigned Subsection) {
8282
PendingLabels.push_back(PendingLabel(label, Subsection));
8383
}
8484

85-
void MCSection::flushPendingLabels(MCFragment *F, uint64_t FOffset,
86-
unsigned Subsection) {
85+
void MCSection::flushPendingLabels(MCFragment *F, unsigned Subsection) {
8786
// Set the fragment and fragment offset for all pending symbols in the
8887
// specified Subsection, and remove those symbols from the pending list.
8988
for (auto It = PendingLabels.begin(); It != PendingLabels.end(); ++It) {
9089
PendingLabel& Label = *It;
9190
if (Label.Subsection == Subsection) {
9291
Label.Sym->setFragment(F);
93-
Label.Sym->setOffset(FOffset);
92+
assert(Label.Sym->getOffset() == 0);
9493
PendingLabels.erase(It--);
9594
}
9695
}
@@ -105,7 +104,7 @@ void MCSection::flushPendingLabels() {
105104
MCFragment *F = new MCDataFragment();
106105
addFragment(*F);
107106
F->setParent(this);
108-
flushPendingLabels(F, 0, Label.Subsection);
107+
flushPendingLabels(F, Label.Subsection);
109108
}
110109
}
111110

‎llvm/lib/MC/MCWasmStreamer.cpp

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,6 @@ using namespace llvm;
3939

4040
MCWasmStreamer::~MCWasmStreamer() = default; // anchor.
4141

42-
void MCWasmStreamer::mergeFragment(MCDataFragment *DF, MCDataFragment *EF) {
43-
flushPendingLabels(DF, DF->getContents().size());
44-
45-
for (unsigned I = 0, E = EF->getFixups().size(); I != E; ++I) {
46-
EF->getFixups()[I].setOffset(EF->getFixups()[I].getOffset() +
47-
DF->getContents().size());
48-
DF->getFixups().push_back(EF->getFixups()[I]);
49-
}
50-
if (DF->getSubtargetInfo() == nullptr && EF->getSubtargetInfo())
51-
DF->setHasInstructions(*EF->getSubtargetInfo());
52-
DF->getContents().append(EF->getContents().begin(), EF->getContents().end());
53-
}
54-
5542
void MCWasmStreamer::emitLabel(MCSymbol *S, SMLoc Loc) {
5643
auto *Symbol = cast<MCSymbolWasm>(S);
5744
MCObjectStreamer::emitLabel(Symbol, Loc);

‎llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-OPT %s
44
# RUN: llvm-mc -filetype=obj -triple i686-pc-linux-gnu -mcpu=pentiumpro -mc-relax-all %s -o - \
55
# RUN: | llvm-objdump -d --no-show-raw-insn - \
6-
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-RELAX %s
6+
# RUN: | FileCheck --check-prefixes=CHECK,CHECK-OPT %s
77

88
.text
99
foo:
@@ -13,11 +13,7 @@ foo:
1313
.bundle_lock align_to_end
1414
# CHECK: 1: nopw %cs:(%eax,%eax)
1515
# CHECK: 10: nopw %cs:(%eax,%eax)
16-
# CHECK-RELAX: 1a: nop
17-
# CHECK-RELAX: 20: nopw %cs:(%eax,%eax)
18-
# CHECK-RELAX: 2a: nopw %cs:(%eax,%eax)
1916
# CHECK-OPT: 1b: calll 0x1c
20-
# CHECK-RELAX: 3b: calll 0x3c
2117
calll bar # 5 bytes
2218
.bundle_unlock
2319
ret # 1 byte

‎llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,20 @@
33
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-OPT %s
44
# RUN: llvm-mc -filetype=obj -triple i686-pc-linux-gnu -mcpu=pentiumpro -mc-relax-all %s -o - \
55
# RUN: | llvm-objdump --no-print-imm-hex -d --no-show-raw-insn - \
6-
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-RELAX %s
6+
# RUN: | FileCheck --check-prefixes=CHECK,CHECK-OPT %s
77

88
.text
99
foo:
1010
.bundle_align_mode 5
1111
push %ebp # 1 byte
1212
.align 16
1313
# CHECK: 1: nopw %cs:(%eax,%eax)
14-
# CHECK-RELAX: 10: nopw %cs:(%eax,%eax)
15-
# CHECK-RELAX: 1a: nop
1614
# CHECK-OPT: 10: movl $1, (%esp)
17-
# CHECK-RELAX: 20: movl $1, (%esp)
1815
movl $0x1, (%esp) # 7 bytes
1916
movl $0x1, (%esp) # 7 bytes
2017
# CHECK-OPT: 1e: nop
2118
movl $0x2, 0x1(%esp) # 8 bytes
2219
movl $0x2, 0x1(%esp) # 8 bytes
23-
# CHECK-RELAX: 3e: nop
24-
# CHECK-RELAX: 40: movl $2, 1(%esp)
2520
movl $0x2, 0x1(%esp) # 8 bytes
2621
movl $0x2, (%esp) # 7 bytes
2722
# CHECK-OPT: 3f: nop

0 commit comments

Comments
 (0)