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

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2413
+  else if (!match(&I, m_LogicalAnd(m_Value(Cond1), m_Value(Cond2))))
+    return false;
+
----------------
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.


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


================
Comment at: llvm/test/Transforms/LICM/min_max.ll:883
 
 ; TODO: Turn ule against constant into ult against constant, then do the same as in test_ult.
 ; https://alive2.llvm.org/ce/z/rucunQ
----------------
ule against constant is already canonicalized to ult by InstCombine, no need to handle here.


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

https://reviews.llvm.org/D143726