[PATCH] D19821: [EarlyCSE] Use MemorySSA if available.

[George Burgess IV via llvm-commits]

> With the caveat that this will never occur for loads unless you remove
the stores in the way of those specific loads ;)

Yup! I should've been more clear about that -- my bad. :)

On Mon, Jun 20, 2016 at 1:26 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Mon, Jun 20, 2016 at 12:56 PM, George Burgess IV <
> george.burgess.iv at gmail.com> wrote:
>
>> george.burgess.iv added a comment.
>>
>> > George, can you stare at these quickly and see if any of your caching
>> changes/etc will help?
>>
>>
>> That depends on what exactly is slowing the benchmarks down so much. If
>> our usage pattern is query -> remove -> query -> remove, then our cache may
>> become useless, since (worst case) we drop the entire thing on each
>> removal. If we primarily query defs, then this pattern gives us the same
>> effectively-n^2 behavior of MemDep.
>
>
>
> With the caveat that this will never occur for loads unless you remove the
> stores in the way of those specific loads ;)
>
>
>> One of the big goals of the new cache/walker is to allow us to drop as
>> little as possible.
>>
>
> The nice part about walkers is that we  can use different walkers for
> different purposes.
>
>
>> In terms of pure walker/cache speed, the current walker is happy to do a
>> lot of potentially useless work walking phis we can't optimize; the one I'm
>> working on will do as little work as possible in that case. Also, the
>> current walker potentially does a lot of domtree queries when caching
>> results, whereas the one I'm working on does none (except in asserts).
>> Glancing at some of the benchmarks, I'm not sure if any of that is what's
>> slowing us down here, though.
>>
>> If you'd like, I'm happy to profile/poke around and give you a more
>> definitive answer.
>>
>>
>> ================
>> Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:500
>> @@ -485,1 +499,3 @@
>>
>> +// Determine if the memory referenced by LaterInst is from the same heap
>> version
>> +// as EarlierInst.
>> ----------------
>> Nit: Please use ///
>>
>> ================
>> Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:571
>> @@ +570,3 @@
>> +      MSSAWalker->getClobberingMemoryAccess(EarlierInst);
>> +  if (EarlierHeapGen == LaterHeapGen)
>> +    return true;
>> ----------------
>> Nit: `return EarlierHeapGen == LaterHeapGen`?
>>
>>
>> http://reviews.llvm.org/D19821
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160620/d3d6852a/attachment.html>