[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 20:56:52 PDT 2016


So now I'm even more confused. If it's using the memdep cache, why does
changing basicaa work at all?
Should it not still get the cached info

On Wed, Jul 13, 2016, 8:22 PM Ehsan A Amiri <amehsan at ca.ibm.com> wrote:

> Daniel,
>
> The soultion you are proposing does not work. Actually it is already
> implemented and we have this problem. Very early in processInstruction,
> there is a call to SimplifyInstruction, which provides that exact same
> functionality for PHI nodes and we still have this problem.
>
> This goes back to what we already discussed in detail in the PR. Actually
> GVN does not directly make an Alias Analysis query. It makes a MemDep
> query. At the first iteration of GVN, when thet mem-dep does not have the
> results cached (and so it does all the work, which includes alias analysis
> query), the phi exists. After the phi is removed, the cache already
> contains invalid information, so memdep does not make a new alias analysis
> query. It just provides us with its invalid cache information. So we are
> back at our previous discussion :)
>
> Thanks
> Ehsan
>
> [image: Inactive hide details for Daniel Berlin ---07/13/2016 09:34:56
> PM---On Wed, Jul 13, 2016 at 5:55 PM, Ehsan Amiri <amehsan at ca.ib]Daniel
> Berlin ---07/13/2016 09:34:56 PM---On Wed, Jul 13, 2016 at 5:55 PM, Ehsan
> Amiri <amehsan at ca.ibm.com> wrote: > amehsan added a comment.
>
> From: Daniel Berlin <dberlin at dberlin.org>
> To: reviews+D22305+public+92ca108e50bc4651 at reviews.llvm.org
> Cc: Ehsan A Amiri/Toronto/IBM at IBMCA, Hal Finkel <hfinkel at anl.gov>,
> llvm-commits <llvm-commits at lists.llvm.org>
> Date: 07/13/2016 09:34 PM
> Subject: Re: [PATCH] D22305: [BasicAA] Strip phi nodes, when all incoming
> values are the same.
> ------------------------------
>
>
>
>
>
>
> On Wed, Jul 13, 2016 at 5:55 PM, Ehsan Amiri <*amehsan at ca.ibm.com*
> <amehsan at ca.ibm.com>> wrote:
>
>    amehsan added a comment.
>
>    @dberlin
>
>    I see two concerns in your comment:
>
>    > > Why not fix GVN to do that?
>
>    >
>
>    >
>
>    >
>
>
>    Before GVN this phi node has two different incoming values. At some
>    point A in GVN, one of the incoming values change and we get the phi node
>    in the example above. At some point B in the GVN, the phi node is removed.
>    Somewhere between A and B, GVN makes an alias analysis query. So I think
>    there is not really a problem in GVN to fix.
>
>
> I'm going to pretty strongly disagree.  If GVN expects AA to give it good
> intermediate answers, it needs to give AA good intermediate code.
>
> GVN does a lot of simplification already. In fact, this would probably be
> one of the cheapest things GVN does It seems 100% trivial to make this
> work, unless you can show me it won't :)
>
> Change:
>
>    for (BasicBlock::iterator BI = BB->begin(), BE = BB->end();
>         BI != BE;) {
>      if (!ReplaceWithConstMap.empty())
>        ChangedFunction |= replaceOperandsWithConsts(&*BI);
>      ChangedFunction |= processInstruction(&*BI);
>
> To have a processPhiNode call at the beginning.
> In processPhiNode, if all arguments are the same, RAUW with the phi node
> result.
>
> Or heck, throw it in processInstruction
>
> Now, if that doesn't work, or you have actual inter-pass examples where
> this is getting left around, sure, fine.
> I want to avoid useless checks in alias analysis (and elsewhere) for
> non-canonical code.
>
> Look at it another way:
>
> If this is common IR, every analysis is likely to need to do something
> with it, at some cost.
>
> It's much cheaper to *not* have to process it at all, than have to process
> it everywhere.
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160714/6bdf1ce8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160714/6bdf1ce8/attachment.gif>


More information about the llvm-commits mailing list