[InstCombine] Add transforms `(icmp spred (and X, Y), X)` if `X` or `Y` are known signed/unsigned #94417

goldsteinn · 2024-06-05T00:30:52Z

[InstCombine] Add transforms (icmp spred (and X, Y), X) if X or Y are known signed/unsigned

Several transforms:
1) If known Y < 0:
- slt -> ult: https://alive2.llvm.org/ce/z/9zt2iK
- sle -> ule: https://alive2.llvm.org/ce/z/SPoPNF
- sgt -> ugt: https://alive2.llvm.org/ce/z/IGNxAk
- sge -> uge: https://alive2.llvm.org/ce/z/joqTvR
2) If known Y >= 0:
- (X & PosY) s<= X --> X s>= 0
- https://alive2.llvm.org/ce/z/7e-5BQ
- (X & PosY) s> X --> X s< 0
- https://alive2.llvm.org/ce/z/jvT4Gb
3) If known X < 0:
- (NegX & Y) s> NegX --> Y s>= 0
- https://alive2.llvm.org/ce/z/ApkaEh
- (NegX & Y) s<= NegX --> Y s< 0
- https://alive2.llvm.org/ce/z/oRnfHp

llvmbot · 2024-06-05T00:31:39Z

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

[InstCombine] Make getKnownSign a member function of InstCombiner; NFC
[InstCombine] Add transforms (icmp spred (and X, Y), X) if X or Y are known signed/unsigned

Several transforms:
1) If known Y < 0:
- slt -> ult: https://alive2.llvm.org/ce/z/9zt2iK
- sle -> ule: https://alive2.llvm.org/ce/z/SPoPNF
- sgt -> ugt: https://alive2.llvm.org/ce/z/IGNxAk
- sge -> uge: https://alive2.llvm.org/ce/z/joqTvR
2) If known Y >= 0:
- (X & PosY) s> X --> X s< 0
- https://alive2.llvm.org/ce/z/7e-5BQ
- (X & PosY) s> X --> X s< 0
- https://alive2.llvm.org/ce/z/jvT4Gb
3) If known X < 0:
- (NegX & Y) s> NegX --> Y s>= 0
- https://alive2.llvm.org/ce/z/ApkaEh
- (NegX & Y) s<= NegX --> Y s< 0
- https://alive2.llvm.org/ce/z/oRnfHp

Patch is 23.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/94417.diff

9 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+12-18)
(modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+33)
(modified) llvm/lib/Transforms/InstCombine/InstCombineInternal.h (+7)
(modified) llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll (+1-2)
(modified) llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sgt-to-icmp-sgt.ll (+1-2)
(modified) llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll (+1-2)
(modified) llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll (+1-2)
(modified) llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll (+34-38)
(modified) llvm/test/Transforms/InstCombine/icmp-of-and-x.ll (+13-25)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index b6f339da31f7f..3f156cad7b0da 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1041,10 +1041,9 @@ Instruction *InstCombinerImpl::foldIntrinsicIsFPClass(IntrinsicInst &II) {
   return nullptr;
 }
 
-static std::optional<bool> getKnownSign(Value *Op, Instruction *CxtI,
-                                   const DataLayout &DL, AssumptionCache *AC,
-                                   DominatorTree *DT) {
-  KnownBits Known = computeKnownBits(Op, DL, 0, AC, CxtI, DT);
+std::optional<bool> InstCombinerImpl::getKnownSign(Value *Op,
+                                                   Instruction *CxtI) const {
+  KnownBits Known = llvm::computeKnownBits(Op, DL, /*Depth*/ 0, &AC, CxtI, &DT);
   if (Known.isNonNegative())
     return false;
   if (Known.isNegative())
@@ -1058,13 +1057,10 @@ static std::optional<bool> getKnownSign(Value *Op, Instruction *CxtI,
       ICmpInst::ICMP_SLT, Op, Constant::getNullValue(Op->getType()), CxtI, DL);
 }
 
-static std::optional<bool> getKnownSignOrZero(Value *Op, Instruction *CxtI,
-                                              const DataLayout &DL,
-                                              AssumptionCache *AC,
-                                              DominatorTree *DT) {
-  if (std::optional<bool> Sign = getKnownSign(Op, CxtI, DL, AC, DT))
+std::optional<bool>
+InstCombinerImpl::getKnownSignOrZero(Value *Op, Instruction *CxtI) const {
+  if (std::optional<bool> Sign = getKnownSign(Op, CxtI))
     return Sign;
-
   Value *X, *Y;
   if (match(Op, m_NSWSub(m_Value(X), m_Value(Y))))
     return isImpliedByDomCondition(ICmpInst::ICMP_SLE, X, Y, CxtI, DL);
@@ -1074,12 +1070,11 @@ static std::optional<bool> getKnownSignOrZero(Value *Op, Instruction *CxtI,
 
 /// Return true if two values \p Op0 and \p Op1 are known to have the same sign.
 static bool signBitMustBeTheSame(Value *Op0, Value *Op1, Instruction *CxtI,
-                                 const DataLayout &DL, AssumptionCache *AC,
-                                 DominatorTree *DT) {
-  std::optional<bool> Known1 = getKnownSign(Op1, CxtI, DL, AC, DT);
+                                 InstCombinerImpl &IC) {
+  std::optional<bool> Known1 = IC.getKnownSign(Op1, CxtI);
   if (!Known1)
     return false;
-  std::optional<bool> Known0 = getKnownSign(Op0, CxtI, DL, AC, DT);
+  std::optional<bool> Known0 = IC.getKnownSign(Op0, CxtI);
   if (!Known0)
     return false;
   return *Known0 == *Known1;
@@ -1627,8 +1622,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
       return replaceOperand(*II, 0, XY);
     }
 
-    if (std::optional<bool> Known =
-            getKnownSignOrZero(IIOperand, II, DL, &AC, &DT)) {
+    if (std::optional<bool> Known = getKnownSignOrZero(IIOperand, II)) {
       // abs(x) -> x if x >= 0 (include abs(x-y) --> x - y where x >= y)
       // abs(x) -> x if x > 0 (include abs(x-y) --> x - y where x > y)
       if (!*Known)
@@ -1753,7 +1747,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
       bool UseAndN = IID == Intrinsic::smin || IID == Intrinsic::umin;
 
       if (IID == Intrinsic::smax || IID == Intrinsic::smin) {
-        auto KnownSign = getKnownSign(X, II, DL, &AC, &DT);
+        auto KnownSign = getKnownSign(X, II);
         if (KnownSign == std::nullopt) {
           UseOr = false;
           UseAndN = false;
@@ -2614,7 +2608,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
       FastMathFlags InnerFlags = cast<FPMathOperator>(Src)->getFastMathFlags();
 
       if ((FMF.allowReassoc() && InnerFlags.allowReassoc()) ||
-          signBitMustBeTheSame(Exp, InnerExp, II, DL, &AC, &DT)) {
+          signBitMustBeTheSame(Exp, InnerExp, II, *this)) {
         // TODO: Add nsw/nuw probably safe if integer type exceeds exponent
         // width.
         Value *NewExp = Builder.CreateAdd(InnerExp, Exp);
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 89193f8ff94b6..d29f16d63b05b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -4745,6 +4745,39 @@ static Instruction *foldICmpAndXX(ICmpInst &I, const SimplifyQuery &Q,
                           Constant::getNullValue(Op1->getType()));
   }
 
+  if (!ICmpInst::isSigned(Pred))
+    return nullptr;
+
+  // (X & NegY) spred X --> (X & NegY) upred X
+  if (IC.getKnownSign(A, &I).value_or(false))
+    return new ICmpInst(ICmpInst::getUnsignedPredicate(Pred), Op0, Op1);
+
+  if (Pred != ICmpInst::ICMP_SLE && Pred != ICmpInst::ICMP_SGT)
+    return nullptr;
+
+  if (!IC.getKnownSignOrZero(A, &I).value_or(true)) {
+    // (X & PosY) s<= X --> X s>= 0
+    if (Pred == ICmpInst::ICMP_SLE)
+      return new ICmpInst(ICmpInst::ICMP_SGE, Op1,
+                          Constant::getNullValue(Op1->getType()));
+    // (X & PosY) s> X --> X s< 0
+    if (Pred == ICmpInst::ICMP_SGT)
+      return new ICmpInst(ICmpInst::ICMP_SLT, Op1,
+                          Constant::getNullValue(Op1->getType()));
+  }
+
+  if (IC.getKnownSign(Op1, &I).value_or(false)) {
+    // (NegX & Y) s> NegX --> Y s>= 0
+    if (Pred == ICmpInst::ICMP_SGT)
+      return new ICmpInst(ICmpInst::ICMP_SGE, A,
+                          Constant::getNullValue(A->getType()));
+
+    // (NegX & Y) s<= NegX --> Y s< 0
+    if (Pred == ICmpInst::ICMP_SLE)
+      return new ICmpInst(ICmpInst::ICMP_SLT, A,
+                          Constant::getNullValue(A->getType()));
+  }
+
   return nullptr;
 }
 
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 984f02bcccad7..bbd8fc5bf8fe5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -491,6 +491,13 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
       Instruction::BinaryOps BinaryOp, bool IsSigned,
       Value *LHS, Value *RHS, Instruction *CxtI) const;
 
+  // Return true if known negative, false if known positive, and nullopt if
+  // unknown.
+  std::optional<bool> getKnownSign(Value *Op, Instruction *CxtI) const;
+  // Return true if known negative or zero, false if known non-zero positive,
+  // and nullopt if unknown.
+  std::optional<bool> getKnownSignOrZero(Value *Op, Instruction *CxtI) const;
+
   /// Performs a few simplifications for operators which are associative
   /// or commutative.
   bool SimplifyAssociativeOrCommutative(BinaryOperator &I);
diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll
index ae503bfb1cfe2..e103fe9440986 100644
--- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll
+++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll
@@ -98,8 +98,7 @@ declare i8 @gen8()
 define i1 @c0() {
 ; CHECK-LABEL: @c0(
 ; CHECK-NEXT:    [[X:%.*]] = call i8 @gen8()
-; CHECK-NEXT:    [[TMP0:%.*]] = and i8 [[X]], 3
-; CHECK-NEXT:    [[RET:%.*]] = icmp sge i8 [[X]], [[TMP0]]
+; CHECK-NEXT:    [[RET:%.*]] = icmp sgt i8 [[X]], -1
 ; CHECK-NEXT:    ret i1 [[RET]]
 ;
   %x = call i8 @gen8()
diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sgt-to-icmp-sgt.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sgt-to-icmp-sgt.ll
index d1dd411ee86b3..bbd733e86a32d 100644
--- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sgt-to-icmp-sgt.ll
+++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sgt-to-icmp-sgt.ll
@@ -125,8 +125,7 @@ define i1 @oneuse0() {
 
 define i1 @c0(i8 %x) {
 ; CHECK-LABEL: @c0(
-; CHECK-NEXT:    [[TMP0:%.*]] = and i8 [[X:%.*]], 3
-; CHECK-NEXT:    [[RET:%.*]] = icmp sgt i8 [[TMP0]], [[X]]
+; CHECK-NEXT:    [[RET:%.*]] = icmp slt i8 [[X:%.*]], 0
 ; CHECK-NEXT:    ret i1 [[RET]]
 ;
   %tmp0 = and i8 %x, 3
diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll
index 4bed21a525f05..b167c8ad25aa9 100644
--- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll
+++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll
@@ -113,8 +113,7 @@ define i1 @oneuse0() {
 
 define i1 @c0(i8 %x) {
 ; CHECK-LABEL: @c0(
-; CHECK-NEXT:    [[TMP0:%.*]] = and i8 [[X:%.*]], 3
-; CHECK-NEXT:    [[RET:%.*]] = icmp sle i8 [[TMP0]], [[X]]
+; CHECK-NEXT:    [[RET:%.*]] = icmp sgt i8 [[X:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[RET]]
 ;
   %tmp0 = and i8 %x, 3
diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll
index 8415204a4915a..8281502447732 100644
--- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll
+++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-slt-to-icmp-sgt.ll
@@ -108,8 +108,7 @@ declare i8 @gen8()
 define i1 @c0() {
 ; CHECK-LABEL: @c0(
 ; CHECK-NEXT:    [[X:%.*]] = call i8 @gen8()
-; CHECK-NEXT:    [[TMP0:%.*]] = and i8 [[X]], 3
-; CHECK-NEXT:    [[RET:%.*]] = icmp slt i8 [[X]], [[TMP0]]
+; CHECK-NEXT:    [[RET:%.*]] = icmp slt i8 [[X]], 0
 ; CHECK-NEXT:    ret i1 [[RET]]
 ;
   %x = call i8 @gen8()
diff --git a/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll b/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
index 8bb7fd0e522cb..0aace5f52c96c 100644
--- a/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
@@ -7,8 +7,8 @@ define i1 @src_is_mask_zext(i16 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_mask_zext(
 ; CHECK-NEXT:    [[M_IN:%.*]] = lshr i8 -1, [[Y:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = zext i8 [[M_IN]] to i16
-; CHECK-NEXT:    [[X:%.*]] = xor i16 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i16 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i16 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i16 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i16 %x_in, 123
@@ -83,8 +83,8 @@ define i1 @src_is_mask_and(i8 %x_in, i8 %y, i8 %z) {
 ; CHECK-NEXT:    [[MY:%.*]] = lshr i8 7, [[Y:%.*]]
 ; CHECK-NEXT:    [[MZ:%.*]] = lshr i8 -1, [[Z:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = and i8 [[MY]], [[MZ]]
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -121,8 +121,8 @@ define i1 @src_is_mask_or(i8 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_mask_or(
 ; CHECK-NEXT:    [[MY:%.*]] = lshr i8 -1, [[Y:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = and i8 [[MY]], 7
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -138,8 +138,8 @@ define i1 @src_is_mask_xor(i8 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_mask_xor(
 ; CHECK-NEXT:    [[Y_M1:%.*]] = add i8 [[Y:%.*]], -1
 ; CHECK-NEXT:    [[MASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -173,8 +173,8 @@ define i1 @src_is_mask_select(i8 %x_in, i8 %y, i1 %cond) {
 ; CHECK-NEXT:    [[Y_M1:%.*]] = add i8 [[Y:%.*]], -1
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[MASK:%.*]] = select i1 [[COND:%.*]], i8 [[YMASK]], i8 15
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -249,8 +249,8 @@ define i1 @src_is_mask_lshr(i8 %x_in, i8 %y, i8 %z, i1 %cond) {
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[SMASK:%.*]] = select i1 [[COND:%.*]], i8 [[YMASK]], i8 15
 ; CHECK-NEXT:    [[MASK:%.*]] = lshr i8 [[SMASK]], [[Z:%.*]]
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -269,8 +269,8 @@ define i1 @src_is_mask_ashr(i8 %x_in, i8 %y, i8 %z, i1 %cond) {
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[SMASK:%.*]] = select i1 [[COND:%.*]], i8 [[YMASK]], i8 15
 ; CHECK-NEXT:    [[MASK:%.*]] = ashr i8 [[SMASK]], [[Z:%.*]]
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -287,8 +287,8 @@ define i1 @src_is_mask_p2_m1(i8 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_mask_p2_m1(
 ; CHECK-NEXT:    [[P2ORZ:%.*]] = shl i8 2, [[Y:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = add i8 [[P2ORZ]], -1
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -304,8 +304,8 @@ define i1 @src_is_mask_umax(i8 %x_in, i8 %y) {
 ; CHECK-NEXT:    [[Y_M1:%.*]] = add i8 [[Y:%.*]], -1
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[MASK:%.*]] = call i8 @llvm.umax.i8(i8 [[YMASK]], i8 3)
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -324,8 +324,8 @@ define i1 @src_is_mask_umin(i8 %x_in, i8 %y, i8 %z) {
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[ZMASK:%.*]] = lshr i8 15, [[Z:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = call i8 @llvm.umin.i8(i8 [[YMASK]], i8 [[ZMASK]])
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -364,8 +364,8 @@ define i1 @src_is_mask_smax(i8 %x_in, i8 %y) {
 ; CHECK-NEXT:    [[Y_M1:%.*]] = add i8 [[Y:%.*]], -1
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[MASK:%.*]] = call i8 @llvm.smax.i8(i8 [[YMASK]], i8 -1)
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -383,8 +383,8 @@ define i1 @src_is_mask_smin(i8 %x_in, i8 %y) {
 ; CHECK-NEXT:    [[Y_M1:%.*]] = add i8 [[Y:%.*]], -1
 ; CHECK-NEXT:    [[YMASK:%.*]] = xor i8 [[Y_M1]], [[Y]]
 ; CHECK-NEXT:    [[MASK:%.*]] = call i8 @llvm.smin.i8(i8 [[YMASK]], i8 0)
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -401,8 +401,8 @@ define i1 @src_is_mask_bitreverse_not_mask(i8 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_mask_bitreverse_not_mask(
 ; CHECK-NEXT:    [[NMASK:%.*]] = shl nsw i8 -1, [[Y:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = call i8 @llvm.bitreverse.i8(i8 [[NMASK]])
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[MASK]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], 123
+; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[TMP1]], [[MASK]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -455,9 +455,9 @@ define i1 @src_is_notmask_shl(i8 %x_in, i8 %y, i1 %cond) {
 define i1 @src_is_notmask_x_xor_neg_x(i8 %x_in, i8 %y, i1 %cond) {
 ; CHECK-LABEL: @src_is_notmask_x_xor_neg_x(
 ; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[NEG_Y:%.*]] = add i8 [[Y:%.*]], -1
-; CHECK-NEXT:    [[NOTMASK0:%.*]] = xor i8 [[NEG_Y]], [[Y]]
-; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[COND:%.*]], i8 [[NOTMASK0]], i8 7
+; CHECK-NEXT:    [[TMP1:%.*]] = add i8 [[Y:%.*]], -1
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i8 [[TMP1]], [[Y]]
+; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[COND:%.*]], i8 [[TMP2]], i8 7
 ; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[TMP3]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
@@ -473,9 +473,9 @@ define i1 @src_is_notmask_x_xor_neg_x(i8 %x_in, i8 %y, i1 %cond) {
 define i1 @src_is_notmask_x_xor_neg_x_inv(i8 %x_in, i8 %y, i1 %cond) {
 ; CHECK-LABEL: @src_is_notmask_x_xor_neg_x_inv(
 ; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[NEG_Y:%.*]] = add i8 [[Y:%.*]], -1
-; CHECK-NEXT:    [[NOTMASK0:%.*]] = xor i8 [[NEG_Y]], [[Y]]
-; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[COND:%.*]], i8 [[NOTMASK0]], i8 7
+; CHECK-NEXT:    [[TMP1:%.*]] = add i8 [[Y:%.*]], -1
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i8 [[TMP1]], [[Y]]
+; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[COND:%.*]], i8 [[TMP2]], i8 7
 ; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[TMP3]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
@@ -625,9 +625,7 @@ define i1 @src_is_notmask_xor_fail(i8 %x_in, i8 %y) {
 
 define i1 @src_is_mask_const_slt(i8 %x_in) {
 ; CHECK-LABEL: @src_is_mask_const_slt(
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], 7
-; CHECK-NEXT:    [[R:%.*]] = icmp slt i8 [[X]], [[AND]]
+; CHECK-NEXT:    [[R:%.*]] = icmp slt i8 [[X_IN:%.*]], 0
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -650,9 +648,7 @@ define i1 @src_is_mask_const_sgt(i8 %x_in) {
 
 define i1 @src_is_mask_const_sle(i8 %x_in) {
 ; CHECK-LABEL: @src_is_mask_const_sle(
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], 31
-; CHECK-NEXT:    [[R:%.*]] = icmp sle i8 [[AND]], [[X]]
+; CHECK-NEXT:    [[R:%.*]] = icmp sgt i8 [[X_IN:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
diff --git a/llvm/test/Transforms/InstCombine/icmp-of-and-x.ll b/llvm/test/Transforms/InstCombine/icmp-of-and-x.ll
index 0f26be12c39cc..75badabda01ae 100644
--- a/llvm/test/Transforms/InstCombine/icmp-of-and-x.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-of-and-x.ll
@@ -58,7 +58,7 @@ define i1 @icmp_sge_x_negy(i8 %x, i8 %y) {
 ; CHECK-NEXT:    [[CY:%.*]] = icmp slt i8 [[Y:%.*]], 0
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[CY]])
 ; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X:%.*]], [[Y]]
-; CHECK-NEXT:    [[Z:%.*]] = icmp sge i8 [[AND]], [[X]]
+; CHECK-NEXT:    [[Z:%.*]] = icmp eq i8 [[AND]], [[X]]
 ; CHECK-NEXT:    ret i1 [[Z]]
 ;
   %cy = icmp slt i8 %y, 0
@@ -74,7 +74,7 @@ define i1 @icmp_slt_x_negy(i8 %x, i8 %y) {
 ; CHECK-NEXT:    br i1 [[CY]], label [[NEGY:%.*]], label [[POSY:%.*]]
 ; CHECK:       negy:
 ; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X:%.*]], [[Y]]
-; CHECK-NEXT:    [[Z:%.*]] = icmp slt i8 [[AND]], [[X]]
+; CHECK-NEXT:    [[Z:%.*]] = icmp ne i8 [[AND]], [[X]]
 ; ...
[truncated]

nikic

Use isKnownNonNegative() / isKnownNegative() instead of getKnownSign(). Only use getKnownSign() if you need to handle both the negative and the non-negative case.

The special case that getKnownSign() implements is intended for llvm.abs only, and should not be taken as justification to use this API in any other circumstance.

dtcxzyw · 2024-06-05T16:54:44Z

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

+    if (Pred == ICmpInst::ICMP_SLE)
+      return new ICmpInst(ICmpInst::ICMP_SGE, Op1,
+                          Constant::getNullValue(Op1->getType()));
+    // (X & PosY) s> X --> X s< 0
+    if (Pred == ICmpInst::ICMP_SGT)
+      return new ICmpInst(ICmpInst::ICMP_SLT, Op1,
+                          Constant::getNullValue(Op1->getType()));


Suggested change

if (Pred == ICmpInst::ICMP_SLE)

return new ICmpInst(ICmpInst::ICMP_SGE, Op1,

Constant::getNullValue(Op1->getType()));

// (X & PosY) s> X --> X s< 0

if (Pred == ICmpInst::ICMP_SGT)

return new ICmpInst(ICmpInst::ICMP_SLT, Op1,

Constant::getNullValue(Op1->getType()));

// (X & PosY) s> X --> X s< 0

return new ICmpInst(Pred == ICmpInst::ICMP_SLE ? ICmpInst::ICMP_SGE : ICmpInst::ICMP_SLT, Op1,

Constant::getNullValue(Op1->getType()));

or

Suggested change

if (Pred == ICmpInst::ICMP_SLE)

return new ICmpInst(ICmpInst::ICMP_SGE, Op1,

Constant::getNullValue(Op1->getType()));

// (X & PosY) s> X --> X s< 0

if (Pred == ICmpInst::ICMP_SGT)

return new ICmpInst(ICmpInst::ICMP_SLT, Op1,

Constant::getNullValue(Op1->getType()));

// (X & PosY) s> X --> X s< 0

return new ICmpInst(ICmpInst::getSwappedPredicate(Pred), Op1,

Constant::getNullValue(Op1->getType()));

It would be better to canonicalize X s>= 0 -> X s> -1 here.

Personally prefer to leave canonicalization up to the designed folds. Otherwise think you tend to further complicate the code/dup the code/make it harder to change these things.

It's good to directly emit canonical IR when convenient, but I agree that we shouldn't go out of our way when it would require extra code.

PR Link: llvm/llvm-project#94417

dtcxzyw · 2024-06-05T17:58:19Z

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

+
+  KnownBits KnownY = IC.computeKnownBits(A, /*Depth=*/0, &I);
+  // (X & NegY) spred X --> (X & NegY) upred X
+  if (KnownY.isNegative())


Can you double check that all cases are covered by existing tests?

It is tests in Transforms/InstCombine/icmp-of-and-x.ll.

Without it we get the following regressions:

KnownBits KnownY = IC.computeKnownBits(A, /*Depth=*/0, &I); // (X & NegY) spred X --> (X & NegY) upred X - if (KnownY.isNegative()) + if (KnownY.isNegative() && 0) return new ICmpInst(ICmpInst::getUnsignedPredicate(Pred), Op0, Op1); if (Pred != ICmpInst::ICMP_SLE && Pred != ICmpInst::ICMP_SGT) modified llvm/test/Transforms/InstCombine/icmp-of-and-x.ll @@ -58,7 +58,7 @@ define i1 @icmp_sge_x_negy(i8 %x, i8 %y) { ; CHECK-NEXT: [[CY:%.*]] = icmp slt i8 [[Y:%.*]], 0 ; CHECK-NEXT: call void @llvm.assume(i1 [[CY]]) ; CHECK-NEXT: [[AND:%.*]] = and i8 [[X:%.*]], [[Y]] -; CHECK-NEXT: [[Z:%.*]] = icmp eq i8 [[AND]], [[X]] +; CHECK-NEXT: [[Z:%.*]] = icmp sge i8 [[AND]], [[X]] ; CHECK-NEXT: ret i1 [[Z]] ; %cy = icmp slt i8 %y, 0 @@ -74,7 +74,7 @@ define i1 @icmp_slt_x_negy(i8 %x, i8 %y) { ; CHECK-NEXT: br i1 [[CY]], label [[NEGY:%.*]], label [[POSY:%.*]] ; CHECK: negy: ; CHECK-NEXT: [[AND:%.*]] = and i8 [[X:%.*]], [[Y]] -; CHECK-NEXT: [[Z:%.*]] = icmp ne i8 [[AND]], [[X]] +; CHECK-NEXT: [[Z:%.*]] = icmp slt i8 [[AND]], [[X]] ; CHECK-NEXT: ret i1 [[Z]] ; CHECK: posy: ; CHECK-NEXT: [[R:%.*]] = call i1 @barrier() @@ -116,7 +116,10 @@ posy: define i1 @icmp_sle_x_negy(i8 %x, i8 %yy) { ; CHECK-LABEL: @icmp_sle_x_negy( -; CHECK-NEXT: ret i1 true +; CHECK-NEXT: [[Y:%.*]] = or i8 [[YY:%.*]], -128 +; CHECK-NEXT: [[AND:%.*]] = and i8 [[Y]], [[X:%.*]] +; CHECK-NEXT: [[Z:%.*]] = icmp sle i8 [[AND]], [[X]] +; CHECK-NEXT: ret i1 [[Z]] ; %y = or i8 %yy, 128 %and = and i8 %y, %x @@ -126,7 +129,10 @@ define i1 @icmp_sle_x_negy(i8 %x, i8 %yy) { define <2 x i1> @icmp_sgt_x_negy(<2 x i8> %x, <2 x i8> %yy) { ; CHECK-LABEL: @icmp_sgt_x_negy( -; CHECK-NEXT: ret <2 x i1> zeroinitializer +; CHECK-NEXT: [[Y:%.*]] = or <2 x i8> [[YY:%.*]], <i8 -128, i8 -128> +; CHECK-NEXT: [[AND:%.*]] = and <2 x i8> [[Y]], [[X:%.*]] +; CHECK-NEXT: [[Z:%.*]] = icmp sgt <2 x i8> [[AND]], [[X]] +; CHECK-NEXT: ret <2 x i1> [[Z]] ; %y = or <2 x i8> %yy, <i8 128, i8 128> %and = and <2 x i8> %y, %x

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

…Y` are known signed/unsigned Several transforms: 1) If known `Y < 0`: - slt -> ult: https://alive2.llvm.org/ce/z/9zt2iK - sle -> ule: https://alive2.llvm.org/ce/z/SPoPNF - sgt -> ugt: https://alive2.llvm.org/ce/z/IGNxAk - sge -> uge: https://alive2.llvm.org/ce/z/joqTvR 2) If known `Y >= 0`: - `(X & PosY) s> X --> X s< 0` - https://alive2.llvm.org/ce/z/7e-5BQ - `(X & PosY) s> X --> X s< 0` - https://alive2.llvm.org/ce/z/jvT4Gb 3) If known `X < 0`: - `(NegX & Y) s> NegX --> Y s>= 0` - https://alive2.llvm.org/ce/z/ApkaEh - `(NegX & Y) s<= NegX --> Y s< 0` - https://alive2.llvm.org/ce/z/oRnfHp

nikic

LGTM

goldsteinn requested a review from nikic as a code owner June 5, 2024 00:30

llvmbot added the llvm:transforms label Jun 5, 2024

goldsteinn requested a review from dtcxzyw June 5, 2024 00:31

goldsteinn changed the title ~~goldsteinn/cmp and spred~~ [InstCombine] Add transforms (icmp spred (and X, Y), X) if X or Y are known signed/unsigned Jun 5, 2024

nikic requested changes Jun 5, 2024

View reviewed changes

goldsteinn force-pushed the goldsteinn/cmp-and-spred branch from cd73851 to 611d1e4 Compare June 5, 2024 15:57

dtcxzyw reviewed Jun 5, 2024

View reviewed changes

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jun 5, 2024

pre-commit: test PR94417

e8d618a

PR Link: llvm/llvm-project#94417

dtcxzyw mentioned this pull request Jun 5, 2024

pre-commit: test PR94417 dtcxzyw/llvm-opt-benchmark#661

Closed

dtcxzyw reviewed Jun 5, 2024

View reviewed changes

goldsteinn force-pushed the goldsteinn/cmp-and-spred branch from 611d1e4 to b05dce7 Compare June 5, 2024 20:15

nikic reviewed Jun 5, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

goldsteinn force-pushed the goldsteinn/cmp-and-spred branch from b05dce7 to 9d8d267 Compare June 5, 2024 21:33

nikic approved these changes Jun 6, 2024

View reviewed changes

goldsteinn closed this in c3c443b Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstCombine] Add transforms `(icmp spred (and X, Y), X)` if `X` or `Y` are known signed/unsigned #94417

[InstCombine] Add transforms `(icmp spred (and X, Y), X)` if `X` or `Y` are known signed/unsigned #94417

goldsteinn commented Jun 5, 2024 •

edited by dtcxzyw

Loading

llvmbot commented Jun 5, 2024

nikic left a comment •

edited

Loading

dtcxzyw Jun 5, 2024

dtcxzyw Jun 5, 2024

goldsteinn Jun 5, 2024

nikic Jun 6, 2024

dtcxzyw Jun 5, 2024

goldsteinn Jun 5, 2024

nikic left a comment

[InstCombine] Add transforms (icmp spred (and X, Y), X) if X or Y are known signed/unsigned #94417

[InstCombine] Add transforms (icmp spred (and X, Y), X) if X or Y are known signed/unsigned #94417

Conversation

goldsteinn commented Jun 5, 2024 • edited by dtcxzyw Loading

llvmbot commented Jun 5, 2024

nikic left a comment • edited Loading

Choose a reason for hiding this comment

dtcxzyw Jun 5, 2024

Choose a reason for hiding this comment

dtcxzyw Jun 5, 2024

Choose a reason for hiding this comment

goldsteinn Jun 5, 2024

Choose a reason for hiding this comment

nikic Jun 6, 2024

Choose a reason for hiding this comment

dtcxzyw Jun 5, 2024

Choose a reason for hiding this comment

goldsteinn Jun 5, 2024

Choose a reason for hiding this comment

nikic left a comment

Choose a reason for hiding this comment

[InstCombine] Add transforms `(icmp spred (and X, Y), X)` if `X` or `Y` are known signed/unsigned #94417

[InstCombine] Add transforms `(icmp spred (and X, Y), X)` if `X` or `Y` are known signed/unsigned #94417

goldsteinn commented Jun 5, 2024 •

edited by dtcxzyw

Loading

nikic left a comment •

edited

Loading