[x86] eliminate even more sign-bit tests with vector select

[spatel]

Summary

This shortcoming was noted in D47330, and the test diffs show we already had other examples where we failed to fold to a SHRUNKBLEND:

/// Dynamic (non-constant condition) vector blend where only the sign bits
/// of the condition elements are used. This is used to enforce that the
/// condition mask is not valid for generic VSELECT optimizations.

This patch implements an idea from D48043 and would obsolete that patch because it catches more cases (notable the AVX1 case that was missed there). All we're doing is allowing the existing transform to fire more often by removing the post-legalize constraint. All of the relevant feature checks and other predicates are left as-is.

[RKSimon]

LGTM

lib/Target/X86/X86ISelLowering.cpp

 
 /// If this is a *dynamic* select (non-constant condition) and we can match
 /// this node with one of the variable blend instructions, restructure the
 /// condition so that blends can use the high (sign) bit of each element.
 static SDValue combineVSelectToShrunkBlend(SDNode *N, SelectionDAG &DAG,
                                            TargetLowering::DAGCombinerInfo &DCI,
                                            const X86Subtarget &Subtarget) {
   SDValue Cond = N->getOperand(0);
-  if (N->getOpcode() != ISD::VSELECT || !DCI.isBeforeLegalizeOps() ||
-      DCI.isBeforeLegalize() ||
+  if (N->getOpcode() != ISD::VSELECT ||
       ISD::isBuildVectorOfConstantSDNodes(Cond.getNode()))
     return SDValue();
 
-  // Don't optimize vector selects that map to mask-registers.
+  // Don't optimize before the condition has been transformed to a legal type
+  // and don't ever optimize vector selects that map to AVX512 mask-registers.
   unsigned BitWidth = Cond.getScalarValueSizeInBits();
-  if (BitWidth == 1)
-    return SDValue();
+  if (BitWidth < 8 || BitWidth > 64)
 
   // We can only handle the cases where VSELECT is directly legal on the
   // subtarget. We custom lower VSELECT nodes with constant conditions and
   // this makes it hard to see whether a dynamic VSELECT will correctly
   // lower, so we both check the operation's status and explicitly handle the
   // cases where a *dynamic* blend will fail even though a constant-condition
   // blend could be custom lowered.
   // FIXME: We should find a better way to handle this class of problems.
if (VT.is512BitVector())
     return SDValue();
 
   // TODO: Add other opcodes eventually lowered into BLEND.
   for (SDNode::use_iterator UI = Cond->use_begin(), UE = Cond->use_end();
        UI != UE; ++UI)
     if (UI->getOpcode() != ISD::VSELECT || UI.getOperandNo() != 0)
       return SDValue();
 
-  assert(BitWidth >= 8 && BitWidth <= 64 && "Invalid mask size");
   APInt DemandedMask(APInt::getSignMask(BitWidth));
   KnownBits Known;
   TargetLowering::TargetLoweringOpt TLO(DAG, !DCI.isBeforeLegalize(),
                                         !DCI.isBeforeLegalizeOps());
   if (!TLI.SimplifyDemandedBits(Cond, DemandedMask, Known, TLO, 0, true))
     return SDValue();
 
   // If we changed the computation somewhere in the DAG, this change will

test/CodeGen/X86/vsel-cmp-load.ll

 
 define <8 x i32> @slt_zero(<8 x i8>* %p, <8 x i32> %x, <8 x i32> %y) {
 ; AVX1-LABEL: slt_zero:
 ; AVX1:       # %bb.0:
 ; AVX1-NEXT:    vpmovsxbw (%rdi), %xmm2
 ; AVX1-NEXT:    vpmovsxwd %xmm2, %xmm3
 ; AVX1-NEXT:    vpshufd {{.*#+}} xmm2 = xmm2[2,3,0,1]
 ; AVX1-NEXT:    vpmovsxwd %xmm2, %xmm2
-; AVX1-NEXT:    vpxor %xmm4, %xmm4, %xmm4
-; AVX1-NEXT:    vpcmpgtd %xmm2, %xmm4, %xmm2
-; AVX1-NEXT:    vpcmpgtd %xmm3, %xmm4, %xmm3
 ; AVX1-NEXT:    vinsertf128 $1, %xmm2, %ymm3, %ymm2
 ; AVX1-NEXT:    vblendvps %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX1-NEXT:    retq
 ;
 ; AVX2-LABEL: slt_zero:
 ; AVX2:       # %bb.0:
 ; AVX2-NEXT:    vpmovsxbd (%rdi), %ymm2
-; AVX2-NEXT:    vpxor %xmm3, %xmm3, %xmm3
-; AVX2-NEXT:    vpcmpgtd %ymm2, %ymm3, %ymm2
 ; AVX2-NEXT:    vblendvps %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX2-NEXT:    retq
 ;
 ; AVX512-LABEL: slt_zero:
 ; AVX512:       # %bb.0:
 ; AVX512-NEXT:    vpmovsxbw (%rdi), %xmm2
 ; AVX512-NEXT:    vpmovw2m %xmm2, %k1
 ; AVX512-NEXT:    vpblendmd %ymm0, %ymm1, %ymm0 {%k1}

 ; AVX512-NEXT:    vblendmpd %ymm0, %ymm1, %ymm0 {%k1}
 ; AVX512-NEXT:    retq
   %load = load <4 x i8>, <4 x i8>* %p
   %cmp = icmp sgt <4 x i8> %load, zeroinitializer
   %sel = select <4 x i1> %cmp, <4 x double> %x, <4 x double> %y
   ret <4 x double> %sel
 }
 
-; FIXME: The compare with 0 for AVX2 should be eliminated.
-
 define <8 x float> @slt_zero_fp_select(<8 x i16>* %p, <8 x float> %x, <8 x float> %y) {
 ; AVX1-LABEL: slt_zero_fp_select:
 ; AVX1:       # %bb.0:
 ; AVX1-NEXT:    vpmovsxwd 8(%rdi), %xmm2
 ; AVX1-NEXT:    vpmovsxwd (%rdi), %xmm3
 ; AVX1-NEXT:    vinsertf128 $1, %xmm2, %ymm3, %ymm2
 ; AVX1-NEXT:    vblendvps %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX1-NEXT:    retq
 ;
 ; AVX2-LABEL: slt_zero_fp_select:
 ; AVX2:       # %bb.0:
 ; AVX2-NEXT:    vpmovsxwd (%rdi), %ymm2
-; AVX2-NEXT:    vpxor %xmm3, %xmm3, %xmm3
-; AVX2-NEXT:    vpcmpgtd %ymm2, %ymm3, %ymm2
 ; AVX2-NEXT:    vblendvps %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX2-NEXT:    retq
 ;
 ; AVX512-LABEL: slt_zero_fp_select:
 ; AVX512:       # %bb.0:
 ; AVX512-NEXT:    vpxor %xmm2, %xmm2, %xmm2
 ; AVX512-NEXT:    vpcmpgtw (%rdi), %xmm2, %k1
 ; AVX512-NEXT:    vblendmps %ymm0, %ymm1, %ymm0 {%k1}
 ; AVX512-NEXT:    retq
   %load = load <8 x i16>, <8 x i16>* %p
   %cmp = icmp slt <8 x i16> %load, zeroinitializer
   %sel = select <8 x i1> %cmp, <8 x float> %x, <8 x float> %y
   ret <8 x float> %sel
 }
 

test/CodeGen/X86/vselect-pcmp.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx       | FileCheck %s --check-prefix=AVX --check-prefix=AVX12F --check-prefix=AVX12 --check-prefix=AVX1
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx2      | FileCheck %s --check-prefix=AVX --check-prefix=AVX12F --check-prefix=AVX12 --check-prefix=AVX2
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx512f   | FileCheck %s --check-prefix=AVX --check-prefix=AVX12F --check-prefix=AVX512 --check-prefix=AVX512F
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx512vl  | FileCheck %s --check-prefix=AVX                       --check-prefix=AVX512 --check-prefix=AVX512VL
 
 ; The condition vector for BLENDV* only cares about the sign bit of each element.
 ; So in these tests, if we generate BLENDV*, we should be able to remove the redundant cmp op.
 
 ; Test 128-bit vectors for all legal element types.
 
-; FIXME: Why didn't AVX-512 optimize too?
-
 define <16 x i8> @signbit_sel_v16i8(<16 x i8> %x, <16 x i8> %y, <16 x i8> %mask) {
-; AVX12-LABEL: signbit_sel_v16i8:
-; AVX12:       # %bb.0:
-; AVX12-NEXT:    vpblendvb %xmm2, %xmm0, %xmm1, %xmm0
-; AVX12-NEXT:    retq
-;
-; AVX512-LABEL: signbit_sel_v16i8:
-; AVX512:       # %bb.0:
-; AVX512-NEXT:    vpxor %xmm3, %xmm3, %xmm3
-; AVX512-NEXT:    vpcmpgtb %xmm2, %xmm3, %xmm2
-; AVX512-NEXT:    vpblendvb %xmm2, %xmm0, %xmm1, %xmm0
-; AVX512-NEXT:    retq
+; AVX-LABEL: signbit_sel_v16i8:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vpblendvb %xmm2, %xmm0, %xmm1, %xmm0
+; AVX-NEXT:    retq
   %tr = icmp slt <16 x i8> %mask, zeroinitializer
   %z = select <16 x i1> %tr, <16 x i8> %x, <16 x i8> %y
   ret <16 x i8> %z
 }
 
 ; Sorry 16-bit, you're not important enough to support?
 
 define <8 x i16> @signbit_sel_v8i16(<8 x i16> %x, <8 x i16> %y, <8 x i16> %mask) {

 ;
 ; AVX2-LABEL: signbit_sel_v32i8:
 ; AVX2:       # %bb.0:
 ; AVX2-NEXT:    vpblendvb %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX2-NEXT:    retq
 ;
 ; AVX512-LABEL: signbit_sel_v32i8:
 ; AVX512:       # %bb.0:
-; AVX512-NEXT:    vpxor %xmm3, %xmm3, %xmm3
-; AVX512-NEXT:    vpcmpgtb %ymm2, %ymm3, %ymm2
 ; AVX512-NEXT:    vpblendvb %ymm2, %ymm0, %ymm1, %ymm0
 ; AVX512-NEXT:    retq
   %tr = icmp slt <32 x i8> %mask, zeroinitializer
   %z = select <32 x i1> %tr, <32 x i8> %x, <32 x i8> %y
   ret <32 x i8> %z
 }
 
 ; Sorry 16-bit, you'll never be important enough to support?