[llvm] Orc fix waitingongraph coalescer remove (PR #169287)
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 23 23:22:39 PST 2025
https://github.com/lhames created https://github.com/llvm/llvm-project/pull/169287
None
>From 29b1f738bff68cccdbd759e5dd4166d9a1b9a489 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Mon, 24 Nov 2025 17:25:06 +1100
Subject: [PATCH 1/2] [ORC] Avoid self-dependence in SuperNode dependence
graph.
Avoid adding any given SuperNode SN to its own SuperNode-deps set. This saves
us from trying to redundantly merge its dependencies back into itself (a no-op,
but a potentially expensive one).
---
llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h b/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
index f9995917363f9..9f14c8b2efd5f 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
@@ -500,7 +500,7 @@ template <typename ContainerIdT, typename ElementIdT> class WaitingOnGraph {
if (I == SN->Deps.end())
continue;
for (auto &[DefElem, DefSN] : DefElems)
- if (I->second.erase(DefElem))
+ if (I->second.erase(DefElem) && DefSN != SN.get())
SNDeps.insert(DefSN);
if (I->second.empty())
SN->Deps.erase(I);
@@ -511,11 +511,13 @@ template <typename ContainerIdT, typename ElementIdT> class WaitingOnGraph {
// Compute transitive closure of deps for each node.
static void propagateSuperNodeDeps(SuperNodeDepsMap &SuperNodeDeps) {
for (auto &[SN, Deps] : SuperNodeDeps) {
- DenseSet<SuperNode *> Reachable({SN});
+ DenseSet<SuperNode *> Reachable;
SmallVector<SuperNode *> Worklist(Deps.begin(), Deps.end());
while (!Worklist.empty()) {
auto *DepSN = Worklist.pop_back_val();
+ if (DepSN == SN)
+ continue;
if (!Reachable.insert(DepSN).second)
continue;
auto I = SuperNodeDeps.find(DepSN);
@@ -537,9 +539,11 @@ template <typename ContainerIdT, typename ElementIdT> class WaitingOnGraph {
if (I == SuperNodeDeps.end())
continue;
- for (auto *DepSN : I->second)
+ for (auto *DepSN : I->second) {
+ assert(DepSN != SN.get() && "Unexpected self-dependence for SN");
for (auto &[Container, Elems] : DepSN->Deps)
SN->Deps[Container].insert(Elems.begin(), Elems.end());
+ }
}
}
>From 383c513447eff7433276626204c091e86ae50e20 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Mon, 24 Nov 2025 18:08:03 +1100
Subject: [PATCH 2/2] [ORC] Fix bug in WaitingOnGraph::Coalescer::remove.
This method takes a predicate, but was only deleting the first node for which
the predicate returned true.
This patch updates it to remove all nodes for which the predicate returns true.
Bug found by inspection while investigating
https://github.com/llvm/llvm-project/issues/169135. No testcase, since
exercising this bug would involve finding sets with identical hashes.
---
.../llvm/ExecutionEngine/Orc/WaitingOnGraph.h | 27 ++++++++++---------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h b/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
index 9f14c8b2efd5f..a7ba79164c471 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/WaitingOnGraph.h
@@ -155,22 +155,20 @@ template <typename ContainerIdT, typename ElementIdT> class WaitingOnGraph {
}
template <typename Pred> void remove(Pred &&Remove) {
+ std::vector<hash_code> HashesToErase;
for (auto &[Hash, SNs] : CanonicalSNs) {
- bool Found = false;
- for (size_t I = 0; I != SNs.size(); ++I) {
+ for (size_t I = 0; I != SNs.size();) {
if (Remove(SNs[I])) {
std::swap(SNs[I], SNs.back());
SNs.pop_back();
- Found = true;
- break;
- }
- }
- if (Found) {
- if (SNs.empty())
- CanonicalSNs.erase(Hash);
- break;
+ } else
+ ++I;
}
+ if (SNs.empty())
+ HashesToErase.push_back(Hash);
}
+ for (auto Hash : HashesToErase)
+ CanonicalSNs.erase(Hash);
}
private:
@@ -396,9 +394,14 @@ template <typename ContainerIdT, typename ElementIdT> class WaitingOnGraph {
++I;
}
+ CoalesceToPendingSNs.remove([&](SuperNode *SN) {
+ for (auto &E : FailedSNs)
+ if (E.get() == SN)
+ return true;
+ return false;
+ });
+
for (auto &SN : FailedSNs) {
- CoalesceToPendingSNs.remove(
- [&](SuperNode *SNC) { return SNC == SN.get(); });
for (auto &[Container, Elems] : SN->Defs) {
assert(ElemToPendingSN.count(Container));
auto &CElems = ElemToPendingSN[Container];
More information about the llvm-commits
mailing list