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

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.
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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210126/d5df7053/attachment.html>

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-dev at lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/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 <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. 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?
>
> _______________________________________________
> 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/20210127/da9e2c53/attachment.html>

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.

On Wed, Jan 27, 2021 at 9:03 AM Philip Reames <listmail at
philipreames.com>
wrote:
> +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 <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. 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?
>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at
lists.llvm.orghttps://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/20210202/cf6e273f/attachment.html>