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

[Mandeep Singh Grang via Phabricator via llvm-commits]

mgrang added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:68
+
+
+    if (Kind < FirstTargetFixupKind)
----------------
Extra newline?


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:181
+  for (unsigned i = 0; i != 4; ++i) {
+    unsigned Idx = i;
+    Data[Offset + i] |= uint8_t((Value >> (Idx * 8)) & 0xff);
----------------
Do we really need to use another variable Idx? Can't we simply use i?


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp:68
+    return "pcrel_hi";
+  default:
+    llvm_unreachable("Invalid ELF symbol kind");
----------------
Default should be the first case in a switch.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp:96
+    return ((Value + 0x800) >> 12) & 0xfffff;
+  default:
+    llvm_unreachable("Invalid kind");
----------------
Ditto.


https://reviews.llvm.org/D23568