[llvm] 8df376d - [InstCombine] Remove buggy zext of icmp eq with pow2 fold (PR57899)

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-09-22T16:37:10+02:00
New Revision: 8df376db7282b955e7990cb8887ee9dcd3565040

URL: https://github.com/llvm/llvm-project/commit/8df376db7282b955e7990cb8887ee9dcd3565040
DIFF: https://github.com/llvm/llvm-project/commit/8df376db7282b955e7990cb8887ee9dcd3565040.diff

LOG: [InstCombine] Remove buggy zext of icmp eq with pow2 fold (PR57899)

For the case where the constant is a power of two rather than zero,
the fold is incorrect, because it fails to check that the bit set
in the LHS matches the bit in the RHS.

Rather than fixing this, remove the power of two handling entirely,
as a different fold will already canonicalize such comparisons to
use a zero constant.

Fixes https://github.com/llvm/llvm-project/issues/57899.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
    llvm/test/Transforms/InstCombine/zext.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index cf144dd919b5..c91786148beb 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -1014,15 +1014,9 @@ Instruction *InstCombinerImpl::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext)
 
     // zext (X == 0) to i32 --> X^1      iff X has only the low bit set.
     // zext (X == 0) to i32 --> (X>>1)^1 iff X has only the 2nd bit set.
-    // zext (X == 1) to i32 --> X        iff X has only the low bit set.
-    // zext (X == 2) to i32 --> X>>1     iff X has only the 2nd bit set.
     // zext (X != 0) to i32 --> X        iff X has only the low bit set.
     // zext (X != 0) to i32 --> X>>1     iff X has only the 2nd bit set.
-    // zext (X != 1) to i32 --> X^1      iff X has only the low bit set.
-    // zext (X != 2) to i32 --> (X>>1)^1 iff X has only the 2nd bit set.
-    if ((Op1CV->isZero() || Op1CV->isPowerOf2()) &&
-        // This only works for EQ and NE
-        Cmp->isEquality()) {
+    if (Op1CV->isZero() && Cmp->isEquality()) {
       // If Op1C some other power of two, convert:
       KnownBits Known = computeKnownBits(Cmp->getOperand(0), 0, &Zext);
 
@@ -1038,7 +1032,7 @@ Instruction *InstCombinerImpl::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext)
                                   In->getName() + ".lobit");
         }
 
-        if (!Op1CV->isZero() == isNE) { // Toggle the low bit.
+        if (!isNE) { // Toggle the low bit.
           Constant *One = ConstantInt::get(In->getType(), 1);
           In = Builder.CreateXor(In, One);
         }

diff  --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll
index 82d91798438e..765ae1a1b64a 100644
--- a/llvm/test/Transforms/InstCombine/zext.ll
+++ b/llvm/test/Transforms/InstCombine/zext.ll
@@ -511,7 +511,6 @@ define i8 @disguised_signbit_clear_test(i64 %x) {
   ret i8 %t6
 }
 
-; FIXME: Currently miscompiled.
 define i16 @pr57899(i1 %c, i32 %x) {
 ; CHECK-LABEL: @pr57899(
 ; CHECK-NEXT:  entry:
@@ -519,7 +518,7 @@ define i16 @pr57899(i1 %c, i32 %x) {
 ; CHECK:       if:
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
-; CHECK-NEXT:    ret i16 0
+; CHECK-NEXT:    ret i16 1
 ;
 entry:
   br i1 %c, label %if, label %join