[PATCH][AVX512] Enable intrinsics for vpcmpeq

[Adam Nemet]

On Sep 25, 2014, at 12:43 AM, Robert Khasanov <rob.khasanov at gmail.com> wrote:

> The last patch is incorrect. See this version instead of previous one.
> Sorry for that.
> 
> 2014-09-25 11:40 GMT+04:00 Robert Khasanov <rob.khasanov at gmail.com>:
> Hi Elena, Adam,
> 
> In these patches I enable intrinsics for vpcmpeq{bwdq} instructions.
> Since result of the instructions is mask, I enable new intrinsics type CMP_MASK and assume to enable CMP_MASK_CC in future for vpcmp{bwdq} instructions (due to additional CC argument).
> Also I extended argument types when intrinsics generated through TableGen (IIT_V64) to support 64 packed data.
> In last patch I enabled INSERT_SUBVECTOR for v8i1 to legalizer to support converting v2i1 and v4i1 to v8i1 and then bitcasting to i8.
> 
> Please let me know if it looks good.

Hi Robert,

+    case CMP_MASK: {
+      EVT VT = Op.getOperand(1).getValueType();
+      EVT MaskVT = EVT::getVectorVT(*DAG.getContext(), MVT::i1,
+                                    VT.getVectorNumElements());
+      SDValue Cmp = DAG.getNode(IntrData->Opc0, dl, MaskVT,
+                                Op.getOperand(1), Op.getOperand(2));
+      SDValue Res;
+      if (isAllOnes(Op.getOperand(3))) {
+        Res = Cmp;
+      } else {
+        Res = DAG.getNode(ISD::AND, dl, MaskVT, Cmp,
+                          DAG.getNode(ISD::BITCAST, dl, MaskVT,
+                                      Op.getOperand(3)));
+      }
+      return DAG.getNode(ISD::BITCAST, dl, Op.getValueType(), Res);
+    }

It would be good to reuse (and extend if needed) the helper getVectorMaskingNode for this.  That should capture what it takes from a expression to become a masked expression in one place (just like AVX512_masking in td).  Obviously the BITCAST at the end does not belong there there but the information that masked compare is canonicalized to an AND rather than a VSELECT is useful.

As you see I also don’t check for the mask value to be AllOnes in getVectorMaskingNode.  We can add that for now if you need but I think that should be an orthogonal DAGCombiner transformation to cover more cases.

+    setOperationAction(ISD::INSERT_SUBVECTOR,   MVT::v8i1, Legal);

Can you just enable it like that?  I understand that you need it to lower the intrinsics but you may need to add some more patterns to TD to recognize this in the general case.

-        Res = DAG.getNode(ISD::AND, dl, MaskVT, Cmp,
-                          DAG.getNode(ISD::BITCAST, dl, MaskVT,
-                                      Op.getOperand(3)));
+        RMask = DAG.getNode(ISD::AND, dl, MaskVT, Cmp,
+                            DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, MaskVT,
+                              DAG.getNode(ISD::BITCAST, dl, BitcastVT, Mask),
+                              DAG.getIntPtrConstant(0)));
       }
-      return DAG.getNode(ISD::BITCAST, dl, Op.getValueType(), Res);
+      return DAG.getNode(ISD::BITCAST, dl, Op.getValueType(),
+                         DAG.getNode(ISD::INSERT_SUBVECTOR, dl, BitcastVT,
+                                     DAG.getUNDEF(BitcastVT), RMask,
+                                     DAG.getIntPtrConstant(0)));

This could use a comment with an example DAG that we’re trying to create.

Thanks,
Adam


> 
> Robert
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140925/8ddb6a6f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 003_vpcmpeq_256_128.patch
Type: application/octet-stream
Size: 21820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140925/8ddb6a6f/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140925/8ddb6a6f/attachment-0001.html>