[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

[Daniel Berlin via llvm-commits]

On Thu, Jul 7, 2016 at 1:45 AM, Geoff Berry <gberry at codeaurora.org> wrote:

>
>
> On 7/5/2016 8:21 PM, Daniel Berlin wrote:
>
>
>
> On Wed, Jul 6, 2016 at 1:48 AM, Geoff Berry <gberry at codeaurora.org> wrote:
>
>> gberry added a comment.
>>
>> Sorry if this isn't the right place, but I have a general question about
>> what it means to preserve MemorySSA, specifically regarding the defining
>> accesses of MemoryUse nodes.  Is the idea here that we make a best effort
>> to keep the MemoryUse defining access links optimized (i.e. never pointing
>> to a no-alias def)?
>
>
> Yes. You should fix anything obviously affected/invalidated (this is a
> step past what memdep does, which is mostly nothing). But you should not
> have to redo the links themselves except those related to the load/store
> you touched.
>
> The *cache* (if any), needs more invalidation, however.
> So if you destroy a load, that means you pretty much have to do nothing.
> If you destroy a store, you either need fine-grained tracking, or destroy
> the entire cache.
>
>
> Unless I'm missing something, this doesn't jibe with the caching walker
> changes under review (http://reviews.llvm.org/D21777).  If the walker
> doesn't cache MemoryUse clobbering accesses (assuming that a MemoryUse's
> defining access is always the "best" clobber),
>

It doesn't cache, but it should still check.


> then invalidating the cache when e.g. destroying a store isn't good enough
> since the cache is never consulted when querying about a MemoryUse's
> clobber.
>

The cache is part of the walker, so if the walker has no cache and always
looks, it doesn't matter.
If it is assuming the answer, that seems ... wrong.

I would expect it to do the one check.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160707/607bbd1a/attachment-0001.html>