[PATCH] D23568: [RISCV 10/10] Add common fixups and relocations

[Sameer AbuAsal via Phabricator via llvm-commits]

sabuasal added inline comments.
Herald added subscribers: kito-cheng, niosHD, apazos, johnrusso, rbar.


================
Comment at: llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:148
+    case RISCVMCExpr::VK_RISCV_LO:
+      FixupKind = MIFrm == RISCVII::InstFormatI ? RISCV::fixup_riscv_lo12_i
+                                                : RISCV::fixup_riscv_lo12_s;
----------------
I think this is problematic, fixup_riscv_lo12_i is only used for S instruction format in RISCV. I think this should be
if  (MIFrom == RISCVII::InstFormatI) {
  FixUpKind = RISCV::fixup_riscv_lo12_i;
} else if (RISCVII::InstFormatS) {
 FixUpKind = RISCV::fixup_riscv_lo12_s;
} else {
  llvm_unreacable("unexpected Instructio type for relocation  FixUpKind = RISCV::fixup_riscv_lo12_i");
}

Other wise you'll mistakenly  end up generating instructions instructions with wrong fixup kinds!


Repository:
  rL LLVM

https://reviews.llvm.org/D23568