[PATCH] D149742: [SLP]Improve isGatherShuffledEntry by trying per-register shuffle.
Valeriy Dmitriev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 18 18:20:03 PDT 2023
vdmitrie added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6991
constexpr static TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput;
+ bool SameNodesEstimated = true;
----------------
I'm not sure I understand the purpose of this flag. Could you please describe?
It is quite confusing that it is already set right at construction when no estimation was done yet.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7255
+ InVectors.back().get<const TreeEntry *>() == E2) ||
+ (!E2 && InVectors.size() >= 1 &&
+ InVectors.front().get<const TreeEntry *>() == &E1)) {
----------------
InVectors.size() >=1 is same as !InVectors.empty()
Having this check assumes that InVectors can be empty here, but at the same time InVectors.front() below (at line 7267) assumes it is not empty. So which one is the right assumption?
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7718
std::optional<TargetTransformInfo::ShuffleKind> ExtractShuffle;
- std::optional<TargetTransformInfo::ShuffleKind> GatherShuffle;
- SmallVector<const TreeEntry *> Entries;
+ SmallVector<std::optional<TargetTransformInfo::ShuffleKind>> GatherShuffle;
+ SmallVector<SmallVector<const TreeEntry *>> Entries;
----------------
perhaps worth renaming to GatherShuffles since it becomes an array of optionals
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7720
+ SmallVector<SmallVector<const TreeEntry *>> Entries;
+ Type *ScalarTy = GatheredScalars.front()->getType();
// Check for gathered extracts.
----------------
This ScalarTy declaration hides one at the function scope.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7729
+ unsigned NumParts = TTI->getNumberOfParts(FixedVectorType::get(
+ GatheredScalars.front()->getType(), GatheredScalars.size()));
+ if (NumParts == 0 || NumParts >= GatheredScalars.size())
----------------
ScalarTy ?
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7740
}
+ } else if (auto *VecTy =
+ FixedVectorType::get(VL.front()->getType(), VL.size());
----------------
VecTy is already defined at 7674. Can just reuse it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149742/new/
https://reviews.llvm.org/D149742
More information about the llvm-commits
mailing list