[PATCH] D52177: [InstCombine] Fold ~A - Min/Max(~A, O) -> Max/Min(A, ~O) - A

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I was hoping to find a more general solution for min/max with nots, but I'm not seeing it, so just a few nits in the inline comments.
Craig's been fighting infinite loops in this area, so let's see if he has any comments on the safety constraints.



================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1670
+    // Min/Max(O, ~A) - ~A -> A - Max/Min(A, ~O)
+    // So long as O here is freely invertable, this will be neutral or a win.
+    Value *LHS, *RHS, *A;
----------------
typo: invertible


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1683-1684
+        std::swap(LHS, RHS);
+      if (IsFreeToInvert(LHS, !LHS->hasNUsesOrMore(3)) &&
+          !NotA->hasNUsesOrMore(4)) {
+        // Note: We don't generate the inverse max/min, just create the not of
----------------
The uses constraints deserve a code comment.


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:181-183
+  // Selects with invertable operands are freely invertable
+  if (match(V, m_Select(m_Value(), m_Not(m_Value()), m_Not(m_Value()))))
+    return WillInvertAllUses;
----------------
This is similar to what I was imagining in a comment in D51964, but I'm still not sure if we need to special-case min/max patterns for the extra-uses.

The LHS->hasNUsesOrMore(3) hack in the caller might be enough...


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:181
 
+  // Selects with invertable operands are freely invertable
+  if (match(V, m_Select(m_Value(), m_Not(m_Value()), m_Not(m_Value()))))
----------------
typo: invertible


https://reviews.llvm.org/D52177