From 2033df5958105a27b4dab09f3013f1e4c88bbd59 Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Mon, 6 Apr 2026 15:03:09 +0200 Subject: [PATCH 1/4] utils: refactor TopoSort --- kernel/utils.h | 57 +++++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/kernel/utils.h b/kernel/utils.h index 46a196789..1d6c038fd 100644 --- a/kernel/utils.h +++ b/kernel/utils.h @@ -125,20 +125,20 @@ public: }; -// ------------------------------------------------ -// A simple class for topological sorting -// ------------------------------------------------ +// --------------------------------------------------- +// Best-effort topological sorting with loop detection +// --------------------------------------------------- template > class TopoSort { - public: +public: // We use this ordering of the edges in the adjacency matrix for // exact compatibility with an older implementation. struct IndirectCmp { - IndirectCmp(const std::vector &nodes) : node_cmp_(), nodes_(nodes) {} + IndirectCmp(const std::vector &nodes) : node_cmp_(), nodes_(nodes) {} bool operator()(int a, int b) const { - log_assert(static_cast(a) < nodes_.size()); + log_assert(static_cast(a) < nodes_.size()); log_assert(static_cast(b) < nodes_.size()); return node_cmp_(nodes_[a], nodes_[b]); } @@ -147,7 +147,9 @@ template > class TopoSort }; bool analyze_loops; + // The stability doesn't rely on std::less of T, so pointers are safe std::map node_to_index; + // edges[i] is the set of nodes with an edge into node i std::vector> edges; std::vector sorted; std::set> loops; @@ -160,10 +162,10 @@ template > class TopoSort int node(T n) { - auto rv = node_to_index.emplace(n, static_cast(nodes.size())); - if (rv.second) { - nodes.push_back(n); - edges.push_back(std::set(indirect_cmp)); + auto rv = node_to_index.emplace(n, static_cast(nodes.size())); + if (rv.second) { + nodes.push_back(n); + edges.push_back(std::set(indirect_cmp)); } return rv.first->second; } @@ -183,13 +185,14 @@ template > class TopoSort sorted.clear(); found_loops = false; - std::vector marked_cells(edges.size(), false); - std::vector active_cells(edges.size(), false); - std::vector active_stack; + std::vector node_is_sorted(edges.size(), false); + std::vector node_is_on_stack(edges.size(), false); + // Only used with analyze_loops + std::vector stack; sorted.reserve(edges.size()); for (const auto &it : node_to_index) - sort_worker(it.second, marked_cells, active_cells, active_stack); + sort_worker(it.second, node_is_sorted, node_is_on_stack, stack); log_assert(GetSize(sorted) == GetSize(nodes)); @@ -211,19 +214,20 @@ template > class TopoSort return database; } - private: +private: bool found_loops; std::vector nodes; const IndirectCmp indirect_cmp; - void sort_worker(const int root_index, std::vector &marked_cells, std::vector &active_cells, std::vector &active_stack) + void sort_worker(const int root_index, std::vector &node_is_sorted, std::vector &node_is_on_stack, std::vector &stack) { - if (active_cells[root_index]) { + if (node_is_on_stack[root_index]) { + // We've been here before, meaning we have a loop found_loops = true; if (analyze_loops) { std::vector loop; - for (int i = GetSize(active_stack) - 1; i >= 0; i--) { - const int index = active_stack[i]; + for (int i = GetSize(stack) - 1; i >= 0; i--) { + const int index = stack[i]; loop.push_back(nodes[index]); if (index == root_index) break; @@ -233,23 +237,24 @@ template > class TopoSort return; } - if (marked_cells[root_index]) + // We're done if we've already sorted this subgraph + if (node_is_sorted[root_index]) return; if (!edges[root_index].empty()) { if (analyze_loops) - active_stack.push_back(root_index); - active_cells[root_index] = true; + stack.push_back(root_index); + node_is_on_stack[root_index] = true; for (int left_n : edges[root_index]) - sort_worker(left_n, marked_cells, active_cells, active_stack); + sort_worker(left_n, node_is_sorted, node_is_on_stack, stack); if (analyze_loops) - active_stack.pop_back(); - active_cells[root_index] = false; + stack.pop_back(); + node_is_on_stack[root_index] = false; } - marked_cells[root_index] = true; + node_is_sorted[root_index] = true; sorted.push_back(nodes[root_index]); } }; From 41b41fefb38900e12e709137a35565cceb0bf02b Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Mon, 6 Apr 2026 15:03:29 +0200 Subject: [PATCH 2/4] utils: forbid the use of std::less on pointers in TopoSort --- kernel/utils.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/utils.h b/kernel/utils.h index 1d6c038fd..e71fb4911 100644 --- a/kernel/utils.h +++ b/kernel/utils.h @@ -131,6 +131,8 @@ public: template > class TopoSort { + static_assert(!(std::is_pointer::value && std::is_same>::value), + "std::less is run-to-run unstable for pointers"); public: // We use this ordering of the edges in the adjacency matrix for // exact compatibility with an older implementation. From cd49dc7be85d5909efb56902d3df8efede6fa6d8 Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Mon, 6 Apr 2026 15:09:33 +0200 Subject: [PATCH 3/4] cxxrtl: stable TopoSort --- backends/cxxrtl/cxxrtl_backend.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc index 71913d2db..ab5576e43 100644 --- a/backends/cxxrtl/cxxrtl_backend.cc +++ b/backends/cxxrtl/cxxrtl_backend.cc @@ -2776,7 +2776,8 @@ struct CxxrtlWorker { { RTLIL::Module *top_module = nullptr; std::vector modules; - TopoSort topo_design; + using Order = IdString::compare_ptr_by_name; + TopoSort topo_design; for (auto module : design->modules()) { if (!design->selected_module(module)) continue; From 41b69df2cb0f554dc31f0320360e339cf2edb9d2 Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Mon, 6 Apr 2026 15:09:42 +0200 Subject: [PATCH 4/4] abc_new: stable TopoSort --- passes/techmap/abc_new.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/passes/techmap/abc_new.cc b/passes/techmap/abc_new.cc index 4e279c577..0afabce11 100644 --- a/passes/techmap/abc_new.cc +++ b/passes/techmap/abc_new.cc @@ -18,7 +18,7 @@ */ #include "kernel/register.h" -#include "kernel/rtlil.h" +#include "kernel/yosys_common.h" #include "kernel/utils.h" USING_YOSYS_NAMESPACE @@ -27,7 +27,8 @@ PRIVATE_NAMESPACE_BEGIN std::vector order_modules(Design *design, std::vector modules) { std::set modules_set(modules.begin(), modules.end()); - TopoSort sort; + using Order = IdString::compare_ptr_by_name; + TopoSort sort; for (auto m : modules) { sort.node(m);