llvm dev - May 2016 - [MemorySSA] Potential CachingMemorySSAWalker bug

I suspect something is pulling the RHS of the memorydef and caching it for
calls it should not be used for.
In particular, i suspect we are about to discover we can't cache the
results from both versions of getClobberingMemoryAccess together, or that
the cache is not always getting consistently written.




On Mon, May 2, 2016 at 11:16 AM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:
> Yeah, that sounds like a fun bug. I'll take a look later today and see
> what I can find out. :)
>
> On Fri, Apr 29, 2016 at 2:55 PM, Geoff Berry <gberry at
codeaurora.org>
> wrote:
>
>> Hi guys,
>>
>> I think I have run into another CachingMemorySSAWalker cache bug. 
It's a
>> bit tricky to reproduce, so I'd like to start by trying to show you
what is
>> happening when running EarlyCSE with my local changes to use MemorySSA.
>> I've attached a debug log that shows that the value returned by
>> getClobberingMemoryAccess(Inst) after a call to removeMemoryAccess is
>> wrong.  The MemorySSA node in question is MemoryUse(7), and the
corruption
>> happens after a call to remove MemoryUse(2), at which point its clobber
>> value changes to '1 = MemoryDef(liveOnEntry)'.  The interesting
thing is
>> that is doesn't seem to be the first call to
getClobberingMemoryAccess
>> after the removal that causes the corruption, but rather the second.
>> You'll notice that I added calls to getClobberingMemoryAccess when
doing
>> MSSA.dump(), which is what I'm using to attempt to figure out when
the
>> cache gets corrupted.
>> Hopefully this is enough information to debug the problem.  If not
>> perhaps we can look at getting my EarlyCSE changes checked in in a
disabled
>> state so you can reproduce the problem directly.  I'm also happy to
help
>> debug it farther.
>>
>> --
>> Geoff Berry
>> Employee of Qualcomm Innovation Center, Inc.
>>  Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160502/71c3c4db/attachment.html>

(IE the thing we cache has to be the same no matter which version is
called. If it can't be for some reason, we can't cache results from both
in
the same cache. If it can be, i suspect there is a bug in making that work
:P)


I suspect the latter, and not the former.


On Mon, May 2, 2016 at 11:32 AM, Daniel Berlin <dberlin at dberlin.org>
wrote:
> I suspect something is pulling the RHS of the memorydef and caching it for
> calls it should not be used for.
> In particular, i suspect we are about to discover we can't cache the
> results from both versions of getClobberingMemoryAccess together, or that
> the cache is not always getting consistently written.
>
>
>
>
> On Mon, May 2, 2016 at 11:16 AM, George Burgess IV <
> george.burgess.iv at gmail.com> wrote:
>
>> Yeah, that sounds like a fun bug. I'll take a look later today and
see
>> what I can find out. :)
>>
>> On Fri, Apr 29, 2016 at 2:55 PM, Geoff Berry <gberry at
codeaurora.org>
>> wrote:
>>
>>> Hi guys,
>>>
>>> I think I have run into another CachingMemorySSAWalker cache bug. 
It's
>>> a bit tricky to reproduce, so I'd like to start by trying to
show you what
>>> is happening when running EarlyCSE with my local changes to use
MemorySSA.
>>> I've attached a debug log that shows that the value returned by
>>> getClobberingMemoryAccess(Inst) after a call to removeMemoryAccess
is
>>> wrong.  The MemorySSA node in question is MemoryUse(7), and the
corruption
>>> happens after a call to remove MemoryUse(2), at which point its
clobber
>>> value changes to '1 = MemoryDef(liveOnEntry)'.  The
interesting thing is
>>> that is doesn't seem to be the first call to
getClobberingMemoryAccess
>>> after the removal that causes the corruption, but rather the
second.
>>> You'll notice that I added calls to getClobberingMemoryAccess
when doing
>>> MSSA.dump(), which is what I'm using to attempt to figure out
when the
>>> cache gets corrupted.
>>> Hopefully this is enough information to debug the problem.  If not
>>> perhaps we can look at getting my EarlyCSE changes checked in in a
disabled
>>> state so you can reproduce the problem directly.  I'm also
happy to help
>>> debug it farther.
>>>
>>> --
>>> Geoff Berry
>>> Employee of Qualcomm Innovation Center, Inc.
>>>  Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160502/e23f08bd/attachment.html>

I've put my changes to EarlyCSE that trigger this case up on phab here: 
http://reviews.llvm.org/D19821.  These changes depend on 
http://reviews.llvm.org/D19664 so that will need to be applied first.  
With these changes applied, the original attached .ll file should 
trigger this bug when compiled with opt -early-cse -early-cse-use-memoryssa

On 5/2/2016 2:34 PM, Daniel Berlin wrote:
> (IE the thing we cache has to be the same no matter which version is 
> called. If it can't be for some reason, we can't cache results from
> both in the same cache. If it can be, i suspect there is a bug in 
> making that work :P)
>
> I suspect the latter, and not the former.
>
>
> On Mon, May 2, 2016 at 11:32 AM, Daniel Berlin <dberlin at dberlin.org 
> <mailto:dberlin at dberlin.org>> wrote:
>
>     I suspect something is pulling the RHS of the memorydef and
>     caching it for calls it should not be used for.
>     In particular, i suspect we are about to discover we can't cache
>     the results from both versions of getClobberingMemoryAccess
>     together, or that the cache is not always getting consistently
>     written.
>
>
>
>
>     On Mon, May 2, 2016 at 11:16 AM, George Burgess IV
>     <george.burgess.iv at gmail.com <mailto:george.burgess.iv at
gmail.com>>
>     wrote:
>
>         Yeah, that sounds like a fun bug. I'll take a look later today
>         and see what I can find out. :)
>
>         On Fri, Apr 29, 2016 at 2:55 PM, Geoff Berry
>         <gberry at codeaurora.org <mailto:gberry at
codeaurora.org>> wrote:
>
>             Hi guys,
>
>             I think I have run into another CachingMemorySSAWalker
>             cache bug. It's a bit tricky to reproduce, so I'd like
to
>             start by trying to show you what is happening when running
>             EarlyCSE with my local changes to use MemorySSA.  I've
>             attached a debug log that shows that the value returned by
>             getClobberingMemoryAccess(Inst) after a call to
>             removeMemoryAccess is wrong.  The MemorySSA node in
>             question is MemoryUse(7), and the corruption happens after
>             a call to remove MemoryUse(2), at which point its clobber
>             value changes to '1 = MemoryDef(liveOnEntry)'.  The
>             interesting thing is that is doesn't seem to be the first
>             call to getClobberingMemoryAccess after the removal that
>             causes the corruption, but rather the second.  You'll
>             notice that I added calls to getClobberingMemoryAccess
>             when doing MSSA.dump(), which is what I'm using to attempt
>             to figure out when the cache gets corrupted.
>
>             Hopefully this is enough information to debug the
>             problem.  If not perhaps we can look at getting my
>             EarlyCSE changes checked in in a disabled state so you can
>             reproduce the problem directly.  I'm also happy to help
>             debug it farther.
>
>             -- 
>             Geoff Berry
>             Employee of Qualcomm Innovation Center, Inc.
>               Qualcomm Innovation Center, Inc. is a member of Code Aurora
Forum, a Linux Foundation Collaborative Project
>
>
>
>
-- 
Geoff Berry
Employee of Qualcomm Innovation Center, Inc.
  Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160502/eb576e30/attachment.html>