[X86] Convert X86ISD::SELECT to ISD::VSELECT just before instruction selection to avoid duplicate patterns

[craig.topper]

Summary

Similar to what we do for X86ISD::SHRUNKBLEND just turn X86ISD::SELECT into ISD::VSELECT. This allows us to remove the duplicated TRUNC patterns.

[delena]

Please add a test.

[craig.topper]

What sort of test are you looking for? The truncate intrinsic tests should already cover this.

[delena]

Ok. This is NFC. Why we can't generate (or stay with) ISD::SELECT from the beginning?

[craig.topper]

Here's the comment from getVectorMaskingNode

case X86ISD::VTRUNC:
case X86ISD::VTRUNCS:
case X86ISD::VTRUNCUS:
case X86ISD::CVTPS2PH:
  // We can't use ISD::VSELECT here because it is not always "Legal"
  // for the destination type. For example vpmovqb require only AVX512
  // and vselect that can operate on byte element type require BWI
  OpcodeSelect = X86ISD::SELECT;
  break;
}

[delena]

Yes, I saw this comment. We create a custom node in order to avoid an additional "entrance" to LowerSELECT(), right?
In general, this approach is generic - create a custom node to stop the lowering process.
If we'll keep a table of custom-to-common last-minute replacement, it'd be, probably, more robust.
It may be implemented later.

X86select node is not used any more, it can be removed from td patterns.

[craig.topper]

I did remove the usages from the td patterns. It wasn't used in very many places.

The true side of the ternary in this code also looks dead. We never get this far for vector types. So we never select X86ISD::SELECT. But its not strictly related to the goal of this patch.

if (Subtarget.hasAVX512()) {
  SDValue Cmp = DAG.getNode(X86ISD::FSETCCM, DL, MVT::v1i1, CondOp0,
                            CondOp1, DAG.getConstant(SSECC, DL, MVT::i8));
  return DAG.getNode(VT.isVector() ? X86ISD::SELECT : X86ISD::SELECTS,
                     DL, VT, Cmp, Op1, Op2);
}

[delena]

In D38022#874828, @craig.topper wrote:

I did remove the usages from the td patterns. It wasn't used in very many places.

I meant X86InstrFragmentsSIMD.td.

[craig.topper]

Remove from X86InstrFragments.td

lib/Target/X86/X86ISelDAGToDAG.cpp

if (Subtarget->isTarget64BitILP32()) {
       return;
     }
     break;
   }
   case X86ISD::GlobalBaseReg:
     ReplaceNode(Node, getGlobalBaseReg());
     return;
 
+  case X86ISD::SELECT:
   case X86ISD::SHRUNKBLEND: {
-    // SHRUNKBLEND selects like a regular VSELECT.
+    // SHRUNKBLEND selects like a regular VSELECT. Same with X86ISD::SELECT.
     SDValue VSelect = CurDAG->getNode(
         ISD::VSELECT, SDLoc(Node), Node->getValueType(0), Node->getOperand(0),
         Node->getOperand(1), Node->getOperand(2));
     ReplaceNode(Node, VSelect.getNode());
     SelectCode(VSelect.getNode());
     // We already called ReplaceUses.
     return;
   }

lib/Target/X86/X86InstrAVX512.td

 
 multiclass avx512_cvtps2ph<X86VectorVTInfo _dest, X86VectorVTInfo _src,
                            X86MemOperand x86memop> {
   defm rr : AVX512_maskable<0x1D, MRMDestReg, _dest ,(outs _dest.RC:$dst),
                    (ins _src.RC:$src1, i32u8imm:$src2),
                    "vcvtps2ph", "$src2, $src1", "$src1, $src2",
                    (X86cvtps2ph (_src.VT _src.RC:$src1),
                                 (i32 imm:$src2)),
-                   NoItinerary, 0, 0, X86select>, AVX512AIi8Base;
+                   NoItinerary, 0, 0>, AVX512AIi8Base;
   def mr : AVX512AIi8<0x1D, MRMDestMem, (outs),
              (ins x86memop:$dst, _src.RC:$src1, i32u8imm:$src2),
              "vcvtps2ph\t{$src2, $src1, $dst|$dst, $src1, $src2}",
              [(store (_dest.VT (X86cvtps2ph (_src.VT _src.RC:$src1),
                                      (i32 imm:$src2))),
                                      addr:$dst)]>;
   let hasSideEffects = 0, mayStore = 1 in
   def mrk : AVX512AIi8<0x1D, MRMDestMem, (outs),
multiclass avx512_trunc_common<bits<8> opc, string OpcodeStr, SDNode OpNode,
                               X86VectorVTInfo SrcInfo, X86VectorVTInfo DestInfo,
                               X86MemOperand x86memop> {
   let ExeDomain = DestInfo.ExeDomain in
   defm rr  : AVX512_maskable<opc, MRMDestReg, DestInfo, (outs DestInfo.RC:$dst),
                       (ins SrcInfo.RC:$src1), OpcodeStr ,"$src1", "$src1",
                       (DestInfo.VT (OpNode (SrcInfo.VT SrcInfo.RC:$src1)))>,
                        EVEX, T8XS;
 
-  // for intrinsic patter match
-  def : Pat<(DestInfo.VT (X86select DestInfo.KRCWM:$mask,
-                           (DestInfo.VT (OpNode (SrcInfo.VT SrcInfo.RC:$src1))),
-                           DestInfo.ImmAllZerosV)),
-            (!cast<Instruction>(NAME#SrcInfo.ZSuffix##rrkz) DestInfo.KRCWM:$mask ,
-                                      SrcInfo.RC:$src1)>;
-
-  def : Pat<(DestInfo.VT (X86select DestInfo.KRCWM:$mask,
-                           (DestInfo.VT (OpNode (SrcInfo.VT SrcInfo.RC:$src1))),
-                           DestInfo.RC:$src0)),
-            (!cast<Instruction>(NAME#SrcInfo.ZSuffix##rrk) DestInfo.RC:$src0,
-                                      DestInfo.KRCWM:$mask ,
-                                      SrcInfo.RC:$src1)>;
-
   let mayStore = 1, mayLoad = 1, hasSideEffects = 0,
       ExeDomain = DestInfo.ExeDomain in {
     def mr : AVX512XS8I<opc, MRMDestMem, (outs),
                (ins x86memop:$dst, SrcInfo.RC:$src),
                OpcodeStr # "\t{$src, $dst|$dst, $src}",
                []>, EVEX;
 
     def mrk : AVX512XS8I<opc, MRMDestMem, (outs),