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

27622 ↗

(On Diff #79387)

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 ↗

(On Diff #79441)

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

test/CodeGen/X86/avx512-vbroadcast.ll

223 ↗

(On Diff #79441)

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

[igorb]

test/CodeGen/X86/avx512-vbroadcast.ll

223 ↗

(On Diff #79441)

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 ↗

(On Diff #79441)

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 ↗

(On Diff #80272)

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

27575 ↗

(On Diff #79441)

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 ↗

(On Diff #80408)

please add space after //
the same bellow.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

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() && Cond.hasOneUse()) {
+      //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() &&

llvm/trunk/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
 }