From df9a03c021d6c5ebc287fba1df0d68f2287f2420 Mon Sep 17 00:00:00 2001 From: Jeffrey Byrnes Date: Thu, 7 Dec 2023 17:10:10 -0800 Subject: [PATCH] [AMDGPU] [IGLP]: Fix assert (#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` Change-Id: Iad948777862423288eb1d471961145bd51e07072 --- llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp | 49 ++++++++++++--------- llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h | 2 +- llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 6 ++- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp index 927e56c0496ecb..c79c083ea8d226 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp @@ -345,13 +345,13 @@ class PipelineSolver { // return the number of edges missed. int addEdges(SmallVectorImpl &SyncPipeline, SUnit *SU, int SGID, std::vector> &AddedEdges); - // Link the pipeline as if \p SU was in the SchedGroup with ID \p SGID. It - // returns the cost (in terms of missed pipeline edges), and tracks the edges - // added in \p AddedEdges + /// Link the pipeline as if \p SU was in the SchedGroup with ID \p SGID. It + /// returns the cost (in terms of missed pipeline edges), and tracks the edges + /// added in \p AddedEdges template int linkSUnit(SUnit *SU, int SGID, std::vector> &AddedEdges, T I, T E); - // Remove the edges passed via \p AddedEdges + /// Remove the edges passed via \p AddedEdges void removeEdges(const std::vector> &AddedEdges); // Convert the passed in maps to arrays for bidirectional iterators void convertSyncMapsToArrays(); @@ -851,11 +851,11 @@ class IGLPStrategy { const SIInstrInfo *TII; public: - // Add SchedGroups to \p Pipeline to implement this Strategy. + /// Add SchedGroups to \p SyncedSchedGroups to implement this Strategy. virtual void applyIGLPStrategy( DenseMap &SyncedInstrs, DenseMap> &SyncedSchedGroups, - bool IsPostRA) = 0; + bool IsReentry) = 0; // Returns true if this strategy should be applied to a ScheduleDAG. virtual bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) = 0; @@ -874,7 +874,7 @@ class MFMASmallGemmOpt final : public IGLPStrategy { void applyIGLPStrategy( DenseMap &SyncedInstrs, DenseMap> &SyncedSchedGroups, - bool IsPostRA) override; + bool IsReentry) override; bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) override { return true; } @@ -887,7 +887,7 @@ class MFMASmallGemmOpt final : public IGLPStrategy { void MFMASmallGemmOpt::applyIGLPStrategy( DenseMap &SyncedInstrs, DenseMap> &SyncedSchedGroups, - bool IsPostRA) { + bool IsReentry) { // Count the number of MFMA instructions. unsigned MFMACount = 0; for (const MachineInstr &I : *DAG) @@ -1050,8 +1050,8 @@ class MFMASmallGemmSingleWaveOpt final : public IGLPStrategy { : InstructionRule(TII, SGID, NeedsCache) {} }; - // Whether the SU shares a V_PERM predecessor with any SU in the SchedGroup - // that is /p Distance steps away + /// Whether the SU shares a V_PERM predecessor with any SU in the SchedGroup + /// that is \p Distance steps away class SharesPredWithPrevNthGroup final : public InstructionRule { private: unsigned Distance = 1; @@ -1106,7 +1106,7 @@ class MFMASmallGemmSingleWaveOpt final : public IGLPStrategy { void applyIGLPStrategy( DenseMap &SyncedInstrs, DenseMap> &SyncedSchedGroups, - bool IsPostRA) override; + bool IsReentry) override; bool shouldApplyStrategy(ScheduleDAGInstrs *DAG) override { return true; } @@ -1123,12 +1123,12 @@ static unsigned DSWWithSharedVMEMCount = 0; void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy( DenseMap &SyncedInstrs, DenseMap> &SyncedSchedGroups, - bool IsPostRA) { + bool IsReentry) { unsigned MFMACount = 0; unsigned DSRCount = 0; - assert((IsPostRA || (DSWCount == 0 && DSWWithPermCount == 0 && - DSWWithSharedVMEMCount == 0)) && + assert((IsReentry || (DSWCount == 0 && DSWWithPermCount == 0 && + DSWWithSharedVMEMCount == 0)) && "DSWCounters should be zero in pre-RA scheduling!"); SmallVector DSWithPerms; for (auto &SU : DAG->SUnits) { @@ -1138,7 +1138,7 @@ void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy( else if (TII->isDS(*I)) { if (I->mayLoad()) ++DSRCount; - else if (I->mayStore() && !IsPostRA) { + else if (I->mayStore() && !IsReentry) { ++DSWCount; for (auto Pred : SU.Preds) { if (Pred.getSUnit()->getInstr()->getOpcode() == @@ -1151,7 +1151,7 @@ void MFMASmallGemmSingleWaveOpt::applyIGLPStrategy( } } - if (!IsPostRA) { + if (!IsReentry) { DSWWithPermCount = DSWithPerms.size(); auto I = DSWithPerms.begin(); auto E = DSWithPerms.end(); @@ -1418,11 +1418,11 @@ class IGroupLPDAGMutation : public ScheduleDAGMutation { // first created SchedGroup first. bool IsBottomUp = 1; - // Whether the mutation is being applied to post RA scheduling - bool IsPostRA = false; + // Whether or not this is a reentry into the IGroupLPDAGMutation. + bool IsReentry = false; IGroupLPDAGMutation() = default; - IGroupLPDAGMutation(bool IsPostRA) : IsPostRA(IsPostRA) {} + IGroupLPDAGMutation(bool IsReentry) : IsReentry(IsReentry) {} }; unsigned SchedGroup::NumSchedGroups = 0; @@ -1710,7 +1710,7 @@ void IGroupLPDAGMutation::initIGLPOpt(SUnit &SU) { auto S = createIGLPStrategy(StrategyID, DAG, TII); if (S->shouldApplyStrategy(DAG)) { IsBottomUp = S->IsBottomUp; - S->applyIGLPStrategy(SyncedInstrs, SyncedSchedGroups, IsPostRA); + S->applyIGLPStrategy(SyncedInstrs, SyncedSchedGroups, IsReentry); } } @@ -1718,8 +1718,13 @@ void IGroupLPDAGMutation::initIGLPOpt(SUnit &SU) { namespace llvm { -std::unique_ptr createIGroupLPDAGMutation(bool IsPostRA) { - return std::make_unique(IsPostRA); +/// \p IsReentry specifes whether or not this is a reentry into the +/// IGroupLPDAGMutation. Since there may be multiple scheduling passes on the +/// same scheduling region (e.g. pre and post-RA scheduling / multiple +/// scheduling "phases"), we can reenter this mutation framework more than once +/// for a given region. +std::unique_ptr createIGroupLPDAGMutation(bool IsReentry) { + return std::make_unique(IsReentry); } } // end namespace llvm diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h index eee2a48de396ff..3ec8be4f889205 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.h @@ -14,7 +14,7 @@ namespace llvm { -std::unique_ptr createIGroupLPDAGMutation(bool IsPostRA); +std::unique_ptr createIGroupLPDAGMutation(bool IsReentry); } // namespace llvm diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp index f77ecba01e20a7..0daf26610461e6 100644 --- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp +++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp @@ -842,7 +842,9 @@ bool GCNSchedStage::initGCNRegion() { StageID != GCNSchedStageID::UnclusteredHighRPReschedule) { SavedMutations.clear(); SavedMutations.swap(DAG.Mutations); - DAG.addMutation(createIGroupLPDAGMutation(/*IsPostRA=*/false)); + bool IsInitialStage = StageID == GCNSchedStageID::OccInitialSchedule || + StageID == GCNSchedStageID::ILPInitialSchedule; + DAG.addMutation(createIGroupLPDAGMutation(/*IsReentry=*/!IsInitialStage)); } return true; } @@ -1555,7 +1557,7 @@ void GCNPostScheduleDAGMILive::schedule() { if (HasIGLPInstrs) { SavedMutations.clear(); SavedMutations.swap(Mutations); - addMutation(createIGroupLPDAGMutation(/*IsPostRA=*/true)); + addMutation(createIGroupLPDAGMutation(/*IsReentry=*/true)); } ScheduleDAGMI::schedule();