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

[Daniel Berlin via llvm-dev]

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/fec8aca6/attachment.html>