From b324f3f5ccc7a4a52924f9245e51f07066cd6c00 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 5 Jan 2026 15:47:02 +0100 Subject: [PATCH] Bugfix/enhancement: keeping clusters and nets in sync when joining nets. This will enhance robustness under presence of soft connection errors --- src/db/db/dbCircuit.cc | 10 +- src/db/db/dbHierNetworkProcessor.cc | 56 +++++++- src/db/db/dbHierNetworkProcessor.h | 11 ++ src/db/db/dbLayoutToNetlist.cc | 16 +++ src/db/unit_tests/dbLayoutToNetlistTests.cc | 108 +++++++++++++- src/tl/tl/tlSList.h | 148 +++++++++++--------- testdata/algo/device_extract_l15.gds | Bin 0 -> 4024 bytes 7 files changed, 274 insertions(+), 75 deletions(-) create mode 100644 testdata/algo/device_extract_l15.gds diff --git a/src/db/db/dbCircuit.cc b/src/db/db/dbCircuit.cc index 47237e579..17965840d 100644 --- a/src/db/db/dbCircuit.cc +++ b/src/db/db/dbCircuit.cc @@ -407,14 +407,16 @@ void Circuit::join_nets (Net *net, Net *with) subcircuit->connect_pin (with->begin_subcircuit_pins ()->pin_id (), net); } - while (with->begin_pins () != with->end_pins ()) { - join_pin_with_net (with->begin_pins ()->pin_id (), net); - } - if (netlist ()->callbacks ()) { netlist ()->callbacks ()->link_nets (net, with); } + // NOTE: this needs to happen after the callback was called because further up in the + // hierarchy we want the clusters to be joined already + while (with->begin_pins () != with->end_pins ()) { + join_pin_with_net (with->begin_pins ()->pin_id (), net); + } + // create a new name for the joined net net->set_name (join_names (net->name (), with->name ())); diff --git a/src/db/db/dbHierNetworkProcessor.cc b/src/db/db/dbHierNetworkProcessor.cc index bb60fa35e..256e87ad7 100644 --- a/src/db/db/dbHierNetworkProcessor.cc +++ b/src/db/db/dbHierNetworkProcessor.cc @@ -1577,6 +1577,38 @@ connected_clusters::add_connection (typename local_cluster::id_type id, co m_rev_connections [inst] = id; } +template +void +connected_clusters::rename_connection (const ClusterInstance &inst, typename local_cluster::id_type to_id) +{ + ClusterInstance new_inst (to_id, inst); + + auto rc = m_rev_connections.find (inst); + if (rc == m_rev_connections.end ()) { + return; // TODO: assert? + } + + auto id = rc->second; + + // TODO: this linear search may be slow + auto &connections = m_connections [id]; + for (auto i = connections.begin (); i != connections.end (); ++i) { + if (*i == inst) { + *i = new_inst; + break; + } + } + + m_rev_connections.erase (rc); + + // NOTE: if a connection to the new cluster already exists, we will not insert here. + // This may mean we are connecting two clusters here on parent level. In the netlist, this + // is reflected by having multiple upward pins. Right now, we cannot reflect this case + // in the reverse connection structures and keep the first one only in the reverse + // lookup. + m_rev_connections.insert (std::make_pair (new_inst, id)); +} + template void connected_clusters::join_cluster_with (typename local_cluster::id_type id, typename local_cluster::id_type with_id) @@ -1595,15 +1627,35 @@ connected_clusters::join_cluster_with (typename local_cluster::id_type id, connections_type &to_join = tc->second; for (connections_type::const_iterator c = to_join.begin (); c != to_join.end (); ++c) { - m_rev_connections [*c] = id; + m_rev_connections.insert (std::make_pair (*c, id)); } connections_type &target = m_connections [id]; - target.splice (to_join); + + if (target.empty ()) { + + target.swap (to_join); + + } else if (! to_join.empty ()) { + + // Join while removing duplicates + std::set in_target (target.begin (), target.end ()); + for (auto j = to_join.begin (); j != to_join.end (); ++j) { + if (in_target.find (*j) == in_target.end ()) { + target.push_back (*j); + } + } + + } m_connections.erase (tc); } + + if (m_connected_clusters.find (with_id) != m_connected_clusters.end ()) { + m_connected_clusters.insert (id); + m_connected_clusters.erase (with_id); + } } template diff --git a/src/db/db/dbHierNetworkProcessor.h b/src/db/db/dbHierNetworkProcessor.h index d625a78d3..2bc31bb06 100644 --- a/src/db/db/dbHierNetworkProcessor.h +++ b/src/db/db/dbHierNetworkProcessor.h @@ -844,6 +844,12 @@ public: // .. nothing yet .. } + ClusterInstance (size_t id, const db::ClusterInstElement &inst_element) + : ClusterInstElement (inst_element), m_id (id) + { + // .. nothing yet .. + } + ClusterInstance (size_t id) : ClusterInstElement (), m_id (id) { @@ -1255,6 +1261,11 @@ public: */ void add_connection (typename local_cluster::id_type, const ClusterInstance &inst); + /** + * @brief Changes the cluster ID of the connection + */ + void rename_connection (const ClusterInstance &inst, typename local_cluster::id_type to_id); + /** * @brief Joins the cluster id with the cluster with_id * diff --git a/src/db/db/dbLayoutToNetlist.cc b/src/db/db/dbLayoutToNetlist.cc index 52637e829..9ca1c1f9f 100644 --- a/src/db/db/dbLayoutToNetlist.cc +++ b/src/db/db/dbLayoutToNetlist.cc @@ -218,6 +218,22 @@ void LayoutToNetlist::link_nets (const db::Net *net, const db::Net *with) connected_clusters &clusters = m_net_clusters.clusters_per_cell (net->circuit ()->cell_index ()); clusters.join_cluster_with (net->cluster_id (), with->cluster_id ()); + + // fix the connections from the parents + + const db::Cell &cc = internal_layout ()->cell (net->circuit ()->cell_index ()); + + for (db::Cell::parent_inst_iterator p = cc.begin_parent_insts (); ! p.at_end (); ++p) { + + connected_clusters &pclusters = m_net_clusters.clusters_per_cell (p->parent_cell_index ()); + + db::CellInstArray ci = p->child_inst ().cell_inst (); + for (db::CellInstArray::iterator ia = ci.begin (); ! ia.at_end(); ++ia) { + db::ClusterInstance cli (with->cluster_id (), ci.object ().cell_index (), ci.complex_trans (*ia), p->child_inst ().prop_id ()); + pclusters.rename_connection (cli, net->cluster_id ()); + } + + } } size_t LayoutToNetlist::link_net_to_parent_circuit (const Net *subcircuit_net, Circuit *parent_circuit, const DCplxTrans &dtrans) diff --git a/src/db/unit_tests/dbLayoutToNetlistTests.cc b/src/db/unit_tests/dbLayoutToNetlistTests.cc index 2ed2ebf85..29f947573 100644 --- a/src/db/unit_tests/dbLayoutToNetlistTests.cc +++ b/src/db/unit_tests/dbLayoutToNetlistTests.cc @@ -3388,7 +3388,112 @@ TEST(14_JoinNets) db::compare_layouts (_this, ly, au); } -TEST(15_MeasureNet) +TEST(15_JoinNetsAfterExtraction) +{ + db::Layout ly; + TestRig test_rig (ly); + + { + db::LoadLayoutOptions options; + options.get_options ().layer_map = test_rig.lmap (); + options.get_options ().create_other_layers = false; + + std::string fn (tl::testdata ()); + fn = tl::combine_path (fn, "algo"); + fn = tl::combine_path (fn, "device_extract_l15.gds"); + + tl::InputStream stream (fn); + db::Reader reader (stream); + reader.read (ly, options); + } + + std::unique_ptr l2n (test_rig.make_l2n ()); + + l2n->extract_netlist (); + + db::Netlist *nl = l2n->netlist (); + + EXPECT_EQ (nl->to_string (), + "circuit TOP ();\n" + " subcircuit A $1 (NET1=A,NET2=B);\n" + "end;\n" + "circuit A (NET1=NET1,NET2=NET2);\n" + "end;\n" + ); + + db::Circuit *top_circuit = nl->circuit_by_name ("TOP"); + db::Circuit *a_circuit = nl->circuit_by_name ("A"); + + auto &top_cc = l2n->net_clusters ().clusters_per_cell (top_circuit->cell_index ()); + auto &a_cc = l2n->net_clusters ().clusters_per_cell (a_circuit->cell_index ()); + + db::Net *a_net1 = a_circuit->net_by_name ("NET1"); + db::Net *a_net2 = a_circuit->net_by_name ("NET2"); + db::Net *a_net3 = a_circuit->net_by_name ("NET3"); + tl_assert (a_net1 != 0); + tl_assert (a_net2 != 0); + tl_assert (a_net3 != 0); + + db::Net *top_neta = top_circuit->net_by_name ("A"); + db::Net *top_netb = top_circuit->net_by_name ("B"); + tl_assert (top_neta != 0); + tl_assert (top_netb != 0); + + auto cid1 = a_net1->cluster_id (); + auto cid2 = a_net2->cluster_id (); + auto cid3 = a_net3->cluster_id (); + + EXPECT_EQ (a_cc.is_root (cid1), false); + EXPECT_EQ (a_cc.is_root (cid2), false); + EXPECT_EQ (a_cc.is_root (cid3), true); + + // Join local NET3 and NET2 which has upward connections + + a_circuit->join_nets (a_net3, a_net2); + + EXPECT_EQ (nl->to_string (), + "circuit TOP ();\n" + " subcircuit A $1 (NET1=A,NET2=B);\n" + "end;\n" + "circuit A (NET1=NET1,NET2='NET2,NET3');\n" + "end;\n" + ); + + EXPECT_EQ (a_cc.is_root (cid3), false); + + // B net from top circuit has connections to NET2 which must have changed to NET3 + + const auto &b_conn = top_cc.connections_for_cluster (top_netb->cluster_id ()); + tl_assert (b_conn.size () == size_t (1)); + EXPECT_EQ (b_conn.front ().inst_trans ().to_string (), "r0 *1 -8000,3000"); + EXPECT_EQ (b_conn.front ().inst_cell_index (), a_circuit->cell_index ()); + EXPECT_EQ (b_conn.front ().id (), a_net3->cluster_id ()); + EXPECT_EQ (top_cc.find_cluster_with_connection (b_conn.front ()), top_netb->cluster_id ()); + + // Now join NET1 and NET3 in circuit A which effectively shortens A and B in TOP + + a_circuit->join_nets (a_net3, a_net1); + + EXPECT_EQ (nl->to_string (), + "circuit TOP ();\n" + " subcircuit A $1 ('NET1,NET2'='A,B');\n" + "end;\n" + "circuit A ('NET1,NET2'='NET1,NET2,NET3');\n" + "end;\n" + ); + + const db::Net *top_netab = top_circuit->net_by_name ("A,B"); + tl_assert (top_netab != 0); + + const auto &ab_conn = top_cc.connections_for_cluster (top_netab->cluster_id ()); + tl_assert (ab_conn.size () == size_t (1)); + EXPECT_EQ (ab_conn.front ().inst_trans ().to_string (), "r0 *1 -8000,3000"); + EXPECT_EQ (ab_conn.front ().inst_cell_index (), a_circuit->cell_index ()); + EXPECT_EQ (ab_conn.front ().id (), a_net3->cluster_id ()); + EXPECT_EQ (top_cc.find_cluster_with_connection (ab_conn.front ()), top_netab->cluster_id ()); +} + +TEST(20_MeasureNet) { db::Layout ly; db::LayerMap lmap; @@ -3487,3 +3592,4 @@ TEST(15_MeasureNet) db::compare_layouts (_this, ly, au, db::NoNormalization); } + diff --git a/src/tl/tl/tlSList.h b/src/tl/tl/tlSList.h index 1d28570a9..a5412e40a 100644 --- a/src/tl/tl/tlSList.h +++ b/src/tl/tl/tlSList.h @@ -46,78 +46,90 @@ namespace tl * - empty */ +template +struct slist_node_type +{ + slist_node_type (const T &_t) : next (0), t (_t) { } + slist_node_type (T &&_t) : next (0), t (_t) { } + slist_node_type *next; + T t; +}; + +template +class slist_const_iterator; + +template +class slist_iterator +{ +public: + typedef slist_node_type node_type; + typedef std::forward_iterator_tag iterator_category; + typedef T value_type; + typedef T &reference; + typedef T *pointer; + typedef void difference_type; + + slist_iterator (node_type *p = 0) : mp_p (p) { } + slist_iterator operator++ () { mp_p = mp_p->next; return *this; } + + T *operator-> () const + { + return &mp_p->t; + } + + T &operator* () const + { + return mp_p->t; + } + + bool operator== (slist_iterator other) const { return mp_p == other.mp_p; } + bool operator!= (slist_iterator other) const { return mp_p != other.mp_p; } + +private: + friend class slist_const_iterator; + node_type *mp_p; +}; + +template +class slist_const_iterator +{ +public: + typedef slist_node_type node_type; + typedef std::forward_iterator_tag iterator_category; + typedef const T value_type; + typedef const T &reference; + typedef const T *pointer; + typedef void difference_type; + + slist_const_iterator (slist_iterator i) : mp_p (i.mp_p) { } + slist_const_iterator (const node_type *p = 0) : mp_p (p) { } + slist_const_iterator operator++ () { mp_p = mp_p->next; return *this; } + + const T *operator-> () const + { + return &mp_p->t; + } + + const T &operator* () const + { + return mp_p->t; + } + + bool operator== (slist_const_iterator other) const { return mp_p == other.mp_p; } + bool operator!= (slist_const_iterator other) const { return mp_p != other.mp_p; } + +private: + const node_type *mp_p; +}; + template class slist { -private: - struct node_type - { - node_type (const T &_t) : next (0), t (_t) { } - node_type (T &&_t) : next (0), t (_t) { } - node_type *next; - T t; - }; - public: - class const_iterator; - - class iterator - { - public: - typedef std::forward_iterator_tag category; - typedef T value_type; - typedef T &reference; - typedef T *pointer; - - iterator (node_type *p = 0) : mp_p (p) { } - iterator operator++ () { mp_p = mp_p->next; return *this; } - - T *operator-> () const - { - return &mp_p->t; - } - - T &operator* () const - { - return mp_p->t; - } - - bool operator== (iterator other) const { return mp_p == other.mp_p; } - bool operator!= (iterator other) const { return mp_p != other.mp_p; } - - private: - friend class slist::const_iterator; - node_type *mp_p; - }; - - class const_iterator - { - public: - typedef std::forward_iterator_tag category; - typedef const T value_type; - typedef const T &reference; - typedef const T *pointer; - - const_iterator (iterator i) : mp_p (i.mp_p) { } - const_iterator (const node_type *p = 0) : mp_p (p) { } - const_iterator operator++ () { mp_p = mp_p->next; return *this; } - - const T *operator-> () const - { - return &mp_p->t; - } - - const T &operator* () const - { - return mp_p->t; - } - - bool operator== (const_iterator other) const { return mp_p == other.mp_p; } - bool operator!= (const_iterator other) const { return mp_p != other.mp_p; } - - private: - const node_type *mp_p; - }; + typedef slist_node_type node_type; + typedef T value_type; + typedef slist_const_iterator const_iterator; + typedef slist_iterator iterator; slist () : mp_first (0), mp_last (0), m_size (0) diff --git a/testdata/algo/device_extract_l15.gds b/testdata/algo/device_extract_l15.gds new file mode 100644 index 0000000000000000000000000000000000000000..b9fa14783b3860c70d395bd38c0281cba01c353e GIT binary patch literal 4024 zcma);U1%It6vxlZ&fb~*NL!Q88jba%6+~jxrdDjRBr&NC2}wz|v4%kM=!+m2q=-sU z3u+Mxib(V!h}MU`36?_hARCuCQWwypE+}OX104LF6{o6bMOD2 zb3e|xmkhb?IqG#>{~vOwNLAWT52XK7;F^iCVWRFr@#N5l&yN56?Z$H>*PmSZVy;HL zMZ2zSOl^8-a)!tYh)jn|E}1qYPpEd9v+2?I5op$g;T5pK{)K9{`)3{d7dppShi^=^ zxA-^d`R}a4|NB_KRp{P=@54&9Gc)n0^zi+A)DE%^>p!AYd!jGN{uN^X=3(B&U~d7x zThzUeNnyBMXA%`260bhxo)2TpP_nm~*b09W`Rf=7+`B^b;9x?vgXy!8(La7$^i0;D zI+Kv~al2K|ez_0x$W8IDR6Cf~`^nxk$B)Kf|EyJ7{ActBUX%El?dBi1Tm5r8$H~;V zm1@`ctFpIPzx^&MfH~k$Kvmw;%Jt|Q^q9%f+S4*yd++0)r+OdRtv8R|Und!RKzi?T zUIJsrCTB5nnW)^UwFQyloMN8%MC;MMwY<@$)!gS?AepOk*)l>)9~~#o2t9SFjBsAz z`W9sY5%%3%#KEw@drr`hvC)-(41sqES=*@z-UzBPGEqbimS6IJgp^v+I z>m9@sQ|)FsqfgZ^?CpD4pBuvPd+g)e_#ad4jT?D-XvSHO*hg8PQ0+9HuPfqGbT3o^6 zYEVvgGYs#1hU~c}>_Ap{OGHYw2iG#Qsbuy1QV_j+9D3}gQ0>f2oQOSpBS3u+>wrLd z%-cRJ&%IsmZWQX9QST8vF4Dbm((fAkUCihgD&w|!X>?*D?!Wt;P@6h~z7-tS zPJT}WU3`l<)XVom{Rh62TA5j;OwQkail-Nb%=^#r|AcCXDV;MZz1*0r;oFV%UC3dj z+Es5~x9xjza@P6e@cSrljpcP9eTViF4-@s@)9QPv630tS4f!ei5fosrD9qrebe? z0{tcU2mOnf_s0pvox5rj_ULO=_&;+iT9g|L8;B*~ep53q#AmN1gPS=dRsYnmtdy-6&SG92ZH|T) OC--YsP*g