[x86, AVX] allow FP vector select folding to bitwise logic ops (PR28895)

[spatel]

Summary

This handles the case in:
https://llvm.org/bugs/show_bug.cgi?id=28895

...but I suspect that we may not be getting all of the possibilities yet. Eg, we can use 'X86::FANDN' for scalar FP select combines.

[andreadb]

Hi Simon,

It looks good to me too.
Could you please also add extra test cased with ordered comparisons? I am only seeing tests for unordered comparisons in 'select-with-and-or.ll'.

Thanks for fixing this!
-Andrea

[spatel]

In D23337#510940, @andreadb wrote:

Could you please also add extra test cased with ordered comparisons? I am only seeing tests for unordered comparisons in 'select-with-and-or.ll'.

Good point - I lazily copied the existing tests here but only changed the select operands to FP types. I changed the tests to use varying predicates in:
rL278229

Let me know if you see any holes in the coverage - fun mental exercise of translating LLVM predicates to SSE predicates... :)
If there is a bug in that part, it should be independent of this patch.

[andreadb]

In D23337#511223, @spatel wrote:

In D23337#510940, @andreadb wrote:

Could you please also add extra test cased with ordered comparisons? I am only seeing tests for unordered comparisons in 'select-with-and-or.ll'.

Good point - I lazily copied the existing tests here but only changed the select operands to FP types. I changed the tests to use varying predicates in:
rL278229

Let me know if you see any holes in the coverage - fun mental exercise of translating LLVM predicates to SSE predicates... :)
If there is a bug in that part, it should be independent of this patch.

Hi Sanjay,

The test coverage seems OK.
If you really want to increase the test coverage, then you could probably add a test for fcmp ole.

Something like this:

;;;;
define <4 x float> @test_ole(<4 x float> %A, <4 x float> %B, <4 x float> %C) {
entry:

%0 = fcmp ole <4 x float> %A, %B
%1 = select <4 x i1> %0, <4 x float> %C, <4 x float> zeroinitializer
ret <4 x float> %1

}
;;;;

That said, your patch does the right thing.
For the example above, I verified that the compiler now generates a vcmpleps + vandps.

So, the patch lgtm. Thanks!

lib/Target/X86/X86ISelLowering.cpp

static SDValue combineVSelectWithAllOnesOrZeros(SDNode *N, SelectionDAG &DAG) {
   SDValue Cond = N->getOperand(0);
   SDValue LHS = N->getOperand(1);
   SDValue RHS = N->getOperand(2);
   EVT VT = LHS.getValueType();
   EVT CondVT = Cond.getValueType();
   SDLoc DL(N);
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
 
-  // Condition value type must match vselect operand type.
-  if (N->getOpcode() != ISD::VSELECT || CondVT != VT)
+  if (N->getOpcode() != ISD::VSELECT)
     return SDValue();
 
-  assert(Cond.getValueType().isVector() &&
-         "Vector select expects a vector selector!");
+  assert(CondVT.isVector() && "Vector select expects a vector selector!");
+
+  // To use the condition operand as a bitwise mask, it must have elements that
+  // are the same size as the select elements. Ie, the condition operand must
+  // have already been promoted from the IR select condition type <N x i1>.
+  // Don't check if the types themselves are equal because that excludes
+  // vector floating-point selects.
+  if (CondVT.getScalarSizeInBits() != VT.getScalarSizeInBits())
+    return SDValue();
 
   bool TValIsAllOnes = ISD::isBuildVectorAllOnes(LHS.getNode());
   bool FValIsAllZeros = ISD::isBuildVectorAllZeros(RHS.getNode());
 
   // Try to invert the condition if true value is not all 1s and false value is
   // not all 0s.
   if (!TValIsAllOnes && !FValIsAllZeros &&
       // Check if the selector will be produced by CMPP*/PCMP*.

test/CodeGen/X86/avx512-cvt.ll

; SKX-NEXT:    retq
   %a = fptrunc <4 x double> %b to <4 x float>
   ret <4 x float> %a
 }
 
 define <4 x float> @fptrunc02(<4 x double> %b, <4 x i1> %mask) {
 ; KNL-LABEL: fptrunc02:
 ; KNL:       ## BB#0:
 ; KNL-NEXT:    vpslld $31, %xmm1, %xmm1
+; KNL-NEXT:    vpsrad $31, %xmm1, %xmm1
 ; KNL-NEXT:    vcvtpd2psy %ymm0, %xmm0
-; KNL-NEXT:    vxorps %xmm2, %xmm2, %xmm2
-; KNL-NEXT:    vblendvps %xmm1, %xmm0, %xmm2, %xmm0
+; KNL-NEXT:    vpand %xmm0, %xmm1, %xmm0
 ; KNL-NEXT:    retq
 ;
 ; SKX-LABEL: fptrunc02:
 ; SKX:       ## BB#0:
 ; SKX-NEXT:    vpslld $31, %xmm1, %xmm1
 ; SKX-NEXT:    vptestmd %xmm1, %xmm1, %k1
 ; SKX-NEXT:    vcvtpd2ps %ymm0, %xmm0 {%k1} {z}
 ; SKX-NEXT:    retq
; ALL-NEXT:    retq
   ret <8 x double> %a
 }
 
 define <4 x double> @fpext01(<4 x float> %b, <4 x double>%b1, <4 x double>%a1) {
 ; KNL-LABEL: fpext01:
 ; KNL:       ## BB#0:
 ; KNL-NEXT:    vcvtps2pd %xmm0, %ymm0
 ; KNL-NEXT:    vcmpltpd %ymm2, %ymm1, %ymm1
-; KNL-NEXT:    vxorpd %ymm2, %ymm2, %ymm2
-; KNL-NEXT:    vblendvpd %ymm1, %ymm0, %ymm2, %ymm0
+; KNL-NEXT:    vandpd %ymm0, %ymm1, %ymm0
 ; KNL-NEXT:    retq
 ;
 ; SKX-LABEL: fpext01:
 ; SKX:       ## BB#0:
 ; SKX-NEXT:    vcmpltpd %ymm2, %ymm1, %k1
 ; SKX-NEXT:    vcvtps2pd %xmm0, %ymm0 {%k1} {z}
 ; SKX-NEXT:    retq
   %a = fpext <4 x float> %b to <4 x double>

test/CodeGen/X86/select-with-and-or.ll

 
 ; FIXME: None of these should use vblendvpd.
 ; Repeat all with FP types.
 
 define <2 x double> @test1f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test1f:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm2, %xmm1, %xmm0
+; CHECK-NEXT:    vandpd %xmm2, %xmm0, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> %c, <2 x double> zeroinitializer
   ret <2 x double> %r
 }
 
 define <2 x double> @test2f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test2f:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm1, %xmm2, %xmm0
+; CHECK-NEXT:    vorpd %xmm2, %xmm0, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> <double 0xffffffffffffffff, double 0xffffffffffffffff>, <2 x double> %c
   ret <2 x double> %r
 }
 
 define <2 x double> @test3f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test3f:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm1, %xmm2, %xmm0
+; CHECK-NEXT:    vcmplepd %xmm0, %xmm1, %xmm0
+; CHECK-NEXT:    vandpd %xmm2, %xmm0, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> zeroinitializer, <2 x double> %c
   ret <2 x double> %r
 }
 
 define <2 x double> @test4f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test4f:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm2, %xmm1, %xmm0
+; CHECK-NEXT:    vcmplepd %xmm0, %xmm1, %xmm0
+; CHECK-NEXT:    vorpd %xmm2, %xmm0, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> %c, <2 x double> <double 0xffffffffffffffff, double 0xffffffffffffffff>
   ret <2 x double> %r
 }
 
 define <2 x double> @test5f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test5f:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vxorpd %xmm2, %xmm2, %xmm2
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm1, %xmm2, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> <double 0xffffffffffffffff, double 0xffffffffffffffff>, <2 x double> zeroinitializer
   ret <2 x double> %r
 }
 
 define <2 x double> @test6f(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
 ; CHECK-LABEL: test6f:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vpcmpeqd %xmm2, %xmm2, %xmm2
-; CHECK-NEXT:    vblendvpd %xmm0, %xmm1, %xmm2, %xmm0
+; CHECK-NEXT:    vcmplepd %xmm0, %xmm1, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %r = select <2 x i1> %f, <2 x double> zeroinitializer, <2 x double> <double 0xffffffffffffffff, double 0xffffffffffffffff>
   ret <2 x double> %r
 }
 
 define <2 x double> @test7f(<2 x double> %a, <2 x double> %b, <2 x double>* %p) {
 ; CHECK-LABEL: test7f:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vblendvpd %xmm0, (%rdi), %xmm1, %xmm0
+; CHECK-NEXT:    vandpd (%rdi), %xmm0, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %f = fcmp ult <2 x double> %a, %b
   %l = load <2 x double>, <2 x double>* %p, align 16
   %r = select <2 x i1> %f, <2 x double> %l, <2 x double> zeroinitializer
   ret <2 x double> %r
 }