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

[Sameer AbuAsal via Phabricator via llvm-commits]

sabuasal added inline comments.


================
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;
----------------
sabuasal wrote:
> 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!
correction: *fixup_riscv_lo12_s*  is only used for S instruction format in RISCV. 


Repository:
  rL LLVM

https://reviews.llvm.org/D23568