<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div class="moz-cite-prefix">On 10/03/2021 14:30, Thomas Preud'homme wrote:<br>
</div>
<blockquote type="cite" cite="mid:a4483bd9-a713-ce6b-0ff9-e8c7c929be2b@graphcore.ai">
<div class="moz-cite-prefix">On 10/03/2021 10:58, Florian Hahn wrote:<br>
</div>
<blockquote type="cite" cite="mid:49A8BFCB-B281-474C-8F4A-4FA6607C54D3@apple.com">
<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On Mar 9, 2021, at 13:13, Thomas Preud'homme <<a href="mailto:thomasp@graphcore.ai" class="" moz-do-not-send="true">thomasp@graphcore.ai</a>> wrote:</div>
<br>
<div class="">
<div class="">
<p class="">I tried the patch (thanks) but that did not remove any of the PHI (the 2 loads are still there and thus the bitcast don't appear to have the same source). I'll try tolook at InstCombine to see why loads are not CSE'd.</p>
<div class=""><br class="">
</div>
</div>
</div>
</blockquote>
<br class="">
</div>
<div><br class="">
</div>
<div>I’m not sure I follow here. For your example (spurious_loop_peeling.cpp), it looks like there’s no peeling happening any more after the patch landed, at least when building for ARM64:
<a href="https://godbolt.org/z/q6d6Kn" class="" moz-do-not-send="true">https://godbolt.org/z/q6d6Kn</a> . Is there anything else that’s going wrong?</div>
</blockquote>
<p><br>
</p>
<p>The testcase I sent is indeed fixed with your commit. However the code it is inspired from still shows unwanted peeling. I'm going to investigate what causes the difference.</p>
<p><br>
</p>
<p>Best regards,</p>
<p><br>
</p>
<p>Thomas</p>
</blockquote>
<p><br>
</p>
<p>Sorry for the late reply. FYI the difference is because the original code is using pointer rather than reference parameter (see attachment). This leads to LICM not hoisting the load out of the outermost loop due to isSafeToExecuteUnconditionally returning
false. This happens because the base pointer of the GEP used by the load is not sufficiently aligned. isDereferenceableAndAlignedPointer() from Loads.cpp calls Value::getPointerAlignment which returns an alignment of 1 and deduced that the alignment is not
enough compared to the load requirement.</p>
<p>In my case however I know for certain that the this pointer is sufficiently aligned. Unfortunately I could not find a way to indicate it to the compiler. I tried to use __builtin_assume_aligned on the this pointer and use the return value for all access
but that did not make any difference.</p>
<p><br>
</p>
<p>So to summarize:</p>
<p><br>
</p>
<p>Load whose base is a function parameter gets duplicated by loop rotate, LICM cannot hoist it out completely (it does get hoisted out of the innerloop) due to alignment issue which means a phi remains in the innerloop when loop peeling happens. This leads
to code bloat and in our case lack of vectorization.</p>
<p><br>
</p>
<p>However clearly GVN thinks the load outside the loop is the same as the one in the loop and so the one in the loop can be removed. That seems inconsistent with the behaviour of LICM so I'm gonna try to look into this.</p>
<p><br>
</p>
<p>Best regards,</p>
<p><br>
</p>
<p>Thomas<br>
</p>
</body>
</html>