[llvm] 43b5fba - Revert "[InstCombine] Handle logical op in simplifyRangeCheck() (PR59484)"
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 14 03:04:30 PST 2022
Author: Nikita Popov
Date: 2022-12-14T12:04:21+01:00
New Revision: 43b5fbae3b00ff9dc11e5d102aad3a80be9f5067
URL: https://github.com/llvm/llvm-project/commit/43b5fbae3b00ff9dc11e5d102aad3a80be9f5067
DIFF: https://github.com/llvm/llvm-project/commit/43b5fbae3b00ff9dc11e5d102aad3a80be9f5067.diff
LOG: Revert "[InstCombine] Handle logical op in simplifyRangeCheck() (PR59484)"
This reverts commit 492c471839a66e354ebe696bd3e15f7477c63613.
As pointed out by nloped, the transform in f2 is not correct: If
%shr is poison, then freeze may result in a negative value. The
transform is correct in the case where the freeze is pushed through
the operation in a way that guarantees the result is non-negative,
which is the case I had tested.
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 66636adb2559f..66f171254234b 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 IsLogical) {
+ bool Inverted) {
// 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,8 +709,6 @@ 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);
}
@@ -2724,18 +2722,19 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
if (Value *V = foldIsPowerOf2OrZero(RHS, LHS, IsAnd, Builder))
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;
+ // 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 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;
+ // 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;
+ }
// 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 b7690a5ccdb78..ee0ea357542b0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -103,8 +103,7 @@ 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,
- bool IsLogical);
+ Value *simplifyRangeCheck(ICmpInst *Cmp0, ICmpInst *Cmp1, bool Inverted);
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 19ae14cf58d6b..284b098b02afa 100644
--- a/llvm/test/Transforms/InstCombine/pr49688.ll
+++ b/llvm/test/Transforms/InstCombine/pr49688.ll
@@ -5,10 +5,11 @@
define i1 @f(i32 %i1) {
; CHECK-LABEL: @f(
; CHECK-NEXT: entry:
-; 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]]
+; 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]]
;
entry:
%cmp = icmp slt i32 %i1, 0
@@ -21,10 +22,11 @@ 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: [[TMP1:%.*]] = freeze i32 [[SHR]]
-; CHECK-NEXT: [[TMP2:%.*]] = icmp ult i32 [[TMP1]], [[G:%.*]]
-; CHECK-NEXT: [[LOR_EXT:%.*]] = zext i1 [[TMP2]] to i32
+; 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: 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 6c12ba94ea3d1..3617abc03d37b 100644
--- a/llvm/test/Transforms/InstCombine/range-check.ll
+++ b/llvm/test/Transforms/InstCombine/range-check.ll
@@ -19,10 +19,11 @@ define i1 @test_and1(i32 %x, i32 %n) {
define i1 @test_and1_logical(i32 %x, i32 %n) {
; CHECK-LABEL: @test_and1_logical(
-; 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]]
+; 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]]
;
%nn = and i32 %n, 2147483647
%a = icmp sge i32 %x, 0
@@ -46,10 +47,11 @@ define i1 @test_and2(i32 %x, i32 %n) {
define i1 @test_and2_logical(i32 %x, i32 %n) {
; CHECK-LABEL: @test_and2_logical(
-; 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]]
+; 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]]
;
%nn = and i32 %n, 2147483647
%a = icmp sgt i32 %x, -1
@@ -125,10 +127,11 @@ define i1 @test_or1(i32 %x, i32 %n) {
define i1 @test_or1_logical(i32 %x, i32 %n) {
; CHECK-LABEL: @test_or1_logical(
-; 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]]
+; 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]]
;
%nn = and i32 %n, 2147483647
%a = icmp slt i32 %x, 0
@@ -152,10 +155,11 @@ define i1 @test_or2(i32 %x, i32 %n) {
define i1 @test_or2_logical(i32 %x, i32 %n) {
; CHECK-LABEL: @test_or2_logical(
-; 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]]
+; 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]]
;
%nn = and i32 %n, 2147483647
%a = icmp sle i32 %x, -1
More information about the llvm-commits
mailing list