[PATCH] D33097: [x86] Follow-up to r302785 - don't widen vselect conditions when the vNi1 type being split into is a legal type (like it usually is w/ AVX-512).

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 11 04:53:05 PDT 2017


chandlerc created this revision.
Herald added a subscriber: mcrosier.

The logic to widen the vselect stuff only makes sense if there are very
subtle issues where the vNi1 type used isn't generally legal but can
sneak through from a SETCC to a VSELECT because it will be matched away.
When the mask vectors are just normal vectors, we don't want to widen
them.

This would also paper over the bug fixed by r302785, because now we
never create the unfortunate pattern. I think the fix in r302785 is
still correct and important but we may no longer have a way to test it.

This does make the generated code for legalized types such as those in
r302785 much nicer, as shown in the test case.

Not really sure how folks want to proceed, so since this is just
a qualitative improvement, sending it for review to collect thoughts.


https://reviews.llvm.org/D33097

Files:
  lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
  test/CodeGen/X86/avx512-vselect.ll


Index: test/CodeGen/X86/avx512-vselect.ll
===================================================================
--- test/CodeGen/X86/avx512-vselect.ll
+++ test/CodeGen/X86/avx512-vselect.ll
@@ -28,13 +28,9 @@
 ; CHECK-SKX-NEXT:    vxorps %zmm6, %zmm6, %zmm6
 ; CHECK-SKX-NEXT:    vcmpltps %zmm0, %zmm6, %k0
 ; CHECK-SKX-NEXT:    vcmpltps %zmm6, %zmm1, %k1
-; CHECK-SKX-NEXT:    korw %k1, %k0, %k0
-; CHECK-SKX-NEXT:    kshiftrw $8, %k0, %k1
-; CHECK-SKX-NEXT:    vpmovm2q %k1, %zmm1
-; CHECK-SKX-NEXT:    vpmovm2q %k0, %zmm0
-; CHECK-SKX-NEXT:    vptestmq %zmm0, %zmm0, %k1
+; CHECK-SKX-NEXT:    korw %k1, %k0, %k1
 ; CHECK-SKX-NEXT:    vblendmpd %zmm2, %zmm4, %zmm0 {%k1}
-; CHECK-SKX-NEXT:    vptestmq %zmm1, %zmm1, %k1
+; CHECK-SKX-NEXT:    kshiftrw $8, %k1, %k1
 ; CHECK-SKX-NEXT:    vblendmpd %zmm3, %zmm5, %zmm1 {%k1}
 ; CHECK-SKX-NEXT:    retq
 ;
@@ -44,12 +40,8 @@
 ; CHECK-KNL-NEXT:    vcmpltps %zmm0, %zmm6, %k0
 ; CHECK-KNL-NEXT:    vcmpltps %zmm6, %zmm1, %k1
 ; CHECK-KNL-NEXT:    korw %k1, %k0, %k1
-; CHECK-KNL-NEXT:    kshiftrw $8, %k1, %k2
-; CHECK-KNL-NEXT:    vpternlogq $255, %zmm1, %zmm1, %zmm1 {%k2} {z}
-; CHECK-KNL-NEXT:    vpternlogq $255, %zmm0, %zmm0, %zmm0 {%k1} {z}
-; CHECK-KNL-NEXT:    vptestmq %zmm0, %zmm0, %k1
 ; CHECK-KNL-NEXT:    vblendmpd %zmm2, %zmm4, %zmm0 {%k1}
-; CHECK-KNL-NEXT:    vptestmq %zmm1, %zmm1, %k1
+; CHECK-KNL-NEXT:    kshiftrw $8, %k1, %k1
 ; CHECK-KNL-NEXT:    vblendmpd %zmm3, %zmm5, %zmm1 {%k1}
 ; CHECK-KNL-NEXT:    retq
 entry:
Index: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -3048,6 +3048,7 @@
 SDValue DAGTypeLegalizer::WidenVSELECTAndMask(SDNode *N) {
   LLVMContext &Ctx = *DAG.getContext();
   SDValue Cond = N->getOperand(0);
+  EVT CondVT = Cond.getValueType();
 
   if (N->getOpcode() != ISD::VSELECT)
     return SDValue();
@@ -3057,7 +3058,7 @@
 
   // If this is a splitted VSELECT that was previously already handled, do
   // nothing.
-  if (Cond->getValueType(0).getScalarSizeInBits() != 1)
+  if (CondVT.getScalarSizeInBits() != 1)
     return SDValue();
 
   EVT VSelVT = N->getValueType(0);
@@ -3073,7 +3074,14 @@
   if (FinalVT.getVectorNumElements() == 1)
     return SDValue();
 
-  // If there is support for an i1 vector mask, don't touch.
+  // If the resulting i1 vector mask is a fully legal type, don't widen it.
+  EVT FinalMaskVT = EVT::getVectorVT(Ctx, CondVT.getVectorElementType(),
+                                     FinalVT.getVectorNumElements());
+  if (TLI.getTypeAction(Ctx, FinalMaskVT) == TargetLowering::TypeLegal)
+    return SDValue();
+
+  // If there is support for an i1 vector mask when coming out of a SETCC node,
+  // don't touch.
   if (Cond.getOpcode() == ISD::SETCC) {
     EVT SetCCOpVT = Cond->getOperand(0).getValueType();
     while (TLI.getTypeAction(Ctx, SetCCOpVT) != TargetLowering::TypeLegal)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33097.98615.patch
Type: text/x-patch
Size: 2998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170511/11e6a2ff/attachment.bin>


More information about the llvm-commits mailing list