functional backend: topological sort starts with the output and next states nodes, other nodes get deleted

backends/functional/test_generic.cc

@@ -129,14 +129,13 @@ struct FunctionalTestGeneric : public Pass
 
 		size_t argidx = 1;
 		extra_args(args, argidx, design);
-
+/*
 		MemContentsTest test(8, 16);
 
 		std::random_device seed_dev;
 		std::mt19937 rnd(23); //seed_dev());
 		test.run(rnd, 1000);
-
+*/
-/*
 
 		for (auto module : design->selected_modules()) {
             log("Dumping module `%s'.\n", module->name.c_str());
@@ -144,11 +143,10 @@ struct FunctionalTestGeneric : public Pass
 			for(auto node : fir)
 				std::cout << RTLIL::unescape_id(node.name()) << " = " << node.to_string([](auto n) { return RTLIL::unescape_id(n.name()); }) << "\n";
 			for(auto [name, sort] : fir.outputs())
-				std::cout << RTLIL::unescape_id(name) << " = " << RTLIL::unescape_id(fir.get_output_node(name).name()) << "\n";
+				std::cout << "output " << RTLIL::unescape_id(name) << " = " << RTLIL::unescape_id(fir.get_output_node(name).name()) << "\n";
 			for(auto [name, sort] : fir.state())
-				std::cout << RTLIL::unescape_id(name) << " = " << RTLIL::unescape_id(fir.get_state_next_node(name).name()) << "\n";
+				std::cout << "state " << RTLIL::unescape_id(name) << " = " << RTLIL::unescape_id(fir.get_state_next_node(name).name()) << "\n";
 		}
-*/
 	}
 } FunctionalCxxBackend;
 

kernel/functional.h

@@ -268,6 +268,19 @@ public:
         return added;
     }
 
+    void compact_args()
+    {
+        std::vector<int> new_args;
+        for (auto &node : nodes)
+        {
+            int new_offset = GetSize(new_args);
+            for (int i = 0; i < node.arg_count; i++)
+                new_args.push_back(args[node.arg_offset + i]);
+            node.arg_offset = new_offset;
+        }
+        std::swap(args, new_args);
+    }
+
     void permute(std::vector<int> const &perm)
     {
         log_assert(perm.size() <= nodes.size());
@@ -276,7 +289,7 @@ public:
         for (int i = 0; i < GetSize(perm); ++i)
         {
             int j = perm[i];
-            log_assert(j >= 0 && j < GetSize(perm));
+            log_assert(j >= 0 && j < GetSize(nodes));
             log_assert(inv_perm[j] == -1);
             inv_perm[j] = i;
         }
@@ -301,15 +314,18 @@ public:
         std::swap(nodes, new_nodes);
         std::swap(sparse_attrs, new_sparse_attrs);
 
+        compact_args();
         for (int &arg : args)
         {
             log_assert(arg < GetSize(inv_perm));
+            log_assert(inv_perm[arg] >= 0);
             arg = inv_perm[arg];
         }
 
         for (auto &key : keys_)
         {
             log_assert(key.second < GetSize(inv_perm));
+            log_assert(inv_perm[key.second] >= 0);
             key.second = inv_perm[key.second];
         }
     }

kernel/functionalir.cc

@@ -657,19 +657,27 @@ void FunctionalIR::topological_sort() {
     Graph::SccAdaptor compute_graph_scc(_graph);
     bool scc = false;
     std::vector<int> perm;
-    topo_sorted_sccs(compute_graph_scc, [&](int *begin, int *end) {
+    TopoSortedSccs toposort(compute_graph_scc, [&](int *begin, int *end) {
         perm.insert(perm.end(), begin, end);
         if (end > begin + 1)
         {
-            log_warning("SCC:");
+            log_warning("Combinational loop:\n");
-            for (int *i = begin; i != end; ++i)
+            for (int *i = begin; i != end; ++i) {
-                log(" %d", *i);
+				Node node(_graph[*i]);
+                log("- %s = %s\n", RTLIL::unescape_id(node.name()).c_str(), node.to_string().c_str());
+			}
             log("\n");
             scc = true;
         }
-    }, /* sources_first */ true);
+    });
+	for(const auto &[name, sort]: _state_sorts)
+		toposort.process(get_state_next_node(name).id());
+	for(const auto &[name, sort]: _output_sorts)
+		toposort.process(get_output_node(name).id());
+	// any nodes untouched by this point are dead code and will be removed by permute
     _graph.permute(perm);
-    if(scc) log_error("combinational loops, aborting\n");
+    if(scc) log_error("The design contains combinational loops. This is not supported by the functional backend. "
+		"Try `scc -select; simplemap; select -clear` to avoid this error.\n");
 }
 
 static IdString merge_name(IdString a, IdString b) {

kernel/topo_scc.h

@@ -194,7 +194,7 @@ public:
 };
 
 template<typename G, typename ComponentCallback>
-void topo_sorted_sccs(G &graph, ComponentCallback component, bool sources_first = false)
+class TopoSortedSccs
 {
     typedef typename G::node_enumerator node_enumerator;
     typedef typename G::successor_enumerator successor_enumerator;
@@ -210,14 +210,20 @@ void topo_sorted_sccs(G &graph, ComponentCallback component, bool sources_first
         {}
     };
 
+    G &graph;
+    ComponentCallback component;
+
     std::vector<dfs_entry> dfs_stack;
     std::vector<node_type> component_stack;
     int next_index = 0;
 
+public:
+    TopoSortedSccs(G &graph, ComponentCallback component)
+    : graph(graph), component(component) {}
 
-    node_enumerator nodes = graph.enumerate_nodes();
+    // process all sources (nodes without a successor)
-
+    TopoSortedSccs &process_sources() {
-    if (sources_first) {
+        node_enumerator nodes = graph.enumerate_nodes();
         while (!nodes.finished()) {
             node_type node = nodes.next();
             successor_enumerator successors = graph.enumerate_successors(node);
@@ -231,16 +237,23 @@ void topo_sorted_sccs(G &graph, ComponentCallback component, bool sources_first
                 graph.dfs_index(node) = INT_MAX;
             }
         }
-        nodes = graph.enumerate_nodes();
+        return *this;
     }
 
-    // iterate over all nodes to ensure we process the whole graph
+    // process all remaining nodes in the graph
-    while (!nodes.finished()) {
+    TopoSortedSccs &process_all() {   
-        node_type node = nodes.next();
+        node_enumerator nodes = graph.enumerate_nodes();
+        // iterate over all nodes to ensure we process the whole graph
+        while (!nodes.finished())
+            process(nodes.next());
+        return *this;
+    }
+
+    // process all nodes that are reachable from a given start node
+    TopoSortedSccs &process(node_type node) {
         // only start a new search if the node wasn't visited yet
         if (graph.dfs_index(node) >= 0)
-            continue;
+            return *this;
-
         while (true) {
             // at this point we're visiting the node for the first time during
             // the DFS search
@@ -299,7 +312,7 @@ void topo_sorted_sccs(G &graph, ComponentCallback component, bool sources_first
                     // continues the search at the parent node or returns to
                     // the outer loop if we already are at the root node.
                     if (dfs_stack.empty())
-                        goto next_search;
+                        return *this;
                     auto &dfs_top = dfs_stack.back();
 
                     node = dfs_top.node;
@@ -336,9 +349,8 @@ void topo_sorted_sccs(G &graph, ComponentCallback component, bool sources_first
                 }
             }
         }
-    next_search:;
     }
-}
+};
 
 YOSYS_NAMESPACE_END
 

passes/cmds/example_dt.cc

@@ -183,7 +183,7 @@ struct ExampleDtPass : public Pass
 
 
 			std::vector<int> perm;
-			topo_sorted_sccs(compute_graph_scc, [&](int *begin, int *end) {
+			TopoSortedSccs(compute_graph_scc, [&](int *begin, int *end) {
 				perm.insert(perm.end(), begin, end);
 				if (end > begin + 1)
 				{
@@ -192,7 +192,7 @@ struct ExampleDtPass : public Pass
 						log(" %d", *i);
 					log("\n");
 				}
-			}, /* sources_first */ true);
+			}).process_sources().process_all();
 			compute_graph.permute(perm);