riscv-non-isa / riscv-elf-psabi-doc

A RISC-V ELF psABI Document
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
663 stars 158 forks source link

Add GP-relative relocations #394

Closed luismarques closed 9 months ago

luismarques commented 11 months ago

Following-up on @kito-cheng's suggestion to add the ePIC proposal content to the existing psABI documents, this initial PR adds the GP-relative relocations. These GP-relocations should be useful for purposes besides ePIC (e.g. see previous proposals of the compact/large code models).

For ease of review, I will make a separate PR to add R_RISCV_GPREL_ADD, used for relaxation purposes, unless you prefer otherwise.

I'm not sure these GP relocations merit a GP-relative addressing section, such as the existing "PC-Relative Symbol Addresses", as there aren't the same kind of complications to comment on. If you feel it's needed, I will add one to this PR.

kito-cheng commented 11 months ago

Did you mind post the link of the lld or ld.bfd implementation for the process in the psABI - we require PoC for either linker :)

luismarques commented 11 months ago

Did you mind post the link of the lld or ld.bfd implementation for the process in the psABI - we require PoC for either linker :)

Here's a link to the first implementation I am aware of of these GPREL relocations, by Evandro and Nelson / SiFive, for the bfd linker: https://github.com/ebahapo/riscv-binutils-gdb/commit/615efc0c960b15eebaf20dde6253283f40cc3d7e.

kito-cheng commented 11 months ago

cc @Nelson1225 @MaskRay linker experts

Nelson1225 commented 11 months ago

LGTM, too. It's good to see this series is still going on :)

luismarques commented 10 months ago

cc @Nelson1225 @MaskRay linker experts

ping @MaskRay and other linker experts

rui314 commented 10 months ago

This is looking good, but we are not going to use these relocations without relaxation because the size matters for the use case, no? If so, I think we want to define the relaxation scheme at the same time.

luismarques commented 10 months ago

This is looking good, but we are not going to use these relocations without relaxation because the size matters for the use case, no? If so, I think we want to define the relaxation scheme at the same time.

I have a use case where relaxation is not essential. I've decided to split relaxation into a separate PR for ease of review but if you really want I can extend this PR.

rui314 commented 10 months ago

I feel more comfortable with relaxation because we want to make sure that these relocations are actually relaxable. But I think that's not really a concern, as they can obviously be relaxable in the same way as R_RISCV_TPREL_* relocs. So I'm fine with this. What do you think @MaskRay?

kito-cheng commented 9 months ago

I am OK to add relaxation later, let moving forward :)

sorear commented 5 months ago

I'd like to revert this before it gets into a tagged release. Neither ePIC nor FDPIC will use it except possibly for linker-internal purposes after relaxation, and we don't assign numbers to linker-internal relocation types. My apologies for missing this earlier.

MaskRay commented 5 months ago

I'd like to revert this before it gets into a tagged release. Neither ePIC nor FDPIC will use it except possibly for linker-internal purposes after relaxation, and we don't assign numbers to linker-internal relocation types. My apologies for missing this earlier.

I think S + A - GP can be used for non-preemptible data symbols (*_symbol_binds_local_p in GCC) with FDPIC, which is the reason I think this PR is acceptable.

I am indeed concerned with the rest part of ePIC, so I cannot sign off on them.

sorear commented 5 months ago

You can only use it for non-preemptible symbols if you know for a fact at the reference site that the referenced symbol is allocated to writable memory. I don't think it's going to get anywhere near the amount of usage required to justify 1/48 of the RV32 relocation space.

MaskRay commented 5 months ago

You can only use it for non-preemptible symbols if you know for a fact at the reference site that the referenced symbol is allocated to writable memory. I don't think it's going to get anywhere near the amount of usage required to justify 1/48 of the RV32 relocation space.

I agree with you. I have also checked GCC arm and sh4's codegen for -mfdpic. I apologize for accepting this previously. Since no upstream toolchain/libc has ever used these relocation types, it should be fine to revert the change.

GOT-indirect addressing is required for accessing data symbols under two conditions:

If the referenced data symbol is non-preemptible and guaranteed to be in the text segment, we can use PC-relative addressing. However, this scenario is remarkably rare in practice. The most reasonable use case is like the following:

const int ro_array[] = {1, 2, 3, 4}; // text segment

int get_ro_array_elem(int i) { return ro_array[i]; }

GOT-indirect addressing does not necessarily require GOT. The linker can optimize it, rendering the 3 relocation code less useful.

sorear commented 4 months ago

Revert PR is #427.