Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ class SeedContainer {
/// Note that the bundles themselves may have additional ordering, created
/// by the subclasses by insertAt. The bundles themselves may also have used
/// instructions.

// TODO: Range_size counts fully used-bundles. Further, iterating over
// anything other than the Bundles in a SeedContainer includes used seeds,
// so for now just check that removing all the seeds from a bundle also
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sentence "so for now just check that removing all the seeds from a bundle also empties the bundle" seems to refer to the test.

// empties the bundle. Rework the iterator logic to clean this up.
iterator(BundleMapT &Map, BundleMapT::iterator MapIt, ValT *Vec, int VecIdx)
: Map(&Map), MapIt(MapIt), Vec(Vec), VecIdx(VecIdx) {}
value_type &operator*() {
Expand Down Expand Up @@ -288,7 +293,7 @@ class SeedCollector {
SeedContainer StoreSeeds;
SeedContainer LoadSeeds;
Context &Ctx;

Context::CallbackID EraseCallbackID;
/// \Returns the number of SeedBundle groups for all seed types.
/// This is to be used for limiting compilation time.
unsigned totalNumSeedGroups() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,19 @@ template bool isValidMemSeed<StoreInst>(StoreInst *LSI);

SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE)
: StoreSeeds(SE), LoadSeeds(SE), Ctx(BB->getContext()) {
// TODO: Register a callback for updating the Collector data structures upon
// instr removal

bool CollectStores = CollectSeeds.find(StoreSeedsDef) != std::string::npos;
bool CollectLoads = CollectSeeds.find(LoadSeedsDef) != std::string::npos;
if (!CollectStores && !CollectLoads)
return;

EraseCallbackID = Ctx.registerEraseInstrCallback([this](Instruction *I) {
if (auto SI = dyn_cast<StoreInst>(I))
StoreSeeds.erase(SI);
else if (auto LI = dyn_cast<LoadInst>(I))
LoadSeeds.erase(LI);
});

// Actually collect the seeds.
for (auto &I : *BB) {
if (StoreInst *SI = dyn_cast<StoreInst>(&I))
Expand All @@ -181,8 +187,7 @@ SeedCollector::SeedCollector(BasicBlock *BB, ScalarEvolution &SE)
}

SeedCollector::~SeedCollector() {
// TODO: Unregister the callback for updating the seed datastructures upon
// instr removal
Ctx.unregisterEraseInstrCallback(EraseCallbackID);
}

#ifndef NDEBUG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,21 @@ define void @foo(ptr noalias %ptr, float %val) {
// Expect just one vector of store seeds
EXPECT_EQ(range_size(StoreSeedsRange), 1u);
ExpectThatElementsAre(SB, {St0, St1, St2, St3});
// Check that the EraseInstr callback works. TODO: Range_size counts fully
// used-bundles even though the iterator skips them. Further, iterating over
// anything other than the Bundles in a SeedContainer includes used seeds. So
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: TODO: in a separate line.
also double whitespace before "So".

// for now just check that removing all the seeds from a bundle also empties
// the bundle.
St0->eraseFromParent();
St1->eraseFromParent();
St2->eraseFromParent();
St3->eraseFromParent();
size_t nonEmptyBundleCount = 0;
for (auto &B : SC.getStoreSeeds()) {
(void)B;
nonEmptyBundleCount++;
}
EXPECT_EQ(nonEmptyBundleCount, 0u);
}

TEST_F(SeedBundleTest, VectorStores) {
Expand Down
Loading