[MC][RISCV] Make .reloc support arbitrary relocation types

[MaskRay]

Summary

Similar to D76746 (ARM), D76754 (AArch64) and llvmorg-11-init-6967-g152d14da64c (x86)

[MaskRay]

Forgot to upload reloc-directive-err.s

[MaskRay]

Ping :)

[luismarques]

I think this looks good. I'll let @lenary have another look.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

93–94

Shouldn't this also check the upper bound?

385–386

Ditto.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp

55–56

Ditto.

[MaskRay]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

93–94

Not very necessarily.

D76746 touched the upper bound. The point of the upper bound is actually quite weak: it is used as an assert in MCFixup::create to reject certain apparently invalid relocations.

I want to keep the code as-is to be consistent with changes I made to ARM/AArch64/X86/PowerPC.

[MaskRay]

Ping

[lenary]

I think this looks good, but I definitely feel out of my depth in evaluating it, not really knowing this part of the backend and any invariants that need to be preserved. For this reason I'm unwilling to give an explicit approval/rejection.

I do think the testing seems adequate, and this is a useful feature that we should enable.

[MaskRay]

Thanks! I have made very similar changes to x86/PPC/AArch64/ARM. I think I am well-versed in MC. Given no objection, I will push this.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h

bool fixupNeedsRelaxationAdvanced(const MCFixup &Fixup, bool Resolved,
                                     const MCRelaxableFragment *DF,
                                     const MCAsmLayout &Layout,
                                     const bool WasForced) const override;
 
   unsigned getNumFixupKinds() const override {
     return RISCV::NumTargetFixupKinds;
   }
 
-  const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override {
-    const static MCFixupKindInfo Infos[] = {
-      // This table *must* be in the order that the fixup_* kinds are defined in
-      // RISCVFixupKinds.h.
-      //
-      // name                      offset bits  flags
-      { "fixup_riscv_hi20",         12,     20,  0 },
-      { "fixup_riscv_lo12_i",       20,     12,  0 },
-      { "fixup_riscv_lo12_s",        0,     32,  0 },
-      { "fixup_riscv_pcrel_hi20",   12,     20,
-        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
-      { "fixup_riscv_pcrel_lo12_i", 20,     12,
-        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
-      { "fixup_riscv_pcrel_lo12_s",  0,     32,
-        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
-      { "fixup_riscv_got_hi20",     12,     20,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_tprel_hi20",   12,     20,  0 },
-      { "fixup_riscv_tprel_lo12_i", 20,     12,  0 },
-      { "fixup_riscv_tprel_lo12_s",  0,     32,  0 },
-      { "fixup_riscv_tprel_add",     0,      0,  0 },
-      { "fixup_riscv_tls_got_hi20", 12,     20,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_tls_gd_hi20",  12,     20,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_jal",          12,     20,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_branch",        0,     32,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_rvc_jump",      2,     11,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_rvc_branch",    0,     16,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_call",          0,     64,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_call_plt",      0,     64,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_relax",         0,      0,  0 },
-      { "fixup_riscv_align",         0,      0,  0 }
-    };
-    static_assert((array_lengthof(Infos)) == RISCV::NumTargetFixupKinds,
-                  "Not all fixup kinds added to Infos array");
+  Optional<MCFixupKind> getFixupKind(StringRef Name) const override;
 
-    if (Kind < FirstTargetFixupKind)
-      return MCAsmBackend::getFixupKindInfo(Kind);
-
-    assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
-           "Invalid kind!");
-    return Infos[Kind - FirstTargetFixupKind];
-  }
+  const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;
 
   bool mayNeedRelaxation(const MCInst &Inst,
                          const MCSubtargetInfo &STI) const override;
   unsigned getRelaxedOpcode(unsigned Op) const;
 
   void relaxInstruction(const MCInst &Inst, const MCSubtargetInfo &STI,
                         MCInst &Res) const override;
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
+Optional<MCFixupKind> RISCVAsmBackend::getFixupKind(StringRef Name) const {
+  if (STI.getTargetTriple().isOSBinFormatELF()) {
+    unsigned Type;
+    Type = llvm::StringSwitch<unsigned>(Name)
+#define ELF_RELOC(X, Y) .Case(#X, Y)
+#include "llvm/BinaryFormat/ELFRelocs/RISCV.def"
+#undef ELF_RELOC
+               .Default(-1u);
+    if (Type != -1u)
+      return static_cast<MCFixupKind>(FirstLiteralRelocationKind + Type);
+  }
+  return None;
+}
+
+const MCFixupKindInfo &
+RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
+  const static MCFixupKindInfo Infos[] = {
+      // This table *must* be in the order that the fixup_* kinds are defined in
+      // RISCVFixupKinds.h.
+      //
+      // name                      offset bits  flags
+      {"fixup_riscv_hi20", 12, 20, 0},
+      {"fixup_riscv_lo12_i", 20, 12, 0},
+      {"fixup_riscv_lo12_s", 0, 32, 0},
+      {"fixup_riscv_pcrel_hi20", 12, 20,
+       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_pcrel_lo12_i", 20, 12,
+       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_pcrel_lo12_s", 0, 32,
+       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_got_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_tprel_hi20", 12, 20, 0},
+      {"fixup_riscv_tprel_lo12_i", 20, 12, 0},
+      {"fixup_riscv_tprel_lo12_s", 0, 32, 0},
+      {"fixup_riscv_tprel_add", 0, 0, 0},
+      {"fixup_riscv_tls_got_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_tls_gd_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_jal", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_branch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_rvc_jump", 2, 11, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_rvc_branch", 0, 16, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_call", 0, 64, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_call_plt", 0, 64, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_relax", 0, 0, 0},
+      {"fixup_riscv_align", 0, 0, 0}};
+  static_assert((array_lengthof(Infos)) == RISCV::NumTargetFixupKinds,
+                "Not all fixup kinds added to Infos array");
+
+  // Fixup kinds from .reloc directive are like R_RISCV_NONE. They
+  // do not require any extra processing.
+  if (Kind >= FirstLiteralRelocationKind)
+    return MCAsmBackend::getFixupKindInfo(FK_NONE);
+
+  if (Kind < FirstTargetFixupKind)
+    return MCAsmBackend::getFixupKindInfo(Kind);
+
+  assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
+         "Invalid kind!");
+  return Infos[Kind - FirstTargetFixupKind];
+}
+
 // If linker relaxation is enabled, or the relax option had previously been
 // enabled, always emit relocations even if the fixup can be resolved. This is
 // necessary for correctness as offsets may change during relaxation.
 bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                             const MCFixup &Fixup,
                                             const MCValue &Target) {
+  if (Fixup.getKind() >= FirstLiteralRelocationKind)
+    return true;
   switch (Fixup.getTargetKind()) {
   default:
     break;
   case FK_Data_1:
   case FK_Data_2:
   case FK_Data_4:
   case FK_Data_8:
     if (Target.isAbsolute())
bool RISCVAsmBackend::evaluateTargetFixup(
   return true;
 }
 
 void RISCVAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                  const MCValue &Target,
                                  MutableArrayRef<char> Data, uint64_t Value,
                                  bool IsResolved,
                                  const MCSubtargetInfo *STI) const {
+  MCFixupKind Kind = Fixup.getKind();
+  if (Kind >= FirstLiteralRelocationKind)
+    return;
   MCContext &Ctx = Asm.getContext();
-  MCFixupKindInfo Info = getFixupKindInfo(Fixup.getKind());
+  MCFixupKindInfo Info = getFixupKindInfo(Kind);
   if (!Value)
     return; // Doesn't change encoding.
   // Apply any target-specific value adjustments.
   Value = adjustFixupValue(Fixup, Value, Ctx);
 
   // Shift the value into position.
   Value <<= Info.TargetOffset;
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp

 
 unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
                                             const MCValue &Target,
                                             const MCFixup &Fixup,
                                             bool IsPCRel) const {
   const MCExpr *Expr = Fixup.getValue();
   // Determine the type of the relocation
   unsigned Kind = Fixup.getTargetKind();
+  if (Kind >= FirstLiteralRelocationKind)
+    return Kind - FirstLiteralRelocationKind;
   if (IsPCRel) {
     switch (Kind) {
     default:
       Ctx.reportError(Fixup.getLoc(), "Unsupported relocation type");
       return ELF::R_RISCV_NONE;
     case FK_Data_4:
     case FK_PCRel_4:
       return ELF::R_RISCV_32_PCREL;

llvm/test/MC/RISCV/reloc-directive.s

+# RUN: llvm-mc -triple=riscv32 %s | FileCheck --check-prefix=PRINT %s
+# RUN: llvm-mc -triple=riscv64 %s | FileCheck --check-prefix=PRINT %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s | llvm-readobj -r | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s | llvm-readobj -r | FileCheck %s
+
+# PRINT: .reloc 8, R_RISCV_NONE, .data
+# PRINT: .reloc 4, R_RISCV_NONE, foo+4
+# PRINT: .reloc 0, R_RISCV_NONE, 8
+# PRINT: .reloc 0, R_RISCV_32, .data+2
+# PRINT: .reloc 0, R_RISCV_SET32, foo+3
+# PRINT: .reloc 0, R_RISCV_32_PCREL, 5
+
+# CHECK:      0x8 R_RISCV_NONE .data 0x0
+# CHECK-NEXT: 0x4 R_RISCV_NONE foo 0x4
+# CHECK-NEXT: 0x0 R_RISCV_NONE - 0x8
+# CHECK-NEXT: 0x0 R_RISCV_32 .data 0x2
+# CHECK-NEXT: 0x0 R_RISCV_SET32 foo 0x3
+# CHECK-NEXT: 0x0 R_RISCV_32_PCREL - 0x5
+.text
+  ret
+  nop
+  nop
+  .reloc 8, R_RISCV_NONE, .data
+  .reloc 4, R_RISCV_NONE, foo+4
+  .reloc 0, R_RISCV_NONE, 8
+
+  .reloc 0, R_RISCV_32, .data+2
+  .reloc 0, R_RISCV_SET32, foo+3
+  .reloc 0, R_RISCV_32_PCREL, 5
+
+.data
+.globl foo
+foo:
+  .word 0
+  .word 0
+  .word 0