From d48792a942165be9353c50e1c09bcbfc624b8c97 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 10 Aug 2022 00:41:19 +0200 Subject: [PATCH] Trying to establish first log abilities to LVS compare --- src/db/db/dbNetlistCompare.cc | 58 ++++++++++++++++++++-- src/db/db/dbNetlistCompareCore.cc | 62 ++++++++++++++++++++++-- src/db/db/dbNetlistCompareCore.h | 5 ++ src/db/db/dbNetlistCompareUtils.cc | 23 ++++++++- src/db/db/dbNetlistCompareUtils.h | 2 + src/db/db/dbNetlistCrossReference.cc | 7 ++- src/db/db/dbNetlistCrossReference.h | 17 +++++++ src/layui/layui/NetlistBrowserPage.ui | 13 ++--- src/layui/layui/layNetlistBrowserPage.cc | 22 +++++++-- src/layui/layui/layNetlistLogModel.cc | 31 +++++++++--- src/layui/layui/layNetlistLogModel.h | 2 +- 11 files changed, 211 insertions(+), 31 deletions(-) diff --git a/src/db/db/dbNetlistCompare.cc b/src/db/db/dbNetlistCompare.cc index 849854db3..38bf26d94 100644 --- a/src/db/db/dbNetlistCompare.cc +++ b/src/db/db/dbNetlistCompare.cc @@ -379,8 +379,16 @@ NetlistComparer::compare_impl (const db::Netlist *a, const db::Netlist *b) const } else { if (mp_logger) { - mp_logger->circuit_skipped (ca, cb, generate_subcircuits_not_verified_warning (ca, verified_circuits_a, cb, verified_circuits_b)); + + std::string msg = generate_subcircuits_not_verified_warning (ca, verified_circuits_a, cb, verified_circuits_b); + + if (mp_logger->wants_log_entries ()) { + mp_logger->log_entry (db::NetlistCompareLogger::Error, msg); + } + + mp_logger->circuit_skipped (ca, cb, msg); good = false; + } } @@ -442,7 +450,6 @@ NetlistComparer::all_subcircuits_verified (const db::Circuit *c, const std::set< return false; } } - return true; } @@ -889,6 +896,10 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, // in must_match mode, check if the nets are identical if (mp_logger) { if (p->second && ! exact_match) { + if (mp_logger->wants_log_entries ()) { + mp_logger->log_entry (db::NetlistCompareLogger::Error, + tl::sprintf (tl::to_string (tr ("Nets %s are paired explicitly, but are not identical topologically")), nets2string (p->first))); + } mp_logger->net_mismatch (p->first.first, p->first.second); } else { mp_logger->match_nets (p->first.first, p->first.second); @@ -944,7 +955,8 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, // Three passes: one without ambiguities, the second one with ambiguities and names (optional) and a third with ambiguities with topology - for (int pass = 0; pass < 3 && ! good; ++pass) { + int num_passes = 3; + for (int pass = 0; pass < num_passes && ! good; ++pass) { if (pass == 1 && m_dont_consider_net_names) { // skip the named pass in "don't consider net names" mode @@ -973,6 +985,8 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, compare.logger = mp_logger; compare.progress = &progress; + std::vector nodes, other_nodes; + good = true; while (true) { @@ -1015,7 +1029,8 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, // derive new identities through topology: first collect all nets with the same topological signature - std::vector nodes, other_nodes; + nodes.clear (); + other_nodes.clear (); std::vector no_edges; no_edges.push_back (NetGraphNode::edge_type ()); @@ -1070,6 +1085,10 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, } + if (pass + 1 == num_passes && ! good && mp_logger && mp_logger->wants_log_entries ()) { + compare.analyze_failed_matches (nodes, other_nodes); + } + } // Report missing net assignment @@ -1119,6 +1138,33 @@ NetlistComparer::compare_circuits (const db::Circuit *c1, const db::Circuit *c2, return good; } +static void +analyze_pin_mismatch (const db::Pin *pin1, const db::Circuit *c1, const db::Pin *pin2, const db::Circuit * /*c2*/, db::NetlistCompareLogger *logger) +{ + if (! pin1) { + logger->log_entry (db::NetlistCompareLogger::Error, tl::sprintf (tl::to_string (tr ("No equivalent pin %s from reference netlist found in netlist.\nThis is an indication that a physical connection is not made to the subcircuit.")), pin2->expanded_name ())); + } + + if (! pin2) { + + logger->log_entry (db::NetlistCompareLogger::Error, tl::sprintf (tl::to_string (tr ("No equivalent pin %s from netlist found in reference netlist.\nThis is an indication that additional physical connections are made to the subcircuit cell.")), pin1->expanded_name ())); + + // attempt to identify pins which are creating invalid connections + for (auto p = c1->begin_parents (); p != c1->end_parents (); ++p) { + for (auto c = p->begin_subcircuits (); c != p->end_subcircuits (); ++c) { + const db::SubCircuit &sc = *c; + if (sc.circuit_ref () == c1) { + const db::Net *net = sc.net_for_pin (pin1->id ()); + if (net && (net->subcircuit_pin_count () > 1 || net->terminal_count () > 0 || net->pin_count () > 0)) { + logger->log_entry (db::NetlistCompareLogger::Info, tl::sprintf (tl::to_string (tr ("Potential invalid connection in circuit %s, subcircuit cell reference at %s")), p->name (), sc.trans ().to_string ())); + } + } + } + } + + } +} + bool NetlistComparer::handle_pin_mismatch (const db::NetGraph &g1, const db::Circuit *c1, const db::Pin *pin1, const db::NetGraph &g2, const db::Circuit *c2, const db::Pin *pin2) const { @@ -1163,7 +1209,11 @@ NetlistComparer::handle_pin_mismatch (const db::NetGraph &g1, const db::Circuit } return true; } else { + if (mp_logger) { + if (mp_logger->wants_log_entries ()) { + analyze_pin_mismatch (pin1, c1, pin2, c2, mp_logger); + } mp_logger->pin_mismatch (pin1, pin2); } return false; diff --git a/src/db/db/dbNetlistCompareCore.cc b/src/db/db/dbNetlistCompareCore.cc index e2313c2cc..9fd956b87 100644 --- a/src/db/db/dbNetlistCompareCore.cc +++ b/src/db/db/dbNetlistCompareCore.cc @@ -31,6 +31,7 @@ #include "tlAssert.h" #include "tlLog.h" +#include "tlInternational.h" namespace db { @@ -1045,6 +1046,10 @@ NetlistCompareCore::derive_node_identities_from_ambiguity_group (const NodeRange if (ambiguous) { if (logger) { + if (logger->wants_log_entries ()) { + logger->log_entry (db::NetlistCompareLogger::Warning, + tl::sprintf (tl::to_string (tr ("Matching nets %s from an ambiguous group of nets")), nets2string (p->first->net (), p->second->net ()))); + } logger->match_ambiguous_nets (p->first->net (), p->second->net ()); } for (db::Net::const_pin_iterator i = p->first->net ()->begin_pins (); i != p->first->net ()->end_pins (); ++i) { @@ -1066,8 +1071,10 @@ NetlistCompareCore::derive_node_identities_from_ambiguity_group (const NodeRange std::vector >::const_iterator p = pairs.begin (); for (std::list::iterator tn_of_pair = tn_for_pairs.begin (); tn_of_pair != tn_for_pairs.end (); ++tn_of_pair, ++p) { + bool was_ambiguous = equivalent_other_nodes.has_attribute (p->second); + // Note: this would propagate ambiguities to all *derived* mappings. But this probably goes too far: - // bool ambiguous = equivalent_other_nodes.has_attribute (p->second); + // bool ambiguous = was_ambiguous; // Instead we ignore propagated ambiguitied for now: bool ambiguous = false; @@ -1089,13 +1096,18 @@ NetlistCompareCore::derive_node_identities_from_ambiguity_group (const NodeRange NetGraphNode *n_other = & mp_other_graph->node (other_net_index); if (db::NetlistCompareGlobalOptions::options ()->debug_netcompare || tl::verbosity () >= 40) { - if (ambiguous) { - tl::info << indent_s << "deduced ambiguous match: " << n->net ()->expanded_name () << " vs. " << n_other->net ()->expanded_name (); + if (was_ambiguous) { + tl::info << indent_s << "deduced from ambiguous match: " << n->net ()->expanded_name () << " vs. " << n_other->net ()->expanded_name (); } else { tl::info << indent_s << "deduced match: " << n->net ()->expanded_name () << " vs. " << n_other->net ()->expanded_name (); } } + if (logger && logger->wants_log_entries () && was_ambiguous) { + logger->log_entry (db::NetlistCompareLogger::Info, + tl::sprintf (tl::to_string (tr ("Matching nets %s following an ambiguous match")), nets2string (n->net (), n_other->net ()))); + } + if (ambiguous) { if (logger) { logger->match_ambiguous_nets (n->net (), n_other->net ()); @@ -1234,6 +1246,50 @@ NetlistCompareCore::derive_node_identities_from_singular_match (const NetGraphNo } } +void +NetlistCompareCore::analyze_failed_matches (std::vector &nodes, std::vector &other_nodes) const +{ + // Determine the range of nodes with same identity + + std::vector::const_iterator n1 = nodes.begin (); + std::vector::const_iterator n2 = other_nodes.begin (); + + std::vector::const_iterator> singular1, singular2; + + while (n1 != nodes.end () && n2 != other_nodes.end ()) { + + if (n1->node->has_other ()) { + ++n1; + continue; + } else if (n2->node->has_other ()) { + ++n2; + continue; + } + + if (*n1->node < *n2->node) { + singular1.push_back (n1); + ++n1; + continue; + } else if (*n2->node < *n1->node) { + singular2.push_back (n2); + ++n2; + continue; + } + + ++n1; + ++n2; + + } + + for (auto i = singular1.begin (); i != singular1.end (); ++i) { + logger->log_entry (db::NetlistCompareLogger::Error, tl::sprintf (tl::to_string (tr ("Net %s from primary netlist does not match topologically to any net from reference netlist")), (*i)->node->net ()->expanded_name ())); + } + + for (auto i = singular2.begin (); i != singular2.end (); ++i) { + logger->log_entry (db::NetlistCompareLogger::Error, tl::sprintf (tl::to_string (tr ("Net %s from reference netlist does not match topologically to any net from primary netlist")), (*i)->node->net ()->expanded_name ())); + } +} + size_t NetlistCompareCore::derive_node_identities_from_node_set (std::vector &nodes, std::vector &other_nodes) const { diff --git a/src/db/db/dbNetlistCompareCore.h b/src/db/db/dbNetlistCompareCore.h index 659fe7498..e496920b5 100644 --- a/src/db/db/dbNetlistCompareCore.h +++ b/src/db/db/dbNetlistCompareCore.h @@ -85,6 +85,11 @@ public: */ size_t derive_node_identities_from_node_set (std::vector &nodes, std::vector &other_nodes) const; + /** + * @brief Analyzes the non-matched remaining nodes and produces log output + */ + void analyze_failed_matches (std::vector &nodes, std::vector &other_nodes) const; + size_t max_depth; size_t max_n_branch; bool depth_first; diff --git a/src/db/db/dbNetlistCompareUtils.cc b/src/db/db/dbNetlistCompareUtils.cc index 11608333a..95be3973f 100644 --- a/src/db/db/dbNetlistCompareUtils.cc +++ b/src/db/db/dbNetlistCompareUtils.cc @@ -61,7 +61,8 @@ NetlistCompareGlobalOptions::options () // -------------------------------------------------------------------------------------------------------------------- // Some utilities -std::string nl_compare_debug_indent (size_t depth) +std::string +nl_compare_debug_indent (size_t depth) { std::string s; for (size_t d = 0; d < depth; ++d) { @@ -70,6 +71,26 @@ std::string nl_compare_debug_indent (size_t depth) return s; } +const std::string var_sep (" \u21D4 "); + +std::string +nets2string (const db::Net *a, const db::Net *b) +{ + if (a->expanded_name () != b->expanded_name ()) { + return a->expanded_name () + var_sep + b->expanded_name (); + } else { + return a->expanded_name (); + } +} + +std::string +nets2string (const std::pair &np) +{ + return nets2string (np.first, np.second); +} + + + // -------------------------------------------------------------------------------------------------------------------- // Some functions diff --git a/src/db/db/dbNetlistCompareUtils.h b/src/db/db/dbNetlistCompareUtils.h index fc74c4193..7570fabb6 100644 --- a/src/db/db/dbNetlistCompareUtils.h +++ b/src/db/db/dbNetlistCompareUtils.h @@ -83,6 +83,8 @@ const size_t unknown_id = std::numeric_limits::max () - 1; // Some utilities std::string nl_compare_debug_indent (size_t depth); +std::string nets2string (const db::Net *a, const db::Net *b); +std::string nets2string (const std::pair &np); // -------------------------------------------------------------------------------------------------------------------- // Net name compare diff --git a/src/db/db/dbNetlistCrossReference.cc b/src/db/db/dbNetlistCrossReference.cc index 65fc77923..7b9b7c408 100644 --- a/src/db/db/dbNetlistCrossReference.cc +++ b/src/db/db/dbNetlistCrossReference.cc @@ -27,6 +27,7 @@ namespace db { NetlistCrossReference::NetlistCrossReference () + : m_wants_log_entries (true) { // .. nothing yet .. } @@ -419,7 +420,11 @@ NetlistCrossReference::gen_end_circuit (const db::Circuit *, const db::Circuit * void NetlistCrossReference::gen_log_entry (Severity severity, const std::string &msg) { - mp_per_circuit_data->log_entries.push_back (LogEntryData (severity, msg)); + if (mp_per_circuit_data) { + mp_per_circuit_data->log_entries.push_back (LogEntryData (severity, msg)); + } else { + m_other_log_entries.push_back (LogEntryData (severity, msg)); + } } void diff --git a/src/db/db/dbNetlistCrossReference.h b/src/db/db/dbNetlistCrossReference.h index 9a81519c5..015a5326b 100644 --- a/src/db/db/dbNetlistCrossReference.h +++ b/src/db/db/dbNetlistCrossReference.h @@ -258,6 +258,16 @@ public: gen_subcircuits (a, b, Mismatch, msg); } + virtual bool wants_log_entries () const + { + return m_wants_log_entries; + } + + void set_wants_log_entries (bool f) + { + m_wants_log_entries = f; + } + void clear (); size_t circuit_count () const @@ -279,6 +289,11 @@ public: return m_circuits.end (); } + const PerCircuitData::log_entries_type &other_log_entries () const + { + return m_other_log_entries; + } + const db::Pin *other_pin_for (const db::Pin *pin) const; const db::Device *other_device_for (const db::Device *device) const; const db::SubCircuit *other_subcircuit_for (const db::SubCircuit *subcircuit) const; @@ -313,6 +328,8 @@ private: std::map m_other_subcircuit; std::pair m_current_circuits; PerCircuitData *mp_per_circuit_data; + PerCircuitData::log_entries_type m_other_log_entries; + bool m_wants_log_entries; void establish_pair (const db::Circuit *a, const db::Circuit *b); void establish_pair (const db::Net *a, const db::Net *b, Status status, const std::string &msg); diff --git a/src/layui/layui/NetlistBrowserPage.ui b/src/layui/layui/NetlistBrowserPage.ui index 5e84ba2d0..74697088b 100644 --- a/src/layui/layui/NetlistBrowserPage.ui +++ b/src/layui/layui/NetlistBrowserPage.ui @@ -573,6 +573,9 @@ + + Qt::ActionsContextMenu + true @@ -620,16 +623,6 @@ - - - Collapse All - - - - - Expand All - - true diff --git a/src/layui/layui/layNetlistBrowserPage.cc b/src/layui/layui/layNetlistBrowserPage.cc index 350ddebce..ee30ee713 100644 --- a/src/layui/layui/layNetlistBrowserPage.cc +++ b/src/layui/layui/layNetlistBrowserPage.cc @@ -139,9 +139,19 @@ NetlistBrowserPage::NetlistBrowserPage (QWidget * /*parent*/) m_show_all_action->setCheckable (true); m_show_all_action->setChecked (m_show_all); + { + QAction *collapse_all = new QAction (QObject::tr ("Collapse All"), log_view); + connect (collapse_all, SIGNAL (triggered ()), log_view, SLOT (collapseAll ())); + log_view->addAction (collapse_all); + + QAction *expand_all = new QAction (QObject::tr ("Expand All"), log_view); + connect (expand_all, SIGNAL (triggered ()), log_view, SLOT (expandAll ())); + log_view->addAction (expand_all); + } + QTreeView *dt[] = { nl_directory_tree, sch_directory_tree, xref_directory_tree }; - for (int m = 0; m < sizeof (dt) / sizeof (dt[0]); ++m) { + for (int m = 0; m < int (sizeof (dt) / sizeof (dt[0])); ++m) { QTreeView *directory_tree = dt[m]; @@ -152,8 +162,12 @@ NetlistBrowserPage::NetlistBrowserPage (QWidget * /*parent*/) QAction *sep; directory_tree->addAction (m_show_all_action); - directory_tree->addAction (actionCollapseAll); + QAction *collapse_all = new QAction (QObject::tr ("Collapse All"), directory_tree); + connect (collapse_all, SIGNAL (triggered ()), directory_tree, SLOT (collapseAll ())); + directory_tree->addAction (collapse_all); // TODO: this gives a too big tree - confine to single branches? + // QAction *expand_all = new QAction (QObject::tr ("Expand All"), directory_tree); + // connect (expand_all, SIGNAL (triggered ()), directory_tree, SLOT (expandAll ())); // directory_tree->addAction (actionExpandAll); sep = new QAction (directory_tree); sep->setSeparator (true); @@ -185,7 +199,7 @@ NetlistBrowserPage::NetlistBrowserPage (QWidget * /*parent*/) QTreeView *ht[] = { nl_hierarchy_tree, sch_hierarchy_tree, xref_hierarchy_tree }; - for (int m = 0; m < sizeof (ht) / sizeof (ht[0]); ++m) { + for (int m = 0; m < int (sizeof (ht) / sizeof (ht[0])); ++m) { QTreeView *hierarchy_tree = ht[m]; @@ -903,7 +917,7 @@ static QModelIndex find_next (QTreeView *view, QAbstractItemModel *model, const bool has_next = false; - if (model->hasChildren (current) && rows_stack.size () < max_depth - 1) { + if (model->hasChildren (current) && int (rows_stack.size ()) < max_depth - 1) { int row_count = model->rowCount (current); if (row_count > 0) { diff --git a/src/layui/layui/layNetlistLogModel.cc b/src/layui/layui/layNetlistLogModel.cc index 650218fe1..db146893e 100644 --- a/src/layui/layui/layNetlistLogModel.cc +++ b/src/layui/layui/layNetlistLogModel.cc @@ -39,7 +39,14 @@ namespace { { inline bool operator() (const Obj *a, const Obj *b) const { - return a->name () < b->name (); + if ((a != 0) != (b != 0)) { + return (a != 0) < (b != 0); + } + if (! a) { + return false; + } else { + return a->name () < b->name (); + } } }; @@ -60,8 +67,8 @@ namespace { struct CircuitsCompareByName { - bool operator() (const std::pair, const db::NetlistCrossReference::PerCircuitData *> &a, - const std::pair, const db::NetlistCrossReference::PerCircuitData *> &b) const + bool operator() (const std::pair, const db::NetlistCrossReference::PerCircuitData::log_entries_type *> &a, + const std::pair, const db::NetlistCrossReference::PerCircuitData::log_entries_type *> &b) const { return sort_pair > () (a.first, b.first); } @@ -77,10 +84,14 @@ NetlistLogModel::NetlistLogModel (QWidget *parent, const db::NetlistCrossReferen tl_assert (cross_ref->netlist_a () != 0); tl_assert (cross_ref->netlist_b () != 0); + if (! cross_ref->other_log_entries ().empty ()) { + m_circuits.push_back (std::make_pair (std::make_pair ((const db::Circuit *)0, (const db::Circuit *)0), &cross_ref->other_log_entries ())); + } + for (auto i = cross_ref->begin_circuits (); i != cross_ref->end_circuits (); ++i) { const db::NetlistCrossReference::PerCircuitData *pcd = cross_ref->per_circuit_data_for (*i); if (pcd && i->first && i->second && ! pcd->log_entries.empty ()) { - m_circuits.push_back (std::make_pair (*i, pcd)); + m_circuits.push_back (std::make_pair (*i, &pcd->log_entries)); } } @@ -126,7 +137,7 @@ NetlistLogModel::rowCount (const QModelIndex &parent) const } else if (parent.parent ().isValid ()) { return 0; } else if (parent.row () >= 0 && parent.row () < int (m_circuits.size ())) { - return int (m_circuits [parent.row ()].second->log_entries.size ()); + return int (m_circuits [parent.row ()].second->size ()); } else { return 0; } @@ -145,7 +156,7 @@ NetlistLogModel::data (const QModelIndex &index, int role) const if (index.parent ().isValid ()) { const circuit_entry *ce = (const circuit_entry *) index.internalPointer (); if (ce) { - le = &ce->second->log_entries [index.row ()]; + le = &(*ce->second) [index.row ()]; } } @@ -169,7 +180,9 @@ NetlistLogModel::data (const QModelIndex &index, int role) const } } else if (index.row () >= 0 && index.row () < int (m_circuits.size ())) { const std::pair &cp = m_circuits [index.row ()].first; - if (cp.first->name () != cp.second->name ()) { + if (! cp.first || ! cp.second) { + return QVariant (tr ("General")); + } else if (cp.first->name () != cp.second->name ()) { return QVariant (tr ("Circuit ") + tl::to_qstring (cp.first->name () + var_sep + cp.second->name ())); } else { return QVariant (tr ("Circuit ") + tl::to_qstring (cp.first->name ())); @@ -184,6 +197,10 @@ NetlistLogModel::data (const QModelIndex &index, int role) const f.setBold (true); return QVariant (f); } + } else { + QFont f; + f.setBold (true); + return QVariant (f); } } else if (role == Qt::ForegroundRole) { diff --git a/src/layui/layui/layNetlistLogModel.h b/src/layui/layui/layNetlistLogModel.h index 5d8253a47..4dcdde4da 100644 --- a/src/layui/layui/layNetlistLogModel.h +++ b/src/layui/layui/layNetlistLogModel.h @@ -51,7 +51,7 @@ public: virtual QVariant headerData (int section, Qt::Orientation orientation, int role) const; private: - typedef std::pair, const db::NetlistCrossReference::PerCircuitData *> circuit_entry; + typedef std::pair, const db::NetlistCrossReference::PerCircuitData::log_entries_type *> circuit_entry; std::vector m_circuits; };