[PATCH] D145687: [BOLT] Add minimal RISC-V 64-bit support

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: bolt/include/bolt/Core/BinaryContext.h:730
 
+  bool isRISCV() const { return TheTriple->getArch() == llvm::Triple::riscv64; }
+
----------------
Should this be isRISCV64()?


================
Comment at: bolt/lib/Core/Relocation.cpp:505
+    return extractUImmRISCV(Contents);
+  case ELF::R_RISCV_PCREL_LO12_I:
+    return extractIImmRISCV(Contents);
----------------
What about PCREL_LO12_S?


================
Comment at: bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp:39
+
+  bool isNoop(const MCInst &Inst) const override {
+    return Inst.getOpcode() == RISCV::ADDI &&
----------------
Does this need to support c.nop?


================
Comment at: bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp:253
+    case ELF::R_RISCV_PCREL_LO12_I:
+    case ELF::R_RISCV_PCREL_LO12_S:
+      return RISCVMCExpr::create(Expr, RISCVMCExpr::VK_RISCV_PCREL_LO, Ctx);
----------------
This references R_RISCV_PCREL_LO12_S but nothing else in this patch does.


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

https://reviews.llvm.org/D145687