<div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 27, 2021 at 9:03 AM Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>+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.</p>
<p>minor comment inline below<br>
</p>
<p>Philip<br>
</p>
<div>On 1/26/21 9:17 AM, Arthur Eubanks via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,<br>
<br>
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!)<br>
<br>
<a href="https://reviews.llvm.org/D95380" target="_blank">https://reviews.llvm.org/D95380</a>
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.<br>
<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=46649" target="_blank">Here</a> is the umbrella bug for
turning on the new PM with blockers. The main one is <a href="https://bugs.llvm.org/show_bug.cgi?id=48819" target="_blank">loop unswitching on divergent loop
conditions is unsafe</a>, 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).<br>
</div>
</blockquote>
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. <br>
<blockquote type="cite">
<div dir="ltr"><br>
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?<br>
<br>
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.<br>
<br>
Any concerns/comments?<br>
</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>