Skip to content

[InstCombine] simplify icmp pred x, ~x #73990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 6, 2025
Merged

[InstCombine] simplify icmp pred x, ~x #73990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea9f69f
[InstCombine] Add test for simplify `X comp X^Neg_C`
ParkHanbum Nov 30, 2023
0d681e7
[InstCombine] simplify `X (comp) X^Neg_C` (#57532)
ParkHanbum Nov 30, 2023
e9aba12
Updated comments to use A instead of C
ParkHanbum Oct 16, 2024
11efb03
Replace llvm_unreachable with conservative nullptr return to handle p…
ParkHanbum Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4964,6 +4964,27 @@ static Instruction *foldICmpXorXX(ICmpInst &I, const SimplifyQuery &Q,
if (PredOut != Pred && isKnownNonZero(A, Q))
return new ICmpInst(PredOut, Op0, Op1);

// These transform work when A is negative.
// X s< X^A, X s<= X^A, X u> X^A, X u>= X^A --> X s< 0
// X s> X^A, X s>= X^A, X u< X^A, X u<= X^A --> X s>= 0
if (match(A, m_Negative())) {
CmpInst::Predicate NewPred;
switch (ICmpInst::getStrictPredicate(Pred)) {
default:
return nullptr;
case ICmpInst::ICMP_SLT:
case ICmpInst::ICMP_UGT:
NewPred = ICmpInst::ICMP_SLT;
break;
case ICmpInst::ICMP_SGT:
case ICmpInst::ICMP_ULT:
NewPred = ICmpInst::ICMP_SGE;
break;
}
Constant *Const = Constant::getNullValue(Op0->getType());
return new ICmpInst(NewPred, Op0, Const);
}

return nullptr;
}

Expand Down
343 changes: 343 additions & 0 deletions llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there are bit too many tests here for such a simple transform :) See https://llvm.org/docs/InstCombineContributorGuide.html#general-testing-considerations:

Add representative tests for each test category (discussed below), but don’t test all combinations of everything. If you have multi-use tests, and you have commuted tests, you shouldn’t also add commuted multi-use tests.

It's not necessary to have a variant of every test with -1 and -128. It's okay to have most tests with -1 and just one with -128.

Similarly, we don't need to have an i128 variant of every tests. It's again something where it's fine to test it just once.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,349 @@ declare void @llvm.assume(i1)
declare void @barrier()
declare void @use.i8(i8)

; Test case of X comp X^Neg_C, which have Transform to SLT.
; X s< X^Neg_C --> X s< 0
define i1 @src_slt(i8 %x) {
; CHECK-LABEL: @src_slt(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp slt i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_slt_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_slt_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp slt <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X s<= X^Neg_C --> X s< 0
define i1 @src_sle(i8 %x) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at this: https://llvm.godbolt.org/z/zvMcnvsE4 Currently, the src_sle and src_sle_comm (and other comm pairs) actually all test the case where the not is on the left hand side. To test the case where the not is on the right hand side, you need to introduce an additional, higher-complexity operation, like a mul in this example.

The other thing I noticed is that predicates already get canonicalized to from non-strict to strict, apparently thanks to this transform:

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Line 4537 in 1c49419

// icmp (X ^ Y_NonZero) u>= X --> icmp (X ^ Y_NonZero) u> X
I don't think this really changes anything for your patch, but it indicates that maybe there is some generalization that doesn't require the xor constant to be -1. But let's leave that for another patch...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic I could not even imagine that!! thank you for let me know!

I don't sure that I exact understood what you say.
are you want to that I need to change tests so that tests are can tested as purpose as for now.

should I change the code shown in the link next time?
or I could move my codes position into that function foldICmpXorXX?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic maybe this problem solved after implemented code moved to foidIcmpXorXX. how do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment comment is not done. The tests still don't properly check the commuted case -- and it looks like the implementation doesn't handle the commuted case either. It would be probably be better to move the code inside foldICmpXorXX(), because that already has handling for the commutation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After #91185 you shouldn't need to change the tests anymore -- just make sure that the commuted ones actually do get folded.

; CHECK-LABEL: @src_sle(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp sle i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_sle_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_sle_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp sle <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X u> X^Neg_C --> X s< 0
define i1 @src_ugt(i8 %x) {
; CHECK-LABEL: @src_ugt(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp ugt i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_ugt_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_ugt_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp ugt <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X u>= X^Neg_C --> X s< 0
define i1 @src_uge(i8 %x) {
; CHECK-LABEL: @src_uge(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp uge i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_uge_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_uge_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp uge <2 x i8> %x, %not
ret <2 x i1> %cmp
}

define <2 x i1> @src_uge_vec_min(<2 x i8> %x) {
; CHECK-LABEL: @src_uge_vec_min(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -128, i8 -128>
%cmp = icmp uge <2 x i8> %x, %not
ret <2 x i1> %cmp
}

define <2 x i1> @src_uge_vec_128(<2 x i128> %x) {
; CHECK-LABEL: @src_uge_vec_128(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i128> [[X:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i128> %x, <i128 -170141183460469231731687303715884105728, i128 -170141183460469231731687303715884105728>
%cmp = icmp uge <2 x i128> %x, %not
ret <2 x i1> %cmp
}

; Test case of X comp X^Neg_C, which have Transform to SGT.
; X s> X^Neg_C --> X s> -1
define i1 @src_sgt(i8 %x) {
; CHECK-LABEL: @src_sgt(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp sgt i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_sgt_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_sgt_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp sgt <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X s>= X^Neg_C --> X s> -1
define i1 @src_sge(i8 %x) {
; CHECK-LABEL: @src_sge(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp sge i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_sge_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_sge_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp sge <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X u< X^Neg_C --> X s> -1
define i1 @src_ult(i8 %x) {
; CHECK-LABEL: @src_ult(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp ult i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_ult_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_ult_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp ult <2 x i8> %x, %not
ret <2 x i1> %cmp
}

; X u<= X^Neg_C --> X s> -1
define i1 @src_ule(i8 %x) {
; CHECK-LABEL: @src_ule(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
; CHECK-NEXT: ret i1 [[CMP]]
;
%not = xor i8 %x, -1
%cmp = icmp ule i8 %x, %not
ret i1 %cmp
}

define <2 x i1> @src_ule_vec(<2 x i8> %x) {
; CHECK-LABEL: @src_ule_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -1, i8 -1>
%cmp = icmp ule <2 x i8> %x, %not
ret <2 x i1> %cmp
}

define <2 x i1> @src_ule_vec_min(<2 x i8> %x) {
; CHECK-LABEL: @src_ule_vec_min(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i8> %x, <i8 -128, i8 -128>
%cmp = icmp ule <2 x i8> %x, %not
ret <2 x i1> %cmp
}

define <2 x i1> @src_ule_vec_128(<2 x i128> %x) {
; CHECK-LABEL: @src_ule_vec_128(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i128> [[X:%.*]], <i128 -1, i128 -1>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%not = xor <2 x i128> %x, <i128 -170141183460469231731687303715884105728, i128 -170141183460469231731687303715884105728>
%cmp = icmp ule <2 x i128> %x, %not
ret <2 x i1> %cmp
}

; X comp X^Neg_C tests. negative
; X comp Y
define i1 @src_sle_xny(i8 %x, i8 %y) {
; CHECK-LABEL: @src_sle_xny(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp sle i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp sle i8 %x, %y.not
ret i1 %cmp
}
define i1 @src_sle_nyx(i8 %x, i8 %y) {
; CHECK-LABEL: @src_sle_nyx(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp sge i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp sle i8 %y.not, %x
ret i1 %cmp
}
define i1 @src_sge_xny(i8 %x, i8 %y) {
; CHECK-LABEL: @src_sge_xny(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp sge i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp sge i8 %x, %y.not
ret i1 %cmp
}
define i1 @src_sge_nyx(i8 %x, i8 %y) {
; CHECK-LABEL: @src_sge_nyx(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp sle i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp sge i8 %y.not, %x
ret i1 %cmp
}
define i1 @src_ule_xny(i8 %x, i8 %y) {
; CHECK-LABEL: @src_ule_xny(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp ule i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp ule i8 %x, %y.not
ret i1 %cmp
}
define i1 @src_ule_nyx(i8 %x, i8 %y) {
; CHECK-LABEL: @src_ule_nyx(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp uge i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp ule i8 %y.not, %x
ret i1 %cmp
}
define i1 @src_uge_xny(i8 %x, i8 %y) {
; CHECK-LABEL: @src_uge_xny(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp uge i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp uge i8 %x, %y.not
ret i1 %cmp
}
define i1 @src_uge_nyx(i8 %x, i8 %y) {
; CHECK-LABEL: @src_uge_nyx(
; CHECK-NEXT: [[Y_NOT:%.*]] = xor i8 [[Y:%.*]], -1
; CHECK-NEXT: [[CMP:%.*]] = icmp ule i8 [[X:%.*]], [[Y_NOT]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%y.not = xor i8 %y, -1
%cmp = icmp uge i8 %y.not, %x
ret i1 %cmp
}

; X comp X^Neg_C tests. negative
; (X+1) comp X^Neg_C
define i1 @src_sle_incx_nx(i8 %x) {
; CHECK-LABEL: @src_sle_incx_nx(
; CHECK-NEXT: [[TMP1:%.*]] = sub i8 -2, [[X:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp sge i8 [[TMP1]], [[X]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%nx = xor i8 %x, -1
%inc.x = add i8 %x, 1
%cmp = icmp sle i8 %inc.x, %nx
ret i1 %cmp
}
; (X-1) comp X^Neg_C
define i1 @src_sle_decx_nx(i8 %x) {
; CHECK-LABEL: @src_sle_decx_nx(
; CHECK-NEXT: [[TMP1:%.*]] = sub i8 0, [[X:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp sle i8 [[X]], [[TMP1]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%nx = xor i8 %x, -1
%dec.x = add i8 %x, -1
%cmp = icmp sle i8 %dec.x, %nx
ret i1 %cmp
}
; X comp (X+1)^Neg_C
define i1 @src_sle_x_nincx(i8 %x) {
; CHECK-LABEL: @src_sle_x_nincx(
; CHECK-NEXT: [[NOT_INC_X:%.*]] = sub i8 -2, [[X:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp sle i8 [[X]], [[NOT_INC_X]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%inc.x = add i8 %x, 1
%not.inc.x = xor i8 %inc.x, -1
%cmp = icmp sle i8 %x, %not.inc.x
ret i1 %cmp
}
; X comp (X-1)^Neg_C
define i1 @src_sle_x_ndecx(i8 %x) {
; CHECK-LABEL: @src_sle_x_ndecx(
; CHECK-NEXT: [[NOT_DEC_X:%.*]] = sub i8 0, [[X:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp sle i8 [[X]], [[NOT_DEC_X]]
; CHECK-NEXT: ret i1 [[CMP]]
;
%dec.x = add i8 %x, -1
%not.dec.x = xor i8 %dec.x, -1
%cmp = icmp sle i8 %x, %not.dec.x
ret i1 %cmp
}

; test for (~x ^ y) < ~z
define i1 @test_xor1(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @test_xor1(
Expand Down
Loading