[PATCH] D61907: [RISCV] Leave pcrel_hi/pcrel_lo fixup pairs unresolved

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill created this revision.
lewis-revill added reviewers: asb, jrtc27, kito-cheng.
Herald added subscribers: llvm-commits, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: LLVM.

This patch modifies shouldForceRelocation to always return true for pcrel_hi and pcrel_lo fixups. This avoids having to use various levels of indirection in order to correctly determine whether a given one of these fixups can be safely resolved, since there are various situations where determining whether a given fixup can be safely resolved relies on knowing whether its corresponding fixup could be safely resolved.


Repository:
  rL LLVM

https://reviews.llvm.org/D61907

Files:
  lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  test/MC/RISCV/fixups.s
  test/MC/RISCV/linker-relaxation.s


Index: test/MC/RISCV/linker-relaxation.s
===================================================================
--- test/MC/RISCV/linker-relaxation.s
+++ test/MC/RISCV/linker-relaxation.s
@@ -114,7 +114,7 @@
 
 2:
 auipc t1, %pcrel_hi(bar)
-# NORELAX-RELOC-NOT: R_RISCV_PCREL_HI20
+# NORELAX-RELOC: R_RISCV_PCREL_HI20 bar 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
 # RELAX-RELOC: R_RISCV_PCREL_HI20 bar 0x0
 # RELAX-RELOC: R_RISCV_RELAX - 0x0
@@ -122,7 +122,7 @@
 # RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
 
 addi t1, t1, %pcrel_lo(2b)
-# NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_I
+# NORELAX-RELOC: R_RISCV_PCREL_LO12_I bar 0x4
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
 # RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
 # RELAX-RELOC: R_RISCV_RELAX - 0x0
@@ -130,7 +130,7 @@
 # RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
 
 sb t1, %pcrel_lo(2b)(a2)
-# NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_S
+# NORELAX-RELOC: R_RISCV_PCREL_LO12_S bar 0x8
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
 # RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
 # RELAX-RELOC: R_RISCV_RELAX - 0x0
Index: test/MC/RISCV/fixups.s
===================================================================
--- test/MC/RISCV/fixups.s
+++ test/MC/RISCV/fixups.s
@@ -26,14 +26,17 @@
 
 1:
 auipc t1, %pcrel_hi(.LBB0)
+# CHECK-REL: R_RISCV_PCREL_HI20 .LBB0 0x0
 # 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-REL: R_RISCV_PCREL_LO12_I .LBB0 0x4
 # CHECK-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp0), kind: fixup_riscv_pcrel_lo12_i
-# CHECK-INSTR: addi t1, t1, -16
+# CHECK-INSTR: addi t1, t1, 0
 sw t1, %pcrel_lo(1b)(t1)
+# CHECK-REL: R_RISCV_PCREL_LO12_S .LBB0 0x8
 # CHECK-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp0), kind: fixup_riscv_pcrel_lo12_s
-# CHECK-INSTR: sw t1, -16(t1)
+# CHECK-INSTR: sw t1, 0(t1)
 
 jal zero, .LBB0
 # CHECK-FIXUP: fixup A - offset: 0, value: .LBB0, kind: fixup_riscv_jal
Index: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
===================================================================
--- lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -36,11 +36,14 @@
   case RISCV::fixup_riscv_got_hi20:
   case RISCV::fixup_riscv_tls_got_hi20:
   case RISCV::fixup_riscv_tls_gd_hi20:
-    return true;
+  case RISCV::fixup_riscv_pcrel_hi20:
+      ShouldForce = true;
+      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.
+    // pcrel_lo fixups should always cause relocations to be forced, but we can
+    // do some error checking here anyway to ensure the corresponding fixup
+    // exists and is valid.
     const MCFixup *T = cast<RISCVMCExpr>(Fixup.getValue())->getPCRelHiFixup();
     if (!T) {
       Asm.getContext().reportError(Fixup.getLoc(),
@@ -55,11 +58,8 @@
     case RISCV::fixup_riscv_got_hi20:
     case RISCV::fixup_riscv_tls_got_hi20:
     case RISCV::fixup_riscv_tls_gd_hi20:
-      ShouldForce = true;
-      break;
     case RISCV::fixup_riscv_pcrel_hi20:
-      ShouldForce = T->getValue()->findAssociatedFragment() !=
-                    Fixup.getValue()->findAssociatedFragment();
+      ShouldForce = true;
       break;
     }
     break;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61907.199461.patch
Type: text/x-patch
Size: 3439 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190514/9c211f57/attachment.bin>