[PATCH] D76292: [ARM] Change VDUP type to i32 for MVE

Thu Mar 19 03:45:01 PDT 2020

dmgreen added a comment.

In D76292#1926983 <https://reviews.llvm.org/D76292#1926983>, @efriedma wrote:

> I haven't been following the MVE work that closely, but changing the operand type of MVE vdup makes sense.  My one concern here is the potential for confusion due to the opcode; VDUP for NEON and MVE have the same opcode and result type, but the operand types are different.  Doesn't really matter much for isel patterns, but could be confusing for writing target-specific combines.

Thanks for taking a look. It is indeed interesting trying to keep two entirely independent vector architectures happy in the same backend. It seems mostly OK so far, and they have helpfully shared quite a bit of code. The creation of VDUP is one such place, where we only had to modify the existing code a little. I didn't change that code to force the type where they are created to try and keep it cleaner, and it can happen in multiple places. Hence the fold in PerformVDUPCombine.

I think I'd prefer to keep the same opcode between the two archs, unless you have some better suggestion? Even with the two input types, we can probably keep the logic in PerformVDUPCombine separate.

> Yes, PerformVMOVhrCombine could be improved to handle more cases, independent of what happens here.

I was hoping someone would be able to say "just use an X", and I was missing something obvious. But I will look into some VMOVhr combines, at least for the loads here.

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

https://reviews.llvm.org/D76292