Fix bug 30945- [AVX512] Failure to flip vector comparison to remove not mask instruction

[m_zuckerman]

Summary

Adding new optimization opportunity by adding new X86ISelLowering pattern. The test case was shown in https://llvm.org/bugs/show_bug.cgi?id=30945.

Test explanation:
Select gets three arguments mask, op and op2. In this case, the Mask is a result of ICMP. The ICMP instruction compares (with equal operand) the zero initializer vector and the result of the first ICMP.

In general, The result of "cmp eq, op1, zero initializers" is "not(op1)" where op1 is a mask. By rearranging of the two arguments inside the Select instruction, we can get the same result. Without the necessary of the middle phase ("cmp eq, op1, zero initializers").

Missed optimization opportunity:
vpcmpled %zmm0, %zmm1, %k0
knotw %k0, %k1

can be combine to
vpcmpgtd %zmm0, %zmm2, %k1

[igorb]

please update summery, add more details.

lib/Target/X86/X86ISelLowering.cpp

16

please add comments.
make this transformation only for AVX512 ( you may benefit only if instruction with zero mask exist)

[delena]

lib/Target/X86/X86ISelLowering.cpp

27575

You check N->getOpcode() == ISD::VSELECT 2 lines above.

test/CodeGen/X86/avx512-vbroadcast.ll

223

I think that the logic here is broken. %k1 should be a result of unordered cmp.

[igorb]

test/CodeGen/X86/avx512-vbroadcast.ll

223

I belive the code is correct.
In this case Vselect condition has more than one use, could you please check why/when the second cmp was created instead KNOT.

[m_zuckerman]

lib/Target/X86/X86ISelLowering.cpp

27575

Yes I know, but if someone will delete this line I have the assert to check it also.

[delena]

lib/Target/X86/X86ISelLowering.cpp

27574

I'd put Cond.hasOneUse() here, not in the common code. I assume that we'll need all patterns back in this case.

27575

I don't think that we need duplication here.

[igorb]

Thanks,
one minor comment, please see in code.

LGTM

lib/Target/X86/X86ISelLowering.cpp

27572

please add space after //
the same bellow.

lib/Target/X86/X86ISelLowering.cpp

 //===-- X86ISelLowering.cpp - X86 DAG Lowering Implementation -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the interfaces that X86 uses to lower LLVM code into a
 // selection DAG.
 //
 //===----------------------------------------------------------------------===//
 
 #include "X86ISelLowering.h"
 #include "Utils/X86ShuffleDecode.h"
 #include "X86CallingConv.h"
 #include "X86FrameLowering.h"
 #include "X86InstrBuilder.h"
 #include "X86MachineFunctionInfo.h"
 #include "X86ShuffleDecodeConstantPool.h"
 #include "X86TargetMachine.h"
 #include "X86TargetObjectFile.h"
 #include "llvm/ADT/SmallBitVector.h"
static SDValue combineExtractVectorElt(SDNode *N, SelectionDAG &DAG,
   }
 
   // The replacement was made in place; don't return anything.
   return SDValue();
 }
 
 /// If a vector select has an operand that is -1 or 0, simplify the select to a
 /// bitwise logic operation.
-static SDValue combineVSelectWithAllOnesOrZeros(SDNode *N, SelectionDAG &DAG) {
+static SDValue combineVSelectWithAllOnesOrZeros(SDNode *N, SelectionDAG &DAG,
+                                                const X86Subtarget &Subtarget) {
   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();
 
   if (N->getOpcode() != ISD::VSELECT)
     return SDValue();
 
+  bool FValIsAllZeros = ISD::isBuildVectorAllZeros(LHS.getNode());
+  //Check if the first operand is all zeros.This situation only 
+  //applies to avx512.
+  if (FValIsAllZeros && Subtarget.hasAVX512()) {
+    assert((N->getOpcode() == ISD::VSELECT) && "expects a vector selector!");
+    //Invert the cond to not(cond) : xor(op,allones)=not(op)
+    SDValue CondNew = DAG.getNode(ISD::XOR, DL, Cond.getValueType(), Cond, 
+                                  DAG.getConstant(1, DL, Cond.getValueType()));
+    //Vselect cond, op1, op2 = Vselect not(cond), op2, op1
+    return DAG.getNode(ISD::VSELECT, DL, VT, CondNew, RHS, LHS);
+  }
+
   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());
+  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*.
       Cond.getOpcode() == ISD::SETCC &&
       // Check if SETCC has already been promoted.
       TLI.getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), VT) ==
if (Other.getNode() && Other->getNumOperands() == 2 &&
             // don't rely on particular values of undef lanes.
             return DAG.getNode(
                 X86ISD::SUBUS, DL, VT, OpLHS,
                 DAG.getConstant(OpRHSConst->getAPIntValue(), DL, VT));
         }
     }
   }
 
-  if (SDValue V = combineVSelectWithAllOnesOrZeros(N, DAG))
+  if (SDValue V = combineVSelectWithAllOnesOrZeros(N, DAG, Subtarget))
     return V;
 
   // 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 the blends can use the high bit of each element and use
   // SimplifyDemandedBits to simplify the condition operand.
   if (N->getOpcode() == ISD::VSELECT && DCI.isBeforeLegalizeOps() &&
       !DCI.isBeforeLegalize() &&

lib/Target/X86/X86InstrAVX512.td

 def VMOVAPSZ256mr_NOVLX : I<0, Pseudo, (outs), (ins f256mem:$dst, VR256X:$src),
                             "", []>;
 def VMOVUPSZ128mr_NOVLX : I<0, Pseudo, (outs), (ins f128mem:$dst, VR128X:$src),
                             "", []>;
 def VMOVUPSZ256mr_NOVLX : I<0, Pseudo, (outs), (ins f256mem:$dst, VR256X:$src),
                             "", []>;
 }
 
-def : Pat<(v8i64 (vselect VK8WM:$mask, (bc_v8i64 (v16i32 immAllZerosV)),
-                          (v8i64 VR512:$src))),
-   (VMOVDQA64Zrrkz (COPY_TO_REGCLASS (KNOTWrr (COPY_TO_REGCLASS VK8:$mask, VK16)),
-                                              VK8), VR512:$src)>;
-
-def : Pat<(v16i32 (vselect VK16WM:$mask, (v16i32 immAllZerosV),
-                           (v16i32 VR512:$src))),
-                  (VMOVDQA32Zrrkz (KNOTWrr VK16WM:$mask), VR512:$src)>;
-
-// These patterns exist to prevent the above patterns from introducing a second
-// mask inversion when one already exists.
-def : Pat<(v8i64 (vselect (xor VK8:$mask, (v8i1 immAllOnesV)),
-                          (bc_v8i64 (v16i32 immAllZerosV)),
-                          (v8i64 VR512:$src))),
-                 (VMOVDQA64Zrrkz VK8:$mask, VR512:$src)>;
-def : Pat<(v16i32 (vselect (xor VK16:$mask, (v16i1 immAllOnesV)),
-                           (v16i32 immAllZerosV),
-                           (v16i32 VR512:$src))),
-                  (VMOVDQA32Zrrkz VK16WM:$mask, VR512:$src)>;
-
 let Predicates = [HasVLX, NoBWI] in {
   // 128-bit load/store without BWI.
   def : Pat<(alignedstore (v8i16 VR128:$src), addr:$dst),
             (VMOVDQA32Z128mr addr:$dst, VR128:$src)>;
   def : Pat<(alignedstore (v16i8 VR128:$src), addr:$dst),
             (VMOVDQA32Z128mr addr:$dst, VR128:$src)>;
   def : Pat<(store (v8i16 VR128:$src), addr:$dst),
             (VMOVDQU32Z128mr addr:$dst, VR128:$src)>;

test/CodeGen/X86/avx512-vbroadcast.ll

; ALL-NEXT:    retq
   ret <16 x float> %b
 }
 
 define <16 x i32> @test_vbroadcast() {
 ; ALL-LABEL: test_vbroadcast:
 ; ALL:       # BB#0: # %entry
 ; ALL-NEXT:    vpxord %zmm0, %zmm0, %zmm0
 ; ALL-NEXT:    vcmpunordps %zmm0, %zmm0, %k1
-; ALL-NEXT:    vpternlogd $255, %zmm0, %zmm0, %zmm0
-; ALL-NEXT:    vmovdqa32 %zmm0, %zmm0 {%k1} {z}
-; ALL-NEXT:    knotw %k1, %k1
-; ALL-NEXT:    vmovdqa32 %zmm0, %zmm0 {%k1} {z}
+; ALL-NEXT:    vpternlogd $255, %zmm1, %zmm1, %zmm1
+; ALL-NEXT:    vmovdqa32 %zmm1, %zmm1 {%k1} {z}
+; ALL-NEXT:    vcmpordps %zmm0, %zmm0, %k1
+; ALL-NEXT:    vmovdqa32 %zmm1, %zmm0 {%k1} {z}
 ; ALL-NEXT:    retq
 entry:
   %0 = sext <16 x i1> zeroinitializer to <16 x i32>
   %1 = fcmp uno <16 x float> undef, zeroinitializer
   %2 = sext <16 x i1> %1 to <16 x i32>
   %3 = select <16 x i1> %1, <16 x i32> %0, <16 x i32> %2
   ret <16 x i32> %3
 }

test/CodeGen/X86/avx512-vec-cmp.ll

; CHECK-NEXT:    retq
   %cmpvector_i = fcmp oeq <16 x float> %a, %b
   %conv = zext <16 x i1> %cmpvector_i to <16 x i32>
   ret <16 x i32> %conv
 }
 
 define <16 x i32> @test14(<16 x i32>%a, <16 x i32>%b) {
 ; CHECK-LABEL: test14:
 ; CHECK:       ## BB#0:
-; CHECK-NEXT:    vpsubd %zmm1, %zmm0, %zmm1
-; CHECK-NEXT:    vpcmpled %zmm0, %zmm1, %k0
-; CHECK-NEXT:    knotw %k0, %k1
-; CHECK-NEXT:    vmovdqa32 %zmm1, %zmm0 {%k1} {z}
+; CHECK-NEXT:    vpsubd %zmm1, %zmm0, %zmm2
+; CHECK-NEXT:    vpcmpgtd %zmm0, %zmm2, %k1
+; CHECK-NEXT:    vpsubd %zmm1, %zmm0, %zmm0 {%k1} {z}
 ; CHECK-NEXT:    retq
   %sub_r = sub <16 x i32> %a, %b
   %cmp.i2.i = icmp sgt <16 x i32> %sub_r, %a
   %sext.i3.i = sext <16 x i1> %cmp.i2.i to <16 x i32>
   %mask = icmp eq <16 x i32> %sext.i3.i, zeroinitializer
   %res = select <16 x i1> %mask, <16 x i32> zeroinitializer, <16 x i32> %sub_r
   ret <16 x i32>%res
 }
 
 define <8 x i64> @test15(<8 x i64>%a, <8 x i64>%b) {
 ; CHECK-LABEL: test15:
 ; CHECK:       ## BB#0:
-; CHECK-NEXT:    vpsubq %zmm1, %zmm0, %zmm1
-; CHECK-NEXT:    vpcmpleq %zmm0, %zmm1, %k0
-; CHECK-NEXT:    knotw %k0, %k1
-; CHECK-NEXT:    vmovdqa64 %zmm1, %zmm0 {%k1} {z}
+; CHECK-NEXT:    vpsubq %zmm1, %zmm0, %zmm2
+; CHECK-NEXT:    vpcmpgtq %zmm0, %zmm2, %k1
+; CHECK-NEXT:    vpsubq %zmm1, %zmm0, %zmm0 {%k1} {z}
 ; CHECK-NEXT:    retq
   %sub_r = sub <8 x i64> %a, %b
   %cmp.i2.i = icmp sgt <8 x i64> %sub_r, %a
   %sext.i3.i = sext <8 x i1> %cmp.i2.i to <8 x i64>
   %mask = icmp eq <8 x i64> %sext.i3.i, zeroinitializer
   %res = select <8 x i1> %mask, <8 x i64> zeroinitializer, <8 x i64> %sub_r
   ret <8 x i64>%res
 }