This is an archive of the discontinued LLVM Phabricator instance.

[JITLink][RISCV] Support R_RISCV_SET* and R_RISCV_32_PCREL relocations
ClosedPublic

Authored by fourdim on Jan 11 2022, 7:23 PM.

Details

Summary

This patch supports R_RISCV_SET* and R_RISCV_32_PCREL relocations in JITLink.

Diff Detail

Event Timeline

fourdim created this revision.Jan 11 2022, 7:23 PM
fourdim requested review of this revision.Jan 11 2022, 7:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2022, 7:23 PM

You can write tests just fine in assembly. Use .reloc if needed. You don't need .eh_frame for that.

llvm/include/llvm/ExecutionEngine/JITLink/riscv.h
87

These are all missing a /

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
251

These all operate on an 8/16/32-bit value (SET6 operates on the low 6 bits of an 8-bit value). These do not relocate instructions, they relocate data. Again, please look at LLD before implementing things rather than guessing.

276

This is not an R_RISCV_SET* relocation, yet the commit message says that's that is added

276

*that's all that is added

fourdim retitled this revision from [JITLink][RISCV] Support R_RISCV_SET relocations to [JITLink][RISCV] Support R_RISCV_SET* and R_RISCV_32_PCREL relocations.Jan 12 2022, 8:03 PM
fourdim edited the summary of this revision. (Show Details)

You can write tests just fine in assembly. Use .reloc if needed. You don't need .eh_frame for that.

Sorry for my stupid mistakes.
I'm planning to upload a new diff, however, I'm still confused about the .reloc.
It appears that it only adds a R_RISCV_SET* relocation in the binary.
I'm checking lld/test/ELF/riscv-reloc-add.s and D63183, and got no idea on how to set the value.
Particularly, D63183 says,

Note llvm-mc cannot currently produce R_RISCV_SET* so they are not tested.

And the test case now still does not add R_RISCV_SET* test.
Would you be so kind to give some hints on how to test it?

jrtc27 added a comment.EditedJan 12 2022, 8:30 PM

You can write tests just fine in assembly. Use .reloc if needed. You don't need .eh_frame for that.

Sorry for my stupid mistakes.
I'm planning to upload a new diff, however, I'm still confused about the .reloc.
It appears that it only adds a R_RISCV_SET* relocation in the binary.
I'm checking lld/test/ELF/riscv-reloc-add.s and D63183, and got no idea on how to set the value.
Particularly, D63183 says,

Note llvm-mc cannot currently produce R_RISCV_SET* so they are not tested.

And the test case now still does not add R_RISCV_SET* test.
Would you be so kind to give some hints on how to test it?

That comment likely predates .reloc support for RISC-V. You use .reloc like:

    .reloc 1f, R_RISCV_SET6, foo
1:  .byte 0

i.e. it takes the location to relocate, the relocation name and the value to use

fourdim updated this revision to Diff 399694.Jan 13 2022, 9:08 AM

Add test cases and address the comments.

fourdim updated this revision to Diff 402195.Jan 22 2022, 2:15 AM
fourdim marked 4 inline comments as done.

Include the recent commits.

This revision is now accepted and ready to land.Jan 22 2022, 2:30 AM
This revision was landed with ongoing or failed builds.Jan 22 2022, 2:32 AM
This revision was automatically updated to reflect the committed changes.