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

[Hal Finkel via llvm-commits]

----- Original Message -----

> From: "Daniel Berlin" <dberlin at dberlin.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>,
> reviews+D22305+public+92ca108e50bc4651 at reviews.llvm.org, "Ehsan A
> Amiri" <amehsan at ca.ibm.com>
> Sent: Thursday, July 14, 2016 9:38:55 PM
> Subject: Re: [PATCH] D22305: [BasicAA] Strip phi nodes, when all
> incoming values are the same.

> On Thu, Jul 14, 2016 at 6:34 PM, Hal Finkel < hfinkel at anl.gov >
> wrote:

> > > From: "Daniel Berlin" < dberlin at dberlin.org >
> > 
> 
> > > To: "Hal Finkel" < hfinkel at anl.gov >
> > 
> 
> > > Cc: "llvm-commits" < llvm-commits at lists.llvm.org >,
> > > reviews+D22305+public+92ca108e50bc4651 at reviews.llvm.org , "Ehsan
> > > A
> > > Amiri" < amehsan at ca.ibm.com >
> > 
> 
> > > Sent: Thursday, July 14, 2016 8:30:09 PM
> > 
> 
> > > Subject: Re: [PATCH] D22305: [BasicAA] Strip phi nodes, when all
> > > incoming values are the same.
> > 
> 

> > > > (2) is, in theory, the right thing to do. Even if we were to
> > > > consider
> > > > uniform PHIs to be anti-canonical, and thus something which
> > > > should
> > > > be simplified, we can't simplify often enough to prevent these
> > > > from
> > > > blocking useful analysis work.
> > > 
> > 
> 
> > > FWIW, i'm fine with this approach if our approach is going to be
> > > as
> > > you say - that we will not simplify often enough.
> > 
> 

> > > Right now, as i said, we simplify *everywhere*, and every one of
> > > those calls will eliminate this phi node.
> > 
> 

> > > So it's only *this particular path* that misses all those calls.
> > 
> 

> > > For example, if the alias check had gone through a gep of a phi
> > > anywhere, it would simplify the phi as part of
> > > getunderlyingobject,
> > > etc.
> > 
> 

> > > > Arbitrary uses of RAUW can create these PHIs, and we can't (and
> > > > probably shouldn't) run InstCombine in between every other
> > > > pass.
> > > > This is a local pattern that stripPointerCasts, and similar
> > > > functions, can look through.
> > > 
> > 
> 

> > > Fine with this as long as we maybe stop trying to simplify
> > > instructions 8 or 9 times, and instead do it once (max) per
> > > instructions, and make this part of it.
> > 
> 

> > > (IE This would mean we would have SimplifyAndGetUnderlyingObject
> > > and
> > > GetUnderlyingObject, and we simplify once and call the latter or
> > > something, or whatever. Not suggesting we decide this second,
> > > just
> > > suggesting that we agree if this is going to be our general
> > > approach).
> > 
> 
> > This is hard because we don't cache the simplifications in any way.
> > It is not like we're updating the IR when we discover some
> > simplification; we're only using the simplified version in place.
> > I'm not sure how to fix this. Maybe we should run InstSimplify a
> > lot
> > more often. It is not as expensive was InstCombine by a large
> > margin
> > (IIRC).
> 

> > > > (3) is also, in theory, the right thing to do. The memdep
> > > > cache,
> > > > by
> > > > necessity, caches negative results.
> > > 
> > 
> 
> BTW, i missed this part. I haven't looked at the memdep caches in a
> while, but if by "caching negative results" you mean it caches
> anything other than noalias, that seems .. wrong, since otherwise it
> would do all the work to prove things are noalias again and again :)

> In that case, you can of course, simply cache that the answer is "no
> dependencies", and i thought it did.

> > > > Each GVN iteration, however, performs "information revealing"
> > > > operations which can make the cached results more conservative
> > > > than
> > > > a new query's results.
> > > 
> > 
> 

> > > Yes.
> > 
> 
> > > It's theoretically possible to make it less expensive than
> > > blowing
> > > away the whole cache, but so far, experience has told me that
> > > fully
> > > maintaining the cache becomes slower than redoing the queries :P
> > 
> 
> > I'm not sure how much this helps, but we could/should only clear
> > out
> > the MayAlias results.
> 
> Again, haven't looked at detail at the caches for a while, but my
> recollection is this is the vast majority of results.

Yes, MayAlias is almost certainly the vast majority of results, and my point is that it is only this majority of results that we should, if compile-time constraints allow, clear from the cache in between GVN iterations. 

-Hal 

-- 

Hal Finkel 
Assistant Computational Scientist 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160714/562813cb/attachment-0001.html>