This is an archive of the discontinued LLVM Phabricator instance.

[NewPM] Add extension points to LTO pipeline in PassBuilder
ClosedPublic

Authored by EliaGeretto on Feb 24 2022, 8:12 AM.

Details

Summary

This PR adds two extension points to the default LTO pipeline in PassBuilder, one at the beginning and one at the end. These two extension points already existed in the old pass manager, the aim is to replicate the same functionality in the new one.

Diff Detail

Event Timeline

EliaGeretto created this revision.Feb 24 2022, 8:12 AM
EliaGeretto requested review of this revision.Feb 24 2022, 8:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2022, 8:12 AM

seems reasonable to me
any tests for O1/O2?

I added tests for -O1 and -O2.

I think new-pm-lto-defaults.ll is better for the O1/O2 tests since we actually see where they are in the pipeline, see new-pm-thinlto-defaults.ll for inspiration
otherwise looks good

Given that new-pm-lto-defaults.ll was also not a great fit, I decided to move all the tests for the LTO extension points to a new new-pm-lto-ep-callbacks.ll file, which is very similar to new-pm-O0-ep-callbacks.ll but has a more meaningful name.

EliaGeretto retitled this revision from Add extension points to LTO pipeline in new pass manager to [NewPM] Add extension points to LTO pipeline in PassBuilder.Feb 25 2022, 1:21 AM
EliaGeretto edited the summary of this revision. (Show Details)

sorry for the nitpicking, but I was thinking of something like https://github.com/aeubanks/llvm-project/commit/e93062bac54abf72b49e58415d682adde1deec7c
seeing exactly where in the pipeline the extension point passes run is kinda nice and actually caused me to move it around a tiny bit (probably doesn't matter, but it's consistent with other default pipelines)

llvm/tools/opt/NewPMDriver.cpp
244

ModulePassManager, ditto below

No problem, I will add your changes and reupload the patch then. Also, for my benefit, why moving the Early extension point after the Annotation2MetadataPass? I was not sure about the proper position, so I am interested in knowing the logic, if you do not mind.

I have incorporated the changes proposed by @aeubanks in my patch. I have also moved the O0 tests back to new-pm-O0-ep-callbacks.ll.

This revision is now accepted and ready to land.Feb 25 2022, 10:45 AM

Also, looking at the correction you made in NewPMDriver.cpp, I have noticed that the same issue is present for the OptimizerLast extension point as well. It accepts module passes, but it is tested with a function pass.

do you need me to push this for you?

Also, looking at the correction you made in NewPMDriver.cpp, I have noticed that the same issue is present for the OptimizerLast extension point as well. It accepts module passes, but it is tested with a function pass.

ah I can fix that separately

Yes, please, I don't have commit access. You can use Elia Geretto <elia.f.geretto@gmail.com> for the Author field. Also, I already have a patch for the OptimizerLast stuff, I am going to submit that one as well separately.

This revision was landed with ongoing or failed builds.Feb 25 2022, 2:49 PM
This revision was automatically updated to reflect the committed changes.