[PATCH] D157803: [JITLink][RISCV] Implement eh_frame handling

Mon Sep 18 01:19:56 PDT 2023

jobnoorman added inline comments.

================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:462
       break;
+    case Delta32: {
+      int64_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
----------------
Hahnfeld wrote:
> jobnoorman wrote:
> > This is the same as `R_RISCV_32_PCREL` so maybe just reuse that one? At some point, `R_RISCV_32_PCREL` should be renamed to `Delta32` though.
> Actually the (positive) `Delta` edges are not needed at all; the hypothetical `AddFDEToCIEEdges` discussed in https://reviews.llvm.org/D157802 only needs `NegDelta32`. That's another reason why I think it's the way to go.
Ah yes, you're right. I see the point about `AddFDEToCIEEdges` but I worry a bit about the (non-trivial) code duplication it would introduce. The documentation of `EHFrameEdgeFixer` mentions that it should be fine to provide `Edge::Invalid` for unsupported edge kinds but the implementation doesn't seem to check for that. Maybe this could be a way to configure it to not do more than necessary on RISC-V?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157803