[llvm] ca6e117 - [InstCombine] reorder icmp with offset folds for better results

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2021-07-14T12:12:05-04:00
New Revision: ca6e117d86341682e20025f204294c0ca78c355e

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

LOG: [InstCombine] reorder icmp with offset folds for better results

This set of folds was added recently with:
c7b658aeb526
0c400e895306
40b752d28d95

...and I noted that this wasn't likely to fire in code derived
from C/C++ source because of nsw in particular. But I didn't
notice that I had placed the code above the no-wrap block
of transforms.

This is likely the cause of regressions noted from the previous
commit because -- as shown in the test diffs -- we may have
transformed into a compare with an arbitrary constant rather
than a simpler signbit test.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
    llvm/test/Transforms/InstCombine/icmp-add.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 031b95e9c087..6de4c4d4aa34 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2650,25 +2650,6 @@ Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
   Value *X = Add->getOperand(0);
   Type *Ty = Add->getType();
   const CmpInst::Predicate Pred = Cmp.getPredicate();
-  const APInt SMax = APInt::getSignedMaxValue(Ty->getScalarSizeInBits());
-  const APInt SMin = APInt::getSignedMinValue(Ty->getScalarSizeInBits());
-
-  // Fold compare with offset to opposite sign compare if it eliminates offset:
-  // (X + C2) >u C --> X <s -C2 (if C == C2 + SMAX)
-  if (Pred == CmpInst::ICMP_UGT && C == *C2 + SMax)
-    return new ICmpInst(ICmpInst::ICMP_SLT, X, ConstantInt::get(Ty, -(*C2)));
-
-  // (X + C2) <u C --> X >s ~C2 (if C == C2 + SMIN)
-  if (Pred == CmpInst::ICMP_ULT && C == *C2 + SMin)
-    return new ICmpInst(ICmpInst::ICMP_SGT, X, ConstantInt::get(Ty, ~(*C2)));
-
-  // (X + C2) >s C --> X <u (SMAX - C) (if C == C2 - 1)
-  if (Pred == CmpInst::ICMP_SGT && C == *C2 - 1)
-    return new ICmpInst(ICmpInst::ICMP_ULT, X, ConstantInt::get(Ty, SMax - C));
-
-  // (X + C2) <s C --> X >u (C ^ SMAX) (if C == C2)
-  if (Pred == CmpInst::ICMP_SLT && C == *C2)
-    return new ICmpInst(ICmpInst::ICMP_UGT, X, ConstantInt::get(Ty, C ^ SMax));
 
   // If the add does not wrap, we can always adjust the compare by subtracting
   // the constants. Equality comparisons are handled elsewhere. SGE/SLE/UGE/ULE
@@ -2703,6 +2684,28 @@ Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
       return new ICmpInst(ICmpInst::ICMP_UGE, X, ConstantInt::get(Ty, Lower));
   }
 
+  // This set of folds is intentionally placed after folds that use no-wrapping
+  // flags because those folds are likely better for later analysis/codegen.
+  const APInt SMax = APInt::getSignedMaxValue(Ty->getScalarSizeInBits());
+  const APInt SMin = APInt::getSignedMinValue(Ty->getScalarSizeInBits());
+
+  // Fold compare with offset to opposite sign compare if it eliminates offset:
+  // (X + C2) >u C --> X <s -C2 (if C == C2 + SMAX)
+  if (Pred == CmpInst::ICMP_UGT && C == *C2 + SMax)
+    return new ICmpInst(ICmpInst::ICMP_SLT, X, ConstantInt::get(Ty, -(*C2)));
+
+  // (X + C2) <u C --> X >s ~C2 (if C == C2 + SMIN)
+  if (Pred == CmpInst::ICMP_ULT && C == *C2 + SMin)
+    return new ICmpInst(ICmpInst::ICMP_SGT, X, ConstantInt::get(Ty, ~(*C2)));
+
+  // (X + C2) >s C --> X <u (SMAX - C) (if C == C2 - 1)
+  if (Pred == CmpInst::ICMP_SGT && C == *C2 - 1)
+    return new ICmpInst(ICmpInst::ICMP_ULT, X, ConstantInt::get(Ty, SMax - C));
+
+  // (X + C2) <s C --> X >u (C ^ SMAX) (if C == C2)
+  if (Pred == CmpInst::ICMP_SLT && C == *C2)
+    return new ICmpInst(ICmpInst::ICMP_UGT, X, ConstantInt::get(Ty, C ^ SMax));
+
   if (!Add->hasOneUse())
     return nullptr;
 

diff  --git a/llvm/test/Transforms/InstCombine/icmp-add.ll b/llvm/test/Transforms/InstCombine/icmp-add.ll
index 3a9880697b21..c6dfb9d0e5d8 100644
--- a/llvm/test/Transforms/InstCombine/icmp-add.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-add.ll
@@ -796,7 +796,7 @@ define i1 @ugt_wrong_offset(i8 %a) {
 
 define i1 @ugt_offset_nuw(i8 %a) {
 ; CHECK-LABEL: @ugt_offset_nuw(
-; CHECK-NEXT:    [[OV:%.*]] = icmp slt i8 [[A:%.*]], -124
+; CHECK-NEXT:    [[OV:%.*]] = icmp slt i8 [[A:%.*]], 0
 ; CHECK-NEXT:    ret i1 [[OV]]
 ;
   %t = add nuw i8 %a, 124
@@ -852,7 +852,7 @@ define i1 @ult_wrong_offset(i8 %a) {
 
 define i1 @ult_offset_nuw(i8 %a) {
 ; CHECK-LABEL: @ult_offset_nuw(
-; CHECK-NEXT:    [[OV:%.*]] = icmp sgt i8 [[A:%.*]], -43
+; CHECK-NEXT:    [[OV:%.*]] = icmp sgt i8 [[A:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[OV]]
 ;
   %t = add nuw i8 %a, 42
@@ -908,7 +908,7 @@ define i1 @sgt_wrong_offset(i8 %a) {
 
 define i1 @sgt_offset_nsw(i8 %a, i8 %c) {
 ; CHECK-LABEL: @sgt_offset_nsw(
-; CHECK-NEXT:    [[OV:%.*]] = icmp ult i8 [[A:%.*]], 86
+; CHECK-NEXT:    [[OV:%.*]] = icmp sgt i8 [[A:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[OV]]
 ;
   %t = add nsw i8 %a, 42