[PATCH] D65052: [ARM] MVE predicate register support

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Just a first scan, I will look more closely tomorrow.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:6720
+
+  assert(ST->hasMVEIntegerOps());
+
----------------
perhaps a message in this assert.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7385
+  SDValue AllOnes =
+      DAG.getTargetConstant(ARM_AM::createNEONModImm(0xe, 0xff), dl, MVT::i32);
+  AllOnes = DAG.getNode(ARMISD::VMOVIMM, dl, MVT::v16i8, AllOnes);
----------------
is it weird to call `createNEONModImm` here?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7408
+  SDValue RecastV1;
+  // If the real predicate is not v16i1 then we need to recast this to a
+  // v16i1. This cannot be done with an ordinary bitcast because the sizes
----------------
what do you mean with the "real predicate"?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7679
+
+  if (Op.getValueType().getScalarSizeInBits() == 1)
+    return LowerINSERT_VECTOR_ELT_i1(Op, DAG);
----------------
do we want to check or assert that we're targeting MVE here?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7787
+  unsigned j = 0;
+  for (unsigned i = 0; i < NewOp1VT.getVectorNumElements(); i++, j++) {
+    SDValue Elt = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::i32, NewV1,
----------------
Can this be a little helper function/lambda so that we  don't duplicate this below for Op2?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7870
+  SDValue SubVec = DAG.getNode(ISD::UNDEF, dl, SubVT);
+  for (unsigned i = Index, j = 0; i < (Index + NumElts); i++, j++) {
+    SDValue Elt = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::i32, NewV1,
----------------
and this (plus the switch above) looks suspiciously similar, except that the node is a ISD::EXTRACT_VECTOR_ELT here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65052/new/

https://reviews.llvm.org/D65052