This is an archive of the discontinued LLVM Phabricator instance.

[LTO] Support new PM in ThinLTOCodeGenerator.
ClosedPublic

Authored by fhahn on May 17 2021, 7:35 AM.

Details

Summary

This patch adds initial support for using the new pass manager when
doing ThinLTO via libLTO.

Diff Detail

Event Timeline

fhahn created this revision.May 17 2021, 7:35 AM
fhahn requested review of this revision.May 17 2021, 7:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2021, 7:35 AM

That is easier than I think. Is there still any missing function in terms of NewPM after this patch?

llvm/lib/LTO/ThinLTOCodeGenerator.cpp
533

There is another instance of optimizeModule. Maybe update that as well?

llvm/test/ThinLTO/X86/diagnostic-handler-remarks.ll
23

Why this test case needs to be changed for switch PM?

fhahn updated this revision to Diff 345985.May 17 2021, 1:19 PM

clang-format, also update second use or optimizeModule.

fhahn marked an inline comment as done.May 17 2021, 1:21 PM
fhahn added a subscriber: thegameg.
fhahn added inline comments.
llvm/test/ThinLTO/X86/diagnostic-handler-remarks.ll
23

The new PM does inlining a bit different, that's where the difference comes from I think. Perhaps @thegameg has any additional insight in the test case?

thegameg added inline comments.May 17 2021, 2:14 PM
llvm/test/ThinLTO/X86/diagnostic-handler-remarks.ll
23

Looks like we went from foo inlined into main to bar inlined into foo. Maybe we now have two remarks: bar inlined into foo + foo inlined into main?

fhahn updated this revision to Diff 350622.Jun 8 2021, 9:04 AM

Updated to also add the -debug-pass-manager option, so it is easier to check that -use-new-pm works as expected. I also pinned the remarks tests to use the legacy PM and updated llvm/test/ThinLTO/X86/newpm-basic.ll to check that the new PM is run via the debug output.

That is easier than I think. Is there still any missing function in terms of NewPM after this patch?

It's still not unified with the new LTO API, so if anything in the pipeline setup changes in the new LTO API we have to also update the code here. But other than that, ThinLTO should use the NewPM same as with the new API.

fhahn added inline comments.Jun 8 2021, 9:06 AM
llvm/test/ThinLTO/X86/diagnostic-handler-remarks.ll
23

Yep, but I don't think this change is really relevant for the switch to the new PM, as different inlining decisions are just a side-effect of using the new PM. In the latest version I pinned the remarks tests to use the legacy PM and updated llvm/test/ThinLTO/X86/newpm-basic.ll to check that -use-new-pm works as expected.

This revision is now accepted and ready to land.Jun 8 2021, 1:56 PM
This revision was landed with ongoing or failed builds.Jun 9 2021, 2:08 AM
This revision was automatically updated to reflect the committed changes.