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

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

25

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

25

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

[delena]

lib/Target/X86/X86ISelLowering.cpp

24

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

25

I don't think that we need duplication here.

[igorb]

Thanks,
one minor comment, please see in code.

LGTM

lib/Target/X86/X86ISelLowering.cpp

22

please add space after //
the same bellow.

lib/Target/X86/X86ISelLowering.cpp

 
 #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"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/CodeGen/IntrinsicLowering.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
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();
 
+  bool FValIsAllZeros = ISD::isBuildVectorAllZeros(LHS.getNode());
+  //Check if the first operand is all zeros.
+  if (FValIsAllZeros &&
+      // Check if the selector will be produced by CMPP*/PCMP*.
+      Cond.getOpcode() == ISD::SETCC &&
+      N->getOpcode() == ISD::VSELECT) {
+      ISD::CondCode CC = cast<CondCodeSDNode>(Cond->getOperand(2))->get();
+      // Check if the compare instruction uses SETEQ with all zeros. 
+      if (CC == ISD::SETEQ) {
+        SDValue CC = Cond.getOperand(2);
+        ISD::CondCode NewCC = ISD::getSetCCInverse(
+                                   cast<CondCodeSDNode>(CC)->get(),
+                                   Cond.getOperand(0).getValueType().isInteger());
+        SDValue CondNew = DAG.getSetCC(DL, CondVT, Cond.getOperand(0), 
+                                       Cond.getOperand(1), NewCC);
+        return DAG.getNode(ISD::VSELECT, DL, VT, CondNew, 
+                           N->getOperand(2), N->getOperand(1));
+      }
+  }
+
   if (N->getOpcode() != ISD::VSELECT)
     return SDValue();
 
   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) ==

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
 }