[llvm-dev] [LICM][MemorySSA] Converting LICM pass to use MemorySSA to avoid AliasSet collapse issue

[George Burgess via llvm-dev]

Hi!

> readonly calls are treated as clobbers by MemorySSA which leads to extra
walking of MemoryDefs to not regress some EarlyCSE test cases

Can you share a testcase for this too, please? Specifically, the first test
in test/Transforms/Util/MemorySSA/function-mem-attrs.ll shows that we
should treat readonly calls + calls to readonly functions as MemoryUses, so
if we're thinking they're clobbers somehow, that sounds like a bug...

Thank you!
George

On Wed, Apr 20, 2016 at 12:06 PM, Geoff Berry <gberry at codeaurora.org> wrote:

> 1)      Sounds good.  This isn’t holding me up so I’ll just try to keep
> an eye out for these changes.
>
>
>
> 2)      I’ve attached an example IR file and debug log of where the
> caching is going bad.  It depends on my changes to EarlyCSE, but hopefully
> it is clear from the debug output what is going on.  Let me know if there
> is a better way to get this repro case to you.  Also, I’ll be on IRC for
> the next couple of hours if you would like to have a quicker discussion.
>
>
>
> --
>
> Geoff Berry
>
> Employee of Qualcomm Innovation Center, Inc.
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
>
> *From:* Daniel Berlin [mailto:dberlin at dberlin.org]
> *Sent:* Wednesday, April 20, 2016 1:06 PM
> *To:* Geoff Berry <gberry at codeaurora.org>; George Burgess <gbiv at google.com
> >
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* Re: [LICM][MemorySSA] Converting LICM pass to use MemorySSA to
> avoid AliasSet collapse issue
>
>
>
>
>
>
>
> On Wed, Apr 20, 2016 at 9:58 AM, Geoff Berry <gberry at codeaurora.org>
> wrote:
>
> Hi Daniel,
>
>
>
> Thanks for the info.  I’ve started looking into converting EarlyCSE to use
> MemorySSA first since 1) I don’t think it needs any additional MemorySSA
> update API and 2) the particular case I’m looking at needs EarlyCSE to
> catch more load cases before LICM to be profitable.
>
> I have a prototype working, but have run into two issues:
>
>
>
> 1)      readonly calls are treated as clobbers by MemorySSA which leads
> to extra walking of MemoryDefs to not regress some EarlyCSE test cases.
> This isn’t a huge deal, I’m just wondering if it is intentional or
> something that just hasn’t been gotten to yet.
>
>
>
> George is working on the optimizations, of which this is one.
>
> I think this is one of the ones his current patch (under review) addresses.
>
>
>
> 2)      There seems to be a bug in the CachingMemorySSAWalker
> invalidation causing it to return MemoryAccess nodes that have been
> removed.  In the case I’m seeing, a call node is removed from MemorySSA
> which causes CachingMemorySSAWalker::invalidateInfo() to clear the
> CachedUpwardsClobberingCall map.  However, this same call node is present
> as a value in the CachedUpwardsClobberingAccess map,
>
>
>
> Unless i'm missing something, this should not have happened, and we should
> assert they are not being added to the cache.
>
>
>
> The truth is the caching parts are complicated and ugly. It was meant to
> be a pretty simple cache, but it's known to be inefficient (memory wise)
> and it's on the list of things to clean up and make sane.
>
>
>
> Do you have a testcase where this happens?
>
> A quick glance says we check whether it's a call in all the right places,
> which means there must be a place we are not *setting* isCall properly.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160420/71e40b1b/attachment.html>