[PATCH] D24805: [GVNSink] Initial GVNSink prototype

[Daniel Berlin via llvm-commits]

>
>
> ================
> Comment at: lib/Transforms/Scalar/GVNSink.cpp:664
> @@ +663,3 @@
> +
> +    // FIXME: I'm certain this MemorySSA update code is wrong, but not
> sure
> +    // how to fix it.
> ----------------
> I'll have a look.
>
>
Among other things, note that we build pruned memoryssa form.

This means if you have

if ()
{
  memorydef
  memoryuse
}
else {
  memorydef
  memoryuse
}
<no memoryuse>

we will not place a phi below the if statement.

This basically means any pass that does sinking may have to create phis if
they don't exist.

If you delete
IDFs.setLiveInBlocks(LiveInBlocks);  in MemorySSA.cpp

you should be guaranteed that the MemoryPhi's you need for sinking should
already exist, and it should be a matter of just the right move and
replacement steps.

I'm pretty convinced at this point it's not worth saving a small number of
unused MemoryPhis to make updating more complicated for all users.

So if you want to send a patch to delete that line in MemorySSA (and
update/remove the one test that tests it), i'll LGTM.





> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24805
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160921/36c14fd5/attachment.html>