[PATCH] D7864: This patch introduces MemorySSA, a virtual SSA form for memory.Details on what it looks like are in MemorySSA.h

[Daniel Berlin via llvm-commits]

On Thu, Jan 7, 2016 at 4:27 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> hfinkel added inline comments.
>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:1124
> @@ +1123,3 @@
> +  auto CurrAccessPair = UpwardsBFSWalkAccess(StartingAccess, Prev, Q);
> +  // Either we will have found something that conflicts with us, or we
> will have
> +  // hit the liveOnEntry. Check for liveOnEntry.
> ----------------
> This algorithm in getClobberingMemoryAccess does not seem correct.



> If we have this:
>
>      [ entry ]
>          |
>         ...
>          |
>      (clobbering access - a)
>         ...
>     /          \
>   ...         (clobbering access - b)
>    |              |
>   ...           ....
>    \              /
>           ...
>    (starting access)
>
> UpwardsBFSWalkAccess seems to walk upward until it finds its first
> clobbering access, and returns that when found. In the case above, if we
> find clobbering access (b) first, then we're okay, because we'll pick some
> dominating def farther down the path which is on all paths to both
> dominating accesses.


> But, if we hit clobbering access (a) first, then we'll accept that
> (incorrectly) as the new final access, because it dominates the original.
> However, not all paths to all aliasing defs terminate (or pass through)
> that access.
>


So, note, in the above case,  there *must* be a memory phi in the block of
starting access, merging the two memory accesses. We should say that is the
clobbering access.


But you are correct, that this version, in the middle of refactoring, did
not finish this rule correctly. The original algorithm only walked past a
phi if all paths away from the phi reached the same clobbering access.

This one should do the same.



>   [entry]
>     |
>   .....
>   (clobbering access - b)
>    |
>   ....   ________________________________
>     \   /                                                               |
>      (x)                                                               |
>    ......                                                               |
>       |                                                                 |
>       |    ______________________                 |
>        \   /                                        |                   |
>   (starting access)                        |                   |
>        ...                                           |                   |
>    (clobbering access - a)              |                   |
>       ...                                            |                   |
>       | |                                            |                   |
>       | |______________________|                   |
>       |                                                                  |
>       |_________________________________|
>
>

Again, we will have a memory phi node here, and we will say the phi node is
the clobbering access. We will walk one step from the phi, hit a, and say
"we have a clobbering access."
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160107/d0d4dead/attachment.html>