[InstCombine] Drop samesign flag in foldAndOrOfICmpsWithConstEq

[dtcxzyw]

In 5dbfca3 we assume that RHS is poison implies LHS is also poison. It doesn't hold after introducing samesign flag.

This patch drops the samesign flag on RHS if the original expression is a logical and/or.

It is an alternative to #112475.
Closes #112467.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

In 5dbfca3 we assume that RHS is poison implies LHS is also poison. It doesn't hold after introducing samesign flag.

This patch drops the samesign flag on RHS if the original expression is a logical and/or.

It is an alternative to #112475.
Closes #112467.

---

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

3 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+7-1)
• (modified) llvm/test/Transforms/InstCombine/and-or-icmp-min-max.ll (+11)
• (modified) llvm/test/Transforms/InstCombine/and-or-icmp-nullptr.ll (+13)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 64bee4ab974ede..c8407e8ba5aba8 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3369,8 +3369,14 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
   // We can convert this case to bitwise and, because both operands are used
   // on the LHS, and as such poison from both will propagate.
   if (Value *V = foldAndOrOfICmpsWithConstEq(RHS, LHS, IsAnd,
-                                             /*IsLogical*/ false, Builder, Q))
+                                             /*IsLogical=*/false, Builder, Q)) {
+    // If RHS is still used, we should drop samesign flag.
+    if (IsLogical && RHS->hasSameSign() && !RHS->use_empty()) {
+      RHS->setSameSign(false);
+      addToWorklist(RHS);
+    }
     return V;
+  }
 
   if (Value *V = foldIsPowerOf2OrZero(LHS, RHS, IsAnd, Builder, *this))
     return V;
diff --git a/llvm/test/Transforms/InstCombine/and-or-icmp-min-max.ll b/llvm/test/Transforms/InstCombine/and-or-icmp-min-max.ll
index 058847a75bde7f..cc55c4a39a2678 100644
--- a/llvm/test/Transforms/InstCombine/and-or-icmp-min-max.ll
+++ b/llvm/test/Transforms/InstCombine/and-or-icmp-min-max.ll
@@ -689,6 +689,17 @@ define i1 @sge_and_max_logical(i8 %x, i8 %y)  {
   ret i1 %r
 }
 
+define i1 @sge_and_max_logical_samesign(i8 %x, i8 %y)  {
+; CHECK-LABEL: @sge_and_max_logical_samesign(
+; CHECK-NEXT:    [[CMPEQ:%.*]] = icmp eq i8 [[X:%.*]], 127
+; CHECK-NEXT:    ret i1 [[CMPEQ]]
+;
+  %cmp = icmp sge i8 %x, %y
+  %cmpeq = icmp samesign eq i8 %x, 127
+  %r = select i1 %cmp, i1 %cmpeq, i1 false
+  ret i1 %r
+}
+
 define i1 @sge_and_max_commute(i8 %x, i8 %y)  {
 ; CHECK-LABEL: @sge_and_max_commute(
 ; CHECK-NEXT:    [[CMPEQ:%.*]] = icmp eq i8 [[X:%.*]], 127
diff --git a/llvm/test/Transforms/InstCombine/and-or-icmp-nullptr.ll b/llvm/test/Transforms/InstCombine/and-or-icmp-nullptr.ll
index d533cc7048536c..8650b89c3b9ef3 100644
--- a/llvm/test/Transforms/InstCombine/and-or-icmp-nullptr.ll
+++ b/llvm/test/Transforms/InstCombine/and-or-icmp-nullptr.ll
@@ -592,6 +592,19 @@ define i1 @sgt_and_min_logical(ptr %x, ptr %y)  {
   ret i1 %r
 }
 
+define i1 @sgt_and_min_logical_samesign(ptr %x, ptr %y)  {
+; CHECK-LABEL: @sgt_and_min_logical_samesign(
+; CHECK-NEXT:    [[CMPEQ:%.*]] = icmp eq ptr [[X:%.*]], null
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt ptr [[Y:%.*]], null
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[CMPEQ]], [[TMP1]]
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %cmp = icmp sgt ptr %x, %y
+  %cmpeq = icmp samesign eq ptr %x, null
+  %r = select i1 %cmp, i1 %cmpeq, i1 false
+  ret i1 %r
+}
+
 define i1 @sle_or_not_min(ptr %x, ptr %y)  {
 ; CHECK-LABEL: @sle_or_not_min(
 ; CHECK-NEXT:    [[CMPEQ:%.*]] = icmp ne ptr [[X:%.*]], null

[nikic]

LGTM

[antoniofrighetto]

LGTM.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 11 "Add check check-libc-amdgcn-amd-amdhsa".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/7139

Here is the relevant piece of the build log for the reference

Step 11 (Add check check-libc-amdgcn-amd-amdhsa) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-libc-amdgcn-amd-amdhsa'], attempting to kill
...
[2396/2641] Linking CXX executable libc/test/src/inttypes/libc.test.src.inttypes.strtoimax_test.__hermetic__.__build__
[2397/2641] Linking CXX executable libc/test/src/inttypes/libc.test.src.inttypes.strtoumax_test.__hermetic__.__build__
[2398/2641] Linking CXX executable libc/test/src/string/libc.test.src.string.strsep_test.__hermetic__.__build__
[2399/2641] Linking CXX executable libc/test/src/stdio/libc.test.src.stdio.vsnprintf_test.__hermetic__.__build__
[2400/2641] Linking CXX executable libc/test/src/string/libc.test.src.string.memcmp_test.__hermetic__.__build__
[2401/2641] Linking CXX executable libc/test/src/time/libc.test.src.time.clock_gettime_test.__hermetic__.__build__
[2402/2641] Linking CXX executable libc/test/src/string/libc.test.src.string.memmove_test.__hermetic__.__build__
[2403/2641] Linking CXX executable libc/test/src/string/libc.test.src.string.strncat_test.__hermetic__.__build__
[2404/2641] Linking CXX executable libc/test/src/stdio/libc.test.src.stdio.asprintf_test.__hermetic__.__build__
[2405/2641] Linking CXX executable libc/test/src/stdio/libc.test.src.stdio.vasprintf_test.__hermetic__.__build__
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-libc-amdgcn-amd-amdhsa'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1467.011683