[PATCH] D29021: [ELF] - Stop handling local symbols in a special way.

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar added a comment.
>
> In https://reviews.llvm.org/D29021#653797, @ruiu wrote:
>
>> Or, if you find that adding a new field to Symbol class doesn't really hurt any metrics, you can simply add it. Or, even if it slows down the linker a little bit, you may still be able to convince that that is necessary. I believe adding a new field is the simplest and sufficient way to do what you want to do, but I think you want to stop to think and investigate a few different ways, and then make your conclusion (because all I said were my guesses and not conclusions in any sense.)
>
>
> Mmmm. Sorry, did you get this had LGTM and was already landed ? (I am not sure, I thought we discussing post commit details.)
>
> I think I agree with Rafael that for now we can go with this solution and see if we want to optimize for -r or not separatelly later.
> -relocatable output is not general use case and I think I am agree extending Symbol simply not worth doing if can be avoided.
> No matter if that slows down linker or not.
>
> This patch removed KeptLocalSyms and localized symbols handling in one place, what IMO made code cleaner. So it was useful to have.
> And even if we decide to add field to Symbol, this patch helps to do that in a easy way still.

Yes, the way I see it with this patch
* The code got simpler.
* Hopefully unblocked -emit-relocs.
* -r got slower. Once -emit-relocs is in we can investigate the if -r is
  too slow for the any use case.

Cheers,
Rafael