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

Fri Jul 9 07:03:31 PDT 2021

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM

================
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
----------------
dmgreen wrote:
> 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.
No, it's fine by me too, just started to wonder about it....

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

https://reviews.llvm.org/D105680