-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[SLP] More OOP to simplify vectorizeStores() (NFC) #134605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
sdesmalen-arm
merged 4 commits into
llvm:main
from
gbossu:gbossu.slp.oop.vectorizeStores
Apr 17, 2025
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
ef65d73
[SLP] More OOP to simplify vectorizeStores() (NFC)
gbossu c185092
fixup! [SLP] More OOP to simplify vectorizeStores() (NFC)
gbossu f935439
fixup! [SLP] More OOP to simplify vectorizeStores() (NFC)
gbossu d03b68b
fixup! [SLP] More OOP to simplify vectorizeStores() (NFC)
gbossu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20856,9 +20856,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); | ||
|
@@ -20873,12 +20880,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 | ||
|
@@ -21166,14 +21219,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 | ||
|
@@ -21209,52 +21255,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}}. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that |
||
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)) { | ||
|
@@ -21265,7 +21289,7 @@ bool SLPVectorizerPass::vectorizeStores( | |
// Check that we do not try to vectorize stores of different types. | ||
if (PrevValTy != SI->getValueOperand()->getType()) { | ||
for (RelatedStoreInsts &StoreSeq : SortedStores) | ||
TryToVectorize(StoreSeq.Instrs); | ||
TryToVectorize(StoreSeq.getStores()); | ||
SortedStores.clear(); | ||
PrevValTy = SI->getValueOperand()->getType(); | ||
} | ||
|
@@ -21274,7 +21298,7 @@ bool SLPVectorizerPass::vectorizeStores( | |
|
||
// Final vectorization attempt. | ||
for (RelatedStoreInsts &StoreSeq : SortedStores) | ||
TryToVectorize(StoreSeq.Instrs); | ||
TryToVectorize(StoreSeq.getStores()); | ||
|
||
return Changed; | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.