[PATCH] D33540: [PM/ThinLTO] Port the ThinLTO pipeline (both components) to the new PM.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D33540#765031, @chandlerc wrote:

> In https://reviews.llvm.org/D33540#764937, @davidxl wrote:
>
> > 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.
>
>
> Is there a rationale for the position in the old PM? Was this discussed somewhere?


Do you really think I flipped a coin? `git log`/`git blame` easily find the original review that introduced the ThinLTO pipeline: http://reviews.llvm.org/D17115

> I'd almost rather adjust the position in the old PM to be consistent with here.

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).

See also later tweaking in:  http://reviews.llvm.org/D19773 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).



================
Comment at: include/llvm/Passes/PassBuilder.h:256
+  buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level,
+                                     bool DebugLogging = false);
+
----------------
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.


https://reviews.llvm.org/D33540