[PATCH] D24517: GVN-hoist: fix store past load dependence analysis (PR30216)

[Daniel Berlin via llvm-commits]

and this could be solved by not stopping the walk until you hit
liveonentry. This is one of the rare cases where optimizing uses makes the
algorithm slower, but this is also because the only real solution is in
fact, different chains than we do now
(you want the upward use set.  If we build a form, this will be sparse and
a single user, if not, it's a set)



On Wed, Sep 14, 2016 at 10:04 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

> Yes, this is the issue where I said "it assumes certain things about
> aliasing" :)
>
>
> On Wed, Sep 14, 2016 at 9:04 AM, Geoff Berry <gberry at codeaurora.org>
> wrote:
>
>> I don't think this is right.  Consider attempting to hoist the store %p
>> just before the load in the following case:
>>
>> store @A ; 1 = MemoryDef(LiveIn)
>> load @B ; MemoryUse(LiveIn)
>> store %p ; 2 = MemoryDef(1)
>>
>> You won't see the load as a use of store @A because it has been optimized
>> to point to LiveOnEntry, so it will appear that it is safe to hoist it when
>> it is not.
>>
>>
>> On 9/13/2016 7:38 PM, Daniel Berlin wrote:
>>
>>
>>>> I'm not sure the MemSSA will have all the links to be able to walk
>>>> the WAR dependences.
>>>>
>>>
>>> It does not.
>>>
>>
>> Actually, you can build a walker already that will give you war
>> dependence info.
>>
>> Here is the pseudo code for a getClobberingAccess (note that it is faster
>> to build the entire aliased load set than to call this multiple times.
>> Noted below what to change to do that).  Note that it assumes transitivity
>> in aliasing, but this is easy to fix by changing the outerworklist to also
>> include store uses.
>>
>> Initialize OuterWorklist to be cachingWalker->getClobberingAc
>> cess(OriginalStore)
>> while (!OuterWorklist.empty())
>>   CurrentStore = OuterWorklist.pop_front();
>>
>>   // Any load that is aliased with the store must be a user of the store
>> in use-optimized memoryssa
>>   for each User of CurrentStore:
>>     if (isa<MemoryUse>(User))
>>       UseWorklist.push_back(User)
>>
>>   while (!UseWorklist.empty())
>>     CurrentAccess = UseWorklist.pop_front();
>>     if (StoreClobbersLoad(OriginalStore, CurrentAccess)) // This is
>> instructionClobbersQuery, basically
>>         return CurrentAccess // If you change this to insert it into a
>> set instead of returning, you can get the entire set of dominating aliased
>> loads
>>
>>   // Keep going up until we've gotten past the attempted hoist point
>>   if (!MSSA->dominates(CurrentStore, HoistPoint))
>>     OuterWorklist.push_back(cachingWalker->getClobberingAccess(C
>> urrentStore->getDefiningAccess()))
>>
>>
>> Note that this should check <= the number of loads than a version that
>> walks all loads does.
>>
>>
>> --
>> Geoff Berry
>> Employee of Qualcomm Datacenter Technologies, Inc.
>>  Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160914/db25cc1b/attachment.html>