[llvm] [MC] Aligned bundling: remove special handling for RelaxAll (PR #95188)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 18:44:01 PDT 2024
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/95188
>From eae4e408418cd29533228d236e23785071e16cdc Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Tue, 11 Jun 2024 18:31:33 -0700
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
llvm/include/llvm/MC/MCELFStreamer.h | 8 --
llvm/include/llvm/MC/MCWasmStreamer.h | 3 -
llvm/lib/MC/MCELFStreamer.cpp | 89 +------------------
llvm/lib/MC/MCObjectStreamer.cpp | 8 +-
llvm/lib/MC/MCWasmStreamer.cpp | 13 ---
.../bundle-group-too-large-error.s | 2 +-
.../AlignedBundling/misaligned-bundle-group.s | 6 +-
.../X86/AlignedBundling/misaligned-bundle.s | 7 +-
8 files changed, 8 insertions(+), 128 deletions(-)
diff --git a/llvm/include/llvm/MC/MCELFStreamer.h b/llvm/include/llvm/MC/MCELFStreamer.h
index 1ff029d44d376..5f8ae5ace56fd 100644
--- a/llvm/include/llvm/MC/MCELFStreamer.h
+++ b/llvm/include/llvm/MC/MCELFStreamer.h
@@ -39,7 +39,6 @@ class MCELFStreamer : public MCObjectStreamer {
/// state management
void reset() override {
SeenIdent = false;
- BundleGroups.clear();
MCObjectStreamer::reset();
}
@@ -142,14 +141,7 @@ class MCELFStreamer : public MCObjectStreamer {
void finalizeCGProfileEntry(const MCSymbolRefExpr *&S, uint64_t Offset);
void finalizeCGProfile();
- /// Merge the content of the fragment \p EF into the fragment \p DF.
- void mergeFragment(MCDataFragment *, MCDataFragment *);
-
bool SeenIdent = false;
-
- /// BundleGroups - The stack of fragments holding the bundle-locked
- /// instructions.
- SmallVector<MCDataFragment *, 4> BundleGroups;
};
MCELFStreamer *createARMELFStreamer(MCContext &Context,
diff --git a/llvm/include/llvm/MC/MCWasmStreamer.h b/llvm/include/llvm/MC/MCWasmStreamer.h
index f58405214a80a..f95628d5e0e5f 100644
--- a/llvm/include/llvm/MC/MCWasmStreamer.h
+++ b/llvm/include/llvm/MC/MCWasmStreamer.h
@@ -73,9 +73,6 @@ class MCWasmStreamer : public MCObjectStreamer {
void fixSymbolsInTLSFixups(const MCExpr *expr);
- /// Merge the content of the fragment \p EF into the fragment \p DF.
- void mergeFragment(MCDataFragment *, MCDataFragment *);
-
bool SeenIdent;
};
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 23e926c3a9d14..1c8de26d01b30 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -50,44 +50,6 @@ bool MCELFStreamer::isBundleLocked() const {
return getCurrentSectionOnly()->isBundleLocked();
}
-void MCELFStreamer::mergeFragment(MCDataFragment *DF,
- MCDataFragment *EF) {
- MCAssembler &Assembler = getAssembler();
-
- if (Assembler.isBundlingEnabled() && Assembler.getRelaxAll()) {
- uint64_t FSize = EF->getContents().size();
-
- if (FSize > Assembler.getBundleAlignSize())
- report_fatal_error("Fragment can't be larger than a bundle size");
-
- uint64_t RequiredBundlePadding = computeBundlePadding(
- Assembler, EF, DF->getContents().size(), FSize);
-
- if (RequiredBundlePadding > UINT8_MAX)
- report_fatal_error("Padding cannot exceed 255 bytes");
-
- if (RequiredBundlePadding > 0) {
- SmallString<256> Code;
- raw_svector_ostream VecOS(Code);
- EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
- Assembler.writeFragmentPadding(VecOS, *EF, FSize);
-
- DF->getContents().append(Code.begin(), Code.end());
- }
- }
-
- flushPendingLabels(DF, DF->getContents().size());
-
- for (unsigned i = 0, e = EF->getFixups().size(); i != e; ++i) {
- EF->getFixups()[i].setOffset(EF->getFixups()[i].getOffset() +
- DF->getContents().size());
- DF->getFixups().push_back(EF->getFixups()[i]);
- }
- if (DF->getSubtargetInfo() == nullptr && EF->getSubtargetInfo())
- DF->setHasInstructions(*EF->getSubtargetInfo());
- DF->getContents().append(EF->getContents().begin(), EF->getContents().end());
-}
-
void MCELFStreamer::initSections(bool NoExecStack, const MCSubtargetInfo &STI) {
MCContext &Ctx = getContext();
switchSection(Ctx.getObjectFileInfo()->getTextSection());
@@ -575,24 +537,12 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
if (Assembler.isBundlingEnabled()) {
MCSection &Sec = *getCurrentSectionOnly();
- if (Assembler.getRelaxAll() && isBundleLocked()) {
- // If the -mc-relax-all flag is used and we are bundle-locked, we re-use
- // the current bundle group.
- DF = BundleGroups.back();
- CheckBundleSubtargets(DF->getSubtargetInfo(), &STI);
- }
- else if (Assembler.getRelaxAll() && !isBundleLocked())
- // When not in a bundle-locked group and the -mc-relax-all flag is used,
- // we create a new temporary fragment which will be later merged into
- // the current fragment.
- DF = new MCDataFragment();
- else if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
+ if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
// If we are bundle-locked, we re-use the current fragment.
// The bundle-locking directive ensures this is a new data fragment.
DF = cast<MCDataFragment>(getCurrentFragment());
CheckBundleSubtargets(DF->getSubtargetInfo(), &STI);
- }
- else if (!isBundleLocked() && Fixups.size() == 0) {
+ } else if (!isBundleLocked() && Fixups.size() == 0) {
// Optimize memory usage by emitting the instruction to a
// MCCompactEncodedInstFragment when not in a bundle-locked group and
// there are no fixups registered.
@@ -631,13 +581,6 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
getAssembler().getBackend().RelaxFixupKind)
DF->setLinkerRelaxable();
DF->getContents().append(Code.begin(), Code.end());
-
- if (Assembler.isBundlingEnabled() && Assembler.getRelaxAll()) {
- if (!isBundleLocked()) {
- mergeFragment(getOrCreateDataFragment(&STI), DF);
- delete DF;
- }
- }
}
void MCELFStreamer::emitBundleAlignMode(Align Alignment) {
@@ -659,12 +602,6 @@ void MCELFStreamer::emitBundleLock(bool AlignToEnd) {
if (!isBundleLocked())
Sec.setBundleGroupBeforeFirstInst(true);
- if (getAssembler().getRelaxAll() && !isBundleLocked()) {
- // TODO: drop the lock state and set directly in the fragment
- MCDataFragment *DF = new MCDataFragment();
- BundleGroups.push_back(DF);
- }
-
Sec.setBundleLockState(AlignToEnd ? MCSection::BundleLockedAlignToEnd
: MCSection::BundleLocked);
}
@@ -679,27 +616,7 @@ void MCELFStreamer::emitBundleUnlock() {
else if (Sec.isBundleGroupBeforeFirstInst())
report_fatal_error("Empty bundle-locked group is forbidden");
- // When the -mc-relax-all flag is used, we emit instructions to fragments
- // stored on a stack. When the bundle unlock is emitted, we pop a fragment
- // from the stack a merge it to the one below.
- if (getAssembler().getRelaxAll()) {
- assert(!BundleGroups.empty() && "There are no bundle groups");
- MCDataFragment *DF = BundleGroups.back();
-
- // FIXME: Use BundleGroups to track the lock state instead.
- Sec.setBundleLockState(MCSection::NotBundleLocked);
-
- // FIXME: Use more separate fragments for nested groups.
- if (!isBundleLocked()) {
- mergeFragment(getOrCreateDataFragment(DF->getSubtargetInfo()), DF);
- BundleGroups.pop_back();
- delete DF;
- }
-
- if (Sec.getBundleLockState() != MCSection::BundleLockedAlignToEnd)
- getOrCreateDataFragment()->setAlignToBundleEnd(false);
- } else
- Sec.setBundleLockState(MCSection::NotBundleLocked);
+ Sec.setBundleLockState(MCSection::NotBundleLocked);
}
void MCELFStreamer::finishImpl() {
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index bf1ce76cdc14b..e3b2801b25329 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -215,7 +215,7 @@ static bool canReuseDataFragment(const MCDataFragment &F,
// When bundling is enabled, we don't want to add data to a fragment that
// already has instructions (see MCELFStreamer::emitInstToData for details)
if (Assembler.isBundlingEnabled())
- return Assembler.getRelaxAll();
+ return false;
// If the subtarget is changed mid fragment we start a new fragment to record
// the new STI.
return !STI || F.getSubtargetInfo() == STI;
@@ -292,8 +292,7 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
// Otherwise queue the label and set its fragment pointer when we emit the
// next fragment.
auto *F = dyn_cast_or_null<MCDataFragment>(getCurrentFragment());
- if (F && !(getAssembler().isBundlingEnabled() &&
- getAssembler().getRelaxAll())) {
+ if (F) {
Symbol->setFragment(F);
Symbol->setOffset(F->getContents().size());
} else {
@@ -465,9 +464,6 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
const MCSubtargetInfo &STI) {
- if (getAssembler().getRelaxAll() && getAssembler().isBundlingEnabled())
- llvm_unreachable("All instructions should have already been relaxed");
-
// Always create a new, separate fragment here, because its size can change
// during relaxation.
MCRelaxableFragment *IF = new MCRelaxableFragment(Inst, STI);
diff --git a/llvm/lib/MC/MCWasmStreamer.cpp b/llvm/lib/MC/MCWasmStreamer.cpp
index c553ede77555a..8b59a6c3446f9 100644
--- a/llvm/lib/MC/MCWasmStreamer.cpp
+++ b/llvm/lib/MC/MCWasmStreamer.cpp
@@ -39,19 +39,6 @@ using namespace llvm;
MCWasmStreamer::~MCWasmStreamer() = default; // anchor.
-void MCWasmStreamer::mergeFragment(MCDataFragment *DF, MCDataFragment *EF) {
- flushPendingLabels(DF, DF->getContents().size());
-
- for (unsigned I = 0, E = EF->getFixups().size(); I != E; ++I) {
- EF->getFixups()[I].setOffset(EF->getFixups()[I].getOffset() +
- DF->getContents().size());
- DF->getFixups().push_back(EF->getFixups()[I]);
- }
- if (DF->getSubtargetInfo() == nullptr && EF->getSubtargetInfo())
- DF->setHasInstructions(*EF->getSubtargetInfo());
- DF->getContents().append(EF->getContents().begin(), EF->getContents().end());
-}
-
void MCWasmStreamer::emitLabel(MCSymbol *S, SMLoc Loc) {
auto *Symbol = cast<MCSymbolWasm>(S);
MCObjectStreamer::emitLabel(Symbol, Loc);
diff --git a/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s b/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
index 697b8bf6ab6c0..b3b4d9b6e9324 100644
--- a/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
+++ b/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
@@ -1,5 +1,5 @@
# RUN: not --crash llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
-# RUN: not --crash llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu -mc-relax-all %s -o - 2>&1 | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu -mc-relax-all %s -o /dev/null
# CHECK: ERROR: Fragment can't be larger than a bundle size
diff --git a/llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s b/llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s
index 6962a2a65960d..92bd9ec016bd5 100644
--- a/llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s
+++ b/llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s
@@ -3,7 +3,7 @@
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-OPT %s
# RUN: llvm-mc -filetype=obj -triple i686-pc-linux-gnu -mcpu=pentiumpro -mc-relax-all %s -o - \
# RUN: | llvm-objdump -d --no-show-raw-insn - \
-# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-RELAX %s
+# RUN: | FileCheck --check-prefixes=CHECK,CHECK-OPT %s
.text
foo:
@@ -13,11 +13,7 @@ foo:
.bundle_lock align_to_end
# CHECK: 1: nopw %cs:(%eax,%eax)
# CHECK: 10: nopw %cs:(%eax,%eax)
-# CHECK-RELAX: 1a: nop
-# CHECK-RELAX: 20: nopw %cs:(%eax,%eax)
-# CHECK-RELAX: 2a: nopw %cs:(%eax,%eax)
# CHECK-OPT: 1b: calll 0x1c
-# CHECK-RELAX: 3b: calll 0x3c
calll bar # 5 bytes
.bundle_unlock
ret # 1 byte
diff --git a/llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s b/llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s
index 7a84bffc1821e..0bf5cfd802be9 100644
--- a/llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s
+++ b/llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s
@@ -3,7 +3,7 @@
# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-OPT %s
# RUN: llvm-mc -filetype=obj -triple i686-pc-linux-gnu -mcpu=pentiumpro -mc-relax-all %s -o - \
# RUN: | llvm-objdump --no-print-imm-hex -d --no-show-raw-insn - \
-# RUN: | FileCheck -check-prefix=CHECK -check-prefix=CHECK-RELAX %s
+# RUN: | FileCheck --check-prefixes=CHECK,CHECK-OPT %s
.text
foo:
@@ -11,17 +11,12 @@ foo:
push %ebp # 1 byte
.align 16
# CHECK: 1: nopw %cs:(%eax,%eax)
-# CHECK-RELAX: 10: nopw %cs:(%eax,%eax)
-# CHECK-RELAX: 1a: nop
# CHECK-OPT: 10: movl $1, (%esp)
-# CHECK-RELAX: 20: movl $1, (%esp)
movl $0x1, (%esp) # 7 bytes
movl $0x1, (%esp) # 7 bytes
# CHECK-OPT: 1e: nop
movl $0x2, 0x1(%esp) # 8 bytes
movl $0x2, 0x1(%esp) # 8 bytes
-# CHECK-RELAX: 3e: nop
-# CHECK-RELAX: 40: movl $2, 1(%esp)
movl $0x2, 0x1(%esp) # 8 bytes
movl $0x2, (%esp) # 7 bytes
# CHECK-OPT: 3f: nop
>From 3590a05d5a60069bc4073e354f96fd9eec6fc8a4 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Tue, 11 Jun 2024 18:43:52 -0700
Subject: [PATCH 2/2] drop another piece of code that is part of
https://reviews.llvm.org/D8072
Created using spr 1.3.5-bogner
---
llvm/lib/MC/MCAssembler.cpp | 8 +-------
.../MC/X86/AlignedBundling/bundle-group-too-large-error.s | 2 +-
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 4ff606d373238..f9ed6fa0d5633 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -420,12 +420,6 @@ void MCAsmLayout::layoutBundle(MCFragment *F) {
// The fragment's offset will point to after the padding, and its computed
// size won't include the padding.
//
- // When the -mc-relax-all flag is used, we optimize bundling by writting the
- // padding directly into fragments when the instructions are emitted inside
- // the streamer. When the fragment is larger than the bundle size, we need to
- // ensure that it's bundle aligned. This means that if we end up with
- // multiple fragments, we must emit bundle padding between fragments.
- //
// ".align N" is an example of a directive that introduces multiple
// fragments. We could add a special case to handle ".align N" by emitting
// within-fragment padding (which would produce less padding when N is less
@@ -436,7 +430,7 @@ void MCAsmLayout::layoutBundle(MCFragment *F) {
MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
uint64_t FSize = Assembler.computeFragmentSize(*this, *EF);
- if (!Assembler.getRelaxAll() && FSize > Assembler.getBundleAlignSize())
+ if (FSize > Assembler.getBundleAlignSize())
report_fatal_error("Fragment can't be larger than a bundle size");
uint64_t RequiredBundlePadding =
diff --git a/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s b/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
index b3b4d9b6e9324..697b8bf6ab6c0 100644
--- a/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
+++ b/llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s
@@ -1,5 +1,5 @@
# RUN: not --crash llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
-# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu -mc-relax-all %s -o /dev/null
+# RUN: not --crash llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu -mc-relax-all %s -o - 2>&1 | FileCheck %s
# CHECK: ERROR: Fragment can't be larger than a bundle size
More information about the llvm-commits
mailing list