<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 10, 2016 at 5:01 PM, Mehdi AMINI 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mehdi_amini added inline comments.<br>
<span><br>
<br>
================<br>
Comment at: ELF/Driver.cpp:487<br>
+    error("--lto-partitions: number of threads must be > 0");<br>
+  Config->ThinLtoJobs = getInteger(Args, OPT_thinlto_jobs, 1);<br>
+  if (Config->ThinLtoJobs == 0)<br>
----------------<br>
</span><span>pcc wrote:<br>
> mehdi_amini wrote:<br>
> > pcc wrote:<br>
> > > I think this would result in a default parallelism level of 1. Can we avoid passing a ThinBackend if the user did not specify a parallelism level?<br>
> > Doesn't it mean that if the user does not specify `--thinlto-jobs` on the command line, ThinLTO won't work? I think it would even lead to the LTO library crashing if the bitcode has been generated with ThinLTO? Is there a test that shows what happen in this case? (I don't see any, but I didn't look closely)<br>
> > The default should be something like `std::hardware_concurrency`.<br>
> No, if the ThinBackend is not supplied, LTO will create one for you. See: <a href="http://llvm-cs.pcc.me.uk/lib/LTO/LTO.cpp#229" rel="noreferrer" target="_blank">http://llvm-cs.pcc.me.uk/lib/L<wbr>TO/LTO.cpp#229</a><br>
><br>
> I think we will eventually want to replace the `thread::hardware_concurrency(<wbr>)` default with something else that will lead to better utilisation of the hardware. In Chromium at least we've observed that `hardware_concurrency` leads to poor utilisation on machines with a large number of cores (<a href="https://bugs.chromium.org/p/chromium/issues/detail?id=645295#c20" rel="noreferrer" target="_blank">https://bugs.chromium.org/p/c<wbr>hromium/issues/detail?id=64529<wbr>5#c20</a>). So I'd prefer to keep any default out of the clients.<br>
><br>
> You're right, we should have a test that does not pass a `--thinlto-jobs` flag. @davide can you please add one?<br>
> No, if the ThinBackend is not supplied, LTO will create one for you. See: <a href="http://llvm-cs.pcc.me.uk/lib/LTO/LTO.cpp#229" rel="noreferrer" target="_blank">http://llvm-cs.pcc.me.uk/lib/L<wbr>TO/LTO.cpp#229</a><br>
<br>
</span>OK! Great :)<br>
<span><br>
> I think we will eventually want to replace the thread::hardware_concurrency() default with something else that will lead to better utilisation of the hardware. In Chromium at least we've observed that hardware_concurrency leads to poor utilisation on machines with a large number of cores (<a href="https://bugs.chromium.org/p/chromium/issues/detail?id=645295#c20" rel="noreferrer" target="_blank">https://bugs.chromium.org/p/c<wbr>hromium/issues/detail?id=64529<wbr>5#c20</a>). So I'd prefer to keep any default out of the clients.<br>
<br>
</span>Yes, Apple's clang is using the number of physical cores because of that, but my rational was to limit the memory consumption at a time where debug info where even more costly than now for ThinLTO.<br>
So indeed we plan to add support for this in LLVM.<br></blockquote><div><br></div><div>I've looked at this a bit a few weeks ago and had a lot of trouble finding a reliable way to compute the number of physical cores on X86, at least with Linux. There is a way to compute this from "lscpu -p", but I haven't found a good way to compute this from source code that isn't very architecture/implementation dependent. Mehdi indicated that MacOS has a method for accessing this info that he is using for Xcode.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On the other hand, David and Teresa benchmarks building Chromium on a 16-cores Xeon machine (32 hyper threaded cores) and they reported 2m18s for the ThinLTO plugin with 32 threads vs  3m38s with 32 threads. So your link is puzzling...<br></blockquote><div><br></div><div>From follow-on discussion it looks like you were looking at the cc_unittest numbers, and the numbers above we collected didn't include the thin link, so they aren't too far off IIUC.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="m_-4749080140227386598m_-1671767252507034069HOEnZb"><div class="m_-4749080140227386598m_-1671767252507034069h5"><br>
<br>
<a href="https://reviews.llvm.org/D25452" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2545<wbr>2</a><br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="m_-4749080140227386598m_-1671767252507034069gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:408-460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</div></div>