Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISC-V psABI doesn't seem to allow relaxation of TLS GD #308

Open
rui314 opened this issue Aug 1, 2022 · 5 comments
Open

RISC-V psABI doesn't seem to allow relaxation of TLS GD #308

rui314 opened this issue Aug 1, 2022 · 5 comments

Comments

@rui314
Copy link
Collaborator

rui314 commented Aug 1, 2022

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#global-dynamic

In TLS GD access model, the address of a thread-local variable is obtained using three instructions

    auipc a0,0                    # R_RISCV_TLS_GD_HI20 (symbol)
    addi  a0,a0,0                 # R_RISCV_PCREL_LO12_I (label)

    call  __tls_get_addr@plt

However, if a linker knows the the thread-local variable's offset from tp, it can relax the instructions by setting the offset directly to a0 and replaces call ___tls_get_addr@plt with a nop.

The problem here is this: in this RISC-V psABI, there seems to be no way to find a call instruction for an auipc with a R_RISCV_TLS_GD_HI20 relocation. It doesn't mandate that the call instruction immediately follow the addi instruction, so the call can be placed at any place after addi (as long as a0 is preserved).

As a result, it's impossible to relax TLS GD to TLS LE. All TLS accesses has to be resolved using a function call at runtime which is pretty expensive.

@kito-cheng
Copy link
Collaborator

kito-cheng commented Aug 1, 2022

My first impression is adding a R_RISCV_PCREL_LO12_I-like relocation maybe R_RISCV_TLS_CALL_LINK which point to label, then linker will know the auipc linked with addi and call?

Need inputs from all other linker hackers @MaskRay @jrtc27 @Nelson1225

label:
    auipc a0,0                    # R_RISCV_TLS_GD_HI20 (symbol)
    addi  a0,a0,0                 # R_RISCV_PCREL_LO12_I (label)

    call  __tls_get_addr@plt # R_RISCV_TLS_CALL_LINK(label) R_RISCV_CALL_PLT(__tls_get_addr)

@rui314
Copy link
Collaborator Author

rui314 commented Aug 1, 2022

If we have to make a change to the toolchain to fix this problem, I believe the best approach is to define TLSDESC (#94) and implement it instead, and keep the existing TLSGD as-is for backward compatibility.

@MaskRay
Copy link
Collaborator

MaskRay commented Aug 1, 2022

I think we just need TLSDESC (#94) and initial-exec to local-exec optimization (#122). We don't need to add optimizations to TLS GD which will be superseded by TLSDESC.

@aswaterman
Copy link
Contributor

Agreed with @rui314 and @MaskRay.

@Nelson1225
Copy link
Collaborator

Nelson1225 commented Aug 2, 2022

I think we just need TLSDESC (#94) and initial-exec to local-exec optimization (#122).

From my side, this is the right way to do. There are many PRs in the sourceware for this topic, https://sourceware.org/bugzilla/show_bug.cgi?id=27953. Just that I am not sure if we should do these while relaxing, or just do these while relocating. Maybe we also can do these while relaxing, but like the alignment relaxation that cannot be disabled.

label:
    auipc a0,0                    # R_RISCV_TLS_GD_HI20 (symbol)
    addi  a0,a0,0                 # R_RISCV_PCREL_LO12_I (label)

    call  __tls_get_addr@plt # R_RISCV_TLS_CALL_LINK(label) R_RISCV_CALL_PLT(__tls_get_addr)

Yeah, if there are any patterns that need to chain all the related instructions, then currently define a new relocation like pcrel_lo is the way, or refer to the corresponding symbol directly is another way (like R_RISCV_TLS_ADD). Personally I prefer the later. BTW, for this case, we will have two relocations for the call, R_RISCV_CALL_PLT and R_RISCV_TLS_CALL_LINK (label or symbol), so make sure they are both together should be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants