[PATCH] D117082: [JITLink][RISCV] Support R_RISCV_SET relocations

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

You can write tests just fine in assembly. Use `.reloc` if needed. You don't need `.eh_frame` for that.



================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/riscv.h:87
+
+  // Local label assignment
+  //
----------------
These are all missing a /


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:251
+      int64_t Value = (E.getTarget().getAddress() + E.getAddend()).getValue();
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+      int64_t Word6 = Value & 0x3f;
----------------
These all operate on an 8/16/32-bit value (SET6 operates on the low 6 bits of an 8-bit value). These do not relocate instructions, they relocate data. Again, please look at LLD before implementing things rather than guessing.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:276
+    }
+    case R_RISCV_32_PCREL: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
----------------
This is not an R_RISCV_SET* relocation, yet the commit message says that's that is added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117082