[PATCH] D82550: [SLPVectorizer] handle vectorized lib functions
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 25 08:00:50 PDT 2020
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
More information about the llvm-commits
mailing list