<div dir="ltr"><div>Hi Sanjoy, </div><div><br></div><div>I agree the reassociation performed here is generally good, and <span style="font-size:13.1999998092651px;line-height:1.5">I wish I could put it in -reassociate. </span></div><div><span style="font-size:13.1999998092651px;line-height:1.5"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:1.5">However, I have two major concerns.</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">1. The algorithm in -reassociate is local in the sense that it ranks nodes in each individual expression tree and reorders them according to their ranks. The algorithm implemented here is global: it reassociates an expression in the way that we can leverage other instructions in the same function. I also found the workflow of my algorithm nicely fit into SLSR which scans instructions in the DFS order and simplifies them with respect to their dominators. </span><br></div><div><span style="line-height:1.5;font-size:13.1999998092651px">2. -reassociate happens too early. Many opportunities I discovered are exposed after loop unrolling, but -reassociate happens before loop unrolling. </span></div><div><span style="line-height:1.5;font-size:13.1999998092651px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:1.5">Jingyue </span><br></div></div><br><div class="gmail_quote">On Wed, Apr 8, 2015 at 1:23 PM Sanjoy Das <<a href="mailto:sanjoy@playingwithpointers.com">sanjoy@playingwithpointers.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Should this rule live inside `-reassociate`?  Optimizing<br>
<br>
   define void @reassociate(i32 %a, i32 %b, i32 %c) {<br>
    %1 = add i32 %a, %c<br>
    call void @foo(i32 %1)<br>
    %2 = add i32 %a, %b<br>
    %3 = add i32 %2, %c<br>
    call void @foo(i32 %3)<br>
    ret void<br>
  }<br>
<br>
to<br>
<br>
   define void @reassociate(i32 %a, i32 %b, i32 %c) {<br>
    %1 = add i32 %a, %c<br>
    call void @foo(i32 %1)<br>
    %3 = add i32 %1, %b<br>
    call void @foo(i32 %3)<br>
    ret void<br>
  }<br>
<br>
looks like a generally good idea to me.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D8898" target="_blank">http://reviews.llvm.org/D8898</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>
</blockquote></div>