[llvm] r357374 - [RISCV] Don't evaluatePCRelLo if a relocation will be forced (e.g. due to linker relaxation)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 31 19:38:27 PDT 2019


Author: asb
Date: Sun Mar 31 19:38:27 2019
New Revision: 357374

URL: http://llvm.org/viewvc/llvm-project?rev=357374&view=rev
Log:
[RISCV] Don't evaluatePCRelLo if a relocation will be forced (e.g. due to linker relaxation)

A pcrel_lo will point to the associated pcrel_hi fixup which in turn points to
the real target. RISCVMCExpr::evaluatePCRelLo will work around this
indirection in order to allow the fixup to be evaluate properly. However, if
relocations are forced (e.g. due to linker relaxation is enabled) then its
evaluation is undesired and will result in a relocation with the wrong target.

This patch modifies evaluatePCRelLo so it will not try to evaluate if the
fixup will be forced as a relocation. A new helper method is added to
RISCVAsmBackend to query this.

Differential Revision: https://reviews.llvm.org/D59686

Modified:
    llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
    llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
    llvm/trunk/test/MC/RISCV/linker-relaxation.s
    llvm/trunk/test/MC/RISCV/option-relax.s

Modified: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h?rev=357374&r1=357373&r2=357374&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h Sun Mar 31 19:38:27 2019
@@ -42,11 +42,18 @@ public:
 
   void setForceRelocs() { ForceRelocs = true; }
 
+  // Returns true if relocations will be forced for shouldForceRelocation by
+  // default. This will be true if relaxation is enabled or had previously
+  // been enabled.
+  bool willForceRelocations() const {
+    return ForceRelocs || STI.getFeatureBits()[RISCV::FeatureRelax];
+  }
+
   // Generate diff expression relocations if the relax feature is enabled or had
   // previously been enabled, otherwise it is safe for the assembler to
   // calculate these internally.
   bool requiresDiffExpressionRelocations() const override {
-    return STI.getFeatureBits()[RISCV::FeatureRelax] || ForceRelocs;
+    return willForceRelocations();
   }
 
   // Return Size with extra Nop Bytes for alignment directive in code section.

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=357374&r1=357373&r2=357374&view=diff
==============================================================================
--- llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (original)
+++ llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp Sun Mar 31 19:38:27 2019
@@ -11,9 +11,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "RISCV.h"
 #include "RISCVMCExpr.h"
+#include "MCTargetDesc/RISCVAsmBackend.h"
+#include "RISCV.h"
 #include "RISCVFixupKinds.h"
+#include "llvm/MC/MCAsmLayout.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCStreamer.h"
@@ -87,6 +89,16 @@ bool RISCVMCExpr::evaluatePCRelLo(MCValu
   // (<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.
+
+  // Don't try to evaluate if the fixup will be forced as a relocation (e.g.
+  // as linker relaxation is enabled). If we evaluated pcrel_lo in this case,
+  // the modified fixup will be converted into a relocation that no longer
+  // points to the pcrel_hi as the linker requires.
+  auto &RAB =
+      static_cast<RISCVAsmBackend &>(Layout->getAssembler().getBackend());
+  if (RAB.willForceRelocations())
+    return false;
+
   MCValue AUIPCLoc;
   if (!getSubExpr()->evaluateAsValue(AUIPCLoc, *Layout))
     return false;

Modified: llvm/trunk/test/MC/RISCV/linker-relaxation.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/linker-relaxation.s?rev=357374&r1=357373&r2=357374&view=diff
==============================================================================
--- llvm/trunk/test/MC/RISCV/linker-relaxation.s (original)
+++ llvm/trunk/test/MC/RISCV/linker-relaxation.s Sun Mar 31 19:38:27 2019
@@ -121,14 +121,10 @@ auipc t1, %pcrel_hi(bar)
 # RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_hi(bar), kind: fixup_riscv_pcrel_hi20
 # RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
 
-# TODO/FIXME: The generated PCREL_LO relocations are incorrect.
-# RISCVMCExpr::evaluatePCRelLo should not be evaluating the fixup when linker
-# relaxation is enabled.
-
 addi t1, t1, %pcrel_lo(2b)
 # NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_I
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_I bar 0x4
+# RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
 # RELAX-RELOC: R_RISCV_RELAX - 0x0
 # RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp1), kind: fixup_riscv_pcrel_lo12_i
 # RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
@@ -136,7 +132,7 @@ addi t1, t1, %pcrel_lo(2b)
 sb t1, %pcrel_lo(2b)(a2)
 # NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_S
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_S bar 0x8
+# RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
 # RELAX-RELOC: R_RISCV_RELAX - 0x0
 # RELAX-FIXUP: fixup A - offset: 0, value: %pcrel_lo(.Ltmp1), kind: fixup_riscv_pcrel_lo12_s
 # RELAX-FIXUP: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax

Modified: llvm/trunk/test/MC/RISCV/option-relax.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/RISCV/option-relax.s?rev=357374&r1=357373&r2=357374&view=diff
==============================================================================
--- llvm/trunk/test/MC/RISCV/option-relax.s (original)
+++ llvm/trunk/test/MC/RISCV/option-relax.s Sun Mar 31 19:38:27 2019
@@ -64,3 +64,9 @@ call baz
 jal zero, .L1
 # CHECK-RELOC-NEXT: R_RISCV_BRANCH
 beq s1, s1, .L1
+
+1:
+# CHECK-RELOC-NEXT: R_RISCV_PCREL_HI20 .L1
+auipc t1, %pcrel_hi(.L1)
+# CHECK-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp0
+addi t1, t1, %pcrel_lo(1b)




More information about the llvm-commits mailing list