[SLP][AArch64] Incorrectly estimated intrinsic as a function call

[dtemirbulatov]

Summary

I found that currently we incorrectly assume intrinsic as a function call and it prevents us from the opportunity to vectorize. On Aarch64 Cortex-A53 we think that llvm.fmuladd.f64 is a function call which is wrong, but we could ask the backend about the cost of such an intrinsic to determine on how it ends instruction(s)/function call. This is one of the reasons why LNT's matmul_f64_4x4.c is not vectorized on AArch64.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7570–7591

Can you tranform all this code to lambda?

7571–7572

if (auto *II = dyn_cast<IntrinsicInst>(&*PrevInstIt))

7574–7576

Sink this code to the else substatement.

7578–7580

Braces

7586–7591

Do we have a btter way to check if the intrinsic is lowered as an instruction rather than the call?

NoCallIntrinsic = IntrCost < CallCost;

llvm/test/Transforms/SLPVectorizer/AArch64/fmulladd.ll

1

Need to precommit the test

3–4

I assume this can be removed

9

cleanup attrs, I assume they are not required.

75

Remove attribute reference

[dtemirbulatov]

Rebased, Addressed remarks.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7586–7591

No, it looks to me the only way. We could duplicate the code from getTypeBasedIntrinsicInstrCost() and ask about legality of an operation from a target, but it would be too much to duplicate. Also, we could hardcode minimal call cost, for example 10, but it would look incorrect to me too.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7572–7574

You can drop braces here

7574

No need for else after if with the return substatemenet.

[dtemirbulatov]

Addressed comments.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7576

Do you still need this cast to get the list of arguments?

[dtemirbulatov]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7576

yes, in order to get correctly prepared ICA. Otherwise sometime we could get incorrect cost or compile time error if we provide empty set of arguments.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7576

II is already an IntrinsicInst, which has CallInst as the base class. I mean, you don't need to do another cast here, you can use II instead of CI

[dtemirbulatov]

Removed unnecessary cast to CallInst.

[ABataev]

LG with a nit

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7593

Remove extra parens around isa call.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

BasicBlock::reverse_iterator InstIt = ++Inst->getIterator().getReverse(),
                                  PrevInstIt =
                                      PrevInst->getIterator().getReverse();
     while (InstIt != PrevInstIt) {
       if (PrevInstIt == PrevInst->getParent()->rend()) {
         PrevInstIt = Inst->getParent()->rbegin();
         continue;
       }
 
+      auto NoCallIntrinsic = [this](Instruction *I) {
+        if (auto *II = dyn_cast<IntrinsicInst>(I)) {
+          if (II->isAssumeLikeIntrinsic())
+            return true;
+          FastMathFlags FMF;
+          SmallVector<Type *, 4> Tys;
+          for (auto &ArgOp : II->args())
+            Tys.push_back(ArgOp->getType());
+          if (auto *FPMO = dyn_cast<FPMathOperator>(II))
+            FMF = FPMO->getFastMathFlags();
+          IntrinsicCostAttributes ICA(II->getIntrinsicID(), II->getType(), Tys,
+                                      FMF);
+          InstructionCost IntrCost =
+              TTI->getIntrinsicInstrCost(ICA, TTI::TCK_RecipThroughput);
+          InstructionCost CallCost = TTI->getCallInstrCost(
+              nullptr, II->getType(), Tys, TTI::TCK_RecipThroughput);
+          if (IntrCost < CallCost)
+            return true;
+        }
+        return false;
+      };
+
       // Debug information does not impact spill cost.
-      if ((isa<CallInst>(&*PrevInstIt) &&
-           !isa<DbgInfoIntrinsic>(&*PrevInstIt)) &&
+      if ((isa<CallInst>(&*PrevInstIt)) && !NoCallIntrinsic(&*PrevInstIt) &&
           &*PrevInstIt != PrevInst)
         NumCalls++;
 
       ++PrevInstIt;
     }
 
     if (NumCalls) {
       SmallVector<Type*, 4> V;

llvm/test/Transforms/SLPVectorizer/AArch64/fmulladd.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=slp-vectorizer -mtriple=aarch64-unknown-unknown -mcpu=cortex-a53 -S | FileCheck %s
 
 @b = common global i32 0, align 4
 @a = common global ptr null, align 8
 
 define void @foo(ptr %d, ptr %e) {
 ; CHECK-LABEL: @foo(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds [4 x double], ptr [[D:%.*]], i64 2
-; CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
 ; CHECK-NEXT:    [[ARRAYIDX3:%.*]] = getelementptr inbounds [4 x double], ptr [[D]], i64 0, i64 3
-; CHECK-NEXT:    [[TMP1:%.*]] = load double, ptr [[ARRAYIDX3]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr [[ARRAYIDX3]], align 8
 ; CHECK-NEXT:    [[ARRAYIDX4:%.*]] = getelementptr inbounds [4 x double], ptr [[E:%.*]], i64 3
-; CHECK-NEXT:    [[TMP2:%.*]] = load double, ptr [[ARRAYIDX4]], align 8
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP1]], double [[TMP2]], double [[TMP0]])
-; CHECK-NEXT:    [[ARRAYIDX8:%.*]] = getelementptr inbounds [4 x double], ptr [[D]], i64 2, i64 1
-; CHECK-NEXT:    [[TMP4:%.*]] = load double, ptr [[ARRAYIDX8]], align 8
-; CHECK-NEXT:    [[ARRAYIDX12:%.*]] = getelementptr inbounds [4 x double], ptr [[E]], i64 3, i64 1
-; CHECK-NEXT:    [[TMP5:%.*]] = load double, ptr [[ARRAYIDX12]], align 8
-; CHECK-NEXT:    [[TMP6:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP1]], double [[TMP5]], double [[TMP4]])
 ; CHECK-NEXT:    [[ARRAYIDX15:%.*]] = getelementptr inbounds [4 x double], ptr [[D]], i64 1, i64 3
-; CHECK-NEXT:    [[TMP7:%.*]] = load double, ptr [[ARRAYIDX15]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = load double, ptr [[ARRAYIDX15]], align 8
 ; CHECK-NEXT:    [[ARRAYIDX17:%.*]] = getelementptr inbounds [4 x double], ptr [[D]], i64 3, i64 2
-; CHECK-NEXT:    [[TMP8:%.*]] = load double, ptr [[ARRAYIDX17]], align 8
+; CHECK-NEXT:    [[TMP2:%.*]] = load double, ptr [[ARRAYIDX17]], align 8
 ; CHECK-NEXT:    [[ARRAYIDX19:%.*]] = getelementptr inbounds [4 x double], ptr [[E]], i64 2, i64 3
-; CHECK-NEXT:    [[TMP9:%.*]] = load double, ptr [[ARRAYIDX19]], align 8
-; CHECK-NEXT:    [[TMP10:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP8]], double [[TMP9]], double [[TMP7]])
+; CHECK-NEXT:    [[TMP3:%.*]] = load double, ptr [[ARRAYIDX19]], align 8
+; CHECK-NEXT:    [[TMP4:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP2]], double [[TMP3]], double [[TMP1]])
 ; CHECK-NEXT:    [[ARRAYIDX21:%.*]] = getelementptr inbounds [4 x double], ptr [[D]], i64 3, i64 3
-; CHECK-NEXT:    [[TMP11:%.*]] = load double, ptr [[ARRAYIDX21]], align 8
+; CHECK-NEXT:    [[TMP5:%.*]] = load double, ptr [[ARRAYIDX21]], align 8
 ; CHECK-NEXT:    [[ARRAYIDX23:%.*]] = getelementptr inbounds [4 x double], ptr [[E]], i64 3, i64 3
-; CHECK-NEXT:    [[TMP12:%.*]] = load double, ptr [[ARRAYIDX23]], align 8
-; CHECK-NEXT:    [[TMP13:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP11]], double [[TMP12]], double [[TMP10]])
-; CHECK-NEXT:    [[TMP14:%.*]] = load ptr, ptr @a, align 8
-; CHECK-NEXT:    store double [[TMP3]], ptr [[TMP14]], align 8
-; CHECK-NEXT:    [[F_SROA_4_0__SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[TMP14]], i64 8
-; CHECK-NEXT:    store double [[TMP6]], ptr [[F_SROA_4_0__SROA_IDX]], align 8
-; CHECK-NEXT:    [[F_SROA_539_0__SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[TMP14]], i64 40
-; CHECK-NEXT:    store double [[TMP13]], ptr [[F_SROA_539_0__SROA_IDX]], align 8
+; CHECK-NEXT:    [[TMP6:%.*]] = load double, ptr [[ARRAYIDX23]], align 8
+; CHECK-NEXT:    [[TMP7:%.*]] = tail call double @llvm.fmuladd.f64(double [[TMP5]], double [[TMP6]], double [[TMP4]])
+; CHECK-NEXT:    [[TMP8:%.*]] = load ptr, ptr @a, align 8
+; CHECK-NEXT:    [[TMP9:%.*]] = load <2 x double>, ptr [[ARRAYIDX]], align 8
+; CHECK-NEXT:    [[TMP10:%.*]] = load <2 x double>, ptr [[ARRAYIDX4]], align 8
+; CHECK-NEXT:    [[TMP11:%.*]] = insertelement <2 x double> poison, double [[TMP0]], i32 0
+; CHECK-NEXT:    [[TMP12:%.*]] = shufflevector <2 x double> [[TMP11]], <2 x double> poison, <2 x i32> zeroinitializer
+; CHECK-NEXT:    [[TMP13:%.*]] = call <2 x double> @llvm.fmuladd.v2f64(<2 x double> [[TMP12]], <2 x double> [[TMP10]], <2 x double> [[TMP9]])
+; CHECK-NEXT:    store <2 x double> [[TMP13]], ptr [[TMP8]], align 8
+; CHECK-NEXT:    [[F_SROA_539_0__SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[TMP8]], i64 40
+; CHECK-NEXT:    store double [[TMP7]], ptr [[F_SROA_539_0__SROA_IDX]], align 8
 ; CHECK-NEXT:    store i32 6, ptr @b, align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %arrayidx = getelementptr inbounds [4 x double], ptr %d, i64 2
   %0 = load double, ptr %arrayidx, align 8
   %arrayidx3 = getelementptr inbounds [4 x double], ptr %d, i64 0, i64 3
   %1 = load double, ptr %arrayidx3, align 8
entry:
   %f.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %14, i64 8
   store double %6, ptr %f.sroa.4.0..sroa_idx, align 8
   %f.sroa.539.0..sroa_idx = getelementptr inbounds i8, ptr %14, i64 40
   store double %13, ptr %f.sroa.539.0..sroa_idx, align 8
   store i32 6, ptr @b, align 4
   ret void
 }
 
 declare double @llvm.fmuladd.f64(double, double, double)