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

Mon Apr 17 04:39:09 PDT 2023

igor.kirillov added inline comments.

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

-  if (!TL->isComplexDeinterleavingSupported()) {
+  if (!TL->isComplexDeinterleavingSupported(true) &&
+      !TL->isComplexDeinterleavingSupported(false)) {
----------------
mgabka wrote:
> 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. 
CMLA are not a problem - we are checking that scalar type is not **Int** in **AArch64TargetLowering::isComplexDeinterleavingOperationSupported**

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147451