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

[Philip Reames via llvm-dev]

Mehdi is completely correct.  I'd assumed I knew what the flag being 
discussed from context did and I was wrong.  Mehdi, thank you for 
pointing this out.

Philip

On 12/4/20 4:09 PM, Mehdi AMINI wrote:
>
>
> On Fri, Dec 4, 2020 at 3:28 PM Philip Reames via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>
>     You are proposing to move code for the new pass manager into
>     conditional compilation.  I am strongly opposed.
>
> I suspect there is a misunderstanding: the proposal is not to leave 
> *out* the new PM with this flag, it is to allow the CMake flag to 
> operate in more places:
>
> https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L706-L707
> set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
>   "Enable the experimental new pass manager by default.")
>
> This allows to build clang/LLVM with the new pass manager enabled by 
> default. Despite the description it only affects the clang driver 
> enabling the new PM.
>
> The intent as I understand it is to extend the applicability of this 
> flag, so that it applies to LTO and opt and ...  ; that way any 
> buildbot with this flag turned on would check *more* of LLVM with the 
> new PM.
>
> As I understand it, this is only intended to help drive the migration 
> further by validating the use of the new PM in more places in LLVM 
> right now.
>
> -- 
> Mehdi
>
>
>     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
>>
>     _______________________________________________
>     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/20201205/9b4a23bb/attachment.html>