[llvm] [AMDGPU] IGroupLP: Fix BestCost assignment in greedy solver (NFC) (PR #186995)
Frederik Harwath via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 20 07:11:07 PDT 2026
https://github.com/frederik-h updated https://github.com/llvm/llvm-project/pull/186995
>From 28bee97420fd459099b6146a10ec9d14752dca77 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <fharwath at amd.com>
Date: Tue, 17 Mar 2026 06:30:35 -0400
Subject: [PATCH 1/3] [AMDGPU] IGroupLP: Fix BestCost assignment (NFC)
The greedy solver's greedyFind incorrectly reports the cost of the
last processed group instead of the best one. In practice, this does
not have any effect since (1) the cost is only used to decide whether
or not to run the exact solver and for this it only matters if it is
zero or not, and (2) the edges of the best group are used correctly.
Use the best group cost and add debug output which always outputs the
greedy solver's overall cost.
---
llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
index e13147e9461eb..583235f5bb760 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
@@ -694,7 +694,6 @@ void PipelineSolver::greedyFind(
std::list<std::pair<SUnit *, SUnit *>> &AddedEdges, T I, T E) {
SUToCandSGsPair CurrSU = PipelineInstrs[CurrSyncGroupIdx][CurrConflInstNo];
int BestNodeCost = -1;
- int TempCost;
SchedGroup *BestGroup = nullptr;
int BestGroupID = -1;
std::list<std::pair<SUnit *, SUnit *>> BestEdges;
@@ -726,7 +725,7 @@ void PipelineSolver::greedyFind(
}
std::list<std::pair<SUnit *, SUnit *>> TempEdges;
- TempCost = addEdges(SyncPipeline, CurrSU.first, CandSGID, TempEdges);
+ int TempCost = addEdges(SyncPipeline, CurrSU.first, CandSGID, TempEdges);
LLVM_DEBUG(dbgs() << "Cost of Group " << TempCost << "\n");
if (TempCost < BestNodeCost || BestNodeCost == -1) {
@@ -756,7 +755,7 @@ void PipelineSolver::greedyFind(
LLVM_DEBUG(dbgs() << "Best Group has ID: " << BestGroupID << " and Mask"
<< (int)BestGroup->getMask() << "\n");
- BestCost += TempCost;
+ BestCost += BestNodeCost;
} else
BestCost += MissPenalty;
@@ -812,6 +811,7 @@ void PipelineSolver::solve() {
} else { // Use the Greedy Algorithm by default
LLVM_DEBUG(dbgs() << "Starting GREEDY pipeline solver\n");
solveGreedy();
+ LLVM_DEBUG(dbgs() << "Greedy produced best cost of " << BestCost << "\n");
}
makePipeline();
>From 104217de70b69fd2aad94fa5b0e00b812d1013b9 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <fharwath at amd.com>
Date: Tue, 17 Mar 2026 06:58:43 -0400
Subject: [PATCH 2/3] [AMDGPU] IGroupLP: Use struct for best group info in
greedyFind
---
llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp | 43 ++++++++++++-----------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
index 583235f5bb760..b5427f7acb9b4 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
@@ -693,10 +693,14 @@ template <typename T>
void PipelineSolver::greedyFind(
std::list<std::pair<SUnit *, SUnit *>> &AddedEdges, T I, T E) {
SUToCandSGsPair CurrSU = PipelineInstrs[CurrSyncGroupIdx][CurrConflInstNo];
- int BestNodeCost = -1;
- SchedGroup *BestGroup = nullptr;
- int BestGroupID = -1;
- std::list<std::pair<SUnit *, SUnit *>> BestEdges;
+
+ struct GroupInfo {
+ SchedGroup *SG;
+ int Cost = 0;
+ std::list<std::pair<SUnit *, SUnit *>> Edges;
+ };
+ std::optional<GroupInfo> Best;
+
auto &SyncPipeline = CurrPipeline[CurrSyncGroupIdx];
LLVM_DEBUG(dbgs() << "Fitting SU(" << CurrSU.first->NodeNum
<< ") in Pipeline # " << CurrSyncGroupIdx << "\n");
@@ -728,34 +732,33 @@ void PipelineSolver::greedyFind(
int TempCost = addEdges(SyncPipeline, CurrSU.first, CandSGID, TempEdges);
LLVM_DEBUG(dbgs() << "Cost of Group " << TempCost << "\n");
- if (TempCost < BestNodeCost || BestNodeCost == -1) {
- BestEdges = TempEdges;
- BestGroup = Match;
- BestNodeCost = TempCost;
- BestGroupID = CandSGID;
-
- if (BestNodeCost == 0)
+ if (!Best.has_value() || TempCost < Best->Cost) {
+ Best = {Match, TempCost, TempEdges};
+ if (Best->Cost == 0)
break;
}
removeEdges(TempEdges);
}
- if (BestGroupID != -1) {
- BestGroup->add(*CurrSU.first);
+ if (Best.has_value()) {
+ SchedGroup *SG = Best->SG;
+ std::list<std::pair<SUnit *, SUnit *>> &Edges = Best->Edges;
+
+ SG->add(*CurrSU.first);
if (AddedEdges.empty())
- AddedEdges = BestEdges;
+ AddedEdges = Edges;
else
- AddedEdges.splice(std::prev(AddedEdges.cend()), BestEdges);
+ AddedEdges.splice(std::prev(AddedEdges.cend()), Edges);
- for (const std::pair<SUnit *, SUnit *> &E : BestEdges) {
- if (!BestGroup->tryAddEdge(E.first, E.second))
+ for (const std::pair<SUnit *, SUnit *> &E : Edges) {
+ if (!SG->tryAddEdge(E.first, E.second))
llvm_unreachable("Edges known to be insertable.");
}
- LLVM_DEBUG(dbgs() << "Best Group has ID: " << BestGroupID << " and Mask"
- << (int)BestGroup->getMask() << "\n");
- BestCost += BestNodeCost;
+ LLVM_DEBUG(dbgs() << "Best Group has ID: " << SG->getSGID() << " and Mask"
+ << (int)SG->getMask() << "\n");
+ BestCost += Best->Cost;
} else
BestCost += MissPenalty;
>From 27c623f763a1b4e224accd7239791e5218406ad0 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <fharwath at amd.com>
Date: Fri, 20 Mar 2026 10:10:06 -0400
Subject: [PATCH 3/3] Remove useless assignment
---
llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
index b5427f7acb9b4..b69c342439059 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
@@ -761,8 +761,6 @@ void PipelineSolver::greedyFind(
BestCost += Best->Cost;
} else
BestCost += MissPenalty;
-
- CurrPipeline[CurrSyncGroupIdx] = SyncPipeline;
}
bool PipelineSolver::solveGreedy() {
More information about the llvm-commits
mailing list