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

Define GOT-Relative data relocation #402

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

PiJoules
Copy link
Contributor

@PiJoules PiJoules commented Sep 28, 2023

This introduces a new relocation R_RISCV_GOT32_PCREL that follows the existing wording to “R_RISCV_32_PCREL”, but instead evaluates to the 32-bit offset between a GOT entry for a given symbol and the current location where the relocation is applied, so its equation would be “G + GOT - P + A”.

See #399 (comment) for a code example.

@rui314
Copy link
Collaborator

rui314 commented Sep 29, 2023

The format of the table is broken. Check it yourself and please fix the table format. Screenshot 2023-09-29 at 11 14 28

riscv-elf.adoc Outdated Show resolved Hide resolved
Copy link
Collaborator

@rui314 rui314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question that naturally raises would be whether or not we want a 64-bit variant of this relocation, but I guess we don't need it at this moment because even the usual GOT_HI20 and LO12_I combination can't address the entire 64-bit address space.

What is the assembly syntax for this new relocation?

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
@PiJoules
Copy link
Contributor Author

PiJoules commented Oct 3, 2023

I think the formatting should be fixed now

@rui314
Copy link
Collaborator

rui314 commented Oct 4, 2023

s/PC relative/PC-relative/ for the sake of consistency.

Other than that, LGTM

@kito-cheng
Copy link
Collaborator

LGTM, but just require a PoC implementation either LD.BFD or LLD as convention :)

@MaskRay
Copy link
Collaborator

MaskRay commented Nov 22, 2023

I think this extension is fine.

We need a code example for my question on #399 . You can edit the description of the first comment to link to #399

riscv-elf.adoc Outdated Show resolved Hide resolved
@PiJoules
Copy link
Contributor Author

I think this extension is fine.

We need a code example for my question on #399 . You can edit the description of the first comment to link to #399

Done

This introduces a new relocation `R_RISCV_GOT32_PCREL` follows the existing
wording to “R_RISCV_32_PCREL”, but instead evaluates to the 32-bit offset
between a GOT entry for a given symbol and the current location where the
relocation is applied, so its equation would be “G + GOT - P + A”.
@kito-cheng
Copy link
Collaborator

We got LGTM from two linker experts, also PoC for lld llvm/llvm-project#72587

c.c. @Nelson1225 just remind you there are new relocation added.

@kito-cheng kito-cheng merged commit bb6f298 into riscv-non-isa:master Dec 7, 2023
4 checks passed
@PiJoules PiJoules deleted the R_RISCV_GOT32_PCREL branch December 7, 2023 19:12
PiJoules added a commit to PiJoules/llvm-project that referenced this pull request Jan 10, 2024
This is the followup implementation to
riscv-non-isa/riscv-elf-psabi-doc#402 that
supports this relocation in llvm and lld.
PiJoules added a commit to llvm/llvm-project that referenced this pull request Jan 10, 2024
This is the followup implementation to
riscv-non-isa/riscv-elf-psabi-doc#402 that
supports this relocation in llvm and lld.
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This is the followup implementation to
riscv-non-isa/riscv-elf-psabi-doc#402 that
supports this relocation in llvm and lld.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants