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

@@ -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 ()));
 

src/db/db/dbHierNetworkProcessor.cc

@@ -1577,6 +1577,38 @@ connected_clusters<T>::add_connection (typename local_cluster<T>::id_type id, co
   m_rev_connections [inst] = id;
 }
 
+template <class T>
+void
+connected_clusters<T>::rename_connection (const ClusterInstance &inst, typename local_cluster<T>::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 <class T>
 void
 connected_clusters<T>::join_cluster_with (typename local_cluster<T>::id_type id, typename local_cluster<T>::id_type with_id)
@@ -1595,15 +1627,35 @@ connected_clusters<T>::join_cluster_with (typename local_cluster<T>::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<connections_type::value_type> 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 <class T>

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<T>::id_type, const ClusterInstance &inst);
 
+  /**
+   *  @brief Changes the cluster ID of the connection
+   */
+  void rename_connection (const ClusterInstance &inst, typename local_cluster<T>::id_type to_id);
+
   /**
    *  @brief Joins the cluster id with the cluster with_id
    *

src/db/db/dbLayoutToNetlist.cc

@@ -218,6 +218,22 @@ void LayoutToNetlist::link_nets (const db::Net *net, const db::Net *with)
 
   connected_clusters<db::NetShape> &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<db::NetShape> &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)

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<db::CommonReaderOptions> ().layer_map = test_rig.lmap ();
+    options.get_options<db::CommonReaderOptions> ().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<db::LayoutToNetlist> 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);
 }
 
+

src/tl/tl/tlSList.h

@@ -46,78 +46,90 @@ namespace tl
  *  - empty
  */
 
+template <class T>
+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 T>
+class slist_const_iterator;
+
+template <class T>
+class slist_iterator
+{
+public:
+  typedef slist_node_type<T> 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<T> other) const { return mp_p == other.mp_p; }
+  bool operator!= (slist_iterator<T> other) const { return mp_p != other.mp_p; }
+
+private:
+  friend class slist_const_iterator<T>;
+  node_type *mp_p;
+};
+
+template <class T>
+class slist_const_iterator
+{
+public:
+  typedef slist_node_type<T> 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<T> 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<T> other) const { return mp_p == other.mp_p; }
+  bool operator!= (slist_const_iterator<T> other) const { return mp_p != other.mp_p; }
+
+private:
+  const node_type *mp_p;
+};
+
 template <class T>
 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<T>::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<T> node_type;
+  typedef T value_type;
+  typedef slist_const_iterator<T> const_iterator;
+  typedef slist_iterator<T> iterator;
 
   slist ()
     : mp_first (0), mp_last (0), m_size (0)