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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
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;
----------------
dmgreen wrote:
> spatel wrote:
> > 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...
> The LHS->hasNUsesOrMore(3) isn't _meant_ as a hack. Unless you mean it's ugly? It should have 2 uses from the min/max, so 3+ means we wouldn't invert all uses.
> 
> The motivating case here (umin3_not_all_ops_extra_uses_invert_subs) is something like min(min(not(a), not(b)), not(c)), but with subs in there too. So there's two min's, one we are folding from, and other we are checking is freely invertible. That's what this is trying to catch.
> 
> I was reluctant to make IsFreeToInvert recursive, but that would make it more powerful and remove the need for just checking m_Not's here.
Yeah - the hack comment was really about the ugliness, and the root cause of that is not having intrinsics for integer min/max...but it'd still be easier reading if we had hasNUsesOrLess()?


https://reviews.llvm.org/D52177