[PATCH] D159082: [ELF][RISCV] Implement --emit-relocs with relaxation

Fri Sep 8 01:41:00 PDT 2023

jobnoorman added a comment.

In D159082#4641491 <https://reviews.llvm.org/D159082#4641491>, @MaskRay wrote:

> Opening an https://sourceware.org/ issue is useful but I do believe R_RISCV_RELAX=>R_RISCV_NONE on our side is unnecessary.

Agreed.

> --emit-relocs is a debug or advanced feature. Giving more information by preserving the original type has some value. The R_RISCV_RELAX=>R_RISCV_NONE write is unneeded complexity, albeit small.

I assume you meant `R_RISCV_ALIGN` => `R_RISCV_NONE` here? I've reverted the patch to its former behavior (emitting `R_RISCV_ALIGN`) in any case.

================
Comment at: lld/ELF/InputSection.cpp:355
+  if (config->relax && !config->relocatable && config->emachine == EM_RISCV) {
+    // On RISC-V, relaxation might change relocations so we cannot simply copy
+    // from the input section.
----------------
MaskRay wrote:
> jrtc27 wrote:
> > Isn’t that true of other architectures though? TLS relaxations, GOT->PCREL relaxation, …
> Perhaps Cause and Effect should not be used here. I.e. remove "so", and just state what we do.
Updated, is this what you had in mind?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159082