[InstCombine] Fold xored one-complemented operand comparisons #69882

elhewaty · 2023-10-22T15:10:18Z

[InstCombine] Add test coverage for comparisons of operands including one-complemented oparands(NFC).
[InstCombine] Fold xored one-complemented operand comparisons.
Alive2: https://alive2.llvm.org/ce/z/PZMJeB
Fixes clang is suboptimal for (~a ^ b) < ~a #69803.

llvmbot · 2023-10-22T15:11:17Z

@llvm/pr-subscribers-llvm-transforms

Author: None (elhewaty)

Changes

[InstCombine] Add test coverage for comparisons of operands including one-complemented oparands(NFC).
[InstCombine] Fold xored one-complemented operand comparisons.

Full diff: https://github.com/llvm/llvm-project/pull/69882.diff

2 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+29)
(modified) llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll (+112)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 66e2b6c72cce46c..4c19edfb27d2f2b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -6929,6 +6929,23 @@ static Instruction *foldReductionIdiom(ICmpInst &I,
   return nullptr;
 }
 
+// Gets the inverse of the predicate, but not the full predicate,
+// it doesn't change the equality, e.g SLE <-> SGE, SLT <-> SGT,
+// ULE <-> UGE, ULT <-> UGT
+static ICmpInst::Predicate ConvertPred(ICmpInst::Predicate Pred) {
+  switch(Pred) {
+    case ICmpInst::ICMP_SLE: return ICmpInst::ICMP_SGE;
+    case ICmpInst::ICMP_SGE: return ICmpInst::ICMP_SLE;
+    case ICmpInst::ICMP_SLT: return ICmpInst::ICMP_SGT;
+    case ICmpInst::ICMP_SGT: return ICmpInst::ICMP_SLT;
+    case ICmpInst::ICMP_ULE: return ICmpInst::ICMP_UGE;
+    case ICmpInst::ICMP_UGE: return ICmpInst::ICMP_ULE;
+    case ICmpInst::ICMP_ULT: return ICmpInst::ICMP_UGT;
+    case ICmpInst::ICMP_UGT: return ICmpInst::ICMP_ULT;
+    default: llvm_unreachable("Invalid Predicate");
+  }
+}
+
 Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
   bool Changed = false;
   const SimplifyQuery Q = SQ.getWithInstruction(&I);
@@ -7127,6 +7144,18 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
       return new ICmpInst(I.getInversePredicate(), Builder.CreateAnd(A, B),
                           Op1);
 
+    // Transform (~A ^ B) s< ~A --> (A ^ B) s> A,
+    //           (~A ^ B) s> ~A --> (A ^ B) s< A,
+    //           (~A ^ B) s<= ~A --> (A ^ B) s>= A,
+    //           (~A ^ B) s>= ~A --> (A ^ B) s<= A,
+    //           (~A ^ B) u< ~A --> (A ^ B) u< A,
+    //           (~A ^ B) u> ~A --> (A ^ B) u< A,
+    //           (~A ^ B) u<= ~A --> (A ^ B) u>= A,
+    // and       (~A ^ B) u>= ~A --> (A ^ B) <= A
+    if (match(Op0, m_Xor(m_Not(m_Value(A)), m_Value(B))) &&
+        match(Op1, m_Not(m_Value(A))) && !I.isEquality())
+      return new ICmpInst(ConvertPred(Pred), Builder.CreateXor(A, B), A);
+
     // ~X < ~Y --> Y < X
     // ~X < C -->  X > ~C
     if (match(Op0, m_Not(m_Value(A)))) {
diff --git a/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll b/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
index 9b6572697cf5e8f..893fb868e6adc77 100644
--- a/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll
@@ -4,6 +4,118 @@
 declare void @llvm.assume(i1)
 declare void @barrier()
 
+define i32 @test_slt_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_slt_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp slt i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_sle_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_sle_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp sge i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp sle i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_sgt_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_sgt_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp slt i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp sgt i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_sge_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_sge_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp sle i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp sge i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_ult_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_ult_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp ugt i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp ult i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_ule_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_ule_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp uge i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp ule i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_ugt_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_ugt_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp ult i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp ugt i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
+define i32 @test_uge_xor(i32 %0, i32 %1) {
+; CHECK-LABEL: @test_uge_xor(
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP0:%.*]], [[TMP1:%.*]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp ule i32 [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[TMP5:%.*]] = zext i1 [[TMP4]] to i32
+; CHECK-NEXT:    ret i32 [[TMP5]]
+;
+  %3 = xor i32 %0, -1
+  %4 = xor i32 %3, %1
+  %5 = icmp uge i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6
+}
+
 define i1 @xor_uge(i8 %x, i8 %y) {
 ; CHECK-LABEL: @xor_uge(
 ; CHECK-NEXT:    [[YNZ:%.*]] = icmp ne i8 [[Y:%.*]], 0

elhewaty · 2023-10-22T15:13:46Z

@dtcxzyw please have a look.

elhewaty · 2023-10-22T15:20:43Z

(~A ^ B) s< ~A --> (A ^ B) s> A
https://alive2.llvm.org/ce/z/kptBzu

(~A ^ B) s> ~A --> (A ^ B) s< A
https://alive2.llvm.org/ce/z/3_86kB

(~A ^ B) s<= ~A --> (A ^ B) s>= A
https://alive2.llvm.org/ce/z/qS7atw

(~A ^ B) s>= ~A --> (A ^ B) s<= A
https://alive2.llvm.org/ce/z/qS7atw

(~A ^ B) u< ~A --> (A ^ B) u< A
https://alive2.llvm.org/ce/z/4Z5Mpi

(~A ^ B) u> ~A --> (A ^ B) u< A
https://alive2.llvm.org/ce/z/mpY1Vb

(~A ^ B) u<= ~A --> (A ^ B) u>= A
https://alive2.llvm.org/ce/z/udZM2U

(~A ^ B) u>= ~A --> (A ^ B) <= A
https://alive2.llvm.org/ce/z/22u3Zm

github-actions · 2023-10-22T15:20:52Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 01893b54835ba8c0e97d67361a7b6128cd775565 b348c4631aa4da367e34a77c57c074fd3499f28a -- llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 83cb65aa0d..588b16a8c6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -7328,13 +7328,12 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
   // [0, UMAX], but it may still be fractional.  See if it is fractional by
   // casting the FP value to the integer value and back, checking for equality.
   // Don't do this for zero, because -0.0 is not fractional.
-  Constant *RHSInt = LHSUnsigned
-    ? ConstantExpr::getFPToUI(RHSC, IntTy)
-    : ConstantExpr::getFPToSI(RHSC, IntTy);
+  Constant *RHSInt = LHSUnsigned ? ConstantExpr::getFPToUI(RHSC, IntTy)
+                                 : ConstantExpr::getFPToSI(RHSC, IntTy);
   if (!RHS.isZero()) {
     bool Equal = LHSUnsigned
-      ? ConstantExpr::getUIToFP(RHSInt, RHSC->getType()) == RHSC
-      : ConstantExpr::getSIToFP(RHSInt, RHSC->getType()) == RHSC;
+                     ? ConstantExpr::getUIToFP(RHSInt, RHSC->getType()) == RHSC
+                     : ConstantExpr::getSIToFP(RHSInt, RHSC->getType()) == RHSC;
     if (!Equal) {
       // If we had a comparison against a fractional value, we have to adjust
       // the compare predicate and sometimes the value.  RHSC is rounded towards

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

goldsteinn · 2023-10-22T17:08:37Z

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll

+  %4 = xor i32 %3, %1
+  %5 = icmp uge i32 %4, %3
+  %6 = zext i1 %5 to i32
+  ret i32 %6


Can you swap the operand order in some of the tests. I.e all your tests of of the form:
~A ^ B pred A. Can you:

Add tests where its form: A pred Xor

Add tests where the xor is of form A ^ ~B

Can you add tests for ~A ^ B pred ~C

You don't need to add new tests for all these cases, just flip around operands in some of your current tests.

elhewaty · 2023-10-22T21:12:05Z

Alive2: https://alive2.llvm.org/ce/z/9DoJpD

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

elhewaty · 2023-10-24T15:19:35Z

@dtcxzyw why did the build fail?

dtcxzyw · 2023-10-24T15:25:56Z

@dtcxzyw why did the build fail?

You should add Value *C to shadow the definition const APInt *C in the outer scope.

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

goldsteinn · 2023-10-24T17:15:47Z

@dtcxzyw why did the build fail?

You should add Value *C to shadow the definition const APInt *C in the outer scope.

Or instead rename variables to X/Y/Z as shadow variables can cause confusion and in llvm C usually means constant which isn't the case here.

nikic · 2023-10-25T15:15:26Z

@elhewaty This is due to an incorrect rebase around the processUMulZExtIdiom() call. Can you please rebase another time over ea99df2, which should remove all the diffs unrelated to your change?

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll

elhewaty · 2023-11-13T06:28:16Z

@dtcxzyw updated.

dtcxzyw

LGTM with the value name fix in test_slt_xor and test_sle_xor.

… one-complemented oparands(NFC)

elhewaty · 2023-11-13T11:41:01Z

@dtcxzyw I used git clang-format but the check failed

dtcxzyw

LGTM.

dtcxzyw · 2023-11-13T11:45:34Z

@dtcxzyw I used git clang-format but the check failed

Emm, it looks like an internal failure of the buildbot.

elhewaty · 2023-11-13T12:03:18Z

So, What is the future of the pull?
Will it land?

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

elhewaty · 2023-11-14T09:06:59Z

@dtcxzyw Please take a look.

dtcxzyw

LGTM. Please wait for additional approval from other reviewers.

elhewaty · 2023-11-14T13:14:16Z

@nikic @goldsteinn @arsenm

nikic

LGTM

…9882) - [InstCombine] Add test coverage for comparisons of operands including one-complemented oparands(NFC). - [InstCombine] Fold xored one-complemented operand comparisons. Alive2: https://alive2.llvm.org/ce/z/PZMJeB Fixes llvm#69803.

elhewaty requested a review from nikic as a code owner October 22, 2023 15:10

llvmbot added the llvm:transforms label Oct 22, 2023

dtcxzyw requested a review from goldsteinn October 22, 2023 15:15

dtcxzyw changed the title ~~instcombine~~ [InstCombine] Fold xored one-complemented operand comparisons Oct 22, 2023

elhewaty mentioned this pull request Oct 22, 2023

clang is suboptimal for (~a ^ b) < ~a #69803

Closed

dtcxzyw requested changes Oct 22, 2023

View reviewed changes

dtcxzyw requested a review from arsenm October 22, 2023 15:59

goldsteinn reviewed Oct 22, 2023

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Oct 22, 2023

View reviewed changes

elhewaty force-pushed the instcombine branch from 4520481 to bd304b9 Compare October 22, 2023 21:07

goldsteinn reviewed Oct 22, 2023

View reviewed changes

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll Show resolved Hide resolved

elhewaty force-pushed the instcombine branch from bd304b9 to df596ac Compare October 22, 2023 22:52

elhewaty requested review from goldsteinn and dtcxzyw October 23, 2023 18:16

dtcxzyw requested changes Oct 24, 2023

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

elhewaty requested a review from dtcxzyw October 24, 2023 14:42

elhewaty force-pushed the instcombine branch from 1f52ac6 to eaa0d70 Compare October 24, 2023 14:55

dtcxzyw reviewed Oct 24, 2023

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

elhewaty force-pushed the instcombine branch from eaa0d70 to 9ce44ee Compare October 24, 2023 16:16

elhewaty requested a review from dtcxzyw October 24, 2023 16:16

elhewaty force-pushed the instcombine branch 2 times, most recently from f58c447 to a7ec75f Compare October 24, 2023 18:59

elhewaty force-pushed the instcombine branch from d155878 to 61c05b7 Compare October 25, 2023 15:56

elhewaty requested a review from dtcxzyw October 25, 2023 16:40

nikic reviewed Oct 26, 2023

View reviewed changes

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll Show resolved Hide resolved

nikic reviewed Oct 26, 2023

View reviewed changes

llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll Outdated Show resolved Hide resolved

elhewaty force-pushed the instcombine branch from 61c05b7 to 3a58da4 Compare November 13, 2023 06:25

elhewaty requested a review from nikic November 13, 2023 06:27

dtcxzyw approved these changes Nov 13, 2023

View reviewed changes

[InstCombine] Add test coverage for comparisons of operands including…

f42483f

… one-complemented oparands(NFC)

elhewaty force-pushed the instcombine branch from 3a58da4 to 7b9aaff Compare November 13, 2023 09:54

elhewaty requested a review from dtcxzyw November 13, 2023 09:55

dtcxzyw approved these changes Nov 13, 2023

View reviewed changes

dtcxzyw reviewed Nov 13, 2023

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp Outdated Show resolved Hide resolved

[InstCombine] Fold xored one-complemented operand comparisons.

b348c46

elhewaty force-pushed the instcombine branch from 7b9aaff to b348c46 Compare November 13, 2023 12:55

elhewaty requested a review from dtcxzyw November 13, 2023 12:57

dtcxzyw approved these changes Nov 14, 2023

View reviewed changes

nikic approved these changes Nov 14, 2023

View reviewed changes

dtcxzyw merged commit daddf40 into llvm:main Nov 14, 2023

elhewaty deleted the instcombine branch November 14, 2023 14:20

[InstCombine] Fold xored one-complemented operand comparisons #69882

[InstCombine] Fold xored one-complemented operand comparisons #69882

Uh oh!

Conversation

elhewaty commented Oct 22, 2023 • edited by dtcxzyw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 22, 2023

Uh oh!

elhewaty commented Oct 22, 2023

Uh oh!

elhewaty commented Oct 22, 2023

Uh oh!

github-actions bot commented Oct 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goldsteinn Oct 22, 2023

Choose a reason for hiding this comment

Uh oh!

elhewaty commented Oct 22, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elhewaty commented Oct 24, 2023

Uh oh!

dtcxzyw commented Oct 24, 2023

Uh oh!

Uh oh!

goldsteinn commented Oct 24, 2023

Uh oh!

nikic commented Oct 25, 2023

Uh oh!

Uh oh!

Uh oh!

elhewaty commented Nov 13, 2023

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

elhewaty commented Nov 13, 2023

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw commented Nov 13, 2023

Uh oh!

elhewaty commented Nov 13, 2023

Uh oh!

Uh oh!

elhewaty commented Nov 14, 2023

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

elhewaty commented Nov 14, 2023

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elhewaty commented Oct 22, 2023 •

edited by dtcxzyw

Loading

github-actions bot commented Oct 22, 2023 •

edited

Loading