[PATCH] D136233: [SimpleLoopUnswitch] Inject loop-invariant conditions and unswitch them when it's profitable

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2880-2915
+/// Tries to canonicalize condition described by:
+///
+///   br (LHS pred RHS), label IfTrue, label IfFalse
+///
+/// into its equivalent where `Pred` is something that we support for injected
+/// invariants (so far it is limited to ult), LHS in canonicalized form is
+/// non-invariant and RHS is an invariant.
----------------
mkazantsev wrote:
> apilipenko wrote:
> > Can be separated to a follow up patch.
> It makes the scope so narrow that what remains makes no sense.
No, because this is functionally incorrect. The code below relies on fact that predicate is canonicalized, e.g. RHS is invariant.

Besides, even if it was correct (i.e. checks are duplicated outside this method), it makes the scope of this transform very narrow, so we just won't benefit from it.




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

https://reviews.llvm.org/D136233