[PATCH] D132262: RISCV: adjust relocation emission

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd updated this revision to Diff 462282.
compnerd added a comment.

Stylistic changes suggested by @MaskRay offline.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132262/new/

https://reviews.llvm.org/D132262

Files:
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/test/MC/RISCV/riscv64-64b-pcrel.s


Index: llvm/test/MC/RISCV/riscv64-64b-pcrel.s
===================================================================
--- llvm/test/MC/RISCV/riscv64-64b-pcrel.s
+++ llvm/test/MC/RISCV/riscv64-64b-pcrel.s
@@ -1,5 +1,6 @@
-# RUN: llvm-mc -triple riscv64-unknown-linux-gnu -filetype obj -o - %s \
+# RUN: not llvm-mc -triple riscv64-unknown-linux-gnu -filetype obj -o - %s 2>%t.log \
 # RUN:   | llvm-readobj -r - | FileCheck %s
+# RUN: FileCheck -check-prefix CHECK-ERROR %s < %t.log
 
 # CHECK: Relocations [
 # CHECK:  .relasx {
@@ -10,10 +11,6 @@
 # CHECK-NEXT:    0x0 R_RISCV_ADD64 x 0x0
 # CHECK-NEXT:    0x0 R_RISCV_SUB64 y 0x0
 # CHECK:  }
-# CHECK:  .relasz {
-# CHECK-NEXT:    0x0 R_RISCV_ADD64 z 0x0
-# CHECK-NEXT:    0x0 R_RISCV_SUB64 a 0x0
-# CHECK:  }
 # CHECK:  .relasa {
 # CHECK-NEXT:    0x0 R_RISCV_ADD64 a 0x0
 # CHECK-NEXT:    0x0 R_RISCV_SUB64 z 0x0
@@ -32,6 +29,10 @@
 z:
 	.quad z-a
 
+# CHECK-ERROR: Cannot represent a difference across sections
+# CHECK-ERROR: 	.quad z-a
+# CHECK-ERROR:        ^
+
 	.section	sa
 a:
 	.quad a-z
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
===================================================================
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -209,14 +209,6 @@
 
   static bool requiresFixups(MCContext &C, const MCExpr *Value,
                              const MCExpr *&LHS, const MCExpr *&RHS) {
-    auto IsMetadataOrEHFrameSection = [](const MCSection &S) -> bool {
-      // Additionally check .apple_names/.apple_types. They are fixed-size and
-      // do not need fixups. llvm-dwarfdump --apple-names does not process
-      // R_RISCV_{ADD,SUB}32 in them.
-      return S.getKind().isMetadata() || S.getName() == ".eh_frame" ||
-             S.getName() == ".apple_names" || S.getName() == ".apple_types";
-    };
-
     const auto *MBE = dyn_cast<MCBinaryExpr>(Value);
     if (MBE == nullptr)
       return false;
@@ -235,15 +227,21 @@
                              MCConstantExpr::create(E.getConstant(), C), C);
     RHS = E.getSymB();
 
-    // TODO: when available, R_RISCV_n_PCREL should be preferred.
-
-    // Avoid pairwise relocations for symbolic difference in debug and .eh_frame
-    if (A.isInSection())
-      return !IsMetadataOrEHFrameSection(A.getSection());
-    if (B.isInSection())
-      return !IsMetadataOrEHFrameSection(B.getSection());
-    // as well as for absolute symbols.
-    return !A.getName().empty() || !B.getName().empty();
+    // If either symbol is in a text section, we need to delay the relocation
+    // evaluation as relaxation may alter the size of the symbol.
+    //
+    // Unfortunately, we cannot identify if the symbol was built with relaxation
+    // as we do not track the state per symbol or section.  However, BFD will
+    // always emit the relocation and so we follow suit which avoids the need to
+    // track that information.
+    if (A.isInSection() && A.getSection().getKind().isText())
+      return true;
+    if (B.isInSection() && B.getSection().getKind().isText())
+      return true;
+
+    // Support cross-section symbolic differences ...
+    return A.isInSection() && B.isInSection() &&
+           A.getSection().getName() != B.getSection().getName();
   }
 
   void reset() override {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132262.462282.patch
Type: text/x-patch
Size: 3333 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220922/ce2f1d31/attachment.bin>