[PATCH] D21297: [ELF][MIPS] Support GOT entries for non-preemptible symbols with different addends

[Rui Ueyama via llvm-commits]

ruiu added a comment.

I think I don't fully digest the MIPS ABI yet, so I guess this review is superficial. I'll take time to read it again. Overall it looks good though.


================
Comment at: ELF/OutputSections.cpp:96
@@ -96,1 +95,3 @@
+template <class ELFT>
+void GotSection<ELFT>::addEntry(SymbolBody &Sym, uintX_t Addend, RelExpr Expr) {
   if (Config->EMachine == EM_MIPS) {
----------------
Define GotSection::addMipsEntry and move MIPS-related code there, so that this function is as short as

  if (Config->EMachine == EM_MIPS) {
    addMipsEntry(Sym, Addend, Expr);
    return;
  Sym.GotIndex = Entries.size();
  Entries.push_back(&Sym);


================
Comment at: ELF/OutputSections.cpp:131-132
@@ +130,4 @@
+    } else if (Expr == R_MIPS_GOT_LOCAL) {
+      auto P = MipsLocalMap.insert(
+          std::make_pair(MipsGotSymAddend(&Sym, Addend), MipsLocalMap.size()));
+      if (P.second)
----------------
Can this be

  MipsLocalMap.insert({{&Sym, Addend}, MipsLocalMap.size()})

?

================
Comment at: ELF/OutputSections.cpp:188
@@ +187,3 @@
+    return (MipsPageEntries + It->second) * sizeof(uintX_t) - MipsGPOffset;
+  fatal("Cannot find GOT entry for the symbol");
+}
----------------
If the control cannot reach here only because of a bug, use llvm_unreachable.

================
Comment at: ELF/OutputSections.h:135
@@ -133,1 +134,3 @@
   unsigned getMipsLocalEntriesNum() const;
+  // Calculate MIPS GOT offset adjustment. Functon take in account number
+  // of entries in the global and local parts of the GOT and shift offset
----------------
Add a newline before the comment.

================
Comment at: ELF/OutputSections.h:154
@@ -148,1 +153,3 @@
   llvm::DenseMap<uintX_t, size_t> MipsLocalGotPos;
+  // Entries from the 'local' part of MISP GOT with 'full' addresses.
+  // We have to track pairs of Symbol and Addends because each such pair
----------------
Add newlines before comments.

================
Comment at: ELF/Symbols.h:99
@@ -98,1 +98,3 @@
 
+  unsigned IsInGlobalMipsGot : 1;
+
----------------
Move this after `IsLocal` so that `SymbolKind` is aligned on 8 byte boundary.


Repository:
  rL LLVM

http://reviews.llvm.org/D21297