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

Sebastian Pop via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 13 13:55:26 PDT 2016


> 1. No MemorySSA form handles *hoisting* stores (not gcc, not open64's
> mu/chi/etc).
> It requires building the reverse problem to make these links exist.

Would it be possible to provide an inverse MemorySSA graph:
like a CFG df_walk has the idf_walk, except as you are saying,
that for MemSSA we would have to build the inverse graph that
is not just an upside-down version of MemSSA.

MemSSA represents RAW and WAW dependences.
The inverseMemSSA would represent WAR dependences.

> Instead, most things *sink* stores, which MemorySSA makes easy.
> See SSUPRE, etc.
>
> 2. If you are going to write your own walkers, etc, we don't make it easy
> enough.
>
> My initial view is: Rather than do this as a completely separate piece by
> piece query interface, the intention was for folks to write custom walkers,
> and use those.
> For example, the code in this patch could be written as a MemorySSA walker
> that implemented "getClobberingMemoryAccess" for stores.
> It has a different definition of what it means to be a clobbering memory
> access than the default walker.
> That's okay.
>
> *Inside of walkers*, we should make instructionClobbersQuery available.
>
> Thoughts?

I'm not sure the MemSSA will have all the links to be able to walk
the WAR dependences.

In the mean time, is the current patch suitable to commit to fix the miscompile?

Thanks,
Sebastian


More information about the llvm-commits mailing list