Page MenuHomePhabricator

[ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
ClosedPublic

Authored by tejohnson on Apr 23 2019, 8:35 AM.

Details

Summary

The opt level was not being passed down to the ThinLTO backend when
invoked via clang (for distributed ThinLTO).

This exposed an issue where the new PM was asserting if the Thin or
regular LTO backend pipelines were invoked with -O0 (not a new issue,
could be provoked by invoking in-process *LTO backends via linker using
new PM and -O0). Fix this similar to the old PM where -O0 only does the
necessary lowering of type metadata (WPD and LowerTypeTest passes) and
then quits, rather than asserting.

Event Timeline

tejohnson created this revision.Apr 23 2019, 8:35 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 23 2019, 8:35 AM
mehdi_amini added inline comments.Apr 23 2019, 9:17 AM
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

This is intended? I'm surprised the two PMs don't have the same list of passes for a given opt level?

tejohnson marked an inline comment as done.Apr 23 2019, 9:22 AM
tejohnson added a subscriber: chandlerc.
tejohnson added inline comments.
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

I'm really not sure. I did compare the post-link LTO pipelines of both PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes than the new PM at O1. Probably a question for @chandlerc ? In any case, I didn't want to address it here since it is orthogonal.

tejohnson marked an inline comment as done.Apr 23 2019, 9:39 AM
tejohnson added inline comments.
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

Some more info:

This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses essentially the same as a normal opt pipeline so would be consistent at -O1.

The optimization missing from the new PM regular LTO post link pipeline that affects this test is SimplifyCFG. This and a few other optimizations are added in the old PM at O1 via PassManagerBuilder::addLateLTOOptimizationPasses. Note that PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 (similar to where we exit early in the new PM for the regular LTO post link compilation). But the "late" LTO optimization passes are added unconditionally above -O0. Is this correct/desired? There isn't an equivalent in the new PM.

chandlerc added inline comments.Apr 23 2019, 10:17 AM
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

I don't think it was intentional, but I'm not sure I would directly replicate it if it requires adding an entirely new customization point. Is their some simpler way of getting equivalent results at O1?

tejohnson marked an inline comment as done.Apr 23 2019, 10:32 AM
tejohnson added inline comments.
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

Yeah we can presumably just add those optimizations to the -O1 early exit path in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would like to get this one in as it is a bugfix and orthogonal).

xur accepted this revision.Apr 23 2019, 10:46 AM

LGTM. We need to Initialize the OptLevel no matter what.

This revision is now accepted and ready to land.Apr 23 2019, 10:46 AM
mehdi_amini added inline comments.Apr 23 2019, 11:07 AM
llvm/test/tools/gold/X86/opt-level.ll
53 ↗(On Diff #196263)

(I fully agree it is orthogonal, I just spotted this as a separate bug indeed, thanks for fixing!)

ormris added a subscriber: ormris.Apr 23 2019, 11:21 AM
This revision was automatically updated to reflect the committed changes.