[llvm] [MC] Aligned bundling: remove special handling for RelaxAll (PR #95188)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 18:32:13 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
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 3 changed tests. However, most `-mc-relax-all` tests do
not need changes.
RelaxAll used to be the default for clang -O0. This mode has significant
code size drawbacks and newer Clang doesn't use it (#<!-- -->90013).
---
Full diff: https://github.com/llvm/llvm-project/pull/95188.diff
8 Files Affected:
- (modified) llvm/include/llvm/MC/MCELFStreamer.h (-8)
- (modified) llvm/include/llvm/MC/MCWasmStreamer.h (-3)
- (modified) llvm/lib/MC/MCELFStreamer.cpp (+3-86)
- (modified) llvm/lib/MC/MCObjectStreamer.cpp (+2-6)
- (modified) llvm/lib/MC/MCWasmStreamer.cpp (-13)
- (modified) llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s (+1-1)
- (modified) llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s (+1-5)
- (modified) llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s (+1-6)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/95188
More information about the llvm-commits
mailing list