Skip to content

Commit

Permalink
[SLP] Have only ready items in ready list [NFC]
Browse files Browse the repository at this point in the history
This adds the assertion that all items in the ready list are in-fact scheduleable entities ready to be scheduled.  This involves changing the ReadyInsts structure to be a set, and fixing a couple places where we left nodes on the list when they were no longer ready.
  • Loading branch information
preames committed Feb 4, 2022
1 parent 25d50a0 commit bb9964b
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
Expand Up @@ -2743,6 +2743,11 @@ class BoUpSLP {
"primary schedule data not in window?");
doForAllOpcodes(I, [](ScheduleData *SD) { SD->verify(); });
}

for (auto *SD : ReadyInsts) {
assert(SD->isSchedulingEntity() && SD->isReady() &&
"item in ready list not ready?");
}
}

void doForAllOpcodes(Value *V,
Expand All @@ -2764,7 +2769,7 @@ class BoUpSLP {
if (SD->isSchedulingEntity() && SD->isReady()) {
ReadyList.insert(SD);
LLVM_DEBUG(dbgs()
<< "SLP: initially in ready list: " << *I << "\n");
<< "SLP: initially in ready list: " << *SD << "\n");
}
});
}
Expand Down Expand Up @@ -2828,12 +2833,8 @@ class BoUpSLP {
DenseMap<Value *, SmallDenseMap<Value *, ScheduleData *>>
ExtraScheduleDataMap;

struct ReadyList : SmallVector<ScheduleData *, 8> {
void insert(ScheduleData *SD) { push_back(SD); }
};

/// The ready-list for scheduling (only used for the dry-run).
ReadyList ReadyInsts;
SetVector<ScheduleData *> ReadyInsts;

/// The first instruction of the scheduling region.
Instruction *ScheduleStart = nullptr;
Expand Down Expand Up @@ -7539,25 +7540,27 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
doForAllOpcodes(I, [](ScheduleData *SD) { SD->clearDependencies(); });
ReSchedule = true;
}
if (ReSchedule) {
resetSchedule();
initialFillReadyList(ReadyInsts);
}
if (Bundle) {
LLVM_DEBUG(dbgs() << "SLP: try schedule bundle " << *Bundle
<< " in block " << BB->getName() << "\n");
calculateDependencies(Bundle, /*InsertInReadyList=*/true, SLP);
}

if (ReSchedule) {
resetSchedule();
initialFillReadyList(ReadyInsts);
}

// Now try to schedule the new bundle or (if no bundle) just calculate
// dependencies. As soon as the bundle is "ready" it means that there are no
// cyclic dependencies and we can schedule it. Note that's important that we
// don't "schedule" the bundle yet (see cancelScheduling).
while (((!Bundle && ReSchedule) || (Bundle && !Bundle->isReady())) &&
!ReadyInsts.empty()) {
ScheduleData *Picked = ReadyInsts.pop_back_val();
if (Picked->isSchedulingEntity() && Picked->isReady())
schedule(Picked, ReadyInsts);
assert(Picked->isSchedulingEntity() && Picked->isReady() &&
"must be ready to schedule");
schedule(Picked, ReadyInsts);
}
};

Expand All @@ -7581,6 +7584,11 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
ScheduleData *BundleMember = getScheduleData(V);
assert(BundleMember &&
"no ScheduleData for bundle member (maybe not in same basic block)");

// Make sure we don't leave the pieces of the bundle in the ready list when
// whole bundle might not be ready.
ReadyInsts.remove(BundleMember);

if (!BundleMember->IsScheduled)
continue;
// A bundle member was scheduled as single instruction before and now
Expand Down Expand Up @@ -7612,6 +7620,10 @@ void BoUpSLP::BlockScheduling::cancelScheduling(ArrayRef<Value *> VL,
assert(Bundle->isSchedulingEntity() && Bundle->isPartOfBundle() &&
"tried to unbundle something which is not a bundle");

// Remove the bundle from the ready list.
if (Bundle->isReady())
ReadyInsts.remove(Bundle);

// Un-bundle: make single instructions out of the bundle.
ScheduleData *BundleMember = Bundle;
while (BundleMember) {
Expand Down Expand Up @@ -7861,7 +7873,7 @@ void BoUpSLP::BlockScheduling::calculateDependencies(ScheduleData *SD,
}
}
if (InsertInReadyList && SD->isReady()) {
ReadyInsts.push_back(SD);
ReadyInsts.insert(SD);
LLVM_DEBUG(dbgs() << "SLP: gets ready on update: " << *SD->Inst
<< "\n");
}
Expand Down

0 comments on commit bb9964b

Please sign in to comment.