[PATCH] D69510: [ARM] MVE reverse shuffles.

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 2 inline comments as done.
dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7659
+  EVT VT = Op.getValueType();
 
+  SDValue OpLHS = DAG.getNode(ARMISD::VREV64, DL, VT, Op.getOperand(0));
----------------
samparker wrote:
> I think keeping some kind of assert is a good idea here.
Yeah, OK. Probably best. I had this going with v4i32 too, which is where this was lost. But until double moves are selected better, that doesn't improve things.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:642
+          {ISD::VECTOR_SHUFFLE, MVT::v4f32, 5},
+          {ISD::VECTOR_SHUFFLE, MVT::v8i16, 5},
+          {ISD::VECTOR_SHUFFLE, MVT::v8f16, 5},
----------------
samparker wrote:
> should we be considering v4i16 and v8i8 too?
Those 2 will be type legalised to a v4i32 and v8i16, and the final cost will use the legal type (multiplied by the type legalisation cost, which looks like it will be 1 from the tests).

Because we don't use vmovd's yet, I guess the cost of a v4i32 should be 4, not 5.  I'll update that, but they will hopefully drop down to 3 in the future, like the rest.


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

https://reviews.llvm.org/D69510