[PATCH] D15112: [ELF] MIPS paired R_MIPS_HI16/LO16 relocations support

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Overall looks good but a few nits.


================
Comment at: ELF/InputSection.cpp:96
@@ -95,2 +95,3 @@
 template <class ELFT>
 template <bool isRela>
+uint8_t *
----------------
Do you have to template isRela? IIRC, it is always Elf_Rel on MIPS.

================
Comment at: ELF/InputSection.cpp:106-118
@@ +105,15 @@
+    return nullptr;
+  switch (Type) {
+  case R_MIPS_HI16:
+    Type = R_MIPS_LO16;
+    break;
+  case R_MIPS_PCHI16:
+    Type = R_MIPS_PCLO16;
+    break;
+  case R_MICROMIPS_HI16:
+    Type = R_MICROMIPS_LO16;
+    break;
+  default:
+    return nullptr;
+  }
+  for (const auto &RI : Rels) {
----------------
I'd write this as it's shorter.

  if (Type == R_MIPS_HI16)
    Type = R_MIPS_LO16;
  else if (Type == R_MIPS_PCHI16)
    Type = R_MIPS_PCLO16;
  else if (Type == R_MICROMIPS_HI16)
    Type = R_MICROMIPS_LO16;
  else
    return nullptr;

================
Comment at: ELF/Target.h:62
@@ -62,1 +61,3 @@
+                           uint64_t P, uint64_t SA,
+                           uint8_t *PairedLoc = nullptr) const = 0;
   virtual bool isTlsOptimized(unsigned Type, const SymbolBody *S) const;
----------------
Remove "= nullptr" since the argument is always passed.


Repository:
  rL LLVM

http://reviews.llvm.org/D15112