Skip to content

Commit 6d8b44a

Browse files
authored
[AMDGPU] [IGLP]: Fix assert (llvm#73710)
We can also re-enter IGLP mutation via later `SchedStage`s in the `GCNMaxOccupancySchedStrategy` . This is sort of NFC in that there is no changed behavior for the only current client of `IsReentry`
1 parent 0808be4 commit 6d8b44a

File tree

3 files changed

+32
-25
lines changed

3 files changed

+32
-25
lines changed

llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -345,13 +345,13 @@ class PipelineSolver {
345345
// return the number of edges missed.
346346
int addEdges(SmallVectorImpl<SchedGroup> &SyncPipeline, SUnit *SU, int SGID,
347347
std::vector<std::pair<SUnit *, SUnit *>> &AddedEdges);
348-
// Link the pipeline as if \p SU was in the SchedGroup with ID \p SGID. It
349-
// returns the cost (in terms of missed pipeline edges), and tracks the edges
350-
// added in \p AddedEdges
348+
/// Link the pipeline as if \p SU was in the SchedGroup with ID \p SGID. It
349+
/// returns the cost (in terms of missed pipeline edges), and tracks the edges
350+
/// added in \p AddedEdges
351351
template <typename T>
352352
int linkSUnit(SUnit *SU, int SGID,
353353
std::vector<std::pair<SUnit *, SUnit *>> &AddedEdges, T I, T E);
354-
// Remove the edges passed via \p AddedEdges
354+
/// Remove the edges passed via \p AddedEdges
355355
void removeEdges(const std::vector<std::pair<SUnit *, SUnit *>> &AddedEdges);
356356
// Convert the passed in maps to arrays for bidirectional iterators
357357
void convertSyncMapsToArrays();
@@ -847,11 +847,11 @@ class IGLPStrategy {
847847
const SIInstrInfo *TII;
848848

849849
public:
850-
// Add SchedGroups to \p Pipeline to implement this Strategy.
850+
/// Add SchedGroups to \p SyncedSchedGroups to implement this Strategy.
851851
virtual void applyIGLPStrategy(
852852
DenseMap<int, SUnitsToCandidateSGsMap> &SyncedInstrs,
853853
DenseMap<int, SmallVector<SchedGroup, 4>> &SyncedSchedGroups,
854-
bool IsPostRA) = 0;
854+
bool IsReentry) = 0;
855855

856856
// Returns true if this strategy should be applied to a ScheduleDAG.
857857
virtual bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) = 0;
@@ -870,7 +870,7 @@ class MFMASmallGemmOpt final : public IGLPStrategy {
870870
void applyIGLPStrategy(
871871
DenseMap<int, SUnitsToCandidateSGsMap> &SyncedInstrs,
872872
DenseMap<int, SmallVector<SchedGroup, 4>> &SyncedSchedGroups,
873-
bool IsPostRA) override;
873+
bool IsReentry) override;
874874

875875
bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) override { return true; }
876876

@@ -883,7 +883,7 @@ class MFMASmallGemmOpt final : public IGLPStrategy {
883883
void MFMASmallGemmOpt::applyIGLPStrategy(
884884
DenseMap<int, SUnitsToCandidateSGsMap> &SyncedInstrs,
885885
DenseMap<int, SmallVector<SchedGroup, 4>> &SyncedSchedGroups,
886-
bool IsPostRA) {
886+
bool IsReentry) {
887887
// Count the number of MFMA instructions.
888888
unsigned MFMACount = 0;
889889
for (const MachineInstr &I : *DAG)
@@ -1045,8 +1045,8 @@ class MFMASmallGemmSingleWaveOpt final : public IGLPStrategy {
10451045
: InstructionRule(TII, SGID, NeedsCache) {}
10461046
};
10471047

1048-
// Whether the SU shares a V_PERM predecessor with any SU in the SchedGroup
1049-
// that is /p Distance steps away
1048+
/// Whether the SU shares a V_PERM predecessor with any SU in the SchedGroup
1049+
/// that is \p Distance steps away
10501050
class SharesPredWithPrevNthGroup final : public InstructionRule {
10511051
private:
10521052
unsigned Distance = 1;
@@ -1100,7 +1100,7 @@ class MFMASmallGemmSingleWaveOpt final : public IGLPStrategy {
11001100
void applyIGLPStrategy(
11011101
DenseMap<int, SUnitsToCandidateSGsMap> &SyncedInstrs,
11021102
DenseMap<int, SmallVector<SchedGroup, 4>> &SyncedSchedGroups,
1103-
bool IsPostRA) override;
1103+
bool IsReentry) override;
11041104

11051105
bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) override { return true; }
11061106

@@ -1117,12 +1117,12 @@ static unsigned DSWWithSharedVMEMCount = 0;
11171117
void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy(
11181118
DenseMap<int, SUnitsToCandidateSGsMap> &SyncedInstrs,
11191119
DenseMap<int, SmallVector<SchedGroup, 4>> &SyncedSchedGroups,
1120-
bool IsPostRA) {
1120+
bool IsReentry) {
11211121
unsigned MFMACount = 0;
11221122
unsigned DSRCount = 0;
11231123

1124-
assert((IsPostRA || (DSWCount == 0 && DSWWithPermCount == 0 &&
1125-
DSWWithSharedVMEMCount == 0)) &&
1124+
assert((IsReentry || (DSWCount == 0 && DSWWithPermCount == 0 &&
1125+
DSWWithSharedVMEMCount == 0)) &&
11261126
"DSWCounters should be zero in pre-RA scheduling!");
11271127
SmallVector<SUnit *, 6> DSWithPerms;
11281128
for (auto &SU : DAG->SUnits) {
@@ -1132,7 +1132,7 @@ void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy(
11321132
else if (TII->isDS(*I)) {
11331133
if (I->mayLoad())
11341134
++DSRCount;
1135-
else if (I->mayStore() && !IsPostRA) {
1135+
else if (I->mayStore() && !IsReentry) {
11361136
++DSWCount;
11371137
for (auto Pred : SU.Preds) {
11381138
if (Pred.getSUnit()->getInstr()->getOpcode() ==
@@ -1145,7 +1145,7 @@ void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy(
11451145
}
11461146
}
11471147

1148-
if (!IsPostRA) {
1148+
if (!IsReentry) {
11491149
DSWWithPermCount = DSWithPerms.size();
11501150
auto I = DSWithPerms.begin();
11511151
auto E = DSWithPerms.end();
@@ -1412,11 +1412,11 @@ class IGroupLPDAGMutation : public ScheduleDAGMutation {
14121412
// first created SchedGroup first.
14131413
bool IsBottomUp = 1;
14141414

1415-
// Whether the mutation is being applied to post RA scheduling
1416-
bool IsPostRA = false;
1415+
// Whether or not this is a reentry into the IGroupLPDAGMutation.
1416+
bool IsReentry = false;
14171417

14181418
IGroupLPDAGMutation() = default;
1419-
IGroupLPDAGMutation(bool IsPostRA) : IsPostRA(IsPostRA) {}
1419+
IGroupLPDAGMutation(bool IsReentry) : IsReentry(IsReentry) {}
14201420
};
14211421

14221422
unsigned SchedGroup::NumSchedGroups = 0;
@@ -1704,16 +1704,21 @@ void IGroupLPDAGMutation::initIGLPOpt(SUnit &SU) {
17041704
auto S = createIGLPStrategy(StrategyID, DAG, TII);
17051705
if (S->shouldApplyStrategy(DAG)) {
17061706
IsBottomUp = S->IsBottomUp;
1707-
S->applyIGLPStrategy(SyncedInstrs, SyncedSchedGroups, IsPostRA);
1707+
S->applyIGLPStrategy(SyncedInstrs, SyncedSchedGroups, IsReentry);
17081708
}
17091709
}
17101710

17111711
} // namespace
17121712

17131713
namespace llvm {
17141714

1715-
std::unique_ptr<ScheduleDAGMutation> createIGroupLPDAGMutation(bool IsPostRA) {
1716-
return std::make_unique<IGroupLPDAGMutation>(IsPostRA);
1715+
/// \p IsReentry specifes whether or not this is a reentry into the
1716+
/// IGroupLPDAGMutation. Since there may be multiple scheduling passes on the
1717+
/// same scheduling region (e.g. pre and post-RA scheduling / multiple
1718+
/// scheduling "phases"), we can reenter this mutation framework more than once
1719+
/// for a given region.
1720+
std::unique_ptr<ScheduleDAGMutation> createIGroupLPDAGMutation(bool IsReentry) {
1721+
return std::make_unique<IGroupLPDAGMutation>(IsReentry);
17171722
}
17181723

17191724
} // end namespace llvm

llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
namespace llvm {
1616

17-
std::unique_ptr<ScheduleDAGMutation> createIGroupLPDAGMutation(bool IsPostRA);
17+
std::unique_ptr<ScheduleDAGMutation> createIGroupLPDAGMutation(bool IsReentry);
1818

1919
} // namespace llvm
2020

llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,9 @@ bool GCNSchedStage::initGCNRegion() {
853853
StageID != GCNSchedStageID::UnclusteredHighRPReschedule) {
854854
SavedMutations.clear();
855855
SavedMutations.swap(DAG.Mutations);
856-
DAG.addMutation(createIGroupLPDAGMutation(/*IsPostRA=*/false));
856+
bool IsInitialStage = StageID == GCNSchedStageID::OccInitialSchedule ||
857+
StageID == GCNSchedStageID::ILPInitialSchedule;
858+
DAG.addMutation(createIGroupLPDAGMutation(/*IsReentry=*/!IsInitialStage));
857859
}
858860

859861
return true;
@@ -1567,7 +1569,7 @@ void GCNPostScheduleDAGMILive::schedule() {
15671569
if (HasIGLPInstrs) {
15681570
SavedMutations.clear();
15691571
SavedMutations.swap(Mutations);
1570-
addMutation(createIGroupLPDAGMutation(/*IsPostRA=*/true));
1572+
addMutation(createIGroupLPDAGMutation(/*IsReentry=*/true));
15711573
}
15721574

15731575
ScheduleDAGMI::schedule();

0 commit comments

Comments
 (0)