Merge pull request #2140 from whitequark/cxxrtl-aliases

cxxrtl: disambiguate values/wires and their aliases in debug info

backends/cxxrtl/cxxrtl.h

@@ -716,6 +716,9 @@ struct metadata {
 
 typedef std::map<std::string, metadata> metadata_map;
 
+// Helper class to disambiguate values/wires and their aliases.
+struct debug_alias {};
+
 // This structure is intended for consumption via foreign function interfaces, like Python's ctypes.
 // Because of this it uses a C-style layout that is easy to parse rather than more idiomatic C++.
 //
@@ -726,6 +729,7 @@ struct debug_item : ::cxxrtl_object {
 		VALUE  = CXXRTL_VALUE,
 		WIRE   = CXXRTL_WIRE,
 		MEMORY = CXXRTL_MEMORY,
+		ALIAS  = CXXRTL_ALIAS,
 	};
 
 	debug_item(const ::cxxrtl_object &object) : cxxrtl_object(object) {}
@@ -748,7 +752,7 @@ struct debug_item : ::cxxrtl_object {
 		type  = VALUE;
 		width = Bits;
 		depth = 1;
-		curr  = const_cast<uint32_t*>(item.data);
+		curr  = const_cast<chunk_t*>(item.data);
 		next  = nullptr;
 	}
 
@@ -774,6 +778,29 @@ struct debug_item : ::cxxrtl_object {
 		curr  = item.data.empty() ? nullptr : item.data[0].data;
 		next  = nullptr;
 	}
+
+	template<size_t Bits>
+	debug_item(debug_alias, const value<Bits> &item) {
+		static_assert(sizeof(item) == value<Bits>::chunks * sizeof(chunk_t),
+		              "value<Bits> is not compatible with C layout");
+		type  = ALIAS;
+		width = Bits;
+		depth = 1;
+		curr  = const_cast<chunk_t*>(item.data);
+		next  = nullptr;
+	}
+
+	template<size_t Bits>
+	debug_item(debug_alias, const wire<Bits> &item) {
+		static_assert(sizeof(item.curr) == value<Bits>::chunks * sizeof(chunk_t) &&
+		              sizeof(item.next) == value<Bits>::chunks * sizeof(chunk_t),
+		              "wire<Bits> is not compatible with C layout");
+		type  = ALIAS;
+		width = Bits;
+		depth = 1;
+		curr  = const_cast<chunk_t*>(item.curr.data);
+		next  = nullptr;
+	}
 };
 static_assert(std::is_standard_layout<debug_item>::value, "debug_item is not compatible with C layout");
 

backends/cxxrtl/cxxrtl_backend.cc

@@ -1646,7 +1646,7 @@ struct CxxrtlWorker {
 				} 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";
+					f << ", debug_item(debug_alias(), " << mangle(debug_alias_wires[wire]) << "));\n";
 					count_alias_wires++;
 				} else if (!localized_wires.count(wire)) {
 					// Member wire

backends/cxxrtl/cxxrtl_capi.h

@@ -89,7 +89,14 @@ enum cxxrtl_type {
 	// always NULL.
 	CXXRTL_MEMORY = 2,
 
-	// More object types will be added in the future, but the existing ones will never change.
+	// Aliases correspond to netlist nodes driven by another node such that their value is always
+	// exactly equal, or driven by a constant value.
+	//
+	// Aliases can be inspected via the `curr` pointer. They cannot be modified, and the `next`
+	// pointer is always NULL.
+	CXXRTL_ALIAS = 3,
+
+	// More object types may be added in the future, but the existing ones will never change.
 };
 
 // Description of a simulated object.
@@ -123,7 +130,7 @@ struct cxxrtl_object {
 	uint32_t *curr;
 	uint32_t *next;
 
-	// More description fields will be added in the future, but the existing ones will never change.
+	// More description fields may be added in the future, but the existing ones will never change.
 };
 
 // Retrieve description of a simulated object.

backends/cxxrtl/cxxrtl_vcd.h

@@ -104,13 +104,13 @@ class vcd_writer {
 		buffer += '\n';
 	}
 
-	const variable &register_variable(size_t width, chunk_t *curr, bool immutable = false) {
+	const variable &register_variable(size_t width, chunk_t *curr, bool constant = 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) {
+			if (constant) {
 				variables.emplace_back(variable { variables.size(), width, curr, (size_t)-1 });
 			} else {
 				variables.emplace_back(variable { variables.size(), width, curr, cache.size() });
@@ -122,7 +122,7 @@ class vcd_writer {
 
 	bool test_variable(const variable &var) {
 		if (var.prev_off == (size_t)-1)
-			return false; // immutable
+			return false; // constant
 		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;
@@ -164,7 +164,7 @@ public:
 		switch (item.type) {
 			// Not the best naming but oh well...
 			case debug_item::VALUE:
-				emit_var(register_variable(item.width, item.curr, /*immutable=*/item.next == nullptr), "wire", name);
+				emit_var(register_variable(item.width, item.curr, /*constant=*/item.next == nullptr), "wire", name);
 				break;
 			case debug_item::WIRE:
 				emit_var(register_variable(item.width, item.curr), "reg", name);
@@ -178,6 +178,13 @@ public:
 				}
 				break;
 			}
+			case debug_item::ALIAS:
+				// Like VALUE, but, even though `item.next == nullptr` always holds, the underlying value
+				// can actually change, and must be tracked. In most cases the VCD identifier will be
+				// unified with the aliased reg, but we should handle the case where only the alias is
+				// added to the VCD writer, too.
+				emit_var(register_variable(item.width, item.curr), "wire", name);
+				break;
 		}
 	}
 
@@ -198,7 +205,7 @@ public:
 
 	void add_without_memories(const debug_items &items) {
 		this->template add(items, [](const std::string &, const debug_item &item) {
-			return item.type == debug_item::VALUE || item.type == debug_item::WIRE;
+			return item.type != debug_item::MEMORY;
 		});
 	}