[PATCH] D147451: [CodeGen] Enable AArch64 SVE FCMLA/FCADD instruction generation in ComplexDeinterleaving

Mon Apr 17 01:44:54 PDT 2023

mgabka added inline comments.

================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:340

-  if (!TL->isComplexDeinterleavingSupported()) {
+  if (!TL->isComplexDeinterleavingSupported(true) &&
+      !TL->isComplexDeinterleavingSupported(false)) {
----------------
dmgreen wrote:
> I don't think this needs to call with a Scalable flag. The AArch64TargetLowering::isComplexDeinterleavingSupported routine can just return true if it has sve or complexnums.
The isComplexDeinterleavingSupported  with a flag is used inside   isComplexDeinterleavingOperationSupported  to check if for given vector type (scalable or fixed width) we have required architecture extensions avaialble.
For scalable vectors we need just sve, while for fixed width we need to have the ComplxNum feature.

However I realized that there is probably a bug here, as the SVE feature enables only scalable FCMLA, while scalable CMLA are available only when SVE2 feature is enabled, so I think we should make sure that isComplexDeinterleavingOperationSupported takes that into account and add extra testing.

I guess what David suggest is to make "isComplexDeinterleavingSupported" function as generic as possible and leave all the detailed checks to  isComplexDeinterleavingOperationSupported , what looks reasonable to me. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147451