[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