Page MenuHomePhabricator

Define the ThinLTO Pipeline
ClosedPublic

Authored by mehdi_amini on Feb 10 2016, 4:59 PM.

Details

Summary

On the contrary to Full LTO, ThinLTO can afford to shift compile time
from the frontend to the linker: both phases are parallel.
This pipeline is based on the proposal in D13443 for full LTO. We ]
didn't move forward on this proposal because the link was far too long
after that.

This patch refactor the "function simplification" passes that are part
of the inliner loop in a helper function (this part is NFC and can be
commited separately to simplify the diff). The ThinLTO pipeline
integrates in the regular O2/O3 flow:

  • The compile phase perform the inliner with a somehow lighter function simplification. (TODO: tune the inliner thresholds here) This is intendend to simplify the IR and get rid of obvious things like linkonce_odr that will be inlined.
  • The link phase will run the pipeline from the start, extended with some specific passes that leverage the augmented knowledge we have during LTO. Especially after the inliner is done, a sequence of globalDCE/globalOpt is performed, followed by another run of the "function simplification" passes.

The measurements on the public test suite as well as on our internal
suite show an overall net improvement. The binary size for the clang
executable is reduced by 5%. We're still tuning it with the bringup
of ThinLTO but this should provide a good starting point.

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Define the ThinLTO Pipeline.
mehdi_amini updated this object.
mehdi_amini added a reviewer: tejohnson.
mehdi_amini added subscribers: dexonsmith, llvm-commits.
mgrang added a subscriber: mgrang.Feb 10 2016, 5:05 PM
mgrang added inline comments.
lib/Transforms/IPO/PassManagerBuilder.cpp
362

Period required at the end of comment.

429

Period required at the end of comment.

432

Period required at the end of comment.

Upload a new diff rebased on top of the NFC changes to reduce the diff.

mehdi_amini added inline comments.Feb 10 2016, 5:13 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
356

I can fix these comments as part of the NFC part of the changes (I updated the diff to rebase on the existing code after refactoring).
(I plan to commit separately and the comments are like that in trunk right now)

davidxl added a subscriber: xur.Feb 10 2016, 11:05 PM
davidxl added inline comments.Feb 10 2016, 11:09 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
366–369

This needs to be run in PrepareForThinLTO. During PerformThinLTO, only cross module indirect call promotion transformation needs to be done here.

tejohnson added inline comments.Feb 11 2016, 8:55 AM
lib/Transforms/IPO/PassManagerBuilder.cpp
366–369

Right, this should be guarded by (!PerformThinLTO).

I think the IC promotion pass is not yet committed. But eventually we will add another round of it just after FunctionImport.

393

typo: s/performs/perform/

710

Don't we also need to do some of the LTO passes here? E.g. FunctionImporting is added in addLTOOptimizationPasses.

mehdi_amini added inline comments.Feb 11 2016, 9:04 AM
lib/Transforms/IPO/PassManagerBuilder.cpp
366–369

Good point, I didn't pay much attention to PGO, I'll update.

710

Yes we can add it the same way it is done in the LTO pipeline, i.e. guarded by the presence of the FunctionIndex.

Right now I rather have the logic in the linker plugin and separate the import from the optimization. This is also required for the incremental scheme (see the beginning of ProcessThinLTOModule () in D17066).

Should take into account all the comments

mehdi_amini marked 10 inline comments as done.Feb 12 2016, 10:26 AM
tejohnson added inline comments.Feb 12 2016, 2:14 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
710

Ok, for now I suppose you can just set the FunctionIndex to null before invoking this.

I haven't done a full comparison between the LTO pipeline and the new ThinLTO pipeline here, is there anything else done in the LTO pipeline that is worth adding to this, or is it all covered by the module passes? Looks like it from my quick scan but I didn't compare extensively...

Also, after this goes in clang should be changed to invoke this instead of populateLTOPassManager in EmitAssemblyHelper::CreatePasses when we have a function index. And I can change my gold threads patch for ThinLTO (D15390) to do the same for ThinLTO compiles.

mehdi_amini added inline comments.Feb 14 2016, 9:12 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
710

I tend to be data driven, so we're looking at benchmarks and tracking issues right now, but this is a good starting point.

tejohnson accepted this revision.Feb 16 2016, 9:09 AM
tejohnson edited edge metadata.

LGTM

This revision is now accepted and ready to land.Feb 16 2016, 9:09 AM