[X86] Allow all legal integers to optimize smin with 0

[AZero13]

It makes no sense why smin has to be limited to 32 and 64 bits.

hasAndNot only exists for 32 and 64 bits, so this does not affect smax.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: AZero13 (AZero13)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1-1)
• (modified) llvm/test/CodeGen/X86/select-smin-smax.ll (+85)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index ce4c061725f7b..923a9b7cda341 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -25077,7 +25077,7 @@ SDValue X86TargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
     } else if (SDValue R = LowerSELECTWithCmpZero(CmpOp0, Op1, Op2, CondCode,
                                                   DL, DAG, Subtarget)) {
       return R;
-    } else if ((VT == MVT::i32 || VT == MVT::i64) && isNullConstant(Op2) &&
+    } else if (VT.isScalarInteger() && isNullConstant(Op2) &&
                Cmp.getNode()->hasOneUse() && (CmpOp0 == Op1) &&
                ((CondCode == X86::COND_S) ||                    // smin(x, 0)
                 (CondCode == X86::COND_G && hasAndNot(Op1)))) { // smax(x, 0)
diff --git a/llvm/test/CodeGen/X86/select-smin-smax.ll b/llvm/test/CodeGen/X86/select-smin-smax.ll
index a7fb60f37ea0c..abc54a184fe42 100644
--- a/llvm/test/CodeGen/X86/select-smin-smax.ll
+++ b/llvm/test/CodeGen/X86/select-smin-smax.ll
@@ -2,11 +2,62 @@
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=-bmi < %s | FileCheck %s --check-prefixes=CHECK,CHECK-NOBMI
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi < %s | FileCheck %s --check-prefixes=CHECK,CHECK-BMI
 
+declare i8 @llvm.smax.i8(i8, i8)
+declare i8 @llvm.smin.i8(i8, i8)
+declare i16 @llvm.smax.i16(i16, i16)
+declare i16 @llvm.smin.i16(i16, i16)
 declare i32 @llvm.smax.i32(i32, i32)
 declare i32 @llvm.smin.i32(i32, i32)
 declare i64 @llvm.smax.i64(i64, i64)
 declare i64 @llvm.smin.i64(i64, i64)
 
+define i8 @test_i8_smax(i8 %a) nounwind {
+; CHECK-LABEL: test_i8_smax:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    testb %dil, %dil
+; CHECK-NEXT:    cmovgl %edi, %eax
+; CHECK-NEXT:    # kill: def $al killed $al killed $eax
+; CHECK-NEXT:    retq
+  %r = call i8 @llvm.smax.i8(i8 %a, i8 0)
+  ret i8 %r
+}
+
+define i8 @test_i8_smin(i8 %a) nounwind {
+; CHECK-LABEL: test_i8_smin:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    sarb $7, %al
+; CHECK-NEXT:    andb %dil, %al
+; CHECK-NEXT:    retq
+  %r = call i8 @llvm.smin.i8(i8 %a, i8 0)
+  ret i8 %r
+}
+
+define i16 @test_i16_smax(i16 %a) nounwind {
+; CHECK-LABEL: test_i16_smax:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    testw %di, %di
+; CHECK-NEXT:    cmovgl %edi, %eax
+; CHECK-NEXT:    # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT:    retq
+  %r = call i16 @llvm.smax.i16(i16 %a, i16 0)
+  ret i16 %r
+}
+
+define i16 @test_i16_smin(i16 %a) nounwind {
+; CHECK-LABEL: test_i16_smin:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movswl %di, %eax
+; CHECK-NEXT:    sarl $15, %eax
+; CHECK-NEXT:    andl %edi, %eax
+; CHECK-NEXT:    # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT:    retq
+  %r = call i16 @llvm.smin.i16(i16 %a, i16 0)
+  ret i16 %r
+}
+
 define i32 @test_i32_smax(i32 %a) nounwind {
 ; CHECK-NOBMI-LABEL: test_i32_smax:
 ; CHECK-NOBMI:       # %bb.0:
@@ -64,3 +115,37 @@ define i64 @test_i64_smin(i64 %a) nounwind {
   %r = call i64 @llvm.smin.i64(i64 %a, i64 0)
   ret i64 %r
 }
+
+define i32 @test_i32_smax_mul_use(i32 %a) nounwind {
+; CHECK-NOBMI-LABEL: test_i32_smax_mul_use:
+; CHECK-NOBMI:       # %bb.0:
+; CHECK-NOBMI-NEXT:    xorl %eax, %eax
+; CHECK-NOBMI-NEXT:    testl %edi, %edi
+; CHECK-NOBMI-NEXT:    cmovgl %edi, %eax
+; CHECK-NOBMI-NEXT:    retq
+;
+; CHECK-BMI-LABEL: test_i32_smax_mul_use:
+; CHECK-BMI:       # %bb.0:
+; CHECK-BMI-NEXT:    movl %edi, %eax
+; CHECK-BMI-NEXT:    sarl $31, %eax
+; CHECK-BMI-NEXT:    andnl %edi, %eax, %eax
+; CHECK-BMI-NEXT:    retq
+  %r = call i32 @llvm.smax.i32(i32 %a, i32 0)
+  ret i32 %r
+}
+
+define i32 @test_i32_smin_mul_use(i32 %a, i32 %b, i32 %c) nounwind {
+; CHECK-LABEL: test_i32_smin_mul_use:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    testl %edi, %edi
+; CHECK-NEXT:    cmovsl %edi, %eax
+; CHECK-NEXT:    cmovnsl %edx, %esi
+; CHECK-NEXT:    addl %esi, %eax
+; CHECK-NEXT:    retq
+  %r = call i32 @llvm.smin.i32(i32 %a, i32 0)
+  %cmp = icmp slt i32 %a, 0
+  %ans = select i1 %cmp, i32 %b, i32 %c
+  %res = add i32 %ans, %r
+  ret i32 %res
+}

[topperc]

Your second commit changes the IR for a test case that should have been in your first commit.

[RKSimon]

I've updated select-smin-smax.ll with fuller test coverage (including your new tests) - please can you update the patch to show the new diffs?

[AZero13]

@RKSimon

[AZero13]

Deep apologies for this being in the backlog. I wanted to finish my other patches before rebasing this one and rebuilding on my machine.

[RKSimon]

LGTM - cheers