[llvm] r299851 - [InstCombine] fix matching of or-of-icmps constants (PR32524)

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Apr 10 11:55:57 2017
New Revision: 299851

URL: http://llvm.org/viewvc/llvm-project?rev=299851&view=rev
Log:
[InstCombine] fix matching of or-of-icmps constants (PR32524)

Also, make the same change in and-of-icmps and remove a hack for detecting that case.

Finally, add some FIXME comments because the code duplication here is awful.

This should fix the remaining IR problem noted in:
https://bugs.llvm.org/show_bug.cgi?id=32524


Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/trunk/test/Transforms/InstCombine/or.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=299851&r1=299850&r2=299851&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Mon Apr 10 11:55:57 2017
@@ -807,6 +807,7 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
     }
   }
 
+  // FIXME: The code below is duplicated in FoldOrOfICmps.
   // From here on, we only handle:
   //    (icmp1 A, C1) & (icmp2 A, C2) --> something simpler.
   if (Val != Val2)
@@ -825,11 +826,14 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
 
   // Ensure that the larger constant is on the RHS.
   bool ShouldSwap;
-  if (CmpInst::isSigned(PredL) ||
-      (ICmpInst::isEquality(PredL) && CmpInst::isSigned(PredR)))
-    ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
-  else
+  if (CmpInst::isUnsigned(PredL) || CmpInst::isUnsigned(PredR)) {
+    // We have an unsigned compare (possibly with an equality compare), so treat
+    // the constants as unsigned.
     ShouldSwap = LHSC->getValue().ugt(RHSC->getValue());
+  } else {
+    // Equality transforms treat the constants as signed.
+    ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
+  }
 
   if (ShouldSwap) {
     std::swap(LHS, RHS);
@@ -877,10 +881,6 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
     case ICmpInst::ICMP_SGT: // (X != 13 & X s> 15) -> X s> 15
       return RHS;
     case ICmpInst::ICMP_NE:
-      // Special case to get the ordering right when the values wrap around
-      // zero.
-      if (LHSC->getValue() == 0 && RHSC->getValue().isAllOnesValue())
-        std::swap(LHSC, RHSC);
       if (LHSC == SubOne(RHSC)) { // (X != 13 & X != 14) -> X-13 >u 1
         Constant *AddC = ConstantExpr::getNeg(LHSC);
         Value *Add = Builder->CreateAdd(Val, AddC, Val->getName() + ".off");
@@ -1727,6 +1727,7 @@ Value *InstCombiner::FoldOrOfICmps(ICmpI
         return Builder->CreateICmpULE(Val, LHSC);
   }
 
+  // FIXME: The code below is duplicated in FoldAndOfICmps.
   // From here on, we only handle:
   //    (icmp1 A, C1) | (icmp2 A, C2) --> something simpler.
   if (Val != Val2)
@@ -1745,11 +1746,14 @@ Value *InstCombiner::FoldOrOfICmps(ICmpI
 
   // Ensure that the larger constant is on the RHS.
   bool ShouldSwap;
-  if (CmpInst::isSigned(PredL) ||
-      (ICmpInst::isEquality(PredL) && CmpInst::isSigned(PredR)))
-    ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
-  else
+  if (CmpInst::isUnsigned(PredL) || CmpInst::isUnsigned(PredR)) {
+    // We have an unsigned compare (possibly with an equality compare), so treat
+    // the constants as unsigned.
     ShouldSwap = LHSC->getValue().ugt(RHSC->getValue());
+  } else {
+    // Equality transforms treat the constants as signed.
+    ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
+  }
 
   if (ShouldSwap) {
     std::swap(LHS, RHS);

Modified: llvm/trunk/test/Transforms/InstCombine/or.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/or.ll?rev=299851&r1=299850&r2=299851&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/or.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/or.ll Mon Apr 10 11:55:57 2017
@@ -223,10 +223,9 @@ define i1 @test19(i32 %A) {
 
 define i1 @or_icmps_eq_diff1(i32 %x) {
 ; CHECK-LABEL: @or_icmps_eq_diff1(
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 %x, -1
-; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i32 %x, 0
-; CHECK-NEXT:    [[LOGIC:%.*]] = or i1 [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret i1 [[LOGIC]]
+; CHECK-NEXT:    [[X_OFF:%.*]] = add i32 %x, 1
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[X_OFF]], 2
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %cmp1 = icmp eq i32 %x, -1
   %cmp2 = icmp eq i32 %x, 0