[PATCH] D22305: [BasicAA] Strip phi nodes, when all incoming values are the same.

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 13 09:10:15 PDT 2016


This seems like a bad example.
For that example, the right solution is clearly to not generate a phi with
identical arguments.
Why not fix GVN to do that?

Given %f35.pre-phi.i = phi float* [ %.pre6.i, %entry.if.else_crit_edge.i ],
[ %.pre6.i, %land.lhs.true.if.else_crit_edge.i ]

%pre6.i must dominate this block, or else it would not be available through
both edges.

Thus, the phi is unnecessary, you could simply use %pre6.i instead, and you
would not have to add this code to BasicAA.

Given we generated this phi, we can simply not generate it :)

Do you have an example where that is not the case, and your patch to
BasicAA is needed?
(I would think EarlyCSE would eliminate all the above cases in input files
before other opts saw it)

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.

IE if we next discover cases where all the phi arguments are redundant
bitcasts of the same value, do we add that?
What about more complex operations?


On Wed, Jul 13, 2016 at 8:57 AM, Ehsan Amiri <amehsan at ca.ibm.com> wrote:

> amehsan created this revision.
> amehsan added reviewers: dberlin, hfinkel.
> amehsan added a subscriber: llvm-commits.
>
> This fixes the performance problem in PR27740. Also this patch includes
> code for a compile time opportunity in BasicAA.
>
> Problem in 27740:
>
> for the following pair :
> %f35.pre-phi.i = phi float* [ %.pre6.i, %entry.if.else_crit_edge.i ], [
> %.pre6.i, %land.lhs.true.if.else_crit_edge.i ]
> %f11 = getelementptr inbounds %struct.s, %struct.s* %p1, i64 0, i32 1
>
> where:
> %.pre6.i = getelementptr inbounds %struct.s, %struct.s* %p1, i64 0, i32 3
>
> 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 [[
> https://llvm.org/bugs/show_bug.cgi?id=27740#c28 | this comment ]] and
> also discussed with Hal.
>
> 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.
>
>
>
> http://reviews.llvm.org/D22305
>
> Files:
>   include/llvm/Analysis/BasicAliasAnalysis.h
>   lib/Analysis/BasicAliasAnalysis.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160713/01748869/attachment.html>


More information about the llvm-commits mailing list