Automerge: RISCV,LoongArch: Encode RELAX relocation implicitly

MaskRay · github-actions[bot] · commit 79e46c3f6469 · 2025-05-20T01:41:37.000Z
When linker relaxation is enabled, relaxable relocations are followed by a R_RISCV_RELAX/R_LARCH_RELAX relocation. They are encoded as two fixups by CodeEmitter and expected to have the same `IsResolved` value within MCAssembler::evaluateFixup (they must lead to either 0 or 2 relocations). This scheme wasite space and requires RISCVAsmBackend::shouldForceRelocation to be conservative. This patch introduces MCFixup::NeedsRelax to encode the RELAX relocation implicitly. The fixup will lead to either 0 or 2 relocations. Pull Request: llvm/llvm-project#140494
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -41,7 +41,7 @@ class raw_ostream;
 /// Generic interface to target specific assembler backends.
 class MCAsmBackend {
 protected: // Can only create subclasses.
-  MCAsmBackend(llvm::endianness Endian, unsigned RelaxFixupKind = 0);
+  MCAsmBackend(llvm::endianness Endian, bool LinkerRelaxation = false);
 
 public:
   MCAsmBackend(const MCAsmBackend &) = delete;
@@ -50,10 +50,9 @@ class MCAsmBackend {
 
   const llvm::endianness Endian;
 
-  /// Fixup kind used for linker relaxation. Currently only used by RISC-V
-  /// and LoongArch.
-  const unsigned RelaxFixupKind;
-  bool allowLinkerRelaxation() const { return RelaxFixupKind != 0; }
+  /// True for RISC-V and LoongArch. Relaxable relocations are marked with a
+  /// RELAX relocation.
+  bool allowLinkerRelaxation() const { return LinkerRelaxation; }
 
   /// Return true if this target might automatically pad instructions and thus
   /// need to emit padding enable/disable directives around sensative code.
@@ -217,6 +216,9 @@ class MCAsmBackend {
   }
 
   bool isDarwinCanonicalPersonality(const MCSymbol *Sym) const;
+
+private:
+  const bool LinkerRelaxation;
 };
 
 } // end namespace llvm
diff --git a/llvm/include/llvm/MC/MCFixup.h b/llvm/include/llvm/MC/MCFixup.h
@@ -73,6 +73,12 @@ class MCFixup {
   /// determine how the operand value should be encoded into the instruction.
   MCFixupKind Kind = FK_NONE;
 
+  /// Used by RISC-V style linker relaxation. If the fixup is unresolved,
+  /// whether a RELAX relocation should follow.
+  bool NeedsRelax = false;
+
+  /// Consider bit fields if we need more flags.
+
   /// The source location which gave rise to the fixup, if any.
   SMLoc Loc;
 public:
@@ -99,6 +105,9 @@ class MCFixup {
 
   const MCExpr *getValue() const { return Value; }
 
+  bool needsRelax() const { return NeedsRelax; }
+  void setNeedsRelax() { NeedsRelax = true; }
+
   /// Return the generic fixup kind for a value with the given size. It
   /// is an error to pass an unsupported size.
   static MCFixupKind getKindForSize(unsigned Size, bool IsPCRel) {
diff --git a/llvm/lib/MC/MCAsmBackend.cpp b/llvm/lib/MC/MCAsmBackend.cpp
@@ -24,8 +24,8 @@
 
 using namespace llvm;
 
-MCAsmBackend::MCAsmBackend(llvm::endianness Endian, unsigned RelaxFixupKind)
-    : Endian(Endian), RelaxFixupKind(RelaxFixupKind) {}
+MCAsmBackend::MCAsmBackend(llvm::endianness Endian, bool LinkerRelaxation)
+    : Endian(Endian), LinkerRelaxation(LinkerRelaxation) {}
 
 MCAsmBackend::~MCAsmBackend() = default;
 
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
@@ -448,13 +448,13 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
                                            DF->getFixups(), STI);
 
   auto Fixups = MutableArrayRef(DF->getFixups()).slice(FixupStartIndex);
-  for (auto &Fixup : Fixups)
+  for (auto &Fixup : Fixups) {
     Fixup.setOffset(Fixup.getOffset() + CodeOffset);
+    if (Fixup.needsRelax())
+      DF->setLinkerRelaxable();
+  }
 
   DF->setHasInstructions(STI);
-  if (!Fixups.empty() && Fixups.back().getTargetKind() ==
-                             getAssembler().getBackend().RelaxFixupKind)
-    DF->setLinkerRelaxable();
 }
 
 void MCELFStreamer::emitBundleAlignMode(Align Alignment) {
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -31,8 +31,8 @@ using namespace llvm;
 LoongArchAsmBackend::LoongArchAsmBackend(const MCSubtargetInfo &STI,
                                          uint8_t OSABI, bool Is64Bit,
                                          const MCTargetOptions &Options)
-    : MCAsmBackend(llvm::endianness::little, ELF::R_LARCH_RELAX), STI(STI),
-      OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {}
+    : MCAsmBackend(llvm::endianness::little, /*LinkerRelaxation=*/true),
+      STI(STI), OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {}
 
 std::optional<MCFixupKind>
 LoongArchAsmBackend::getFixupKind(StringRef Name) const {
@@ -444,60 +444,72 @@ bool LoongArchAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
     return MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue, IsResolved,
                                   CurSTI);
   };
-  if (!Target.getSubSym())
-    return Fallback();
-  assert(Target.getSpecifier() == 0 &&
-         "relocatable SymA-SymB cannot have relocation specifier");
-  std::pair<MCFixupKind, MCFixupKind> FK;
   uint64_t FixedValueA, FixedValueB;
-  const MCSymbol &SA = *Target.getAddSym();
-  const MCSymbol &SB = *Target.getSubSym();
-
-  bool force = !SA.isInSection() || !SB.isInSection();
-  if (!force) {
-    const MCSection &SecA = SA.getSection();
-    const MCSection &SecB = SB.getSection();
-
-    // We need record relocation if SecA != SecB. Usually SecB is same as the
-    // section of Fixup, which will be record the relocation as PCRel. If SecB
-    // is not same as the section of Fixup, it will report error. Just return
-    // false and then this work can be finished by handleFixup.
-    if (&SecA != &SecB)
-      return Fallback();
-
-    // In SecA == SecB case. If the linker relaxation is enabled, we need record
-    // the ADD, SUB relocations. Otherwise the FixedValue has already been calc-
-    // ulated out in evaluateFixup, return true and avoid record relocations.
-    if (!STI.hasFeature(LoongArch::FeatureRelax))
-      return true;
+  if (Target.getSubSym()) {
+    assert(Target.getSpecifier() == 0 &&
+           "relocatable SymA-SymB cannot have relocation specifier");
+    std::pair<MCFixupKind, MCFixupKind> FK;
+    const MCSymbol &SA = *Target.getAddSym();
+    const MCSymbol &SB = *Target.getSubSym();
+
+    bool force = !SA.isInSection() || !SB.isInSection();
+    if (!force) {
+      const MCSection &SecA = SA.getSection();
+      const MCSection &SecB = SB.getSection();
+
+      // We need record relocation if SecA != SecB. Usually SecB is same as the
+      // section of Fixup, which will be record the relocation as PCRel. If SecB
+      // is not same as the section of Fixup, it will report error. Just return
+      // false and then this work can be finished by handleFixup.
+      if (&SecA != &SecB)
+        return Fallback();
+
+      // In SecA == SecB case. If the linker relaxation is enabled, we need
+      // record the ADD, SUB relocations. Otherwise the FixedValue has already
+      // been calc- ulated out in evaluateFixup, return true and avoid record
+      // relocations.
+      if (!STI.hasFeature(LoongArch::FeatureRelax))
+        return true;
+    }
+
+    switch (Fixup.getKind()) {
+    case llvm::FK_Data_1:
+      FK = getRelocPairForSize(8);
+      break;
+    case llvm::FK_Data_2:
+      FK = getRelocPairForSize(16);
+      break;
+    case llvm::FK_Data_4:
+      FK = getRelocPairForSize(32);
+      break;
+    case llvm::FK_Data_8:
+      FK = getRelocPairForSize(64);
+      break;
+    case llvm::FK_Data_leb128:
+      FK = getRelocPairForSize(128);
+      break;
+    default:
+      llvm_unreachable("unsupported fixup size");
+    }
+    MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
+    MCValue B = MCValue::get(Target.getSubSym());
+    auto FA = MCFixup::create(Fixup.getOffset(), nullptr, std::get<0>(FK));
+    auto FB = MCFixup::create(Fixup.getOffset(), nullptr, std::get<1>(FK));
+    Asm.getWriter().recordRelocation(Asm, &F, FA, A, FixedValueA);
+    Asm.getWriter().recordRelocation(Asm, &F, FB, B, FixedValueB);
+    FixedValue = FixedValueA - FixedValueB;
+    return false;
   }
 
-  switch (Fixup.getKind()) {
-  case llvm::FK_Data_1:
-    FK = getRelocPairForSize(8);
-    break;
-  case llvm::FK_Data_2:
-    FK = getRelocPairForSize(16);
-    break;
-  case llvm::FK_Data_4:
-    FK = getRelocPairForSize(32);
-    break;
-  case llvm::FK_Data_8:
-    FK = getRelocPairForSize(64);
-    break;
-  case llvm::FK_Data_leb128:
-    FK = getRelocPairForSize(128);
-    break;
-  default:
-    llvm_unreachable("unsupported fixup size");
+  IsResolved = Fallback();
+  // If linker relaxation is enabled and supported by the current relocation,
+  // append a RELAX relocation.
+  if (Fixup.needsRelax()) {
+    auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_LARCH_RELAX);
+    Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
+                                     FixedValueA);
   }
-  MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
-  MCValue B = MCValue::get(Target.getSubSym());
-  auto FA = MCFixup::create(Fixup.getOffset(), nullptr, std::get<0>(FK));
-  auto FB = MCFixup::create(Fixup.getOffset(), nullptr, std::get<1>(FK));
-  Asm.getWriter().recordRelocation(Asm, &F, FA, A, FixedValueA);
-  Asm.getWriter().recordRelocation(Asm, &F, FB, B, FixedValueB);
-  FixedValue = FixedValueA - FixedValueB;
+
   return true;
 }
 
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCCodeEmitter.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCCodeEmitter.cpp
@@ -199,14 +199,11 @@ LoongArchMCCodeEmitter::getExprOpValue(const MCInst &MI, const MCOperand &MO,
 
   Fixups.push_back(
       MCFixup::create(0, Expr, MCFixupKind(FixupKind), MI.getLoc()));
-
-  // Emit an R_LARCH_RELAX if linker relaxation is enabled and LAExpr has relax
-  // hint.
-  if (EnableRelax && RelaxCandidate) {
-    const MCConstantExpr *Dummy = MCConstantExpr::create(0, Ctx);
-    Fixups.push_back(
-        MCFixup::create(0, Dummy, ELF::R_LARCH_RELAX, MI.getLoc()));
-  }
+  // If linker relaxation is enabled and supported by this relocation, set
+  // a bit so that if fixup is unresolved, a R_LARCH_RELAX relocation will be
+  // appended.
+  if (EnableRelax && RelaxCandidate)
+    Fixups.back().setNeedsRelax();
 
   return 0;
 }
@@ -256,13 +253,8 @@ void LoongArchMCCodeEmitter::expandAddTPRel(const MCInst &MI,
   // Emit the correct %le_add_r relocation for the symbol.
   Fixups.push_back(
       MCFixup::create(0, Expr, ELF::R_LARCH_TLS_LE_ADD_R, MI.getLoc()));
-
-  // Emit R_LARCH_RELAX for %le_add_r when the relax feature is enabled.
-  if (STI.hasFeature(LoongArch::FeatureRelax)) {
-    const MCConstantExpr *Dummy = MCConstantExpr::create(0, Ctx);
-    Fixups.push_back(
-        MCFixup::create(0, Dummy, ELF::R_LARCH_RELAX, MI.getLoc()));
-  }
+  if (STI.hasFeature(LoongArch::FeatureRelax))
+    Fixups.back().setNeedsRelax();
 
   // Emit a normal ADD instruction with the given operands.
   unsigned ADD = MI.getOpcode() == LoongArch::PseudoAddTPRel_D
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -36,8 +36,8 @@ static cl::opt<bool> ULEB128Reloc(
 
 RISCVAsmBackend::RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI,
                                  bool Is64Bit, const MCTargetOptions &Options)
-    : MCAsmBackend(llvm::endianness::little, ELF::R_RISCV_RELAX), STI(STI),
-      OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {
+    : MCAsmBackend(llvm::endianness::little, /*LinkerRelaxation=*/true),
+      STI(STI), OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {
   RISCVFeatures::validate(STI.getTargetTriple(), STI.getFeatureBits());
 }
 
@@ -620,45 +620,56 @@ bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
                                const MCFixup &Fixup, const MCValue &Target,
                                uint64_t &FixedValue, bool IsResolved,
                                const MCSubtargetInfo *STI) {
-  if (!Target.getSubSym())
-    return MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue, IsResolved,
-                                  STI);
-  assert(Target.getSpecifier() == 0 &&
-         "relocatable SymA-SymB cannot have relocation specifier");
   uint64_t FixedValueA, FixedValueB;
-  unsigned TA = 0, TB = 0;
-  switch (Fixup.getKind()) {
-  case llvm::FK_Data_1:
-    TA = ELF::R_RISCV_ADD8;
-    TB = ELF::R_RISCV_SUB8;
-    break;
-  case llvm::FK_Data_2:
-    TA = ELF::R_RISCV_ADD16;
-    TB = ELF::R_RISCV_SUB16;
-    break;
-  case llvm::FK_Data_4:
-    TA = ELF::R_RISCV_ADD32;
-    TB = ELF::R_RISCV_SUB32;
-    break;
-  case llvm::FK_Data_8:
-    TA = ELF::R_RISCV_ADD64;
-    TB = ELF::R_RISCV_SUB64;
-    break;
-  case llvm::FK_Data_leb128:
-    TA = ELF::R_RISCV_SET_ULEB128;
-    TB = ELF::R_RISCV_SUB_ULEB128;
-    break;
-  default:
-    llvm_unreachable("unsupported fixup size");
+  if (Target.getSubSym()) {
+    assert(Target.getSpecifier() == 0 &&
+           "relocatable SymA-SymB cannot have relocation specifier");
+    unsigned TA = 0, TB = 0;
+    switch (Fixup.getKind()) {
+    case llvm::FK_Data_1:
+      TA = ELF::R_RISCV_ADD8;
+      TB = ELF::R_RISCV_SUB8;
+      break;
+    case llvm::FK_Data_2:
+      TA = ELF::R_RISCV_ADD16;
+      TB = ELF::R_RISCV_SUB16;
+      break;
+    case llvm::FK_Data_4:
+      TA = ELF::R_RISCV_ADD32;
+      TB = ELF::R_RISCV_SUB32;
+      break;
+    case llvm::FK_Data_8:
+      TA = ELF::R_RISCV_ADD64;
+      TB = ELF::R_RISCV_SUB64;
+      break;
+    case llvm::FK_Data_leb128:
+      TA = ELF::R_RISCV_SET_ULEB128;
+      TB = ELF::R_RISCV_SUB_ULEB128;
+      break;
+    default:
+      llvm_unreachable("unsupported fixup size");
+    }
+    MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
+    MCValue B = MCValue::get(Target.getSubSym());
+    auto FA = MCFixup::create(Fixup.getOffset(), nullptr, TA);
+    auto FB = MCFixup::create(Fixup.getOffset(), nullptr, TB);
+    Asm.getWriter().recordRelocation(Asm, &F, FA, A, FixedValueA);
+    Asm.getWriter().recordRelocation(Asm, &F, FB, B, FixedValueB);
+    FixedValue = FixedValueA - FixedValueB;
+    return false;
   }
-  MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
-  MCValue B = MCValue::get(Target.getSubSym());
-  auto FA = MCFixup::create(Fixup.getOffset(), nullptr, TA);
-  auto FB = MCFixup::create(Fixup.getOffset(), nullptr, TB);
-  Asm.getWriter().recordRelocation(Asm, &F, FA, A, FixedValueA);
-  Asm.getWriter().recordRelocation(Asm, &F, FB, B, FixedValueB);
-  FixedValue = FixedValueA - FixedValueB;
-  return true;
+
+  IsResolved = MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue,
+                                      IsResolved, STI);
+  // If linker relaxation is enabled and supported by the current relocation,
+  // append a RELAX relocation.
+  if (Fixup.needsRelax()) {
+    auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
+    Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
+                                     FixedValueA);
+  }
+
+  return false;
 }
 
 void RISCVAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
