InstCombine: avoid non-reducing xor combine

[t.p.northover]

Summary

We should only fold min(not X, C) -> not max(X, not C) if not C is as simple as C, otherwise there's a competing not max(X, not Y) -> min(not X, Y) that will form an infinite loop. Checking for a non-expr constant is a good way to do this (the fold above already takes care of C == not D so we don't lose any power that way).

[lebedev.ri]

Constant expressions are evil.
Thanks, LG.

[spatel]

LGTM - we may want to do a bulk search/replace of m_Constant() -> m_ImmConstant() to avoid these loops.

[spatel]

I pushed an alternative fix with:
8a10f4a0f661a1171 ( D109058 )

So I think we can abandon this patch. If there's some constant expression variant that still escapes/loops, let me know.

[t.p.northover]

Thanks for that fix, sorry I was silent.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

if (match(I0, m_Not(m_Value(X)))) {
       // max (not X), (not Y) --> not (min X, Y)
       Intrinsic::ID InvID = getInverseMinMaxIntrinsic(IID);
       if (match(I1, m_Not(m_Value(Y))) &&
           (I0->hasOneUse() || I1->hasOneUse())) {
         Value *InvMaxMin = Builder.CreateBinaryIntrinsic(InvID, X, Y);
         return BinaryOperator::CreateNot(InvMaxMin);
       }
       // max (not X), C --> not(min X, ~C)
-      if (match(I1, m_Constant(C)) && I0->hasOneUse()) {
+      if (match(I1, m_ImmConstant(C)) && I0->hasOneUse()) {
         Constant *NotC = ConstantExpr::getNot(C);
         Value *InvMaxMin = Builder.CreateBinaryIntrinsic(InvID, X, NotC);
         return BinaryOperator::CreateNot(InvMaxMin);
       }
     }
 
     // smax(X, -X) --> abs(X)
     // smin(X, -X) --> -abs(X)

llvm/test/Transforms/InstCombine/minmax-intrinsics.ll

 ; CHECK-NEXT:    [[M:%.*]] = xor i8 [[TMP1]], -1
 ; CHECK-NEXT:    ret i8 [[M]]
 ;
   %notx = xor i8 %x, -1
   %m = call i8 @llvm.umin.i8(i8 -45, i8 %notx)
   ret i8 %m
 }
 
+define i8 @umin_of_not_and_nontrivial_const(i8 %x) {
+; CHECK-LABEL: @umin_of_not_and_nontrivial_const(
+; CHECK-NEXT:    [[NOTX:%.*]] = xor i8 %x, -1
+; CHECK-NEXT:    [[M:%.*]] = call i8 @llvm.umin.i8(i8 [[NOTX]], i8 ptrtoint ({{.*}}))
+; CHECK-NEXT:    ret i8 [[M]]
+;
+  %notx = xor i8 %x, -1
+  %m = call i8 @llvm.umin.i8(i8 ptrtoint (i8(i8)* @umin_of_not_and_nontrivial_const to i8), i8 %notx)
+  ret i8 %m
+}
+
 ; Negative test - too many uses
 
 define i8 @umin_of_not_and_const_uses(i8 %x) {
 ; CHECK-LABEL: @umin_of_not_and_const_uses(
 ; CHECK-NEXT:    [[NOTX:%.*]] = xor i8 [[X:%.*]], -1
 ; CHECK-NEXT:    call void @use(i8 [[NOTX]])
 ; CHECK-NEXT:    [[M:%.*]] = call i8 @llvm.umin.i8(i8 [[NOTX]], i8 -45)
 ; CHECK-NEXT:    ret i8 [[M]]