[PATCH] D143726: [LICM] Simplify (X < A && X < B) into (X < MIN(A, B)) if MIN(A, B) is loop-invariant

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2443
+    // canonical (unsigned) form;
+    // - Swap LHS and RHS to match the pattern;
+    // etc.
----------------
nikic wrote:
> This TODO is already done :)
Indeed.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2413
+  else if (!match(&I, m_LogicalAnd(m_Value(Cond1), m_Value(Cond2))))
+    return false;
+
----------------
nikic wrote:
> mkazantsev wrote:
> > mkazantsev wrote:
> > > nikic wrote:
> > > > This transform is not legal for logical and/or. Counter-proof: https://alive2.llvm.org/ce/z/7sVXdy Please limit this to just m_Or/m_And and add a test that logical case is not folded.
> > > Good catch! Thanks for pointing out.
> > Logican and can be supported if RHS's are noundef. But I'll make it in a follow-up.
> If RHS is undef, logical and will be converted to bitwise and by InstCombine. If we want to support the transform for logical and, we need to freeze the RHS.
That's an option too.


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

https://reviews.llvm.org/D143726