[llvm] [InstCombine] Respect `samesign` flag in `foldAndOrOfICmpsWithConstEq` (PR #112475)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

<details>
<summary>Changes</summary>

In https://github.com/llvm/llvm-project/commit/5dbfca30c1a672cd0c5089df2b4fdd171436643a we assume that RHS is poison implies LHS is also poison. It doesn't hold after introducing `samesign` flag. This patch treats this case as logical and swap operands iff `RHS` has `samesign` flag and the original expression is a logical and/or.

Alternative solution 1: just drop `samesign` flag in RHS after folding. 
Alternative solution 2: reject this fold if RHS contains `samesign` flag.

Close https://github.com/llvm/llvm-project/issues/112467.


---
Full diff: https://github.com/llvm/llvm-project/pull/112475.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+17-8) 
- (modified) llvm/test/Transforms/InstCombine/and-or-icmps.ll (+13) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 64bee4ab974ede..22728bc8abda46 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -1261,7 +1261,8 @@ Value *InstCombinerImpl::foldEqOfParts(ICmpInst *Cmp0, ICmpInst *Cmp1,
 static Value *foldAndOrOfICmpsWithConstEq(ICmpInst *Cmp0, ICmpInst *Cmp1,
                                           bool IsAnd, bool IsLogical,
                                           InstCombiner::BuilderTy &Builder,
-                                          const SimplifyQuery &Q) {
+                                          const SimplifyQuery &Q,
+                                          bool SwapOpOrder) {
   // Match an equality compare with a non-poison constant as Cmp0.
   // Also, give up if the compare can be constant-folded to avoid looping.
   ICmpInst::Predicate Pred0;
@@ -1295,9 +1296,14 @@ static Value *foldAndOrOfICmpsWithConstEq(ICmpInst *Cmp0, ICmpInst *Cmp1,
       return nullptr;
     SubstituteCmp = Builder.CreateICmp(Pred1, Y, C);
   }
-  if (IsLogical)
-    return IsAnd ? Builder.CreateLogicalAnd(Cmp0, SubstituteCmp)
-                 : Builder.CreateLogicalOr(Cmp0, SubstituteCmp);
+  if (IsLogical) {
+    Value *LHS = Cmp0;
+    Value *RHS = SubstituteCmp;
+    if (SwapOpOrder)
+      std::swap(LHS, RHS);
+    return IsAnd ? Builder.CreateLogicalAnd(LHS, RHS)
+                 : Builder.CreateLogicalOr(LHS, RHS);
+  }
   return Builder.CreateBinOp(IsAnd ? Instruction::And : Instruction::Or, Cmp0,
                              SubstituteCmp);
 }
@@ -3363,13 +3369,16 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
                                                   /*IsLogical*/ false, Builder))
     return V;
 
-  if (Value *V =
-          foldAndOrOfICmpsWithConstEq(LHS, RHS, IsAnd, IsLogical, Builder, Q))
+  if (Value *V = foldAndOrOfICmpsWithConstEq(LHS, RHS, IsAnd, IsLogical,
+                                             Builder, Q, /*SwapOpOrder=*/false))
     return V;
   // We can convert this case to bitwise and, because both operands are used
-  // on the LHS, and as such poison from both will propagate.
+  // on the LHS, and as such poison from both will propagate (unless RHS has
+  // samesign flag).
   if (Value *V = foldAndOrOfICmpsWithConstEq(RHS, LHS, IsAnd,
-                                             /*IsLogical*/ false, Builder, Q))
+                                             /*IsLogical=*/IsLogical &&
+                                                 RHS->hasSameSign(),
+                                             Builder, Q, /*SwapOpOrder=*/true))
     return V;
 
   if (Value *V = foldIsPowerOf2OrZero(LHS, RHS, IsAnd, Builder, *this))
diff --git a/llvm/test/Transforms/InstCombine/and-or-icmps.ll b/llvm/test/Transforms/InstCombine/and-or-icmps.ll
index eb4723c86542de..a02db166acf5da 100644
--- a/llvm/test/Transforms/InstCombine/and-or-icmps.ll
+++ b/llvm/test/Transforms/InstCombine/and-or-icmps.ll
@@ -3416,3 +3416,16 @@ define i1 @and_ugt_to_mask_off_by_one(i8 %x) {
   %and2 = and i1 %cmp, %cmp2
   ret i1 %and2
 }
+
+define i1 @logical_or_icmp_eq_const_samesign(i8 %x, i8 %y) {
+; CHECK-LABEL: @logical_or_icmp_eq_const_samesign(
+; CHECK-NEXT:    [[CMPEQ:%.*]] = icmp samesign ne i8 [[X:%.*]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i8 [[Y:%.*]], 0
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[TMP1]], i1 true, i1 [[CMPEQ]]
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %cmp = icmp sgt i8 %x, %y
+  %cmpeq = icmp samesign ne i8 %x, 0
+  %r = select i1 %cmp, i1 true, i1 %cmpeq
+  ret i1 %r
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/112475