[SandboxVec][SeedCollector] Implement collection of seeds with different types #146171
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
Member
|
@llvm/pr-subscribers-llvm-transforms Author: vporpo (vporpo) ChangesUp until now the seed collector could only collect seeds with the same element type. For example, Full diff: https://github.com/llvm/llvm-project/pull/146171.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
index ec70350691abe..b289520fa83af 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
@@ -191,7 +191,8 @@ class SeedContainer {
ScalarEvolution &SE;
- template <typename LoadOrStoreT> KeyT getKey(LoadOrStoreT *LSI) const;
+ template <typename LoadOrStoreT>
+ KeyT getKey(LoadOrStoreT *LSI, bool AllowDiffTypes) const;
public:
SeedContainer(ScalarEvolution &SE) : SE(SE) {}
@@ -267,7 +268,8 @@ class SeedContainer {
bool operator!=(const iterator &Other) const { return !(*this == Other); }
};
using const_iterator = BundleMapT::const_iterator;
- template <typename LoadOrStoreT> void insert(LoadOrStoreT *LSI);
+ template <typename LoadOrStoreT>
+ void insert(LoadOrStoreT *LSI, bool AllowDiffTypes);
// To support constant-time erase, these just mark the element used, rather
// than actually removing them from the bundle.
LLVM_ABI bool erase(Instruction *I);
@@ -291,9 +293,9 @@ class SeedContainer {
// Explicit instantiations
extern template LLVM_TEMPLATE_ABI void
-SeedContainer::insert<LoadInst>(LoadInst *);
+SeedContainer::insert<LoadInst>(LoadInst *, bool);
extern template LLVM_TEMPLATE_ABI void
-SeedContainer::insert<StoreInst>(StoreInst *);
+SeedContainer::insert<StoreInst>(StoreInst *, bool);
class SeedCollector {
SeedContainer StoreSeeds;
@@ -308,7 +310,8 @@ class SeedCollector {
public:
LLVM_ABI SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
- bool CollectStores, bool CollectLoads);
+ bool CollectStores, bool CollectLoads,
+ bool AllowDiffTypes = false);
LLVM_ABI ~SeedCollector();
iterator_range<SeedContainer::iterator> getStoreSeeds() {
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
index 1934326866cdf..815dda38b0b28 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
@@ -73,22 +73,28 @@ ArrayRef<Instruction *> SeedBundle::getSlice(unsigned StartIdx,
}
template <typename LoadOrStoreT>
-SeedContainer::KeyT SeedContainer::getKey(LoadOrStoreT *LSI) const {
+SeedContainer::KeyT SeedContainer::getKey(LoadOrStoreT *LSI,
+ bool AllowDiffTypes) const {
assert((isa<LoadInst>(LSI) || isa<StoreInst>(LSI)) &&
"Expected Load or Store!");
Value *Ptr = Utils::getMemInstructionBase(LSI);
Instruction::Opcode Op = LSI->getOpcode();
- Type *Ty = Utils::getExpectedType(LSI);
- if (auto *VTy = dyn_cast<VectorType>(Ty))
- Ty = VTy->getElementType();
+ Type *Ty;
+ if (AllowDiffTypes) {
+ Ty = nullptr;
+ } else {
+ Ty = Utils::getExpectedType(LSI);
+ if (auto *VTy = dyn_cast<VectorType>(Ty))
+ Ty = VTy->getElementType();
+ }
return {Ptr, Ty, Op};
}
// Explicit instantiations
template SeedContainer::KeyT
-SeedContainer::getKey<LoadInst>(LoadInst *LSI) const;
+SeedContainer::getKey<LoadInst>(LoadInst *LSI, bool AllowDiffTypes) const;
template SeedContainer::KeyT
-SeedContainer::getKey<StoreInst>(StoreInst *LSI) const;
+SeedContainer::getKey<StoreInst>(StoreInst *LSI, bool AllowDiffTypes) const;
bool SeedContainer::erase(Instruction *I) {
assert((isa<LoadInst>(I) || isa<StoreInst>(I)) && "Expected Load or Store!");
@@ -100,9 +106,10 @@ bool SeedContainer::erase(Instruction *I) {
return true;
}
-template <typename LoadOrStoreT> void SeedContainer::insert(LoadOrStoreT *LSI) {
+template <typename LoadOrStoreT>
+void SeedContainer::insert(LoadOrStoreT *LSI, bool AllowDiffTypes) {
// Find the bundle containing seeds for this symbol and type-of-access.
- auto &BundleVec = Bundles[getKey(LSI)];
+ auto &BundleVec = Bundles[getKey(LSI, AllowDiffTypes)];
// Fill this vector of bundles front to back so that only the last bundle in
// the vector may have available space. This avoids iteration to find one with
// space.
@@ -115,9 +122,9 @@ template <typename LoadOrStoreT> void SeedContainer::insert(LoadOrStoreT *LSI) {
}
// Explicit instantiations
-template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<LoadInst>(LoadInst *);
-template LLVM_EXPORT_TEMPLATE void
-SeedContainer::insert<StoreInst>(StoreInst *);
+template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<LoadInst>(LoadInst *, bool);
+template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<StoreInst>(StoreInst *,
+ bool);
#ifndef NDEBUG
void SeedContainer::print(raw_ostream &OS) const {
@@ -158,7 +165,8 @@ template bool isValidMemSeed<LoadInst>(LoadInst *LSI);
template bool isValidMemSeed<StoreInst>(StoreInst *LSI);
SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
- bool CollectStores, bool CollectLoads)
+ bool CollectStores, bool CollectLoads,
+ bool AllowDiffTypes)
: StoreSeeds(SE), LoadSeeds(SE), Ctx(BB->getContext()) {
if (!CollectStores && !CollectLoads)
@@ -175,10 +183,10 @@ SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
for (auto &I : *BB) {
if (StoreInst *SI = dyn_cast<StoreInst>(&I))
if (CollectStores && isValidMemSeed(SI))
- StoreSeeds.insert(SI);
+ StoreSeeds.insert(SI, AllowDiffTypes);
if (LoadInst *LI = dyn_cast<LoadInst>(&I))
if (CollectLoads && isValidMemSeed(LI))
- LoadSeeds.insert(LI);
+ LoadSeeds.insert(LI, AllowDiffTypes);
// Cap compilation time.
if (totalNumSeedGroups() > SeedGroupsLimit)
break;
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
index 7f9a59bd428a0..31b4a5ec9e391 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
@@ -259,10 +259,10 @@ define void @foo(ptr %ptrA, float %val, ptr %ptrB) {
// Check begin() end() when empty.
EXPECT_EQ(SC.begin(), SC.end());
- SC.insert(S0);
- SC.insert(S1);
- SC.insert(S2);
- SC.insert(S3);
+ SC.insert(S0, /*AllowDiffTypes=*/false);
+ SC.insert(S1, /*AllowDiffTypes=*/false);
+ SC.insert(S2, /*AllowDiffTypes=*/false);
+ SC.insert(S3, /*AllowDiffTypes=*/false);
unsigned Cnt = 0;
SmallVector<sandboxir::SeedBundle *> Bndls;
for (auto &SeedBndl : SC) {
@@ -480,6 +480,45 @@ define void @foo(ptr noalias %ptr, float %v, <2 x float> %val) {
ExpectThatElementsAre(SB, {St0, St1, St3});
}
+TEST_F(SeedBundleTest, DiffTypes) {
+ parseIR(C, R"IR(
+define void @foo(ptr noalias %ptr, i8 %v, i16 %v16) {
+bb:
+ %ptr0 = getelementptr i8, ptr %ptr, i32 0
+ %ptr1 = getelementptr i8, ptr %ptr, i32 1
+ %ptr3 = getelementptr i8, ptr %ptr, i32 3
+ store i8 %v, ptr %ptr0
+ store i8 %v, ptr %ptr3
+ store i16 %v16, ptr %ptr1
+ ret void
+}
+)IR");
+ Function &LLVMF = *M->getFunction("foo");
+ DominatorTree DT(LLVMF);
+ TargetLibraryInfoImpl TLII(M->getTargetTriple());
+ TargetLibraryInfo TLI(TLII);
+ DataLayout DL(M->getDataLayout());
+ LoopInfo LI(DT);
+ AssumptionCache AC(LLVMF);
+ ScalarEvolution SE(LLVMF, TLI, AC, DT, LI);
+
+ sandboxir::Context Ctx(C);
+ auto &F = *Ctx.createFunction(&LLVMF);
+ auto BB = F.begin();
+ auto It = std::next(BB->begin(), 3);
+ auto *St0 = &*It++;
+ auto *St3 = &*It++;
+ auto *St1 = &*It++;
+
+ sandboxir::SeedCollector SC(&*BB, SE, /*CollectStores=*/true,
+ /*CollectLoads=*/false, /*AllowDiffTypes=*/true);
+
+ auto StoreSeedsRange = SC.getStoreSeeds();
+ EXPECT_EQ(range_size(StoreSeedsRange), 1u);
+ auto &SB = *StoreSeedsRange.begin();
+ ExpectThatElementsAre(SB, {St0, St1, St3});
+}
+
TEST_F(SeedBundleTest, VectorLoads) {
parseIR(C, R"IR(
define void @foo(ptr noalias %ptr, <2 x float> %val0) {
|
Member
|
@llvm/pr-subscribers-vectorizers Author: vporpo (vporpo) ChangesUp until now the seed collector could only collect seeds with the same element type. For example, Full diff: https://github.com/llvm/llvm-project/pull/146171.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
index ec70350691abe..b289520fa83af 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SeedCollector.h
@@ -191,7 +191,8 @@ class SeedContainer {
ScalarEvolution &SE;
- template <typename LoadOrStoreT> KeyT getKey(LoadOrStoreT *LSI) const;
+ template <typename LoadOrStoreT>
+ KeyT getKey(LoadOrStoreT *LSI, bool AllowDiffTypes) const;
public:
SeedContainer(ScalarEvolution &SE) : SE(SE) {}
@@ -267,7 +268,8 @@ class SeedContainer {
bool operator!=(const iterator &Other) const { return !(*this == Other); }
};
using const_iterator = BundleMapT::const_iterator;
- template <typename LoadOrStoreT> void insert(LoadOrStoreT *LSI);
+ template <typename LoadOrStoreT>
+ void insert(LoadOrStoreT *LSI, bool AllowDiffTypes);
// To support constant-time erase, these just mark the element used, rather
// than actually removing them from the bundle.
LLVM_ABI bool erase(Instruction *I);
@@ -291,9 +293,9 @@ class SeedContainer {
// Explicit instantiations
extern template LLVM_TEMPLATE_ABI void
-SeedContainer::insert<LoadInst>(LoadInst *);
+SeedContainer::insert<LoadInst>(LoadInst *, bool);
extern template LLVM_TEMPLATE_ABI void
-SeedContainer::insert<StoreInst>(StoreInst *);
+SeedContainer::insert<StoreInst>(StoreInst *, bool);
class SeedCollector {
SeedContainer StoreSeeds;
@@ -308,7 +310,8 @@ class SeedCollector {
public:
LLVM_ABI SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
- bool CollectStores, bool CollectLoads);
+ bool CollectStores, bool CollectLoads,
+ bool AllowDiffTypes = false);
LLVM_ABI ~SeedCollector();
iterator_range<SeedContainer::iterator> getStoreSeeds() {
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
index 1934326866cdf..815dda38b0b28 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SeedCollector.cpp
@@ -73,22 +73,28 @@ ArrayRef<Instruction *> SeedBundle::getSlice(unsigned StartIdx,
}
template <typename LoadOrStoreT>
-SeedContainer::KeyT SeedContainer::getKey(LoadOrStoreT *LSI) const {
+SeedContainer::KeyT SeedContainer::getKey(LoadOrStoreT *LSI,
+ bool AllowDiffTypes) const {
assert((isa<LoadInst>(LSI) || isa<StoreInst>(LSI)) &&
"Expected Load or Store!");
Value *Ptr = Utils::getMemInstructionBase(LSI);
Instruction::Opcode Op = LSI->getOpcode();
- Type *Ty = Utils::getExpectedType(LSI);
- if (auto *VTy = dyn_cast<VectorType>(Ty))
- Ty = VTy->getElementType();
+ Type *Ty;
+ if (AllowDiffTypes) {
+ Ty = nullptr;
+ } else {
+ Ty = Utils::getExpectedType(LSI);
+ if (auto *VTy = dyn_cast<VectorType>(Ty))
+ Ty = VTy->getElementType();
+ }
return {Ptr, Ty, Op};
}
// Explicit instantiations
template SeedContainer::KeyT
-SeedContainer::getKey<LoadInst>(LoadInst *LSI) const;
+SeedContainer::getKey<LoadInst>(LoadInst *LSI, bool AllowDiffTypes) const;
template SeedContainer::KeyT
-SeedContainer::getKey<StoreInst>(StoreInst *LSI) const;
+SeedContainer::getKey<StoreInst>(StoreInst *LSI, bool AllowDiffTypes) const;
bool SeedContainer::erase(Instruction *I) {
assert((isa<LoadInst>(I) || isa<StoreInst>(I)) && "Expected Load or Store!");
@@ -100,9 +106,10 @@ bool SeedContainer::erase(Instruction *I) {
return true;
}
-template <typename LoadOrStoreT> void SeedContainer::insert(LoadOrStoreT *LSI) {
+template <typename LoadOrStoreT>
+void SeedContainer::insert(LoadOrStoreT *LSI, bool AllowDiffTypes) {
// Find the bundle containing seeds for this symbol and type-of-access.
- auto &BundleVec = Bundles[getKey(LSI)];
+ auto &BundleVec = Bundles[getKey(LSI, AllowDiffTypes)];
// Fill this vector of bundles front to back so that only the last bundle in
// the vector may have available space. This avoids iteration to find one with
// space.
@@ -115,9 +122,9 @@ template <typename LoadOrStoreT> void SeedContainer::insert(LoadOrStoreT *LSI) {
}
// Explicit instantiations
-template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<LoadInst>(LoadInst *);
-template LLVM_EXPORT_TEMPLATE void
-SeedContainer::insert<StoreInst>(StoreInst *);
+template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<LoadInst>(LoadInst *, bool);
+template LLVM_EXPORT_TEMPLATE void SeedContainer::insert<StoreInst>(StoreInst *,
+ bool);
#ifndef NDEBUG
void SeedContainer::print(raw_ostream &OS) const {
@@ -158,7 +165,8 @@ template bool isValidMemSeed<LoadInst>(LoadInst *LSI);
template bool isValidMemSeed<StoreInst>(StoreInst *LSI);
SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
- bool CollectStores, bool CollectLoads)
+ bool CollectStores, bool CollectLoads,
+ bool AllowDiffTypes)
: StoreSeeds(SE), LoadSeeds(SE), Ctx(BB->getContext()) {
if (!CollectStores && !CollectLoads)
@@ -175,10 +183,10 @@ SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE,
for (auto &I : *BB) {
if (StoreInst *SI = dyn_cast<StoreInst>(&I))
if (CollectStores && isValidMemSeed(SI))
- StoreSeeds.insert(SI);
+ StoreSeeds.insert(SI, AllowDiffTypes);
if (LoadInst *LI = dyn_cast<LoadInst>(&I))
if (CollectLoads && isValidMemSeed(LI))
- LoadSeeds.insert(LI);
+ LoadSeeds.insert(LI, AllowDiffTypes);
// Cap compilation time.
if (totalNumSeedGroups() > SeedGroupsLimit)
break;
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
index 7f9a59bd428a0..31b4a5ec9e391 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SeedCollectorTest.cpp
@@ -259,10 +259,10 @@ define void @foo(ptr %ptrA, float %val, ptr %ptrB) {
// Check begin() end() when empty.
EXPECT_EQ(SC.begin(), SC.end());
- SC.insert(S0);
- SC.insert(S1);
- SC.insert(S2);
- SC.insert(S3);
+ SC.insert(S0, /*AllowDiffTypes=*/false);
+ SC.insert(S1, /*AllowDiffTypes=*/false);
+ SC.insert(S2, /*AllowDiffTypes=*/false);
+ SC.insert(S3, /*AllowDiffTypes=*/false);
unsigned Cnt = 0;
SmallVector<sandboxir::SeedBundle *> Bndls;
for (auto &SeedBndl : SC) {
@@ -480,6 +480,45 @@ define void @foo(ptr noalias %ptr, float %v, <2 x float> %val) {
ExpectThatElementsAre(SB, {St0, St1, St3});
}
+TEST_F(SeedBundleTest, DiffTypes) {
+ parseIR(C, R"IR(
+define void @foo(ptr noalias %ptr, i8 %v, i16 %v16) {
+bb:
+ %ptr0 = getelementptr i8, ptr %ptr, i32 0
+ %ptr1 = getelementptr i8, ptr %ptr, i32 1
+ %ptr3 = getelementptr i8, ptr %ptr, i32 3
+ store i8 %v, ptr %ptr0
+ store i8 %v, ptr %ptr3
+ store i16 %v16, ptr %ptr1
+ ret void
+}
+)IR");
+ Function &LLVMF = *M->getFunction("foo");
+ DominatorTree DT(LLVMF);
+ TargetLibraryInfoImpl TLII(M->getTargetTriple());
+ TargetLibraryInfo TLI(TLII);
+ DataLayout DL(M->getDataLayout());
+ LoopInfo LI(DT);
+ AssumptionCache AC(LLVMF);
+ ScalarEvolution SE(LLVMF, TLI, AC, DT, LI);
+
+ sandboxir::Context Ctx(C);
+ auto &F = *Ctx.createFunction(&LLVMF);
+ auto BB = F.begin();
+ auto It = std::next(BB->begin(), 3);
+ auto *St0 = &*It++;
+ auto *St3 = &*It++;
+ auto *St1 = &*It++;
+
+ sandboxir::SeedCollector SC(&*BB, SE, /*CollectStores=*/true,
+ /*CollectLoads=*/false, /*AllowDiffTypes=*/true);
+
+ auto StoreSeedsRange = SC.getStoreSeeds();
+ EXPECT_EQ(range_size(StoreSeedsRange), 1u);
+ auto &SB = *StoreSeedsRange.begin();
+ ExpectThatElementsAre(SB, {St0, St1, St3});
+}
+
TEST_F(SeedBundleTest, VectorLoads) {
parseIR(C, R"IR(
define void @foo(ptr noalias %ptr, <2 x float> %val0) {
|
…ent types Up until now the seed collector could only collect seeds with the same element type. For example, `i32` and <2 x i32>`. This patch implements the collection of seeds with different types, like `i32` and `i8`.
Contributor
Author
|
ping |
2 similar comments
Contributor
Author
|
ping |
Contributor
Author
|
ping |
slackito
reviewed
Aug 11, 2025
Collaborator
slackito
left a comment
slackito
left a comment
There was a problem hiding this comment.
Apologies for the extremely late review.
So, if I understand correctly, the way this works is by making SeedContainer::getKey use a nullptr for the Type part of the key, so seeds of all types will get bunched into the same key in the container, right? And then the rest of it is just plumbing to pass the AllowDiffTypes flag around as needed.
|
|
||
| template <typename LoadOrStoreT> KeyT getKey(LoadOrStoreT *LSI) const; | ||
| template <typename LoadOrStoreT> | ||
| KeyT getKey(LoadOrStoreT *LSI, bool AllowDiffTypes) const; |
Collaborator
There was a problem hiding this comment.
What's the motivation for having AllowDiffTypes as a configurable thing? Wouldn't we want it to be always on to enable more vectorization opportunities?
Contributor
Author
There was a problem hiding this comment.
Allowing different types is only beneficial for very specific (and limited) vectorization patterns, like load-store pairs of different types. In the general case we can't really vectorize instructions of different types unless we legally convert them into the same type. So allowing different types is not always desired, which is why we have this as an option.
Collaborator
There was a problem hiding this comment.
Thanks for the explanation!
Contributor
Author
|
Apologies for the late reply.
Correct. Since we are no longer separating the seeds according to type we are using nullptr as the key. |
slackito
approved these changes
Aug 19, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/11031 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
4 participants
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.
Up until now the seed collector could only collect seeds with the same element type. For example,
i32and <2 x i32>. This patch implements the collection of seeds with different types, likei32andi8`.