[llvm] 492c471 - [InstCombine] Handle logical op in simplifyRangeCheck() (PR59484)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 00:51:26 PST 2022


Author: Nikita Popov
Date: 2022-12-13T09:51:18+01:00
New Revision: 492c471839a66e354ebe696bd3e15f7477c63613

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

LOG: [InstCombine] Handle logical op in simplifyRangeCheck() (PR59484)

We need to freeze to avoid propagating a potentially poison
upper bound (https://alive2.llvm.org/ce/z/MsD38k).

This resolves the existing TODO in the code.

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

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    llvm/test/Transforms/InstCombine/pr49688.ll
    llvm/test/Transforms/InstCombine/range-check.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 6fc6c33856171..9538f0f4a6bf3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -662,7 +662,7 @@ static Value *foldLogOpOfMaskedICmps(ICmpInst *LHS, ICmpInst *RHS, bool IsAnd,
 /// If \p Inverted is true then the check is for the inverted range, e.g.
 /// (icmp slt x, 0) | (icmp sgt x, n) --> icmp ugt x, n
 Value *InstCombinerImpl::simplifyRangeCheck(ICmpInst *Cmp0, ICmpInst *Cmp1,
-                                            bool Inverted) {
+                                            bool Inverted, bool IsLogical) {
   // Check the lower range comparison, e.g. x >= 0
   // InstCombine already ensured that if there is a constant it's on the RHS.
   ConstantInt *RangeStart = dyn_cast<ConstantInt>(Cmp0->getOperand(1));
@@ -709,6 +709,8 @@ Value *InstCombinerImpl::simplifyRangeCheck(ICmpInst *Cmp0, ICmpInst *Cmp1,
   if (Inverted)
     NewPred = ICmpInst::getInversePredicate(NewPred);
 
+  if (IsLogical)
+    RangeEnd = Builder.CreateFreeze(RangeEnd);
   return Builder.CreateICmp(NewPred, Input, RangeEnd);
 }
 
@@ -2672,19 +2674,18 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
   if (Value *V = foldIsPowerOf2OrZero(RHS, LHS, IsAnd, Builder))
     return V;
 
-  // TODO: One of these directions is fine with logical and/or, the other could
-  // be supported by inserting freeze.
-  if (!IsLogical) {
-    // E.g. (icmp slt x, 0) | (icmp sgt x, n) --> icmp ugt x, n
-    // E.g. (icmp sge x, 0) & (icmp slt x, n) --> icmp ult x, n
-    if (Value *V = simplifyRangeCheck(LHS, RHS, /*Inverted=*/!IsAnd))
-      return V;
+  // E.g. (icmp slt x, 0) | (icmp sgt x, n) --> icmp ugt x, n
+  // E.g. (icmp sge x, 0) & (icmp slt x, n) --> icmp ult x, n
+  if (Value *V = simplifyRangeCheck(LHS, RHS, /*Inverted=*/!IsAnd, IsLogical))
+    return V;
 
-    // E.g. (icmp sgt x, n) | (icmp slt x, 0) --> icmp ugt x, n
-    // E.g. (icmp slt x, n) & (icmp sge x, 0) --> icmp ult x, n
-    if (Value *V = simplifyRangeCheck(RHS, LHS, /*Inverted=*/!IsAnd))
-      return V;
-  }
+  // E.g. (icmp sgt x, n) | (icmp slt x, 0) --> icmp ugt x, n
+  // E.g. (icmp slt x, n) & (icmp sge x, 0) --> icmp ult x, n
+  // We can treat logical like bitwise here, because poison from both x and n
+  // propagates in the original pattern.
+  if (Value *V = simplifyRangeCheck(RHS, LHS, /*Inverted=*/!IsAnd,
+                                    /*IsLogical*/ false))
+    return V;
 
   // TODO: Add conjugated or fold, check whether it is safe for logical and/or.
   if (IsAnd && !IsLogical)

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index aa9f4051331df..7d6cebd6deaf9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -103,7 +103,8 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
   Instruction *visitUDiv(BinaryOperator &I);
   Instruction *visitSDiv(BinaryOperator &I);
   Instruction *visitFDiv(BinaryOperator &I);
-  Value *simplifyRangeCheck(ICmpInst *Cmp0, ICmpInst *Cmp1, bool Inverted);
+  Value *simplifyRangeCheck(ICmpInst *Cmp0, ICmpInst *Cmp1, bool Inverted,
+                            bool IsLogical);
   Instruction *visitAnd(BinaryOperator &I);
   Instruction *visitOr(BinaryOperator &I);
   bool sinkNotIntoOtherHandOfAndOrOr(BinaryOperator &I);

diff  --git a/llvm/test/Transforms/InstCombine/pr49688.ll b/llvm/test/Transforms/InstCombine/pr49688.ll
index 284b098b02afa..19ae14cf58d6b 100644
--- a/llvm/test/Transforms/InstCombine/pr49688.ll
+++ b/llvm/test/Transforms/InstCombine/pr49688.ll
@@ -5,11 +5,10 @@
 define i1 @f(i32 %i1) {
 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[I1:%.*]], 0
-; CHECK-NEXT:    [[SHR:%.*]] = lshr i32 7, [[I1]]
-; CHECK-NEXT:    [[CMP4:%.*]] = icmp slt i32 [[SHR]], [[I1]]
-; CHECK-NEXT:    [[I2:%.*]] = select i1 [[CMP]], i1 true, i1 [[CMP4]]
-; CHECK-NEXT:    ret i1 [[I2]]
+; CHECK-NEXT:    [[SHR:%.*]] = lshr i32 7, [[I1:%.*]]
+; CHECK-NEXT:    [[TMP0:%.*]] = freeze i32 [[SHR]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[TMP0]], [[I1]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
 entry:
   %cmp = icmp slt i32 %i1, 0
@@ -22,11 +21,10 @@ entry:
 ; %cmp should not vanish
 define i32 @f2(i32 signext %g, i32 zeroext %h) {
 ; CHECK-LABEL: @f2(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[G:%.*]], 0
 ; CHECK-NEXT:    [[SHR:%.*]] = lshr i32 7, [[H:%.*]]
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i32 [[SHR]], [[G]]
-; CHECK-NEXT:    [[DOT0:%.*]] = select i1 [[CMP]], i1 true, i1 [[CMP1]]
-; CHECK-NEXT:    [[LOR_EXT:%.*]] = zext i1 [[DOT0]] to i32
+; CHECK-NEXT:    [[TMP1:%.*]] = freeze i32 [[SHR]]
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp ult i32 [[TMP1]], [[G:%.*]]
+; CHECK-NEXT:    [[LOR_EXT:%.*]] = zext i1 [[TMP2]] to i32
 ; CHECK-NEXT:    ret i32 [[LOR_EXT]]
 ;
   %cmp = icmp slt i32 %g, 0

diff  --git a/llvm/test/Transforms/InstCombine/range-check.ll b/llvm/test/Transforms/InstCombine/range-check.ll
index 3617abc03d37b..6c12ba94ea3d1 100644
--- a/llvm/test/Transforms/InstCombine/range-check.ll
+++ b/llvm/test/Transforms/InstCombine/range-check.ll
@@ -19,11 +19,10 @@ define i1 @test_and1(i32 %x, i32 %n) {
 
 define i1 @test_and1_logical(i32 %x, i32 %n) {
 ; CHECK-LABEL: @test_and1_logical(
-; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N:%.*]], 2147483647
-; CHECK-NEXT:    [[A:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[B:%.*]] = icmp sgt i32 [[NN]], [[X]]
-; CHECK-NEXT:    [[C:%.*]] = select i1 [[A]], i1 [[B]], i1 false
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    [[N_FR:%.*]] = freeze i32 [[N:%.*]]
+; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N_FR]], 2147483647
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ugt i32 [[NN]], [[X:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %nn = and i32 %n, 2147483647
   %a = icmp sge i32 %x, 0
@@ -47,11 +46,10 @@ define i1 @test_and2(i32 %x, i32 %n) {
 
 define i1 @test_and2_logical(i32 %x, i32 %n) {
 ; CHECK-LABEL: @test_and2_logical(
-; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N:%.*]], 2147483647
-; CHECK-NEXT:    [[A:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[B:%.*]] = icmp sge i32 [[NN]], [[X]]
-; CHECK-NEXT:    [[C:%.*]] = select i1 [[A]], i1 [[B]], i1 false
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    [[N_FR:%.*]] = freeze i32 [[N:%.*]]
+; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N_FR]], 2147483647
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp uge i32 [[NN]], [[X:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %nn = and i32 %n, 2147483647
   %a = icmp sgt i32 %x, -1
@@ -127,11 +125,10 @@ define i1 @test_or1(i32 %x, i32 %n) {
 
 define i1 @test_or1_logical(i32 %x, i32 %n) {
 ; CHECK-LABEL: @test_or1_logical(
-; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N:%.*]], 2147483647
-; CHECK-NEXT:    [[A:%.*]] = icmp slt i32 [[X:%.*]], 0
-; CHECK-NEXT:    [[B:%.*]] = icmp sle i32 [[NN]], [[X]]
-; CHECK-NEXT:    [[C:%.*]] = select i1 [[A]], i1 true, i1 [[B]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    [[N_FR:%.*]] = freeze i32 [[N:%.*]]
+; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N_FR]], 2147483647
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i32 [[NN]], [[X:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %nn = and i32 %n, 2147483647
   %a = icmp slt i32 %x, 0
@@ -155,11 +152,10 @@ define i1 @test_or2(i32 %x, i32 %n) {
 
 define i1 @test_or2_logical(i32 %x, i32 %n) {
 ; CHECK-LABEL: @test_or2_logical(
-; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N:%.*]], 2147483647
-; CHECK-NEXT:    [[A:%.*]] = icmp slt i32 [[X:%.*]], 0
-; CHECK-NEXT:    [[B:%.*]] = icmp slt i32 [[NN]], [[X]]
-; CHECK-NEXT:    [[C:%.*]] = select i1 [[A]], i1 true, i1 [[B]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    [[N_FR:%.*]] = freeze i32 [[N:%.*]]
+; CHECK-NEXT:    [[NN:%.*]] = and i32 [[N_FR]], 2147483647
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[NN]], [[X:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %nn = and i32 %n, 2147483647
   %a = icmp sle i32 %x, -1


        


More information about the llvm-commits mailing list