[PATCH] D44887: [RISCV] Add shouldForceRelocationWithApplyFixup MC AsmBackend Target Hook

[Shiva Chen via Phabricator via llvm-commits]

shiva0217 updated this revision to Diff 142343.
shiva0217 retitled this revision from "[RISCV] Add shouldForceRelocationWithApplyFixup Target Hook to calculate fixup value but also leave relocation types" to "[RISCV] Add shouldForceRelocationWithApplyFixup MC AsmBackend Target Hook".
shiva0217 edited the summary of this revision.
shiva0217 added a comment.

We try to avoid RISCV MC Branch Relaxation always promote 16-bit branches to 32-bit form by override fixupNeedsRelaxationAdvanced. (https://reviews.llvm.org/D44971) It turns out that there's still some corner case we can't cover. So we would like to revive the patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D44887

Files:
  include/llvm/MC/MCAsmBackend.h
  lib/MC/MCAssembler.cpp
  lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  test/MC/RISCV/compressed-relocations.s
  test/MC/RISCV/rv32-relaxation.s
  test/MC/RISCV/rv64-relaxation.s


Index: test/MC/RISCV/rv64-relaxation.s
===================================================================
--- test/MC/RISCV/rv64-relaxation.s
+++ test/MC/RISCV/rv64-relaxation.s
@@ -1,5 +1,7 @@
 # RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c < %s \
 # RUN:     | llvm-objdump -d -riscv-no-aliases - | FileCheck -check-prefix=INSTR %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c,+relax < %s \
+# RUN:     | llvm-objdump -d -riscv-no-aliases - | FileCheck -check-prefix=INSTR %s
 
 FAR_JUMP_NEGATIVE:
   c.nop
Index: test/MC/RISCV/rv32-relaxation.s
===================================================================
--- test/MC/RISCV/rv32-relaxation.s
+++ test/MC/RISCV/rv32-relaxation.s
@@ -1,5 +1,7 @@
 # RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c < %s \
 # RUN:     | llvm-objdump -d -riscv-no-aliases - | FileCheck -check-prefix=INSTR %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c,+relax < %s \
+# RUN:     | llvm-objdump -d -riscv-no-aliases - | FileCheck -check-prefix=INSTR %s
 
 FAR_JUMP_NEGATIVE:
   c.nop
Index: test/MC/RISCV/compressed-relocations.s
===================================================================
--- test/MC/RISCV/compressed-relocations.s
+++ test/MC/RISCV/compressed-relocations.s
@@ -2,6 +2,8 @@
 # RUN:     | FileCheck -check-prefix=INSTR -check-prefix=FIXUP %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c < %s \
 # RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELOC %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c,+relax < %s \
+# RUN:     | llvm-readobj -r | FileCheck -check-prefix=RELOC %s
 
 # Check prefixes:
 # RELOC - Check the relocation in the object.
Index: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
===================================================================
--- lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -44,9 +44,12 @@
   createObjectWriter(raw_pwrite_stream &OS) const override;
 
   // Preserve relocation types if the linker relaxation enabled. So then
-  // linker could fixup offsets while relaxation.
-  bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
-                             const MCValue &Target) override {
+  // linker could fixup offsets while relaxation. We couldn't preserve
+  // relocation types by define shouldForceRelocation because it will make
+  // RISCV MC Branch Relaxation always promote 16-bit branches to 32bit form.
+  bool shouldForceRelocationWithApplyFixup(const MCAssembler &Asm,
+                                           const MCFixup &Fixup,
+                                           const MCValue &Target) override {
     return STI.getFeatureBits()[RISCV::FeatureRelax];
   }
 
Index: lib/MC/MCAssembler.cpp
===================================================================
--- lib/MC/MCAssembler.cpp
+++ lib/MC/MCAssembler.cpp
@@ -683,6 +683,16 @@
     // 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);
+  } else if (Backend.shouldForceRelocationWithApplyFixup(*this, Fixup, Target)) {
+    // For RISCV branch instructions, we need to preserve relocation types, so
+    // that linker relaxation could modify offset. If we preserve relocation
+    // by define shouldForceRelocation, IsResolved will always false which will
+    // make RISCV MC Branch Relaxation always relax 16-bit branches to 32-bit
+    // form.
+    // RISCV MC Branch Relaxation is needed because RISCV could perform 32-bit
+    // to 16-bit transformation in MC layer.
+    uint64_t Value = 0;
+    getWriter().recordRelocation(*this, Layout, &F, Fixup, Target, Value);
   }
   return std::make_tuple(Target, FixedValue, IsResolved);
 }
Index: include/llvm/MC/MCAsmBackend.h
===================================================================
--- include/llvm/MC/MCAsmBackend.h
+++ include/llvm/MC/MCAsmBackend.h
@@ -76,6 +76,13 @@
     return false;
   }
 
+  /// Same as shouldForceRelocation but also apply Fixup.
+  virtual bool shouldForceRelocationWithApplyFixup(const MCAssembler &Asm,
+                                                   const MCFixup &Fixup,
+                                                   const MCValue &Target) {
+    return false;
+  }
+
   /// Apply the \p Value for given \p Fixup into the provided data fragment, at
   /// the offset specified by the fixup and following the fixup kind as
   /// appropriate. Errors (such as an out of range fixup value) should be


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44887.142343.patch
Type: text/x-patch
Size: 4585 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180413/fc0e097d/attachment-0001.bin>