[PATCH] D149524: [JITLink] Allow edges without a target

[Job Noorman via Phabricator via llvm-commits]

jobnoorman abandoned this revision.
jobnoorman added a comment.

Hi Lang,

In D149524#4308261 <https://reviews.llvm.org/D149524#4308261>, @lhames wrote:

> I believe we can handle `R_RISCV_RELAX` and `R_RISCV_ALIGN` without changing `Edge` -- it would be great to keep "Edges always have targets" as a simple invariants if we can.

Yes, I agree. I found all the `!hasTarget()` checks this patch had to add quite inelegant.

> For `R_RISCV_RELAX` we should add relaxable edge kinds to the RISCV edge kinds enum (along the same lines as the x86-64 relaxable edges, and we should take this opportunity to rename the RISCV edges to bring them in line with the other architectures). When the ELF/RISCV LinkGraphBuilder sees an `R_RISCV_RELAX` relocation it should choose the relaxable variant for the corresponding edge.

This looks like a very interesting approach! I'll look into it next week. For now, I used the same null symbol approach as for `R_RISCV_ALIGN`.

> For `R_RISCV_ALIGN` I think we can reasonably just point the Edge at a null absolute symbol.

Interestingly, both `R_RISCV_ALIGN` and `R_RISCV_RELAX` actually point to some kind of null symbol in the ELF file. It's an undefined symbol (`UND`) though and in an earlier attempt, I actually tried to add this symbol to the `LinkGraph` as some kind of new symbol type (local undefined). However, this had a huge ripple effect so I abandoned that approach for the one in this patch. Using an absolute symbol instead works quite nicely without a large fallout. Thanks for this idea! I created D149541 <https://reviews.llvm.org/D149541> for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149524