[PATCH] D153097: [RISCV] Make linker-relaxable instructions terminate MCDataFragment

Thu Jun 29 09:27:32 PDT 2023

MaskRay added a comment.

In D153097#4460146 <https://reviews.llvm.org/D153097#4460146>, @barannikov88 wrote:

> In D153097#4459780 <https://reviews.llvm.org/D153097#4459780>, @asb wrote:
>
>> I've stepped through the patch description and the code - both of which make sense to me. I'm not familiar enough with the code to be certain there's not an alternative way of achieving the same thing that would be better....but then I'd normally looking to @maskray for such suggestions :) LGTM, just left a minor query regarding tweaking a doc comment.
>
> This was my main concern. The patch assumes there is only one possible fixup that may need linker relaxation, and only supported on ELF targets.
> I can imagine that RISC-V could be supported by non-unix OS, or that more fixups needing linker relaxation can be added, possibly by other targets.
> I hoped my concern could be addressed, but I might as well be nitpicking, as it can easily be addressed in the future.

Thank you for your thoughts. I think our bikeshedding on this point is a fairly minor thing... The two object file formats LLVM supports that will mostly likely add RISC-V support (even when we say "most likely", it can be multiple years) are Mach-O and PE/COFF.

Mach-O has 16 available r_type values, which are very limited, (along with other assembler issues and subsections semantics) make it fairly difficult to add relaxation based on a relocation type.
If they do enable relaxation, they'll likely use `.loh` directive, with is entirely independent of this relocation based mechanism.

PE/COFF has an uint16_t relocation type field. If it does add RISC-V relaxation, it can reuse a relocation type for this purpose.
However, it does not provide an addend field. I haven't thought much about what this will entail, but I think adding relaxation could be quite challenging as well.

Anyhow, even if Mach-O and PE/COFF add RISC-V support and enable relaxation, the mechanism will be quite unclear.
The MC code may require larger changes.
For now, I am not comfortable adding some abstraction to make reasoning about the code slightly more difficult.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153097