[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
Wed Mar 8 10:05:35 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:
> 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).
I prefer avoiding `isText` check. It relies on `.Default(SectionKind::getText());` in `MCContext::getELFSection` which I am still unsure.
In GNU assembler, `.rodata` and `.note` sections use `R_*_{ADD,SUB}*` as well.
The code as-is appears to capture the GNU assembler behavior best, though perhaps there may be some cases that should be rejected while we allow. Hence I say "I suspect we may allow some cases that should be rejected, but this is better than incorrect rejection." in the last sentence of the summary.
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