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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 7 12:13:46 PST 2023


MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:232
+  if (B.isInSection())
+    return !IsEHOrAppleSection(B.getSection());
 
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145474



More information about the llvm-commits mailing list