[llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline

586 views
Skip to first unread message

Arthur Eubanks via llvm-dev

unread,
Aug 24, 2021, 1:44:25 PM8/24/21
to llvm-dev
The new pass manager has been on by default since the 13 branch. Now that we've branched for 14, I'd like to start the process of deprecating and removing legacy pass manager support for the optimization pipeline. This includes clang, opt, and lld LTO support.

Note that this doesn't apply to the codegen pipeline since there's no new pass manager support for that yet.

Are there any objections?

Fangrui Song via llvm-dev

unread,
Aug 24, 2021, 3:02:50 PM8/24/21
to Arthur Eubanks, llvm-dev

"deprecating" and 'removing" are different.

For 14.0.0, do you plan that -DLLVM_ENABLE_NEW_PASS_MANAGER=off will
give a warning or will be completely unsupported?
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Arthur Eubanks via llvm-dev

unread,
Aug 24, 2021, 3:10:16 PM8/24/21
to Fangrui Song, llvm-dev
I probably should have said "deprecating and removing".

My intention was to remove it completely. -DLLVM_ENABLE_NEW_PASS_MANAGER wouldn't do anything and we'd remove the -flegacy-pass-manaager/-fexperimental-new-pass-manager flags, as well as the corresponding lld flags.

Chris Tetreault via llvm-dev

unread,
Aug 24, 2021, 3:21:34 PM8/24/21
to Arthur Eubanks, Fangrui Song, llvm...@lists.llvm.org

Since deprecating something is giving warning that it’s going to be removed, I don’t think it’s fair to deprecate and remove in one step. If legacy pass manager isn’t currently formally deprecated (as in, loudly complains when you try to use it), then I’d personally like to see it deprecated for a release before any steps to completely remove it are taken.

 

From: llvm-dev <llvm-dev...@lists.llvm.org> On Behalf Of Arthur Eubanks via llvm-dev
Sent: Tuesday, August 24, 2021 12:10 PM
To: Fangrui Song <mas...@google.com>
Cc: llvm-dev <llvm...@lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline

 

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

Fāng-ruì Sòng via llvm-dev

unread,
Aug 24, 2021, 3:26:10 PM8/24/21
to Chris Tetreault, llvm...@lists.llvm.org
On Tue, Aug 24, 2021 at 12:21 PM Chris Tetreault <ctet...@quicinc.com> wrote:
>
> Since deprecating something is giving warning that it’s going to be removed, I don’t think it’s fair to deprecate and remove in one step. If legacy pass manager isn’t currently formally deprecated (as in, loudly complains when you try to use it), then I’d personally like to see it deprecated for a release before any steps to completely remove it are taken.

I just wanted to say the same thing:)

I don't mind that we install a warning for -DLLVM_ENABLE_NEW_PASS_MANAGER=off
in the release/13.x branch so that the signal is clearer to downstream users.

> From: llvm-dev <llvm-dev...@lists.llvm.org> On Behalf Of Arthur Eubanks via llvm-dev
> Sent: Tuesday, August 24, 2021 12:10 PM
> To: Fangrui Song <mas...@google.com>
> Cc: llvm-dev <llvm...@lists.llvm.org>
> Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
>
>
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> I probably should have said "deprecating and removing".
>
>
>
> My intention was to remove it completely. -DLLVM_ENABLE_NEW_PASS_MANAGER wouldn't do anything and we'd remove the -flegacy-pass-manaager/-fexperimental-new-pass-manager flags, as well as the corresponding lld flags.
>
>
>
> On Tue, Aug 24, 2021 at 12:02 PM Fangrui Song <mas...@google.com> wrote:
>
> On 2021-08-24, Arthur Eubanks via llvm-dev wrote:
> >The new pass manager has been on by default since the 13 branch. Now that
> >we've branched for 14, I'd like to start the process of deprecating and
> >removing legacy pass manager support for the optimization pipeline. This
> >includes clang, opt, and lld LTO support.
> >
> >Note that this doesn't apply to the codegen pipeline since there's no new
> >pass manager support for that yet.
> >
> >Are there any objections?
>
> "deprecating" and 'removing" are different.
>
> For 14.0.0, do you plan that -DLLVM_ENABLE_NEW_PASS_MANAGER=off will
> give a warning or will be completely unsupported?

--
宋方睿

Arthur Eubanks via llvm-dev

unread,
Aug 24, 2021, 4:47:28 PM8/24/21
to Fāng-ruì Sòng, llvm...@lists.llvm.org
If nobody is setting -DLLVM_ENABLE_NEW_PASS_MANAGER=off then I'm not sure that we need to go through the whole deprecation dance. Is there anybody doing that?

But if we do the deprecation dance, should it be a CMake warning or a clang warning? I'm not sure people will notice a CMake warning.
And would putting that in the 13.x branch be good enough to start removing after 14.x?

Chris Tetreault via llvm-dev

unread,
Aug 24, 2021, 6:39:07 PM8/24/21
to Arthur Eubanks, Fāng-ruì Sòng, llvm...@lists.llvm.org

Downstreams could be re-enabling the legacy pass manager in CMake. It’s still there, and still supported. If a downstream has made non-trivial changes to the pass pipeline for legacy pass manager, it’s going to be a ton of work to ensure performance parity when enabling new pass manager.

 

I think, if LLVM is built with LLVM_ENABLE_NEW_PASS_MANAGER set to OFF, then you should get a big scary warning at CMake configure time, but clang/opt should not complain. If LLVM is built with DLLVM_ENABLE_NEW_PASS_MANAGER set to ON, but the flag is passed to clang/opt to build using the legacy pass manager, you should get a warning from clang/opt.

 

In my opinion, if these deprecation warnings make it in for LLVM 13, then it’s fine to begin removing LPM in LLVM 14.

 

thanks,

   Chris Tetreault

Chris Tetreault via llvm-dev

unread,
Aug 24, 2021, 6:41:17 PM8/24/21
to Chris Tetreault, Arthur Eubanks, Fāng-ruì Sòng, llvm...@lists.llvm.org

Let me clarify that when I say “it’s fine to begin removing LPM in LLVM 14”, I mean that it’s fine to begin removing LPM once LLVM 14 is released, and the version string in main is set to 15.

Fāng-ruì Sòng via llvm-dev

unread,
Aug 24, 2021, 7:53:23 PM8/24/21
to Chris Tetreault, llvm...@lists.llvm.org
On 2021-08-24, Chris Tetreault wrote:
>Let me clarify that when I say “it’s fine to begin removing LPM in LLVM 14”, I mean that it’s fine to begin removing LPM once LLVM 14 is released, and the version string in main is set to 15.

While I think such a deprecation and removal policy may be fine for many non-trivial things, asking
this may be too much for the pass manager. With a grain of salt, "downstreams are on their own." New
PM migration and legacy PM removal has been repeatedly forewarned. I think llvm-project has done
above and beyond what it was expected. The flip in git was done in February.

After 13.0.0 is released for, say 2 months, which additional attests its robustness, I think
removing non-codegen legacy PM pieces should be fine.

>From: llvm-dev <llvm-dev...@lists.llvm.org> On Behalf Of Chris Tetreault via llvm-dev
>Sent: Tuesday, August 24, 2021 3:39 PM
>To: Arthur Eubanks <aeub...@google.com>; Fāng-ruì Sòng <mas...@google.com>
>Cc: llvm...@lists.llvm.org
>Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
>
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>Downstreams could be re-enabling the legacy pass manager in CMake. It’s still there, and still supported. If a downstream has made non-trivial changes to the pass pipeline for legacy pass manager, it’s going to be a ton of work to ensure performance parity when enabling new pass manager.
>
>I think, if LLVM is built with LLVM_ENABLE_NEW_PASS_MANAGER set to OFF, then you should get a big scary warning at CMake configure time, but clang/opt should not complain. If LLVM is built with DLLVM_ENABLE_NEW_PASS_MANAGER set to ON, but the flag is passed to clang/opt to build using the legacy pass manager, you should get a warning from clang/opt.
>
>In my opinion, if these deprecation warnings make it in for LLVM 13, then it’s fine to begin removing LPM in LLVM 14.
>
>thanks,
> Chris Tetreault
>

>From: Arthur Eubanks <aeub...@google.com<mailto:aeub...@google.com>>
>Sent: Tuesday, August 24, 2021 1:47 PM
>To: Fāng-ruì Sòng <mas...@google.com<mailto:mas...@google.com>>
>Cc: Chris Tetreault <ctet...@quicinc.com<mailto:ctet...@quicinc.com>>; llvm...@lists.llvm.org<mailto:llvm...@lists.llvm.org>; Tom Stellard <tste...@redhat.com<mailto:tste...@redhat.com>>
>Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
>
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>If nobody is setting -DLLVM_ENABLE_NEW_PASS_MANAGER=off then I'm not sure that we need to go through the whole deprecation dance. Is there anybody doing that?
>
>But if we do the deprecation dance, should it be a CMake warning or a clang warning? I'm not sure people will notice a CMake warning.
>And would putting that in the 13.x branch be good enough to start removing after 14.x?
>

>On Tue, Aug 24, 2021 at 12:26 PM Fāng-ruì Sòng <mas...@google.com<mailto:mas...@google.com>> wrote:


>On Tue, Aug 24, 2021 at 12:21 PM Chris Tetreault <ctet...@quicinc.com<mailto:ctet...@quicinc.com>> wrote:
>>
>> Since deprecating something is giving warning that it’s going to be removed, I don’t think it’s fair to deprecate and remove in one step. If legacy pass manager isn’t currently formally deprecated (as in, loudly complains when you try to use it), then I’d personally like to see it deprecated for a release before any steps to completely remove it are taken.
>
>I just wanted to say the same thing:)
>
>I don't mind that we install a warning for -DLLVM_ENABLE_NEW_PASS_MANAGER=off
>in the release/13.x branch so that the signal is clearer to downstream users.
>

>> From: llvm-dev <llvm-dev...@lists.llvm.org<mailto:llvm-dev...@lists.llvm.org>> On Behalf Of Arthur Eubanks via llvm-dev
>> Sent: Tuesday, August 24, 2021 12:10 PM
>> To: Fangrui Song <mas...@google.com<mailto:mas...@google.com>>
>> Cc: llvm-dev <llvm...@lists.llvm.org<mailto:llvm...@lists.llvm.org>>
>> Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
>>
>>
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>>
>> I probably should have said "deprecating and removing".
>>
>>
>>
>> My intention was to remove it completely. -DLLVM_ENABLE_NEW_PASS_MANAGER wouldn't do anything and we'd remove the -flegacy-pass-manaager/-fexperimental-new-pass-manager flags, as well as the corresponding lld flags.
>>
>>
>>
>> On Tue, Aug 24, 2021 at 12:02 PM Fangrui Song <mas...@google.com<mailto:mas...@google.com>> wrote:
>>
>> On 2021-08-24, Arthur Eubanks via llvm-dev wrote:
>> >The new pass manager has been on by default since the 13 branch. Now that
>> >we've branched for 14, I'd like to start the process of deprecating and
>> >removing legacy pass manager support for the optimization pipeline. This
>> >includes clang, opt, and lld LTO support.
>> >
>> >Note that this doesn't apply to the codegen pipeline since there's no new
>> >pass manager support for that yet.
>> >
>> >Are there any objections?
>>
>> "deprecating" and 'removing" are different.
>>
>> For 14.0.0, do you plan that -DLLVM_ENABLE_NEW_PASS_MANAGER=off will
>> give a warning or will be completely unsupported?
>
>
>
>--
>宋方睿

Chris Tetreault via llvm-dev

unread,
Aug 25, 2021, 12:38:20 PM8/25/21
to Fāng-ruì Sòng, llvm...@lists.llvm.org
Legacy pass manager is a very large piece of infrastructure in LLVM, and for that reason I think it's especially important to provide a good long deprecation period. Up until now, there's been no deadline for its removal, so for us to say "in 2 months passes are going to start getting deleted" is unfair. As it stands, NPM is a many-years-long project, and it's always been the case that it's going to replace LPM "soon", but there's never been a date. (if I'm wrong, please correct me) Given the level of effort required to migrate to new pass manager, it no surprise that users may not have done it until "soon" became "on this specific date". I know that "downstreams are on their own" is our policy, but it's also the case that we value not making things difficult for downstreams. I know I've repeatedly been told to back out changes due to push back from downstreams.

I don't think a 6 month deprecation period is a big ask. This RFC is essentially proposing "let's remove some dead code", so I don't see how waiting is going to block anybody's work. And as Björn mentioned in his other email (that I assume he meant to CC the lists on, but I'll refrain from quoting him verbatim), there are things we can get started on in the meantime, such as cleaning up the lit tests and changing CMake defaults. But please, let's not remove any actual code until we've had a formal deprecation period.

If the concern is that people will keep pushing code that uses LPM infrastructure, then we can mark it deprecated so the build bots reject it. Preferably we'd do some sort of `#define LLVM_NPM_ATTRIBUTE_DEPRECATED(decl, msg) LLVM_ATTRIBUTE_DEPRECATED(decl, msg)` thing so that downstreams can just redefine one thing instead of deleting a bunch of deprecation flags.

Thanks,
Chris Tetreault

Björn Pettersson A via llvm-dev

unread,
Aug 25, 2021, 1:09:59 PM8/25/21
to llvm-dev
Now with llvm-dev as receiver (thanks to Chris T for noticing the mistake).

-----Original Message-----
From: Björn Pettersson A
Sent: den 25 augusti 2021 16:26
To: Fāng-ruì Sòng <mas...@google.com>; Chris Tetreault <ctet...@quicinc.com>
Subject: RE: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline

> -----Original Message-----
> From: llvm-dev <llvm-dev...@lists.llvm.org> On Behalf Of Fang-ruì Sòng
> via llvm-dev
> Sent: den 25 augusti 2021 01:53
> To: Chris Tetreault <ctet...@quicinc.com>
> Cc: llvm...@lists.llvm.org
> Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the
> optimization pipeline
>
> On 2021-08-24, Chris Tetreault wrote:
> >Let me clarify that when I say “it’s fine to begin removing LPM in LLVM
> 14”, I mean that it’s fine to begin removing LPM once LLVM 14 is released,
> and the version string in main is set to 15.
>
> While I think such a deprecation and removal policy may be fine for many
> non-trivial things, asking
> this may be too much for the pass manager. With a grain of salt,
> "downstreams are on their own." New
> PM migration and legacy PM removal has been repeatedly forewarned. I think
> llvm-project has done
> above and beyond what it was expected. The flip in git was done in
> February.
>
> After 13.0.0 is released for, say 2 months, which additional attests its
> robustness, I think
> removing non-codegen legacy PM pieces should be fine.
>

Yes, since there are codegen passes using legacy PM we can't just removing
everything.

But maybe we can start cleaning up some things, such as:

a) Removing impact of LLVM_ENABLE_NEW_PASS_MANAGER on -enable-new-pm in opt.
Thus, making new-pm the default in opt (not depending on cmake config).
For testing with opt and legacy PM one would need an explicit -enable-new-pm=0.
(One impact of this is that lots of lit tests using opt would run the
new-PM version of the pass all the time. But for most tests, e.g. using
a single pass, the implementation in the pass should be independent
of the pass manager so that should hopefully not be a big loss.)

b) Given (a) we could start to clean up lit tests that run opt to use
-passes syntax.

c) Removing support for using the legacy syntax for specifying passes to
run in opt unless using -enable-new-pm=0.

d) Maybe we can remove the LLVM_ENABLE_NEW_PASS_MANAGER and
ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER completely. Forcing anyone that
still wants to use the legacy PM to use options such as
-flegacy-pass-manager and -enable-new-pm=0 rather than allowing
to build with different defaults.

The cleanups listed above are not really removing support for legacy PM,
it just starts to remove some of the compatibility hacks that has been
used to simplify the pass manager switch.

/Björn

PS. The things I mention above has been really helpful when doing the
migration to new-PM. If there still are project that haven't migrated,
then I understand that it might be harmful if we start cleaning up those
things. Although, I think that there have been lots of discussions and
RFC:s about the deprecation (and that one could expect cleanup to start
after the LLVM 13 branch out). The best way to find out if someone
still use things like LLVM_ENABLE_NEW_PASS_MANAGER could perhaps be by
try to remove it (and then if someone objects we can revert that patch
again).

Mehdi AMINI via llvm-dev

unread,
Aug 25, 2021, 1:31:11 PM8/25/21
to Fāng-ruì Sòng, llvm...@lists.llvm.org
On Tue, Aug 24, 2021 at 4:53 PM Fāng-ruì Sòng via llvm-dev <llvm...@lists.llvm.org> wrote:
On 2021-08-24, Chris Tetreault wrote:
>Let me clarify that when I say “it’s fine to begin removing LPM in LLVM 14”, I mean that it’s fine to begin removing LPM once LLVM 14 is released, and the version string in main is set to 15.

While I think such a deprecation and removal policy may be fine for many non-trivial things, asking
this may be too much for the pass manager.

Why is that? Is there a significant cost to keep the pass manager in LLVM for the next 5 months?
Otherwise it would be nice to release 14 with a release notes that indicates what will be removed in 15 on this aspect.

There are likely things that can be removed quickly already (like any uses of the LPM in clang/lld/...) and keep only the support in opt for now.

-- 
Mehdi

Philip Reames via llvm-dev

unread,
Aug 25, 2021, 2:22:25 PM8/25/21
to Arthur Eubanks, llvm-dev

I'd vote for immediate removal.  I don't have much sympathy for downstream consumers who haven't moved.  This effort has been underway for literal years.  Many - though not by any means all - downstream projects moved *before* upstream finally switched.  Let's put a nail in this coffin, and remove code aggressively.  

Supporting both has serious ongoing costs.  As a real example, I have twice spent time in the last two weeks tracking down some odd quirk of the unrolling implementation to find it supports some quirk of the legacy pass. That slows down development, compromises quality, and is generally a "bad thing".

We might want to wait a couple of weeks/months to ensure stability, but we should only consider the needs to the upstream project itself when doing so.  Giving downstream projects time to react should be an explicit non-goal. 

Philip

p.s. I don't expect this to be the actual decision reached, but since I only see opinions down-thread arguing for migration windows, I wanted to make a point of sharing the opposite opinion.  Fair warning, I probably won't reply to this thread further.  I don't have sufficient interest in the topic to make it worthwhile. 

Arthur Eubanks via llvm-dev

unread,
Aug 27, 2021, 3:42:02 PM8/27/21
to Chris Tetreault, llvm-dev
Are https://reviews.llvm.org/D108789 and https://reviews.llvm.org/D108775 sufficient if we cherrypick them into 13?

Chris Tetreault via llvm-dev

unread,
Aug 27, 2021, 4:44:23 PM8/27/21
to Arthur Eubanks, llvm-dev

I think that’s a sufficiently obnoxious warning. I still strongly prefer that no removals of functionality come until we branch for LLVM 14, but I think this will do for a notice of deprecation.

 

Thanks,

   Chris Tetreault

 

From: Arthur Eubanks <aeub...@google.com>
Sent: Friday, August 27, 2021 12:42 PM
To: Chris Tetreault <ctet...@quicinc.com>

Cc: llvm-dev <llvm...@lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline

 

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

Are https://reviews.llvm.org/D108789 and https://reviews.llvm.org/D108775 sufficient if we cherrypick them into 13?

Mehdi AMINI via llvm-dev

unread,
Aug 27, 2021, 6:55:07 PM8/27/21
to Arthur Eubanks, llvm-dev
These revisions are deprecating (IIUC):
1) Clang user flags to use the legacy pass manager.
2) The CMake build flag that defines the *default* pass manager.

These don't seem like the most impactful for downstream users, are they?

On the other hand, what seems to me to be critical to clarify is if you also intend to deprecate other things, like the use of the legacy pass manager in opt (or as a pass manager in a downstream tool/compiler)? Are we gonna also remove all the legacy adapters that make the passes work in the legacy pass manager as well?

This looks more important to me because this affects directly how other compilers are built on top of LLVM and push them to migrate, while the deprecation revision you sent may just have no effect on them.

Thanks,

-- 
Mehdi

Chris Tetreault via llvm-dev

unread,
Aug 27, 2021, 6:59:05 PM8/27/21
to Mehdi AMINI, Arthur Eubanks, llvm-dev

This is a good point. I’ve stated my preferences, but I think we can all agree that it would be good to settle on a concrete timeline of what’s getting removed, and when.

 

Thanks,

   Chris Tetreault

 

From: Mehdi AMINI <joke...@gmail.com>
Sent: Friday, August 27, 2021 3:54 PM
To: Arthur Eubanks <aeub...@google.com>

James Y Knight via llvm-dev

unread,
Aug 27, 2021, 7:34:16 PM8/27/21
to Mehdi AMINI, llvm-dev
Sounds to me like we need to have a short description of exactly what functionality is going to be deleted, and then put it at the top of the release notes for llvm 13.

Fāng-ruì Sòng via llvm-dev

unread,
Aug 27, 2021, 9:18:05 PM8/27/21
to Mehdi AMINI, llvm-dev
On Fri, Aug 27, 2021 at 3:55 PM Mehdi AMINI via llvm-dev
<llvm...@lists.llvm.org> wrote:
>
> These revisions are deprecating (IIUC):
> 1) Clang user flags to use the legacy pass manager.
> 2) The CMake build flag that defines the *default* pass manager.
>
> These don't seem like the most impactful for downstream users, are they?

The previous disabling mechanism
-DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=off is ignored in main and
release/13.x.
If a user switches (-DLLVM_ENABLE_NEW_PASS_MANAGER=off), they should
notice the warning.

So I think we don't need something like
-DLLVM_FORCE_USE_OLD_TOOLCHAIN=on which was done in 2019 to bump the
toolchain requirement.

--
宋方睿

Arthur Eubanks via llvm-dev

unread,
Aug 30, 2021, 6:24:19 PM8/30/21
to llvm-dev
What legacy PM stuff I'd like to remove soonish:

1) Support for the CMake flag DLLVM_ENABLE_NEW_PASS_MANAGER=off (which is related to defaults for the next 3 items)
2) LTO support in various linkers
3) Clang support for the optimization pipeline
4) opt support for translating `-instcombine` into `-passes=instcombine`, will require updating many tests
5) PassManagerBuilder for building an optimization pipeline with the legacy PM (requires all of the above to be done first)

I can add these to the 13.x release notes.

If there are any existing users right now who continuously test against LLVM trunk and would like some time to port legacy PM passes to the new PM I'm happy to wait a couple of weeks/a month (and happy to answer any questions), but otherwise we shouldn't worry about downstream users who don't speak up. Porting most passes is generally pretty quick and easy, but some passes can be tricky.
As mentioned earlier, having two ways to do something can result in confusion upstream when debugging issues. And code cleanup is nice. Also, a fair number of lit tests currently run against both pass managers and removing some RUN lines against the legacy PM can speed up tests.


What won't be removed anytime soon:
1) General legacy PM infrastructure, such as llvm::legacy::PassManager, since codegen still uses it
2) Most legacy passes, since some IR passes run as part of the codegen pipeline (although no reason to keep around some legacy passes like LTO-specific passes)
        Some backends, via TargetPassConfig::addIRPasses(), will add various passes to the codegen IR pipeline.
3) Testing legacy passes via opt, since again some IR passes run as part of the codegen pipeline
        However, I'd like to explicitly enumerate all the passes that we allow opt to run under the legacy PM. For example, target-specific codegen IR passes like "amdgpu-lower-ctor-dtor" should be run under the legacy PM via `opt -amdgpu-lower-ctor-dtor`. But most passes like instcombine should only be invokable via the `opt -passes=instcombine` syntax so that we don't have to worry about people testing the wrong pass manager by using the wrong syntax. Even if some passes like instcombine are added to the codegen pipeline in a legacy pass manager, I don't think the confusion of `opt -instcombine` vs `opt -passes=instcombine` is worth the test coverage we may gain by being able to run instcombine with the legacy PM. Perhaps if we find that some passes do regress only under the legacy PM and not the new PM we can revisit this point.

Chris Tetreault via llvm-dev

unread,
Aug 30, 2021, 8:16:39 PM8/30/21
to Arthur Eubanks, llvm-dev

For the record, I am a downstream user who is speaking up.

 

Migrating a customized downstream from legacy pass manager to new pass manager involves more than just porting a few passes. Several passes (such as the inliner) were rewritten from scratch. Effort was made to make the new versions of these rewritten passes match the original, but the interaction of the whole pass pipeline can be very fickle. What was determined to be good enough upstream might not be quite right in the face of downstream modifications. And if these rewritten passes were customized, then these customizations will be lost. Additionally, the construction of the pass pipeline has been fine tuned over years by the community, and by downstream users to produce good results, and mitigating regressions caused by switching to the new pass manager takes time. It’s easy to miss changes, and this can result in spending a bunch of time trying to figure out why some workload had a 3% regression.

 

I deal with this stuff every day at work, so I feel the pain of maintaining two pass infrastructures. However, I don’t understand what the hurry is. Who is blocked by legacy pass manager remaining operational? If maintaining the legacy pass manager is slowing the velocity of development in upstream, I think it’s reasonable to put it on life support and only fix correctness issues and egregious perf regressions. Downstreams that are ready can stop supporting it, but I don’t see why we can’t wait 6 more months to begin deleting code in upstream. I feel like “one whole release” is a pretty standard minimum period of time to deprecate major functionality in a software project. Sure, it’s been a long time coming, but legacy pass manager was never deprecated. No date was ever given for its removal.

 

I think that it is reasonable to state that legacy pass manager will be removed in the release of LLVM 14, and once the version string of main is set to 15 it’s open season to immediately begin gutting it. Prior to that, only a minimal level of effort need be made to maintain performance of the legacy pass manager. (i.e., downstreams who care about it can fix the bugs themselves) In the meantime, we can migrate the lit tests to new pass manager. New code that is added should prioritize the performance of the pass pipeline constructed with the new pass manager over that of the legacy pass manager. New passes that are added need not be ported to the legacy pass manager.  All I’m asking is that we not delete code until we’ve had a real deprecation period.

 

Thanks,

   Chris Tetreault

Arthur Eubanks via llvm-dev

unread,
Aug 31, 2021, 5:34:51 PM8/31/21
to Chris Tetreault, llvm-dev
Debugging new PM regressions is reasonable. I thought people were just taking too long to port passes. Given that argument I'm content to wait until the next branch.

We can start migrating lit tests as you say.

Chris Tetreault via llvm-dev

unread,
Aug 31, 2021, 5:55:05 PM8/31/21
to Arthur Eubanks, llvm-dev

I appreciate it. This will be super helpful for us!

Florian Hahn via llvm-dev

unread,
Sep 1, 2021, 9:30:48 AM9/1/21
to Arthur Eubanks, llvm-dev

> On 31 Aug 2021, at 23:34, Arthur Eubanks via llvm-dev <llvm...@lists.llvm.org> wrote:
>
> Debugging new PM regressions is reasonable. I thought people were just taking too long to port passes. Given that argument I'm content to wait until the next branch.
>
> We can start migrating lit tests as you say.


FWIW I think it makes sense to delay removing legacy PM code by six months, start moving lit tests now and also not require to investigate/fix legacy PM-only issues.

But I think it would be good to spell out the decision somewhere; maybe add a note that legacy PM support is due to be removed after the 14.0 release in the release notes on the 13.x and main branches? IMO that could be considered sufficient notice and we won’t be back to discussing whether to go ahead with the removal in 6 months again.

Cheers,
Florian

Reply all
Reply to author
Forward
0 new messages