<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 3, 2017 at 6:28 PM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-">On Wed, May 3, 2017 at 6:12 PM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>2017-05-03 18:07 GMT-07:00 Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, May 3, 2017 at 5:58 PM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br><div class="gmail_quote"><span><div>On Wed, May 3, 2017 at 5:51 PM Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote">On Wed, May 3, 2017 at 5:48 PM, Mehdi AMINI <span><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br><div class="gmail_quote"><span><div>On Wed, May 3, 2017 at 5:44 PM Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>On Wed, May 3, 2017 at 5:40 PM, Mehdi AMINI <span><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br></div></div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br><div class="gmail_quote"><span><div>On Wed, May 3, 2017 at 5:26 PM Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>On Wed, May 3, 2017 at 5:13 PM, Mehdi AMINI <span><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br></div></div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>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.</div><div>And there are over 50 binaries in LLVM, not just one.</div></div></blockquote><div><br></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div>We do not support incremental linking, </div></div></div></div></blockquote><div><br></div></span><div>I'm talking about ThinLTO incremental linking, which we support.</div></div></div></blockquote><div><br></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div>How can that be faster than the regular build?</div></div></div></div></blockquote><div><br></div></span><div>Not sure what you mean: on my mac ld64 links clang in less than 2s.</div></div></div></blockquote><div><br></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div>But that is ld64. We are talking about LLD, no?</div></div></div></div></blockquote><div><br></div></span><div>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. </div><div>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.</div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div></span><div>I disagree: we should define what is a *correct* build setting, and warn if it is not honored.</div><div>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.</div></div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div><div><br></div><div>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.</div><div><br></div><div>For example, in CMake, I only know how to do it after looking at Rafael's <a href="https://github.com/espindola/llvm-scripts">https://github.com/espindola/llvm-scripts</a> (and I have no idea how Rafael figured it out).</div><div><br></div><div>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.</div><div><br></div><div>So my vote is that:</div><div><br></div><div>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.</div><div>2. We emit an error mentioning the opt-in flag</div><div><br></div><div>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.</div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> 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.</div></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div><div>-- Sean Silva</div></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_quote"><span><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote"><div>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.</div></div></div></div></blockquote><div><br></div></span><div>So yes we need to read all the archives.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>And you still don't address the "principle of least surprise": the configuration is *not* what is expected from the user.</div></blockquote><div><br></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div>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).</div></div></div></div></blockquote><div><br></div></span><div>That is why the warning is deserved.</div></div></div></blockquote><div><br></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div>But you no longer need it with this change.</div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_quote"><span class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023HOEnZb"><font color="#888888"><div>-- </div><div>Mehdi</div></font></span><span><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div><div><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><span class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></font></span></div><div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268HOEnZb"><div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268h5"><div class="gmail_extra"><br><div class="gmail_quote">2017-05-03 16:51 GMT-07:00 Rui Ueyama <span><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>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.<div><div><br></div><div>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.</div></div></div><div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268m_-8371825007347637933HOEnZb"><div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268m_-8371825007347637933h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 3, 2017 at 4:23 PM, Mehdi AMINI via Phabricator <span><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">mehdi_amini added a comment.<br>
<br>
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!<br>
<span><br>
> True, but on the other hand, it's pretty much the exact same work that the archiver would need to do,<br>
<br>
</span>The archiver do it once for potentially a lot of linker invocations.<br>
<span><br>
> and asking the user to change their archiver and rebuild would probably consume even more time.<br>
<br>
</span>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.<br>
<div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268m_-8371825007347637933m_-9189500792541224586HOEnZb"><div class="gmail-m_6803058981280818251m_-5105120536124157243m_2112540154002462270m_5271627582276695267m_-3168000319173216388m_-3516729254996079657m_6458433144927871023m_1695972829203370783m_4231870801368009268m_-8371825007347637933m_-9189500792541224586h5"><br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D32721" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3272<wbr>1</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div></div></div></blockquote></span></div></div>
</blockquote></div></div></div></blockquote></span></div></div>
</blockquote></div></div></div></blockquote></span></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>