[llvm] r333079 - [RISCV] Add symbol diff relocation support for RISC-V

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Wed May 23 05:36:18 PDT 2018


Author: asb
Date: Wed May 23 05:36:18 2018
New Revision: 333079

URL: http://llvm.org/viewvc/llvm-project?rev=333079&view=rev
Log:
[RISCV] Add symbol diff relocation support for RISC-V

For RISC-V it is desirable to have relaxation happen in the linker once 
addresses are known, and as such the size between two instructions/byte 
sequences in a section could change.

For most assembler expressions, this is fine, as the absolute address results 
in the expression being converted to a fixup, and finally relocations. 
However, for expressions such as .quad .L2-.L1, the assembler folds this down 
to a constant once fragments are laid out, under the assumption that the 
difference can no longer change, although in the case of linker relaxation the 
differences can change at link time, so the constant is incorrect. One place 
where this commonly appears is in debug information, where the size of a 
function expression is in a form similar to the above.

This patch extends the assembler to allow an AsmBackend to declare that it 
does not want the assembler to fold down this expression, and instead generate 
a pair of relocations that allow the linker to carry out the calculation. In 
this case, the expression is not folded, but when it comes to emitting a 
fixup, the generic FK_Data_* fixups are converted into a pair, one for the 
addition half, one for the subtraction, and this is passed to the relocation 
generating methods as usual. I have named these FK_Data_Add_* and 
FK_Data_Sub_* to indicate which half these are for.

For RISC-V, which supports this via e.g. the R_RISCV_ADD64, R_RISCV_SUB64 pair 
of relocations, these are also set to always emit relocations relative to 
local symbols rather than section offsets. This is to deal with the fact that 
if relocations were calculated on e.g. .text+8 and .text+4, the result 12 
would be stored rather than 4 as both addends are added in the linker.

Differential Revision: https://reviews.llvm.org/D45181
Patch by Simon Cook.


Added:
    llvm/trunk/test/MC/RISCV/fixups-expr.s
    llvm/trunk/test/MC/RISCV/hilo-constaddr-expr.s
Modified:
    llvm/trunk/include/llvm/MC/MCAsmBackend.h
    llvm/trunk/include/llvm/MC/MCFixup.h
    llvm/trunk/lib/MC/MCAsmBackend.cpp
    llvm/trunk/lib/MC/MCAssembler.cpp
    llvm/trunk/lib/MC/MCExpr.cpp
    llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
    llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
    llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
    llvm/trunk/test/MC/RISCV/hilo-constaddr.s

Modified: llvm/trunk/include/llvm/MC/MCAsmBackend.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmBackend.h?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCAsmBackend.h (original)
+++ llvm/trunk/include/llvm/MC/MCAsmBackend.h Wed May 23 05:36:18 2018
@@ -96,6 +96,10 @@ public:
                           const MCValue &Target, MutableArrayRef<char> Data,
                           uint64_t Value, bool IsResolved) const = 0;
 
+  /// Check whether the given target requires emitting differences of two
+  /// symbols as a set of relocations.
+  virtual bool requiresDiffExpressionRelocations() const { return false; }
+
   /// @}
 
   /// \name Target Relaxation Interfaces

Modified: llvm/trunk/include/llvm/MC/MCFixup.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCFixup.h?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCFixup.h (original)
+++ llvm/trunk/include/llvm/MC/MCFixup.h Wed May 23 05:36:18 2018
@@ -41,6 +41,14 @@ enum MCFixupKind {
   FK_SecRel_2,   ///< A two-byte section relative fixup.
   FK_SecRel_4,   ///< A four-byte section relative fixup.
   FK_SecRel_8,   ///< A eight-byte section relative fixup.
+  FK_Data_Add_1, ///< A one-byte add fixup.
+  FK_Data_Add_2, ///< A two-byte add fixup.
+  FK_Data_Add_4, ///< A four-byte add fixup.
+  FK_Data_Add_8, ///< A eight-byte add fixup.
+  FK_Data_Sub_1, ///< A one-byte sub fixup.
+  FK_Data_Sub_2, ///< A two-byte sub fixup.
+  FK_Data_Sub_4, ///< A four-byte sub fixup.
+  FK_Data_Sub_8, ///< A eight-byte sub fixup.
 
   FirstTargetFixupKind = 128,
 
@@ -90,6 +98,28 @@ public:
     return FI;
   }
 
+  /// Return a fixup corresponding to the add half of a add/sub fixup pair for
+  /// the given Fixup.
+  static MCFixup createAddFor(const MCFixup &Fixup) {
+    MCFixup FI;
+    FI.Value = Fixup.getValue();
+    FI.Offset = Fixup.getOffset();
+    FI.Kind = (unsigned)getAddKindForKind(Fixup.getKind());
+    FI.Loc = Fixup.getLoc();
+    return FI;
+  }
+
+  /// Return a fixup corresponding to the sub half of a add/sub fixup pair for
+  /// the given Fixup.
+  static MCFixup createSubFor(const MCFixup &Fixup) {
+    MCFixup FI;
+    FI.Value = Fixup.getValue();
+    FI.Offset = Fixup.getOffset();
+    FI.Kind = (unsigned)getSubKindForKind(Fixup.getKind());
+    FI.Loc = Fixup.getLoc();
+    return FI;
+  }
+
   MCFixupKind getKind() const { return MCFixupKind(Kind); }
 
   uint32_t getOffset() const { return Offset; }
@@ -109,6 +139,30 @@ public:
     }
   }
 
+  /// Return the generic fixup kind for an addition with a given size. It
+  /// is an error to pass an unsupported size.
+  static MCFixupKind getAddKindForKind(unsigned Kind) {
+    switch (Kind) {
+    default: llvm_unreachable("Unknown type to convert!");
+    case FK_Data_1: return FK_Data_Add_1;
+    case FK_Data_2: return FK_Data_Add_2;
+    case FK_Data_4: return FK_Data_Add_4;
+    case FK_Data_8: return FK_Data_Add_8;
+    }
+  }
+
+  /// Return the generic fixup kind for an subtraction with a given size. It
+  /// is an error to pass an unsupported size.
+  static MCFixupKind getSubKindForKind(unsigned Kind) {
+    switch (Kind) {
+    default: llvm_unreachable("Unknown type to convert!");
+    case FK_Data_1: return FK_Data_Sub_1;
+    case FK_Data_2: return FK_Data_Sub_2;
+    case FK_Data_4: return FK_Data_Sub_4;
+    case FK_Data_8: return FK_Data_Sub_8;
+    }
+  }
+
   SMLoc getLoc() const { return Loc; }
 };
 

Modified: llvm/trunk/lib/MC/MCAsmBackend.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmBackend.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCAsmBackend.cpp (original)
+++ llvm/trunk/lib/MC/MCAsmBackend.cpp Wed May 23 05:36:18 2018
@@ -84,7 +84,15 @@ const MCFixupKindInfo &MCAsmBackend::get
       {"FK_SecRel_1", 0, 8, 0},
       {"FK_SecRel_2", 0, 16, 0},
       {"FK_SecRel_4", 0, 32, 0},
-      {"FK_SecRel_8", 0, 64, 0}};
+      {"FK_SecRel_8", 0, 64, 0},
+      {"FK_Data_Add_1", 0, 8, 0},
+      {"FK_Data_Add_2", 0, 16, 0},
+      {"FK_Data_Add_4", 0, 32, 0},
+      {"FK_Data_Add_8", 0, 64, 0},
+      {"FK_Data_Sub_1", 0, 8, 0},
+      {"FK_Data_Sub_2", 0, 16, 0},
+      {"FK_Data_Sub_4", 0, 32, 0},
+      {"FK_Data_Sub_8", 0, 64, 0}};
 
   assert((size_t)Kind <= array_lengthof(Builtins) && "Unknown fixup kind");
   return Builtins[Kind];

Modified: llvm/trunk/lib/MC/MCAssembler.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCAssembler.cpp (original)
+++ llvm/trunk/lib/MC/MCAssembler.cpp Wed May 23 05:36:18 2018
@@ -720,7 +720,26 @@ MCAssembler::handleFixup(const MCAsmLayo
     // The fixup was unresolved, we need a relocation. Inform the object
     // writer of the relocation, and give it an opportunity to adjust the
     // fixup value if need be.
-    getWriter().recordRelocation(*this, Layout, &F, Fixup, Target, FixedValue);
+    if (Target.getSymA() && Target.getSymB() &&
+        getBackend().requiresDiffExpressionRelocations()) {
+      // The fixup represents the difference between two symbols, which the
+      // backend has indicated must be resolved at link time. Split up the fixup
+      // into two relocations, one for the add, and one for the sub, and emit
+      // both of these. The constant will be associated with the add half of the
+      // expression.
+      MCFixup FixupAdd = MCFixup::createAddFor(Fixup);
+      MCValue TargetAdd =
+          MCValue::get(Target.getSymA(), nullptr, Target.getConstant());
+      getWriter().recordRelocation(*this, Layout, &F, FixupAdd, TargetAdd,
+                                   FixedValue);
+      MCFixup FixupSub = MCFixup::createSubFor(Fixup);
+      MCValue TargetSub = MCValue::get(Target.getSymB());
+      getWriter().recordRelocation(*this, Layout, &F, FixupSub, TargetSub,
+                                   FixedValue);
+    } else {
+      getWriter().recordRelocation(*this, Layout, &F, Fixup, Target,
+                                   FixedValue);
+    }
   }
   return std::make_tuple(Target, FixedValue, IsResolved);
 }

Modified: llvm/trunk/lib/MC/MCExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCExpr.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCExpr.cpp (original)
+++ llvm/trunk/lib/MC/MCExpr.cpp Wed May 23 05:36:18 2018
@@ -11,6 +11,7 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCAsmLayout.h"
 #include "llvm/MC/MCAssembler.h"
@@ -576,8 +577,12 @@ EvaluateSymbolicAdd(const MCAssembler *A
   assert((!Layout || Asm) &&
          "Must have an assembler object if layout is given!");
 
-  // If we have a layout, we can fold resolved differences.
-  if (Asm) {
+  // If we have a layout, we can fold resolved differences. Do not do this if
+  // the backend requires this to be emitted as individual relocations, unless
+  // the InSet flag is set to get the current difference anyway (used for
+  // example to calculate symbol sizes).
+  if (Asm &&
+      (InSet || !Asm->getBackend().requiresDiffExpressionRelocations())) {
     // First, fold out any differences which are fully resolved. By
     // reassociating terms in
     //   Result = (LHS_A - LHS_B + LHS_Cst) + (RHS_A - RHS_B + RHS_Cst).

Modified: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp Wed May 23 05:36:18 2018
@@ -37,6 +37,11 @@ public:
         Is64Bit(Is64Bit) {}
   ~RISCVAsmBackend() override {}
 
+  // Generate diff expression relocations if the relax feature is enabled,
+  // otherwise it is safe for the assembler to calculate these internally.
+  bool requiresDiffExpressionRelocations() const override {
+    return STI.getFeatureBits()[RISCV::FeatureRelax];
+  }
   void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                   const MCValue &Target, MutableArrayRef<char> Data,
                   uint64_t Value, bool IsResolved) const override;

Modified: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp Wed May 23 05:36:18 2018
@@ -56,6 +56,22 @@ unsigned RISCVELFObjectWriter::getRelocT
     return ELF::R_RISCV_32;
   case FK_Data_8:
     return ELF::R_RISCV_64;
+  case FK_Data_Add_1:
+    return ELF::R_RISCV_ADD8;
+  case FK_Data_Add_2:
+    return ELF::R_RISCV_ADD16;
+  case FK_Data_Add_4:
+    return ELF::R_RISCV_ADD32;
+  case FK_Data_Add_8:
+    return ELF::R_RISCV_ADD64;
+  case FK_Data_Sub_1:
+    return ELF::R_RISCV_SUB8;
+  case FK_Data_Sub_2:
+    return ELF::R_RISCV_SUB16;
+  case FK_Data_Sub_4:
+    return ELF::R_RISCV_SUB32;
+  case FK_Data_Sub_8:
+    return ELF::R_RISCV_SUB64;
   case RISCV::fixup_riscv_hi20:
     return ELF::R_RISCV_HI20;
   case RISCV::fixup_riscv_lo12_i:

Modified: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp Wed May 23 05:36:18 2018
@@ -44,7 +44,23 @@ void RISCVMCExpr::printImpl(raw_ostream
 bool RISCVMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
                                             const MCAsmLayout *Layout,
                                             const MCFixup *Fixup) const {
-  return getSubExpr()->evaluateAsRelocatable(Res, Layout, Fixup);
+  if (!getSubExpr()->evaluateAsRelocatable(Res, Layout, Fixup))
+    return false;
+
+  // Some custom fixup types are not valid with symbol difference expressions
+  if (Res.getSymA() && Res.getSymB()) {
+    switch (getKind()) {
+    default:
+      return true;
+    case VK_RISCV_LO:
+    case VK_RISCV_HI:
+    case VK_RISCV_PCREL_LO:
+    case VK_RISCV_PCREL_HI:
+      return false;
+    }
+  }
+
+  return true;
 }
 
 void RISCVMCExpr::visitUsedExpr(MCStreamer &Streamer) const {

Added: llvm/trunk/test/MC/RISCV/fixups-expr.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/fixups-expr.s?rev=333079&view=auto
==============================================================================
--- llvm/trunk/test/MC/RISCV/fixups-expr.s (added)
+++ llvm/trunk/test/MC/RISCV/fixups-expr.s Wed May 23 05:36:18 2018
@@ -0,0 +1,47 @@
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+relax %s \
+# RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELAX %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=-relax %s \
+# RUN:     | llvm-readobj -r | FileCheck -check-prefix=NORELAX %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax %s \
+# RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELAX %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=-relax %s \
+# RUN:     | llvm-readobj -r | FileCheck -check-prefix=NORELAX %s
+
+# Check that subtraction expressions are emitted as two relocations
+# only when relaxation is enabled
+
+.globl G1
+.globl G2
+.L1:
+G1:
+addi a0, a0, 0
+.L2:
+G2:
+
+.data
+.dword .L2-.L1
+.dword G2-G1
+.word .L2-.L1
+.word G2-G1
+.half .L2-.L1
+.half G2-G1
+.byte .L2-.L1
+.byte G2-G1
+# RELAX: 0x0 R_RISCV_ADD64 .L2 0x0
+# RELAX: 0x0 R_RISCV_SUB64 .L1 0x0
+# RELAX: 0x8 R_RISCV_ADD64 G2 0x0
+# RELAX: 0x8 R_RISCV_SUB64 G1 0x0
+# RELAX: 0x10 R_RISCV_ADD32 .L2 0x0
+# RELAX: 0x10 R_RISCV_SUB32 .L1 0x0
+# RELAX: 0x14 R_RISCV_ADD32 G2 0x0
+# RELAX: 0x14 R_RISCV_SUB32 G1 0x0
+# RELAX: 0x18 R_RISCV_ADD16 .L2 0x0
+# RELAX: 0x18 R_RISCV_SUB16 .L1 0x0
+# RELAX: 0x1A R_RISCV_ADD16 G2 0x0
+# RELAX: 0x1A R_RISCV_SUB16 G1 0x0
+# RELAX: 0x1C R_RISCV_ADD8 .L2 0x0
+# RELAX: 0x1C R_RISCV_SUB8 .L1 0x0
+# RELAX: 0x1D R_RISCV_ADD8 G2 0x0
+# RELAX: 0x1D R_RISCV_SUB8 G1 0x0
+# NORELAX-NOT: R_RISCV

Added: llvm/trunk/test/MC/RISCV/hilo-constaddr-expr.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/hilo-constaddr-expr.s?rev=333079&view=auto
==============================================================================
--- llvm/trunk/test/MC/RISCV/hilo-constaddr-expr.s (added)
+++ llvm/trunk/test/MC/RISCV/hilo-constaddr-expr.s Wed May 23 05:36:18 2018
@@ -0,0 +1,31 @@
+# RUN: not llvm-mc -filetype=obj -triple=riscv32 -mattr=+relax %s 2>&1 \
+# RUN:     | FileCheck %s -check-prefix=CHECK-RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=-relax %s 2>&1 \
+# RUN:     | llvm-objdump -d - | FileCheck %s -check-prefix=CHECK-INSTR
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=-relax %s 2>&1 \
+# RUN:     | llvm-objdump -r - | FileCheck %s -check-prefix=CHECK-REL
+
+# Check the assembler rejects hi and lo expressions with constant expressions
+# involving labels when diff expressions are emitted as relocation pairs.
+# Test case derived from test/MC/Mips/hilo-addressing.s
+
+tmp1:
+  # Emit zeros so that difference between tmp1 and tmp3 is 0x30124 bytes.
+  .fill 0x30124-8
+tmp2:
+# CHECK-RELAX: :[[@LINE+1]]:{{[0-9]+}}: error: expected relocatable expression
+  lui t0, %hi(tmp3-tmp1)
+# CHECK-RELAX: :[[@LINE+1]]:{{[0-9]+}}: error: expected relocatable expression
+  lw ra, %lo(tmp3-tmp1)(t0)
+# CHECK-INSTR: lui t0, 48
+# CHECK-INSTR: lw ra, 292(t0)
+
+tmp3:
+# CHECK-RELAX: :[[@LINE+1]]:{{[0-9]+}}: error: expected relocatable expression
+  lui t1, %hi(tmp2-tmp3)
+# CHECK-RELAX: :[[@LINE+1]]:{{[0-9]+}}: error: expected relocatable expression
+  lw sp, %lo(tmp2-tmp3)(t1)
+# CHECK-INSTR: lui t1, 0
+# CHECK-INSTR: lw sp, -8(t1)
+
+# CHECK-REL-NOT: R_RISCV
\ No newline at end of file

Modified: llvm/trunk/test/MC/RISCV/hilo-constaddr.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/hilo-constaddr.s?rev=333079&r1=333078&r2=333079&view=diff
==============================================================================
--- llvm/trunk/test/MC/RISCV/hilo-constaddr.s (original)
+++ llvm/trunk/test/MC/RISCV/hilo-constaddr.s Wed May 23 05:36:18 2018
@@ -4,9 +4,8 @@
 # RUN: llvm-mc -filetype=obj -triple=riscv32 %s \
 # RUN:  | llvm-readobj -r | FileCheck %s -check-prefix=CHECK-REL
 
-# Check the assembler can handle hi and lo expressions with a constant 
-# address, and constant expressions involving labels. Test case derived from 
-# test/MC/Mips/hilo-addressing.s
+# Check the assembler can handle hi and lo expressions with a constant
+# address. Test case derived from test/MC/Mips/hilo-addressing.s
 
 # Check that 1 is added to the high 20 bits if bit 11 of the low part is 1.
 .equ addr, 0xdeadbeef
@@ -15,25 +14,4 @@
 # CHECK-INSTR: lui t0, 912092
 # CHECK-INSTR: lw ra, -273(t0)
 
-# Check that assembler can handle %hi(label1 - label2) and %lo(label1 - label2)
-# expressions.
-
-tmp1:
-  # Emit zeros so that difference between tmp1 and tmp3 is 0x30124 bytes.
-  .fill 0x30124-8
-tmp2:
-  lui t0, %hi(tmp3-tmp1)
-  lw ra, %lo(tmp3-tmp1)(t0)
-# CHECK-INSTR: lui t0, 48
-# CHECK-INSTR: lw ra, 292(t0)
-
-tmp3:
-  lui t1, %hi(tmp2-tmp3)
-  lw sp, %lo(tmp2-tmp3)(t1)
-# CHECK-INSTR: lui t1, 0
-# CHECK-INSTR: lw sp, -8(t1)
-
-# Check that a relocation isn't emitted for %hi(label1 - label2) and
-# %lo(label1 - label2) expressions.
-
 # CHECK-REL-NOT: R_RISCV




More information about the llvm-commits mailing list