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

[Daniel Berlin via llvm-commits]

On Wed, Sep 14, 2016 at 2:20 PM, Sebastian Pop <sebpop.llvm at gmail.com>
wrote:

> On Tue, Sep 13, 2016 at 7:38 PM, Daniel Berlin via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Actually, you can build a walker already that will give you war
> dependence
> > info.
>
> The more I try, the more I think WAR dependences cannot be extracted
> from the current MemorySSA walker: all liveOnEntry uses would not
> be reached by walking through the MSSA.
>

It must be linked to some store, so this is false.
In fact, it must be linked to either live on entry (which has uses), or a
store that clobbers your store in use-optimized memoryssa.
In non-use-optimized memoryssa, you would also have to walk downward uses
because it may be attached to something else.


>The users of CurrentStore are all downwards: these users are RAW
>dependent on CurrentStore.

Yes.


>     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()))

The only way to reach uses above a store is by walking through the
> def-def chain as in the above line, and then from there walking all the
> uses, that may also be uses of OriginalStore.


No.



> This is exactly what is currently implemented in trunk: from OriginalStore
> we go to ->getDefiningAccess() and from there we walk through all uses.
> It does not work because when the def-def chain may point to a store that
> does not clobber the OriginalStore,


getDefiningAccess does not, but getClobberingMemoryAccess gives you
something that *does* clobber it.

This is why i said:
Given how we determine aliasing, and we use optimize, an aliased load must
be attached to *either* live on entry, *or* a store that clobbers your store

This is guaranteed by the property of use optimization.
Now, if use-opt failed, then yeah, you may have to go through everything.

Note: I don't claim this is efficient or sensible. I'm simply refuting the
claim that they literally cannot be extracted.

They can.
It's just inefficient as hell :)


Note that dependence does not necessarily help you with safety in some of
the trickier cases of what you can tag loads with (invariantness, etc). You
may not be able to hoist stores  anyway, even though they have no
noticeable WAR dependence.





> and then the reachable uses are not
> clobering OriginalStore.



> Another example in which there is no def-def chain and
> it is illegal to move the store above the load:
> load @A ; MemoryUse(LiveIn)
> store @A ; 1 = MemoryDef(LiveIn)


liveOnEntry uses cannot be reached by walking through the MSSA.


Uh?

These will both be uses of the live on entry access.
It's a real access, it has real uses :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160914/8b0def9a/attachment.html>