[ARM] Add instruction selection patterns for vmin/vmax

[sbaranga]

Summary

The mid-end was generating vector smin/smax/umin/umax nodes, but
we were using vbsl to generatate the code. This adds the vmin/vmax
patterns and a test to check that we are now generating vmin/vmax
instructions.

[rengolin]

LGTM. Thanks!

[jmolloy]

This isn't the best way to do this. Instead, we should be modifying the definition of VMINs and friends to match a "smin" node instead of a "int_arm_neon_smin".

Then, we should add lowering to ARMISelLowering.cpp to map from arm_neon_vmins to ISD::SMIN. See how I did it for FMINNAN/FMINNUM as an example.

I've noticed that you've copied this mechanism from the AArch64 backend. Mea culpa - I obviously did it suboptimally there too. Either change AArch64 too as a followup or raise a PR and I'll get around to it.

Cheers,

James

[rengolin]

Sorry James, I assumed it was ok. I'm ok with waiting for a proper implementation on this, and then change the AArch64 later.

[sbaranga]

Removed patterns and modified .td definition to match generic min/max nodes
instead of intrisic ones.

Added code to lower neon intrinsic nodes to generic min/max.

[sbaranga]

Thanks, I'll just follow up on AArch64 as well after this.

-Silviu

[jmolloy]

Much nicer! LGTM

[sbaranga]

Thanks, r245439!

lib/Target/ARM/ARMISelLowering.cpp

void ARMTargetLowering::addTypeForNEON(MVT VT, MVT PromotedLdStVT,
   setOperationAction(ISD::SREM, VT, Expand);
   setOperationAction(ISD::UREM, VT, Expand);
   setOperationAction(ISD::FREM, VT, Expand);
 
   if (VT.isInteger()) {
     setOperationAction(ISD::SABSDIFF, VT, Legal);
     setOperationAction(ISD::UABSDIFF, VT, Legal);
   }
+  if (!VT.isFloatingPoint() &&
+      VT != MVT::v2i64 && VT != MVT::v1i64)
+    for (unsigned Opcode : {ISD::SMIN, ISD::SMAX, ISD::UMIN, ISD::UMAX})
+      setOperationAction(Opcode, VT, Legal);
+
 }
 
 void ARMTargetLowering::addDRTypeForNEON(MVT VT) {
   addRegisterClass(VT, &ARM::DPRRegClass);
   addTypeForNEON(VT, MVT::f64, MVT::v2i32);
 }
 
 void ARMTargetLowering::addQRTypeForNEON(MVT VT) {
ARMTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG,
   }
   case Intrinsic::arm_neon_vminnm:
   case Intrinsic::arm_neon_vmaxnm: {
     unsigned NewOpc = (IntNo == Intrinsic::arm_neon_vminnm)
       ? ISD::FMINNUM : ISD::FMAXNUM;
     return DAG.getNode(NewOpc, SDLoc(Op), Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   }
+  case Intrinsic::arm_neon_vminu:
+  case Intrinsic::arm_neon_vmaxu: {
+    if (Op.getValueType().isFloatingPoint())
+      return SDValue();
+    unsigned NewOpc = (IntNo == Intrinsic::arm_neon_vminu)
+      ? ISD::UMIN : ISD::UMAX;
+    return DAG.getNode(NewOpc, SDLoc(Op), Op.getValueType(),
+                         Op.getOperand(1), Op.getOperand(2));
+  }
   case Intrinsic::arm_neon_vmins:
   case Intrinsic::arm_neon_vmaxs: {
     // v{min,max}s is overloaded between signed integers and floats.
-    if (!Op.getValueType().isFloatingPoint())
-      return SDValue();
+    if (!Op.getValueType().isFloatingPoint()) {
+      unsigned NewOpc = (IntNo == Intrinsic::arm_neon_vmins)
+        ? ISD::SMIN : ISD::SMAX;
+      return DAG.getNode(NewOpc, SDLoc(Op), Op.getValueType(),
+                         Op.getOperand(1), Op.getOperand(2));
+    }
     unsigned NewOpc = (IntNo == Intrinsic::arm_neon_vmins)
       ? ISD::FMINNAN : ISD::FMAXNAN;
     return DAG.getNode(NewOpc, SDLoc(Op), Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   }
   }
 }
 

lib/Target/ARM/ARMInstrNEON.td

 defm VABALu   : N3VLIntExtOp_QHS<1,1,0b0101,0, IIC_VABAD,
                                  "vabal", "u", uabsdiff, zext, add>;
 
 // Vector Maximum and Minimum.
 
 //   VMAX     : Vector Maximum
 defm VMAXs    : N3VInt_QHS<0, 0, 0b0110, 0, N3RegFrm,
                            IIC_VSUBi4D, IIC_VSUBi4D, IIC_VSUBi4Q, IIC_VSUBi4Q,
-                           "vmax", "s", int_arm_neon_vmaxs, 1>;
+                           "vmax", "s", smax, 1>;
 defm VMAXu    : N3VInt_QHS<1, 0, 0b0110, 0, N3RegFrm,
                            IIC_VSUBi4D, IIC_VSUBi4D, IIC_VSUBi4Q, IIC_VSUBi4Q,
-                           "vmax", "u", int_arm_neon_vmaxu, 1>;
+                           "vmax", "u", umax, 1>;
 def  VMAXfd   : N3VDInt<0, 0, 0b00, 0b1111, 0, N3RegFrm, IIC_VBIND,
                         "vmax", "f32",
                         v2f32, v2f32, fmaxnan, 1>;
 def  VMAXfq   : N3VQInt<0, 0, 0b00, 0b1111, 0, N3RegFrm, IIC_VBINQ,
                         "vmax", "f32",
                         v4f32, v4f32, fmaxnan, 1>;
 
 // VMAXNM
 let PostEncoderMethod = "NEONThumb2V8PostEncoder", DecoderNamespace = "v8NEON" in {
   def VMAXNMND  : N3VDIntnp<0b00110, 0b00, 0b1111, 0, 1,
                             N3RegFrm, NoItinerary, "vmaxnm", "f32",
                             v2f32, v2f32, fmaxnum, 1>,
                             Requires<[HasV8, HasNEON]>;
   def VMAXNMNQ  : N3VQIntnp<0b00110, 0b00, 0b1111, 1, 1,
                             N3RegFrm, NoItinerary, "vmaxnm", "f32",
                             v4f32, v4f32, fmaxnum, 1>,
                             Requires<[HasV8, HasNEON]>;
 }
 
 //   VMIN     : Vector Minimum
 defm VMINs    : N3VInt_QHS<0, 0, 0b0110, 1, N3RegFrm,
                            IIC_VSUBi4D, IIC_VSUBi4D, IIC_VSUBi4Q, IIC_VSUBi4Q,
-                           "vmin", "s", int_arm_neon_vmins, 1>;
+                           "vmin", "s", smin, 1>;
 defm VMINu    : N3VInt_QHS<1, 0, 0b0110, 1, N3RegFrm,
                            IIC_VSUBi4D, IIC_VSUBi4D, IIC_VSUBi4Q, IIC_VSUBi4Q,
-                           "vmin", "u", int_arm_neon_vminu, 1>;
+                           "vmin", "u", umin, 1>;
 def  VMINfd   : N3VDInt<0, 0, 0b10, 0b1111, 0, N3RegFrm, IIC_VBIND,
                         "vmin", "f32",
                         v2f32, v2f32, fminnan, 1>;
 def  VMINfq   : N3VQInt<0, 0, 0b10, 0b1111, 0, N3RegFrm, IIC_VBINQ,
                         "vmin", "f32",
                         v4f32, v4f32, fminnan, 1>;
 
 // VMINNM

test/CodeGen/ARM/minmax.ll

+; RUN: llc < %s -mtriple=armv8-linux-gnu -mattr=+neon | FileCheck %s
+
+; CHECK-LABEL: t1
+; CHECK: vmax.s32 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <4 x i32> @t1(<4 x i32> %a, <4 x i32> %b) {
+  %t1 = icmp sgt <4 x i32> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i32> %a, <4 x i32> %b
+  ret <4 x i32> %t2
+}
+
+; CHECK-LABEL: t2
+; CHECK: vmin.s32 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <4 x i32> @t2(<4 x i32> %a, <4 x i32> %b) {
+  %t1 = icmp slt <4 x i32> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i32> %a, <4 x i32> %b
+  ret <4 x i32> %t2
+}
+
+; CHECK-LABEL: t3
+; CHECK: vmax.u32 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <4 x i32> @t3(<4 x i32> %a, <4 x i32> %b) {
+  %t1 = icmp ugt <4 x i32> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i32> %a, <4 x i32> %b
+  ret <4 x i32> %t2
+}
+
+; CHECK-LABEL: t4
+; CHECK: vmin.u32 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <4 x i32> @t4(<4 x i32> %a, <4 x i32> %b) {
+  %t1 = icmp ult <4 x i32> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i32> %a, <4 x i32> %b
+  ret <4 x i32> %t2
+}
+
+; CHECK-LABEL: t5
+; CHECK: vmax.s32 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <2 x i32> @t5(<2 x i32> %a, <2 x i32> %b) {
+  %t1 = icmp sgt <2 x i32> %a, %b
+  %t2 = select <2 x i1> %t1, <2 x i32> %a, <2 x i32> %b
+  ret <2 x i32> %t2
+}
+
+; CHECK-LABEL: t6
+; CHECK: vmin.s32 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <2 x i32> @t6(<2 x i32> %a, <2 x i32> %b) {
+  %t1 = icmp slt <2 x i32> %a, %b
+  %t2 = select <2 x i1> %t1, <2 x i32> %a, <2 x i32> %b
+  ret <2 x i32> %t2
+}
+
+; CHECK-LABEL: t7
+; CHECK: vmax.u32 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <2 x i32> @t7(<2 x i32> %a, <2 x i32> %b) {
+  %t1 = icmp ugt <2 x i32> %a, %b
+  %t2 = select <2 x i1> %t1, <2 x i32> %a, <2 x i32> %b
+  ret <2 x i32> %t2
+}
+
+; CHECK-LABEL: t8
+; CHECK: vmin.u32 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <2 x i32> @t8(<2 x i32> %a, <2 x i32> %b) {
+  %t1 = icmp ult <2 x i32> %a, %b
+  %t2 = select <2 x i1> %t1, <2 x i32> %a, <2 x i32> %b
+  ret <2 x i32> %t2
+}
+
+; CHECK-LABEL: t9
+; CHECK: vmax.s16 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <8 x i16> @t9(<8 x i16> %a, <8 x i16> %b) {
+  %t1 = icmp sgt <8 x i16> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i16> %a, <8 x i16> %b
+  ret <8 x i16> %t2
+}
+
+; CHECK-LABEL: t10
+; CHECK: vmin.s16 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <8 x i16> @t10(<8 x i16> %a, <8 x i16> %b) {
+  %t1 = icmp slt <8 x i16> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i16> %a, <8 x i16> %b
+  ret <8 x i16> %t2
+}
+
+; CHECK-LABEL: t11
+; CHECK: vmax.u16 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <8 x i16> @t11(<8 x i16> %a, <8 x i16> %b) {
+  %t1 = icmp ugt <8 x i16> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i16> %a, <8 x i16> %b
+  ret <8 x i16> %t2
+}
+
+; CHECK-LABEL: t12
+; CHECK: vmin.u16 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <8 x i16> @t12(<8 x i16> %a, <8 x i16> %b) {
+  %t1 = icmp ult <8 x i16> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i16> %a, <8 x i16> %b
+  ret <8 x i16> %t2
+}
+
+; CHECK-LABEL: t13
+; CHECK: vmax.s16
+define <4 x i16> @t13(<4 x i16> %a, <4 x i16> %b) {
+  %t1 = icmp sgt <4 x i16> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i16> %a, <4 x i16> %b
+  ret <4 x i16> %t2
+}
+
+; CHECK-LABEL: t14
+; CHECK: vmin.s16 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <4 x i16> @t14(<4 x i16> %a, <4 x i16> %b) {
+  %t1 = icmp slt <4 x i16> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i16> %a, <4 x i16> %b
+  ret <4 x i16> %t2
+}
+
+; CHECK-LABEL: t15
+; CHECK: vmax.u16 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <4 x i16> @t15(<4 x i16> %a, <4 x i16> %b) {
+  %t1 = icmp ugt <4 x i16> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i16> %a, <4 x i16> %b
+  ret <4 x i16> %t2
+}
+
+; CHECK-LABEL: t16
+; CHECK: vmin.u16 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <4 x i16> @t16(<4 x i16> %a, <4 x i16> %b) {
+  %t1 = icmp ult <4 x i16> %a, %b
+  %t2 = select <4 x i1> %t1, <4 x i16> %a, <4 x i16> %b
+  ret <4 x i16> %t2
+}
+
+; CHECK-LABEL: t17
+; CHECK: vmax.s8 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <16 x i8> @t17(<16 x i8> %a, <16 x i8> %b) {
+  %t1 = icmp sgt <16 x i8> %a, %b
+  %t2 = select <16 x i1> %t1, <16 x i8> %a, <16 x i8> %b
+  ret <16 x i8> %t2
+}
+
+; CHECK-LABEL: t18
+; CHECK: vmin.s8 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <16 x i8> @t18(<16 x i8> %a, <16 x i8> %b) {
+  %t1 = icmp slt <16 x i8> %a, %b
+  %t2 = select <16 x i1> %t1, <16 x i8> %a, <16 x i8> %b
+  ret <16 x i8> %t2
+}
+
+; CHECK-LABEL: t19
+; CHECK: vmax.u8 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <16 x i8> @t19(<16 x i8> %a, <16 x i8> %b) {
+  %t1 = icmp ugt <16 x i8> %a, %b
+  %t2 = select <16 x i1> %t1, <16 x i8> %a, <16 x i8> %b
+  ret <16 x i8> %t2
+}
+
+; CHECK-LABEL: t20
+; CHECK: vmin.u8 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
+define <16 x i8> @t20(<16 x i8> %a, <16 x i8> %b) {
+  %t1 = icmp ult <16 x i8> %a, %b
+  %t2 = select <16 x i1> %t1, <16 x i8> %a, <16 x i8> %b
+  ret <16 x i8> %t2
+}
+
+; CHECK-LABEL: t21
+; CHECK: vmax.s8 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <8 x i8> @t21(<8 x i8> %a, <8 x i8> %b) {
+  %t1 = icmp sgt <8 x i8> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i8> %a, <8 x i8> %b
+  ret <8 x i8> %t2
+}
+
+; CHECK-LABEL: t22
+; CHECK: vmin.s8 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <8 x i8> @t22(<8 x i8> %a, <8 x i8> %b) {
+  %t1 = icmp slt <8 x i8> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i8> %a, <8 x i8> %b
+  ret <8 x i8> %t2
+}
+
+; CHECK-LABEL: t23
+; CHECK: vmax.u8 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <8 x i8> @t23(<8 x i8> %a, <8 x i8> %b) {
+  %t1 = icmp ugt <8 x i8> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i8> %a, <8 x i8> %b
+  ret <8 x i8> %t2
+}
+
+; CHECK-LABEL: t24
+; CHECK: vmin.u8 {{d[0-9]+}}, {{d[0-9]+}}, {{d[0-9]+}}
+define <8 x i8> @t24(<8 x i8> %a, <8 x i8> %b) {
+  %t1 = icmp ult <8 x i8> %a, %b
+  %t2 = select <8 x i1> %t1, <8 x i8> %a, <8 x i8> %b
+  ret <8 x i8> %t2
+}

test/CodeGen/ARM/vselect_imax.ll

 ; RUN: opt < %s  -cost-model -analyze -mtriple=thumbv7-apple-ios6.0.0 -march=arm -mcpu=cortex-a8 | FileCheck %s --check-prefix=COST
 ; RUN: llc -mtriple=arm-eabi -mattr=+neon %s -o - | FileCheck %s
 ; Make sure that ARM backend with NEON handles vselect.
 
 define void @vmax_v4i32(<4 x i32>* %m, <4 x i32> %a, <4 x i32> %b) {
-; CHECK: vcgt.s32 [[QR:q[0-9]+]], [[Q1:q[0-9]+]], [[Q2:q[0-9]+]]
-; CHECK: vbsl [[QR]], [[Q1]], [[Q2]]
+; CHECK: vmax.s32 {{q[0-9]+}}, {{q[0-9]+}}, {{q[0-9]+}}
     %cmpres = icmp sgt <4 x i32> %a, %b
     %maxres = select <4 x i1> %cmpres, <4 x i32> %a,  <4 x i32> %b
     store <4 x i32> %maxres, <4 x i32>* %m
     ret void
 }
 
 ; We adjusted the cost model of the following selects. When we improve code
 ; lowering we also need to adjust the cost.
 %T0_10 = type <16 x i16>
 %T1_10 = type <16 x i1>
 ; CHECK-LABEL: func_blend10:
 define void @func_blend10(%T0_10* %loadaddr, %T0_10* %loadaddr2,
                            %T1_10* %blend, %T0_10* %storeaddr) {
   %v0 = load %T0_10, %T0_10* %loadaddr
   %v1 = load %T0_10, %T0_10* %loadaddr2
   %c = icmp slt %T0_10 %v0, %v1
-; CHECK: vbsl
-; CHECK: vbsl
+; CHECK: vmin.s16
+; CHECK: vmin.s16
 ; COST: func_blend10
 ; COST: cost of 40 {{.*}} select
   %r = select %T1_10 %c, %T0_10 %v0, %T0_10 %v1
   store %T0_10 %r, %T0_10* %storeaddr
   ret void
 }
 %T0_14 = type <8 x i32>
 %T1_14 = type <8 x i1>
 ; CHECK-LABEL: func_blend14:
 define void @func_blend14(%T0_14* %loadaddr, %T0_14* %loadaddr2,
                            %T1_14* %blend, %T0_14* %storeaddr) {
   %v0 = load %T0_14, %T0_14* %loadaddr
   %v1 = load %T0_14, %T0_14* %loadaddr2
   %c = icmp slt %T0_14 %v0, %v1
-; CHECK: vbsl
-; CHECK: vbsl
+; CHECK: vmin.s32
+; CHECK: vmin.s32
 ; COST: func_blend14
 ; COST: cost of 41 {{.*}} select
   %r = select %T1_14 %c, %T0_14 %v0, %T0_14 %v1
   store %T0_14 %r, %T0_14* %storeaddr
   ret void
 }
 %T0_15 = type <16 x i32>
 %T1_15 = type <16 x i1>
 ; CHECK-LABEL: func_blend15:
 define void @func_blend15(%T0_15* %loadaddr, %T0_15* %loadaddr2,
                            %T1_15* %blend, %T0_15* %storeaddr) {
-; CHECK: vbsl
-; CHECK: vbsl
+; CHECK: vmin.s32
+; CHECK: vmin.s32
   %v0 = load %T0_15, %T0_15* %loadaddr
   %v1 = load %T0_15, %T0_15* %loadaddr2
   %c = icmp slt %T0_15 %v0, %v1
 ; COST: func_blend15
 ; COST: cost of 82 {{.*}} select
   %r = select %T1_15 %c, %T0_15 %v0, %T0_15 %v1
   store %T0_15 %r, %T0_15* %storeaddr
   ret void