<div dir="ltr">So, this is an awfully specific redundancy to detect in GVN.<div><br>The more general case doesn't use select's but phi's, and is tested in gcc with GVNPRE with testcases like:</div><div><br><br></div><div><div> int</div><div> foo (int i)</div><div> {</div><div>     int a, b;</div><div>     if (i)</div><div>         a = 3, b = 2;</div><div>     else</div><div>         a = 2, b = 3;</div><div>     return a + b;</div><div> }</div></div><div><br></div><div>and more complex testcase that involve expressions.</div><div>But the basic idea is the same - if both values are computed already, the end is redundant.</div><div>(here, the result is constant along both edges)<br></div><div><br></div>GVN doesn't touch this (and wouldn't with your patch):<br><br><div> ; Function Attrs: nounwind readnone ssp uwtable</div><div> define i32 @foo(i32 %i) #0 {</div><div>   %1 = icmp eq i32 %i, 0</div><div>   %. = select i1 %1, i32 2, i32 3</div><div>   %.1 = select i1 %1, i32 3, i32 2</div><div>   %2 = add nsw i32 %., %.1</div><div>   ret i32 %2</div><div> }</div><div><br></div><div><br></div><div>GCC treats this as a GVNPRE problem rather than a GVN one, because it involves computing available values along edges.</div><div><br></div><div>Putting these as selects, sadly, makes it harder for things like PRE to see this (though you could treat the selects are false blocks with incoming edges)</div><div>It would compute the values are available along each edge already, and insert a no-cost phi to merge them.</div><div><br></div><div><br></div><div>In any case, if we want it, sure.  I have no objections as even in GVN, this is trivial to add to new gvn, it just costs time as a special case until i rewrite PRE based on GVN.</div><div> </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 12, 2015 at 12:34 PM, Chad Rosier <span dir="ltr"><<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Revised patch with the following changes:<br>
-Reworked the code so that we now use def-use and use-def, rather than iterate over all instructions in the block.<br>
-Store can now feed the select.  I.e.,<br>
<span class=""><br>
  a = (load (gep ptr_a 0 0))<br>
</span>  store b (gep ptr_b 0 0))<br>
<span class="">  c = (load (gep (select (cond, ptr_a, ptr_b)) 0 0))<br>
  --><br>
  a = (load (gep ptr_a, 0, 0))<br>
</span>  store b (gep ptr_b 0 0))<br>
  c = (select (cond, a, b))<br>
<br>
There were no correctness or performance regressions across spec2000, spec2006, and eembc on an A57 device.  The only benchmarks that hit this transform are spec2000/gcc, spec2000/vpr, and spec2006/dealII.  I saw a ~1% improvement in vpr, but that's basically noise. :o/<br>
<br>
Also, adding Daniel B. to the reviews as he seems to be working in this area as of late.<br>
<br>
All, please have a look.<br>
<span class="HOEnZb"><font color="#888888"><br>
Chad<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<a href="http://reviews.llvm.org/D8120" target="_blank">http://reviews.llvm.org/D8120</a><br>
<br>
Files:<br>
  lib/Transforms/Scalar/GVN.cpp<br>
  test/Transforms/GVN/load-gep-select.ll<br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</div></div></blockquote></div><br></div>