This is an archive of the discontinued LLVM Phabricator instance.

[RuntimeDyld][ELF] Add minimal RISC-V support
AbandonedPublic

Authored by jobnoorman on Mar 9 2023, 5:55 AM.

Details

Summary

This patch is part of implementing initial RISC-V support in BOLT.

I understand from other discussions (e.g. D127842, D98560) that JITLink
might be preferred over RuntimeDyld in general. However, since BOLT
still uses RuntimeDyld, adding minimal RISC-V support to it seems the
easiest way forward for now.

The following relocations are implemented:

  • R_RISCV_CALL(_PLT)
  • R_RISCV_GOT_HI20
  • R_RISCV_PCREL_HI20
  • R_RISCV_PCREL_LO12_I
  • R_RISCV_32_PCREL
  • R_RISCV_ADD32
  • R_RISCV_SUB32

Most are fairly straightforward. The exception is R_RISCV_PCREL_LO12_I,
which is a relocation that refers to a corresponding *HI20 one (i.e.,
the symbol it refers to points to an instruction with a *HI20
relocation). In order to implement this, a member (PCRelHIRelocs) was
added to the RuntimeDyldELF class that maps offsets with a *HI20
relocation to their target address. Whenever a LO12 relocations is
encountered, this map is queried in order to find its target. Note that
this assumes that *HI20 relocations are processed before the
corresponding LO12 one. This seems to hold for my test cases but I'm not
entirely sure if this is actually guaranteed.

This patch also contains test cases for all relocations. However, there
is an issue with testing R_RISCV_32_PCREL: llvm-mc only emits this for
.eh_frame sections and uses unnamed symbols there. Since the symbol
lookup in RuntimeDyld seems to be based only on symbol names, these
symbols don't get resolved properly (Value is always 0).

This is not really a problem for the initial BOLT bring-up since we can
test on code that doesn't need .eh_frames. However, since GCC's startup
code contains them, we need to handle them somehow.

Note that I have included a (disabled) test case for R_RISCV_32_PCREL
relocations that passes if llvm-mc is modified to use section-relative
relocations in .eh_frame. I'm not sure if it makes sense to keep it in
this patch though.

Diff Detail

Event Timeline

jobnoorman created this revision.Mar 9 2023, 5:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 5:55 AM
jobnoorman requested review of this revision.Mar 9 2023, 5:55 AM

Friendly ping.

Is there something preventing Bolt from moving to ORC / JITLink? If Bolt is able to move over then the aim should be to do that. If Bolt is unable to move over then we need to know why so that we can address the issue. RuntimeDyld is very much in maintenance mode at the moment, and we're working hard to reach parity in backend coverage so that we can officially deprecate it.

None of that is a total blocker to landing this, but the bar is high, and it should be understood that Bolt will need to migrate in the future.

Is there something preventing Bolt from moving to ORC / JITLink? If Bolt is able to move over then the aim should be to do that. If Bolt is unable to move over then we need to know why so that we can address the issue. RuntimeDyld is very much in maintenance mode at the moment, and we're working hard to reach parity in backend coverage so that we can officially deprecate it.

None of that is a total blocker to landing this, but the bar is high, and it should be understood that Bolt will need to migrate in the future.

Hi @lhames , is JITLink mature for X86 and AArch64? Those are the two requirements for BOLT.

Is there something preventing Bolt from moving to ORC / JITLink? If Bolt is able to move over then the aim should be to do that. If Bolt is unable to move over then we need to know why so that we can address the issue. RuntimeDyld is very much in maintenance mode at the moment, and we're working hard to reach parity in backend coverage so that we can officially deprecate it.

None of that is a total blocker to landing this, but the bar is high, and it should be understood that Bolt will need to migrate in the future.

Hi @lhames , is JITLink mature for X86 and AArch64? Those are the two requirements for BOLT.

@rafaelauler JITLink's MachO x86_64 and aarch64 backends are mature. ELF x86_64 and aarch64 are mature for PIC, but may be missing some static relocations. These relocations can easily be added if needed, and should not be considered a blocker to moving over.

COFF x86-64 is solid (though new). I believe JITLink's support for it is already strictly better than RuntimeDyld's. COFF/aarch64 hasn't been added yet, but should be doable if you need it.

jobnoorman abandoned this revision.May 22 2023, 3:06 AM

BOLT's JITLink port has been accepted (D147544) so I will close this.