[PATCH] D105680: [ARM] Lower v16i8 -> i64 VMLA reductions.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16051
   auto Create64bitNode = [&](unsigned Opcode, ArrayRef<SDValue> Ops) {
+    // Split illegal MVT::v16i8->i64 vector reductions into two legal v8i16->i64
+    // reductions. The operands are extended with MVEEXT, but as they are
----------------
SjoerdMeijer wrote:
> Perhaps a silly question on the use of 'illegal'. Type `v16i8` isn't an illegal type, it is just not supported for these VMLALV instructions. Thus, I was wondering if 'unsupported' would be better, to avoid possible confusion with type legalization in general if that makes sense?
I think both class as legalization. You both legalize the types and legalize the operations. A v4i32 bitreverse is illegal, for example, even if the v4i32 type is legal. We need to turn it into a v16i8 bitreverse and a VREV32.

I don't have a super strong opinion if you think it's best to change it - I think the two words essentially mean the same thing here :) But illegal sounds fine to me.


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

https://reviews.llvm.org/D105680