[InstCombine] Always try to invert non-canonical predicate of an icmp

Summary:
The actual transform i was going after was:
https://rise4fun.com/Alive/Tp9H
```
Name: zz
Pre: isPowerOf2(C0) && isPowerOf2(C1) && C1 == C0
%t0 = and i8 %x, C0
%r = icmp eq i8 %t0, C1
  =>
%t = icmp eq i8 %t0, 0
%r = xor i1 %t, -1

Name: zz
Pre: isPowerOf2(C0)
%t0 = and i8 %x, C0
%r = icmp ne i8 %t0, 0
  =>
%t = icmp eq i8 %t0, 0
%r = xor i1 %t, -1
```
but as it can be seen from the current tests, we already canonicalize most of it,
and we are only missing handling multi-use non-canonical icmp predicates.

If we have both `!=0` and `==0`, even though we can CSE them,
we end up being stuck with them. We should canonicalize to the `==0`.

I believe this is one of the cleanup steps i'll need after `-scalarizer`
if i end up proceeding with my WIP alloca promotion helper pass.

Reviewers: spatel, jdoerfert, nikic

Reviewed By: nikic

Subscribers: zzheng, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D83139

Author: LebedevRI

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

@@ -5283,6 +5283,47 @@ static ICmpInst *canonicalizeCmpWithConstant(ICmpInst &I) {
   return new ICmpInst(FlippedStrictness->first, Op0, FlippedStrictness->second);
 }
 
+/// If we have a comparison with a non-canonical predicate, if we can update
+/// all the users, invert the predicate and adjust all the users.
+static CmpInst *canonicalizeICmpPredicate(CmpInst &I) {
+  // Is the predicate already canonical?
+  CmpInst::Predicate Pred = I.getPredicate();
+  if (isCanonicalPredicate(Pred))
+    return nullptr;
+
+  // Can all users be adjusted to predicate inversion?
+  if (!canFreelyInvertAllUsersOf(&I, /*IgnoredUser=*/nullptr))
+    return nullptr;
+
+  // Ok, we can canonicalize comparison!
+  // Let's first invert the comparison's predicate.
+  I.setPredicate(CmpInst::getInversePredicate(Pred));
+  I.setName(I.getName() + ".not");
+
+  // And now let's adjust every user.
+  for (User *U : I.users()) {
+    switch (cast<Instruction>(U)->getOpcode()) {
+    case Instruction::Select: {
+      auto *SI = cast<SelectInst>(U);
+      SI->swapValues();
+      SI->swapProfMetadata();
+      break;
+    }
+    case Instruction::Br:
+      cast<BranchInst>(U)->swapSuccessors(); // swaps prof metadata too
+      break;
+    case Instruction::Xor:
+      U->replaceAllUsesWith(&I);
+      break;
+    default:
+      llvm_unreachable("Got unexpected user - out of sync with "
+                       "canFreelyInvertAllUsersOf() ?");
+    }
+  }
+
+  return &I;
+}
+
 /// Integer compare with boolean values can always be turned into bitwise ops.
 static Instruction *canonicalizeICmpBool(ICmpInst &I,
                                          InstCombiner::BuilderTy &Builder) {
@@ -5521,8 +5562,11 @@ Instruction *InstCombiner::visitICmpInst(ICmpInst &I) {
     if (Instruction *Res = canonicalizeICmpBool(I, Builder))
       return Res;
 
-  if (ICmpInst *NewICmp = canonicalizeCmpWithConstant(I))
-    return NewICmp;
+  if (Instruction *Res = canonicalizeCmpWithConstant(I))
+    return Res;
+
+  if (Instruction *Res = canonicalizeICmpPredicate(I))
+    return Res;
 
   if (Instruction *Res = foldICmpWithConstant(I))
     return Res;

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

@@ -215,18 +215,23 @@ static inline bool isFreeToInvert(Value *V, bool WillInvertAllUses) {
 }
 
 /// Given i1 V, can every user of V be freely adapted if V is changed to !V ?
+/// InstCombine's canonicalizeICmpPredicate() must be kept in sync with this fn.
 ///
 /// See also: isFreeToInvert()
 static inline bool canFreelyInvertAllUsersOf(Value *V, Value *IgnoredUser) {
   // Look at every user of V.
-  for (User *U : V->users()) {
-    if (U == IgnoredUser)
+  for (Use &U : V->uses()) {
+    if (U.getUser() == IgnoredUser)
       continue; // Don't consider this user.
 
-    auto *I = cast<Instruction>(U);
+    auto *I = cast<Instruction>(U.getUser());
     switch (I->getOpcode()) {
     case Instruction::Select:
+      if (U.getOperandNo() != 0) // Only if the value is used as select cond.
+        return false;
+      break;
     case Instruction::Br:
+      assert(U.getOperandNo() == 0 && "Must be branching on that value.");
       break; // Free to invert by swapping true/false values/destinations.
     case Instruction::Xor: // Can invert 'xor' if it's a 'not', by ignoring it.
       if (!match(I, m_Not(m_Value())))

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

@@ -2532,21 +2532,7 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
   if (Instruction *I = canonicalizeScalarSelectOfVecs(SI, *this))
     return I;
 
-  // Canonicalize a one-use integer compare with a non-canonical predicate by
-  // inverting the predicate and swapping the select operands. This matches a
-  // compare canonicalization for conditional branches.
-  // TODO: Should we do the same for FP compares?
   CmpInst::Predicate Pred;
-  if (match(CondVal, m_OneUse(m_ICmp(Pred, m_Value(), m_Value()))) &&
-      !isCanonicalPredicate(Pred)) {
-    // Swap true/false values and condition.
-    CmpInst *Cond = cast<CmpInst>(CondVal);
-    Cond->setPredicate(CmpInst::getInversePredicate(Pred));
-    SI.swapValues();
-    SI.swapProfMetadata();
-    Worklist.push(Cond);
-    return &SI;
-  }
 
   if (SelType->isIntOrIntVectorTy(1) &&
       TrueVal->getType() == CondVal->getType()) {

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

@@ -2755,9 +2755,9 @@ Instruction *InstCombiner::visitBranchInst(BranchInst &BI) {
     return replaceOperand(
         BI, 0, ConstantInt::getFalse(BI.getCondition()->getType()));
 
-  // Canonicalize, for example, icmp_ne -> icmp_eq or fcmp_one -> fcmp_oeq.
+  // Canonicalize, for example, fcmp_one -> fcmp_oeq.
   CmpInst::Predicate Pred;
-  if (match(&BI, m_Br(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())),
+  if (match(&BI, m_Br(m_OneUse(m_FCmp(Pred, m_Value(), m_Value())),
                       m_BasicBlock(), m_BasicBlock())) &&
       !isCanonicalPredicate(Pred)) {
     // Swap destinations and condition.

llvm/test/ThinLTO/X86/cfi-devirt.ll

@@ -110,10 +110,10 @@ cont2:
 
   ; Check that traps are conditional. Invalid TYPE_ID can cause
   ; unconditional traps.
-  ; CHECK-IR: br i1 {{.*}}, label %trap
+  ; CHECK-IR: br i1 {{.*}}, label %trap, label %cont2
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %8
+  ; CHECK-IR: %call3 = tail call i32 %7
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }

llvm/test/Transforms/InstCombine/canonicalize-selects-icmp-condition-bittest.ll

@@ -8,8 +8,8 @@ declare void @use1(i1)
 define i8 @p0(i8 %x, i8 %v0, i8 %v1) {
 ; CHECK-LABEL: @p0(
 ; CHECK-NEXT:    [[T0:%.*]] = and i8 [[X:%.*]], 1
-; CHECK-NEXT:    [[T1:%.*]] = icmp eq i8 [[T0]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1]], i8 [[V1:%.*]], i8 [[V0:%.*]], !prof !0
+; CHECK-NEXT:    [[T1_NOT:%.*]] = icmp eq i8 [[T0]], 0
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1_NOT]], i8 [[V1:%.*]], i8 [[V0:%.*]], !prof !0
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %t0 = and i8 %x, 1
@@ -20,8 +20,8 @@ define i8 @p0(i8 %x, i8 %v0, i8 %v1) {
 define i8 @p1(i8 %x, i8 %v0, i8 %v1) {
 ; CHECK-LABEL: @p1(
 ; CHECK-NEXT:    [[T0:%.*]] = and i8 [[X:%.*]], 1
-; CHECK-NEXT:    [[T1:%.*]] = icmp eq i8 [[T0]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1]], i8 [[V1:%.*]], i8 [[V0:%.*]]
+; CHECK-NEXT:    [[T1_NOT:%.*]] = icmp eq i8 [[T0]], 0
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1_NOT]], i8 [[V1:%.*]], i8 [[V0:%.*]]
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %t0 = and i8 %x, 1
@@ -51,14 +51,14 @@ define i8 @t3(i8 %x, i8 %v0, i8 %v1, i8 %v2, i8 %v3, i8* %out, i1 %c) {
 ; CHECK-LABEL: @t3(
 ; CHECK-NEXT:  bb0:
 ; CHECK-NEXT:    [[T0:%.*]] = and i8 [[X:%.*]], 1
-; CHECK-NEXT:    [[T1:%.*]] = icmp ne i8 [[T0]], 0
+; CHECK-NEXT:    [[T1_NOT:%.*]] = icmp eq i8 [[T0]], 0
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; CHECK:       bb1:
-; CHECK-NEXT:    [[R0:%.*]] = select i1 [[T1]], i8 [[V0:%.*]], i8 [[V1:%.*]]
+; CHECK-NEXT:    [[R0:%.*]] = select i1 [[T1_NOT]], i8 [[V1:%.*]], i8 [[V0:%.*]]
 ; CHECK-NEXT:    store i8 [[R0]], i8* [[OUT:%.*]], align 1
 ; CHECK-NEXT:    br label [[BB2]]
 ; CHECK:       bb2:
-; CHECK-NEXT:    [[R1:%.*]] = select i1 [[T1]], i8 [[V2:%.*]], i8 [[V3:%.*]]
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[T1_NOT]], i8 [[V3:%.*]], i8 [[V2:%.*]]
 ; CHECK-NEXT:    ret i8 [[R1]]
 ;
 bb0:
@@ -76,10 +76,10 @@ bb2:
 define i8 @t4(i8 %x, i8 %v0, i8 %v1, i8 %v2, i8 %v3, i8* %out) {
 ; CHECK-LABEL: @t4(
 ; CHECK-NEXT:    [[T0:%.*]] = and i8 [[X:%.*]], 1
-; CHECK-NEXT:    [[T1:%.*]] = icmp ne i8 [[T0]], 0
-; CHECK-NEXT:    [[R0:%.*]] = select i1 [[T1]], i8 [[V0:%.*]], i8 [[V1:%.*]]
+; CHECK-NEXT:    [[T1_NOT:%.*]] = icmp eq i8 [[T0]], 0
+; CHECK-NEXT:    [[R0:%.*]] = select i1 [[T1_NOT]], i8 [[V1:%.*]], i8 [[V0:%.*]]
 ; CHECK-NEXT:    store i8 [[R0]], i8* [[OUT:%.*]], align 1
-; CHECK-NEXT:    [[R1:%.*]] = select i1 [[T1]], i8 [[V2:%.*]], i8 [[V3:%.*]]
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[T1_NOT]], i8 [[V3:%.*]], i8 [[V2:%.*]]
 ; CHECK-NEXT:    ret i8 [[R1]]
 ;
   %t0 = and i8 %x, 1
@@ -112,8 +112,8 @@ define i8 @n6(i8 %x, i8 %v0, i8 %v1) {
 define i8 @n7(i8 %x, i8 %v0, i8 %v1) {
 ; CHECK-LABEL: @n7(
 ; CHECK-NEXT:    [[T0:%.*]] = and i8 [[X:%.*]], 1
-; CHECK-NEXT:    [[T1:%.*]] = icmp eq i8 [[T0]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1]], i8 [[V0:%.*]], i8 [[V1:%.*]]
+; CHECK-NEXT:    [[T1_NOT_NOT:%.*]] = icmp eq i8 [[T0]], 0
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[T1_NOT_NOT]], i8 [[V0:%.*]], i8 [[V1:%.*]]
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %t0 = and i8 %x, 1

llvm/test/Transforms/InstCombine/icmp-mul-zext.ll

@@ -13,16 +13,16 @@ define i32 @sterix(i32, i8, i64) {
 ; CHECK-NEXT:    [[SHR:%.*]] = lshr i32 [[MUL]], [[SH_PROM]]
 ; CHECK-NEXT:    [[CONV2:%.*]] = zext i32 [[SHR]] to i64
 ; CHECK-NEXT:    [[MUL3:%.*]] = mul nuw nsw i64 [[CONV]], [[CONV2]]
-; CHECK-NEXT:    [[TMP3:%.*]] = icmp ugt i64 [[MUL3]], 4294967295
-; CHECK-NEXT:    br i1 [[TMP3]], label [[LOR_END:%.*]], label [[LOR_RHS:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp ult i64 [[MUL3]], 4294967296
+; CHECK-NEXT:    br i1 [[TMP3]], label [[LOR_RHS:%.*]], label [[LOR_END:%.*]]
 ; CHECK:       lor.rhs:
 ; CHECK-NEXT:    [[AND:%.*]] = and i64 [[MUL3]], [[TMP2]]
 ; CHECK-NEXT:    [[CONV4:%.*]] = trunc i64 [[AND]] to i32
-; CHECK-NEXT:    [[TOBOOL7:%.*]] = icmp eq i32 [[CONV4]], 0
-; CHECK-NEXT:    [[PHI_CAST:%.*]] = zext i1 [[TOBOOL7]] to i32
+; CHECK-NEXT:    [[TOBOOL7_NOT:%.*]] = icmp eq i32 [[CONV4]], 0
+; CHECK-NEXT:    [[PHITMP:%.*]] = zext i1 [[TOBOOL7_NOT]] to i32
 ; CHECK-NEXT:    br label [[LOR_END]]
 ; CHECK:       lor.end:
-; CHECK-NEXT:    [[TMP4:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[PHI_CAST]], [[LOR_RHS]] ]
+; CHECK-NEXT:    [[TMP4:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[PHITMP]], [[LOR_RHS]] ]
 ; CHECK-NEXT:    ret i32 [[TMP4]]
 ;
 entry:

llvm/test/Transforms/InstCombine/logical-select.ll

@@ -19,8 +19,8 @@ define i32 @foo(i32 %a, i32 %b, i32 %c, i32 %d) {
 
 define i32 @bar(i32 %a, i32 %b, i32 %c, i32 %d) {
 ; CHECK-LABEL: @bar(
-; CHECK-NEXT:    [[E:%.*]] = icmp slt i32 [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[E]], i32 [[C:%.*]], i32 [[D:%.*]]
+; CHECK-NEXT:    [[E_NOT:%.*]] = icmp slt i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[E_NOT]], i32 [[C:%.*]], i32 [[D:%.*]]
 ; CHECK-NEXT:    ret i32 [[TMP1]]
 ;
   %e = icmp slt i32 %a, %b
@@ -69,8 +69,8 @@ define i32 @fold_inverted_icmp_preds(i32 %a, i32 %b, i32 %c, i32 %d) {
 ; CHECK-LABEL: @fold_inverted_icmp_preds(
 ; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i32 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[CMP1]], i32 [[C:%.*]], i32 0
-; CHECK-NEXT:    [[CMP2:%.*]] = icmp slt i32 [[A]], [[B]]
-; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[CMP2]], i32 0, i32 [[D:%.*]]
+; CHECK-NEXT:    [[CMP2_NOT:%.*]] = icmp slt i32 [[A]], [[B]]
+; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[CMP2_NOT]], i32 0, i32 [[D:%.*]]
 ; CHECK-NEXT:    [[OR:%.*]] = or i32 [[SEL1]], [[SEL2]]
 ; CHECK-NEXT:    ret i32 [[OR]]
 ;
@@ -88,8 +88,8 @@ define i32 @fold_inverted_icmp_preds_reverse(i32 %a, i32 %b, i32 %c, i32 %d) {
 ; CHECK-LABEL: @fold_inverted_icmp_preds_reverse(
 ; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i32 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[CMP1]], i32 0, i32 [[C:%.*]]
-; CHECK-NEXT:    [[CMP2:%.*]] = icmp slt i32 [[A]], [[B]]
-; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[CMP2]], i32 [[D:%.*]], i32 0
+; CHECK-NEXT:    [[CMP2_NOT:%.*]] = icmp slt i32 [[A]], [[B]]
+; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[CMP2_NOT]], i32 [[D:%.*]], i32 0
 ; CHECK-NEXT:    [[OR:%.*]] = or i32 [[SEL1]], [[SEL2]]
 ; CHECK-NEXT:    ret i32 [[OR]]
 ;
@@ -124,8 +124,8 @@ define i32 @fold_inverted_fcmp_preds(float %a, float %b, i32 %c, i32 %d) {
 
 define <2 x i32> @fold_inverted_icmp_vector_preds(<2 x i32> %a, <2 x i32> %b, <2 x i32> %c, <2 x i32> %d) {
 ; CHECK-LABEL: @fold_inverted_icmp_vector_preds(
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq <2 x i32> [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    [[SEL1:%.*]] = select <2 x i1> [[CMP1]], <2 x i32> zeroinitializer, <2 x i32> [[C:%.*]]
+; CHECK-NEXT:    [[CMP1_NOT:%.*]] = icmp eq <2 x i32> [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[SEL1:%.*]] = select <2 x i1> [[CMP1_NOT]], <2 x i32> zeroinitializer, <2 x i32> [[C:%.*]]
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq <2 x i32> [[A]], [[B]]
 ; CHECK-NEXT:    [[SEL2:%.*]] = select <2 x i1> [[CMP2]], <2 x i32> [[D:%.*]], <2 x i32> zeroinitializer
 ; CHECK-NEXT:    [[OR:%.*]] = or <2 x i32> [[SEL1]], [[SEL2]]
@@ -535,9 +535,9 @@ define <4 x i32> @vec_sel_xor_multi_use(<4 x i32> %a, <4 x i32> %b, <4 x i1> %c)
 
 define i32 @allSignBits(i32 %cond, i32 %tval, i32 %fval) {
 ; CHECK-LABEL: @allSignBits(
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i32 [[COND:%.*]], -1
-; CHECK-NEXT:    [[TMP2:%.*]] = select i1 [[TMP1]], i32 [[FVAL:%.*]], i32 [[TVAL:%.*]]
-; CHECK-NEXT:    ret i32 [[TMP2]]
+; CHECK-NEXT:    [[DOTNOT:%.*]] = icmp slt i32 [[COND:%.*]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[DOTNOT]], i32 [[TVAL:%.*]], i32 [[FVAL:%.*]]
+; CHECK-NEXT:    ret i32 [[TMP1]]
 ;
   %bitmask = ashr i32 %cond, 31
   %not_bitmask = xor i32 %bitmask, -1
@@ -549,9 +549,9 @@ define i32 @allSignBits(i32 %cond, i32 %tval, i32 %fval) {
 
 define <4 x i8> @allSignBits_vec(<4 x i8> %cond, <4 x i8> %tval, <4 x i8> %fval) {
 ; CHECK-LABEL: @allSignBits_vec(
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt <4 x i8> [[COND:%.*]], <i8 -1, i8 -1, i8 -1, i8 -1>
-; CHECK-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i8> [[FVAL:%.*]], <4 x i8> [[TVAL:%.*]]
-; CHECK-NEXT:    ret <4 x i8> [[TMP2]]
+; CHECK-NEXT:    [[DOTNOT:%.*]] = icmp sgt <4 x i8> [[COND:%.*]], <i8 -1, i8 -1, i8 -1, i8 -1>
+; CHECK-NEXT:    [[TMP1:%.*]] = select <4 x i1> [[DOTNOT]], <4 x i8> [[FVAL:%.*]], <4 x i8> [[TVAL:%.*]]
+; CHECK-NEXT:    ret <4 x i8> [[TMP1]]
 ;
   %bitmask = ashr <4 x i8> %cond, <i8 7, i8 7, i8 7, i8 7>
   %not_bitmask = xor <4 x i8> %bitmask, <i8 -1, i8 -1, i8 -1, i8 -1>