From 0955a603c889e3ce86ca65cf59ec3ca5650c6517 Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Wed, 10 Jun 2020 14:39:45 +0000
Subject: [PATCH] cxxrtl: disambiguate values/wires and their aliases in debug
info.
With this change, it is easier to see which signals carry state (only
wire<>s appear as `reg` in VCD files) and to construct a minimal
checkpoint (CXXRTL_WIRE debug items represent the canonical smallest
set of state required to fully reconstruct the simulation).
---
backends/cxxrtl/cxxrtl.h | 29 ++++++++++++++++++++++++++++-
backends/cxxrtl/cxxrtl_backend.cc | 2 +-
backends/cxxrtl/cxxrtl_capi.h | 11 +++++++++--
backends/cxxrtl/cxxrtl_vcd.h | 17 ++++++++++++-----
4 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/backends/cxxrtl/cxxrtl.h b/backends/cxxrtl/cxxrtl.h
index c988c9e80..bc3162981 100644
--- a/backends/cxxrtl/cxxrtl.h
+++ b/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");
diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc
index 785625f17..be73c9079 100644
--- a/backends/cxxrtl/cxxrtl_backend.cc
+++ b/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
diff --git a/backends/cxxrtl/cxxrtl_capi.h b/backends/cxxrtl/cxxrtl_capi.h
index 46aa662b2..8bd906ea4 100644
--- a/backends/cxxrtl/cxxrtl_capi.h
+++ b/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.
diff --git a/backends/cxxrtl/cxxrtl_vcd.h b/backends/cxxrtl/cxxrtl_vcd.h
index f6b78bbf7..4c2021e92 100644
--- a/backends/cxxrtl/cxxrtl_vcd.h
+++ b/backends/cxxrtl/cxxrtl_vcd.h
@@ -104,13 +104,13 @@ class vcd_writer {
buffer += '\n';
}
- const variable ®ister_variable(size_t width, chunk_t *curr, bool immutable = false) {
+ const variable ®ister_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;
});
}