From 691418e13a086c096373df13f7302323721faf35 Mon Sep 17 00:00:00 2001 From: whitequark Date: Wed, 2 Sep 2020 17:16:10 +0000 Subject: [PATCH] cxxrtl: expose driver kind in debug information. This can be useful to determine whether the wire should be a part of a design checkpoint, whether it can be used to override design state, and whether driving it may cause a conflict. --- backends/cxxrtl/cxxrtl.h | 11 +++-- backends/cxxrtl/cxxrtl_backend.cc | 68 +++++++++++++++++++++++++++---- backends/cxxrtl/cxxrtl_capi.h | 45 +++++++++++++++++++- 3 files changed, 112 insertions(+), 12 deletions(-) diff --git a/backends/cxxrtl/cxxrtl.h b/backends/cxxrtl/cxxrtl.h index df42f5807..41089a153 100644 --- a/backends/cxxrtl/cxxrtl.h +++ b/backends/cxxrtl/cxxrtl.h @@ -837,6 +837,9 @@ struct debug_item : ::cxxrtl_object { INPUT = CXXRTL_INPUT, OUTPUT = CXXRTL_OUTPUT, INOUT = CXXRTL_INOUT, + DRIVEN_SYNC = CXXRTL_DRIVEN_SYNC, + DRIVEN_COMB = CXXRTL_DRIVEN_COMB, + UNDRIVEN = CXXRTL_UNDRIVEN, }; debug_item(const ::cxxrtl_object &object) : cxxrtl_object(object) {} @@ -856,11 +859,11 @@ struct debug_item : ::cxxrtl_object { } template - debug_item(const value &item, size_t lsb_offset = 0, uint32_t flags_ = 0) { + debug_item(const value &item, size_t lsb_offset = 0) { static_assert(sizeof(item) == value::chunks * sizeof(chunk_t), "value is not compatible with C layout"); type = VALUE; - flags = flags_; + flags = DRIVEN_COMB; width = Bits; lsb_at = lsb_offset; depth = 1; @@ -903,7 +906,7 @@ struct debug_item : ::cxxrtl_object { static_assert(sizeof(item) == value::chunks * sizeof(chunk_t), "value is not compatible with C layout"); type = ALIAS; - flags = 0; + flags = DRIVEN_COMB; width = Bits; lsb_at = lsb_offset; depth = 1; @@ -918,7 +921,7 @@ struct debug_item : ::cxxrtl_object { sizeof(item.next) == value::chunks * sizeof(chunk_t), "wire is not compatible with C layout"); type = ALIAS; - flags = 0; + flags = DRIVEN_COMB; width = Bits; lsb_at = lsb_offset; depth = 1; diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc index 00b2ed315..dfea04409 100644 --- a/backends/cxxrtl/cxxrtl_backend.cc +++ b/backends/cxxrtl/cxxrtl_backend.cc @@ -273,6 +273,7 @@ struct FlowGraph { std::vector nodes; dict> wire_comb_defs, wire_sync_defs, wire_uses; dict wire_def_elidable, wire_use_elidable; + dict bit_has_state; ~FlowGraph() { @@ -294,6 +295,8 @@ struct FlowGraph { wire_comb_defs[chunk.wire].insert(node); } } + for (auto bit : sig.bits()) + bit_has_state[bit] |= is_ff; // Only comb defs of an entire wire in the right order can be elided. if (!is_ff && sig.is_wire()) wire_def_elidable[sig.as_wire()] = elidable; @@ -550,6 +553,7 @@ struct CxxrtlWorker { pool localized_wires; dict debug_alias_wires; dict debug_const_wires; + dict bit_has_state; dict> blackbox_specializations; dict eval_converges; @@ -1636,6 +1640,10 @@ struct CxxrtlWorker { size_t count_alias_wires = 0; size_t count_member_wires = 0; size_t count_skipped_wires = 0; + size_t count_driven_sync = 0; + size_t count_driven_comb = 0; + size_t count_undriven = 0; + size_t count_mixed_driver = 0; inc_indent(); f << indent << "assert(path.empty() || path[path.size() - 1] == ' ');\n"; for (auto wire : module->wires()) { @@ -1661,15 +1669,54 @@ struct CxxrtlWorker { count_alias_wires++; } else if (!localized_wires.count(wire)) { // Member wire - f << indent << "items.add(path + " << escape_cxx_string(get_hdl_name(wire)); - f << ", debug_item(" << mangle(wire) << ", "; - f << wire->start_offset; + std::vector flags; + if (wire->port_input && wire->port_output) - f << ", debug_item::INOUT"; + flags.push_back("INOUT"); else if (wire->port_input) - f << ", debug_item::INPUT"; + flags.push_back("INPUT"); else if (wire->port_output) - f << ", debug_item::OUTPUT"; + flags.push_back("OUTPUT"); + + bool has_driven_sync = false; + bool has_driven_comb = false; + bool has_undriven = false; + SigSpec sig(wire); + for (auto bit : sig.bits()) + if (!bit_has_state.count(bit)) + has_undriven = true; + else if (bit_has_state[bit]) + has_driven_sync = true; + else + has_driven_comb = true; + if (has_driven_sync) + flags.push_back("DRIVEN_SYNC"); + if (has_driven_sync && !has_driven_comb && !has_undriven) + count_driven_sync++; + if (has_driven_comb) + flags.push_back("DRIVEN_COMB"); + if (!has_driven_sync && has_driven_comb && !has_undriven) + count_driven_comb++; + if (has_undriven) + flags.push_back("UNDRIVEN"); + if (!has_driven_sync && !has_driven_comb && has_undriven) + count_undriven++; + if (has_driven_sync + has_driven_comb + has_undriven > 1) + count_mixed_driver++; + + f << indent << "items.add(path + " << escape_cxx_string(get_hdl_name(wire)); + f << ", debug_item(" << mangle(wire) << ", "; + f << wire->start_offset; + bool first = true; + for (auto flag : flags) { + if (first) { + first = false; + f << ", "; + } else { + f << "|"; + } + f << "debug_item::" << flag; + } f << "));\n"; count_member_wires++; } else { @@ -1698,7 +1745,11 @@ struct CxxrtlWorker { log_debug(" Public wires: %zu, of which:\n", count_public_wires); 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(" Member wires: %zu, of which:\n", count_member_wires); + log_debug(" Driven sync: %zu\n", count_driven_sync); + log_debug(" Driven comb: %zu\n", count_driven_comb); + log_debug(" Undriven: %zu\n", count_undriven); + log_debug(" Mixed driver: %zu\n", count_mixed_driver); log_debug(" Other wires: %zu (no debug information)\n", count_skipped_wires); } @@ -2217,6 +2268,9 @@ struct CxxrtlWorker { eval_converges[module] = feedback_wires.empty() && buffered_comb_wires.empty(); + for (auto item : flow.bit_has_state) + bit_has_state.insert(item); + 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. diff --git a/backends/cxxrtl/cxxrtl_capi.h b/backends/cxxrtl/cxxrtl_capi.h index d2e9787dd..385d6dcf3 100644 --- a/backends/cxxrtl/cxxrtl_capi.h +++ b/backends/cxxrtl/cxxrtl_capi.h @@ -73,6 +73,10 @@ int cxxrtl_commit(cxxrtl_handle handle); size_t cxxrtl_step(cxxrtl_handle handle); // Type of a simulated object. +// +// The type of a simulated object indicates the way it is stored and the operations that are legal +// to perform on it (i.e. won't crash the simulation). It says very little about object semantics, +// which is specified through flags. enum cxxrtl_type { // Values correspond to singly buffered netlist nodes, i.e. nodes driven exclusively by // combinatorial cells, or toplevel input nodes. @@ -86,7 +90,8 @@ enum cxxrtl_type { CXXRTL_VALUE = 0, // Wires correspond to doubly buffered netlist nodes, i.e. nodes driven, at least in part, by - // storage cells, or by combinatorial cells that are a part of a feedback path. + // storage cells, or by combinatorial cells that are a part of a feedback path. They are also + // present in non-optimized builds. // // Wires can be inspected via the `curr` pointer and modified via the `next` pointer (which are // distinct for wires). Note that changes to the bits driven by combinatorial cells will be @@ -113,6 +118,12 @@ enum cxxrtl_type { }; // Flags of a simulated object. +// +// The flags of a simulated object indicate its role in the netlist: +// * The flags `CXXRTL_INPUT` and `CXXRTL_OUTPUT` designate module ports. +// * The flags `CXXRTL_DRIVEN_SYNC`, `CXXRTL_DRIVEN_COMB`, and `CXXRTL_UNDRIVEN` specify +// the semantics of node state. An object with several of these flags set has different bits +// follow different semantics. enum cxxrtl_flag { // Node is a module input port. // @@ -131,6 +142,38 @@ enum cxxrtl_flag { // This flag can be set on objects of type `CXXRTL_WIRE`. It may be combined with other flags. CXXRTL_INOUT = (CXXRTL_INPUT|CXXRTL_OUTPUT), + // Node has bits that are driven by a storage cell. + // + // This flag can be set on objects of type `CXXRTL_WIRE`. It may be combined with + // `CXXRTL_DRIVEN_COMB` and `CXXRTL_UNDRIVEN`, as well as other flags. + // + // This flag is set on wires that have bits connected directly to the output of a flip-flop or + // a latch, and hold its state. Many `CXXRTL_WIRE` objects may not have the `CXXRTL_DRIVEN_SYNC` + // flag set; for example, output ports and feedback wires generally won't. Writing to the `next` + // pointer of these wires updates stored state, and for designs without combinatorial loops, + // capturing the value from every of these wires through the `curr` pointer creates a complete + // snapshot of the design state. + CXXRTL_DRIVEN_SYNC = 1 << 2, + + // Node has bits that are driven by a combinatorial cell or another node. + // + // This flag can be set on objects of type `CXXRTL_VALUE` and `CXXRTL_WIRE`. It may be combined + // with `CXXRTL_DRIVEN_SYNC` and `CXXRTL_UNDRIVEN`, as well as other flags. + // + // This flag is set on objects that have bits connected to the output of a combinatorial cell, + // or directly to another node. For designs without combinatorial loops, writing to such bits + // through the `next` pointer (if it is not NULL) has no effect. + CXXRTL_DRIVEN_COMB = 1 << 3, + + // Node has bits that are not driven. + // + // This flag can be set on objects of type `CXXRTL_VALUE` and `CXXRTL_WIRE`. It may be combined + // with `CXXRTL_DRIVEN_SYNC` and `CXXRTL_DRIVEN_COMB`, as well as other flags. + // + // This flag is set on objects that have bits not driven by an output of any cell or by another + // node, such as inputs and dangling wires. + CXXRTL_UNDRIVEN = 1 << 4, + // More object flags may be added in the future, but the existing ones will never change. }; -- 2.30.2