<div dir="ltr"><div>This seems like a bad example.</div><div>For that example, the right solution is clearly to not generate a phi with identical arguments.</div><div>Why not fix GVN to do that?<br><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Given %f35.pre-phi.i = phi float* [ %.pre6.i, %entry.if.else_crit_edge.i ], [ %.pre6.i, %land.lhs.true.if.else_crit_</span><span style="font-size:12.8px">edge.i ]</span><br style="font-size:12.8px"></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">%pre6.i must dominate this block, or else it would not be available through both edges.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Thus, the phi is unnecessary, you could simply use %pre6.i instead, and you would not have to add this code to BasicAA.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Given we generated this phi, we can simply not generate it :)</span></div><div><br></div><div><span style="font-size:12.8px">Do you have an example where that is not the case, and your patch to BasicAA is needed?<br>(I would think EarlyCSE would eliminate all the above cases in input files before other opts saw it)</span></div><div><span style="font-size:12.8px"><br></span></div><div>Past that I'm not sure i believe this is the right thing to do, because it's not clear to me where it logically ends and why.</div><div><br></div><div>IE if we next discover cases where all the phi arguments are redundant bitcasts of the same value, do we add that?</div><div>What about more complex operations?</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 13, 2016 at 8:57 AM, Ehsan Amiri <span dir="ltr"><<a href="mailto:amehsan@ca.ibm.com" target="_blank">amehsan@ca.ibm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">amehsan created this revision.<br>
amehsan added reviewers: dberlin, hfinkel.<br>
amehsan added a subscriber: llvm-commits.<br>
<br>
This fixes the performance problem in PR27740. Also this patch includes code for a compile time opportunity in BasicAA.<br>
<br>
Problem in 27740:<br>
<br>
for the following pair :<br>
%f35.pre-phi.i = phi float* [ %.pre6.i, %entry.if.else_crit_edge.i ], [ %.pre6.i, %land.lhs.true.if.else_crit_edge.i ]<br>
%f11 = getelementptr inbounds %struct.s, %struct.s* %p1, i64 0, i32 1<br>
<br>
where:<br>
%.pre6.i = getelementptr inbounds %struct.s, %struct.s* %p1, i64 0, i32 3<br>
<br>
We cannot prove that the pair is not aliased. This is because BasicAA first goes to aliasGEP, which recursively calls aliasCheck to look at aliasing for the base pointer of the GEP (with unknown size) and the phi node in the original pair. The result of that is a MayAlias.  A number of alternative solutions have been discussed in [[ <a href="https://llvm.org/bugs/show_bug.cgi?id=27740#c28" rel="noreferrer" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=27740#c28</a> | this comment ]] and also discussed with Hal.<br>
<br>
Also, some of the recursive calls to aliasCheck, from alias[GEP|Select|PHI] pass one of the values for which GetUnderlyingObject is already called, again. So if we keep the underlying object that is already calculated around, we won't need to recompute it, given this is an expensive call.<br>
<br>
<br>
<br>
<a href="http://reviews.llvm.org/D22305" rel="noreferrer" target="_blank">http://reviews.llvm.org/D22305</a><br>
<br>
Files:<br>
  include/llvm/Analysis/BasicAliasAnalysis.h<br>
  lib/Analysis/BasicAliasAnalysis.cpp<br>
<br>
</blockquote></div><br></div>