[llvm] [InstCombine] Improve handling of `not` and free inversion. (PR #66787)

[Nikita Popov via llvm-commits]

================
@@ -4317,13 +4327,15 @@ Instruction *InstCombinerImpl::foldNot(BinaryOperator &I) {
   auto *II = dyn_cast<IntrinsicInst>(NotOp);
   if (II && II->hasOneUse()) {
     if (match(NotOp, m_MaxOrMin(m_Value(X), m_Value(Y))) &&
-        isFreeToInvert(X, X->hasOneUse()) &&
-        isFreeToInvert(Y, Y->hasOneUse())) {
-      Intrinsic::ID InvID = getInverseMinMaxIntrinsic(II->getIntrinsicID());
-      Value *NotX = Builder.CreateNot(X);
-      Value *NotY = Builder.CreateNot(Y);
-      Value *InvMaxMin = Builder.CreateBinaryIntrinsic(InvID, NotX, NotY);
-      return replaceInstUsesWith(I, InvMaxMin);
+        isFreeToInvert(X, X->hasOneUse())) {
+      if (Value *NotY = getFreelyInverted(Y, Y->hasOneUse(), &Builder)) {
+        Intrinsic::ID InvID = getInverseMinMaxIntrinsic(II->getIntrinsicID());
+        Value *NotX = getFreelyInverted(X, X->hasOneUse(), &Builder);
+        assert(NotX != nullptr &&
+               "isFreeToInvert desynced with getFreelyInverted");
+        Value *InvMaxMin = Builder.CreateBinaryIntrinsic(InvID, NotX, NotY);
+        return replaceInstUsesWith(I, InvMaxMin);
+      }
----------------
nikic wrote:

I think this implements the same logic as getFreelyInverted() itself -- can we just call getFreelyInverted() on II?

(More generally, we should probably have a generic fold calling getFreelyInverted() here, which likely subsumes some of the other folds in this function. But maybe that's for a followup.)

https://github.com/llvm/llvm-project/pull/66787