-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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.
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
[InstCombine] simplify icmp pred x, ~x
#73990
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-transforms Author: hanbum (ParkHanbum) Changessimplify compare between specific variable Proof: https://alive2.llvm.org/ce/z/89XAvd Fixed #57532. Full diff: https://github.com/llvm/llvm-project/pull/73990.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 9bc84c7dd6e1539..f49cf31ffa82627 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -7035,6 +7035,63 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
return new ICmpInst(I.getSwappedPredicate(Pred), Builder.CreateXor(X, Y),
Z);
+ // Transform X s< ~X --> X s< 0
+ // X s> ~X --> X s> -1
+ // X s>= ~X --> X s> ~X
+ // X s<= ~X --> X s< ~X
+ // X u< ~X --> X u< (SIGNBIT(X))
+ // X u> ~X --> X u> (SIGNBIT(X))
+ // X u<= ~X --> X u< (SIGNBIT(X))
+ // X u>= ~X --> X u> (SIGNBIT(X))
+ // X == ~X --> false
+ // X != ~X --> true
+ if (match(&I, m_c_ICmp(Pred, m_Value(X), m_Value(Y))) &&
+ (match(X, m_c_Xor(m_Specific(Y), m_AllOnes())) ||
+ match(Y, m_c_Xor(m_Specific(X), m_AllOnes())))) {
+ // ~X s< X --> X s> ~X
+ // ~X s> X --> X s< ~X
+ // ~X u< X --> X u> ~X
+ // ~X u> X --> X u< ~X
+ if (match(X, m_c_Xor(m_Specific(Y), m_AllOnes()))) {
+ Pred = I.getSwappedPredicate();
+ std::swap(X, Y);
+ }
+
+ Constant *Const;
+ APInt C(X->getType()->getScalarSizeInBits(), 0);
+ switch (Pred) {
+ case ICmpInst::ICMP_EQ:
+ return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
+ break;
+ case ICmpInst::ICMP_NE:
+ return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
+ break;
+ case ICmpInst::ICMP_UGT:
+ case ICmpInst::ICMP_UGE:
+ case ICmpInst::ICMP_ULT:
+ case ICmpInst::ICMP_ULE:
+ Pred =
+ Pred < ICmpInst::ICMP_ULT ? ICmpInst::ICMP_UGT : ICmpInst::ICMP_ULT;
+ C.setSignBit();
+ Const = ConstantInt::get(X->getType(), C);
+ break;
+ case ICmpInst::ICMP_SGT:
+ case ICmpInst::ICMP_SGE:
+ Pred = ICmpInst::ICMP_SGT;
+ Const = ConstantInt::get(X->getType(), -1);
+ break;
+ case ICmpInst::ICMP_SLT:
+ case ICmpInst::ICMP_SLE:
+ Pred = ICmpInst::ICMP_SLT;
+ Const = ConstantInt::get(X->getType(), 0);
+ break;
+ default:
+ llvm_unreachable("not a valid predicate");
+ }
+
+ return new ICmpInst(Pred, X, Const);
+ }
+
// ~X < ~Y --> Y < X
// ~X < C --> X > ~C
if (match(Op0, m_Not(m_Value(X)))) {
diff --git a/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll b/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
index ef4f2bfecfd8ed9..2975001425bd6e1 100644
--- a/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
@@ -5,6 +5,203 @@ declare void @llvm.assume(i1)
declare void @barrier()
declare void @use.i8(i8)
+; X s< ~X --> X s< 0
+define i1 @src_xnx_slt_slt(i8 %x) {
+; CHECK-LABEL: @src_xnx_slt_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
+}
+; X s> ~X --> X s> -1
+define i1 @src_xnx_sgt_sgt(i8 %x) {
+; CHECK-LABEL: @src_xnx_sgt_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
+}
+; X (compare) ~X can never be equal.
+; X s<= ~X --> X s< ~X
+define i1 @src_xnx_sle_to_slt(i8 %x) {
+; CHECK-LABEL: @src_xnx_sle_to_slt(
+; 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
+}
+; X s>= ~X --> X s> ~X
+define i1 @src_xnx_sge_to_sgt(i8 %x) {
+; CHECK-LABEL: @src_xnx_sge_to_sgt(
+; 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
+}
+; slt or sgt can be converted to the other by swapping the true and false clauses
+; ~X s< X --> X s> ~X
+define i1 @src_nxx_slt_sgt(i8 %x) {
+; CHECK-LABEL: @src_nxx_slt_sgt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp slt i8 %not, %x
+ ret i1 %cmp
+}
+; ~X s> X --> X s< ~X
+define i1 @src_nxx_sgt_slt(i8 %x) {
+; CHECK-LABEL: @src_nxx_sgt_slt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp sgt i8 %not, %x
+ ret i1 %cmp
+}
+
+; X u< ~X --> X u> SIGNBIT_OF(X)
+define i1 @src_xnx_ult_ult(i8 %x) {
+; CHECK-LABEL: @src_xnx_ult_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 i1 @tgt_xnx_ult_ult(i8 %x) {
+; CHECK-LABEL: @tgt_xnx_ult_ult(
+; CHECK-NEXT: [[R:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT: ret i1 [[R]]
+;
+ %r = icmp ult i8 %x, 128
+ ret i1 %r
+}
+; X u> ~X --> X u< SIGNBIT_OF(X)
+define i1 @src_xnx_ugt_ugt(i8 %x) {
+; CHECK-LABEL: @src_xnx_ugt_ugt(
+; 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 i1 @tgt_xnx_ugt_ugt(i8 %x) {
+; CHECK-LABEL: @tgt_xnx_ugt_ugt(
+; CHECK-NEXT: [[R:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT: ret i1 [[R]]
+;
+ %r = icmp ult i8 %x, 128
+ ret i1 %r
+}
+; X (compare) ~X can never be equal.
+; X u<= ~X --> X u< ~X
+define i1 @src_xnx_ule_to_ult(i8 %x) {
+; CHECK-LABEL: @src_xnx_ule_to_ult(
+; 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 i1 @tgt_xnx_ule_to_ult(i8 %x) {
+; CHECK-LABEL: @tgt_xnx_ule_to_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
+}
+; X u>= ~X --> X u> ~X
+define i1 @src_xnx_uge_to_ugt(i8 %x) {
+; CHECK-LABEL: @src_xnx_uge_to_ugt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], -128
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp uge i8 %x, %not
+ ret i1 %cmp
+}
+define i1 @tgt_xnx_uge_to_ugt(i8 %x) {
+; CHECK-LABEL: @tgt_xnx_uge_to_ugt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], -128
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp ugt i8 %x, %not
+ ret i1 %cmp
+}
+; ult or ugt can be converted to the other by swapping the true and false clauses
+; ~X u< X --> X u> ~X
+define i1 @src_nxx_ult_ugt(i8 %x) {
+; CHECK-LABEL: @src_nxx_ult_ugt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], -128
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp ult i8 %not, %x
+ ret i1 %cmp
+}
+define i1 @tgt_nxx_ult_ugt(i8 %x) {
+; CHECK-LABEL: @tgt_nxx_ult_ugt(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], -128
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp ugt i8 %x, %not
+ ret i1 %cmp
+}
+; ~X u> X --> X u< ~X
+define i1 @src_nxx_ugt_ult(i8 %x) {
+; CHECK-LABEL: @src_nxx_ugt_ult(
+; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT: ret i1 [[CMP]]
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp ugt i8 %not, %x
+ ret i1 %cmp
+}
+define i1 @tgt_nxx_ugt_ult(i8 %x) {
+; CHECK-LABEL: @tgt_nxx_ugt_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
+}
+
+; X == ~X -> false
+define i1 @src_xnx_eq_to_0(i8 %x) {
+; CHECK-LABEL: @src_xnx_eq_to_0(
+; CHECK-NEXT: ret i1 false
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp eq i8 %x, %not
+ ret i1 %cmp
+}
+; X != ~X -> true
+define i1 @src_xnx_ne_to_1(i8 %x) {
+; CHECK-LABEL: @src_xnx_ne_to_1(
+; CHECK-NEXT: ret i1 true
+;
+ %not = xor i8 %x, -1
+ %cmp = icmp ne i8 %x, %not
+ ret i1 %cmp
+}
+
; test for (~x ^ y) < ~z
define i1 @test_xor1(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @test_xor1(
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
😭 What's wrong here? The code that appears to violate clang-code-format was not the code I submitted. anyone help |
bac64b3
to
5e5a7c4
Compare
3861d6b
to
d4d7c77
Compare
d4d7c77
to
965a7e1
Compare
x (comp) ~x
icmp pred x, ~x
965a7e1
to
86a9fd8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some style nits.
86a9fd8
to
ff03a29
Compare
ff03a29
to
8b4cfc1
Compare
I do something wrong while resolve conflict. for fix that, temporary closed PR a bit while. |
8b4cfc1
to
5738bb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please wait for additional approval from other reviewers.
5738bb7
to
5de7537
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me as well, but I have one note on the test. I don't think your "commutative" tests are testing what you think they are...
ret i1 %cmp | ||
} | ||
; X s<= ~X --> X s< 0 | ||
define i1 @src_sle(i8 %x) { |
There was a problem hiding this comment.
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:
// icmp (X ^ Y_NonZero) u>= X --> icmp (X ^ Y_NonZero) u> X |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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?
@dtcxzyw How about moving my code inside the foldICmpXorXX function?
|
Yeah, it makes sense. Please keep the commutative tests. |
0ed8159
to
c2c55c6
Compare
more generalized patch: |
Great! Wondering if we should generalize it as these patterns may not exist in real-world applications... |
Confirmed :( |
@sftlbcn I took a rough look at it, I don't understand everything, but it's great! I also wanted to take a more general approach using bit, but It was difficult because I was a newbie. should I close this PR? |
No, you shouldn't, and you should ping the reviewer with in one week if there's not reply. |
No, I think the current implementation is good enough :) |
This patch add testcase for comparison between X and X^Neg_X. comparison between X and X^Neg_C is determined solely by presence of the sign bit, so we can simplify it to checking whether X is negative or not.
This patch simplifies the comparison between X and X^Neg_X. comparison between X and X^Neg_C is determined solely by presence of the sign bit, so we can simplify it to checking whether X is negative or not. Proof: https://alive2.llvm.org/ce/z/P6zXUx
✅ With the latest revision this PR passed the Python code formatter. |
-. eq/ne case removed. |
simplify compare between specific variable
X
andNOT(X)
Proof: https://alive2.llvm.org/ce/z/KTCpjP
Fixed #57532.