[SLP] Cluster SortedBases before sorting. #101144
Merged
Conversation
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
In order to enforce a strict-weak ordering, this patch clusters the bases that are being sorted by the root - the first value in a gep chain. The sorting is then performed in each cluster.
| Root = Gep->getOperand(0); | ||
| SortedBases.emplace_back(Base.first, Strip, Root); | ||
| } | ||
| if (SortedBases.size() <= 16) { |
There was a problem hiding this comment.
I was just limiting the complexity in case it was somehow very large. I can remove it if you think that's fine.
There was a problem hiding this comment.
Better to have some named constants
Comment on lines
4858
to
4859
| auto Begin = SortedBases.begin(); | ||
| auto End = SortedBases.end(); |
| Root = Gep->getOperand(0); | ||
| SortedBases.emplace_back(Base.first, Strip, Root); | ||
| } | ||
| if (SortedBases.size() <= 16) { |
There was a problem hiding this comment.
Better to have some named constants
Comment on lines
4866
to
4870
| while (auto *Gep = dyn_cast<GetElementPtrInst>(V)) { | ||
| if (Gep->getOperand(0) == std::get<1>(V1)) | ||
| return true; | ||
| V = Gep->getOperand(0); | ||
| } |
There was a problem hiding this comment.
Can you do this preliminary, store the result in the container and look into the container rather than running in the loop for each element?
There was a problem hiding this comment.
Where do you mean? I was wondering if there would be a nicer way to pre-calculate the results, but wasn't sure how that would look in a way that was better.
There was a problem hiding this comment.
I mean, instead of a loop in a sort function, do some kind of analysis before or use standard functions, like getUnderlying....
davemgreen
force-pushed
the
gh-slp-clusterbeforesort
branch
from
e051b2d to
0a8e6e4
Compare
davemgreen
force-pushed
the
gh-slp-clusterbeforesort
branch
from
0a8e6e4 to
8b9906e
Compare
|
@llvm/pr-subscribers-llvm-transforms Author: David Green (davemgreen) ChangesIn order to enforce a strict-weak ordering, this patch clusters the bases that are being sorted by the root - the first value in a gep chain. The sorting is then performed in each cluster. Full diff: https://github.com/llvm/llvm-project/pull/101144.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 6501a14d87789..98befd1fd51f5 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -4843,25 +4843,43 @@ static bool clusterSortPtrAccesses(ArrayRef<Value *> VL, Type *ElemTy,
return false;
// If we have a better order, also sort the base pointers by increasing
- // (variable) values if possible, to try and keep the order more regular.
- SmallVector<std::pair<Value *, Value *>> SortedBases;
- for (auto &Base : Bases)
- SortedBases.emplace_back(Base.first,
- Base.first->stripInBoundsConstantOffsets());
- llvm::stable_sort(SortedBases, [](std::pair<Value *, Value *> V1,
- std::pair<Value *, Value *> V2) {
- const Value *V = V2.second;
- while (auto *Gep = dyn_cast<GetElementPtrInst>(V)) {
- if (Gep->getOperand(0) == V1.second)
- return true;
- V = Gep->getOperand(0);
- }
- return false;
- });
+ // (variable) values if possible, to try and keep the order more regular. In
+ // order to create a valid strict-weak order we cluster by the Root of gep
+ // chains and sort within each.
+ SmallVector<std::tuple<Value *, Value *, Value *>> SortedBases;
+ for (auto &Base : Bases) {
+ Value *Strip = Base.first->stripInBoundsConstantOffsets();
+ Value *Root = Strip;
+ while (auto *Gep = dyn_cast<GetElementPtrInst>(Root))
+ Root = Gep->getOperand(0);
+ SortedBases.emplace_back(Base.first, Strip, Root);
+ }
+ auto *Begin = SortedBases.begin();
+ auto *End = SortedBases.end();
+ while (Begin != End) {
+ Value *Root = std::get<2>(*Begin);
+ auto *Mid = std::stable_partition(
+ Begin, End, [&Root](auto V) { return std::get<2>(V) == Root; });
+ DenseMap<Value *, DenseMap<Value *, bool>> LessThan;
+ for (auto I = Begin; I < Mid; ++I)
+ LessThan[std::get<1>(*I)] = DenseMap<Value *, bool>();
+ for (auto I = Begin; I < Mid; ++I) {
+ Value *V = std::get<1>(*I);
+ while (auto *Gep = dyn_cast<GetElementPtrInst>(V)) {
+ V = Gep->getOperand(0);
+ if (LessThan.contains(V))
+ LessThan[V][std::get<1>(*I)] = true;
+ }
+ }
+ std::stable_sort(Begin, Mid, [&LessThan](auto &V1, auto &V2) {
+ return LessThan[std::get<1>(V1)][std::get<1>(V2)];
+ });
+ Begin = Mid;
+ }
// Collect the final order of sorted indices
for (auto Base : SortedBases)
- for (auto &T : Bases[Base.first])
+ for (auto &T : Bases[std::get<0>(Base)])
SortedIndices.push_back(std::get<2>(T));
assert(SortedIndices.size() == VL.size() &&
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/loadorder.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/loadorder.ll
index 6b5503f26fabf..d79aed89b0be7 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/loadorder.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/loadorder.ll
@@ -428,14 +428,14 @@ define i32 @reduce_blockstrided4x4(ptr nocapture noundef readonly %p1, i32 nound
; CHECK-NEXT: [[TMP5:%.*]] = load <4 x i8>, ptr [[ADD_PTR64]], align 1
; CHECK-NEXT: [[TMP6:%.*]] = load <4 x i8>, ptr [[ARRAYIDX3_1]], align 1
; CHECK-NEXT: [[TMP7:%.*]] = load <4 x i8>, ptr [[ARRAYIDX5_1]], align 1
-; CHECK-NEXT: [[TMP8:%.*]] = shufflevector <4 x i8> [[TMP0]], <4 x i8> [[TMP1]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <4 x i8> [[TMP4]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[TMP8:%.*]] = shufflevector <4 x i8> [[TMP0]], <4 x i8> [[TMP4]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <4 x i8> [[TMP1]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP10:%.*]] = shufflevector <16 x i8> [[TMP8]], <16 x i8> [[TMP9]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 16, i32 17, i32 18, i32 19, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP11:%.*]] = shufflevector <4 x i8> [[TMP5]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP12:%.*]] = shufflevector <16 x i8> [[TMP10]], <16 x i8> [[TMP11]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 16, i32 17, i32 18, i32 19>
; CHECK-NEXT: [[TMP13:%.*]] = zext <16 x i8> [[TMP12]] to <16 x i32>
-; CHECK-NEXT: [[TMP14:%.*]] = shufflevector <4 x i8> [[TMP2]], <4 x i8> [[TMP3]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP15:%.*]] = shufflevector <4 x i8> [[TMP6]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[TMP14:%.*]] = shufflevector <4 x i8> [[TMP2]], <4 x i8> [[TMP6]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[TMP15:%.*]] = shufflevector <4 x i8> [[TMP3]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP16:%.*]] = shufflevector <16 x i8> [[TMP14]], <16 x i8> [[TMP15]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 16, i32 17, i32 18, i32 19, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP17:%.*]] = shufflevector <4 x i8> [[TMP7]], <4 x i8> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP18:%.*]] = shufflevector <16 x i8> [[TMP16]], <16 x i8> [[TMP17]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 16, i32 17, i32 18, i32 19>
|
| Begin, End, [&Root](auto V) { return std::get<2>(V) == Root; }); | ||
| DenseMap<Value *, DenseMap<Value *, bool>> LessThan; | ||
| for (auto I = Begin; I < Mid; ++I) | ||
| LessThan[std::get<1>(*I)] = DenseMap<Value *, bool>(); |
There was a problem hiding this comment.
Suggested change
| LessThan[std::get<1>(*I)] = DenseMap<Value *, bool>(); | |
| LessThan.try_mplace(std::get<1>(*I)); |
davemgreen
force-pushed
the
gh-slp-clusterbeforesort
branch
from
8b9906e to
2900800
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/2925 Here is the relevant piece of the build log for the reference: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In order to enforce a strict-weak ordering, this patch clusters the bases that are being sorted by the root - the first value in a gep chain. The sorting is then performed in each cluster.