This is an archive of the discontinued LLVM Phabricator instance.

[JITLink][RISCV] Homogenize immediate handling
ClosedPublic

Authored by Hahnfeld on Jan 1 2023, 10:12 AM.

Details

Summary

Name the variables based on which part of the immediate value is
extracted, as it was already done for R_RISCV_JAL. This makes it
much easier to compare the logic with the spec.
Also only take the lower 12 bits of RawInstr for R_RISCV_JAL.

Diff Detail

Event Timeline

Hahnfeld created this revision.Jan 1 2023, 10:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 1 2023, 10:12 AM
Hahnfeld requested review of this revision.Jan 1 2023, 10:12 AM

I have to confess that the previous way of specifying the arguments to extractBits (before rG95b74d4db0686a8d55fdae1af4e985ea52b2c572) was slightly more convenient because we could just copy from the spec. I personally find this new variable naming more logical, but please let me know what you think. Ideally, we could either share this immediate handling with LLD or directly extract it from the instruction encoding that LLVM knows about anyway.

StephenFan accepted this revision.Jan 1 2023, 8:16 PM

LGTM! Thanks!

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
227–228

It's better to change this in another commit.

This revision is now accepted and ready to land.Jan 1 2023, 8:16 PM
Hahnfeld added inline comments.Jan 2 2023, 1:26 AM
llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
227–228

You mean the change to RawInstr & 0xFFF in a separate commit? Will do.