[PATCH] D95245: [SVE] Add support for scalable vectorization of loops with int/fast FP reductions
Caroline via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 25 01:18:40 PST 2021
CarolineConcatto added a comment.
Hey Kerry,
Thank you for this patch.
I found some nit and I have some suggestions about instructionCost.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4695
// first lane. Otherwise, we generate all VF values.
unsigned Lanes =
Cost->isUniformAfterVectorization(P, VF) ? 1 : VF.getKnownMinValue();
----------------
So once we start to use Scalable vector and we start to use the VF.getKnownMinValue(), shouldn't;t this be multiplied by getMaxVScale()?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4742
// Create a vector of consecutive numbers from zero to VF.
for (unsigned i = 0; i < VF.getKnownMinValue(); ++i)
Indices.push_back(
----------------
Same here, should we not need to multiply by getMaxVScale()?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6177
- assert(LoopCost && "Non-zero loop cost expected");
+ assert(LoopCost.getValue() && "Non-zero loop cost expected");
----------------
I believe we can use LoopCost.isValid(), here!
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6198
TTI.enableAggressiveInterleaving(HasReductions);
- if (!InterleavingRequiresRuntimePointerCheck && LoopCost < SmallLoopCost) {
+ if (!InterleavingRequiresRuntimePointerCheck && (unsigned)*LoopCost.getValue() < SmallLoopCost) {
// We assume that the cost overhead is 1 and we use the cost model
----------------
Can you change SmallLoopCost to be instruction cost as LoopCost, so you don't need to use *LoopCost.getValue()?
And I believe that in the std::min you will not need to use getValue
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7677
// loop below supports scalable VFs.
+
ElementCount VF = UserVFIsLegal ? UserVF : MaxVF;
----------------
nit
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9441
+ if (VF.Cost.isValid())
+ IC = CM.selectInterleaveCount(VF.Width, VF.Cost);
}
----------------
nit
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95245/new/
https://reviews.llvm.org/D95245
More information about the llvm-commits
mailing list