[SLP][NFC]Make TreeEntry a class and store "need-to-schedule" state #140734
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
Created using spr 1.3.5
Member
|
@llvm/pr-subscribers-vectorizers Author: Alexey Bataev (alexey-bataev) ChangesTreeEntry should be a class, not a struct, since it has private members. Full diff: https://github.com/llvm/llvm-project/pull/140734.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index fcb9da637dd37..a8b28dd08a416 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -1744,7 +1744,7 @@ namespace slpvectorizer {
/// Bottom Up SLP Vectorizer.
class BoUpSLP {
- struct TreeEntry;
+ class TreeEntry;
class ScheduleEntity;
class ScheduleData;
class ScheduleBundle;
@@ -3607,7 +3607,8 @@ class BoUpSLP {
/// opportunities.
void reorderGatherNode(TreeEntry &TE);
- struct TreeEntry {
+ class TreeEntry {
+ public:
using VecTreeTy = SmallVector<std::unique_ptr<TreeEntry>, 8>;
TreeEntry(VecTreeTy &Container) : Container(Container) {}
@@ -3774,6 +3775,9 @@ class BoUpSLP {
/// Interleaving factor for interleaved loads Vectorize nodes.
unsigned InterleaveFactor = 0;
+ /// True if the node does not require scheduling.
+ bool DoesNotNeedToSchedule = false;
+
/// Set this bundle's \p OpIdx'th operand to \p OpVL.
void setOperand(unsigned OpIdx, ArrayRef<Value *> OpVL) {
if (Operands.size() < OpIdx + 1)
@@ -3791,6 +3795,16 @@ class BoUpSLP {
/// Sets interleaving factor for the interleaving nodes.
void setInterleave(unsigned Factor) { InterleaveFactor = Factor; }
+ /// Marks the node as one that does not require scheduling.
+ void setDoesNotNeedToSchedule() {
+ assert(::doesNotNeedToSchedule(Scalars) &&
+ "Expected to not need scheduling");
+ DoesNotNeedToSchedule = true;
+ }
+ /// Returns true if the node is marked as one that does not require
+ /// scheduling.
+ bool doesNotNeedToSchedule() const { return DoesNotNeedToSchedule; }
+
/// Set this bundle's operands from \p Operands.
void setOperands(ArrayRef<ValueList> Operands) {
for (unsigned I : seq<unsigned>(Operands.size()))
@@ -4107,6 +4121,8 @@ class BoUpSLP {
}
}
} else if (!Last->isGather()) {
+ if (doesNotNeedToSchedule(VL))
+ Last->setDoesNotNeedToSchedule();
SmallPtrSet<Value *, 4> Processed;
for (Value *V : VL) {
if (isa<PoisonValue>(V))
@@ -4124,7 +4140,7 @@ class BoUpSLP {
// Update the scheduler bundle to point to this TreeEntry.
assert((!Bundle.getBundle().empty() || isa<PHINode>(S.getMainOp()) ||
isVectorLikeInstWithConstOps(S.getMainOp()) ||
- doesNotNeedToSchedule(VL)) &&
+ Last->doesNotNeedToSchedule()) &&
"Bundle and VL out of sync");
if (!Bundle.getBundle().empty()) {
#if !defined(NDEBUG) || defined(EXPENSIVE_CHECKS)
@@ -15331,8 +15347,8 @@ BoUpSLP::isGatherShuffledSingleRegisterEntry(
}
if (!TEUseEI.UserTE->isGather() && !UserPHI &&
- doesNotNeedToSchedule(TEUseEI.UserTE->Scalars) !=
- doesNotNeedToSchedule(UseEI.UserTE->Scalars) &&
+ TEUseEI.UserTE->doesNotNeedToSchedule() !=
+ UseEI.UserTE->doesNotNeedToSchedule() &&
is_contained(UseEI.UserTE->Scalars, TEInsertPt))
continue;
// Check if the user node of the TE comes after user node of TEPtr,
@@ -16127,7 +16143,7 @@ void BoUpSLP::setInsertPointAfterBundle(const TreeEntry *E) {
}
if (IsPHI ||
(!E->isGather() && E->State != TreeEntry::SplitVectorize &&
- doesNotNeedToSchedule(E->Scalars)) ||
+ E->doesNotNeedToSchedule()) ||
(GatheredLoadsEntriesFirst.has_value() &&
E->Idx >= *GatheredLoadsEntriesFirst && !E->isGather() &&
E->getOpcode() == Instruction::Load)) {
@@ -19803,7 +19819,7 @@ void BoUpSLP::scheduleBlock(BlockScheduling *BS) {
if (ScheduleData *SD = BS->getScheduleData(I)) {
[[maybe_unused]] ArrayRef<TreeEntry *> SDTEs = getTreeEntries(I);
assert((isVectorLikeInstWithConstOps(SD->getInst()) || SDTEs.empty() ||
- doesNotNeedToSchedule(SDTEs.front()->Scalars)) &&
+ SDTEs.front()->doesNotNeedToSchedule()) &&
"scheduler and vectorizer bundle mismatch");
SD->setSchedulingPriority(Idx++);
continue;
|
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Alexey Bataev (alexey-bataev) ChangesTreeEntry should be a class, not a struct, since it has private members. Full diff: https://github.com/llvm/llvm-project/pull/140734.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index fcb9da637dd37..a8b28dd08a416 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -1744,7 +1744,7 @@ namespace slpvectorizer {
/// Bottom Up SLP Vectorizer.
class BoUpSLP {
- struct TreeEntry;
+ class TreeEntry;
class ScheduleEntity;
class ScheduleData;
class ScheduleBundle;
@@ -3607,7 +3607,8 @@ class BoUpSLP {
/// opportunities.
void reorderGatherNode(TreeEntry &TE);
- struct TreeEntry {
+ class TreeEntry {
+ public:
using VecTreeTy = SmallVector<std::unique_ptr<TreeEntry>, 8>;
TreeEntry(VecTreeTy &Container) : Container(Container) {}
@@ -3774,6 +3775,9 @@ class BoUpSLP {
/// Interleaving factor for interleaved loads Vectorize nodes.
unsigned InterleaveFactor = 0;
+ /// True if the node does not require scheduling.
+ bool DoesNotNeedToSchedule = false;
+
/// Set this bundle's \p OpIdx'th operand to \p OpVL.
void setOperand(unsigned OpIdx, ArrayRef<Value *> OpVL) {
if (Operands.size() < OpIdx + 1)
@@ -3791,6 +3795,16 @@ class BoUpSLP {
/// Sets interleaving factor for the interleaving nodes.
void setInterleave(unsigned Factor) { InterleaveFactor = Factor; }
+ /// Marks the node as one that does not require scheduling.
+ void setDoesNotNeedToSchedule() {
+ assert(::doesNotNeedToSchedule(Scalars) &&
+ "Expected to not need scheduling");
+ DoesNotNeedToSchedule = true;
+ }
+ /// Returns true if the node is marked as one that does not require
+ /// scheduling.
+ bool doesNotNeedToSchedule() const { return DoesNotNeedToSchedule; }
+
/// Set this bundle's operands from \p Operands.
void setOperands(ArrayRef<ValueList> Operands) {
for (unsigned I : seq<unsigned>(Operands.size()))
@@ -4107,6 +4121,8 @@ class BoUpSLP {
}
}
} else if (!Last->isGather()) {
+ if (doesNotNeedToSchedule(VL))
+ Last->setDoesNotNeedToSchedule();
SmallPtrSet<Value *, 4> Processed;
for (Value *V : VL) {
if (isa<PoisonValue>(V))
@@ -4124,7 +4140,7 @@ class BoUpSLP {
// Update the scheduler bundle to point to this TreeEntry.
assert((!Bundle.getBundle().empty() || isa<PHINode>(S.getMainOp()) ||
isVectorLikeInstWithConstOps(S.getMainOp()) ||
- doesNotNeedToSchedule(VL)) &&
+ Last->doesNotNeedToSchedule()) &&
"Bundle and VL out of sync");
if (!Bundle.getBundle().empty()) {
#if !defined(NDEBUG) || defined(EXPENSIVE_CHECKS)
@@ -15331,8 +15347,8 @@ BoUpSLP::isGatherShuffledSingleRegisterEntry(
}
if (!TEUseEI.UserTE->isGather() && !UserPHI &&
- doesNotNeedToSchedule(TEUseEI.UserTE->Scalars) !=
- doesNotNeedToSchedule(UseEI.UserTE->Scalars) &&
+ TEUseEI.UserTE->doesNotNeedToSchedule() !=
+ UseEI.UserTE->doesNotNeedToSchedule() &&
is_contained(UseEI.UserTE->Scalars, TEInsertPt))
continue;
// Check if the user node of the TE comes after user node of TEPtr,
@@ -16127,7 +16143,7 @@ void BoUpSLP::setInsertPointAfterBundle(const TreeEntry *E) {
}
if (IsPHI ||
(!E->isGather() && E->State != TreeEntry::SplitVectorize &&
- doesNotNeedToSchedule(E->Scalars)) ||
+ E->doesNotNeedToSchedule()) ||
(GatheredLoadsEntriesFirst.has_value() &&
E->Idx >= *GatheredLoadsEntriesFirst && !E->isGather() &&
E->getOpcode() == Instruction::Load)) {
@@ -19803,7 +19819,7 @@ void BoUpSLP::scheduleBlock(BlockScheduling *BS) {
if (ScheduleData *SD = BS->getScheduleData(I)) {
[[maybe_unused]] ArrayRef<TreeEntry *> SDTEs = getTreeEntries(I);
assert((isVectorLikeInstWithConstOps(SD->getInst()) || SDTEs.empty() ||
- doesNotNeedToSchedule(SDTEs.front()->Scalars)) &&
+ SDTEs.front()->doesNotNeedToSchedule()) &&
"scheduler and vectorizer bundle mismatch");
SD->setSchedulingPriority(Idx++);
continue;
|
RKSimon
approved these changes
May 20, 2025
alexey-bataev
deleted the
users/alexey-bataev/spr/slpnfcmake-treeentry-a-class-and-store-need-to-schedule-state
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 20, 2025
…le" state TreeEntry should be a class, not a struct, since it has private members. Also, do no repeat Does-Not-Need-To-Schedule analysis during codegen, codegen may affect the result of the analysis in future patches. Reviewers: hiraditya, HanKuanChen, RKSimon Reviewed By: RKSimon Pull Request: llvm/llvm-project#140734
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.
TreeEntry should be a class, not a struct, since it has private members.
Also, do no repeat Does-Not-Need-To-Schedule analysis during codegen,
codegen may affect the result of the analysis in future patches.