[PATCH] D16324: [ELF][MIPS] Initial support of MIPS local GOT entries

[Simon Atanasyan via llvm-commits]

atanasyan marked an inline comment as done.

================
Comment at: ELF/OutputSections.cpp:154-156
@@ -127,4 +153,5 @@
 template <class ELFT> void GotSection<ELFT>::finalize() {
   this->Header.sh_size =
-      (Target->getGotHeaderEntriesNum() + Entries.size()) * sizeof(uintX_t);
+      (Target->getGotHeaderEntriesNum() + MipsLocalEntries + Entries.size()) *
+      sizeof(uintX_t);
 }
----------------
ruiu wrote:
> atanasyan wrote:
> > ruiu wrote:
> > > This code seems a bit tricky, and I also don't like the idea to look up a table for MIPS local symbols.
> > > 
> > > How about this? We don't need to assign an index in the GOT to Sym->GotIndex() in addEntry. Instead, we can do that in GotSection::finalize(). We can also change addMipsLocalEntry(SymbolBody *) to just add a given SymbolBody to some vector (say, MipsEntries). In finalize(). we assign GotIndex to all symbols in MipsEntries first and then to all symbols in Entries.
> > It is unclear how to deal with local symbols which does not have a corresponding `SymbolBody`.
> > 
> > Now `MipsLocalEntries` holds number of entries reserved for local GOT entries. Some of these entries are for non-local symbols with `SymbolBody` and we can keep them in `MipsEntries`. But other ones are for local symbols, i.e. `section + offset` in fact.
> Ah, I often forgot the fact that we did not create SymbolBody for a local symbol. This is probably a bold suggestion, but for the sake of discussion, what if we create SymbolBodies for local symbols? How much will it increase link time (I'm not requesting to do that in this patch, though)? That would increase link time, but if it is small, then that may make sense because that would simplify some code a lot.
It is a good idea but let's postpone it a bit. I'm going to optimize local GOT entries allocation to reduce number of such entries. To do that I will need to handle local symbols in a more complicated way than just counting GOT16 relocations against them. Maybe in that case SymbolBody for local symbol becomes mandatory.


Repository:
  rL LLVM

http://reviews.llvm.org/D16324