This is an archive of the discontinued LLVM Phabricator instance.

Testing instcombine/licm pass order...
AbandonedPublic

Authored by mnadeem on Oct 3 2021, 2:50 PM.

Details

Reviewers
None
Summary

Move the call around in function simplification so that LICM is run between Instcombine and loop unroll/vectorize
and add an extra call at the end of vector passes to align with non lto flow.

Diff Detail

Event Timeline

mnadeem created this revision.Oct 3 2021, 2:50 PM
mnadeem requested review of this revision.Oct 3 2021, 2:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2021, 2:50 PM

This generally makes sense to me, but can this be split up please?

nikic added a subscriber: nikic.Oct 3 2021, 3:21 PM
nikic added inline comments.
llvm/lib/Passes/PassBuilderPipelines.cpp
561

This LICM run is currently placed to reuse the MSSA analysis computed for MemCpyOpt and DSE. While the LTO change looks reasonable to me, the compile-time impact of this move is unlikely to be worthwhile under global consideration.

mnadeem added inline comments.Oct 4 2021, 4:08 PM
llvm/lib/Passes/PassBuilderPipelines.cpp
561

Looking at the changes in clang/test/CodeGen/thinlto-distributed-newpm.ll it does not look like MemorySSA is being run again.

Does simplifycfg or instcombine invalidate the MSSA analysis?

asbirlea added inline comments.Oct 4 2021, 4:29 PM
llvm/lib/Passes/PassBuilderPipelines.cpp
561

Neither simplifycfg nor instcombine updates MSSA.

nikic added inline comments.Oct 5 2021, 1:04 AM
llvm/lib/Passes/PassBuilderPipelines.cpp
561

With the new pass manager, the pipeline tests no longer tell you whether analyses are invalidated (because invalidation is conditional, and pipeline tests run on trivial code). Only way to tell is by checking which analyses are declared as preserved, or empirically by checking compile-time impact.

ormris removed a subscriber: ormris.Jan 24 2022, 11:36 AM
mnadeem abandoned this revision.Jan 17 2023, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2023, 11:55 AM