[PATCH] D147544: [BOLT] Move from RuntimeDyld to JITLink

Maksim Panchenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 24 15:29:53 PDT 2023


maksfb added a subscriber: yavtuk.
maksfb added a comment.

In D147544#4293847 <https://reviews.llvm.org/D147544#4293847>, @rafauler wrote:

> Noticed another difference here between RuntimeDyld vs. JITLink. Writing here for documentation only, I think it is harmless.
>
> My hugify lib's file are built with one additional .rodata.extra section (.rodata.str1.16.bolt.extra.1) that is entirely useless. The error strings originally stored in this section are materialized in code with movabs to stack space (I think this is an unintended side effect of the way the code is written, see https://github.com/llvm/llvm-project/blob/main/bolt/runtime/hugify.cpp#L119, which forces the compiler to allocate the error message in the stack), rendering this rodata section unnecessary/unreferenced, but still present in the object file. RuntimeDyld doesn't allocate this unnecessary section, but JITLink does allocate.
>
> I'm not sure if RuntimeDyld is being smart about it, or if its a bug. But the binary is not going to crash because we have no relocs against this unnecessary rodata section.

Right. RuntimeDyld will not emit a section without relocations. E.g., for .debug_line, we have to make the section allocatable (https://github.com/llvm/llvm-project/blob/99cfaf0d5ed68d5d4e292fc87a10b1bb26201787/bolt/lib/Core/BinaryEmitter.cpp#L190-L199) and then force a dummy relocation against it (https://github.com/llvm/llvm-project/blob/5f2b0892d5b732a822728f96a57144f6542c155e/bolt/lib/Core/DebugData.cpp#L1631-L1637).

Sounds like with JITLink at least the hack for the relocation is not needed. @lhames, could you please confirm that it's by design?

Overall, the switch to JITLink is going full-swing and it might be too late to stop it :) Besides BOLT, what other major projects are going to be (or perhaps already are) using JITLink/ORC?

@yota9, @yavtuk, @treapster, did you test this diff on aarch64?

We also need to compare BOLT's CPU time and memory for this change before it can be accepted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147544/new/

https://reviews.llvm.org/D147544



More information about the llvm-commits mailing list