<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 9/15/21 9:17 AM, Sjoerd Meijer
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:AS8PR08MB6597C4AA0B08E669E08D9D14FCDB9@AS8PR08MB6597.eurprd08.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
The SLP vectoriser will fail to vectorise this because it wasn't
taught to emit runtime alias analysis checks. This is being
addressed by Florian in <a
href="https://reviews.llvm.org/D102834" id="LPlnk"
moz-do-not-send="true">https://reviews.llvm.org/D102834</a>.
We had many issues with full unrolling removing opportunities
for the loop vectoriser, and the SLP missing some tricks. But <span
style="color: rgb(0, 0, 0); font-family: Calibri, Arial,
Helvetica, sans-serif; font-size: 12pt;">with
<a href="https://reviews.llvm.org/D102834" style="margin:0px"
moz-do-not-send="true">D102834</a> fixed, I expect this loop
to be SLP vectorised and most of this pass ordering problems
to disappear.</span></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
Philip seems happy with this being part of GVN, and I don't have
strong opinions, but GVNHoist seems like the natural place for
this. An alternative strategy could thus be to integrate this
into GVNHoist, enable it by default but only your simple gvn
hoist algorithm (and disable the rest). That would perhaps then
be a first step to address Philip's unhappiness with GVNHoist
and restructure and improve it step by step.</div>
</blockquote>
<p>+1 to this point. This would be a perfectly reasonable way to
implement the MemorySSA based approach in a shippable way. I'm
not opposed to the non-memoryssa variant in (old) GVN, but this
would be fine too.</p>
<p><br>
</p>
<p>Philip<br>
</p>
<blockquote type="cite"
cite="mid:AS8PR08MB6597C4AA0B08E669E08D9D14FCDB9@AS8PR08MB6597.eurprd08.prod.outlook.com">
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt"
face="Calibri, sans-serif" color="#000000"><b>From:</b>
Momchil Velikov <a class="moz-txt-link-rfc2396E" href="mailto:momchil.velikov@gmail.com"><momchil.velikov@gmail.com></a><br>
<b>Sent:</b> 15 September 2021 12:30<br>
<b>To:</b> Sjoerd Meijer <a class="moz-txt-link-rfc2396E" href="mailto:Sjoerd.Meijer@arm.com"><Sjoerd.Meijer@arm.com></a><br>
<b>Cc:</b> Momchil Velikov <a class="moz-txt-link-rfc2396E" href="mailto:Momchil.Velikov@arm.com"><Momchil.Velikov@arm.com></a>;
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a> <a class="moz-txt-link-rfc2396E" href="mailto:llvm-dev@lists.llvm.org"><llvm-dev@lists.llvm.org></a>;
Philip Reames <a class="moz-txt-link-rfc2396E" href="mailto:listmail@philipreames.com"><listmail@philipreames.com></a><br>
<b>Subject:</b> Re: [llvm-dev] [RFC] Simple GVN hoist</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span
style="font-size:11pt;">
<div class="PlainText">On Wed, Sep 15, 2021 at 12:18 PM
Momchil Velikov<br>
<a class="moz-txt-link-rfc2396E" href="mailto:momchil.velikov@gmail.com"><momchil.velikov@gmail.com></a> wrote:<br>
><br>
> On Tue, Sep 14, 2021 at 7:56 PM Sjoerd Meijer via
llvm-dev<br>
> <a class="moz-txt-link-rfc2396E" href="mailto:llvm-dev@lists.llvm.org"><llvm-dev@lists.llvm.org></a> wrote:<br>
> > Fix GVNHoist. I like your patch because it's
small and concise, but missing in the RFC is a discussion
why we don't try to re-enable GVNHoist. I know it was
briefly enabled by default, which was reverted due to
correctness (or was it regressions?) problems. But if this
belongs in GVNHoist, could this for example be added to
GVNHoist, and only this part enabled? Not sure if that's
possible as I haven't looked at GVNHoist.<br>
><br>
> Yes, GVNHoist will do what we need<br>
<br>
[hit send too soon]<br>
<br>
So, GVNHoist will do the hoisting we want.. However, while
technically<br>
possible there a few drawbacks in taking the existing
GVNHoist<br>
approach:<br>
* GVNHoist is large and algorithmically complex<br>
* Even if enabled, it won't solve this specific issue -
after hoisting<br>
of the loads the loop size falls below some unrolling
threshold and<br>
the loop is fully unrolled - instead the loop should be<br>
loop-vectorised, IMHO - so we get a pass order issue.<br>
* After unrolling the SLP vectoriser does not vectorise
the loop - so<br>
we have to fix the SLP vectoriser too.<br>
<br>
While I think all of these are worthwhile, we have on one
hand some<br>
rather simple transformation (the mini-GVNHoist) vs. on
the other hand<br>
the actual GVNHoist+pass manager+SLP Vectoriser, which<br>
could easily turn into a time sink with no end in sight.<br>
</div>
</span></font></div>
</blockquote>
</body>
</html>