[PATCH] D82550: [SLPVectorizer] handle vectorized lib functions

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks for the patch. Some comments inline



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:227
                                SmallVectorImpl<VFInfo> &Mappings) {
+    if (CI.isIndirectCall())
+      return;
----------------
unrelated to the change? If so, better to split it off (and perhaps add a test independent of SLP?)


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3036
+      if (!VecFunc && !isTriviallyVectorizable(ID))
+      {
         BS.cancelScheduling(VL, VL0);
----------------
please run clang format-diff on the patch,


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3066
         }
+
         // Some intrinsics have scalar arguments and should be same in order for
----------------
Please strip the unrelated whitespace changes.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5098
 
+static bool isSideeffectIntrinsic(Instruction *I) {
+  auto *II = dyn_cast<IntrinsicInst>(I);
----------------
This is a NFC change, right? Would be good to split off and commit separately. Looks like an independent improvement.


================
Comment at: llvm/test/Transforms/SLPVectorizer/AArch64/accelerate-vector-functions.ll:16
+; CHECK-NEXT:    [[TMP0:%.*]] = load <4 x float>, <4 x float>* [[A:%.*]], align 16
+; CHECK-NEXT:    [[TMP1:%.*]] = call fast <4 x float> @vsinf(<4 x float> [[TMP0]])
+; CHECK-NEXT:    [[TMP2:%.*]] = extractelement <4 x float> [[TMP1]], i32 0
----------------
It would be good to pre-commit the test and then only have the diff of the changes here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82550