Merge pull request #2121 from whitequark/cxxrtl-debug-aliases

cxxrtl: improve design visibility

4 files changed, 115 insertions, 25 deletions

diff --git a/backends/cxxrtl/cxxrtl.h b/backends/cxxrtl/cxxrtl.h
index 5f74899fd..30f4667c5 100644
--- a/backends/cxxrtl/cxxrtl.h
+++ b/backends/cxxrtl/cxxrtl.h
@@ -742,6 +742,17 @@ struct debug_item : ::cxxrtl_object {
 	}
 
 	template<size_t Bits>
+	debug_item(const value<Bits> &item) {
+		static_assert(sizeof(item) == value<Bits>::chunks * sizeof(chunk_t),
+		              "value<Bits> is not compatible with C layout");
+		type  = VALUE;
+		width = Bits;
+		depth = 1;
+		curr  = const_cast<uint32_t*>(item.data);
+		next  = nullptr;
+	}
+
+	template<size_t Bits>
 	debug_item(wire<Bits> &item) {
 		static_assert(sizeof(item.curr) == value<Bits>::chunks * sizeof(chunk_t) &&
 		              sizeof(item.next) == value<Bits>::chunks * sizeof(chunk_t),
diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc
index 64af5dab8..b3aec2110 100644
--- a/backends/cxxrtl/cxxrtl_backend.cc
+++ b/backends/cxxrtl/cxxrtl_backend.cc
@@ -359,10 +359,10 @@ struct FlowGraph {
 		//
 		// eliminating the unnecessary delta cycle. Conceptually, the CELL_SYNC node type is a series of
 		// connections of the form `connect \lhs \cell.\sync_output`; the right-hand side of these is not
-		// as a wire in RTLIL. If it was expressible, then `\cell.\sync_output` would have a sync def,
-		// and this node would be an ordinary CONNECT node, with `\lhs` having a comb def. Because it isn't,
-		// a special node type is used, the right-hand side does not appear anywhere, and the left-hand
-		// side has a comb def.
+		// expressible as a wire in RTLIL. If it was expressible, then `\cell.\sync_output` would have
+		// a sync def, and this node would be an ordinary CONNECT node, with `\lhs` having a comb def.
+		// Because it isn't, a special node type is used, the right-hand side does not appear anywhere,
+		// and the left-hand side has a comb def.
 		for (auto conn : cell->connections())
 			if (cell->output(conn.first))
 				if (is_cxxrtl_sync_port(cell, conn.first)) {
@@ -539,6 +539,8 @@ struct CxxrtlWorker {
 	dict<const RTLIL::Wire*, FlowGraph::Node> elided_wires;
 	dict<const RTLIL::Module*, std::vector<FlowGraph::Node>> schedule;
 	pool<const RTLIL::Wire*> localized_wires;
+	dict<const RTLIL::Wire*, const RTLIL::Wire*> debug_alias_wires;
+	dict<const RTLIL::Wire*, RTLIL::Const> debug_const_wires;
 	dict<const RTLIL::Module*, pool<std::string>> blackbox_specializations;
 	dict<const RTLIL::Module*, bool> eval_converges;
 
@@ -1606,15 +1608,36 @@ struct CxxrtlWorker {
 
 	void dump_debug_info_method(RTLIL::Module *module)
 	{
+		size_t count_const_wires = 0;
+		size_t count_alias_wires = 0;
+		size_t count_member_wires = 0;
+		size_t count_skipped_wires = 0;
 		inc_indent();
 			f << indent << "assert(path.empty() || path[path.size() - 1] == ' ');\n";
 			for (auto wire : module->wires()) {
 				if (wire->name[0] != '\\')
 					continue;
-				if (localized_wires.count(wire))
-					continue;
-				f << indent << "items.emplace(path + " << escape_cxx_string(get_hdl_name(wire));
-				f << ", debug_item(" << mangle(wire) << "));\n";
+				if (debug_const_wires.count(wire)) {
+					// Wire tied to a constant
+					f << indent << "static const value<" << wire->width << "> const_" << mangle(wire) << " = ";
+					dump_const(debug_const_wires[wire]);
+					f << ";\n";
+					f << indent << "items.emplace(path + " << escape_cxx_string(get_hdl_name(wire));
+					f << ", debug_item(const_" << mangle(wire) << "));\n";
+					count_const_wires++;
+				} else if (debug_alias_wires.count(wire)) {
+					// Alias of a member wire
+					f << indent << "items.emplace(path + " << escape_cxx_string(get_hdl_name(wire));
+					f << ", debug_item(" << mangle(debug_alias_wires[wire]) << "));\n";
+					count_alias_wires++;
+				} else if (!localized_wires.count(wire)) {
+					// Member wire
+					f << indent << "items.emplace(path + " << escape_cxx_string(get_hdl_name(wire));
+					f << ", debug_item(" << mangle(wire) << "));\n";
+					count_member_wires++;
+				} else {
+					count_skipped_wires++;
+				}
 			}
 			for (auto &memory_it : module->memories) {
 				if (memory_it.first[0] != '\\')
@@ -1630,6 +1653,12 @@ struct CxxrtlWorker {
 				f << "path + " << escape_cxx_string(get_hdl_name(cell) + ' ') << ");\n";
 			}
 		dec_indent();
+
+		log_debug("Debug information statistics for module %s:\n", log_id(module));
+		log_debug("  Const wires:  %zu\n", count_const_wires);
+		log_debug("  Alias wires:  %zu\n", count_alias_wires);
+		log_debug("  Member wires: %zu\n", count_member_wires);
+		log_debug("  Other wires:  %zu (no debug information)\n", count_skipped_wires);
 	}
 
 	void dump_metadata_map(const dict<RTLIL::IdString, RTLIL::Const> &metadata_map)
@@ -2141,6 +2170,44 @@ struct CxxrtlWorker {
 			}
 
 			eval_converges[module] = feedback_wires.empty() && buffered_wires.empty();
+
+			if (debug_info) {
+				// Find wires that alias other wires or are tied to a constant; debug information can be enriched with these
+				// at essentially zero additional cost.
+				//
+				// Note that the information collected here can't be used for optimizing the netlist: debug information queries
+				// are pure and run on a design in a stable state, which allows assumptions that do not otherwise hold.
+				for (auto wire : module->wires()) {
+					if (wire->name[0] != '\\')
+						continue;
+					if (!localized_wires[wire])
+						continue;
+					const RTLIL::Wire *wire_it = wire;
+					while (1) {
+						if (!(flow.wire_def_elidable.count(wire_it) && flow.wire_def_elidable[wire_it]))
+							break; // not an alias: complex def
+						log_assert(flow.wire_comb_defs[wire_it].size() == 1);
+						FlowGraph::Node *node = *flow.wire_comb_defs[wire_it].begin();
+						if (node->type != FlowGraph::Node::Type::CONNECT)
+							break; // not an alias: def by cell
+						RTLIL::SigSpec rhs_sig = node->connect.second;
+						if (rhs_sig.is_wire()) {
+							RTLIL::Wire *rhs_wire = rhs_sig.as_wire();
+							if (localized_wires[rhs_wire]) {
+								wire_it = rhs_wire; // maybe an alias
+							} else {
+								debug_alias_wires[wire] = rhs_wire; // is an alias
+								break;
+							}
+						} else if (rhs_sig.is_fully_const()) {
+							debug_const_wires[wire] = rhs_sig.as_const(); // is a const
+							break;
+						} else {
+							break; // not an alias: complex rhs
+						}
+					}
+				}
+			}
 		}
 		if (has_feedback_arcs || has_buffered_wires) {
 			// Although both non-feedback buffered combinatorial wires and apparent feedback wires may be eliminated
@@ -2419,7 +2486,8 @@ struct CxxrtlBackend : public Backend {
 		log("        no debug information.\n");
 		log("\n");
 		log("    -g1\n");
-		log("        debug information for non-localized public wires.\n");
+		log("        debug information for non-optimized public wires. this also makes it\n");
+		log("        possible to use the C API.\n");
 		log("\n");
 	}
 
@@ -2494,7 +2562,7 @@ struct CxxrtlBackend : public Backend {
 			case 0:
 				break;
 			default:
-				log_cmd_error("Invalid optimization level %d.\n", opt_level);
+				log_cmd_error("Invalid debug information level %d.\n", debug_level);
 		}
 
 		std::ofstream intf_f;
diff --git a/backends/cxxrtl/cxxrtl_capi.h b/backends/cxxrtl/cxxrtl_capi.h
index bee5a94c7..46aa662b2 100644
--- a/backends/cxxrtl/cxxrtl_capi.h
+++ b/backends/cxxrtl/cxxrtl_capi.h
@@ -64,9 +64,10 @@ enum cxxrtl_type {
 	// Values correspond to singly buffered netlist nodes, i.e. nodes driven exclusively by
 	// combinatorial cells, or toplevel input nodes.
 	//
-	// Values can be inspected via the `curr` pointer and modified via the `next` pointer (which are
-	// equal for values); however, note that changes to the bits driven by combinatorial cells will
-	// be ignored.
+	// Values can be inspected via the `curr` pointer. If the `next` pointer is NULL, the value is
+	// driven by a constant and can never be modified. Otherwise, the value can be modified through
+	// the `next` pointer (which is equal to `curr` if not NULL). Note that changes to the bits
+	// driven by combinatorial cells will be ignored.
 	//
 	// Values always have depth 1.
 	CXXRTL_VALUE = 0,
@@ -75,8 +76,8 @@ enum cxxrtl_type {
 	// storage cells, or by combinatorial cells that are a part of a feedback path.
 	//
 	// Wires can be inspected via the `curr` pointer and modified via the `next` pointer (which are
-	// distinct for wires); however, note that changes to the bits driven by combinatorial cells will
-	// be ignored.
+	// distinct for wires). Note that changes to the bits driven by combinatorial cells will be
+	// ignored.
 	//
 	// Wires always have depth 1.
 	CXXRTL_WIRE = 1,
diff --git a/backends/cxxrtl/cxxrtl_vcd.h b/backends/cxxrtl/cxxrtl_vcd.h
index 5706917ca..5f5f612b5 100644
--- a/backends/cxxrtl/cxxrtl_vcd.h
+++ b/backends/cxxrtl/cxxrtl_vcd.h
@@ -34,6 +34,7 @@ class vcd_writer {
 	std::vector<std::string> current_scope;
 	std::vector<variable> variables;
 	std::vector<chunk_t> cache;
+	std::map<chunk_t*, size_t> aliases;
 	bool streaming = false;
 
 	void emit_timescale(unsigned number, const std::string &unit) {
@@ -103,13 +104,25 @@ class vcd_writer {
 		buffer += '\n';
 	}
 
-	void append_variable(size_t width, chunk_t *curr) {
-		const size_t chunks = (width + (sizeof(chunk_t) * 8 - 1)) / (sizeof(chunk_t) * 8);
-		variables.emplace_back(variable { variables.size(), width, curr, cache.size() });
-		cache.insert(cache.end(), &curr[0], &curr[chunks]);
+	const variable &register_variable(size_t width, chunk_t *curr, bool immutable = false) {
+		if (aliases.count(curr)) {
+			return variables[aliases[curr]];
+		} else {
+			const size_t chunks = (width + (sizeof(chunk_t) * 8 - 1)) / (sizeof(chunk_t) * 8);
+			aliases[curr] = variables.size();
+			if (immutable) {
+				variables.emplace_back(variable { variables.size(), width, curr, (size_t)-1 });
+			} else {
+				variables.emplace_back(variable { variables.size(), width, curr, cache.size() });
+				cache.insert(cache.end(), &curr[0], &curr[chunks]);
+			}
+			return variables.back();
+		}
 	}
 
 	bool test_variable(const variable &var) {
+		if (var.prev_off == (size_t)-1)
+			return false; // immutable
 		const size_t chunks = (var.width + (sizeof(chunk_t) * 8 - 1)) / (sizeof(chunk_t) * 8);
 		if (std::equal(&var.curr[0], &var.curr[chunks], &cache[var.prev_off])) {
 			return false;
@@ -151,20 +164,17 @@ public:
 		switch (item.type) {
 			// Not the best naming but oh well...
 			case debug_item::VALUE:
-				append_variable(item.width, item.curr);
-				emit_var(variables.back(), "wire", name);
+				emit_var(register_variable(item.width, item.curr, /*immutable=*/item.next == nullptr), "wire", name);
 				break;
 			case debug_item::WIRE:
-				append_variable(item.width, item.curr);
-				emit_var(variables.back(), "reg", name);
+				emit_var(register_variable(item.width, item.curr), "reg", name);
 				break;
 			case debug_item::MEMORY: {
 				const size_t stride = (item.width + (sizeof(chunk_t) * 8 - 1)) / (sizeof(chunk_t) * 8);
 				for (size_t index = 0; index < item.depth; index++) {
 					chunk_t *nth_curr = &item.curr[stride * index];
 					std::string nth_name = name + '[' + std::to_string(index) + ']';
-					append_variable(item.width, nth_curr);
-					emit_var(variables.back(), "reg", nth_name);
+					emit_var(register_variable(item.width, nth_curr), "reg", nth_name);
 				}
 				break;
 			}