Relaxable TLSDESC only requires that R_RISCV_TLSDESC_HI20 to be paired with R_RISCV_RELAX

[MaskRay]

Requiring 4 R_RISCV_RELAX relocations impose a large size increase to the relocatable file. To mitigate this size increase, we can say that the whole TLSDESC code sequence is relaxable if the first instruction (R_RISCV_TLSDESC_HI20) is paired with R_RISCV_RELAX.

A statically linked executable typically has a simple relocation resolver that handles just RELATIVE/IFUNC. For a -fpic -mtls-dialect=desc relocatable file, the linker is required to perform TLS optimization to local-exec for a statically linked executable. Therefore, instruction rewriting is essentially inevitable if the relocation resolver is kept simple. Ths general-dynamic TLS model, without a defined optimization scheme, actually has the same issue.

[MaskRay]

@ilovepi @ishitatsuyuki

My lld/ELF implementation checks whether the first instruction (R_RISCV_TLSDESC_HI20) has R_RISCV_RELAX
llvm/llvm-project#79239

On the LLVM side, I want to push for emitting one R_RISCV_RELAX as well.
If people say "this is not conforming", I think I'd like a mode to remove the redundant R_RISCV_RELAX.

We don't need to do the same as R_RISCV_TPREL_HI20.

[ishitatsuyuki]

Looks sensible. I think mold works without the RELAX annotations on paired relocs too, but I'll need to double check.

[rui314]

I think that this issue is red herring because the number of RELAX relocations for TLSDESC is probably fewer than 0.1% of those for R_RISCV_HI20 and R_RISCV_LO12_I. To solve it, I believe we should really consider something like #401. I'm not opposed to this proposal, but I'm skeptical about whether this change would make any meaningful difference.

[MaskRay]

I think that this issue is red herring because the number of RELAX relocations for TLSDESC is probably fewer than 0.1% of those for R_RISCV_HI20 and R_RISCV_LO12_I. To solve it, I believe we should really consider something like #401. I'm not opposed to this proposal, but I'm skeptical about whether this change would make any meaningful difference.

I made a reply: #401 (comment)

I think we can ensure that R_RISCV_RELAX causes less harm (size increase) for newer relaxable code sequences like TLSDESC.

[kito-cheng]

For those kind of chained relocation, only the first relocation is enough to represent it's relaxable.

R_RISCV_PCREL_HI20/R_RISCV_PCREL_LO12_[I|S] may be the candidate, however I realized we really need a ABI version tag before doing those not fully compatible changes.

And another candidate for this idea is the long instruction sequence version of large code model I think :)

[MaskRay]

Since folks agree that one R_RISCV_RELAX is good enough, I just rebased (to resolve a conflict) and removed

+The `R_RISCV_TLSDESC_LOAD_LO12`, `R_RISCV_TLSDESC_ADD_LO12`, and
+`R_RISCV_TLSDESC_ADD_LO12` relocations may optionally be paired with a
+`R_RISCV_RELAX`.

I added them just in case folks wanted it...

I assume that @ishitatsuyuki will update the gas patch to not emit 4 R_RISCV_RELAX.

[MaskRay]

Can this be merged? :)

[rui314]

LGTM