Skip to content

Commit

Permalink
Merge pull request #4184 from povik/check-loop-edges
Browse files Browse the repository at this point in the history 
Use cell edges data in `check`, improve messages
  • Loading branch information
@nakengelhardt
nakengelhardt authored Mar 25, 2024
2 parents c9d9b9e + 206d894 commit c98cdc2
Show file tree
Hide file tree
Showing 8 changed files with 344 additions and 40 deletions.
93 changes: 93 additions & 0 deletions kernel/celledges.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,87 @@ void shift_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell)
}
}

void packed_mem_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell)
{
log_assert(cell->type == ID($mem_v2));
Const rd_clk_enable = cell->getParam(ID::RD_CLK_ENABLE);
int n_rd_ports = cell->getParam(ID::RD_PORTS).as_int();
int abits = cell->getParam(ID::ABITS).as_int();
int width = cell->getParam(ID::WIDTH).as_int();

for (int i = 0; i < n_rd_ports; i++) {
if (rd_clk_enable[i] != State::S0) {
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::RD_ARST, i, ID::RD_DATA, i * width + k, -1);
continue;
}

for (int j = 0; j < abits; j++)
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::RD_ADDR, i * abits + j,
ID::RD_DATA, i * width + k, -1);
}
}

void memrd_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell)
{
log_assert(cell->type.in(ID($memrd), ID($memrd_v2)));
int abits = cell->getParam(ID::ABITS).as_int();
int width = cell->getParam(ID::WIDTH).as_int();

if (cell->getParam(ID::CLK_ENABLE).as_bool()) {
if (cell->type == ID($memrd_v2)) {
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::ARST, 0, ID::DATA, k, -1);
}
return;
}

for (int j = 0; j < abits; j++)
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::ADDR, j, ID::DATA, k, -1);
}

void mem_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell)
{
if (cell->type == ID($mem_v2))
packed_mem_op(db, cell);
else if (cell->type.in(ID($memrd), ID($memrd_v2)))
memrd_op(db, cell);
else if (cell->type.in(ID($memwr), ID($memwr_v2), ID($meminit)))
return; /* no edges here */
else
log_abort();
}

void ff_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell)
{
int width = cell->getPort(ID::Q).size();

if (cell->type.in(ID($dlatch), ID($adlatch), ID($dlatchsr))) {
for (int k = 0; k < width; k++) {
db->add_edge(cell, ID::D, k, ID::Q, k, -1);
db->add_edge(cell, ID::EN, 0, ID::Q, k, -1);
}
}

if (cell->hasPort(ID::CLR))
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::CLR, 0, ID::Q, k, -1);
if (cell->hasPort(ID::SET))
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::SET, 0, ID::Q, k, -1);
if (cell->hasPort(ID::ALOAD))
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::ALOAD, 0, ID::Q, k, -1);
if (cell->hasPort(ID::AD))
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::AD, k, ID::Q, k, -1);
if (cell->hasPort(ID::ARST))
for (int k = 0; k < width; k++)
db->add_edge(cell, ID::ARST, 0, ID::Q, k, -1);
}

PRIVATE_NAMESPACE_END

bool YOSYS_NAMESPACE_PREFIX AbstractCellEdgesDatabase::add_edges_from_cell(RTLIL::Cell *cell)
Expand Down Expand Up @@ -361,6 +442,18 @@ bool YOSYS_NAMESPACE_PREFIX AbstractCellEdgesDatabase::add_edges_from_cell(RTLIL
return true;
}

if (cell->type.in(ID($mem_v2), ID($memrd), ID($memrd_v2), ID($memwr), ID($memwr_v2), ID($meminit))) {
mem_op(this, cell);
return true;
}

if (RTLIL::builtin_ff_cell_types().count(cell->type)) {
ff_op(this, cell);
return true;
}

// FIXME: $mul $div $mod $divfloor $modfloor $slice $concat
// FIXME: $lut $sop $alu $lcu $macc $fa
// FIXME: $mul $div $mod $divfloor $modfloor $pow $slice $concat $bweqx
// FIXME: $lut $sop $alu $lcu $macc $fa $logic_and $logic_or $bwmux

Expand Down
6 changes: 3 additions & 3 deletions kernel/utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ template <typename T, typename C = std::less<T>, typename OPS = hash_ops<T>> cla
std::map<T, int, C> node_to_index;
std::vector<std::set<int, IndirectCmp>> edges;
std::vector<T> sorted;
std::set<std::set<T, C>> loops;
std::set<std::vector<T>> loops;

TopoSort() : indirect_cmp(nodes)
{
Expand Down Expand Up @@ -220,10 +220,10 @@ template <typename T, typename C = std::less<T>, typename OPS = hash_ops<T>> cla
if (active_cells[root_index]) {
found_loops = true;
if (analyze_loops) {
std::set<T, C> loop;
std::vector<T> loop;
for (int i = GetSize(active_stack) - 1; i >= 0; i--) {
const int index = active_stack[i];
loop.insert(nodes[index]);
loop.push_back(nodes[index]);
if (index == root_index)
break;
}
Expand Down
160 changes: 134 additions & 26 deletions passes/cmds/check.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "kernel/yosys.h"
#include "kernel/sigtools.h"
#include "kernel/celledges.h"
#include "kernel/celltypes.h"
#include "kernel/utils.h"

Expand Down Expand Up @@ -102,10 +103,10 @@ struct CheckPass : public Pass {

SigMap sigmap(module);
dict<SigBit, vector<string>> wire_drivers;
dict<SigBit, Cell *> driver_cells;
dict<SigBit, int> wire_drivers_count;
pool<SigBit> used_wires;
TopoSort<string> topo;

TopoSort<std::pair<RTLIL::IdString, int>> topo;
for (auto &proc_it : module->processes)
{
std::vector<RTLIL::CaseRule*> all_cases = {&proc_it.second->root_case};
Expand Down Expand Up @@ -150,6 +151,50 @@ struct CheckPass : public Pass {
}
}

struct CircuitEdgesDatabase : AbstractCellEdgesDatabase {
TopoSort<std::pair<RTLIL::IdString, int>> &topo;
SigMap sigmap;

CircuitEdgesDatabase(TopoSort<std::pair<RTLIL::IdString, int>> &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 {
SigSpec from_portsig = cell->getPort(from_port);
SigSpec to_portsig = cell->getPort(to_port);
log_assert(from_bit >= 0 && from_bit < from_portsig.size());
log_assert(to_bit >= 0 && to_bit < to_portsig.size());
SigBit from = sigmap(from_portsig[from_bit]);
SigBit to = sigmap(to_portsig[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) {
Expand All @@ -158,31 +203,30 @@ struct CheckPass : public Pass {
counter++;
cell_allowed:;
}

for (auto &conn : cell->connections()) {
bool input = cell->input(conn.first);
bool output = cell->output(conn.first);

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)
topo.edge(stringf("wire %s", log_signal(bit)),
stringf("cell %s (%s)", log_id(cell), log_id(cell->type)));
used_wires.insert(bit);
}
if (cell->output(conn.first))
for (int i = 0; i < GetSize(sig); i++) {
if (logic_cell)
topo.edge(stringf("cell %s (%s)", log_id(cell), log_id(cell->type)),
stringf("wire %s", log_signal(sig[i])));

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 (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) || cell->type.in(ID($mem_v2), ID($memrd), ID($memrd_v2)) \
|| RTLIL::builtin_ff_cell_types().count(cell->type))
edges_db.add_edges_from_cell(cell);
}

pool<SigBit> init_bits;
Expand Down Expand Up @@ -239,8 +283,72 @@ struct CheckPass : public Pass {
topo.sort();
for (auto &loop : topo.loops) {
string message = stringf("found logic loop in module %s:\n", log_id(module));
for (auto &str : loop)
message += stringf(" %s\n", str.c_str());

// `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) {
if (pair.second == -1)
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());
}
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);

if (wire->name.isPublic()) {
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++;
}
Expand Down
16 changes: 5 additions & 11 deletions tests/opt/opt_dff_qd.ys
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module top(...);
input CLK;
input EN;
(* init = 24'h555555 *)
output [19:0] Q;
output [17:0] Q;
input SRST;
input ARST;
input [1:0] CLR;
Expand All @@ -23,34 +23,28 @@ $sdffce #(.CLK_POLARITY(1'b1), .EN_POLARITY(1'b1), .SRST_POLARITY(1'b1), .SRST_V
$dffsr #(.CLK_POLARITY(1'b1), .CLR_POLARITY(1'b1), .SET_POLARITY(1'b0), .WIDTH(2)) ff7 (.CLK(CLK), .SET(SET), .CLR(CLR), .D(Q[15:14]), .Q(Q[15:14]));
$dffsre #(.CLK_POLARITY(1'b1), .EN_POLARITY(1'b0), .CLR_POLARITY(1'b1), .SET_POLARITY(1'b0), .WIDTH(2)) ff8 (.CLK(CLK), .EN(EN), .SET(SET), .CLR(CLR), .D(Q[17:16]), .Q(Q[17:16]));

$dlatch #(.EN_POLARITY(1'b1), .WIDTH(2)) ff9 (.EN(EN), .D(Q[19:18]), .Q(Q[19:18]));
$adlatch #(.EN_POLARITY(1'b0), .ARST_POLARITY(1'b1), .ARST_VALUE(2'h2), .WIDTH(2)) ff10 (.EN(EN), .ARST(ARST), .D(Q[21:20]), .Q(Q[21:20]));
$dlatchsr #(.EN_POLARITY(1'b0), .CLR_POLARITY(1'b1), .SET_POLARITY(1'b0), .WIDTH(2)) ff11 (.EN(EN), .SET(SET), .CLR(CLR), .D(Q[23:22]), .Q(Q[23:22]));

endmodule

EOT

design -save orig

# Equivalence check will fail for unmapped adlatch and dlatchsr due to negative hold hack.
delete top/ff10 top/ff11
equiv_opt -undef -assert -multiclock opt_dff -keepdc

design -load orig
opt_dff -keepdc
select -assert-count 1 t:$and
select -assert-count 3 t:$dffe
select -assert-count 3 t:$dlatch
select -assert-count 3 t:$sr
select -assert-count 2 t:$dlatch
select -assert-count 2 t:$sr
select -assert-none t:$and t:$dffe t:$dlatch t:$sr %% %n t:* %i

design -load orig
simplemap
opt_dff -keepdc
select -assert-count 2 t:$_AND_
select -assert-count 6 t:$_DFFE_??_
select -assert-count 6 t:$_DLATCH_?_
select -assert-count 6 t:$_SR_??_
select -assert-count 4 t:$_DLATCH_?_
select -assert-count 4 t:$_SR_??_
select -assert-none t:$_AND_ t:$_DFFE_??_ t:$_DLATCH_?_ t:$_SR_??_ %% %n t:* %i

Loading

0 comments on commit c98cdc2

Please sign in to comment.