This is an archive of the discontinued LLVM Phabricator instance.

[NewPM] Separate out alias analysis passes in opt
ClosedPublic

Authored by aeubanks on Jun 24 2020, 11:45 AM.

Details

Summary

This somewhat matches the --aa-pipeline option, which separates out any
AA analyses to make sure they run before other passes.

Makes check-llvm failures under new PM go from 2356 -> 2303.

AA passes are not handled by PassBuilder::parsePassPipeline() but rather
PassBuilder::parseAAPipeline(), which is why this fixes some failures.

Diff Detail

Event Timeline

aeubanks created this revision.Jun 24 2020, 11:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 24 2020, 11:45 AM
ychen added a comment.Jun 24 2020, 1:23 PM

LGTM with one nit.

llvm/include/llvm/Passes/PassBuilder.h
520

/// For compatibility with legacy pass manager. I would move this above the introduced loop in opt.cpp. Probably adding something like AA analyses are not specified separately when using legacy PM.

aeubanks updated this revision to Diff 273164.Jun 24 2020, 2:31 PM

Address review

A general comment on the series of patches: it's great to have this means of testing the NPM through the legacy way of specifying passes, but I believe the long term goal is to only rely on specifying via passes=.... Most tests will need to be updated to get there, so having this mass testing available now is very useful!
Just note that when the transition is made to only use passes=... to opt, all additions made solely for matching the legacy -foo-pass arguments will need to be cleaned up (e.g. the API added here in PassBuilder.h)

llvm/tools/opt/NewPMDriver.cpp
331

Could you assert AAPipeline and AA passes inside Passes don't co-exist?

aeubanks updated this revision to Diff 273197.Jun 24 2020, 5:33 PM

Add assert

asbirlea accepted this revision.Jun 24 2020, 7:58 PM

Thank you!

This revision is now accepted and ready to land.Jun 24 2020, 7:58 PM
This revision was automatically updated to reflect the committed changes.