[PATCH] D26298: [ELF][MIPS] N32 ABI support

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Overall looking good.



================
Comment at: ELF/Driver.cpp:686
+template <class ELFT>
+static bool checkMipsN32Abi(const std::vector<InputFile *> &Files) {
+  bool HasN32AbiEmulation = Config->Emulation == "elf32btsmipn32" ||
----------------
Can you move this to isCompatible in SymbolTable.cpp?


================
Comment at: ELF/Relocations.cpp:589
+template <class RelTy>
+static unsigned mergeMipsN32RelTypes(uint32_t &Type,
+                                     typename RelTy::Elf_Addr Offset, RelTy *I,
----------------
Could you return a pair of number of processed symbols and a merged type, so that this function becomes a pure function?


================
Comment at: ELF/Relocations.cpp:590
+static unsigned mergeMipsN32RelTypes(uint32_t &Type,
+                                     typename RelTy::Elf_Addr Offset, RelTy *I,
+                                     RelTy *E) {
----------------
I think Elf_Addr is always 32 bit, so I'd replace with uint32_t.


================
Comment at: ELF/Relocations.cpp:645
+    if (Config->MipsN32Abi)
+      I += mergeMipsN32RelTypes(Type, RI.r_offset, std::next(I), E);
+
----------------
std::next(I) -> I + 1


Repository:
  rL LLVM

https://reviews.llvm.org/D26298