Chris Tetreault via llvm-dev
2021-Aug-31 00:16 UTC
[llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
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
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Arthur
Eubanks via llvm-dev
Sent: Monday, August 30, 2021 3:24 PM
To: llvm-dev <llvm-dev at 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.
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.
On Fri, Aug 27, 2021 at 6:17 PM Fāng-ruì Sòng <maskray at
google.com<mailto:maskray at google.com>> wrote:
On Fri, Aug 27, 2021 at 3:55 PM Mehdi AMINI via llvm-dev
<llvm-dev at lists.llvm.org<mailto:llvm-dev at 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.
> 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
>
>
> On Fri, Aug 27, 2021 at 1:44 PM Chris Tetreault via llvm-dev <llvm-dev
at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
>>
>> 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 <aeubanks at google.com<mailto:aeubanks at
google.com>>
>> Sent: Friday, August 27, 2021 12:42 PM
>> To: Chris Tetreault <ctetreau at quicinc.com<mailto:ctetreau at
quicinc.com>>
>> Cc: llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at
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?
>>
>>
>>
>> On Wed, Aug 25, 2021 at 11:22 AM Philip Reames <listmail at
philipreames.com<mailto:listmail at philipreames.com>> wrote:
>>
>> 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.
>>
>> On 8/24/21 10:44 AM, 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?
>>
>>
>>
>> _______________________________________________
>>
>> LLVM Developers mailing list
>>
>> llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
>>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
--
宋方睿
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210831/3ee912c8/attachment-0001.html>
Arthur Eubanks via llvm-dev
2021-Aug-31 21:34 UTC
[llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
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. On Mon, Aug 30, 2021 at 5:16 PM Chris Tetreault <ctetreau at quicinc.com> wrote:> 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 > > > > *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of *Arthur > Eubanks via llvm-dev > *Sent:* Monday, August 30, 2021 3:24 PM > *To:* llvm-dev <llvm-dev at 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. > > 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. > > > > On Fri, Aug 27, 2021 at 6:17 PM Fāng-ruì Sòng <maskray at google.com> wrote: > > On Fri, Aug 27, 2021 at 3:55 PM Mehdi AMINI via llvm-dev > <llvm-dev at 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. > > > 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 > > > > > > On Fri, Aug 27, 2021 at 1:44 PM Chris Tetreault via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> > >> 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 <aeubanks at google.com> > >> Sent: Friday, August 27, 2021 12:42 PM > >> To: Chris Tetreault <ctetreau at quicinc.com> > >> Cc: llvm-dev <llvm-dev at 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? > >> > >> > >> > >> On Wed, Aug 25, 2021 at 11:22 AM Philip Reames < > listmail at philipreames.com> wrote: > >> > >> 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. > >> > >> On 8/24/21 10:44 AM, 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? > >> > >> > >> > >> _______________________________________________ > >> > >> LLVM Developers mailing list > >> > >> llvm-dev at lists.llvm.org > >> > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > -- > 宋方睿 > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210831/03b9234d/attachment.html>
Florian Hahn via llvm-dev
2021-Sep-01 13:30 UTC
[llvm-dev] [RFC] Deprecating the legacy pass manager for the optimization pipeline
> On 31 Aug 2021, at 23:34, Arthur Eubanks via llvm-dev <llvm-dev at 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