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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
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
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > what do you mean with the "real predicate"?
> The predicate may be a v4i1 or a v8i1. We cast it to a v16i1 to get the types right, using the principle that a v4i1 of abcd and a v16i1 of aaaabbbbccccdddd are the same thing.
Ah okay, cheers, got it.
Perhaps you just want to add this to the comments instead of the "If the real predicate is not v16i1 ".


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7398
+  SDValue AllOnes =
+      DAG.getTargetConstant(ARM_AM::createNEONModImm(0xe, 0xff), dl, MVT::i32);
+  AllOnes = DAG.getNode(ARMISD::VMOVIMM, dl, MVT::v16i8, AllOnes);
----------------
> I can rename it easily enough. createVModImm sound OK?

Yep, sounds good


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7666
+  unsigned Lane = cast<ConstantSDNode>(Op.getOperand(2))->getZExtValue();
+  unsigned LaneWidth = 16 / VecVT.getVectorNumElements();
+  unsigned Mask = ((1 << LaneWidth) - 1) << Lane * LaneWidth;
----------------
Perhaps better to hide magic constant 16 in a MVE subtarget function or something like that?


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

https://reviews.llvm.org/D65052