[PATCH] D57059: [SLP] Initial support for the vectorization of the non-power-of-2 vectors.
Simon Pilgrim via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 28 10:24:53 PDT 2019
RKSimon added a comment.
Some initial comments - there's a lot going on (cutting it into smaller patches would be very useful).
The sdiv undefs test worries me, and I'm not certain if we have good masked load/store costs at the moment.
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1500
+ Value *UseScalar =
+ *llvm::find_if(UseEntry->Scalars, Instruction::classof);
// Some in-tree scalars will remain as scalar in vectorized
----------------
Check for failure instead of just dereferencing?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2008
+ continue;
+ auto *Op = cast<Instruction>(V)->getOperand(1);
if (!isa<ConstantInt>(Op)) {
----------------
drop the * ?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2138
+ for (Value *V : VL) {
+ auto *CI2 = dyn_cast<CallInst>(V);
Operands.push_back(CI2->getArgOperand(i));
----------------
drop the * ?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2298
+ Value *V0;
Type *ScalarTy = VL[0]->getType();
+ VectorType *VecTy;
----------------
Is it safe to use VL[0] like this - do we guarantee the undef types are correct?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2987
bool AllSameOpcodeRight, bool SplatLeft,
bool SplatRight) {
+ Value *VLeft = I.getOperand(0);
----------------
This is going to cause problems for an incoming patch I'm working on - any chance that you can avoid using Instruction &I?
================
Comment at: test/Transforms/SLPVectorizer/AArch64/PR38339.ll:16
+; CHECK-NEXT: store i16 [[T3]], i16* [[PTR2]]
+; CHECK-NEXT: store i16 [[T2]], i16* [[PTR3]]
; CHECK-NEXT: ret void
----------------
regression?
================
Comment at: test/Transforms/SLPVectorizer/X86/addsub.ll:319
+; CHECK-NEXT: [[TMP14:%.*]] = getelementptr inbounds double, double* [[C]], i64 1
+; CHECK-NEXT: store double [[TMP13]], double* [[TMP14]]
; CHECK-NEXT: ret void
----------------
regression?
================
Comment at: test/Transforms/SLPVectorizer/X86/alternate-fp.ll:87
+; SLM-NEXT: [[R6:%.*]] = insertelement <8 x float> [[R5]], float [[AB6]], i32 6
+; SLM-NEXT: [[R7:%.*]] = insertelement <8 x float> [[R6]], float [[AB7]], i32 7
; SLM-NEXT: ret <8 x float> [[R7]]
----------------
regression?
================
Comment at: test/Transforms/SLPVectorizer/X86/alternate-int.ll:590
+; AVX512-LABEL: @sdiv_v8i32_undefs(
+; AVX512-NEXT: ret <8 x i32> undef
;
----------------
This test is checking that the sdiv didn't get vectorized, because if any elt is undef then the whole result is undef - but if its stays scalarized then on elts 0 and 4 become undef.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57059/new/
https://reviews.llvm.org/D57059
More information about the llvm-commits
mailing list