<div dir="ltr">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.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 23, 2017 at 12:52 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">George Rimar via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> writes:<br>
<br>
</span><span class="">> grimar added a comment.<br>
><br>
> In <a href="https://reviews.llvm.org/D29021#653797" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D29021#653797</a>, @ruiu wrote:<br>
><br>
>> 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.)<br>
><br>
><br>
> Mmmm. Sorry, did you get this had LGTM and was already landed ? (I am not sure, I thought we discussing post commit details.)<br>
><br>
> 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.<br>
> -relocatable output is not general use case and I think I am agree extending Symbol simply not worth doing if can be avoided.<br>
> No matter if that slows down linker or not.<br>
><br>
> This patch removed KeptLocalSyms and localized symbols handling in one place, what IMO made code cleaner. So it was useful to have.<br>
> And even if we decide to add field to Symbol, this patch helps to do that in a easy way still.<br>
<br>
</span>Yes, the way I see it with this patch<br>
* The code got simpler.<br>
* Hopefully unblocked -emit-relocs.<br>
* -r got slower. Once -emit-relocs is in we can investigate the if -r is<br>
  too slow for the any use case.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div>