[PATCH] D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Arch/RISCV.cpp:48
+};
+enum Reg {
+  X_RA = 1,
----------------
nit: add a blank line


================
Comment at: ELF/Arch/RISCV.cpp:59-67
+static uint32_t itype(uint32_t Op, uint32_t Rd, uint32_t Rs1, uint32_t Imm) {
+  return Op | (Rd << 7) | (Rs1 << 15) | (Imm << 20);
+}
+static uint32_t rtype(uint32_t Op, uint32_t Rd, uint32_t Rs1, uint32_t Rs2) {
+  return Op | (Rd << 7) | (Rs1 << 15) | (Rs2 << 20);
+}
+static uint32_t utype(uint32_t Op, uint32_t Rd, uint32_t Imm) {
----------------
Nice little assembly routines.


================
Comment at: ELF/Arch/RISCV.cpp:71
+  CopyRel = R_RISCV_COPY;
+  GotRel = Config->Is64 ? R_RISCV_64 : R_RISCV_32;
+  NoneRel = R_RISCV_NONE;
----------------
Does this really support R_RISCV_32?


================
Comment at: ELF/Arch/RISCV.cpp:116
+void RISCV::writeGotHeader(uint8_t *Buf) const {
+  write64le(Buf, Main->Dynamic->getVA());
+}
----------------
For example, it looks like this code assumes RISCV64.


================
Comment at: ELF/Arch/RISCV.cpp:124-125
+void RISCV::writePltHeader(uint8_t *Buf) const {
+  if (Config->EFlags & EF_RISCV_RVE)
+    error("PLT is not supported for RVE");
+
----------------
What is RVE?


================
Comment at: ELF/Arch/RISCV.cpp:140
+  write32le(Buf + 8, itype(Load, X_T3, X_T2, lo12(Offset)));
+  write32le(Buf + 12, itype(ADDI, X_T1, X_T1, -(Target->PltHeaderSize + 12)));
+  write32le(Buf + 16, itype(ADDI, X_T0, X_T2, lo12(Offset)));
----------------
nit: -Target->PltHeaderSize - 12


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D63076