[llvm] [RISCV] Xqcilb: rename JUMP_PLT fixup and drop `@plt` parsing (PR #135507)

[Fangrui Song via llvm-commits]

MaskRay wrote:

> Thanks for bearing with me, I was travelling and ABIs and Jet Lag are not a great mix.
> 
> I sort of see where you're coming from here, but I do actually want to push back a little.
> 
> (As with the ABI, I'm dropping the R_RISCV_ prefix when referring to relocations in this comment because it is annoying to type).
> 
> Originally, there were two relocations for call sequences, so one of them needed a specifier, which was chosen to be @plt for the CALL_PLT relocation. This is the last point the situation is straightforward.
> 
> Then you pointed out the issues with the CALL relocation, so it was deprecated, and we then ended up with two syntaxes to get the CALL_PLT relocation: the one without a specifier, and the one with the (originally needed, now redundant) @plt specifier.
> 
> I would argue that in this circumstance, the @plt specifier still does have some value! Writing call foo at plt means a reader will not expect this instruction to jump directly to foo, they should expect some linker generated code to "get in the way" (unless the linker can optimise it out).

No... The `@plt` specifier is misleading; a PLT is not assured. The symbol may be hidden or defined or -Bsymbolic, and this violates RISC-V's `%specifier(expr)` convention.

MCParser's @ parsing is problematic. Supporting target variations like (`foo+2 at plt` `foo at plt+2` `(foo+2)@plt)  involves messy hacks.

> I aimed with qc.e.jal and qc.e.j to avoid falling into the same trap around relocations, so I have only designed a _PLT relocation. By your argument, only one relocation applying to the instruction means no specifier is needed. However, I do think that the ability for users to write qc.e.jal foo at plt has some value, so they can remind themselves when reading their code that they shouldn't expect a pc-relative jump directly to foo, instead there is something a little more complex happening (unless the linker can optimise it out).

> I know that in Arm/AArch64 land, the ABI has to be fairly clear about which relocations are allowed to use a (range extension) veneer/thunk, and which aren't, because they clobber some caller-saved registers. We don't have range-extension veneers/thunks in the same way for RISC-V, but right now I think that CALL_PLT is the only standard relocation that has "permission" to use a PLT entry, which in my mind is similar enough to a veneer/thunk because it clobbers caller-save registers. Using the _PLT suffix on the name of the QC_E_JUMP_PLT relocation was an indicator of the fact that rule also applied to these relocations. I don't think that the linker is allowed to introduce a PLT entry for e.g. a JAL relocation, because that might be applied to a jump not at a function-call boundary, when the registers clobbered by a PLT entry are still live (Linker relaxation makes this a bit more complex, because JAL relocation could point at the address of a PLT, but only after relaxation when the before-relaxation sequence was relocated with CALL_PLT, which is the thing that had the permission to introduce the PLT entry).
> 
> I don't really mind what the fixup is called, though I'd like it to echo the relocation name, as I think that makes maintenance/understanding much easier.

I get your point, but two syntaxes for a single relocation are misleading.

The only public doc I can find is https://github.com/quic/riscv-elf-psabi-quic-extensions/blob/main/src/quic-extensions.adoc  , which specifies that this instruction is for procedure calls.
`@plt`'s value is minimal compared to the confusion and deviation from modern ABI procedure call conventions (no relocation specifier).

For instance, PPC32 used three redundant relocation types, R_PPC_LOCAL24PC, R_PPC_REL24, and R_PPC_PLTREL24, all with identical semantics.
Recognizing the issue, PPC64 unified them into a single type, R_PPC64_REL24.

That said, if you believe that qc.e.jal might be extended for non-procedure-call purposes and a specifier is important as a new one might be introduced,
I suggest that changing `@plt` to something like `%plt(expr)` and dropping the support for a bare symbol without a relocation specifier.


https://github.com/llvm/llvm-project/pull/135507