[PATCH] D52709: Add -instcombine-code-sinking option

[Zixuan Wu (Zeson) via Phabricator via llvm-commits]

wuzish added a comment.
Herald added a project: LLVM.

In D52709#1263679 <https://reviews.llvm.org/D52709#1263679>, @craig.topper wrote:

> In D52709#1263663 <https://reviews.llvm.org/D52709#1263663>, @xbolva00 wrote:
>
> > Wouldn't it better to pull it out of instcombine into.. let say.. CodeSinkingPass?
>
>
> Absolutely. This code doesn't belong in InstCombine. This code isn't even the code we would use if we had a real sinking pass. This is a very simple sinking algorithm. It only moves code to an immediate successor and it has a bogus one use check in it. I think we should have a real sinking pass. lib/Transforms/Scalar/Sink.cpp is probably a good starting point, I believe its only used by one target today. I think adding a disable here is a good first step towards being able to experiment with a real sinking pass.


I also have a question that why lib/Transforms/Scalar/Sink.cpp is not enabled as default for all targets to do sinking instead of other separate places.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52709/new/

https://reviews.llvm.org/D52709