[InstCombine] Fold comparison of adding two zero extended booleans

[elhewaty]

Summary

zext i1 X + zext i1 Y == 0 --> !(or i1 X, Y)
zext i1 X + zext i1 Y == 1 --> xor i1 X, Y
zext i1 X + zext i1 Y == 2 --> and i1 X, Y

sext i1 X + sext i1 Y == 0 --> !(or i1 X, Y)
sext i1 X + sext i1 Y == 1 --> false
sext i1 X + sext i1 Y == 2 --> false

(sext i1 X + zext i1 Y) == 0 --> !(xor i1 X, Y)
(sext i1 X + zext i1 Y) == 1 --> (!X) & Y
(sext i1 X + zext i1 Y) == 2 --> false

zext i1 X + zext i1 Y != 0 --> or i1 X, Y
zext i1 Op0 + zext i1 Op1 != 1 --> !(xor i1 Op0, Op1)
zext i1 Op0 + zext i1 Op1 != 2 --> !(and i1 Op0, Op1)

sext i1 Op0 + sext i1 Op1 != 0 --> or i1 Op0, Op1
sext i1 Op0 + sext i1 Op1 != 1 --> true
sext i1 Op0 + sext i1 Op1 != 2 --> true

sext i1 Op0 + zext i1 Op1 != 0 --> xor i1 Op0, Op1
sext i1 Op0 + zext i1 Op1 != 1 --> Op0 | (!Op1)
sext i1 Op0 + zext i1 Op1 != 2 --> true

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

2905

Add (zext i1 X + zext i1 Y) == 2 --> and i1 X, Y?

2906

Can also do:

(sext i1 X + sext i1 Y) == 0 --> !(or i1 X, Y)
(sext i1 X + sext i1 Y) == 1 --> false
(sext i1 X + sext i1 Y) == 2 --> false

(sext i1 X + zext i1 Y) == 0 --> (xor i1 X, Y)
(sext i1 X + zext i1 Y) == 1 --> Y
(sext i1 X + zext i1 Y) == 2 --> false

2909

Somewhat nit, but I'm not a fan of using X and Y here. While it fallsthrough to nullptr now, somewhat may change that not realizing its necessary for correctness as X and Y may be overwritten.

Can you change to maybe Xi1 and Yi1 or really any temporary name you want.

2919

nit: No need for braces. Personally would do:

Case 1:
if(C.isOne())
  Cond = Builder.CreateXor(...)
Case 2:
else
   Cond = Builder.CreateNot(...)
...

[arsenm]

llvm/test/Transforms/InstCombine/icmp-add.ll

17

Don’t need the flags

36

Add tests showing the multiple use rejections

[goldstein.w.n]

Can you add alive2 links?

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

2946

You can hoist this for all checks.

2956

Think would be a bit cleaner to do the following:

if(ICmp.IsEquality() && match(X, m_ZExtOrSExt(Op0)) && match(Y, m_ZExtOrSExt(Op1)) && Op0->getType->isI1() && Op1 ->getType()->isI1()) {
 if(match(X, m_Zext()) && match(Y, m_Zext()) {
...
}
if(match(X, m_SExt()) && match(Y, m_SExt()) {
...
}
if(match(X, m_ZExt()) && match(Y, m_Sext()) {
   swap(X, Y);
}
if(match(X, m_Sext() && match(Y, m_Zext()) {
....
}
}

[elhewaty]

@goldstein.w.n

// zext i1 X + zext i1 Y == 0 --> !(or i1 X, Y):
https://alive2.llvm.org/ce/z/Wfs9BS

// sext i1 X + sext i1 Y == 0 --> !(or i1 X, Y):
https://alive2.llvm.org/ce/z/ns3oDR

// zext i1 X + zext i1 Y == 1 --> xor i1 X, Y
https://alive2.llvm.org/ce/z/NvMYj9

// zext i1 X + zext i1 Y == 1 --> and i1 X, Y
https://alive2.llvm.org/ce/z/vZGCgR

// sext i1 X + sext i1 Y == 1 --> false
https://alive2.llvm.org/ce/z/UAg6BB

// sext i1 X + sext i1 Y == 2 --> false
https://alive2.llvm.org/ce/z/89xqHQ

// sext i1 X + zext i1 Y == 0 --> !(xor i1 X, Y)
https://alive2.llvm.org/ce/z/jQAkrJ

// sext i1 X + zext i1 Y == 1 --> !X & Y
https://alive2.llvm.org/ce/z/qYfXq5

// sext i1 X + zext i1 Y == 2 --> false
https://alive2.llvm.org/ce/z/cvw9VW

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

2906

(sext i1 X + zext i1 Y) == 0 --> (xor i1 X, Y)

Is this transformation correct?

https://alive2.llvm.org/ce/z/vS_fMY

I think you meant sext i1 X + zext i1 Y == 0 --> !(xor i1 X, Y)

2906

(sext i1 X + zext i1 Y) == 1 --> Y

i think this also incorrect
what about (!X) & Y

llvm/test/Transforms/InstCombine/icmp-add.ll

36

Can you please explain what do you mean by multiple use rejections?

[arsenm]

llvm/test/Transforms/InstCombine/icmp-add.ll

36

You check (m_OneUse, so should have some negative tests where there is an additional use (e.g a store of the intermediate value). Similarly you don’t have negative tests for other compare types. You could handle ne/lt/ge but should at least have tests for them even if left unhandled

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

2912

You can do here or add TODO for exanding to arbtirary C
For the most part all values but a handful are just constant simplifications.
For sext -1 and -2 are basically parallels to the zext only case.

llvm/test/Transforms/InstCombine/icmp-add.ll

133

Missing test for icmp ne (which I think you will incorrectly simplify as false when it should be true).

[elhewaty]

// zext i1 X + zext i1 Y != 0 --> or i1 X, Y
https://alive2.llvm.org/ce/z/Rg3mm7

// zext i1 Op0 + zext i1 Op1 != 1 --> !(xor i1 Op0, Op1)
https://alive2.llvm.org/ce/z/WFyF_2

// zext i1 Op0 + zext i1 Op1 != 2 --> !(and i1 Op0, Op1)
https://alive2.llvm.org/ce/z/-QgaLR

// sext i1 Op0 + sext i1 Op1 != 0 --> or i1 Op0, Op1
https://alive2.llvm.org/ce/z/oPXhLY

// sext i1 Op0 + sext i1 Op1 != 1 --> true
https://alive2.llvm.org/ce/z/xTWVYh

// sext i1 Op0 + sext i1 Op1 != 2 --> true
https://alive2.llvm.org/ce/z/GoroRZ

// sext i1 Op0 + zext i1 Op1 != 0 --> xor i1 Op0, Op1
https://alive2.llvm.org/ce/z/mN7g3V

// sext i1 Op0 + zext i1 Op1 != 1 --> Op0 | (!Op1)
https://alive2.llvm.org/ce/z/FT_8iD

// sext i1 Op0 + zext i1 Op1 != 2 --> true
https://alive2.llvm.org/ce/z/ih8K7-

llvm/test/Transforms/InstCombine/icmp-add.ll

36

Okay, I will do my best.
But I am just waiting for the code to be fully accepted, then I will try to write tests.

[arsenm]

llvm/test/Transforms/InstCombine/icmp-add.ll

36

This is backwards. It’s easier to review a patch that has full tests preconmitted so the patch diff shows the effects

[elhewaty]

Can any one review this patch

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

2912

I did and added alive2 tests. Please file a bug and assign me to it.
I will be very busy in the next few weeks. If the patch is good at this status, please land it.

llvm/test/Transforms/InstCombine/icmp-add.ll

36

so we need tests for these cases:
zext/zext lt
zext/zext gt

sext/sext lt/gt
sext/zext lt/gt

zext/zext oneuse
sext/zext oneuse
sext/onesext oneuse

133

I added tests and handled these cases in the code

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Instruction *InstCombinerImpl::foldICmpSubConstant(ICmpInst &Cmp,
   return new ICmpInst(SwappedPred, Add, ConstantInt::get(Ty, ~C));
 }
 
 /// Fold icmp (add X, Y), C.
 Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
                                                    BinaryOperator *Add,
                                                    const APInt &C) {
   Value *Y = Add->getOperand(1);
+  Value *X = Add->getOperand(0);
+
+  // We handle all (s/zext i1 X + s/zext i1 Y == 0/1/2) here.
+  if (Cmp.isEquality()) {
+    Value *Op0 = X;
+    Value *Op1 = Y;
+    // Handles zext i1 Op0, zext i1 Op1 == 0/1/2
+    if (match(Add, m_c_Add(m_OneUse(m_ZExt(m_Value(Op0))),
+                           m_OneUse(m_ZExt(m_Value(Op1))))) &&
+        Op0->getType()->isIntOrIntVectorTy(1) &&
+        Op1->getType()->isIntOrIntVectorTy(1)) {
+      Value *Cond = Builder.getFalse();
+      // Case 1: zext i1 Op0 + zext i1 Op1 == 0 --> !(or i1 Op0, Op1)
+      if (C.isZero())
+        Cond = Builder.CreateNot(Builder.CreateOr(Op0, Op1));
+      else if (C.isOne())
+        // Case 2:  zext i1 Op0 + zext i1 Op1 == 1 --> xor i1 Op0, Op1
+        Cond = Builder.CreateXor(Op0, Op1);
+      else if (C.exactLogBase2() == 1)
+        // Case 3: zext i1 Op0 + zext i1 Op1 == 2 --> xor i1 Op0, Op1
+        Cond = Builder.CreateAnd(Op0, Op1);
+      return replaceInstUsesWith(Cmp, Cond);
+    }
+
+    // Handles sext i1 Op0, sext i1 Op1 == 0/1/2
+    if (match(Add, m_c_Add(m_OneUse(m_SExt(m_Value(Op0))),
+                           m_OneUse(m_SExt(m_Value(Op1))))) &&
+        Op0->getType()->isIntOrIntVectorTy(1) &&
+        Op1->getType()->isIntOrIntVectorTy(1)) {
+      Value *Cond = Builder.getFalse();
+      // Case 1: sext i1 Op0 + sext i1 Op1 == 0 --> !(or i1 Op0, Op1)
+      if (C.isZero())
+        Cond = Builder.CreateNot(Builder.CreateOr(Op0, Op1));
+      // Case 2: sext i1 Op0 + sext i1 Op1 == 1 --> false
+      // Case 3: sext i1 Op0 + sext i1 Op2 == 2 --> false,
+      // these two cases were handled int initialization of Cond.
+      return replaceInstUsesWith(Cmp, Cond);
+    }
+
+    // Handles sext i1 Op0 + zext i1 Op1 == 0/1/2
+    if (match(Add, m_c_Add(m_OneUse(m_SExt(m_Value(Op0))),
+                           m_OneUse(m_ZExt(m_Value(Op1))))) &&
+        Op0->getType()->isIntOrIntVectorTy(1) &&
+        Op1->getType()->isIntOrIntVectorTy(1)) {
+      Value *Cond = Builder.getFalse();
+      // Case 1: sext i1 Op0 + zext i1 Op1 == 0 --> !(xor i1 Op0, Op1)
+      if (C.isZero())
+        Cond = Builder.CreateNot(Builder.CreateXor(Op0, Op1));
+      else if (C.isOne())
+        // Case 2: sext i1 Op0 + zext i1 Op1 == 1 --> (!Op0) & Op1
+        Cond = Builder.CreateAnd(Builder.CreateNot(Op0), Op1);
+      // Case 3: sext i1 Op0 + zext i1 Op1 == 2 --> false,
+      // already handled in the initilization of Cond.
+      return replaceInstUsesWith(Cmp, Cond);
+    }
+    return nullptr;
+  }
+
   const APInt *C2;
-  if (Cmp.isEquality() || !match(Y, m_APInt(C2)))
+  if (!match(Y, m_APInt(C2)))
     return nullptr;
 
   // Fold icmp pred (add X, C2), C.
-  Value *X = Add->getOperand(0);
   Type *Ty = Add->getType();
   const CmpInst::Predicate Pred = Cmp.getPredicate();
 
   // If the add does not wrap, we can always adjust the compare by subtracting
   // the constants. Equality comparisons are handled elsewhere. SGE/SLE/UGE/ULE
   // are canonicalized to SGT/SLT/UGT/ULT.
   if ((Add->hasNoSignedWrap() &&
        (Pred == ICmpInst::ICMP_SGT || Pred == ICmpInst::ICMP_SLT)) ||

llvm/test/Transforms/InstCombine/icmp-add.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
 
 declare void @use(i32)
 
 ; PR1949
 
+define i1 @cvt_icmp_0_zext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_0_zext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP0:%.*]] = or i1 [[ARG1:%.*]], [[ARG:%.*]]
+; CHECK-NEXT:    [[I4:%.*]] = xor i1 [[TMP0]], true
+; CHECK-NEXT:    ret i1 [[I4]]
+;
+bb:
+  %i = zext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i2, %i
+  %i4 = icmp eq i32 %i3, 0
+  ret i1 %i4
+}
+
+define i1 @cvt_icmp_1_zext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_1_zext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[I4:%.*]] = xor i1 [[ARG1:%.*]], [[ARG:%.*]]
+; CHECK-NEXT:    ret i1 [[I4]]
+;
+bb:
+  %i = zext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i2, %i
+  %i4 = icmp eq i32 %i3, 1
+  ret i1 %i4
+}
+
+define i1 @cvt_icmp_2_zext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_2_zext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[T:%.*]] = and i1 [[ARG:%.*]], [[ARG1:%.*]]
+; CHECK-NEXT:    ret i1 [[T]]
+;
+bb:
+  %i = zext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 2
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_0_sext_plus_sext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_0_sext_plus_sext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP0:%.*]] = or i1 [[ARG:%.*]], [[ARG1:%.*]]
+; CHECK-NEXT:    [[T:%.*]] = xor i1 [[TMP0]], true
+; CHECK-NEXT:    ret i1 [[T]]
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = sext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 0
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_1_sext_plus_sext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_1_sext_plus_sext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    ret i1 false
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = sext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 1
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_2_sext_plus_sext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_2_sext_plus_sext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    ret i1 false
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = sext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 2
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_0_sext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_0_sext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP0:%.*]] = xor i1 [[ARG:%.*]], [[ARG1:%.*]]
+; CHECK-NEXT:    [[T:%.*]] = xor i1 [[TMP0]], true
+; CHECK-NEXT:    ret i1 [[T]]
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 0
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_1_sext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_1_sext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP0:%.*]] = xor i1 [[ARG:%.*]], true
+; CHECK-NEXT:    [[T:%.*]] = and i1 [[TMP0]], [[ARG1:%.*]]
+; CHECK-NEXT:    ret i1 [[T]]
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 1
+  ret i1 %t
+}
+
+define i1 @cvt_icmp_2_sext_plus_zext(i1 %arg, i1 %arg1) {
+; CHECK-LABEL: @cvt_icmp_2_sext_plus_zext(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    ret i1 false
+;
+bb:
+  %i = sext i1 %arg to i32
+  %i2 = zext i1 %arg1 to i32
+  %i3 = add i32 %i, %i2
+  %t = icmp eq i32 %i3, 2
+  ret i1 %t
+}
+
 define i1 @test1(i32 %a) {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    [[C:%.*]] = icmp ugt i32 [[A:%.*]], -5
 ; CHECK-NEXT:    ret i1 [[C]]
 ;
   %b = add i32 %a, 4
   %c = icmp ult i32 %b, 4
   ret i1 %c