<div dir="ltr">I think Chandler was talking about partial inlining in the PM, not the regular inline pass.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 26, 2017 at 9:44 AM, Mehdi AMINI via Phabricator <span dir="ltr"><<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>
<span class=""><br>
In <a href="https://reviews.llvm.org/D33540#765031" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33540#765031</a>, @chandlerc wrote:<br>
<br>
> In <a href="https://reviews.llvm.org/D33540#764937" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33540#764937</a>, @davidxl wrote:<br>
><br>
> > For partial inlining, leave it consistent with old PM for now.   We can introduce later an internal option to enable/disable pre-thin-link partial inlining and collect more performance number.<br>
><br>
><br>
> Is there a rationale for the position in the old PM? Was this discussed somewhere?<br>
<br>
<br>
</span>Do you really think I flipped a coin? `git log`/`git blame` easily find the original review that introduced the ThinLTO pipeline: <a href="http://reviews.llvm.org/D17115" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17115</a><br>
<span class=""><br>
> I'd almost rather adjust the position in the old PM to be consistent with here.<br>
<br>
</span>What data do you have? Your intuition alone is not a reason to go over the extensive tests we've been doing before setting it up the way it is. The pipeline is what it is now because I started with the same "intuition" and "rational" as you do, and tried to not perform inlining. However I did a bunch of measurement before landing it, and had to realize that (at that time at least), the bitcode was much larger, there were more functions to be imported, and it slowed down the compile time significantly (I'm not saying it is still the case, I don't know, but I don't see data here).<br>
<br>
See also later tweaking in:  <a href="http://reviews.llvm.org/D19773" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19773</a> that shifted more work from the thin-backend to the compile phase, which helped a lot building LLVM (since the compile-phase happens once per file while the Thin-backends run for every binary that is linked).<br>
<br>
<br>
<br>
================<br>
Comment at: include/llvm/Passes/<wbr>PassBuilder.h:256<br>
+  buildThinLTOPreLinkDefaultPipe<wbr>line(OptimizationLevel Level,<br>
+                                     bool DebugLogging = false);<br>
+<br>
----------------<br>
It is the first time I see "prelink" to describe the compile phase. I find this confusing. The "link" happens after ThinLTO, we never link the IR in ThinLTO.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33540" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33540</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>