[PATCH] D28773: [ELF] - Avoid reusing DynsymIndex for -r

[George Rimar via Phabricator via llvm-commits]

grimar abandoned this revision.
grimar added a comment.

In https://reviews.llvm.org/D28773#652532, @ruiu wrote:

> So I took a look at the code, and I think the current implementation of the -r option in regard to symbol indices is unnecessarily complicated. The logic is scattered over Writer.cpp and SyntheticSections.cpp, and we are handling -r in an obscure way there. It needs fixing from a broader perspective (e.g. moving the logic to SynethticSections) instead of a local fix.


Done in https://reviews.llvm.org/D29021.



================
Comment at: ELF/InputSection.cpp:233
 
+template <class ELFT> static uint32_t getSymbolIndex(SymbolBody &Body) {
+  if (Body.IsLocal) {
----------------
ruiu wrote:
> This function seems too heavy to run for each relocation. It is probably not ok to do this much here.
That affects only -relocatable.

But anyways, since this patch is abandoned and D29021 which landed instead also contains linear search,
I had in mind two possible optimizations for new SymbolTableSection::getSymbolIndex()

1) We can cache the result for Body in place of call (copyRelocations) and call getSymbolIndex only if result is not in cache.
2) We can add some new member for doing mapping from Body to index to SymbolTableSection class. To replace linear search with mapping call.

As an alternative we can do nothing for now and see if somebody mets perfomance issues with relocatable option.

What do you think ?


https://reviews.llvm.org/D28773