Skip to content

MC: Refactor FT_Align fragments when linker relaxation is enabled #149465

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

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 18, 2025

Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

  • Introduces MCAsmBackend::relaxAlign to replace the two hooks.
  • Tracks padding size using VarContentEnd (content is ignored).
  • Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

MaskRay added 2 commits July 18, 2025 00:01
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

  • Introduces MCAsmBackend::relaxAlign to replace the two hooks.
  • Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
  • Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

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

10 Files Affected:

  • (modified) llvm/include/llvm/MC/MCAsmBackend.h (+4-14)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+54-56)
  • (modified) llvm/lib/MC/MCExpr.cpp (+1-4)
  • (modified) llvm/lib/MC/MCFragment.cpp (+6-1)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (-6)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp (+55-68)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h (+1-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+19-49)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+1-7)
  • (modified) llvm/test/MC/RISCV/Relocations/mc-dump.s (+4-2)
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 93259b0ea6d74..e476b76e0ab08 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -103,20 +103,6 @@ class LLVM_ABI MCAsmBackend {
   /// Get information on a fixup kind.
   virtual MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const;
 
-  /// Hook to check if extra nop bytes must be inserted for alignment directive.
-  /// For some targets this may be necessary in order to support linker
-  /// relaxation. The number of bytes to insert are returned in Size.
-  virtual bool shouldInsertExtraNopBytesForCodeAlign(const MCFragment &AF,
-                                                     unsigned &Size) {
-    return false;
-  }
-
-  /// Hook which indicates if the target requires a fixup to be generated when
-  /// handling an align directive in an executable section
-  virtual bool shouldInsertFixupForCodeAlign(MCAssembler &Asm, MCFragment &AF) {
-    return false;
-  }
-
   // Evaluate a fixup, returning std::nullopt to use default handling for
   // `Value` and `IsResolved`. Otherwise, returns `IsResolved` with the
   // expectation that the hook updates `Value`.
@@ -174,6 +160,10 @@ class LLVM_ABI MCAsmBackend {
   }
 
   // Defined by linker relaxation targets.
+
+  // Return false to use default handling. Otherwise, set `Size` to the number
+  // of padding bytes.
+  virtual bool relaxAlign(MCFragment &F, unsigned &Size) { return false; }
   virtual bool relaxDwarfLineAddr(MCFragment &, bool &WasRelaxed) const {
     return false;
   }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index f18ca38a27cad..cc6817fc62a24 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -230,22 +230,24 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
   case MCFragment::FT_Align: {
     unsigned Offset = F.Offset + F.getFixedSize();
     unsigned Size = offsetToAlignment(Offset, F.getAlignment());
-
-    // Insert extra Nops for code alignment if the target define
-    // shouldInsertExtraNopBytesForCodeAlign target hook.
-    if (F.getParent()->useCodeAlign() && F.hasAlignEmitNops() &&
-        getBackend().shouldInsertExtraNopBytesForCodeAlign(F, Size))
-      return F.getFixedSize() + Size;
-
-    // If we are padding with nops, force the padding to be larger than the
-    // minimum nop size.
-    if (Size > 0 && F.hasAlignEmitNops()) {
-      while (Size % getBackend().getMinimumNopSize())
-        Size += F.getAlignment().value();
+    auto &Frag = const_cast<MCFragment &>(F);
+    // In the nops mode, RISC-V style linker relaxation might adjust the size
+    // and add a fixup, even if `Size` is originally 0.
+    bool AlignFixup = false;
+    if (F.hasAlignEmitNops()) {
+      AlignFixup = getBackend().relaxAlign(Frag, Size);
+      // If the backend does not handle the fragment specially, pad with nops,
+      // but ensure that the padding is larger than the minimum nop size.
+      if (!AlignFixup)
+        while (Size % getBackend().getMinimumNopSize())
+          Size += F.getAlignment().value();
     }
-    if (Size > F.getAlignMaxBytesToEmit())
+    if (!AlignFixup && Size > F.getAlignMaxBytesToEmit())
       Size = 0;
-    return F.getFixedSize() + Size;
+    Frag.VarContentEnd = F.VarContentStart + Size;
+    if (Frag.VarContentEnd > Frag.getParent()->ContentStorage.size())
+      Frag.getParent()->ContentStorage.resize(Frag.VarContentEnd);
+    return F.getSize();
   }
 
   case MCFragment::FT_Org: {
@@ -419,7 +421,6 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
   switch (F.getKind()) {
   case MCFragment::FT_Data:
   case MCFragment::FT_Relaxable:
-  case MCFragment::FT_Align:
   case MCFragment::FT_LEB:
   case MCFragment::FT_Dwarf:
   case MCFragment::FT_DwarfFrame:
@@ -433,42 +434,44 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
     const auto &EF = cast<MCFragment>(F);
     OS << StringRef(EF.getContents().data(), EF.getContents().size());
     OS << StringRef(EF.getVarContents().data(), EF.getVarContents().size());
-    if (F.getKind() == MCFragment::FT_Align) {
-      ++stats::EmittedAlignFragments;
-      assert(F.getAlignFillLen() &&
-             "Invalid virtual align in concrete fragment!");
-
-      uint64_t Count = (FragmentSize - F.getFixedSize()) / F.getAlignFillLen();
-      assert((FragmentSize - F.getFixedSize()) % F.getAlignFillLen() == 0 &&
-             "computeFragmentSize computed size is incorrect");
-
-      // See if we are aligning with nops, and if so do that first to try to
-      // fill the Count bytes.  Then if that did not fill any bytes or there are
-      // any bytes left to fill use the Value and ValueSize to fill the rest. If
-      // we are aligning with nops, ask that target to emit the right data.
-      if (F.hasAlignEmitNops()) {
-        if (!Asm.getBackend().writeNopData(OS, Count, F.getSubtargetInfo()))
-          report_fatal_error("unable to write nop sequence of " + Twine(Count) +
-                             " bytes");
-      } else {
-        // Otherwise, write out in multiples of the value size.
-        for (uint64_t i = 0; i != Count; ++i) {
-          switch (F.getAlignFillLen()) {
-          default:
-            llvm_unreachable("Invalid size!");
-          case 1:
-            OS << char(F.getAlignFill());
-            break;
-          case 2:
-            support::endian::write<uint16_t>(OS, F.getAlignFill(), Endian);
-            break;
-          case 4:
-            support::endian::write<uint32_t>(OS, F.getAlignFill(), Endian);
-            break;
-          case 8:
-            support::endian::write<uint64_t>(OS, F.getAlignFill(), Endian);
-            break;
-          }
+  } break;
+
+  case MCFragment::FT_Align: {
+    ++stats::EmittedAlignFragments;
+    OS << StringRef(F.getContents().data(), F.getContents().size());
+    assert(F.getAlignFillLen() &&
+           "Invalid virtual align in concrete fragment!");
+
+    uint64_t Count = (FragmentSize - F.getFixedSize()) / F.getAlignFillLen();
+    assert((FragmentSize - F.getFixedSize()) % F.getAlignFillLen() == 0 &&
+           "computeFragmentSize computed size is incorrect");
+
+    // See if we are aligning with nops, and if so do that first to try to
+    // fill the Count bytes.  Then if that did not fill any bytes or there are
+    // any bytes left to fill use the Value and ValueSize to fill the rest. If
+    // we are aligning with nops, ask that target to emit the right data.
+    if (F.hasAlignEmitNops()) {
+      if (!Asm.getBackend().writeNopData(OS, Count, F.getSubtargetInfo()))
+        report_fatal_error("unable to write nop sequence of " + Twine(Count) +
+                           " bytes");
+    } else {
+      // Otherwise, write out in multiples of the value size.
+      for (uint64_t i = 0; i != Count; ++i) {
+        switch (F.getAlignFillLen()) {
+        default:
+          llvm_unreachable("Invalid size!");
+        case 1:
+          OS << char(F.getAlignFill());
+          break;
+        case 2:
+          support::endian::write<uint16_t>(OS, F.getAlignFill(), Endian);
+          break;
+        case 4:
+          support::endian::write<uint32_t>(OS, F.getAlignFill(), Endian);
+          break;
+        case 8:
+          support::endian::write<uint64_t>(OS, F.getAlignFill(), Endian);
+          break;
         }
       }
     }
@@ -739,11 +742,6 @@ void MCAssembler::layout() {
           evaluateFixup(F, Fixup, Target, FixedValue,
                         /*RecordReloc=*/true, Contents);
         }
-      } else if (F.getKind() == MCFragment::FT_Align) {
-        // For RISC-V linker relaxation, an alignment relocation might be
-        // needed.
-        if (F.hasAlignEmitNops())
-          getBackend().shouldInsertFixupForCodeAlign(*this, F);
       }
     }
   }
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index f0f1bd485258f..dbb2fd16eb2e5 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -370,7 +370,6 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
       }
 
       int64_t Num;
-      unsigned Count;
       if (DF) {
         Displacement += DF->getContents().size();
       } else if (F->getKind() == MCFragment::FT_Relaxable &&
@@ -380,9 +379,7 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
         // data fragment.
         Displacement += F->getSize();
       } else if (F->getKind() == MCFragment::FT_Align && Layout &&
-                 F->hasAlignEmitNops() &&
-                 !Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
-                     *F, Count)) {
+                 F->isLinkerRelaxable()) {
         Displacement += Asm->computeFragmentSize(*F);
       } else if (auto *FF = dyn_cast<MCFillFragment>(F);
                  FF && FF->getNumValues().evaluateAsAbsolute(Num)) {
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index d01660f640c46..bfaeb2be1ec90 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -84,8 +84,13 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
     auto Fixed = getContents();
     auto Var = getVarContents();
     OS << " Size:" << Fixed.size();
-    if (getKind() != MCFragment::FT_Data)
+    if (getKind() != MCFragment::FT_Data) {
       OS << '+' << Var.size();
+      // FT_Align uses getVarContents to track the size, but the content is
+      // garbage and not useful.
+      if (getKind() == MCFragment::FT_Align)
+        Var = {};
+    }
     OS << " [";
     for (unsigned i = 0, e = Fixed.size(); i != e; ++i) {
       if (i) OS << ",";
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 6ac598b4d0d3b..99f8dd93efd9e 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -564,12 +564,6 @@ void MCObjectStreamer::emitCodeAlignment(Align Alignment,
   emitValueToAlignment(Alignment, 0, 1, MaxBytesToEmit);
   auto *F = getCurrentFragment();
   F->setAlignEmitNops(true, STI);
-  // With RISC-V style linker relaxation, mark the section as linker-relaxable
-  // if the alignment is larger than the minimum NOP size.
-  unsigned Size;
-  if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
-                                                                        Size))
-    getCurrentSectionOnly()->setLinkerRelaxable();
 }
 
 void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index 032bfea71140f..b1cf001c4bdca 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -177,74 +177,6 @@ void LoongArchAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
   }
 }
 
-// Linker relaxation may change code size. We have to insert Nops
-// for .align directive when linker relaxation enabled. So then Linker
-// could satisfy alignment by removing Nops.
-// The function returns the total Nops Size we need to insert.
-bool LoongArchAsmBackend::shouldInsertExtraNopBytesForCodeAlign(
-    const MCFragment &AF, unsigned &Size) {
-  // Calculate Nops Size only when linker relaxation enabled.
-  if (!AF.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
-    return false;
-
-  // Ignore alignment if MaxBytesToEmit is less than the minimum Nop size.
-  const unsigned MinNopLen = 4;
-  if (AF.getAlignMaxBytesToEmit() < MinNopLen)
-    return false;
-  Size = AF.getAlignment().value() - MinNopLen;
-  return AF.getAlignment() > MinNopLen;
-}
-
-// We need to insert R_LARCH_ALIGN relocation type to indicate the
-// position of Nops and the total bytes of the Nops have been inserted
-// when linker relaxation enabled.
-// The function inserts fixup_loongarch_align fixup which eventually will
-// transfer to R_LARCH_ALIGN relocation type.
-// The improved R_LARCH_ALIGN requires symbol index. The lowest 8 bits of
-// addend represent alignment and the other bits of addend represent the
-// maximum number of bytes to emit. The maximum number of bytes is zero
-// means ignore the emit limit.
-bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
-                                                        MCFragment &AF) {
-  // Insert the fixup only when linker relaxation enabled.
-  if (!AF.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
-    return false;
-
-  // Calculate total Nops we need to insert. If there are none to insert
-  // then simply return.
-  unsigned InsertedNopBytes;
-  if (!shouldInsertExtraNopBytesForCodeAlign(AF, InsertedNopBytes))
-    return false;
-
-  MCSection *Sec = AF.getParent();
-  MCContext &Ctx = getContext();
-  const MCExpr *Dummy = MCConstantExpr::create(0, Ctx);
-  MCFixup Fixup = MCFixup::create(AF.getFixedSize(), Dummy, ELF::R_LARCH_ALIGN);
-  unsigned MaxBytesToEmit = AF.getAlignMaxBytesToEmit();
-
-  auto createExtendedValue = [&]() {
-    const MCSymbolRefExpr *MCSym = getSecToAlignSym()[Sec];
-    if (MCSym == nullptr) {
-      // Define a marker symbol at the section with an offset of 0.
-      MCSymbol *Sym = Ctx.createNamedTempSymbol("la-relax-align");
-      Sym->setFragment(&*Sec->getBeginSymbol()->getFragment());
-      Asm.registerSymbol(*Sym);
-      MCSym = MCSymbolRefExpr::create(Sym, Ctx);
-      getSecToAlignSym()[Sec] = MCSym;
-    }
-    return MCValue::get(&MCSym->getSymbol(), nullptr,
-                        MaxBytesToEmit << 8 | Log2(AF.getAlignment()));
-  };
-
-  uint64_t FixedValue = 0;
-  MCValue Value = MaxBytesToEmit >= InsertedNopBytes
-                      ? MCValue::get(InsertedNopBytes)
-                      : createExtendedValue();
-  Asm.getWriter().recordRelocation(AF, Fixup, Value, FixedValue);
-
-  return true;
-}
-
 bool LoongArchAsmBackend::shouldForceRelocation(const MCFixup &Fixup,
                                                 const MCValue &Target) {
   switch (Fixup.getKind()) {
@@ -279,6 +211,61 @@ getRelocPairForSize(unsigned Size) {
   }
 }
 
+// Linker relaxation may change code size. We have to insert Nops
+// for .align directive when linker relaxation enabled. So then Linker
+// could satisfy alignment by removing Nops.
+// The function returns the total Nops Size we need to insert.
+bool LoongArchAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
+  // Use default handling unless linker relaxation is enabled and the
+  // MaxBytesToEmit >= the nop size.
+  if (!F.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
+    return false;
+  const unsigned MinNopLen = 4;
+  unsigned MaxBytesToEmit = F.getAlignMaxBytesToEmit();
+  if (MaxBytesToEmit < MinNopLen)
+    return false;
+
+  Size = F.getAlignment().value() - MinNopLen;
+  if (F.getAlignment() <= MinNopLen)
+    return false;
+
+  // We need to insert R_LARCH_ALIGN relocation type to indicate the
+  // position of Nops and the total bytes of the Nops have been inserted
+  // when linker relaxation enabled.
+  // The function inserts fixup_loongarch_align fixup which eventually will
+  // transfer to R_LARCH_ALIGN relocation type.
+  // The improved R_LARCH_ALIGN requires symbol index. The lowest 8 bits of
+  // addend represent alignment and the other bits of addend represent the
+  // maximum number of bytes to emit. The maximum number of bytes is zero
+  // means ignore the emit limit.
+  MCContext &Ctx = getContext();
+  const MCExpr *Expr = nullptr;
+  if (MaxBytesToEmit >= Size) {
+    Expr = MCConstantExpr::create(Size, getContext());
+  } else {
+    MCSection *Sec = F.getParent();
+    const MCSymbolRefExpr *SymRef = getSecToAlignSym()[Sec];
+    if (SymRef == nullptr) {
+      // Define a marker symbol at the section with an offset of 0.
+      MCSymbol *Sym = Ctx.createNamedTempSymbol("la-relax-align");
+      Sym->setFragment(&*Sec->getBeginSymbol()->getFragment());
+      Asm->registerSymbol(*Sym);
+      SymRef = MCSymbolRefExpr::create(Sym, Ctx);
+      getSecToAlignSym()[Sec] = SymRef;
+    }
+    Expr = MCBinaryExpr::createAdd(
+        SymRef,
+        MCConstantExpr::create((MaxBytesToEmit << 8) | Log2(F.getAlignment()),
+                               Ctx),
+        Ctx);
+  }
+  MCFixup Fixup =
+      MCFixup::create(0, Expr, FirstLiteralRelocationKind + ELF::R_LARCH_ALIGN);
+  F.setVarFixups({Fixup});
+  F.getParent()->setLinkerRelaxable();
+  return true;
+}
+
 std::pair<bool, bool> LoongArchAsmBackend::relaxLEB128(MCFragment &F,
                                                        int64_t &Value) const {
   const MCExpr &Expr = F.getLEBValue();
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
index 793e4093b1c9e..3d929fc49f95e 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
@@ -45,19 +45,13 @@ class LoongArchAsmBackend : public MCAsmBackend {
                   MutableArrayRef<char> Data, uint64_t Value,
                   bool IsResolved) override;
 
-  // Return Size with extra Nop Bytes for alignment directive in code section.
-  bool shouldInsertExtraNopBytesForCodeAlign(const MCFragment &AF,
-                                             unsigned &Size) override;
-
-  // Insert target specific fixup type for alignment directive in code section.
-  bool shouldInsertFixupForCodeAlign(MCAssembler &Asm, MCFragment &AF) override;
-
   bool shouldForceRelocation(const MCFixup &Fixup, const MCValue &Target);
 
   std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
 
   MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
 
+  bool relaxAlign(MCFragment &F, unsigned &Size) override;
   bool relaxDwarfLineAddr(MCFragment &F, bool &WasRelaxed) const override;
   bool relaxDwarfCFA(MCFragment &F, bool &WasRelaxed) const override;
   std::pair<bool, bool> relaxLEB128(MCFragment &F,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 6bc313656f7c1..3d124bace92fe 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -302,6 +302,25 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
   Inst = std::move(Res);
 }
 
+bool RISCVAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
+  // Use default handling unless linker relaxation is enabled and the alignment
+  // is larger than the nop size.
+  const MCSubtargetInfo *STI = F.getSubtargetInfo();
+  if (!STI->hasFeature(RISCV::FeatureRelax))
+    return false;
+  unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
+  if (F.getAlignment() <= MinNopLen)
+    return false;
+
+  Size = F.getAlignment().value() - MinNopLen;
+  auto *Expr = MCConstantExpr::create(Size, getContext());
+  MCFixup Fixup =
+      MCFixup::create(0, Expr, FirstLiteralRelocationKind + ELF::R_RISCV_ALIGN);
+  F.setVarFixups({Fixup});
+  F.getParent()->setLinkerRelaxable();
+  return true;
+}
+
 bool RISCVAsmBackend::relaxDwarfLineAddr(MCFragment &F,
                                          bool &WasRelaxed) const {
   MCContext &C = getContext();
@@ -887,55 +906,6 @@ void RISCVAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
   }
 }
 
-// Linker relaxation may change code size. We have to insert Nops
-// for .align directive when linker relaxation enabled. So then Linker
-// could satisfy alignment by removing Nops.
-// The function return the total Nops Size we need to insert.
-bool RISCVAsmBackend::shouldInsertExtraNopBytesForCodeAlign(
-    const MCFragment &AF, unsigned &Size) {
-  // Calculate Nops Size only when linker relaxation enabled.
-  const MCSubtargetInfo *STI = AF.getSubtargetInfo();
-  if (!STI->hasFeature(RISCV::FeatureRelax))
-    return false;
-
-  unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
-
-  if (AF.getAlignment() <= MinNopLen) {
-    return false;
-  } else {
-    Size = AF.getAlignment().value() - MinNopLen;
-    return true;
-  }
-}
-
-// We need to insert R_RISCV_ALIGN relocation type to indicate the
-// position of Nops and the total bytes of the Nops have been inserted
-// when linker relaxation en...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-backend-loongarch

Author: Fangrui Song (MaskRay)

Changes

Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

  • Introduces MCAsmBackend::relaxAlign to replace the two hooks.
  • Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
  • Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

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

10 Files Affected:

  • (modified) llvm/include/llvm/MC/MCAsmBackend.h (+4-14)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+54-56)
  • (modified) llvm/lib/MC/MCExpr.cpp (+1-4)
  • (modified) llvm/lib/MC/MCFragment.cpp (+6-1)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (-6)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp (+55-68)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h (+1-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+19-49)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+1-7)
  • (modified) llvm/test/MC/RISCV/Relocations/mc-dump.s (+4-2)
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 93259b0ea6d74..e476b76e0ab08 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -103,20 +103,6 @@ class LLVM_ABI MCAsmBackend {
   /// Get information on a fixup kind.
   virtual MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const;
 
-  /// Hook to check if extra nop bytes must be inserted for alignment directive.
-  /// For some targets this may be necessary in order to support linker
-  /// relaxation. The number of bytes to insert are returned in Size.
-  virtual bool shouldInsertExtraNopBytesForCodeAlign(const MCFragment &AF,
-                                                     unsigned &Size) {
-    return false;
-  }
-
-  /// Hook which indicates if the target requires a fixup to be generated when
-  /// handling an align directive in an executable section
-  virtual bool shouldInsertFixupForCodeAlign(MCAssembler &Asm, MCFragment &AF) {
-    return false;
-  }
-
   // Evaluate a fixup, returning std::nullopt to use default handling for
   // `Value` and `IsResolved`. Otherwise, returns `IsResolved` with the
   // expectation that the hook updates `Value`.
@@ -174,6 +160,10 @@ class LLVM_ABI MCAsmBackend {
   }
 
   // Defined by linker relaxation targets.
+
+  // Return false to use default handling. Otherwise, set `Size` to the number
+  // of padding bytes.
+  virtual bool relaxAlign(MCFragment &F, unsigned &Size) { return false; }
   virtual bool relaxDwarfLineAddr(MCFragment &, bool &WasRelaxed) const {
     return false;
   }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index f18ca38a27cad..cc6817fc62a24 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -230,22 +230,24 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
   case MCFragment::FT_Align: {
     unsigned Offset = F.Offset + F.getFixedSize();
     unsigned Size = offsetToAlignment(Offset, F.getAlignment());
-
-    // Insert extra Nops for code alignment if the target define
-    // shouldInsertExtraNopBytesForCodeAlign target hook.
-    if (F.getParent()->useCodeAlign() && F.hasAlignEmitNops() &&
-        getBackend().shouldInsertExtraNopBytesForCodeAlign(F, Size))
-      return F.getFixedSize() + Size;
-
-    // If we are padding with nops, force the padding to be larger than the
-    // minimum nop size.
-    if (Size > 0 && F.hasAlignEmitNops()) {
-      while (Size % getBackend().getMinimumNopSize())
-        Size += F.getAlignment().value();
+    auto &Frag = const_cast<MCFragment &>(F);
+    // In the nops mode, RISC-V style linker relaxation might adjust the size
+    // and add a fixup, even if `Size` is originally 0.
+    bool AlignFixup = false;
+    if (F.hasAlignEmitNops()) {
+      AlignFixup = getBackend().relaxAlign(Frag, Size);
+      // If the backend does not handle the fragment specially, pad with nops,
+      // but ensure that the padding is larger than the minimum nop size.
+      if (!AlignFixup)
+        while (Size % getBackend().getMinimumNopSize())
+          Size += F.getAlignment().value();
     }
-    if (Size > F.getAlignMaxBytesToEmit())
+    if (!AlignFixup && Size > F.getAlignMaxBytesToEmit())
       Size = 0;
-    return F.getFixedSize() + Size;
+    Frag.VarContentEnd = F.VarContentStart + Size;
+    if (Frag.VarContentEnd > Frag.getParent()->ContentStorage.size())
+      Frag.getParent()->ContentStorage.resize(Frag.VarContentEnd);
+    return F.getSize();
   }
 
   case MCFragment::FT_Org: {
@@ -419,7 +421,6 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
   switch (F.getKind()) {
   case MCFragment::FT_Data:
   case MCFragment::FT_Relaxable:
-  case MCFragment::FT_Align:
   case MCFragment::FT_LEB:
   case MCFragment::FT_Dwarf:
   case MCFragment::FT_DwarfFrame:
@@ -433,42 +434,44 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
     const auto &EF = cast<MCFragment>(F);
     OS << StringRef(EF.getContents().data(), EF.getContents().size());
     OS << StringRef(EF.getVarContents().data(), EF.getVarContents().size());
-    if (F.getKind() == MCFragment::FT_Align) {
-      ++stats::EmittedAlignFragments;
-      assert(F.getAlignFillLen() &&
-             "Invalid virtual align in concrete fragment!");
-
-      uint64_t Count = (FragmentSize - F.getFixedSize()) / F.getAlignFillLen();
-      assert((FragmentSize - F.getFixedSize()) % F.getAlignFillLen() == 0 &&
-             "computeFragmentSize computed size is incorrect");
-
-      // See if we are aligning with nops, and if so do that first to try to
-      // fill the Count bytes.  Then if that did not fill any bytes or there are
-      // any bytes left to fill use the Value and ValueSize to fill the rest. If
-      // we are aligning with nops, ask that target to emit the right data.
-      if (F.hasAlignEmitNops()) {
-        if (!Asm.getBackend().writeNopData(OS, Count, F.getSubtargetInfo()))
-          report_fatal_error("unable to write nop sequence of " + Twine(Count) +
-                             " bytes");
-      } else {
-        // Otherwise, write out in multiples of the value size.
-        for (uint64_t i = 0; i != Count; ++i) {
-          switch (F.getAlignFillLen()) {
-          default:
-            llvm_unreachable("Invalid size!");
-          case 1:
-            OS << char(F.getAlignFill());
-            break;
-          case 2:
-            support::endian::write<uint16_t>(OS, F.getAlignFill(), Endian);
-            break;
-          case 4:
-            support::endian::write<uint32_t>(OS, F.getAlignFill(), Endian);
-            break;
-          case 8:
-            support::endian::write<uint64_t>(OS, F.getAlignFill(), Endian);
-            break;
-          }
+  } break;
+
+  case MCFragment::FT_Align: {
+    ++stats::EmittedAlignFragments;
+    OS << StringRef(F.getContents().data(), F.getContents().size());
+    assert(F.getAlignFillLen() &&
+           "Invalid virtual align in concrete fragment!");
+
+    uint64_t Count = (FragmentSize - F.getFixedSize()) / F.getAlignFillLen();
+    assert((FragmentSize - F.getFixedSize()) % F.getAlignFillLen() == 0 &&
+           "computeFragmentSize computed size is incorrect");
+
+    // See if we are aligning with nops, and if so do that first to try to
+    // fill the Count bytes.  Then if that did not fill any bytes or there are
+    // any bytes left to fill use the Value and ValueSize to fill the rest. If
+    // we are aligning with nops, ask that target to emit the right data.
+    if (F.hasAlignEmitNops()) {
+      if (!Asm.getBackend().writeNopData(OS, Count, F.getSubtargetInfo()))
+        report_fatal_error("unable to write nop sequence of " + Twine(Count) +
+                           " bytes");
+    } else {
+      // Otherwise, write out in multiples of the value size.
+      for (uint64_t i = 0; i != Count; ++i) {
+        switch (F.getAlignFillLen()) {
+        default:
+          llvm_unreachable("Invalid size!");
+        case 1:
+          OS << char(F.getAlignFill());
+          break;
+        case 2:
+          support::endian::write<uint16_t>(OS, F.getAlignFill(), Endian);
+          break;
+        case 4:
+          support::endian::write<uint32_t>(OS, F.getAlignFill(), Endian);
+          break;
+        case 8:
+          support::endian::write<uint64_t>(OS, F.getAlignFill(), Endian);
+          break;
         }
       }
     }
@@ -739,11 +742,6 @@ void MCAssembler::layout() {
           evaluateFixup(F, Fixup, Target, FixedValue,
                         /*RecordReloc=*/true, Contents);
         }
-      } else if (F.getKind() == MCFragment::FT_Align) {
-        // For RISC-V linker relaxation, an alignment relocation might be
-        // needed.
-        if (F.hasAlignEmitNops())
-          getBackend().shouldInsertFixupForCodeAlign(*this, F);
       }
     }
   }
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index f0f1bd485258f..dbb2fd16eb2e5 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -370,7 +370,6 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
       }
 
       int64_t Num;
-      unsigned Count;
       if (DF) {
         Displacement += DF->getContents().size();
       } else if (F->getKind() == MCFragment::FT_Relaxable &&
@@ -380,9 +379,7 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
         // data fragment.
         Displacement += F->getSize();
       } else if (F->getKind() == MCFragment::FT_Align && Layout &&
-                 F->hasAlignEmitNops() &&
-                 !Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
-                     *F, Count)) {
+                 F->isLinkerRelaxable()) {
         Displacement += Asm->computeFragmentSize(*F);
       } else if (auto *FF = dyn_cast<MCFillFragment>(F);
                  FF && FF->getNumValues().evaluateAsAbsolute(Num)) {
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index d01660f640c46..bfaeb2be1ec90 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -84,8 +84,13 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
     auto Fixed = getContents();
     auto Var = getVarContents();
     OS << " Size:" << Fixed.size();
-    if (getKind() != MCFragment::FT_Data)
+    if (getKind() != MCFragment::FT_Data) {
       OS << '+' << Var.size();
+      // FT_Align uses getVarContents to track the size, but the content is
+      // garbage and not useful.
+      if (getKind() == MCFragment::FT_Align)
+        Var = {};
+    }
     OS << " [";
     for (unsigned i = 0, e = Fixed.size(); i != e; ++i) {
       if (i) OS << ",";
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 6ac598b4d0d3b..99f8dd93efd9e 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -564,12 +564,6 @@ void MCObjectStreamer::emitCodeAlignment(Align Alignment,
   emitValueToAlignment(Alignment, 0, 1, MaxBytesToEmit);
   auto *F = getCurrentFragment();
   F->setAlignEmitNops(true, STI);
-  // With RISC-V style linker relaxation, mark the section as linker-relaxable
-  // if the alignment is larger than the minimum NOP size.
-  unsigned Size;
-  if (getAssembler().getBackend().shouldInsertExtraNopBytesForCodeAlign(*F,
-                                                                        Size))
-    getCurrentSectionOnly()->setLinkerRelaxable();
 }
 
 void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index 032bfea71140f..b1cf001c4bdca 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -177,74 +177,6 @@ void LoongArchAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
   }
 }
 
-// Linker relaxation may change code size. We have to insert Nops
-// for .align directive when linker relaxation enabled. So then Linker
-// could satisfy alignment by removing Nops.
-// The function returns the total Nops Size we need to insert.
-bool LoongArchAsmBackend::shouldInsertExtraNopBytesForCodeAlign(
-    const MCFragment &AF, unsigned &Size) {
-  // Calculate Nops Size only when linker relaxation enabled.
-  if (!AF.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
-    return false;
-
-  // Ignore alignment if MaxBytesToEmit is less than the minimum Nop size.
-  const unsigned MinNopLen = 4;
-  if (AF.getAlignMaxBytesToEmit() < MinNopLen)
-    return false;
-  Size = AF.getAlignment().value() - MinNopLen;
-  return AF.getAlignment() > MinNopLen;
-}
-
-// We need to insert R_LARCH_ALIGN relocation type to indicate the
-// position of Nops and the total bytes of the Nops have been inserted
-// when linker relaxation enabled.
-// The function inserts fixup_loongarch_align fixup which eventually will
-// transfer to R_LARCH_ALIGN relocation type.
-// The improved R_LARCH_ALIGN requires symbol index. The lowest 8 bits of
-// addend represent alignment and the other bits of addend represent the
-// maximum number of bytes to emit. The maximum number of bytes is zero
-// means ignore the emit limit.
-bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
-                                                        MCFragment &AF) {
-  // Insert the fixup only when linker relaxation enabled.
-  if (!AF.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
-    return false;
-
-  // Calculate total Nops we need to insert. If there are none to insert
-  // then simply return.
-  unsigned InsertedNopBytes;
-  if (!shouldInsertExtraNopBytesForCodeAlign(AF, InsertedNopBytes))
-    return false;
-
-  MCSection *Sec = AF.getParent();
-  MCContext &Ctx = getContext();
-  const MCExpr *Dummy = MCConstantExpr::create(0, Ctx);
-  MCFixup Fixup = MCFixup::create(AF.getFixedSize(), Dummy, ELF::R_LARCH_ALIGN);
-  unsigned MaxBytesToEmit = AF.getAlignMaxBytesToEmit();
-
-  auto createExtendedValue = [&]() {
-    const MCSymbolRefExpr *MCSym = getSecToAlignSym()[Sec];
-    if (MCSym == nullptr) {
-      // Define a marker symbol at the section with an offset of 0.
-      MCSymbol *Sym = Ctx.createNamedTempSymbol("la-relax-align");
-      Sym->setFragment(&*Sec->getBeginSymbol()->getFragment());
-      Asm.registerSymbol(*Sym);
-      MCSym = MCSymbolRefExpr::create(Sym, Ctx);
-      getSecToAlignSym()[Sec] = MCSym;
-    }
-    return MCValue::get(&MCSym->getSymbol(), nullptr,
-                        MaxBytesToEmit << 8 | Log2(AF.getAlignment()));
-  };
-
-  uint64_t FixedValue = 0;
-  MCValue Value = MaxBytesToEmit >= InsertedNopBytes
-                      ? MCValue::get(InsertedNopBytes)
-                      : createExtendedValue();
-  Asm.getWriter().recordRelocation(AF, Fixup, Value, FixedValue);
-
-  return true;
-}
-
 bool LoongArchAsmBackend::shouldForceRelocation(const MCFixup &Fixup,
                                                 const MCValue &Target) {
   switch (Fixup.getKind()) {
@@ -279,6 +211,61 @@ getRelocPairForSize(unsigned Size) {
   }
 }
 
+// Linker relaxation may change code size. We have to insert Nops
+// for .align directive when linker relaxation enabled. So then Linker
+// could satisfy alignment by removing Nops.
+// The function returns the total Nops Size we need to insert.
+bool LoongArchAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
+  // Use default handling unless linker relaxation is enabled and the
+  // MaxBytesToEmit >= the nop size.
+  if (!F.getSubtargetInfo()->hasFeature(LoongArch::FeatureRelax))
+    return false;
+  const unsigned MinNopLen = 4;
+  unsigned MaxBytesToEmit = F.getAlignMaxBytesToEmit();
+  if (MaxBytesToEmit < MinNopLen)
+    return false;
+
+  Size = F.getAlignment().value() - MinNopLen;
+  if (F.getAlignment() <= MinNopLen)
+    return false;
+
+  // We need to insert R_LARCH_ALIGN relocation type to indicate the
+  // position of Nops and the total bytes of the Nops have been inserted
+  // when linker relaxation enabled.
+  // The function inserts fixup_loongarch_align fixup which eventually will
+  // transfer to R_LARCH_ALIGN relocation type.
+  // The improved R_LARCH_ALIGN requires symbol index. The lowest 8 bits of
+  // addend represent alignment and the other bits of addend represent the
+  // maximum number of bytes to emit. The maximum number of bytes is zero
+  // means ignore the emit limit.
+  MCContext &Ctx = getContext();
+  const MCExpr *Expr = nullptr;
+  if (MaxBytesToEmit >= Size) {
+    Expr = MCConstantExpr::create(Size, getContext());
+  } else {
+    MCSection *Sec = F.getParent();
+    const MCSymbolRefExpr *SymRef = getSecToAlignSym()[Sec];
+    if (SymRef == nullptr) {
+      // Define a marker symbol at the section with an offset of 0.
+      MCSymbol *Sym = Ctx.createNamedTempSymbol("la-relax-align");
+      Sym->setFragment(&*Sec->getBeginSymbol()->getFragment());
+      Asm->registerSymbol(*Sym);
+      SymRef = MCSymbolRefExpr::create(Sym, Ctx);
+      getSecToAlignSym()[Sec] = SymRef;
+    }
+    Expr = MCBinaryExpr::createAdd(
+        SymRef,
+        MCConstantExpr::create((MaxBytesToEmit << 8) | Log2(F.getAlignment()),
+                               Ctx),
+        Ctx);
+  }
+  MCFixup Fixup =
+      MCFixup::create(0, Expr, FirstLiteralRelocationKind + ELF::R_LARCH_ALIGN);
+  F.setVarFixups({Fixup});
+  F.getParent()->setLinkerRelaxable();
+  return true;
+}
+
 std::pair<bool, bool> LoongArchAsmBackend::relaxLEB128(MCFragment &F,
                                                        int64_t &Value) const {
   const MCExpr &Expr = F.getLEBValue();
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
index 793e4093b1c9e..3d929fc49f95e 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
@@ -45,19 +45,13 @@ class LoongArchAsmBackend : public MCAsmBackend {
                   MutableArrayRef<char> Data, uint64_t Value,
                   bool IsResolved) override;
 
-  // Return Size with extra Nop Bytes for alignment directive in code section.
-  bool shouldInsertExtraNopBytesForCodeAlign(const MCFragment &AF,
-                                             unsigned &Size) override;
-
-  // Insert target specific fixup type for alignment directive in code section.
-  bool shouldInsertFixupForCodeAlign(MCAssembler &Asm, MCFragment &AF) override;
-
   bool shouldForceRelocation(const MCFixup &Fixup, const MCValue &Target);
 
   std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
 
   MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
 
+  bool relaxAlign(MCFragment &F, unsigned &Size) override;
   bool relaxDwarfLineAddr(MCFragment &F, bool &WasRelaxed) const override;
   bool relaxDwarfCFA(MCFragment &F, bool &WasRelaxed) const override;
   std::pair<bool, bool> relaxLEB128(MCFragment &F,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 6bc313656f7c1..3d124bace92fe 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -302,6 +302,25 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
   Inst = std::move(Res);
 }
 
+bool RISCVAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
+  // Use default handling unless linker relaxation is enabled and the alignment
+  // is larger than the nop size.
+  const MCSubtargetInfo *STI = F.getSubtargetInfo();
+  if (!STI->hasFeature(RISCV::FeatureRelax))
+    return false;
+  unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
+  if (F.getAlignment() <= MinNopLen)
+    return false;
+
+  Size = F.getAlignment().value() - MinNopLen;
+  auto *Expr = MCConstantExpr::create(Size, getContext());
+  MCFixup Fixup =
+      MCFixup::create(0, Expr, FirstLiteralRelocationKind + ELF::R_RISCV_ALIGN);
+  F.setVarFixups({Fixup});
+  F.getParent()->setLinkerRelaxable();
+  return true;
+}
+
 bool RISCVAsmBackend::relaxDwarfLineAddr(MCFragment &F,
                                          bool &WasRelaxed) const {
   MCContext &C = getContext();
@@ -887,55 +906,6 @@ void RISCVAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
   }
 }
 
-// Linker relaxation may change code size. We have to insert Nops
-// for .align directive when linker relaxation enabled. So then Linker
-// could satisfy alignment by removing Nops.
-// The function return the total Nops Size we need to insert.
-bool RISCVAsmBackend::shouldInsertExtraNopBytesForCodeAlign(
-    const MCFragment &AF, unsigned &Size) {
-  // Calculate Nops Size only when linker relaxation enabled.
-  const MCSubtargetInfo *STI = AF.getSubtargetInfo();
-  if (!STI->hasFeature(RISCV::FeatureRelax))
-    return false;
-
-  unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
-
-  if (AF.getAlignment() <= MinNopLen) {
-    return false;
-  } else {
-    Size = AF.getAlignment().value() - MinNopLen;
-    return true;
-  }
-}
-
-// We need to insert R_RISCV_ALIGN relocation type to indicate the
-// position of Nops and the total bytes of the Nops have been inserted
-// when linker relaxation en...
[truncated]

@MaskRay MaskRay requested a review from SixWeining July 18, 2025 07:01
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.

I like reducing the number of hooks, but the const_cast feels to hacky.

MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 19, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
MaskRay added 4 commits July 18, 2025 20:09
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 19, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 19, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
MaskRay added 2 commits July 19, 2025 17:22
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 20, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentStart and VarContentEnd (content is arbitrary).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
@MaskRay MaskRay requested a review from aengelke July 20, 2025 00:26
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.

LGTM

if (!AlignFixup && Size > F.getAlignMaxBytesToEmit())
Size = 0;
// Update the variable tail size. The content is ignored.
F.VarContentEnd = F.VarContentStart + Size;
Copy link
Contributor

Choose a reason for hiding this comment

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

NB: VarContentStart should be zero here, it is never set for FT_Align. For clarity, I'd suggest:

Suggested change
F.VarContentEnd = F.VarContentStart + Size;
F.VarContentStart = 0;
F.VarContentEnd = Size;

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree! Let me try assert(F.VarContentStart == 0);

@MaskRay
Copy link
Member Author

MaskRay commented Jul 20, 2025

Next steps:

  • Allocate the fixed content as trailing data of the MCFragment by utilizing a special bump allocator (gnulib obstack)
  • Delete MCFixup member variables from MCFragment. Instead, add a MCFragment pointer to MCFixup.

MaskRay added 2 commits July 20, 2025 00:51
Created using spr 1.3.5-bogner

[skip ci]
@MaskRay MaskRay changed the base branch from users/MaskRay/spr/main.mc-refactor-ft_align-fragments-when-linker-relaxation-is-enabled to main July 20, 2025 07:55
@MaskRay MaskRay merged commit fd6d6a7 into main Jul 20, 2025
10 of 15 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/mc-refactor-ft_align-fragments-when-linker-relaxation-is-enabled branch July 20, 2025 07:55
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 20, 2025
…enabled

Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentEnd (content is ignored).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm/llvm-project#149465
MaskRay added a commit that referenced this pull request Jul 20, 2025
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentEnd (content is ignored).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentEnd (content is ignored).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
cachemeifyoucan pushed a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentEnd (content is ignored).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
cachemeifyoucan pushed a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Previously, two MCAsmBackend hooks were used, with
shouldInsertFixupForCodeAlign calling getWriter().recordRelocation
directly, bypassing generic code.

This patch:

* Introduces MCAsmBackend::relaxAlign to replace the two hooks.
* Tracks padding size using VarContentEnd (content is ignored).
* Move setLinkerRelaxable from MCObjectStreamer::emitCodeAlignment to the backends.

Pull Request: llvm#149465
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants