Skip to content

MC: Remove bundle alignment mode #148781

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 15, 2025

The being-removed PNaCl has a Software Fault Isolation mechanism, which
requires that certain instructions and groups of instructions do not
cross a bundle boundary. When .bundle_align_mode is in effect, each
instruction is placed in its own fragment, allowing flexible NOP
padding.

This feature has significantly complicated our refactoring of MCStreamer
and MCFragment, leading to considerable effort spent untangling
it (including flushPendingLabels (7500646),
MCAssembler iteration improvement, and recent MCFragment refactoring).

Minor instructions:u decrease for -O0 -g builds
https://llvm-compile-time-tracker.com/compare.php?from=c06d3a7b728293cbc53ff91239d6cd87c0982ffb&to=45f10e08e468f6c106c48ff3198adb5614511a0d&stat=instructions:u

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-mips

Author: Fangrui Song (MaskRay)

Changes

The being-removed PNaCl has a Software Fault Isolation mechanism, which
requires that certain instructions and groups of instructions do not
cross a bundle boundary. When .bundle_align_mode is in effect, each
instruction is placed in its own fragment, allowing flexible NOP
padding.

This feature has significantly complicated our refactoring of MCStreamer
and MCFragment, leading to considerable effort spent untangling
it (including flushPendingLabels (7500646),
MCAssembler iteration improvement, and recent MCFragment refactoring).


Patch is 153.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148781.diff

45 Files Affected:

  • (modified) llvm/include/llvm/MC/MCAssembler.h (-23)
  • (modified) llvm/include/llvm/MC/MCELFStreamer.h (-5)
  • (modified) llvm/include/llvm/MC/MCObjectStreamer.h (-3)
  • (modified) llvm/include/llvm/MC/MCSection.h (-45)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (-13)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (-21)
  • (modified) llvm/lib/MC/MCAssembler.cpp (-128)
  • (modified) llvm/lib/MC/MCELFStreamer.cpp (+1-113)
  • (modified) llvm/lib/MC/MCFragment.cpp (+2-6)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+2-28)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (-68)
  • (modified) llvm/lib/MC/MCSection.cpp (+2-22)
  • (modified) llvm/lib/MC/MCStreamer.cpp (-3)
  • (modified) llvm/lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp (-9)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (-4)
  • (removed) llvm/test/MC/ARM/AlignedBundling/group-bundle-arm.s (-48)
  • (removed) llvm/test/MC/ARM/AlignedBundling/illegal-subtarget-change.s (-16)
  • (removed) llvm/test/MC/ARM/AlignedBundling/lit.local.cfg (-2)
  • (removed) llvm/test/MC/ARM/AlignedBundling/pad-align-to-bundle-end.s (-41)
  • (removed) llvm/test/MC/ARM/AlignedBundling/subtarget-change.s (-33)
  • (removed) llvm/test/MC/X86/AlignedBundling/align-mode-argument-error.s (-8)
  • (removed) llvm/test/MC/X86/AlignedBundling/asm-printing-bundle-directives.s (-22)
  • (removed) llvm/test/MC/X86/AlignedBundling/autogen-inst-offset-align-to-end.s (-2899)
  • (removed) llvm/test/MC/X86/AlignedBundling/autogen-inst-offset-padding.s (-2674)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s (-18)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-lock-option-error.s (-11)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-subtarget-change-error.s (-16)
  • (removed) llvm/test/MC/X86/AlignedBundling/different-sections.s (-27)
  • (removed) llvm/test/MC/X86/AlignedBundling/labeloffset.s (-85)
  • (removed) llvm/test/MC/X86/AlignedBundling/lit.local.cfg (-2)
  • (removed) llvm/test/MC/X86/AlignedBundling/lock-without-bundle-mode-error.s (-10)
  • (removed) llvm/test/MC/X86/AlignedBundling/long-nop-pad.s (-31)
  • (removed) llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s (-19)
  • (removed) llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s (-26)
  • (removed) llvm/test/MC/X86/AlignedBundling/nesting.s (-73)
  • (removed) llvm/test/MC/X86/AlignedBundling/pad-align-to-bundle-end.s (-35)
  • (removed) llvm/test/MC/X86/AlignedBundling/pad-bundle-groups.s (-49)
  • (removed) llvm/test/MC/X86/AlignedBundling/relax-at-bundle-end.s (-18)
  • (removed) llvm/test/MC/X86/AlignedBundling/relax-in-bundle-group.s (-44)
  • (removed) llvm/test/MC/X86/AlignedBundling/rodata-section.s (-30)
  • (removed) llvm/test/MC/X86/AlignedBundling/section-alignment.s (-23)
  • (removed) llvm/test/MC/X86/AlignedBundling/single-inst-bundling.s (-52)
  • (removed) llvm/test/MC/X86/AlignedBundling/switch-section-locked-error.s (-16)
  • (removed) llvm/test/MC/X86/AlignedBundling/unlock-without-lock-error.s (-11)
  • (removed) llvm/test/MC/X86/align-branch-bundle.s (-21)
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index 1015992cedf29..6b0d1b202e8c2 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -85,11 +85,6 @@ class MCAssembler {
   // refactoring too.
   mutable SmallPtrSet<const MCSymbol *, 32> ThumbFuncs;
 
-  /// The bundle alignment size currently set in the assembler.
-  ///
-  /// By default it's 0, which means bundling is disabled.
-  unsigned BundleAlignSize = 0;
-
   /// Evaluate a fixup to a relocatable expression and the value which should be
   /// placed into the fixup.
   ///
@@ -143,8 +138,6 @@ class MCAssembler {
   /// Compute the effective fragment size.
   LLVM_ABI uint64_t computeFragmentSize(const MCFragment &F) const;
 
-  LLVM_ABI void layoutBundle(MCFragment *Prev, MCFragment *F) const;
-
   // Get the offset of the given fragment inside its containing section.
   uint64_t getFragmentOffset(const MCFragment &F) const { return F.Offset; }
 
@@ -203,16 +196,6 @@ class MCAssembler {
   bool getRelaxAll() const { return RelaxAll; }
   void setRelaxAll(bool Value) { RelaxAll = Value; }
 
-  bool isBundlingEnabled() const { return BundleAlignSize != 0; }
-
-  unsigned getBundleAlignSize() const { return BundleAlignSize; }
-
-  void setBundleAlignSize(unsigned Size) {
-    assert((Size == 0 || !(Size & (Size - 1))) &&
-           "Expect a power-of-two bundle align size");
-    BundleAlignSize = Size;
-  }
-
   const_iterator begin() const { return Sections.begin(); }
   const_iterator end() const { return Sections.end(); }
 
@@ -226,12 +209,6 @@ class MCAssembler {
   LLVM_ABI bool registerSection(MCSection &Section);
   LLVM_ABI bool registerSymbol(const MCSymbol &Symbol);
 
-  /// Write the necessary bundle padding to \p OS.
-  /// Expects a fragment \p F containing instructions and its size \p FSize.
-  LLVM_ABI void writeFragmentPadding(raw_ostream &OS,
-                                     const MCEncodedFragment &F,
-                                     uint64_t FSize) const;
-
   LLVM_ABI void reportError(SMLoc L, const Twine &Msg) const;
   // Record pending errors during layout iteration, as they may go away once the
   // layout is finalized.
diff --git a/llvm/include/llvm/MC/MCELFStreamer.h b/llvm/include/llvm/MC/MCELFStreamer.h
index fa0b5cbde71d9..ccf2f868dec4d 100644
--- a/llvm/include/llvm/MC/MCELFStreamer.h
+++ b/llvm/include/llvm/MC/MCELFStreamer.h
@@ -79,10 +79,6 @@ class MCELFStreamer : public MCObjectStreamer {
   // target-specific code.
   void finishImpl() final;
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
-
   /// ELF object attributes section emission support
   struct AttributeItem {
     // This structure holds all attributes, accounting for their string /
@@ -151,7 +147,6 @@ class MCELFStreamer : public MCObjectStreamer {
   }
 
 private:
-  bool isBundleLocked() const;
   void emitInstToData(const MCInst &Inst, const MCSubtargetInfo &) override;
 
   void finalizeCGProfileEntry(const MCSymbolRefExpr *&S, uint64_t Offset);
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 2b42507d66920..ae08455976dbf 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -127,9 +127,6 @@ class MCObjectStreamer : public MCStreamer {
   /// can change its size during relaxation.
   void emitInstToFragment(const MCInst &Inst, const MCSubtargetInfo &);
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
   void emitBytes(StringRef Data) override;
   void emitValueToAlignment(Align Alignment, int64_t Fill = 0,
                             uint8_t FillLen = 1,
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 64b13972bfca1..01ce3842cd368 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -61,13 +61,6 @@ class LLVM_ABI MCSection {
     SV_DXContainer,
   };
 
-  /// Express the state of bundle locked groups while emitting code.
-  enum BundleLockStateType {
-    NotBundleLocked,
-    BundleLocked,
-    BundleLockedAlignToEnd
-  };
-
   struct iterator {
     MCFragment *F = nullptr;
     iterator() = default;
@@ -94,16 +87,6 @@ class LLVM_ABI MCSection {
   /// The section index in the assemblers section list.
   unsigned Ordinal = 0;
 
-  /// Keeping track of bundle-locked state.
-  BundleLockStateType BundleLockState = NotBundleLocked;
-
-  /// Current nesting depth of bundle_lock directives.
-  unsigned BundleLockNestingDepth = 0;
-
-  /// We've seen a bundle_lock directive but not its first instruction
-  /// yet.
-  bool BundleGroupBeforeFirstInst : 1;
-
   /// Whether this section has had instructions emitted into it.
   bool HasInstructions : 1;
 
@@ -169,17 +152,6 @@ class LLVM_ABI MCSection {
   unsigned getOrdinal() const { return Ordinal; }
   void setOrdinal(unsigned Value) { Ordinal = Value; }
 
-  BundleLockStateType getBundleLockState() const { return BundleLockState; }
-  void setBundleLockState(BundleLockStateType NewState);
-  bool isBundleLocked() const { return BundleLockState != NotBundleLocked; }
-
-  bool isBundleGroupBeforeFirstInst() const {
-    return BundleGroupBeforeFirstInst;
-  }
-  void setBundleGroupBeforeFirstInst(bool IsFirst) {
-    BundleGroupBeforeFirstInst = IsFirst;
-  }
-
   bool hasInstructions() const { return HasInstructions; }
   void setHasInstructions(bool Value) { HasInstructions = Value; }
 
@@ -259,7 +231,6 @@ class MCFragment {
   ///
   /// MCEncodedFragment
   bool HasInstructions : 1;
-  bool AlignToBundleEnd : 1;
   /// MCDataFragment
   bool LinkerRelaxable : 1;
   /// MCRelaxableFragment: x86-specific
@@ -297,7 +268,6 @@ class MCFragment {
 /// Interface implemented by fragments that contain encoded instructions and/or
 /// data.
 class MCEncodedFragment : public MCFragment {
-  uint8_t BundlePadding = 0;
   uint32_t ContentStart = 0;
   uint32_t ContentEnd = 0;
   uint32_t FixupStart = 0;
@@ -329,21 +299,6 @@ class MCEncodedFragment : public MCFragment {
     }
   }
 
-  /// Should this fragment be placed at the end of an aligned bundle?
-  bool alignToBundleEnd() const { return AlignToBundleEnd; }
-  void setAlignToBundleEnd(bool V) { AlignToBundleEnd = V; }
-
-  /// Get the padding size that must be inserted before this fragment.
-  /// Used for bundling. By default, no padding is inserted.
-  /// Note that padding size is restricted to 8 bits. This is an optimization
-  /// to reduce the amount of space used for each fragment. In practice, larger
-  /// padding should never be required.
-  uint8_t getBundlePadding() const { return BundlePadding; }
-
-  /// Set the padding size for this fragment. By default it's a no-op,
-  /// and only some fragments have a meaningful implementation.
-  void setBundlePadding(uint8_t N) { BundlePadding = N; }
-
   /// Retrieve the MCSubTargetInfo in effect when the instruction was encoded.
   /// Guaranteed to be non-null if hasInstructions() == true
   const MCSubtargetInfo *getSubtargetInfo() const { return STI; }
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 4e052ecfe0e86..1f7c8b57540a7 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -1068,19 +1068,6 @@ class LLVM_ABI MCStreamer {
                                const MCPseudoProbeInlineStack &InlineStack,
                                MCSymbol *FnSym);
 
-  /// Set the bundle alignment mode from now on in the section.
-  /// The value 1 means turn the bundle alignment off.
-  virtual void emitBundleAlignMode(Align Alignment);
-
-  /// The following instructions are a bundle-locked group.
-  ///
-  /// \param AlignToEnd - If true, the bundle-locked group will be aligned to
-  ///                     the end of a bundle.
-  virtual void emitBundleLock(bool AlignToEnd);
-
-  /// Ends a bundle-locked group.
-  virtual void emitBundleUnlock();
-
   /// If this file is backed by a assembly streamer, this dumps the
   /// specified string in the output .s file.  This capability is indicated by
   /// the hasRawTextSupport() predicate.  By default this aborts.
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 28b6f28a2e55b..3a330dbfec342 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -407,10 +407,6 @@ class MCAsmStreamer final : public MCStreamer {
                        const MCPseudoProbeInlineStack &InlineStack,
                        MCSymbol *FnSym) override;
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
-
   std::optional<std::pair<bool, std::string>>
   emitRelocDirective(const MCExpr &Offset, StringRef Name, const MCExpr *Expr,
                      SMLoc Loc, const MCSubtargetInfo &STI) override;
@@ -2472,23 +2468,6 @@ void MCAsmStreamer::emitPseudoProbe(uint64_t Guid, uint64_t Index,
   EmitEOL();
 }
 
-void MCAsmStreamer::emitBundleAlignMode(Align Alignment) {
-  OS << "\t.bundle_align_mode " << Log2(Alignment);
-  EmitEOL();
-}
-
-void MCAsmStreamer::emitBundleLock(bool AlignToEnd) {
-  OS << "\t.bundle_lock";
-  if (AlignToEnd)
-    OS << " align_to_end";
-  EmitEOL();
-}
-
-void MCAsmStreamer::emitBundleUnlock() {
-  OS << "\t.bundle_unlock";
-  EmitEOL();
-}
-
 std::optional<std::pair<bool, std::string>>
 MCAsmStreamer::emitRelocDirective(const MCExpr &Offset, StringRef Name,
                                   const MCExpr *Expr, SMLoc,
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index fd8a8c3a79c9f..7f01e272a323e 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -94,7 +94,6 @@ void MCAssembler::reset() {
   Sections.clear();
   Symbols.clear();
   ThumbFuncs.clear();
-  BundleAlignSize = 0;
 
   // reset objects owned by us
   if (getBackendPtr())
@@ -282,87 +281,6 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
   llvm_unreachable("invalid fragment kind");
 }
 
-// Compute the amount of padding required before the fragment \p F to
-// obey bundling restrictions, where \p FOffset is the fragment's offset in
-// its section and \p FSize is the fragment's size.
-static uint64_t computeBundlePadding(unsigned BundleSize,
-                                     const MCEncodedFragment *F,
-                                     uint64_t FOffset, uint64_t FSize) {
-  uint64_t OffsetInBundle = FOffset & (BundleSize - 1);
-  uint64_t EndOfFragment = OffsetInBundle + FSize;
-
-  // There are two kinds of bundling restrictions:
-  //
-  // 1) For alignToBundleEnd(), add padding to ensure that the fragment will
-  //    *end* on a bundle boundary.
-  // 2) Otherwise, check if the fragment would cross a bundle boundary. If it
-  //    would, add padding until the end of the bundle so that the fragment
-  //    will start in a new one.
-  if (F->alignToBundleEnd()) {
-    // Three possibilities here:
-    //
-    // A) The fragment just happens to end at a bundle boundary, so we're good.
-    // B) The fragment ends before the current bundle boundary: pad it just
-    //    enough to reach the boundary.
-    // C) The fragment ends after the current bundle boundary: pad it until it
-    //    reaches the end of the next bundle boundary.
-    //
-    // Note: this code could be made shorter with some modulo trickery, but it's
-    // intentionally kept in its more explicit form for simplicity.
-    if (EndOfFragment == BundleSize)
-      return 0;
-    else if (EndOfFragment < BundleSize)
-      return BundleSize - EndOfFragment;
-    else { // EndOfFragment > BundleSize
-      return 2 * BundleSize - EndOfFragment;
-    }
-  } else if (OffsetInBundle > 0 && EndOfFragment > BundleSize)
-    return BundleSize - OffsetInBundle;
-  else
-    return 0;
-}
-
-void MCAssembler::layoutBundle(MCFragment *Prev, MCFragment *F) const {
-  // If bundling is enabled and this fragment has instructions in it, it has to
-  // obey the bundling restrictions. With padding, we'll have:
-  //
-  //
-  //        BundlePadding
-  //             |||
-  // -------------------------------------
-  //   Prev  |##########|       F        |
-  // -------------------------------------
-  //                    ^
-  //                    |
-  //                    F->Offset
-  //
-  // The fragment's offset will point to after the padding, and its computed
-  // size won't include the padding.
-  //
-  // ".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
-  // than the bundle size), but for now we don't.
-  //
-  assert(isa<MCEncodedFragment>(F) &&
-         "Only MCEncodedFragment implementations have instructions");
-  MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
-  uint64_t FSize = computeFragmentSize(*EF);
-
-  if (FSize > getBundleAlignSize())
-    report_fatal_error("Fragment can't be larger than a bundle size");
-
-  uint64_t RequiredBundlePadding =
-      computeBundlePadding(getBundleAlignSize(), EF, EF->Offset, FSize);
-  if (RequiredBundlePadding > UINT8_MAX)
-    report_fatal_error("Padding cannot exceed 255 bytes");
-  EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
-  EF->Offset += RequiredBundlePadding;
-  if (auto *DF = dyn_cast_or_null<MCDataFragment>(Prev))
-    if (DF->getContents().empty())
-      DF->Offset = EF->Offset;
-}
-
 // Simple getSymbolOffset helper for the non-variable case.
 static bool getLabelOffset(const MCAssembler &Asm, const MCSymbol &S,
                            bool ReportError, uint64_t &Val) {
@@ -480,41 +398,6 @@ bool MCAssembler::registerSymbol(const MCSymbol &Symbol) {
   return Changed;
 }
 
-void MCAssembler::writeFragmentPadding(raw_ostream &OS,
-                                       const MCEncodedFragment &EF,
-                                       uint64_t FSize) const {
-  assert(getBackendPtr() && "Expected assembler backend");
-  // Should NOP padding be written out before this fragment?
-  unsigned BundlePadding = EF.getBundlePadding();
-  if (BundlePadding > 0) {
-    assert(isBundlingEnabled() &&
-           "Writing bundle padding with disabled bundling");
-    assert(EF.hasInstructions() &&
-           "Writing bundle padding for a fragment without instructions");
-
-    unsigned TotalLength = BundlePadding + static_cast<unsigned>(FSize);
-    const MCSubtargetInfo *STI = EF.getSubtargetInfo();
-    if (EF.alignToBundleEnd() && TotalLength > getBundleAlignSize()) {
-      // If the padding itself crosses a bundle boundary, it must be emitted
-      // in 2 pieces, since even nop instructions must not cross boundaries.
-      //             v--------------v   <- BundleAlignSize
-      //        v---------v             <- BundlePadding
-      // ----------------------------
-      // | Prev |####|####|    F    |
-      // ----------------------------
-      //        ^-------------------^   <- TotalLength
-      unsigned DistanceToBoundary = TotalLength - getBundleAlignSize();
-      if (!getBackend().writeNopData(OS, DistanceToBoundary, STI))
-        report_fatal_error("unable to write NOP sequence of " +
-                           Twine(DistanceToBoundary) + " bytes");
-      BundlePadding -= DistanceToBoundary;
-    }
-    if (!getBackend().writeNopData(OS, BundlePadding, STI))
-      report_fatal_error("unable to write NOP sequence of " +
-                         Twine(BundlePadding) + " bytes");
-  }
-}
-
 /// Write the fragment \p F to the output file.
 static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
                           const MCFragment &F) {
@@ -523,9 +406,6 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
 
   llvm::endianness Endian = Asm.getBackend().Endian;
 
-  if (const MCEncodedFragment *EF = dyn_cast<MCEncodedFragment>(&F))
-    Asm.writeFragmentPadding(OS, *EF, FragmentSize);
-
   // This variable (and its dummy usage) is to participate in the assert at
   // the end of the function.
   uint64_t Start = OS.tell();
@@ -1101,17 +981,9 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
 }
 
 void MCAssembler::layoutSection(MCSection &Sec) {
-  MCFragment *Prev = nullptr;
   uint64_t Offset = 0;
   for (MCFragment &F : Sec) {
     F.Offset = Offset;
-    if (LLVM_UNLIKELY(isBundlingEnabled())) {
-      if (F.hasInstructions()) {
-        layoutBundle(Prev, &F);
-        Offset = F.Offset;
-      }
-      Prev = &F;
-    }
     Offset += computeFragmentSize(F);
   }
 }
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 4bc3f4642ea02..3e9c51a6b83d1 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -48,10 +48,6 @@ ELFObjectWriter &MCELFStreamer::getWriter() {
   return static_cast<ELFObjectWriter &>(getAssembler().getWriter());
 }
 
-bool MCELFStreamer::isBundleLocked() const {
-  return getCurrentSectionOnly()->isBundleLocked();
-}
-
 void MCELFStreamer::initSections(bool NoExecStack, const MCSubtargetInfo &STI) {
   MCContext &Ctx = getContext();
   switchSection(Ctx.getObjectFileInfo()->getTextSection());
@@ -83,23 +79,8 @@ void MCELFStreamer::emitLabelAtPos(MCSymbol *S, SMLoc Loc, MCDataFragment &F,
     Symbol->setType(ELF::STT_TLS);
 }
 
-// If bundle alignment is used and there are any instructions in the section, it
-// needs to be aligned to at least the bundle size.
-static void setSectionAlignmentForBundling(const MCAssembler &Assembler,
-                                           MCSection *Section) {
-  if (Assembler.isBundlingEnabled() && Section->hasInstructions())
-    Section->ensureMinAlignment(Align(Assembler.getBundleAlignSize()));
-}
-
 void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   MCAssembler &Asm = getAssembler();
-  if (auto *F = getCurrentFragment()) {
-    if (isBundleLocked())
-      report_fatal_error("Unterminated .bundle_lock when changing a section");
-
-    // Ensure the previous section gets aligned if necessary.
-    setSectionAlignmentForBundling(Asm, F->getParent());
-  }
   auto *SectionELF = static_cast<const MCSectionELF *>(Section);
   const MCSymbol *Grp = SectionELF->getGroup();
   if (Grp)
@@ -315,16 +296,12 @@ void MCELFStreamer::emitLocalCommonSymbol(MCSymbol *S, uint64_t Size,
 
 void MCELFStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
                                   SMLoc Loc) {
-  if (isBundleLocked())
-    report_fatal_error("Emitting values inside a locked bundle is forbidden");
   MCObjectStreamer::emitValueImpl(Value, Size, Loc);
 }
 
 void MCELFStreamer::emitValueToAlignment(Align Alignment, int64_t Value,
                                          uint8_t ValueSize,
                                          unsigned MaxBytesToEmit) {
-  if (isBundleLocked())
-    report_fatal_error("Emitting values inside a locked bundle is forbidden");
   MCObjectStreamer::emitValueToAlignment(Alignment, Value, ValueSize,
                                          MaxBytesToEmit);
 }
@@ -391,59 +368,10 @@ void MCELFStreamer::finalizeCGProfile() {
   popSection();
 }
 
-// A fragment can only have one Subtarget, and when bundling is enabled we
-// sometimes need to use the same fragment. We give an error if there
-// are conflicting Subtargets.
-static void CheckBundleSubtargets(const MCSubtargetInfo *OldSTI,
-                                  const MCSubtargetInfo *NewSTI) {
-  if (OldSTI && NewSTI && OldSTI != NewSTI)
-    report_fatal_error("A Bundle can only have one Subtarget.");
-}
-
 void MCELFStreamer::emitInstToData(const MCInst &Inst,
                                    const MCSubtargetInfo &STI) {
   MCAssembler &Assembler = getAssembler();
-
-  // There are several possibilities here:
-  //
-  // If bundling is disabled, append the encoded instruction to the current data
-  // fragment (or create a new such fragment if the current fragment is not a
-  // data fragment, or the Subtarget has changed).
-  //
-  // If bundling is enabled:
-  // - If we're not in a bundle-locked group, emit the instruction into a
-  //   fragment of its own.
-  // - If we're in a bundle-locked group, app...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-backend-arm

Author: Fangrui Song (MaskRay)

Changes

The being-removed PNaCl has a Software Fault Isolation mechanism, which
requires that certain instructions and groups of instructions do not
cross a bundle boundary. When .bundle_align_mode is in effect, each
instruction is placed in its own fragment, allowing flexible NOP
padding.

This feature has significantly complicated our refactoring of MCStreamer
and MCFragment, leading to considerable effort spent untangling
it (including flushPendingLabels (7500646),
MCAssembler iteration improvement, and recent MCFragment refactoring).


Patch is 153.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148781.diff

45 Files Affected:

  • (modified) llvm/include/llvm/MC/MCAssembler.h (-23)
  • (modified) llvm/include/llvm/MC/MCELFStreamer.h (-5)
  • (modified) llvm/include/llvm/MC/MCObjectStreamer.h (-3)
  • (modified) llvm/include/llvm/MC/MCSection.h (-45)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (-13)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (-21)
  • (modified) llvm/lib/MC/MCAssembler.cpp (-128)
  • (modified) llvm/lib/MC/MCELFStreamer.cpp (+1-113)
  • (modified) llvm/lib/MC/MCFragment.cpp (+2-6)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+2-28)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (-68)
  • (modified) llvm/lib/MC/MCSection.cpp (+2-22)
  • (modified) llvm/lib/MC/MCStreamer.cpp (-3)
  • (modified) llvm/lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp (-9)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (-4)
  • (removed) llvm/test/MC/ARM/AlignedBundling/group-bundle-arm.s (-48)
  • (removed) llvm/test/MC/ARM/AlignedBundling/illegal-subtarget-change.s (-16)
  • (removed) llvm/test/MC/ARM/AlignedBundling/lit.local.cfg (-2)
  • (removed) llvm/test/MC/ARM/AlignedBundling/pad-align-to-bundle-end.s (-41)
  • (removed) llvm/test/MC/ARM/AlignedBundling/subtarget-change.s (-33)
  • (removed) llvm/test/MC/X86/AlignedBundling/align-mode-argument-error.s (-8)
  • (removed) llvm/test/MC/X86/AlignedBundling/asm-printing-bundle-directives.s (-22)
  • (removed) llvm/test/MC/X86/AlignedBundling/autogen-inst-offset-align-to-end.s (-2899)
  • (removed) llvm/test/MC/X86/AlignedBundling/autogen-inst-offset-padding.s (-2674)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-group-too-large-error.s (-18)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-lock-option-error.s (-11)
  • (removed) llvm/test/MC/X86/AlignedBundling/bundle-subtarget-change-error.s (-16)
  • (removed) llvm/test/MC/X86/AlignedBundling/different-sections.s (-27)
  • (removed) llvm/test/MC/X86/AlignedBundling/labeloffset.s (-85)
  • (removed) llvm/test/MC/X86/AlignedBundling/lit.local.cfg (-2)
  • (removed) llvm/test/MC/X86/AlignedBundling/lock-without-bundle-mode-error.s (-10)
  • (removed) llvm/test/MC/X86/AlignedBundling/long-nop-pad.s (-31)
  • (removed) llvm/test/MC/X86/AlignedBundling/misaligned-bundle-group.s (-19)
  • (removed) llvm/test/MC/X86/AlignedBundling/misaligned-bundle.s (-26)
  • (removed) llvm/test/MC/X86/AlignedBundling/nesting.s (-73)
  • (removed) llvm/test/MC/X86/AlignedBundling/pad-align-to-bundle-end.s (-35)
  • (removed) llvm/test/MC/X86/AlignedBundling/pad-bundle-groups.s (-49)
  • (removed) llvm/test/MC/X86/AlignedBundling/relax-at-bundle-end.s (-18)
  • (removed) llvm/test/MC/X86/AlignedBundling/relax-in-bundle-group.s (-44)
  • (removed) llvm/test/MC/X86/AlignedBundling/rodata-section.s (-30)
  • (removed) llvm/test/MC/X86/AlignedBundling/section-alignment.s (-23)
  • (removed) llvm/test/MC/X86/AlignedBundling/single-inst-bundling.s (-52)
  • (removed) llvm/test/MC/X86/AlignedBundling/switch-section-locked-error.s (-16)
  • (removed) llvm/test/MC/X86/AlignedBundling/unlock-without-lock-error.s (-11)
  • (removed) llvm/test/MC/X86/align-branch-bundle.s (-21)
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index 1015992cedf29..6b0d1b202e8c2 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -85,11 +85,6 @@ class MCAssembler {
   // refactoring too.
   mutable SmallPtrSet<const MCSymbol *, 32> ThumbFuncs;
 
-  /// The bundle alignment size currently set in the assembler.
-  ///
-  /// By default it's 0, which means bundling is disabled.
-  unsigned BundleAlignSize = 0;
-
   /// Evaluate a fixup to a relocatable expression and the value which should be
   /// placed into the fixup.
   ///
@@ -143,8 +138,6 @@ class MCAssembler {
   /// Compute the effective fragment size.
   LLVM_ABI uint64_t computeFragmentSize(const MCFragment &F) const;
 
-  LLVM_ABI void layoutBundle(MCFragment *Prev, MCFragment *F) const;
-
   // Get the offset of the given fragment inside its containing section.
   uint64_t getFragmentOffset(const MCFragment &F) const { return F.Offset; }
 
@@ -203,16 +196,6 @@ class MCAssembler {
   bool getRelaxAll() const { return RelaxAll; }
   void setRelaxAll(bool Value) { RelaxAll = Value; }
 
-  bool isBundlingEnabled() const { return BundleAlignSize != 0; }
-
-  unsigned getBundleAlignSize() const { return BundleAlignSize; }
-
-  void setBundleAlignSize(unsigned Size) {
-    assert((Size == 0 || !(Size & (Size - 1))) &&
-           "Expect a power-of-two bundle align size");
-    BundleAlignSize = Size;
-  }
-
   const_iterator begin() const { return Sections.begin(); }
   const_iterator end() const { return Sections.end(); }
 
@@ -226,12 +209,6 @@ class MCAssembler {
   LLVM_ABI bool registerSection(MCSection &Section);
   LLVM_ABI bool registerSymbol(const MCSymbol &Symbol);
 
-  /// Write the necessary bundle padding to \p OS.
-  /// Expects a fragment \p F containing instructions and its size \p FSize.
-  LLVM_ABI void writeFragmentPadding(raw_ostream &OS,
-                                     const MCEncodedFragment &F,
-                                     uint64_t FSize) const;
-
   LLVM_ABI void reportError(SMLoc L, const Twine &Msg) const;
   // Record pending errors during layout iteration, as they may go away once the
   // layout is finalized.
diff --git a/llvm/include/llvm/MC/MCELFStreamer.h b/llvm/include/llvm/MC/MCELFStreamer.h
index fa0b5cbde71d9..ccf2f868dec4d 100644
--- a/llvm/include/llvm/MC/MCELFStreamer.h
+++ b/llvm/include/llvm/MC/MCELFStreamer.h
@@ -79,10 +79,6 @@ class MCELFStreamer : public MCObjectStreamer {
   // target-specific code.
   void finishImpl() final;
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
-
   /// ELF object attributes section emission support
   struct AttributeItem {
     // This structure holds all attributes, accounting for their string /
@@ -151,7 +147,6 @@ class MCELFStreamer : public MCObjectStreamer {
   }
 
 private:
-  bool isBundleLocked() const;
   void emitInstToData(const MCInst &Inst, const MCSubtargetInfo &) override;
 
   void finalizeCGProfileEntry(const MCSymbolRefExpr *&S, uint64_t Offset);
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 2b42507d66920..ae08455976dbf 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -127,9 +127,6 @@ class MCObjectStreamer : public MCStreamer {
   /// can change its size during relaxation.
   void emitInstToFragment(const MCInst &Inst, const MCSubtargetInfo &);
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
   void emitBytes(StringRef Data) override;
   void emitValueToAlignment(Align Alignment, int64_t Fill = 0,
                             uint8_t FillLen = 1,
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 64b13972bfca1..01ce3842cd368 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -61,13 +61,6 @@ class LLVM_ABI MCSection {
     SV_DXContainer,
   };
 
-  /// Express the state of bundle locked groups while emitting code.
-  enum BundleLockStateType {
-    NotBundleLocked,
-    BundleLocked,
-    BundleLockedAlignToEnd
-  };
-
   struct iterator {
     MCFragment *F = nullptr;
     iterator() = default;
@@ -94,16 +87,6 @@ class LLVM_ABI MCSection {
   /// The section index in the assemblers section list.
   unsigned Ordinal = 0;
 
-  /// Keeping track of bundle-locked state.
-  BundleLockStateType BundleLockState = NotBundleLocked;
-
-  /// Current nesting depth of bundle_lock directives.
-  unsigned BundleLockNestingDepth = 0;
-
-  /// We've seen a bundle_lock directive but not its first instruction
-  /// yet.
-  bool BundleGroupBeforeFirstInst : 1;
-
   /// Whether this section has had instructions emitted into it.
   bool HasInstructions : 1;
 
@@ -169,17 +152,6 @@ class LLVM_ABI MCSection {
   unsigned getOrdinal() const { return Ordinal; }
   void setOrdinal(unsigned Value) { Ordinal = Value; }
 
-  BundleLockStateType getBundleLockState() const { return BundleLockState; }
-  void setBundleLockState(BundleLockStateType NewState);
-  bool isBundleLocked() const { return BundleLockState != NotBundleLocked; }
-
-  bool isBundleGroupBeforeFirstInst() const {
-    return BundleGroupBeforeFirstInst;
-  }
-  void setBundleGroupBeforeFirstInst(bool IsFirst) {
-    BundleGroupBeforeFirstInst = IsFirst;
-  }
-
   bool hasInstructions() const { return HasInstructions; }
   void setHasInstructions(bool Value) { HasInstructions = Value; }
 
@@ -259,7 +231,6 @@ class MCFragment {
   ///
   /// MCEncodedFragment
   bool HasInstructions : 1;
-  bool AlignToBundleEnd : 1;
   /// MCDataFragment
   bool LinkerRelaxable : 1;
   /// MCRelaxableFragment: x86-specific
@@ -297,7 +268,6 @@ class MCFragment {
 /// Interface implemented by fragments that contain encoded instructions and/or
 /// data.
 class MCEncodedFragment : public MCFragment {
-  uint8_t BundlePadding = 0;
   uint32_t ContentStart = 0;
   uint32_t ContentEnd = 0;
   uint32_t FixupStart = 0;
@@ -329,21 +299,6 @@ class MCEncodedFragment : public MCFragment {
     }
   }
 
-  /// Should this fragment be placed at the end of an aligned bundle?
-  bool alignToBundleEnd() const { return AlignToBundleEnd; }
-  void setAlignToBundleEnd(bool V) { AlignToBundleEnd = V; }
-
-  /// Get the padding size that must be inserted before this fragment.
-  /// Used for bundling. By default, no padding is inserted.
-  /// Note that padding size is restricted to 8 bits. This is an optimization
-  /// to reduce the amount of space used for each fragment. In practice, larger
-  /// padding should never be required.
-  uint8_t getBundlePadding() const { return BundlePadding; }
-
-  /// Set the padding size for this fragment. By default it's a no-op,
-  /// and only some fragments have a meaningful implementation.
-  void setBundlePadding(uint8_t N) { BundlePadding = N; }
-
   /// Retrieve the MCSubTargetInfo in effect when the instruction was encoded.
   /// Guaranteed to be non-null if hasInstructions() == true
   const MCSubtargetInfo *getSubtargetInfo() const { return STI; }
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 4e052ecfe0e86..1f7c8b57540a7 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -1068,19 +1068,6 @@ class LLVM_ABI MCStreamer {
                                const MCPseudoProbeInlineStack &InlineStack,
                                MCSymbol *FnSym);
 
-  /// Set the bundle alignment mode from now on in the section.
-  /// The value 1 means turn the bundle alignment off.
-  virtual void emitBundleAlignMode(Align Alignment);
-
-  /// The following instructions are a bundle-locked group.
-  ///
-  /// \param AlignToEnd - If true, the bundle-locked group will be aligned to
-  ///                     the end of a bundle.
-  virtual void emitBundleLock(bool AlignToEnd);
-
-  /// Ends a bundle-locked group.
-  virtual void emitBundleUnlock();
-
   /// If this file is backed by a assembly streamer, this dumps the
   /// specified string in the output .s file.  This capability is indicated by
   /// the hasRawTextSupport() predicate.  By default this aborts.
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 28b6f28a2e55b..3a330dbfec342 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -407,10 +407,6 @@ class MCAsmStreamer final : public MCStreamer {
                        const MCPseudoProbeInlineStack &InlineStack,
                        MCSymbol *FnSym) override;
 
-  void emitBundleAlignMode(Align Alignment) override;
-  void emitBundleLock(bool AlignToEnd) override;
-  void emitBundleUnlock() override;
-
   std::optional<std::pair<bool, std::string>>
   emitRelocDirective(const MCExpr &Offset, StringRef Name, const MCExpr *Expr,
                      SMLoc Loc, const MCSubtargetInfo &STI) override;
@@ -2472,23 +2468,6 @@ void MCAsmStreamer::emitPseudoProbe(uint64_t Guid, uint64_t Index,
   EmitEOL();
 }
 
-void MCAsmStreamer::emitBundleAlignMode(Align Alignment) {
-  OS << "\t.bundle_align_mode " << Log2(Alignment);
-  EmitEOL();
-}
-
-void MCAsmStreamer::emitBundleLock(bool AlignToEnd) {
-  OS << "\t.bundle_lock";
-  if (AlignToEnd)
-    OS << " align_to_end";
-  EmitEOL();
-}
-
-void MCAsmStreamer::emitBundleUnlock() {
-  OS << "\t.bundle_unlock";
-  EmitEOL();
-}
-
 std::optional<std::pair<bool, std::string>>
 MCAsmStreamer::emitRelocDirective(const MCExpr &Offset, StringRef Name,
                                   const MCExpr *Expr, SMLoc,
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index fd8a8c3a79c9f..7f01e272a323e 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -94,7 +94,6 @@ void MCAssembler::reset() {
   Sections.clear();
   Symbols.clear();
   ThumbFuncs.clear();
-  BundleAlignSize = 0;
 
   // reset objects owned by us
   if (getBackendPtr())
@@ -282,87 +281,6 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
   llvm_unreachable("invalid fragment kind");
 }
 
-// Compute the amount of padding required before the fragment \p F to
-// obey bundling restrictions, where \p FOffset is the fragment's offset in
-// its section and \p FSize is the fragment's size.
-static uint64_t computeBundlePadding(unsigned BundleSize,
-                                     const MCEncodedFragment *F,
-                                     uint64_t FOffset, uint64_t FSize) {
-  uint64_t OffsetInBundle = FOffset & (BundleSize - 1);
-  uint64_t EndOfFragment = OffsetInBundle + FSize;
-
-  // There are two kinds of bundling restrictions:
-  //
-  // 1) For alignToBundleEnd(), add padding to ensure that the fragment will
-  //    *end* on a bundle boundary.
-  // 2) Otherwise, check if the fragment would cross a bundle boundary. If it
-  //    would, add padding until the end of the bundle so that the fragment
-  //    will start in a new one.
-  if (F->alignToBundleEnd()) {
-    // Three possibilities here:
-    //
-    // A) The fragment just happens to end at a bundle boundary, so we're good.
-    // B) The fragment ends before the current bundle boundary: pad it just
-    //    enough to reach the boundary.
-    // C) The fragment ends after the current bundle boundary: pad it until it
-    //    reaches the end of the next bundle boundary.
-    //
-    // Note: this code could be made shorter with some modulo trickery, but it's
-    // intentionally kept in its more explicit form for simplicity.
-    if (EndOfFragment == BundleSize)
-      return 0;
-    else if (EndOfFragment < BundleSize)
-      return BundleSize - EndOfFragment;
-    else { // EndOfFragment > BundleSize
-      return 2 * BundleSize - EndOfFragment;
-    }
-  } else if (OffsetInBundle > 0 && EndOfFragment > BundleSize)
-    return BundleSize - OffsetInBundle;
-  else
-    return 0;
-}
-
-void MCAssembler::layoutBundle(MCFragment *Prev, MCFragment *F) const {
-  // If bundling is enabled and this fragment has instructions in it, it has to
-  // obey the bundling restrictions. With padding, we'll have:
-  //
-  //
-  //        BundlePadding
-  //             |||
-  // -------------------------------------
-  //   Prev  |##########|       F        |
-  // -------------------------------------
-  //                    ^
-  //                    |
-  //                    F->Offset
-  //
-  // The fragment's offset will point to after the padding, and its computed
-  // size won't include the padding.
-  //
-  // ".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
-  // than the bundle size), but for now we don't.
-  //
-  assert(isa<MCEncodedFragment>(F) &&
-         "Only MCEncodedFragment implementations have instructions");
-  MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
-  uint64_t FSize = computeFragmentSize(*EF);
-
-  if (FSize > getBundleAlignSize())
-    report_fatal_error("Fragment can't be larger than a bundle size");
-
-  uint64_t RequiredBundlePadding =
-      computeBundlePadding(getBundleAlignSize(), EF, EF->Offset, FSize);
-  if (RequiredBundlePadding > UINT8_MAX)
-    report_fatal_error("Padding cannot exceed 255 bytes");
-  EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
-  EF->Offset += RequiredBundlePadding;
-  if (auto *DF = dyn_cast_or_null<MCDataFragment>(Prev))
-    if (DF->getContents().empty())
-      DF->Offset = EF->Offset;
-}
-
 // Simple getSymbolOffset helper for the non-variable case.
 static bool getLabelOffset(const MCAssembler &Asm, const MCSymbol &S,
                            bool ReportError, uint64_t &Val) {
@@ -480,41 +398,6 @@ bool MCAssembler::registerSymbol(const MCSymbol &Symbol) {
   return Changed;
 }
 
-void MCAssembler::writeFragmentPadding(raw_ostream &OS,
-                                       const MCEncodedFragment &EF,
-                                       uint64_t FSize) const {
-  assert(getBackendPtr() && "Expected assembler backend");
-  // Should NOP padding be written out before this fragment?
-  unsigned BundlePadding = EF.getBundlePadding();
-  if (BundlePadding > 0) {
-    assert(isBundlingEnabled() &&
-           "Writing bundle padding with disabled bundling");
-    assert(EF.hasInstructions() &&
-           "Writing bundle padding for a fragment without instructions");
-
-    unsigned TotalLength = BundlePadding + static_cast<unsigned>(FSize);
-    const MCSubtargetInfo *STI = EF.getSubtargetInfo();
-    if (EF.alignToBundleEnd() && TotalLength > getBundleAlignSize()) {
-      // If the padding itself crosses a bundle boundary, it must be emitted
-      // in 2 pieces, since even nop instructions must not cross boundaries.
-      //             v--------------v   <- BundleAlignSize
-      //        v---------v             <- BundlePadding
-      // ----------------------------
-      // | Prev |####|####|    F    |
-      // ----------------------------
-      //        ^-------------------^   <- TotalLength
-      unsigned DistanceToBoundary = TotalLength - getBundleAlignSize();
-      if (!getBackend().writeNopData(OS, DistanceToBoundary, STI))
-        report_fatal_error("unable to write NOP sequence of " +
-                           Twine(DistanceToBoundary) + " bytes");
-      BundlePadding -= DistanceToBoundary;
-    }
-    if (!getBackend().writeNopData(OS, BundlePadding, STI))
-      report_fatal_error("unable to write NOP sequence of " +
-                         Twine(BundlePadding) + " bytes");
-  }
-}
-
 /// Write the fragment \p F to the output file.
 static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
                           const MCFragment &F) {
@@ -523,9 +406,6 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
 
   llvm::endianness Endian = Asm.getBackend().Endian;
 
-  if (const MCEncodedFragment *EF = dyn_cast<MCEncodedFragment>(&F))
-    Asm.writeFragmentPadding(OS, *EF, FragmentSize);
-
   // This variable (and its dummy usage) is to participate in the assert at
   // the end of the function.
   uint64_t Start = OS.tell();
@@ -1101,17 +981,9 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
 }
 
 void MCAssembler::layoutSection(MCSection &Sec) {
-  MCFragment *Prev = nullptr;
   uint64_t Offset = 0;
   for (MCFragment &F : Sec) {
     F.Offset = Offset;
-    if (LLVM_UNLIKELY(isBundlingEnabled())) {
-      if (F.hasInstructions()) {
-        layoutBundle(Prev, &F);
-        Offset = F.Offset;
-      }
-      Prev = &F;
-    }
     Offset += computeFragmentSize(F);
   }
 }
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 4bc3f4642ea02..3e9c51a6b83d1 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -48,10 +48,6 @@ ELFObjectWriter &MCELFStreamer::getWriter() {
   return static_cast<ELFObjectWriter &>(getAssembler().getWriter());
 }
 
-bool MCELFStreamer::isBundleLocked() const {
-  return getCurrentSectionOnly()->isBundleLocked();
-}
-
 void MCELFStreamer::initSections(bool NoExecStack, const MCSubtargetInfo &STI) {
   MCContext &Ctx = getContext();
   switchSection(Ctx.getObjectFileInfo()->getTextSection());
@@ -83,23 +79,8 @@ void MCELFStreamer::emitLabelAtPos(MCSymbol *S, SMLoc Loc, MCDataFragment &F,
     Symbol->setType(ELF::STT_TLS);
 }
 
-// If bundle alignment is used and there are any instructions in the section, it
-// needs to be aligned to at least the bundle size.
-static void setSectionAlignmentForBundling(const MCAssembler &Assembler,
-                                           MCSection *Section) {
-  if (Assembler.isBundlingEnabled() && Section->hasInstructions())
-    Section->ensureMinAlignment(Align(Assembler.getBundleAlignSize()));
-}
-
 void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   MCAssembler &Asm = getAssembler();
-  if (auto *F = getCurrentFragment()) {
-    if (isBundleLocked())
-      report_fatal_error("Unterminated .bundle_lock when changing a section");
-
-    // Ensure the previous section gets aligned if necessary.
-    setSectionAlignmentForBundling(Asm, F->getParent());
-  }
   auto *SectionELF = static_cast<const MCSectionELF *>(Section);
   const MCSymbol *Grp = SectionELF->getGroup();
   if (Grp)
@@ -315,16 +296,12 @@ void MCELFStreamer::emitLocalCommonSymbol(MCSymbol *S, uint64_t Size,
 
 void MCELFStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
                                   SMLoc Loc) {
-  if (isBundleLocked())
-    report_fatal_error("Emitting values inside a locked bundle is forbidden");
   MCObjectStreamer::emitValueImpl(Value, Size, Loc);
 }
 
 void MCELFStreamer::emitValueToAlignment(Align Alignment, int64_t Value,
                                          uint8_t ValueSize,
                                          unsigned MaxBytesToEmit) {
-  if (isBundleLocked())
-    report_fatal_error("Emitting values inside a locked bundle is forbidden");
   MCObjectStreamer::emitValueToAlignment(Alignment, Value, ValueSize,
                                          MaxBytesToEmit);
 }
@@ -391,59 +368,10 @@ void MCELFStreamer::finalizeCGProfile() {
   popSection();
 }
 
-// A fragment can only have one Subtarget, and when bundling is enabled we
-// sometimes need to use the same fragment. We give an error if there
-// are conflicting Subtargets.
-static void CheckBundleSubtargets(const MCSubtargetInfo *OldSTI,
-                                  const MCSubtargetInfo *NewSTI) {
-  if (OldSTI && NewSTI && OldSTI != NewSTI)
-    report_fatal_error("A Bundle can only have one Subtarget.");
-}
-
 void MCELFStreamer::emitInstToData(const MCInst &Inst,
                                    const MCSubtargetInfo &STI) {
   MCAssembler &Assembler = getAssembler();
-
-  // There are several possibilities here:
-  //
-  // If bundling is disabled, append the encoded instruction to the current data
-  // fragment (or create a new such fragment if the current fragment is not a
-  // data fragment, or the Subtarget has changed).
-  //
-  // If bundling is enabled:
-  // - If we're not in a bundle-locked group, emit the instruction into a
-  //   fragment of its own.
-  // - If we're in a bundle-locked group, app...
[truncated]

Copy link
Contributor

@aengelke aengelke left a comment

Choose a reason for hiding this comment

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

Changes look good. Finally.

if (OldSTI && NewSTI && OldSTI != NewSTI)
report_fatal_error("A Bundle can only have one Subtarget.");
}

void MCELFStreamer::emitInstToData(const MCInst &Inst,
Copy link
Contributor

Choose a reason for hiding this comment

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

Integrate linker-relaxable check into MCObjectStreamer, remove this, and make emitInstToData non-virtual. (Also replace MCObjectStreamer's emitInstToData with this one, I think this is more efficient.)

@@ -315,16 +296,12 @@ void MCELFStreamer::emitLocalCommonSymbol(MCSymbol *S, uint64_t Size,

void MCELFStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this.

@@ -315,16 +296,12 @@ void MCELFStreamer::emitLocalCommonSymbol(MCSymbol *S, uint64_t Size,

void MCELFStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
SMLoc Loc) {
if (isBundleLocked())
report_fatal_error("Emitting values inside a locked bundle is forbidden");
MCObjectStreamer::emitValueImpl(Value, Size, Loc);
}

void MCELFStreamer::emitValueToAlignment(Align Alignment, int64_t Value,
Copy link
Contributor

Choose a reason for hiding this comment

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

Dito.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants