[PATCH] D98305: [JITLink] Introduce generic x86-64 support by refactoring MachO/x86-64.

Fri Mar 12 11:09:52 PST 2021

lhames added a comment.

In D98305#2619857 <https://reviews.llvm.org/D98305#2619857>, @sgraenitz wrote:

> Factoring out architecture specific code sounds like a reasonable approach to reduce code duplication across the JITLink backends. As far as I can see, the patch only ports the MachOJITLinker_x86_64 to use the generic functionality. The ELFJITLinker_x86_64 mostly remains unchanged. Is there a conceptual reason for it or would that be a rather straightforward next step?

Yes -- that's a logical next step. I know @jaredwy is working on a refactor of the ELF/x86-64 backend to extract a generic ELF graph builder at the moment, so I wanted to hold off on any changes to the ELF backend in case they interfered with his work.

> What I am not sure I understand yet is how endianness is handled in this scheme. The new applyFixup() function is using hard casts to little-endian when writing fixup values into memory. Is that because addRelocations() is supposed to populate the fixup values with the correct endian already? I think the ELF implementation didn't consider it at all so far, so there is no loss of functionality. Just asking for interest.

There is a hard (but poorly documented) boundary here: Linker *working memory* (block content) should always be in the executor's (target's) endianness, hence the hard casts here. That's because it comes from the object file (which is in the executor's endianness) and will be transferred byte-for-byte to the executor. All other graph values including symbol and block addresses should be in the host's endianness to avoid any unnecessary conversions during arithmetic. Conversions to/from executor endianness should be done on write/reads of working memory only.

> Great documentation for the edge kinds!

Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98305