[llvm] 2ee7fc0 - [SLP] More OOP to simplify vectorizeStores() (NFC) (#134605)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 17 02:02:56 PDT 2025
Author: Gaëtan Bossu
Date: 2025-04-17T10:02:53+01:00
New Revision: 2ee7fc04568f835be00629cb4130171b14b2e7df
URL: https://github.com/llvm/llvm-project/commit/2ee7fc04568f835be00629cb4130171b14b2e7df
DIFF: https://github.com/llvm/llvm-project/commit/2ee7fc04568f835be00629cb4130171b14b2e7df.diff
LOG: [SLP] More OOP to simplify vectorizeStores() (NFC) (#134605)
This moves more code into the RelatedStoreInsts helper class. The
FillStoresSet lambda is now only a couple of lines and is easier to
read.
Added:
Modified:
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 41ad43012d337..fd23fb6c81c2c 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -20859,9 +20859,16 @@ namespace {
/// A group of stores that we'll try to bundle together using vector ops.
/// They are ordered using the signed distance of their address operand to the
/// address of this group's BaseInstr.
-struct RelatedStoreInsts {
- RelatedStoreInsts(unsigned BaseInstrIdx) { reset(BaseInstrIdx); }
+class RelatedStoreInsts {
+public:
+ RelatedStoreInsts(unsigned BaseInstrIdx, ArrayRef<StoreInst *> AllStores)
+ : AllStores(AllStores) {
+ reset(BaseInstrIdx);
+ }
+
void reset(unsigned NewBaseInstr) {
+ assert(NewBaseInstr < AllStores.size() &&
+ "Instruction index out of bounds");
BaseInstrIdx = NewBaseInstr;
Instrs.clear();
insertOrLookup(NewBaseInstr, 0);
@@ -20876,12 +20883,58 @@ struct RelatedStoreInsts {
return Inserted ? std::nullopt : std::optional<unsigned>(It->second);
}
+ using DistToInstMap = std::map<int, unsigned>;
+ const DistToInstMap &getStores() const { return Instrs; }
+
+ /// If \p SI is related to this group of stores, return the distance of its
+ /// pointer operand to the one the group's BaseInstr.
+ std::optional<int> getPointerDiff(StoreInst &SI, const DataLayout &DL,
+ ScalarEvolution &SE) const {
+ StoreInst &BaseStore = *AllStores[BaseInstrIdx];
+ return getPointersDiff(
+ BaseStore.getValueOperand()->getType(), BaseStore.getPointerOperand(),
+ SI.getValueOperand()->getType(), SI.getPointerOperand(), DL, SE,
+ /*StrictCheck=*/true);
+ }
+
+ /// Recompute the pointer distances to be based on \p NewBaseInstIdx.
+ /// Stores whose index is less than \p MinSafeIdx will be dropped.
+ void rebase(unsigned MinSafeIdx, unsigned NewBaseInstIdx,
+ int DistFromCurBase) {
+ DistToInstMap PrevSet = std::move(Instrs);
+ reset(NewBaseInstIdx);
+
+ // Re-insert stores that come after MinSafeIdx to try and vectorize them
+ // again. Their distance will be "rebased" to use NewBaseInstIdx as
+ // reference.
+ for (auto [Dist, InstIdx] : PrevSet) {
+ if (InstIdx >= MinSafeIdx)
+ insertOrLookup(InstIdx, Dist - DistFromCurBase);
+ }
+ }
+
+ /// Remove all stores that have been vectorized from this group.
+ void clearVectorizedStores(const BoUpSLP::ValueSet &VectorizedStores) {
+ DistToInstMap::reverse_iterator LastVectorizedStore = find_if(
+ reverse(Instrs), [&](const std::pair<int, unsigned> &DistAndIdx) {
+ return VectorizedStores.contains(AllStores[DistAndIdx.second]);
+ });
+
+ // Get a forward iterator pointing after the last vectorized store and erase
+ // all stores before it so we don't try to vectorize them again.
+ DistToInstMap::iterator VectorizedStoresEnd = LastVectorizedStore.base();
+ Instrs.erase(Instrs.begin(), VectorizedStoresEnd);
+ }
+
+private:
/// The index of the Base instruction, i.e. the one with a 0 pointer distance.
unsigned BaseInstrIdx;
/// Maps a pointer distance from \p BaseInstrIdx to an instruction index.
- using DistToInstMap = std::map<int, unsigned>;
DistToInstMap Instrs;
+
+ /// Reference to all the stores in the BB being analyzed.
+ ArrayRef<StoreInst *> AllStores;
};
} // end anonymous namespace
@@ -21165,14 +21218,7 @@ bool SLPVectorizerPass::vectorizeStores(
}
};
- // Stores pair (first: index of the store into Stores array ref, address of
- // which taken as base, second: sorted set of pairs {index, dist}, which are
- // indices of stores in the set and their store location distances relative to
- // the base address).
-
- // Need to store the index of the very first store separately, since the set
- // may be reordered after the insertion and the first store may be moved. This
- // container allows to reduce number of calls of getPointersDiff() function.
+ /// Groups of stores to vectorize
SmallVector<RelatedStoreInsts> SortedStores;
// Inserts the specified store SI with the given index Idx to the set of the
@@ -21208,52 +21254,30 @@ bool SLPVectorizerPass::vectorizeStores(
// dependencies and no need to waste compile time to try to vectorize them.
// - Try to vectorize the sequence {1, {1, 0}, {3, 2}}.
auto FillStoresSet = [&](unsigned Idx, StoreInst *SI) {
- for (RelatedStoreInsts &StoreSeq : SortedStores) {
- std::optional<int> Diff = getPointersDiff(
- Stores[StoreSeq.BaseInstrIdx]->getValueOperand()->getType(),
- Stores[StoreSeq.BaseInstrIdx]->getPointerOperand(),
- SI->getValueOperand()->getType(), SI->getPointerOperand(), *DL, *SE,
- /*StrictCheck=*/true);
- if (!Diff)
- continue;
- std::optional<unsigned> PrevInst =
- StoreSeq.insertOrLookup(/*InstrIdx=*/Idx, /*PtrDist=*/*Diff);
- if (!PrevInst) {
- // No store was associated to that distance. Keep collecting.
- return;
- }
- // Try to vectorize the first found set to avoid duplicate analysis.
- TryToVectorize(StoreSeq.Instrs);
- RelatedStoreInsts::DistToInstMap PrevSet;
- copy_if(StoreSeq.Instrs, std::inserter(PrevSet, PrevSet.end()),
- [&](const std::pair<int, unsigned> &DistAndIdx) {
- return DistAndIdx.second > *PrevInst;
- });
- StoreSeq.reset(Idx);
- // Insert stores that followed previous match to try to vectorize them
- // with this store.
- unsigned StartIdx = *PrevInst + 1;
- SmallBitVector UsedStores(Idx - StartIdx);
- // Distances to previously found dup store (or this store, since they
- // store to the same addresses).
- SmallVector<int> Dists(Idx - StartIdx, 0);
- for (auto [PtrDist, InstIdx] : reverse(PrevSet)) {
- // Do not try to vectorize sequences, we already tried.
- if (VectorizedStores.contains(Stores[InstIdx]))
- break;
- unsigned BI = InstIdx - StartIdx;
- UsedStores.set(BI);
- Dists[BI] = PtrDist - *Diff;
- }
- for (unsigned I = StartIdx; I < Idx; ++I) {
- unsigned BI = I - StartIdx;
- if (UsedStores.test(BI))
- StoreSeq.insertOrLookup(I, Dists[BI]);
- }
+ std::optional<int> PtrDist;
+ auto *RelatedStores = find_if(
+ SortedStores, [&PtrDist, SI, this](const RelatedStoreInsts &StoreSeq) {
+ PtrDist = StoreSeq.getPointerDiff(*SI, *DL, *SE);
+ return PtrDist.has_value();
+ });
+
+ // We did not find a comparable store, start a new group.
+ if (RelatedStores == SortedStores.end()) {
+ SortedStores.emplace_back(Idx, Stores);
return;
}
- // We did not find a comparable store, start a new sequence.
- SortedStores.emplace_back(Idx);
+
+ // If there is already a store in the group with the same PtrDiff, try to
+ // vectorize the existing instructions before adding the current store.
+ // Otherwise, insert this store and keep collecting.
+ if (std::optional<unsigned> PrevInst =
+ RelatedStores->insertOrLookup(Idx, *PtrDist)) {
+ TryToVectorize(RelatedStores->getStores());
+ RelatedStores->clearVectorizedStores(VectorizedStores);
+ RelatedStores->rebase(/*MinSafeIdx=*/*PrevInst + 1,
+ /*NewBaseInstIdx=*/Idx,
+ /*DistFromCurBase=*/*PtrDist);
+ }
};
Type *PrevValTy = nullptr;
for (auto [I, SI] : enumerate(Stores)) {
@@ -21264,7 +21288,7 @@ bool SLPVectorizerPass::vectorizeStores(
// Check that we do not try to vectorize stores of
diff erent types.
if (PrevValTy != SI->getValueOperand()->getType()) {
for (RelatedStoreInsts &StoreSeq : SortedStores)
- TryToVectorize(StoreSeq.Instrs);
+ TryToVectorize(StoreSeq.getStores());
SortedStores.clear();
PrevValTy = SI->getValueOperand()->getType();
}
@@ -21273,7 +21297,7 @@ bool SLPVectorizerPass::vectorizeStores(
// Final vectorization attempt.
for (RelatedStoreInsts &StoreSeq : SortedStores)
- TryToVectorize(StoreSeq.Instrs);
+ TryToVectorize(StoreSeq.getStores());
return Changed;
}
More information about the llvm-commits
mailing list