[PATCH] D127549: RISCV: handle 64-bit PCREL data relocations

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:235
+    if (A.isInSection())
+      return !IsDebugOrEHFrameSection(A.getSection());
+    if (B.isInSection())
----------------
isInSection() checks defined non-absolute symbols. I think all the conditions here are to make MCDwarf.cpp use constants/32_PCREL if applicable.

Can `S.getKind().isMetadata()` unnecessarily match non-MCDwarf created sections?

Note that we have other functions (e.g. `isTemporary`) which are usable but I haven't thought hard for the best condition here.


================
Comment at: llvm/test/MC/RISCV/riscv64-64b-pcrel.s:9
+# CHECK:  .relasy {
+# CHECK-NEXT:    0x0 R_RISCV_ADD64 x 0x0
+# CHECK-NEXT:    0x0 R_RISCV_SUB64 y 0x0
----------------
ADD/SUB are for labels. The definition isn't clear in the psABI, but in spirit they should be https://sourceware.org/binutils/docs/as/Symbol-Names.html local labels. At the very least they need to be STB_LOCAL.

Supporting STB_GLOBAL is fine as GNU as supports such subtraction for STB_GLOBAL for various ports but they can use a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127549