<div dir="ltr">IIUC, GEP merging can be harmful regardless of where the target is in loop context or not. Wei's heuristic filters out lots of harmful cases. A more general fix might need introduce more sophisticated cost analysis -- it is possible (as mentioned by Quentin) for this simple heuristic to filter out beneficial merging, but test cases are needed for that.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 14, 2015 at 1:16 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Quentin Colombet wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The thing with the "both constants" approach is that we may miss some cases where an addressing mode may apply. In particular, on x86, when one argument of the add is a constant, we would be able to match reg1 + reg2 + displacement.<br>
<br>
That being said, your numbers imply this is not a problem in practice, and most targets do not support reg1 + reg2 + reg3/imm addressing mode. Anyway, we could recover from that later on if needed.<br>
<br>
So LGTM.<br>
</blockquote>
<br></span>
I might be misunderstanding why this patch helps, please bear with me. Is this not a special case of a general problem, mainly that we're merging something into a loop that was previously two operations one inside and one outside the loop? Why wouldn't this problem come up in other ways? We already have other problems where we introduce loop carried dependencies where ones previously didn't exist, shouldn't we approach this by adding an API to detect those and then making transforms conditional on that?<span class="HOEnZb"><font color="#888888"><br>
<br>
Nick</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks Wei!<br>
-Quentin<br>
<br>
<br>
REPOSITORY<br>
   rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D8911" target="_blank">http://reviews.llvm.org/D8911</a><br>
<br>
EMAIL PREFERENCES<br>
   <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
<br>
<br>
<br>
</blockquote>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>