[PATCH] D67158: [ARM] Begin adding IR intrinsics for MVE instructions.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:312
+// Integer vector types that don't treat signed and unsigned differently.
+def v16i8_info : MVEVectorVTInfo<v16i8, v16i1, 0b00, "i8",  ?>;
+def v8i16_info : MVEVectorVTInfo<v8i16, v8i1,  0b01, "i16", ?>;
----------------
Maybe call these MVEv16i8? Or v16i8_t or v16i8info or something?

Otherwise it looks very useful.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-intrinsics/vaddq.ll:24
+
+define arm_aapcs_vfpcc <16 x i8> @test_vaddq_m_s8(<16 x i8> %inactive, <16 x i8> %a, <16 x i8> %b, i16 zeroext %p) {
+; CHECK-LABEL: test_vaddq_m_s8:
----------------
dmgreen wrote:
> For the rest of the tests, at least for codegen we have tried to fill in all the combinations for type and operations (at least the legal types). It can be useful for making sure nothing is missed (here or in the future when some refactoring happens).
> 
> Whether you want to do the same thing here is up to you, or whether you think that having interesting combinations is enough (adds with v16i8, subs with v4f32 for example).
This could probably do with a reply, one way or the other.

I was previously in the "don't mind either way" camp, now I feel more in the "why not just add them" camp, unless there is some reason not to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67158