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

[Geoff Berry via llvm-commits]

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->getClobberingAccess(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(CurrentStore->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/0ca7f590/attachment.html>