[PATCH] D51398: [InstCombine] Fold (min/max ~X, Y) -> ~(max/min X, ~Y) when Y is freely invertible

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Looks ok to me. A note and a remark

- The code is now doing the same thing but either for the LHS or RHS. Is there any way to deduplicate it?
- It looks like we want to more thoroughly (read: in all cases possible) hoist the `not` ops out like this? @spatel did bring that up before https://reviews.llvm.org/D50301#1191081



================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2926-2927
     if (SelectPatternResult::isMinOrMax(SPF)) {
-      Value *X;
-      if (match(RHS, m_Not(m_Value(X))))
-        std::swap(RHS, LHS);
-
-      if (match(LHS, m_Not(m_Value(X)))) {
+      if (match(LHS, m_Not(m_Value(X))) &&
+          !IsFreeToInvert(X, X->hasOneUse())) {
         Value *NotY = Builder.CreateNot(RHS);
----------------
We are checking `!IsFreeToInvert()` because we might be called before we try to actually fold the `not` into `X`?
Should probably be a comment.


https://reviews.llvm.org/D51398