[PATCH] D145474: [RISCV][MC] Adjust conditions to emit R_RISCV_ADD*/R_RISCV_SUB* pairs

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:232
+  if (B.isInSection())
+    return !IsEHOrAppleSection(B.getSection());
 
----------------
MaskRay wrote:
> compnerd wrote:
> > This really seems like the original cases were correct but need to be augmented further to handle data sections.  If there is a cross-section textual reference, that should generate the relocation pair.  The data section is interesting because the `.eh_frame`, `.apple_names`, `.apple_types`, and `.debug_*` do not want the pairwise relocation.  Adjusting this to handle it that way would not only fix this, but also likely fix the cases of fission that we currently fail on.
> I think we want to avoid reliance on `isText()` (`Kind` as set by `MCContext::getELFSection`). What are the changes you envision?
> 
> I think it's right to hard code `.eh_frame`, `.apple_names`, and `.apple_types` for now, and don't try using a generic rule to handle them. `.eh_frame` may possibly be removed if we can fix `MCDwarf.cpp` (not sure how much effort it requires).
I think that we should prefer `isText` over the section name checks.  RISCV seems to want to prefer the pair wise relocation in all "text" cases and in very specific data cases (as `.debug_*`, `.eh_frame`, `.apple_names`, `.apple_types` etc are all data sections where we don't want the pair wise relocation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145474