address review comments

Author: pskrgag

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

@@ -1330,16 +1330,21 @@ Instruction *InstCombinerImpl::foldNextMultiply(ICmpInst &Cmp) {
   CmpInst::Predicate Pred = Cmp.getPredicate();
   const APInt *NegConst, *MaskConst, *NumCost;
 
-  if (Pred != ICmpInst::ICMP_EQ)
+  if (!ICmpInst::isEquality(Pred))
     return nullptr;
 
   // Match num + neg
-  if (!match(Op0, m_And(m_Value(Add), m_Value(Neg))))
+  if (!match(Op0, m_c_And(m_Value(Add), m_Value(Neg))))
     return nullptr;
 
-  // Match num & mask
-  if (!match(Add, m_Add(m_Value(Num), m_Value(Mask))))
-    return nullptr;
+  // Match num & mask and handle commutative care
+  if (!match(Add, m_c_Add(m_Value(Num), m_Value(Mask)))) {
+    if (match(Neg, m_c_Add(m_Value(Num), m_Value(Mask)))) {
+      std::swap(Add, Neg);
+    } else {
+      return nullptr;
+    }
+  }
 
   // Check the constant case
   if (match(Neg, m_APInt(NegConst)) && match(Mask, m_APInt(MaskConst))) {
@@ -1355,10 +1360,14 @@ Instruction *InstCombinerImpl::foldNextMultiply(ICmpInst &Cmp) {
     if (!match(Neg, m_Sub(m_Zero(), m_Value(Value))))
       return nullptr;
 
-    // mask = %val - 1, which can be represented as sub %val, 1 or add %val, -1
-    if (!match(Mask, m_Add(m_Value(Value), m_AllOnes())) &&
-        !match(Mask, m_Sub(m_Value(Value), m_One())))
-      return nullptr;
+    // mask = add %val, -1. No commutative here, since it's canonical representation for sub %val, -1
+    if (!match(Mask, m_Add(m_Value(Value), m_AllOnes()))) {
+      if (match(Num, m_Add(m_Value(Value), m_AllOnes()))) {
+        std::swap(Mask, Num);
+      } else {
+        return nullptr;
+      }
+    }
 
     // Value should be a known power-of-two.
     if (!isKnownToBeAPowerOfTwo(Value, false, &Cmp))
@@ -1376,9 +1385,8 @@ Instruction *InstCombinerImpl::foldNextMultiply(ICmpInst &Cmp) {
   // Create new icmp eq (num & (val - 1)), 0
   auto NewAnd = Builder.CreateAnd(Num, Mask);
   auto Zero = llvm::Constant::getNullValue(Num->getType());
-  auto ICmp = Builder.CreateICmp(CmpInst::ICMP_EQ, NewAnd, Zero);
 
-  return replaceInstUsesWith(Cmp, ICmp);
+  return new ICmpInst(Pred, NewAnd, Zero);
 }
 
 /// Fold icmp Pred X, C.

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

@@ -3307,8 +3307,8 @@ define i1 @val_is_aligend_sub(i32 %num, i32 %val) {
 ; CHECK-NEXT:    [[POW:%.*]] = icmp eq i32 [[TMP1]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[POW]])
 ; CHECK-NEXT:    [[NEG:%.*]] = add i32 [[NUM]], -1
-; CHECK-NEXT:    [[_2_SROA_0_0:%.*]] = and i32 [[NUM_BIASED:%.*]], [[NEG]]
-; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[_2_SROA_0_0]], 0
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[NUM1:%.*]], [[NEG]]
+; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[TMP2]], 0
 ; CHECK-NEXT:    ret i1 [[_0]]
 ;
   %1 = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 %val)
@@ -3330,8 +3330,8 @@ define i1 @val_is_aligend_add(i32 %num, i32 %val) {
 ; CHECK-NEXT:    [[POW:%.*]] = icmp eq i32 [[TMP1]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[POW]])
 ; CHECK-NEXT:    [[NEG:%.*]] = add i32 [[NUM]], -1
-; CHECK-NEXT:    [[_2_SROA_0_0:%.*]] = and i32 [[NUM_BIASED:%.*]], [[NEG]]
-; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[_2_SROA_0_0]], 0
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[NUM1:%.*]], [[NEG]]
+; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[TMP2]], 0
 ; CHECK-NEXT:    ret i1 [[_0]]
 ;
   %1 = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 %val)
@@ -3347,6 +3347,52 @@ define i1 @val_is_aligend_add(i32 %num, i32 %val) {
   ret i1 %_0
 }
 
+define i1 @val_is_aligend_add_commute_add(i32 %num, i32 %val) {
+; CHECK-LABEL: @val_is_aligend_add_commute_add(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 [[VAL:%.*]])
+; CHECK-NEXT:    [[POW:%.*]] = icmp eq i32 [[TMP1]], 1
+; CHECK-NEXT:    call void @llvm.assume(i1 [[POW]])
+; CHECK-NEXT:    [[MASK:%.*]] = add i32 [[VAL]], -1
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[NUM:%.*]], [[MASK]]
+; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    ret i1 [[_0]]
+;
+  %1 = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 %val)
+  %pow = icmp eq i32 %1, 1
+  call void @llvm.assume(i1 %pow)
+
+  %mask = add i32 %val, -1
+  %neg = sub nsw i32 0, %val
+
+  %num.biased = add i32 %mask, %num
+  %_2.sroa.0.0 = and i32 %num.biased, %neg
+  %_0 = icmp eq i32 %_2.sroa.0.0, %num
+  ret i1 %_0
+}
+
+define i1 @val_is_aligend_add_commute_and(i32 %num, i32 %val) {
+; CHECK-LABEL: @val_is_aligend_add_commute_and(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 [[VAL:%.*]])
+; CHECK-NEXT:    [[POW:%.*]] = icmp eq i32 [[TMP1]], 1
+; CHECK-NEXT:    call void @llvm.assume(i1 [[POW]])
+; CHECK-NEXT:    [[MASK:%.*]] = add i32 [[VAL]], -1
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[NUM:%.*]], [[MASK]]
+; CHECK-NEXT:    [[_0:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    ret i1 [[_0]]
+;
+  %1 = tail call range(i32 1, 33) i32 @llvm.ctpop.i32(i32 %val)
+  %pow = icmp eq i32 %1, 1
+  call void @llvm.assume(i1 %pow)
+
+  %mask = add i32 %val, -1
+  %neg = sub nsw i32 0, %val
+
+  %num.biased = add i32 %mask, %num
+  %_2.sroa.0.0 = and i32 %neg, %num.biased
+  %_0 = icmp eq i32 %_2.sroa.0.0, %num
+  ret i1 %_0
+}
+
 define i1 @val_is_aligend_const_pow2(i32 %num) {
 ; CHECK-LABEL: @val_is_aligend_const_pow2(
 ; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[NUM:%.*]], 4095