[PATCH] D115242: [ARM] Handle splats of constants for MVE qr instruction

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7720
+// operand Op.
+static bool IsQRMVEInstruction(const SDNode *N, const SDNode *Op) {
+  switch (N->getOpcode()) {
----------------
samtebbs wrote:
> Can this info be put in tablegen somehow? We might well add support for a qr instruction and forget to add it here.
I'm not sure that is very simple simple. We are using the nodes/intrinsics here, not the final instructions. So only looking at ISD::ADD or the Intrinsic::arm_mve_add_predicated intrinsic, etc.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7785
+               [BVN](const SDNode *U) { return IsQRMVEInstruction(U, BVN); })) {
+      EVT DupVT = SplatBitSize == 32   ? MVT::v4i32
+                  : SplatBitSize == 16 ? MVT::v8i16
----------------
samtebbs wrote:
> I know that a 64 bit SplatBitSize shouldn't be legal at this point, but it may be worth adding an assertion to make sure it isn't 64, just for future proofing.
The SplatBitSize is checked in the if above, so shouldn't be 64bits. It is probably worth making sure that SpaltBitSize == VT.getScalarSizeInBits(), to make sure the sizes are the same and the DUP will be able to be folded into the instruction. I'll add that in.


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

https://reviews.llvm.org/D115242