From fa74d0bd1a070a007292cc260b49f183f00ebfd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Mon, 5 Feb 2024 14:44:55 +0100 Subject: [PATCH] check: Use cell edges data in detecting combinational loops --- passes/cmds/check.cc | 161 ++++++++++++++++++++++++++++++++----------- 1 file changed, 121 insertions(+), 40 deletions(-) diff --git a/passes/cmds/check.cc b/passes/cmds/check.cc index c3e775a38..9f70dbdc7 100644 --- a/passes/cmds/check.cc +++ b/passes/cmds/check.cc @@ -19,6 +19,7 @@ #include "kernel/yosys.h" #include "kernel/sigtools.h" +#include "kernel/celledges.h" #include "kernel/celltypes.h" #include "kernel/utils.h" @@ -102,6 +103,7 @@ struct CheckPass : public Pass { SigMap sigmap(module); dict> wire_drivers; + dict driver_cells; dict wire_drivers_count; pool used_wires; TopoSort> topo; @@ -149,6 +151,46 @@ struct CheckPass : public Pass { } } + struct CircuitEdgesDatabase : AbstractCellEdgesDatabase { + TopoSort> &topo; + SigMap sigmap; + + CircuitEdgesDatabase(TopoSort> &topo, SigMap &sigmap) + : topo(topo), sigmap(sigmap) {} + + void add_edge(RTLIL::Cell *cell, RTLIL::IdString from_port, int from_bit, + RTLIL::IdString to_port, int to_bit, int) override { + SigBit from = sigmap(cell->getPort(from_port))[from_bit]; + SigBit to = sigmap(cell->getPort(to_port))[to_bit]; + + if (from.wire && to.wire) + topo.edge(std::make_pair(from.wire->name, from.offset), std::make_pair(to.wire->name, to.offset)); + } + + bool add_edges_from_cell(Cell *cell) { + if (AbstractCellEdgesDatabase::add_edges_from_cell(cell)) + return true; + + // We don't have accurate cell edges, do the fallback of all input-output pairs + for (auto &conn : cell->connections()) { + if (cell->input(conn.first)) + for (auto bit : sigmap(conn.second)) + if (bit.wire) + topo.edge(std::make_pair(bit.wire->name, bit.offset), + std::make_pair(cell->name, -1)); + + if (cell->output(conn.first)) + for (auto bit : sigmap(conn.second)) + if (bit.wire) + topo.edge(std::make_pair(cell->name, -1), + std::make_pair(bit.wire->name, bit.offset)); + } + return true; + } + }; + + CircuitEdgesDatabase edges_db(topo, sigmap); + for (auto cell : module->cells()) { if (mapped && cell->type.begins_with("$") && design->module(cell->type) == nullptr) { @@ -157,31 +199,29 @@ struct CheckPass : public Pass { counter++; cell_allowed:; } - for (auto &conn : cell->connections()) { - SigSpec sig = sigmap(conn.second); - bool logic_cell = yosys_celltypes.cell_evaluable(cell->type); - if (cell->input(conn.first)) - for (auto bit : sig) - if (bit.wire) { - if (logic_cell && bit.wire) - topo.edge(std::make_pair(bit.wire->name, bit.offset), - std::make_pair(cell->name, -1)); - used_wires.insert(bit); - } - if (cell->output(conn.first)) - for (int i = 0; i < GetSize(sig); i++) { - if (logic_cell && sig[i].wire) - topo.edge(std::make_pair(cell->name, -1), - std::make_pair(sig[i].wire->name, sig[i].offset)); - if (sig[i].wire || !cell->input(conn.first)) - wire_drivers[sig[i]].push_back(stringf("port %s[%d] of cell %s (%s)", - log_id(conn.first), i, log_id(cell), log_id(cell->type))); - } - if (!cell->input(conn.first) && cell->output(conn.first)) - for (auto bit : sig) - if (bit.wire) wire_drivers_count[bit]++; + for (auto &conn : cell->connections()) { + bool input = cell->input(conn.first); + bool output = cell->output(conn.first); + + SigSpec sig = sigmap(conn.second); + for (int i = 0; i < sig.size(); i++) { + SigBit bit = sig[i]; + + if (input && bit.wire) + used_wires.insert(bit); + if (output && !input && bit.wire) + wire_drivers_count[bit]++; + if (output && (bit.wire || !input)) + wire_drivers[bit].push_back(stringf("port %s[%d] of cell %s (%s)", log_id(conn.first), i, + log_id(cell), log_id(cell->type))); + if (output) + driver_cells[bit] = cell; + } } + + if (yosys_celltypes.cell_evaluable(cell->type)) + edges_db.add_edges_from_cell(cell); } pool init_bits; @@ -238,27 +278,68 @@ struct CheckPass : public Pass { topo.sort(); for (auto &loop : topo.loops) { string message = stringf("found logic loop in module %s:\n", log_id(module)); + + // `loop` only contains wire bits, or an occassional special helper node for cells for + // which we have done the edges fallback. The cell and its ports that led to an edge is + // an information we need to recover now. For that we need to have the previous wire bit + // of the loop at hand. + SigBit prev; + for (auto it = loop.rbegin(); it != loop.rend(); it++) + if (it->second != -1) { // skip the fallback helper nodes + prev = SigBit(module->wire(it->first), it->second); + break; + } + log_assert(prev != SigBit()); + for (auto &pair : loop) { - RTLIL::AttrObject *obj; if (pair.second == -1) - obj = module->cell(pair.first); - else - obj = module->wire(pair.first); - log_assert(obj); - std::string src; - if (obj->has_attribute(ID::src)) { - std::string src_attr = obj->get_src_attribute(); - src = stringf(" source: %s", src_attr.c_str()); + continue; // helper node for edges fallback, we can ignore it + + struct MatchingEdgePrinter : AbstractCellEdgesDatabase { + std::string &message; + SigMap &sigmap; + SigBit from, to; + int nhits; + const int HITS_LIMIT = 3; + + MatchingEdgePrinter(std::string &message, SigMap &sigmap, SigBit from, SigBit to) + : message(message), sigmap(sigmap), from(from), to(to), nhits(0) {} + + void add_edge(RTLIL::Cell *cell, RTLIL::IdString from_port, int from_bit, + RTLIL::IdString to_port, int to_bit, int) override { + SigBit edge_from = sigmap(cell->getPort(from_port))[from_bit]; + SigBit edge_to = sigmap(cell->getPort(to_port))[to_bit]; + + if (edge_from == from && edge_to == to && nhits++ < HITS_LIMIT) + message += stringf(" %s[%d] --> %s[%d]\n", log_id(from_port), from_bit, + log_id(to_port), to_bit); + if (nhits == HITS_LIMIT) + message += " ...\n"; + } + }; + + Wire *wire = module->wire(pair.first); + log_assert(wire); + SigBit bit(module->wire(pair.first), pair.second); + log_assert(driver_cells.count(bit)); + Cell *driver = driver_cells.at(bit); + + std::string driver_src; + if (driver->has_attribute(ID::src)) { + std::string src_attr = driver->get_src_attribute(); + driver_src = stringf(" source: %s", src_attr.c_str()); } - if (pair.second == -1) { - Cell *cell = module->cell(pair.first); - log_assert(cell); - message += stringf(" cell %s (%s)%s\n", log_id(cell), log_id(cell->type), src.c_str()); - } else { - Wire *wire = module->wire(pair.first); - log_assert(wire); - message += stringf(" wire %s%s\n", log_signal(SigBit(wire, pair.second)), src.c_str()); + message += stringf(" cell %s (%s)%s\n", log_id(driver), log_id(driver->type), driver_src.c_str()); + MatchingEdgePrinter printer(message, sigmap, prev, bit); + printer.add_edges_from_cell(driver); + + std::string wire_src; + if (wire->has_attribute(ID::src)) { + std::string src_attr = wire->get_src_attribute(); + wire_src = stringf(" source: %s", src_attr.c_str()); } + message += stringf(" wire %s%s\n", log_signal(SigBit(wire, pair.second)), wire_src.c_str()); + prev = bit; } log_warning("%s", message.c_str()); counter++;