Skip to content

[InstCombine] smin(smax(X, -1), 1) -> scmp(X, 0) and smax(smin(X, 1), -1) -> scmp(X, 0) #145736

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Jun 25, 2025

@AZero13 AZero13 requested a review from nikic as a code owner June 25, 2025 16:23
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jun 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-llvm-transforms

Author: AZero13 (AZero13)

Changes

Motivating case: https://godbolt.org/z/Wxcc51jcj

Alive2: https://alive2.llvm.org/ce/z/-bPPAg


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+28)
  • (modified) llvm/test/Transforms/InstCombine/compare-3way.ll (+80-20)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index b6ed1dc4331d2..5f1624abf68e2 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1920,6 +1920,34 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
   case Intrinsic::smin: {
     Value *I0 = II->getArgOperand(0), *I1 = II->getArgOperand(1);
     Value *X, *Y;
+
+    // smin(smax(X, -1), 1) -> scmp(X, 0)
+    // smax(smin(X, 1), -1) -> scmp(X, 0)
+    // These patterns clamp X to [-1, 1] which is equivalent to a three-way
+    // comparison
+
+    // TODO: Is One use needed? yes it is one intrinisc less, but these do
+    // expand.
+    if (IID == Intrinsic::smin) {
+      if (match(I0, m_OneUse(m_Intrinsic<Intrinsic::smax>(m_Value(X),
+                                                          m_AllOnes()))) &&
+          match(I1, m_One())) {
+        Value *Zero = ConstantInt::get(X->getType(), 0);
+        return replaceInstUsesWith(
+            CI,
+            Builder.CreateIntrinsic(II->getType(), Intrinsic::scmp, {X, Zero}));
+      }
+    }
+
+    if (IID == Intrinsic::smax) {
+      if (match(I0, m_OneUse(m_Intrinsic<Intrinsic::smin>(m_Value(X), m_One()))) &&
+          match(I1, m_AllOnes())) {
+        Value *Zero = ConstantInt::get(X->getType(), 0);
+        return replaceInstUsesWith(CI,
+            Builder.CreateIntrinsic(II->getType(), Intrinsic::scmp, {X, Zero}));
+      }
+    }
+    
     if (match(I0, m_SExt(m_Value(X))) && match(I1, m_SExt(m_Value(Y))) &&
         (I0->hasOneUse() || I1->hasOneUse()) && X->getType() == Y->getType()) {
       Value *NarrowMaxMin = Builder.CreateBinaryIntrinsic(IID, X, Y);
diff --git a/llvm/test/Transforms/InstCombine/compare-3way.ll b/llvm/test/Transforms/InstCombine/compare-3way.ll
index 5d443cd45238c..8f65756775f1b 100644
--- a/llvm/test/Transforms/InstCombine/compare-3way.ll
+++ b/llvm/test/Transforms/InstCombine/compare-3way.ll
@@ -81,8 +81,8 @@ unreached:
 define void @test_low_sle(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_low_sle
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -105,8 +105,8 @@ unreached:
 define void @test_low_ne(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_low_ne
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp slt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP_NOT]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -130,8 +130,8 @@ unreached:
 define void @test_low_eq(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_low_eq
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -154,8 +154,8 @@ unreached:
 define void @test_mid_sgt(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_mid_sgt
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -178,8 +178,8 @@ unreached:
 define void @test_mid_slt(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_mid_slt
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -252,8 +252,8 @@ unreached:
 define void @test_mid_ne(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_mid_ne
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[EQ:%.*]] = icmp eq i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[EQ]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp eq i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP_NOT]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -277,8 +277,8 @@ unreached:
 define void @test_mid_eq(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_mid_eq
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[EQ:%.*]] = icmp eq i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[EQ]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -348,8 +348,8 @@ unreached:
 define void @test_high_sge(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_high_sge
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -396,8 +396,8 @@ unreached:
 define void @test_high_ne(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_high_ne
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp sgt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP_NOT]], label [[NORMAL:%.*]], label [[UNREACHED:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -421,8 +421,8 @@ unreached:
 define void @test_high_eq(i64 %a, i64 %b) {
 ; CHECK-LABEL: define void @test_high_eq
 ; CHECK-SAME: (i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i64 [[A]], [[B]]
-; CHECK-NEXT:    br i1 [[TMP1]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[UNREACHED:%.*]], label [[NORMAL:%.*]]
 ; CHECK:       normal:
 ; CHECK-NEXT:    ret void
 ; CHECK:       unreached:
@@ -560,3 +560,63 @@ unreached:
   call void @use(i32 %result)
   ret void
 }
+
+define i32 @smax_smin_to_scmp(i32 %x) {
+; CHECK-LABEL: define i32 @smax_smin_to_scmp
+; CHECK-SAME: (i32 [[X:%.*]]) {
+; CHECK-NEXT:    [[COND5:%.*]] = call i32 @llvm.scmp.i32.i32(i32 [[X]], i32 0)
+; CHECK-NEXT:    ret i32 [[COND5]]
+;
+  %cond = call i32 @llvm.smax.i32(i32 %x, i32 -1)
+  %cond5 = call i32 @llvm.smin.i32(i32 %cond, i32 1)
+  ret i32 %cond5
+}
+
+define i16 @smax_smin_to_scmp_i16(i16 %x) {
+; CHECK-LABEL: define i16 @smax_smin_to_scmp_i16
+; CHECK-SAME: (i16 [[X:%.*]]) {
+; CHECK-NEXT:    [[COND5:%.*]] = call i16 @llvm.scmp.i16.i16(i16 [[X]], i16 0)
+; CHECK-NEXT:    ret i16 [[COND5]]
+;
+  %cond = call i16 @llvm.smax.i16(i16 %x, i16 -1)
+  %cond5 = call i16 @llvm.smin.i16(i16 %cond, i16 1)
+  ret i16 %cond5
+}
+
+; Test the reversed pattern: smax(smin(X, 1), -1) -> scmp(X, 0)
+define i32 @smin_smax_to_scmp(i32 %x) {
+; CHECK-LABEL: define i32 @smin_smax_to_scmp
+; CHECK-SAME: (i32 [[X:%.*]]) {
+; CHECK-NEXT:    [[COND5:%.*]] = call i32 @llvm.scmp.i32.i32(i32 [[X]], i32 0)
+; CHECK-NEXT:    ret i32 [[COND5]]
+;
+  %cond = call i32 @llvm.smin.i32(i32 %x, i32 1)
+  %cond5 = call i32 @llvm.smax.i32(i32 %cond, i32 -1)
+  ret i32 %cond5
+}
+
+define i32 @test_max_min_neg(i32 %x) {
+; CHECK-LABEL: define i32 @test_max_min_neg
+; CHECK-SAME: (i32 [[X:%.*]]) {
+; CHECK-NEXT:    [[COND:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 -2)
+; CHECK-NEXT:    [[COND5:%.*]] = call i32 @llvm.smin.i32(i32 [[COND]], i32 1)
+; CHECK-NEXT:    ret i32 [[COND5]]
+;
+  %cond = call i32 @llvm.smax.i32(i32 %x, i32 -2)
+  %cond5 = call i32 @llvm.smin.i32(i32 %cond, i32 1)
+  ret i32 %cond5
+}
+
+define i32 @test_multiple_uses(i32 %x) {
+; CHECK-LABEL: define i32 @test_multiple_uses
+; CHECK-SAME: (i32 [[X:%.*]]) {
+; CHECK-NEXT:    [[COND:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 -1)
+; CHECK-NEXT:    [[COND5:%.*]] = call i32 @llvm.smin.i32(i32 [[COND]], i32 1)
+; CHECK-NEXT:    [[SUM:%.*]] = add i32 [[COND]], [[COND5]]
+; CHECK-NEXT:    ret i32 [[SUM]]
+;
+  %cond = call i32 @llvm.smax.i32(i32 %x, i32 -1)
+  %cond5 = call i32 @llvm.smin.i32(i32 %cond, i32 1)
+  %sum = add i32 %cond, %cond5
+  ret i32 %sum
+}

Copy link

github-actions bot commented Jun 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@AZero13 AZero13 force-pushed the scmp branch 4 times, most recently from b53ab34 to db30a00 Compare June 25, 2025 17:10
@dtcxzyw
Copy link
Member

dtcxzyw commented Jun 27, 2025

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

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index c836ad7ae..1920ee9e4 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1978,8 +1978,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     // And i1's have been changed to and/ors
     // So we only need to check for smin
     if (IID == Intrinsic::smin) {
-      if (match(I0, m_OneUse(m_SMax(m_Value(X),
-                                                          m_AllOnes()))) &&
+      if (match(I0, m_OneUse(m_SMax(m_Value(X), m_AllOnes()))) &&
           match(I1, m_One())) {
         Value *Zero = ConstantInt::get(X->getType(), 0);
         return replaceInstUsesWith(

Please fix the format.

@AZero13
Copy link
Contributor Author

AZero13 commented Jun 27, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️
You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index c836ad7ae..1920ee9e4 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1978,8 +1978,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     // And i1's have been changed to and/ors
     // So we only need to check for smin
     if (IID == Intrinsic::smin) {
-      if (match(I0, m_OneUse(m_SMax(m_Value(X),
-                                                          m_AllOnes()))) &&
+      if (match(I0, m_OneUse(m_SMax(m_Value(X), m_AllOnes()))) &&
           match(I1, m_One())) {
         Value *Zero = ConstantInt::get(X->getType(), 0);
         return replaceInstUsesWith(

Please fix the format.

Fixed!

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@AZero13
Copy link
Contributor Author

AZero13 commented Jun 27, 2025

@nikic Can you please merge?

@AZero13
Copy link
Contributor Author

AZero13 commented Jun 28, 2025

@nikic I do not have merge permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants