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

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:23286
+
+  if (TyWidth > 128) {
+    int Stride = Ty->getNumElements() / 2;
----------------
SjoerdMeijer wrote:
> 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?
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.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:23300
+
+    if (Accumulator) {
+      LowerSplitAcc = B.CreateShuffleVector(Accumulator, LowerSplitMask);
----------------
SjoerdMeijer wrote:
> `Accumulator` always has a value at this point, don't need to check it?
This check is needed, however the previous assignment is not. I've moved it down to where it is needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129066