[PATCH] D42007: [SimplifyCFG] Try to change store operation type when sinking

Matthew Simpson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 12 15:19:56 PST 2018


mssimpso added a comment.

In https://reviews.llvm.org/D42007#975304, @davide wrote:

> I might consider holding my nose, if this restores something that was there.
>  I think the other two passes we have for doing sinking aren't currently enabled (GVNSink and Sink.cpp), although unless somebody puts effort in them this will always be a chicken-egg problem.
>  I'm not sure I'll have the time to review this patch carefully, I don't want to put you on the hook, but if you can consider an alternative, that would be great.
>  If you look at the original review you'll notice that I was fundamentally opposed to the change, see e.g. https://reviews.llvm.org/D38566#926530
>  (FWIW, it doesn't matter where we move sinking/hoisting there will be always some case that we can't get properly. We, of course should prioritize for the common case, at least IMHO).


I agree with that. I'll play around with GVNSink when I have a chance (this particular patch isn't exceptionally important to me right now). If we can enable GVNSink, perhaps we'll be able to completely remove sinking from SimplifyCFG.


Repository:
  rL LLVM

https://reviews.llvm.org/D42007





More information about the llvm-commits mailing list