[AArch64][CodeGen] Add AArch64 support for complex deinterleaving

[NickGuy]

Added context missing from initial patch

[dmgreen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

23293

I think ComplxNum requires NEON, so checking for the first should be enough.

23340

There is quite a lot of debug message left here. I assume it will be removed in the final version?

llvm/test/CodeGen/AArch64/ComplexArithmetic/complex-arithmetic-f16-add.ll

1 ↗

(On Diff #442050)

These don't need to be in a "ComplexArithmetic" directory - they are already easy to identify from the file prefix.

6 ↗

(On Diff #442050)

Can you try and clean up these newlines too. There are gaps in places, but some of the functions are a little jammed together.

[NickGuy]

Addressed comments, cleaned up debug outputs, and implemented vector splitting

[NickGuy]

Addressed comments, and rebased to align with latest revision of D114174 (Diff 16)

[dmgreen]

Can you add tests cases for some of the awkward cases we found in D114174, like the <12 x > vectors (if they are not present already).

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

23292

Can you add some extra newlines between methods

23293

This I don't think needs the NEON check.

23308

Formatting, here and below.

23323

Add a message to the assert

[NickGuy]

Addressed comments, and added more tests

[SjoerdMeijer]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

23330

It isn't clear to me why TyWidth can have any value greater than 128. Don't we expect it to be a multiple of something?

23344

Accumulator always has a value at this point, don't need to check it?

[NickGuy]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

23330

We do, but it's also backed up by the if ((VTyWidth < 128 && VTyWidth != 64) || !llvm::isPowerOf2_32(VTyWidth)) above. Though changing the assert to reflect this restriction might be best, rather than assuming that it's met.

23344

This check is needed, however the previous assignment is not. I've moved it down to where it is needed

[SjoerdMeijer]

Cool, thanks.
Looks like a nice change to me. Will leave further reviews/approval up to Dave.

[dmgreen]

I was hoping we could get some correctness results for the MVE side overnight, but out internal testing seems pretty broken for other reasons. I don't see anything that is complex number related, so hopefully this is OK.

So long as you have done some testing, LGTM.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

23324–23325

Formatting, and it probably wants brackets around the ||