<div dir="ltr"><div>On Wed, May 3, 2017 at 5:40 PM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br><div class="gmail_quote"><span class=""><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:0 0 0 .8ex;border-left:1px #ccc solid;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:0 0 0 .8ex;border-left:1px #ccc solid;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>How can that be faster than the regular build?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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 class=""><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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:0 0 0 .8ex;border-left:1px #ccc solid;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>But you no longer need it with this change.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="gmail_quote"><span class="HOEnZb"><font color="#888888"><div>-- </div><div>Mehdi</div></font></span><span class=""><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><span class="m_1695972829203370783m_4231870801368009268HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></font></span></div><div class="m_1695972829203370783m_4231870801368009268HOEnZb"><div class="m_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:0 0 0 .8ex;border-left:1px #ccc solid;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="m_1695972829203370783m_4231870801368009268m_-8371825007347637933HOEnZb"><div class="m_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:0 0 0 .8ex;border-left:1px #ccc solid;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="m_1695972829203370783m_4231870801368009268m_-8371825007347637933m_-9189500792541224586HOEnZb"><div class="m_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/<wbr>D32721</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><br></div></div>