From eae4e408418cd29533228d236e23785071e16cdc Mon Sep 17 00:00:00 2001 From: Fangrui Song 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 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(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(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(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(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 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(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