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

[Rui Ueyama via llvm-commits]

Sure. So I posed a question rather than requesting a change. If making -r
slower is expected, it needed to be explained in the code as comment.

On Mon, Jan 23, 2017 at 12:52 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170123/735ca129/attachment.html>