<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 4, 2020 at 3:28 PM Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div>
<p>You are proposing to move code for the new pass manager into
conditional compilation. I am strongly opposed.</p></div></blockquote><div>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: </div><div><br></div><div><a href="https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L706-L707" target="_blank">https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L706-L707</a></div><div><div>set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL</div><div> "Enable the experimental new pass manager by default.")</div></div><div><br></div><div>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. </div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div>
<p>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. <br>
</p>
<p>(two minor response inline)<br>
</p>
<p>Philip<br>
</p>
<div>On 12/4/20 3:19 PM, Arthur Eubanks
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">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.</div>
</blockquote>
"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. <br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>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.</div>
</div>
</blockquote>
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?<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<div><br>
</div>
<div>Also as mentioned in previous discussions, lots of people
use the default, which currently is the legacy PM.</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Dec 4, 2020 at 2:18 PM
Philip Reames <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div>
<p>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. <br>
</p>
<p>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.)</p>
<p>Philip<br>
</p>
<div>On 12/1/20 12:34 PM, Arthur Eubanks via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>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. <br>
</div>
<div><br>
</div>
<div>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. <a href="https://github.com/llvm/llvm-project/blob/1314a4938fba865412598b7227cb4657d59cd8bc/lld/wasm/Driver.cpp#L382" target="_blank">Some</a> <a href="https://github.com/llvm/llvm-project/blob/1314a4938fba865412598b7227cb4657d59cd8bc/llvm/include/llvm/LTO/Config.h#L53" target="_blank">examples</a>.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Any objections?</div>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
LLVM Developers mailing list
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div></div></div></div>