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

[Rui Ueyama via llvm-commits]

On Wed, Jan 20, 2016 at 3:15 AM, Simon Atanasyan <simon at atanasyan.com>
wrote:

> 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.


I spent a few hours today to experiment that idea to create SymbolBodies
for local symbols. Looks like we can do that with a few percent increase in
link time, and that would make code much more readable. That's a fair
trade-off. So, I'll take care of this part of code complexity.

It will take more time to ready for code review, though.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160121/11bbe27c/attachment.html>