[PATCH] D32721: Accept archive files with no symbol table instad of warning on them.

[Mehdi AMINI via llvm-commits]

On Wed, May 3, 2017 at 5:44 PM Rui Ueyama <ruiu at google.com> wrote:

> On Wed, May 3, 2017 at 5:40 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>>
>> On Wed, May 3, 2017 at 5:26 PM Rui Ueyama <ruiu at google.com> wrote:
>>
>>> On Wed, May 3, 2017 at 5:13 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>
>>>> Clang is incrementally linking in a matter of a few seconds, so 0.5s to
>>>> read the symbols is a double digit percentage of that.
>>>> And there are over 50 binaries in LLVM, not just one.
>>>>
>>>
>>> We do not support incremental linking,
>>>
>>
>> I'm talking about ThinLTO incremental linking, which we support.
>>
>
> How can that be faster than the regular build?
>

Not sure what you mean: on my mac ld64 links clang in less than 2s.




>
>> but even if we support it, we don't need to read archives that haven't
>>> changed since the last build, so the overhead in that hypothetical case
>>> would be much smaller than 0.5s.
>>>
>>
>> So yes we need to read all the archives.
>>
>>
>>> And you still don't address the "principle of least surprise": the
>>>> configuration is *not* what is expected from the user.
>>>>
>>>
>>> As a naive user of LTO, I was surprised that LTO needs llvm-ar, which is
>>> certainly I didn't expect (due to lack of knowledge).
>>>
>>
>> That is why the warning is deserved.
>>
>
> But you no longer need it with this change.
>
> --
>> Mehdi
>>
>>
>>
>>>
>>>> --
>>>> Mehdi
>>>>
>>>>
>>>> 2017-05-03 16:51 GMT-07:00 Rui Ueyama <ruiu at google.com>:
>>>>
>>>>> The cost of reading symbols from object files in archive files is
>>>>> probably much cheaper than you might be thinking. If I strip all symbols
>>>>> from archives from a clang debug build, LLD takes 8.16 seconds to link,
>>>>> while it can usually link it in 7.65 seconds. So the difference is only 0.5
>>>>> seconds, and clang is a fairly large program as a test. That test case uses
>>>>> ELF, but with Peter's patch I believe reading symbols from bitcode files is
>>>>> fast too.
>>>>>
>>>>> To me 0.5 seconds is too small that I want the tool to "just work"
>>>>> instead of annoy me every time I run make/ninja until I change the build
>>>>> configuration to shave off 0.5 seconds from a LTO build.
>>>>>
>>>>> On Wed, May 3, 2017 at 4:23 PM, Mehdi AMINI via Phabricator <
>>>>> reviews at reviews.llvm.org> wrote:
>>>>>
>>>>>> mehdi_amini added a comment.
>>>>>>
>>>>>> I personally  think *not* warn is a terrible thing to do when there
>>>>>> is a configuration issue. Erroring is annoying, but warning should be
>>>>>> intended in such cases!
>>>>>>
>>>>>> > True, but on the other hand, it's pretty much the exact same work
>>>>>> that the archiver would need to do,
>>>>>>
>>>>>> The archiver do it once for potentially a lot of linker invocations.
>>>>>>
>>>>>> > and asking the user to change their archiver and rebuild would
>>>>>> probably consume even more time.
>>>>>>
>>>>>> This is a one time thing, and the user can live with the warning (or
>>>>>> pass a flag to disable the warning maybe) if they choose to.
>>>>>>
>>>>>>
>>>>>> Repository:
>>>>>>   rL LLVM
>>>>>>
>>>>>> https://reviews.llvm.org/D32721
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170504/ebe08c8a/attachment.html>