This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager
ClosedPublic

Authored by MaskRay on Dec 8 2020, 10:49 PM.

Details

Summary

The new PM is considered stable and many downstream groups have adopted it (some
have adopted it for more than two years). Add -f[no-]legacy-pass-manager to reflect the
fact that it is no longer experimental and the legacy pass manager is something we strive to retire.

In the future, when the legacy PM eventually goes away,
-fno-experimental-new-pass-manager and -flegacy-pass-manager will be removed.

This patch also changes -f[no-]legacy-pass-manager to pass -plugin-opt={new,legacy}-pass-manager to the linker (supported by both ld.lld and LLVMgold.so) when -flto/-flto=thin is specified

Diff Detail

Event Timeline

MaskRay created this revision.Dec 8 2020, 10:49 PM
MaskRay requested review of this revision.Dec 8 2020, 10:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2020, 10:49 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay updated this revision to Diff 310438.Dec 9 2020, 12:03 AM

Improve tests

aeubanks accepted this revision.Dec 9 2020, 9:27 AM

Makes sense to me, thanks!

This revision is now accepted and ready to land.Dec 9 2020, 9:27 AM
rsmith added a comment.Dec 9 2020, 1:51 PM

I don't like having new in the name of the flag; I think that's going to age badly (and I would argue it already has done; the "new pass manager" isn't really all that new any more).

Given that the new pass manager is considered stable and not experimental, can we change our default and instead add a flag to request the old behavior? If we're not ready for that yet, then perhaps the "experimental" label still fits.

MaskRay updated this revision to Diff 310654.Dec 9 2020, 2:09 PM
MaskRay retitled this revision from [Driver] Add -f[no-]new-pass-manager to supersede -f[no-]experimental-new-pass-manager to [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager.
MaskRay edited the summary of this revision. (Show Details)

Add -f[no-]legacy-pass-manager instead

rsmith accepted this revision.Dec 9 2020, 4:45 PM

Thanks!

@rsmith highlit the coroutine problem https://bugs.llvm.org/show_bug.cgi?id=48190 (" ... it breaks support for a C++20 language feature (coroutines)")

Perhaps this means that "EXPERIMENTAL" in ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER should not be removed until it is fixed?

Thanks!

@rsmith highlit the coroutine problem https://bugs.llvm.org/show_bug.cgi?id=48190 (" ... it breaks support for a C++20 language feature (coroutines)")

Perhaps this means that "EXPERIMENTAL" in ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER should not be removed until it is fixed?

I'm looking at this right now, it's the last issue, aside from AMDGPU passes not being ported and some code size regressions, blocking the new pass manager.

MaskRay updated this revision to Diff 310720.Dec 9 2020, 4:54 PM
MaskRay edited the summary of this revision. (Show Details)

-plugin-opt=no-new-pass-manager => -plugin-opt=legacy-pass-manager

This revision was landed with ongoing or failed builds.Dec 9 2020, 4:57 PM
This revision was automatically updated to reflect the committed changes.

that test fails when the NPM is turned on and lit isn't aware of it :)

clang/include/clang/Driver/Options.td
1354–1361

this turns on the new pass manager for clang

MaskRay marked an inline comment as done.Dec 9 2020, 5:19 PM
MaskRay added inline comments.
clang/include/clang/Driver/Options.td
1354–1361
clang/test/Driver/fnew-pass-manager.c