[PATCH] D129066: [AArch64][CodeGen] Add AArch64 support for complex deinterleaving

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

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



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22233
 }
+bool AArch64TargetLowering::isComplexDeinterleavingSupported() const {
+  return Subtarget->hasComplxNum() && Subtarget->hasNEON();
----------------
Can you add some extra newlines between methods


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22249
+
+  return (ScalarTy->isHalfTy() && Subtarget->hasFullFP16()) || ScalarTy->isFloatTy() || ScalarTy->isDoubleTy();
+}
----------------
Formatting, here and below.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22264
+
+  assert(TyWidth >= 128 || TyWidth == 64);
+
----------------
Add a message to the assert


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:21413
+bool AArch64TargetLowering::isComplexDeinterleavingSupported() const {
+  return Subtarget->hasComplxNum() && Subtarget->hasNEON();
+}
----------------
dmgreen wrote:
> I think ComplxNum requires NEON, so checking for the first should be enough.
This I don't think needs the NEON check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129066