This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Indirect call promotion fixes for promoted local functions
ClosedPublic

Authored by tejohnson on Aug 29 2016, 2:11 PM.

Details

Summary

Fix a couple issues limiting the application of indirect call promotion
in ThinLTO mode:

  • Invoke indirect call promotion before globalopt, since it may eliminate imported functions which appear unreferenced.
  • Invoke indirect call promotion with InLTO=true so that the PGOFuncName metadata is used to get the name for locals which would have been renamed during promotion.

Diff Detail

Event Timeline

tejohnson updated this revision to Diff 69615.Aug 29 2016, 2:11 PM
tejohnson retitled this revision from to [ThinLTO] Indirect call promotion fixes for promoted local functions.
tejohnson updated this object.
tejohnson added reviewers: davidxl, mehdi_amini.
tejohnson added subscribers: llvm-commits, Prazek.
davidxl added inline comments.Aug 29 2016, 2:17 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
412

Is there PGOInstr/Annotation pass before this?

tejohnson added inline comments.Aug 29 2016, 2:21 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
412

In the compile step. PerformThinLTO is true when we are in the ThinLTO backends. See also further down where we guard the invocation of addPGOInstrPasses with !PerformThinLTO.

SGTM, couldn't find anything unclear.

davidxl added inline comments.Aug 29 2016, 2:32 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
412

Ok. Can you add a comment here documenting this: with thinLTO, there are two icall promotion pass. One is in the compile phase where PerformThinLTO is false to handle intra-module calls, the other one is here that does cross module ones?

mehdi_amini accepted this revision.Aug 29 2016, 2:32 PM
mehdi_amini edited edge metadata.

LGTM as well.

lib/Transforms/IPO/PassManagerBuilder.cpp
376

Spurious empty line?

440

Do we want to do this in the compile phase?

This revision is now accepted and ready to land.Aug 29 2016, 2:32 PM
tejohnson added inline comments.Aug 29 2016, 3:48 PM
lib/Transforms/IPO/PassManagerBuilder.cpp
376

Will remove

412

Will do.

440

Yes, we want to have one in the compile phase to catch intra-module indirect calls (that will subsequently be eligible for inlining during the compile phase).

This revision was automatically updated to reflect the committed changes.