From 993ef785752c423c5a6f3a9df0d85348312908d9 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 7 Jul 2019 18:17:14 +0200 Subject: [PATCH] WIP: some cleanup/enhancement General topic: abstracts and swappable pins. Issue: we work bottom up and assign pins. This is the basis for net graph building. But swappable means those pins can change. The compare works fine, but debugging output is strange: as the pin assigned is fixed, the nets found to be attached to a circuit might not fit any proposed pin pair (which does not contain swapping). The problem gets worse with abstracts. The enhancements are - Such cases generate only warnings in the browser and the message says swapping might be the case - Floating nets are treated differently. This should lead to a better performance for abstracts/black boxes, but in case of disconnected pins (due to wire errors), floating nets happen to create mismatches in the nets above. - Net graph building does not consider swappable nets. In case of two swappable pins this wouldn't be an issue, but for more than two this would create ambiguities and prevent topological matching. Plus: Debug output option for net graph Tests updated --- src/db/db/dbNetlistCompare.cc | 105 +++++++++++++----- src/db/unit_tests/dbNetlistCompareTests.cc | 40 +++---- .../laybasic/layNetlistBrowserModel.cc | 6 +- 3 files changed, 103 insertions(+), 48 deletions(-) diff --git a/src/db/db/dbNetlistCompare.cc b/src/db/db/dbNetlistCompare.cc index b71b60ed0..fe96c1d4a 100644 --- a/src/db/db/dbNetlistCompare.cc +++ b/src/db/db/dbNetlistCompare.cc @@ -32,6 +32,10 @@ // TODO: make this a feature? // #define PRINT_DEBUG_NETCOMPARE +// verbose net graph output +// TODO: make this a feature? +// #define PRINT_DEBUG_NETGRAPH + // Add this define for case insensitive compare // (applies to circuits, device classes) #define COMPARE_CASE_INSENSITIVE @@ -551,6 +555,25 @@ public: return (m_id1 == other.m_id1 && m_id2 == other.m_id2); } + std::string to_string () const + { + if (is_for_subcircuit ()) { + size_t pin_id1 = std::numeric_limits::max () - m_id1; + size_t pin_id2 = m_id2; + const db::SubCircuit *sc = subcircuit_pair ().first; + const db::Circuit *c = sc->circuit_ref (); + return std::string ("X") + sc->expanded_name () + " " + c->name () + " " + + "(" + c->pin_by_id (pin_id1)->expanded_name () + ")->(" + c->pin_by_id (pin_id2)->expanded_name () + ")"; + } else { + size_t term_id1 = m_id1; + size_t term_id2 = m_id2; + const db::Device *d = device_pair ().first; + const db::DeviceClass *dc = d->device_class (); + return std::string ("D") + d->expanded_name () + " " + dc->name () + " " + + "(" + dc->terminal_definitions () [term_id1].name () + ")->(" + dc->terminal_definitions () [term_id2].name () + ")"; + } + } + private: char m_ref [sizeof (std::pair)]; size_t m_id1, m_id2; @@ -605,27 +628,10 @@ public: const db::SubCircuit *sc = i->subcircuit (); size_t pin_id = i->pin ()->id (); const db::Circuit *cr = sc->circuit_ref (); + const db::Net *net_at_pin = cr->net_for_pin (pin_id); size_t this_pin_id = pin_id; - if (! cr->net_for_pin (pin_id)) { - - // fallback (e.g. when abstract circuits are addressed: just include a transition to 0 - // to make the net distinguishable from a net without this connection. - Transition ed (sc, circuit_categorizer.cat_for_subcircuit (sc), pin_id, pin_id); - - std::map::const_iterator in = n2entry.find (0); - if (in == n2entry.end ()) { - in = n2entry.insert (std::make_pair ((const db::Net *) 0, m_edges.size ())).first; - m_edges.push_back (std::make_pair (std::vector (), std::make_pair (size_t (0), (const db::Net *) 0))); - } - - m_edges [in->second].first.push_back (ed); - - continue; - - } - std::map::const_iterator icm = circuit_map->find (cr); if (icm == circuit_map->end ()) { // this can happen if the other circuit is not present - this is allowed for single-pin @@ -651,6 +657,25 @@ public: pin_id = pin_map->normalize_pin_id (cr, pin_id); + // shortcut for idle pin (e.g. when abstract circuits are addressed: just include a transition to 0 + // to make the net distinguishable from a net without this connection. + + if (! net_at_pin || net_at_pin->is_floating ()) { + + Transition ed (sc, circuit_categorizer.cat_for_subcircuit (sc), pin_id, pin_id); + + std::map::const_iterator in = n2entry.find (0); + if (in == n2entry.end ()) { + in = n2entry.insert (std::make_pair ((const db::Net *) 0, m_edges.size ())).first; + m_edges.push_back (std::make_pair (std::vector (), std::make_pair (size_t (0), (const db::Net *) 0))); + } + + m_edges [in->second].first.push_back (ed); + + continue; + + } + // we cannot afford creating edges from all to all other pins, so we just create edges to the previous and next // pin. This may take more iterations to solve, but should be equivalent. @@ -664,8 +689,6 @@ public: std::vector pids; pids.reserve (take_additional_pins + 1); - // this symmetrizes the pin list with respect to the before-normalization pin id: - pids.push_back (pin_id); for (size_t n = 0; n < take_additional_pins; ++n) { size_t add_pin_id = (pin_id + n + 1) % pin_count; @@ -676,7 +699,7 @@ public: // NOTE: we do not include transitions to equivalent pins in our graph intentionally. // Reasoning: for abstract circuits, transitions are basically useless. For more than // two equivalent pins, the transitions are unpredictable. - && pin_map->normalize_pin_id (cr, cm->this_pin_from_other_pin (add_pin_id)) != pin_id) { + && pin_map->normalize_pin_id (cr, add_pin_id) != pin_id) { pids.push_back (add_pin_id); } else { // skip pins without mapping @@ -689,11 +712,6 @@ public: size_t pin2_id = *i; size_t this_pin2_id = cm->this_pin_from_other_pin (pin2_id); - if (this_pin2_id == this_pin_id) { - // we should not go back to our original, non-normalized pin - continue; - } - // NOTE: if a pin mapping is given, EdgeDesc::pin1_id and EdgeDesc::pin2_id are given // as pin ID's of the other circuit. Transition ed (sc, circuit_categorizer.cat_for_subcircuit (sc), pin_id, pin_map->normalize_pin_id (cr, pin2_id)); @@ -747,6 +765,36 @@ public: } } + std::string to_string () const + { + std::string res = std::string ("["); + if (mp_net) { + res += mp_net->expanded_name (); + } else { + res += "(null)"; + } + res += "]"; + if (m_other_net_index != std::numeric_limits::max ()) { + res += " (other: #" + tl::to_string (m_other_net_index) + ")"; + } + res += "\n"; + + for (std::vector, std::pair > >::const_iterator e = m_edges.begin (); e != m_edges.end (); ++e) { + res += " (\n"; + for (std::vector::const_iterator i = e->first.begin (); i != e->first.end (); ++i) { + res += std::string (" ") + i->to_string () + "\n"; + } + res += " )->"; + if (! e->second.second) { + res += "(null)"; + } else { + res += e->second.second->expanded_name () + "[#" + tl::to_string (e->second.first) + "]"; + } + res += "\n"; + } + return res; + } + const db::Net *net () const { return mp_net; @@ -1170,6 +1218,11 @@ NetGraph::build (const db::Circuit *c, DeviceCategorizer &device_categorizer, Ci for (std::vector::iterator i = m_nodes.begin (); i != m_nodes.end (); ++i) { i->apply_net_index (m_net_index); } +#if defined(PRINT_DEBUG_NETGRAPH) + for (std::vector::iterator i = m_nodes.begin (); i != m_nodes.end (); ++i) { + tl::info << i->to_string () << tl::noendl; + } +#endif } size_t diff --git a/src/db/unit_tests/dbNetlistCompareTests.cc b/src/db/unit_tests/dbNetlistCompareTests.cc index fb4613b62..6c790a387 100644 --- a/src/db/unit_tests/dbNetlistCompareTests.cc +++ b/src/db/unit_tests/dbNetlistCompareTests.cc @@ -1599,8 +1599,8 @@ TEST(11_MismatchingSubcircuits) " device NMOS $2 (S=VSS,G=IN,D=OUT) (L=0.25,W=0.95,AS=0.49875,AD=0.26125,PS=2.95,PD=1.5);\n" "end;\n" "circuit TOP ($0=IN,$1=OUT,$2=VDD,$3=VSS);\n" - " subcircuit INV $1 ($1=IN,$2=INT,$3=VDD,$4=VSS);\n" - " subcircuit INV $2 ($1=INT,$2=OUT,$3=VDD,$4=VSS);\n" + " subcircuit INV $1 ($0=IN,$1=INT,$2=VDD,$3=VSS);\n" + " subcircuit INV $2 ($0=INT,$1=OUT,$2=VDD,$3=VSS);\n" "end;\n"; const char *nls2 = @@ -1610,8 +1610,8 @@ TEST(11_MismatchingSubcircuits) " device PMOS $2 (S=IN,G=IN,D=OUT) (L=0.25,W=0.95,AS=0.49875,AD=0.26125,PS=2.95,PD=1.5);\n" "end;\n" "circuit TOP ($0=OUT,$1=VDD,$2=IN,$3=VSS);\n" - " subcircuit INV $1 ($1=VDD,$2=INT,$3=VSS,$4=OUT);\n" - " subcircuit INV $2 ($1=VDD,$2=IN,$3=VSS,$4=INT);\n" + " subcircuit INV $1 ($0=VDD,$1=INT,$2=VSS,$3=OUT);\n" + " subcircuit INV $2 ($0=VDD,$1=IN,$2=VSS,$3=INT);\n" "end;\n"; db::Netlist nl1, nl2; @@ -1639,10 +1639,12 @@ TEST(11_MismatchingSubcircuits) "end_circuit INV INV NOMATCH\n" "begin_circuit TOP TOP\n" "match_nets OUT OUT\n" - "match_nets VDD VDD\n" + // because VDD is a floating pin in #1 and floating pins are treated different from + // connected ones + "net_mismatch VDD VDD\n" + "match_nets INT INT\n" "match_nets IN IN\n" "match_nets VSS VSS\n" - "match_nets INT INT\n" "match_pins $0 $2\n" "match_pins $1 $0\n" "match_pins $2 $1\n" @@ -1699,7 +1701,7 @@ TEST(11_MismatchingSubcircuits) " net OUT:OUT [Match]\n" " pin $1:$0\n" " subcircuit_pin $2[$1]:$1[$3]\n" - " net VDD:VDD [Match]\n" + " net VDD:VDD [Mismatch]\n" " pin $2:$1\n" " subcircuit_pin $1[$2]:$2[$0]\n" " subcircuit_pin $2[$2]:$1[$0]\n" @@ -1899,10 +1901,10 @@ TEST(14_Subcircuit2Nand) "match_devices $4 $4\n" "end_circuit NAND NAND MATCH\n" "begin_circuit TOP TOP\n" - "match_nets OUT OUT\n" - "match_nets INT INT\n" "match_nets IN2 IN2\n" + "match_nets INT INT\n" "match_nets IN1 IN1\n" + "match_nets OUT OUT\n" "match_nets VDD VDD\n" "match_nets VSS VSS\n" "match_pins $0 $0\n" @@ -2124,10 +2126,10 @@ TEST(14_Subcircuit2MatchWithSwap) "match_devices $4 $4\n" "end_circuit NAND NAND MATCH\n" "begin_circuit TOP TOP\n" - "match_nets OUT OUT\n" - "match_nets INT INT\n" "match_nets IN2 IN2\n" + "match_nets INT INT\n" "match_nets IN1 IN1\n" + "match_nets OUT OUT\n" "match_nets VDD VDD\n" "match_nets VSS VSS\n" "match_pins $0 $0\n" @@ -2194,10 +2196,10 @@ TEST(15_EmptySubCircuitTest) "end_circuit TRANS TRANS MATCH\n" "begin_circuit INV2 INV2\n" "match_nets $5 $5\n" + "match_nets OUT OUT\n" "match_nets $2 $2\n" "match_nets IN IN\n" "match_nets $4 $4\n" - "match_nets OUT OUT\n" "match_pins IN IN\n" "match_pins $1 $1\n" "match_pins OUT OUT\n" @@ -2269,10 +2271,10 @@ TEST(15_EmptySubCircuitWithoutPinNames) "end_circuit TRANS TRANS MATCH\n" "begin_circuit INV2 INV2\n" "match_nets $5 $5\n" + "match_nets OUT OUT\n" "match_nets $2 $2\n" "match_nets IN IN\n" "match_nets $4 $4\n" - "match_nets OUT OUT\n" "match_pins IN IN\n" "match_pins $1 $1\n" "match_pins OUT OUT\n" @@ -2473,12 +2475,12 @@ TEST(17_InherentlyAmbiguousDecoder) "match_nets VDD VDD\n" "match_ambiguous_nets A B\n" "match_ambiguous_nets B A\n" - "match_nets NB NA\n" - "match_nets NA NB\n" - "match_nets NQ0 NQ0\n" - "match_nets NQ2 NQ1\n" "match_nets NQ1 NQ2\n" "match_nets NQ3 NQ3\n" + "match_nets NB NA\n" + "match_nets NQ0 NQ0\n" + "match_nets NA NB\n" + "match_nets NQ2 NQ1\n" "match_pins $0 $0\n" "match_pins $1 $1\n" "match_pins $2 $2\n" @@ -2521,10 +2523,10 @@ TEST(17_InherentlyAmbiguousDecoder) "match_devices $4 $4\n" "end_circuit NAND NAND MATCH\n" "begin_circuit DECODER DECODER\n" - "match_nets NB NB\n" - "match_nets B B\n" "match_nets NA NA\n" "match_nets VDD VDD\n" + "match_nets B B\n" + "match_nets NB NB\n" "match_nets VSS VSS\n" "match_nets NQ0 NQ0\n" "match_nets NQ2 NQ2\n" diff --git a/src/laybasic/laybasic/layNetlistBrowserModel.cc b/src/laybasic/laybasic/layNetlistBrowserModel.cc index 9e9f8b4fb..0bb2b2e23 100644 --- a/src/laybasic/laybasic/layNetlistBrowserModel.cc +++ b/src/laybasic/laybasic/layNetlistBrowserModel.cc @@ -1455,7 +1455,7 @@ NetlistBrowserModel::status (const QModelIndex &index) const if (! is_valid_net_pair (nets_from_subcircuit_pins (subcircuits, pins))) { // This indicates a wrong connection: the nets are associated in a way which is a not // corresponding to a mapped net pair. Report Mismatch here. - return db::NetlistCrossReference::Mismatch; + return db::NetlistCrossReference::MatchWithWarning; } } else if (is_id_circuit_net (id)) { @@ -1492,7 +1492,7 @@ NetlistBrowserModel::status (const QModelIndex &index) const if (! is_valid_net_pair (nets_from_subcircuit_pins (subcircuits, pins))) { // This indicates a wrong connection: the nets are associated in a way which is a not // corresponding to a mapped net pair. Report Mismatch here. - return db::NetlistCrossReference::Mismatch; + return db::NetlistCrossReference::MatchWithWarning; } } @@ -1502,7 +1502,7 @@ NetlistBrowserModel::status (const QModelIndex &index) const static std::string rewire_subcircuit_pins_status_hint () { - return tl::to_string (tr ("The nets attached to the pins are not equivalent.\nRewire the circuit or use 'equivalent_pins' in the LVS script to fix this issue.")); + return tl::to_string (tr ("Either pins or nets don't form a good pair.\nThis means either pin swapping happens (in this case, the nets will still match)\nor the subcircuit wiring is not correct (you'll see an error on the net).")); } QVariant