[RISCV] Properly evaluate fixup_riscv_pcrel_lo12

This is a update to D43157 to correctly handle fixup_riscv_pcrel_lo12. Notable changes: Rebased onto trunk Handle and test S-type Test case pcrel-hilo.s is merged into relocations.s D43157 description: VK_RISCV_PCREL_LO has to be handled specially. The MCExpr inside is actually the location of an auipc instruction with a VK_RISCV_PCREL_HI fixup pointing to the real target. Differential Revision: https://reviews.llvm.org/D54029 Patch by Chih-Mao Chen and Michael Spencer. llvm-svn: 349764
llvm · Dec 20, 2018 · eb3a64a · eb3a64a
1 parent 09c0811
commit eb3a64a
Show file tree

Hide file tree

Showing 7 changed files with 159 additions and 26 deletions.
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "RISCVAsmBackend.h"
+#include "RISCVMCExpr.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
@@ -21,6 +22,44 @@
 
 using namespace llvm;
 
+// 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) {
+  bool ShouldForce = false;
+
+  switch ((unsigned)Fixup.getKind()) {
+  default:
+    break;
+  case RISCV::fixup_riscv_pcrel_lo12_i:
+  case RISCV::fixup_riscv_pcrel_lo12_s:
+    // For pcrel_lo12, force a relocation if the target of the corresponding
+    // pcrel_hi20 is not in the same fragment.
+    const MCFixup *T = cast<RISCVMCExpr>(Fixup.getValue())->getPCRelHiFixup();
+    if (!T) {
+      Asm.getContext().reportError(Fixup.getLoc(),
+                                   "could not find corresponding %pcrel_hi");
+      return false;
+    }
+
+    switch ((unsigned)T->getKind()) {
+    default:
+      llvm_unreachable("Unexpected fixup kind for pcrel_lo12");
+      break;
+    case RISCV::fixup_riscv_pcrel_hi20:
+      ShouldForce = T->getValue()->findAssociatedFragment() !=
+                    Fixup.getValue()->findAssociatedFragment();
+      break;
+    }
+    break;
+  }
+
+  return ShouldForce || STI.getFeatureBits()[RISCV::FeatureRelax] ||
+         ForceRelocs;
+}
+
 bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
                                                    bool Resolved,
                                                    uint64_t Value,

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -49,13 +49,8 @@ class RISCVAsmBackend : public MCAsmBackend {
   std::unique_ptr<MCObjectTargetWriter>
   createObjectTargetWriter() const override;
 
-  // 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 shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
-                             const MCValue &Target) override {
-    return STI.getFeatureBits()[RISCV::FeatureRelax] || ForceRelocs;
-  }
+                             const MCValue &Target) override;
 
   bool fixupNeedsRelaxation(const MCFixup &Fixup, uint64_t Value,
                             const MCRelaxableFragment *DF,

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -14,6 +14,7 @@
 
 #include "RISCV.h"
 #include "RISCVMCExpr.h"
+#include "RISCVFixupKinds.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCStreamer.h"
@@ -40,9 +41,90 @@ void RISCVMCExpr::printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const {
     OS << ')';
 }
 
+const MCFixup *RISCVMCExpr::getPCRelHiFixup() const {
+  MCValue AUIPCLoc;
+  if (!getSubExpr()->evaluateAsRelocatable(AUIPCLoc, nullptr, nullptr))
+    return nullptr;
+
+  const MCSymbolRefExpr *AUIPCSRE = AUIPCLoc.getSymA();
+  if (!AUIPCSRE)
+    return nullptr;
+
+  const auto *DF =
+      dyn_cast_or_null<MCDataFragment>(AUIPCSRE->findAssociatedFragment());
+  if (!DF)
+    return nullptr;
+
+  const MCSymbol *AUIPCSymbol = &AUIPCSRE->getSymbol();
+  for (const MCFixup &F : DF->getFixups()) {
+    if (F.getOffset() != AUIPCSymbol->getOffset())
+      continue;
+
+    switch ((unsigned)F.getKind()) {
+    default:
+      continue;
+    case RISCV::fixup_riscv_pcrel_hi20:
+      return &F;
+    }
+  }
+
+  return nullptr;
+}
+
+bool RISCVMCExpr::evaluatePCRelLo(MCValue &Res, const MCAsmLayout *Layout,
+                                  const MCFixup *Fixup) const {
+  // VK_RISCV_PCREL_LO has to be handled specially.  The MCExpr inside is
+  // actually the location of a auipc instruction with a VK_RISCV_PCREL_HI fixup
+  // pointing to the real target.  We need to generate an MCValue in the form of
+  // (<real target> + <offset from this fixup to the auipc fixup>).  The Fixup
+  // is pcrel relative to the VK_RISCV_PCREL_LO fixup, so we need to add the
+  // offset to the VK_RISCV_PCREL_HI Fixup from VK_RISCV_PCREL_LO to correct.
+  MCValue AUIPCLoc;
+  if (!getSubExpr()->evaluateAsValue(AUIPCLoc, *Layout))
+    return false;
+
+  const MCSymbolRefExpr *AUIPCSRE = AUIPCLoc.getSymA();
+  // Don't try to evaluate %pcrel_hi/%pcrel_lo pairs that cross fragment
+  // boundries.
+  if (!AUIPCSRE ||
+      findAssociatedFragment() != AUIPCSRE->findAssociatedFragment())
+    return false;
+
+  const MCSymbol *AUIPCSymbol = &AUIPCSRE->getSymbol();
+  if (!AUIPCSymbol)
+    return false;
+
+  const MCFixup *TargetFixup = getPCRelHiFixup();
+  if (!TargetFixup)
+    return false;
+
+  if ((unsigned)TargetFixup->getKind() != RISCV::fixup_riscv_pcrel_hi20)
+    return false;
+
+  MCValue Target;
+  if (!TargetFixup->getValue()->evaluateAsValue(Target, *Layout))
+    return false;
+
+  if (!Target.getSymA() || !Target.getSymA()->getSymbol().isInSection())
+    return false;
+
+  if (&Target.getSymA()->getSymbol().getSection() !=
+      findAssociatedFragment()->getParent())
+    return false;
+
+  uint64_t AUIPCOffset = AUIPCSymbol->getOffset();
+
+  Res = MCValue::get(Target.getSymA(), nullptr,
+                     Target.getConstant() + (Fixup->getOffset() - AUIPCOffset));
+  return true;
+}
+
 bool RISCVMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
                                             const MCAsmLayout *Layout,
                                             const MCFixup *Fixup) const {
+  if (Kind == VK_RISCV_PCREL_LO && evaluatePCRelLo(Res, Layout, Fixup))
+    return true;
+
   if (!getSubExpr()->evaluateAsRelocatable(Res, Layout, Fixup))
     return false;
 

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
@@ -39,6 +39,9 @@ class RISCVMCExpr : public MCTargetExpr {
 
   int64_t evaluateAsInt64(int64_t Value) const;
 
+  bool evaluatePCRelLo(MCValue &Res, const MCAsmLayout *Layout,
+                       const MCFixup *Fixup) const;
+
   explicit RISCVMCExpr(const MCExpr *Expr, VariantKind Kind)
       : Expr(Expr), Kind(Kind) {}
 
@@ -50,6 +53,13 @@ class RISCVMCExpr : public MCTargetExpr {
 
   const MCExpr *getSubExpr() const { return Expr; }
 
+  /// Get the MCExpr of the VK_RISCV_PCREL_HI Fixup that the
+  /// VK_RISCV_PCREL_LO points to.
+  ///
+  /// \returns nullptr if this isn't a VK_RISCV_PCREL_LO pointing to a
+  /// VK_RISCV_PCREL_HI.
+  const MCFixup *getPCRelHiFixup() const;
+
   void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override;
   bool evaluateAsRelocatableImpl(MCValue &Res, const MCAsmLayout *Layout,
                                  const MCFixup *Fixup) const override;

diff --git a/llvm/test/MC/RISCV/fixups.s b/llvm/test/MC/RISCV/fixups.s
@@ -24,15 +24,26 @@ sw a0, %lo(val)(t1)
 # CHECK-FIXUP: fixup A - offset: 0, value: %lo(val), kind: fixup_riscv_lo12_s
 # CHECK-INSTR: sw a0, 1656(t1)
 
+1:
+auipc t1, %pcrel_hi(.LBB0)
+# CHECK-FIXUP: fixup A - offset: 0, value: %pcrel_hi(.LBB0), kind: fixup_riscv_pcrel_hi20
+# CHECK-INSTR: auipc t1, 0
+addi t1, t1, %pcrel_lo(1b)
+# CHECK-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp0), kind: fixup_riscv_pcrel_lo12_i
+# CHECK-INSTR: addi t1, t1, -16
+sw t1, %pcrel_lo(1b)(t1)
+# CHECK-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp0), kind: fixup_riscv_pcrel_lo12_s
+# CHECK-INSTR: sw t1, -16(t1)
+
 jal zero, .LBB0
 # CHECK-FIXUP: fixup A - offset: 0, value: .LBB0, kind: fixup_riscv_jal
-# CHECK-INSTR: jal zero, -16
+# CHECK-INSTR: jal zero, -28
 jal zero, .LBB2
 # CHECK-FIXUP: fixup A - offset: 0, value: .LBB2, kind: fixup_riscv_jal
 # CHECK-INSTR: jal zero, 330996
 beq a0, a1, .LBB0
 # CHECK-FIXUP: fixup A - offset: 0, value: .LBB0, kind: fixup_riscv_branch
-# CHECK-INSTR: beq a0, a1, -24
+# CHECK-INSTR: beq a0, a1, -36
 blt a0, a1, .LBB1
 # CHECK-FIXUP: fixup A - offset: 0, value: .LBB1, kind: fixup_riscv_branch
 # CHECK-INSTR: blt a0, a1, 1108

diff --git a/llvm/test/MC/RISCV/pcrel-lo12-invalid.s b/llvm/test/MC/RISCV/pcrel-lo12-invalid.s
@@ -0,0 +1,5 @@
+# RUN: not llvm-mc -triple riscv32 -mattr=-relax -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv32 -mattr=+relax -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
+
+1:
+  addi a0, a0, %pcrel_lo(1b) # CHECK: :[[@LINE]]:3: error: could not find corresponding %pcrel_hi
diff --git a/llvm/test/MC/RISCV/relocations.s b/llvm/test/MC/RISCV/relocations.s
@@ -44,6 +44,7 @@ sb t1, %lo(foo+4)(a2)
 # INSTR: sb t1, %lo(foo+4)(a2)
 # FIXUP: fixup A - offset: 0, value: %lo(foo+4), kind: fixup_riscv_lo12_s
 
+.L0:
 auipc t1, %pcrel_hi(foo)
 # RELOC: R_RISCV_PCREL_HI20 foo 0x0
 # INSTR: auipc t1, %pcrel_hi(foo)
@@ -54,25 +55,15 @@ auipc t1, %pcrel_hi(foo+4)
 # INSTR: auipc t1, %pcrel_hi(foo+4)
 # FIXUP: fixup A - offset: 0, value: %pcrel_hi(foo+4), kind: fixup_riscv_pcrel_hi20
 
-addi t1, t1, %pcrel_lo(foo)
-# RELOC: R_RISCV_PCREL_LO12_I foo 0x0
-# INSTR: addi t1, t1, %pcrel_lo(foo)
-# FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo), kind: fixup_riscv_pcrel_lo12_i
+addi t1, t1, %pcrel_lo(.L0)
+# RELOC: R_RISCV_PCREL_LO12_I .L0 0x0
+# INSTR: addi t1, t1, %pcrel_lo(.L0)
+# FIXUP: fixup A - offset: 0, value: %pcrel_lo(.L0), kind: fixup_riscv_pcrel_lo12_i
 
-addi t1, t1, %pcrel_lo(foo+4)
-# RELOC: R_RISCV_PCREL_LO12_I foo 0x4
-# INSTR: addi t1, t1, %pcrel_lo(foo+4)
-# FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo+4), kind: fixup_riscv_pcrel_lo12_i
-
-sb t1, %pcrel_lo(foo)(a2)
-# RELOC: R_RISCV_PCREL_LO12_S foo 0x0
-# INSTR: sb t1, %pcrel_lo(foo)(a2)
-# FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo), kind: fixup_riscv_pcrel_lo12_s
-
-sb t1, %pcrel_lo(foo+4)(a2)
-# RELOC: R_RISCV_PCREL_LO12_S foo 0x4
-# INSTR: sb t1, %pcrel_lo(foo+4)(a2)
-# FIXUP: fixup A - offset: 0, value: %pcrel_lo(foo+4), kind: fixup_riscv_pcrel_lo12_s
+sb t1, %pcrel_lo(.L0)(a2)
+# RELOC: R_RISCV_PCREL_LO12_S .L0 0x0
+# INSTR: sb t1, %pcrel_lo(.L0)(a2)
+# FIXUP: fixup A - offset: 0, value: %pcrel_lo(.L0), kind: fixup_riscv_pcrel_lo12_s
 
 jal zero, foo
 # RELOC: R_RISCV_JAL