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

[Yingwei Zheng via llvm-commits]

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

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.


>From 0019d6c1bd2fa774ef061c04e5453904d49a1a66 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Wed, 16 Oct 2024 11:50:40 +0800
Subject: [PATCH 1/2] [InstCombine] Add pre-commit tests. NFC.

---
 llvm/test/Transforms/InstCombine/and-or-icmps.ll | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/llvm/test/Transforms/InstCombine/and-or-icmps.ll b/llvm/test/Transforms/InstCombine/and-or-icmps.ll
index eb4723c86542de..87d53f9b46deba 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:%.*]] = or i1 [[CMPEQ]], [[TMP1]]
+; 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
+}

>From d0ccb329d83f2399277d926cea8743694f6ca8a2 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Wed, 16 Oct 2024 12:04:13 +0800
Subject: [PATCH 2/2] [InstCombine] Respect `samesign` flag in
 `foldAndOrOfICmpsWithConstEq`

---
 .../InstCombine/InstCombineAndOrXor.cpp       | 25 +++++++++++++------
 .../Transforms/InstCombine/and-or-icmps.ll    |  2 +-
 2 files changed, 18 insertions(+), 9 deletions(-)

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 87d53f9b46deba..a02db166acf5da 100644
--- a/llvm/test/Transforms/InstCombine/and-or-icmps.ll
+++ b/llvm/test/Transforms/InstCombine/and-or-icmps.ll
@@ -3421,7 +3421,7 @@ 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:%.*]] = or i1 [[CMPEQ]], [[TMP1]]
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[TMP1]], i1 true, i1 [[CMPEQ]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %cmp = icmp sgt i8 %x, %y