[PATCH] D47126: [RISCV] Add missing fixup_riscv_call in MCFixupKindInfo table

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:51
                              const MCValue &Target) override {
+    // Always preserve relocation type for function call.
+    if ((unsigned)Fixup.getKind() == RISCV::fixup_riscv_call)
----------------
This comment should explain why we need to preserve the relocation type. e.g. "We are unable to correctly resolve a fixup_riscv_call, so always emit a relocation."


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:90
+      { "fixup_riscv_rvc_branch",    0,     16,  MCFixupKindInfo::FKF_IsPCRel },
+      { "fixup_riscv_call",          0,     32,  MCFixupKindInfo::FKF_IsPCRel }
     };
----------------
Thinking about it, bits should really be 64 for fixup_riscv_call. With that in place, wouldn't it be feasible to add support to adjustFixupValue so we actually can resolve the fixup?


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:92
     };
+    static_assert((sizeof(Infos) / sizeof(MCFixupKindInfo)) ==
+                  RISCV::NumTargetFixupKinds,
----------------
You can just use `array_lengthof(Infos)`.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:94
+                  RISCV::NumTargetFixupKinds,
+                  "the MCFixupKind numbers in Infos should equal to RISCV::NumTargetFixupKinds");
 
----------------
"Not all fixup kinds added to Infos array" is probably sufficient.


Repository:
  rL LLVM

https://reviews.llvm.org/D47126