[llvm-dev] [RFC] Expanding the scope of ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER

[Philip Reames via llvm-dev]

You are proposing to move code for the new pass manager into conditional 
compilation.  I am strongly opposed.

As for the overall status of the NPM, I find the continued delay in 
switching to be extremely problematic for the health of project long 
term.  I understand the "X doesn't work yet" problem, but a) X is fairly 
small, and b) the folks involved in maintaining X need to pay the cost 
of supporting the old pass manager.  I do want to be careful and state 
explicitly that I'm expressing opinion here, not making an actual 
proposal.  I may get around to the later eventually, but this is not it.

(two minor response inline)

Philip

On 12/4/20 3:19 PM, Arthur Eubanks wrote:
> Implementing this proposal does not in any way stop the flip of the 
> flag, they are completely unrelated. This increases the scope of the 
> new pass manager since much of lld's use of LTO is currently 
> unconditionally using the legacy PM and flipping the flag wouldn't 
> change that.
"the default" for me only means opt and clang.  It doesn't mean llc, or 
any other tool which happens to use the old pm.  If we need clang to 
select the old pass manager at the command line when invoking LTO, that 
doesn't really bug me.
>
> There are some things that the new pass manager doesn't currently 
> support. For example, all of AMDGPU would be broken with the switch to 
> the new pass manager since currently AMDGPU's passes aren't injected 
> into the pipeline. I'm working on the (few) remaining issues and do 
> plan to flip the switch soon.
I find this hard to believe.  Are you possibly talking about 
llc/codegen?  If so, that's well out of scope for what I'm talking 
about.  If not, can you point to a bug so I can see an example?
>
> Also as mentioned in previous discussions, lots of people use the 
> default, which currently is the legacy PM.
>
> On Fri, Dec 4, 2020 at 2:18 PM Philip Reames 
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>
>     I strongly disagree with this proposal.  As in, please do not land
>     patches which implement this proposal.  If anything, we should
>     remove the build time config flag entirely.
>
>     The new manager is mature and has been in wide use for a long time
>     now.  Moving it to a conditional compilation item is a major
>     regression in implied maturity and completely unwarranted.  If
>     anything, we should just flip the dang flag and make people using
>     the old pass manager support it.  (Most downstream groups I know
>     of are running NPM.)
>
>     Philip
>
>     On 12/1/20 12:34 PM, Arthur Eubanks via llvm-dev wrote:
>>     The ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER CMake flag currently
>>     only affects Clang. It should probably also change all other uses
>>     of pass managers where possible.
>>
>>     There are a couple of uses inside LLD for LTO which already have
>>     new/legacy PM flags and should probably look at
>>     ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER to determine the default.
>>     Some
>>     <https://github.com/llvm/llvm-project/blob/1314a4938fba865412598b7227cb4657d59cd8bc/lld/wasm/Driver.cpp#L382>
>>     examples
>>     <https://github.com/llvm/llvm-project/blob/1314a4938fba865412598b7227cb4657d59cd8bc/llvm/include/llvm/LTO/Config.h#L53>.
>>
>>     Also at some point in the future when check-llvm has been fixed
>>     to work with opt's -enable-new-pm flag by default, that should
>>     also be dependent upon ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER.
>>
>>     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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201204/affa5403/attachment-0001.html>