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

Thu May 4 16:21:42 PDT 2017

On Thu, May 4, 2017 at 4:18 PM, Rui Ueyama <ruiu at google.com> wrote:

> The other way of making it user-friendly is to make a change to Clang so
> that it uses ELF (with symbol table but with no other data) as a container
> for bitcode files, so that default ar commands can construct symbol tables
> for them. But as it is an obvious solution, I believe you guys considered
> it before and turned it down.
>
> As to the performance of symbol table-less archives, https://reviews.
> llvm.org/D32881 makes them as fast as (or even faster than for some
> reason) archives with symbol table.
>

Just to be clear, symbol table-less archives are faster in the
single-threaded case too?

-- Sean Silva

>
> My memory is vague too, but I tried -DCMAKE_AR and found that it didn't
> work for me.
>
> On Thu, May 4, 2017 at 4:12 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Thu, May 4, 2017 at 3:58 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>
>>>
>>>
>>> 2017-05-04 15:50 GMT-07:00 Sean Silva <chisophugis at gmail.com>:
>>>
>>>>
>>>>
>>>> On Wed, May 3, 2017 at 6:28 PM, Rui Ueyama via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> On Wed, May 3, 2017 at 6:12 PM, Mehdi AMINI <joker.eph at gmail.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-05-03 18:07 GMT-07:00 Rui Ueyama <ruiu at google.com>:
>>>>>>
>>>>>>> On Wed, May 3, 2017 at 5:58 PM, Mehdi AMINI <joker.eph at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, May 3, 2017 at 5:51 PM Rui Ueyama <ruiu at google.com> wrote:
>>>>>>>>
>>>>>>>>> On Wed, May 3, 2017 at 5:48 PM, Mehdi AMINI <joker.eph at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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 that is ld64. We are talking about LLD, no?
>>>>>>>>>
>>>>>>>>
>>>>>>>> I don't see where you're going: lld is supposed to be fast, isn't
>>>>>>>> it? I assume it has to be able to outspeed ld64.
>>>>>>>> So I'm giving you a reference of what is "a regular" build time and
>>>>>>>> that should explain why you .5s overhead is not trivial.
>>>>>>>>
>>>>>>>
>>>>>>> That is hypothetical. If LLD is already able to link Clang with
>>>>>>> ThinLTO in 2 seconds, it may make sense to warn on 0.5 second loss, but
>>>>>>> that's in reality not the case, so I'm not convinced that we should warn on
>>>>>>> it right now.
>>>>>>>
>>>>>>
>>>>>> I disagree: we should define what is a *correct* build setting, and
>>>>>> warn if it is not honored.
>>>>>> Your changing this definition here, and I doubt it'll be easy to
>>>>>> revert in the future, which is why I don't like this.
>>>>>>
>>>>>
>>>>> That's a valid concern, and this change certainly relaxes the
>>>>> definition what is correct (or at least what is not incorrect). But I still
>>>>> think that that's beneficial for users overall. You are extremely familiar
>>>>> with LLVM LTO, but ordinary developers don't know much about LLVM or build
>>>>> systems compared to you. Attempting to use llvm-ar took a fair amount of
>>>>> time even to me (which I hope better than the average Unix user). If we
>>>>> print out a warning on every linker invocation, we'd probably be teaching
>>>>> users to ignore warnings rather than let them change the build
>>>>> configuration, as it just works with a marginal performance difference.
>>>>>
>>>>
>>>>
>>>> In my experience (and mentioned in this thread too), changing the
>>>> archiver is very difficult. One necessary requirement for emitting a
>>>> warning is that it has to be actionable.
>>>>
>>>> For example, in CMake, I only know how to do it after looking at
>>>> Rafael's https://github.com/espindola/llvm-scripts (and I have no idea
>>>> how Rafael figured it out).
>>>>
>>>
>>> As a side note: this is suboptimal I believe, cmake has -DCMAKE_AR /
>>> -DCMAKE_RANLIB options (just like -DCMAKE_C_COMPILER).
>>>
>>
>> IIRC, I've tried to use those and they didn't work. My memory is vague,
>> but I think the issue was that CMAKE_AR and CMAKE_RANLIB were not
>> propagated to all configuration checks, so that some of them spuriously
>> failed because the CFLAGS had -flto but the CMAKE_AR/CMAKE_RANLIB was not
>> being respected.
>>
>> -- Sean Silva
>>
>>
>>>
>>>
>>>
>>>>
>>>> And I can easily imagine (and have been in situations) where it's just
>>>> not feasible to hook up an LTO-friendly archiver, which would cause users
>>>> to just give up on LTO. So we should cater to the use case of users being
>>>> in a permanent situation of not having an LTO-friendly archiver.
>>>>
>>>> So my vote is that:
>>>>
>>>> 1. We have an opt-in flag for linking without archive symbol tables (or
>>>> some other suitable behavior that provides for this use case). If this
>>>> comes at a nontrivial performance cost, the name should communicate that
>>>> this forces the linker to do extra work so that users don't get confused
>>>> about any slowdown.
>>>> 2. We emit an error mentioning the opt-in flag
>>>>
>>>> Alternatively, if we can handle the no-symbol-table case just as
>>>> efficiently (or nearly as much), we should just do it transparently IMO,
>>>> but that might not be achievable.
>>>>
>>>>
>>>>
>>>>> In most LTO use cases, I believe users enable LTO only to build
>>>>> binaries for shipping. They don't do as many LTO builds as you do. Making
>>>>> it "just work" seems to be worth doing.
>>>>>
>>>>
>>>> IIRC, one of the original goals of ThinLTO is to make it feasible for
>>>> everyday developer Release builds to default to ThinLTO. I.e. it is not
>>>> just "golden"/"final" builds using LTO.
>>>>
>>>> -- Sean Silva
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170504/61ccf03d/attachment.html>