[llvm-dev] [RFC] Changing the default pass manager for the optimization pipeline

[Arthur Eubanks via llvm-dev]

Hi all,

We've been fixing the various remaining issues in order to turn on the new pass manager for the optimization pipeline, and it's about time to turn it on. (Thanks to everyone who has helped with testing and fixing the new pass manager!)

https://reviews.llvm.org/D95380 is the change that would happen, which sets the CMake flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=ON by default. This affects anything that uses the LLVM_ENABLE_NEW_PASS_MANAGER macro, which includes opt's handling of the `opt -instcombine` syntax, clang, and ThinLTO in lld drivers. This does not affect the backend target-specific codegen pipeline since that's mostly not been ported to use the new PM infrastructure yet.

Here is the umbrella bug for turning on the new PM with blockers. The main one is loop unswitching on divergent loop conditions is unsafe, which is being looked into. There's also the LLVM C API and bugpoint that still use the legacy PM, which can be ported later on and only block the removal of the legacy PM. The C API can be worked through (we may need to introduce replacements to the legacy pass manager APIs), but bugpoint will be tricky since it has so many legacy PM-specific hacks and we may need to trim it down if we want it to work with the new PM. Anyway, I don't think any of the remaining blockers are large enough to block the switch (but comments welcome).

I'd like to turn on the new PM by default soonish, after the 12.x branch. Perhaps roughly a week from now barring any major newly discovered regressions?

As for potential issues only uncovered after the switch, if there is a large issue I will roll it back, but for smaller issues I'd rather ask users to pin to the legacy PM while we fix the issues, either via the CMake flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=OFF, or the corresponding compiler flags, like -flegacy-pass-manager for clang.

Any concerns/comments?

[Fangrui Song via llvm-dev]

Forked the thread and changed the subject to talk about bugpoint specifically.
CCed folks on https://bugs.llvm.org/show_bug.cgi?id=48813

On 2021-01-26, Arthur Eubanks via llvm-dev wrote:
>Hi all,
>
>We've been fixing the various remaining issues in order to turn on the new
>pass manager for the optimization pipeline, and it's about time to turn it
>on. (Thanks to everyone who has helped with testing and fixing the new pass
>manager!)
>
>https://reviews.llvm.org/D95380 is the change that would happen, which sets
>the CMake flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=ON by default. This
>affects anything that uses the LLVM_ENABLE_NEW_PASS_MANAGER macro, which
>includes opt's handling of the `opt -instcombine` syntax, clang, and
>ThinLTO in lld drivers. This does not affect the backend target-specific
>codegen pipeline since that's mostly not been ported to use the new PM
>infrastructure yet.
>

>Here <https://bugs.llvm.org/show_bug.cgi?id=46649> is the umbrella bug for

>turning on the new PM with blockers. The main one is loop unswitching on
>divergent loop conditions is unsafe

><https://bugs.llvm.org/show_bug.cgi?id=48819>, which is being looked into.

>There's also the LLVM C API and bugpoint that still use the legacy PM,
>which can be ported later on and only block the removal of the legacy PM.

I have heard several users said they just use

bugpoint -compile-custom -compile-command=./run a.ll
(this usage is similar to llvm-reduce)

but not any other command listed on https://llvm.org/docs/Bugpoint.html .
(I am among them.
https://llvm.org/docs/Bugpoint.html and
https://llvm.org/docs/CommandGuide/bugpoint.html
have no usage examples so I resorted to an external article
http://logan.tw/posts/2014/11/26/llvm-bugpoint/ )

The few things tied to the legacy pass manager are these "other commands".
I hope the active users can

* help improve the documentation (several developers found bugpoint unintuitive
to use and don't use features other than -compile-custom)
* Port legacy PM features to the new pass manager.
* If people think that new reducing strategies can be ported to llvm-reduce, and keep
bugpoint as the few misc features that'll also look good:)

>The C API can be worked through (we may need to introduce replacements to
>the legacy pass manager APIs), but bugpoint will be tricky since it has so
>many legacy PM-specific hacks and we may need to trim it down if we want it
>to work with the new PM. Anyway, I don't think any of the remaining
>blockers are large enough to block the switch (but comments welcome).
>
>I'd like to turn on the new PM by default soonish, after the 12.x branch.
>Perhaps roughly a week from now barring any major newly discovered
>regressions?
>
>As for potential issues only uncovered after the switch, if there is a
>large issue I will roll it back, but for smaller issues I'd rather ask
>users to pin to the legacy PM while we fix the issues, either via the CMake
>flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=OFF, or the corresponding
>compiler flags, like -flegacy-pass-manager for clang.
>
>Any concerns/comments?

>_______________________________________________
>LLVM Developers mailing list
>llvm...@lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Florian Hahn via llvm-dev]

> On Jan 26, 2021, at 22:32, Fangrui Song <mas...@google.com> wrote:
>
> Forked the thread and changed the subject to talk about bugpoint specifically.
> CCed folks on https://bugs.llvm.org/show_bug.cgi?id=48813

Thanks!

FWIW I often use bugpoint’s crashing pass reduction feature ( e.g. `bugpoint -O3 foo.bc` to reduce the sequence of passes exposing a crash) when investigating crashes in LLVM optimizations. This is unfortunately one of the features directly tied to the pass manager.

My main concern is that `opt` and `bugpoint` having different defaults seems to have potential to be quite confusing to user (e.g. `opt -passes=default<O3>` crashes, but `bug point -passes=default<O3>` does not, `bug point -O3 ` works, but does not reproduce the failure, e.g. due to difference in the pipeline).

I don’t know how widely used bugpoint is, but it features prominently in the documentation, e.g. https://llvm.org/docs/HowToSubmitABug.html , which is linked from the getting started page and the documentation sidebar.

I think before switching, it would be good to at least update HowToSubmitABug. We should probably also add a note to Bugpoint’s docs and refer the the documentation for llvm-reduce. It is great if people take time to reduce the test cases for their bug reports and we should try to make the experience as smooth as possible

Cheers,
Florian

[David Greene via llvm-dev]

Florian Hahn via llvm-dev <llvm...@lists.llvm.org> writes:

> FWIW I often use bugpoint’s crashing pass reduction feature (
> e.g. `bugpoint -O3 foo.bc` to reduce the sequence of passes exposing a
> crash) when investigating crashes in LLVM optimizations. This is
> unfortunately one of the features directly tied to the pass manager.

Ditto.

> My main concern is that `opt` and `bugpoint` having different defaults
> seems to have potential to be quite confusing to user (e.g. `opt
> -passes=default<O3>` crashes, but `bug point -passes=default<O3>` does
> not, `bug point -O3 ` works, but does not reproduce the failure,
> e.g. due to difference in the pipeline).

Agreed.

> I don’t know how widely used bugpoint is, but it features prominently
> in the documentation, e.g. https://llvm.org/docs/HowToSubmitABug.html
> , which is linked from the getting started page and the documentation
> sidebar.

I use bugpoint a ton. That said, mostly I've used it for testcase
reduction and have done my own manual optimization pipeline reduction
mostly because codegen optimizations aren't as nicely layered and the
codegen pipeline can't be driven from bugpoint.

Getting codegen passes into the new pass manager has been raised
multiple times because and that would be a good thing to happen but it
doesn't begin to tackle the problem of automatic bug classification in
codegen.

> I think before switching, it would be good to at least update
> HowToSubmitABug. We should probably also add a note to Bugpoint’s docs
> and refer the the documentation for llvm-reduce. It is great if people
> take time to reduce the test cases for their bug reports and we should
> try to make the experience as smooth as possible

Yeah, we need a way to keep bug reports sane. If llvm-reduce can do as
well or better than bugpoint, great, I'm all for it!

-David

[Philip Reames via llvm-dev]

+1 to the strategy and timeline.  This has been a long time in the works and I'm thrilled to see us approaching this major milestone.

minor comment inline below

Philip

On 1/26/21 9:17 AM, Arthur Eubanks via llvm-dev wrote:

Hi all,

We've been fixing the various remaining issues in order to turn on the new pass manager for the optimization pipeline, and it's about time to turn it on. (Thanks to everyone who has helped with testing and fixing the new pass manager!)

https://reviews.llvm.org/D95380 is the change that would happen, which sets the CMake flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=ON by default. This affects anything that uses the LLVM_ENABLE_NEW_PASS_MANAGER macro, which includes opt's handling of the `opt -instcombine` syntax, clang, and ThinLTO in lld drivers. This does not affect the backend target-specific codegen pipeline since that's mostly not been ported to use the new PM infrastructure yet.

Here is the umbrella bug for turning on the new PM with blockers. The main one is loop unswitching on divergent loop conditions is unsafe, which is being looked into. There's also the LLVM C API and bugpoint that still use the legacy PM, which can be ported later on and only block the removal of the legacy PM. The C API can be worked through (we may need to introduce replacements to the legacy pass manager APIs), but bugpoint will be tricky since it has so many legacy PM-specific hacks and we may need to trim it down if we want it to work with the new PM. Anyway, I don't think any of the remaining blockers are large enough to block the switch (but comments welcome).

I see no problem with having these two remain on the legacy pass manager for the moment.  I do think we should expose a new C API for the NewPM and not try to shove the new one into the same API as the old one, but that's a weakly held opinion and easily discussed later.

I'd like to turn on the new PM by default soonish, after the 12.x branch. Perhaps roughly a week from now barring any major newly discovered regressions?

As for potential issues only uncovered after the switch, if there is a large issue I will roll it back, but for smaller issues I'd rather ask users to pin to the legacy PM while we fix the issues, either via the CMake flag -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=OFF, or the corresponding compiler flags, like -flegacy-pass-manager for clang.

Any concerns/comments?

[Philip Reames via llvm-dev]

Same.

> My main concern is that `opt` and `bugpoint` having different defaults seems to have potential to be quite confusing to user (e.g. `opt -passes=default<O3>` crashes, but `bug point -passes=default<O3>` does not, `bug point -O3 ` works, but does not reproduce the failure, e.g. due to difference in the pipeline).
>
> I don’t know how widely used bugpoint is, but it features prominently in the documentation, e.g. https://llvm.org/docs/HowToSubmitABug.html , which is linked from the getting started page and the documentation sidebar.
>
> I think before switching, it would be good to at least update HowToSubmitABug. We should probably also add a note to Bugpoint’s docs and refer the the documentation for llvm-reduce. It is great if people take time to reduce the test cases for their bug reports and we should try to make the experience as smooth as possible

Florian raises a great point here.  I'd be comfortable simply
documenting the issue for the moment, but getting bugpoint properly
ported to the new PM close to the point we switch seems very worthwhile
for the reason Florian nicely describes.

[Arthur Eubanks via llvm-dev]

https://reviews.llvm.org/D95578 for updating HowToSubmitABug with alternatives to bugpoint (and just modernizing it in general).

https://reviews.llvm.org/D86657 (not yet submitted) will dump the IR on an LLVM crash, which would be useful. Extending that to dumping the IR to a file would be even better.

[Fangrui Song via llvm-dev]

On 2021-01-27, Arthur Eubanks wrote:
>https://reviews.llvm.org/D95578 for updating HowToSubmitABug with
>alternatives to bugpoint (and just modernizing it in general).
>
>https://reviews.llvm.org/D86657 (not yet submitted) will dump the IR on an
>LLVM crash, which would be useful. Extending that to dumping the IR to a
>file would be even better.
>
>On Wed, Jan 27, 2021 at 9:06 AM Philip Reames <list...@philipreames.com>
>wrote:

Thanks to Authur for https://reviews.llvm.org/D95578 (improve HowToSubmitABug).
If anyone knows how to improve the bugpoint part or renovate the following bugpoint documentation:

llvm/llvm/docs/Bugpoint.rst
llvm/llvm/docs/BugpointRedesign.rst
llvm/llvm/docs/CommandGuide/bugpoint.rst

That will be very nice.

[Arthur Eubanks via llvm-dev]

There are a couple of failures that I hadn't noticed showing up in the presubmit, as well as some internally reported performance regressions due to NPM-related changes, so this will likely get pushed back.

[Arthur Eubanks via llvm-dev]

This has been submitted as https://reviews.llvm.org/D95380. Please file bugs for any regressions.

[David Blaikie via llvm-dev]

awesome - congrats & great work!

[Eric Christopher via llvm-dev]

Congratulations! Thanks for all of your work here! :)

-eric

[Renato Golin via llvm-dev]

It was bound to have the final teething issues, but I'm glad we're finally pushing this through.

Good work and thank you everyone that worked on it for all these years!

[Björn Pettersson A via llvm-dev]

Hi!

 

Are there any existing plans for making the help text for opt more easy to understand?

Here are some things that have been bugging me so far:

 

Currently it for example doesn’t mention that –O0, –O1, etc are designed for legacy-pm (with new-pm it is a bit of a mess since you get the wrong aa-pipeline when for example using –O3).

So are those options supposed to be removed, or should they remain as a short-form for “-passes=default<O3> -aa-pipeline=default” etc?

 

opt -help-hidden lists all the legacy passes (under “Optimizations available:”), but those now require -enable-new-pm=0. Maybe that is obvious.

But I’m missing a similar list mentioning all things that are legal to put in the -passes string.

 

And options like -debug-pass-manager and -debug-pass=Arguments only work depending on which PM that is used, but that is not mentioned in the help text so one need to figure it out by trial and error. Although most of the time you don’t get an error, you just don’t get the requested functionality. So it would be helpful if the help text mention in what situation those options can be used or not (or there should be errors if using them in the wrong context).

 

Downstream we implement fuzzy testing by using “opt -O3 -debug-pass=Arguments” in order to get a list of passes that are used randomly on the opt command line.

I haven’t figured out yet how to implement something similar with the new pm. Is there a way to make opt output the pass names available (that can be used in -passes string)?

 

Regards,

Björn

[Arthur Eubanks via llvm-dev]

On Thu, Feb 4, 2021 at 9:34 AM Björn Pettersson A <bjorn.a.p...@ericsson.com> wrote:

Hi!

 

Are there any existing plans for making the help text for opt more easy to understand?

Here are some things that have been bugging me so far:

 

Currently it for example doesn’t mention that –O0, –O1, etc are designed for legacy-pm (with new-pm it is a bit of a mess since you get the wrong aa-pipeline when for example using –O3).

So are those options supposed to be removed, or should they remain as a short-form for “-passes=default<O3> -aa-pipeline=default” etc?

I'd like to remove these at some point and just go with -passes='default<O#>', but they'll likely remain until we decide to start removing the legacy PM and we start cleaning up tests to only use -passes=. I'll update the description for these flags.

 

opt -help-hidden lists all the legacy passes (under “Optimizations available:”), but those now require -enable-new-pm=0. Maybe that is obvious.

But I’m missing a similar list mentioning all things that are legal to put in the -passes string.

The legacy PM registered passes to a global variable which made this easier to do in the legacy PM. The new PM doesn't have a global registry, but rather each PassBuilder is initialized with the list of passes on its own. I'll draft up something to try to get this working.

 

And options like -debug-pass-manager and -debug-pass=Arguments only work depending on which PM that is used, but that is not mentioned in the help text so one need to figure it out by trial and error. Although most of the time you don’t get an error, you just don’t get the requested functionality. So it would be helpful if the help text mention in what situation those options can be used or not (or there should be errors if using them in the wrong context).

I'll update the help text. Each pass manager's debug logging is separate, rather than being tied to a global variable like the legacy PM's -debug-pass, so it's a bit weird to check in each new PM PassManager if -debug-pass has been specified.

 

Downstream we implement fuzzy testing by using “opt -O3 -debug-pass=Arguments” in order to get a list of passes that are used randomly on the opt command line.

I haven’t figured out yet how to implement something similar with the new pm. Is there a way to make opt output the pass names available (that can be used in -passes string)?

There was some discussion about similar things in the bugpoint + new PM thread. The way the new PM is designed doesn't really lend itself to this sort of functionality. It could be possible for each type of PassManager to print out the names of all the passes it contains, but we'd need some change with the way we handle pass names (e.g. remove PassInfoMixin and have each pass declare its own name). Rather, I think something like the PassInstrumentation approach that -opt-bisect-limit does is better, where you can turn off a specific set of passes. But I could be convinced of a different approach.

 

Regards,

Björn

Thanks for the questions, these are good for making things clearer to users. 

[Björn Pettersson A via llvm-dev]

Thanks for the answers Arthur!

 

I also noticed that you already put up some patches for review, to address some of my questions 😊

(I ran out of time today, but will take a look at those patches later)

 

/Björn

From: Arthur Eubanks <aeub...@google.com>
Sent: den 4 februari 2021 20:36
To: Björn Pettersson A <bjorn.a.p...@ericsson.com>
Cc: llvm-dev <llvm...@lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] Changing the default pass manager for the optimization pipeline

 

 

 

On Thu, Feb 4, 2021 at 9:34 AM Björn Pettersson A <bjorn.a.p...@ericsson.com> wrote: