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

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:235
+    if (A.isInSection())
+      return !IsDebugOrEHFrameSection(A.getSection());
+    if (B.isInSection())
----------------
MaskRay wrote:
> compnerd wrote:
> > MaskRay wrote:
> > > 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.
> > I don't think that `isTemporary` works.  `isMetadata` shouldn't include anything that is marked as progbits or is part of the text or data segments.
> If the condition is match MCDwarf.cpp produced sections, testing section prefix `.debug` will be more specific.
> 
> isMetadata() catches a lot of non-debug non-SHF_ALLOC sections.
I do agree that doing a prefix-match for `.debug` is likely to be more conservative in what sections are excluded, but I am trying to be more conservative about generating the pairwise relocation.  Avoiding the pair-wise relocations in non-SHF_ALLOC doesn't seem like a bad thing to me. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127549