Merge pull request #1647 from KLayout/lvs-issues

Lvs issues

src/db/db/dbLayoutToNetlist.cc

@@ -473,69 +473,114 @@ void LayoutToNetlist::check_must_connect (const db::Circuit &c, const db::Net &a
     return;
   }
 
-  if (c.begin_refs () != c.end_refs ()) {
+  std::vector<const db::SubCircuit *> path;
+  check_must_connect_impl (c, a, b, c, a, b, path);
+}
+
+static std::string path_msg (const std::vector<const db::SubCircuit *> &path, const db::Circuit &c_org)
+{
+  if (path.empty ()) {
+    return std::string ();
+  }
+
+  std::string msg (".\n" + tl::to_string (tr ("Instance path: ")));
+
+  for (auto p = path.rbegin (); p != path.rend (); ++p) {
+    if (p != path.rbegin ()) {
+      msg += "/";
+    }
+    msg += (*p)->circuit ()->name () + ":" + (*p)->expanded_name () + "[" + (*p)->trans ().to_string () + "]";
+  }
+
+  msg += "/";
+  msg += c_org.name ();
+
+  return msg;
+}
+
+void LayoutToNetlist::check_must_connect_impl (const db::Circuit &c, const db::Net &a, const db::Net &b, const db::Circuit &c_org, const db::Net &a_org, const db::Net &b_org, std::vector<const db::SubCircuit *> &path)
+{
+  if (c.begin_refs () != c.end_refs () && path.empty ()) {
+
     if (a.begin_pins () == a.end_pins ()) {
-      db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s is not connected to outside")), a.expanded_name ()));
+      db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s is not connected to outside")), a_org.expanded_name ()));
       error.set_cell_name (c.name ());
       error.set_category_name ("must-connect");
       log_entry (error);
     }
     if (b.begin_pins () == b.end_pins ()) {
-      db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s is not connected to outside")), a.expanded_name ()));
+      db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s is not connected to outside")), a_org.expanded_name ()));
       error.set_cell_name (c.name ());
       error.set_category_name ("must-connect");
       log_entry (error);
     }
-  } else {
-    if (a.expanded_name () == b.expanded_name ()) {
-      db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s must be connected further up in the hierarchy - this is an error at chip top level")), a.expanded_name ()));
-      warn.set_cell_name (c.name ());
-      warn.set_category_name ("must-connect");
-      log_entry (warn);
+
+  } else if (c.begin_refs () == c.end_refs () || a.begin_pins () == a.end_pins () || b.begin_pins () == b.end_pins ()) {
+
+    if (a_org.expanded_name () == b_org.expanded_name ()) {
+      if (path.empty ()) {
+        db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s must be connected further up in the hierarchy - this is an error at chip top level")), a_org.expanded_name ()) + path_msg (path, c_org));
+        warn.set_cell_name (c.name ());
+        warn.set_category_name ("must-connect");
+        log_entry (warn);
+      } else {
+        db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s of circuit %s must be connected further up in the hierarchy - this is an error at chip top level")), a_org.expanded_name (), c_org.name ()) + path_msg (path, c_org));
+        warn.set_cell_name (c.name ());
+        warn.set_geometry (subcircuit_geometry (*path.back (), internal_layout ()));
+        warn.set_category_name ("must-connect");
+        log_entry (warn);
+      }
     } else {
-      db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s and %s must be connected further up in the hierarchy - this is an error at chip top level")), a.expanded_name (), b.expanded_name ()));
-      warn.set_cell_name (c.name ());
-      warn.set_category_name ("must-connect");
-      log_entry (warn);
+      if (path.empty ()) {
+        db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s and %s must be connected further up in the hierarchy - this is an error at chip top level")), a_org.expanded_name (), b_org.expanded_name ()) + path_msg (path, c_org));
+        warn.set_cell_name (c.name ());
+        warn.set_category_name ("must-connect");
+        log_entry (warn);
+      } else {
+        db::LogEntryData warn (m_top_level_mode ? db::Error : db::Warning, tl::sprintf (tl::to_string (tr ("Must-connect nets %s and %s of circuit %s must be connected further up in the hierarchy - this is an error at chip top level")), a_org.expanded_name (), b_org.expanded_name (), c_org.name ()) + path_msg (path, c_org));
+        warn.set_cell_name (c.name ());
+        warn.set_geometry (subcircuit_geometry (*path.back (), internal_layout ()));
+        warn.set_category_name ("must-connect");
+        log_entry (warn);
+      }
     }
+
   }
 
   if (a.begin_pins () != a.end_pins () && b.begin_pins () != b.end_pins ()) {
+
     for (auto ref = c.begin_refs (); ref != c.end_refs (); ++ref) {
+
       const db::SubCircuit &sc = *ref;
+
       //  TODO: consider the case of multiple pins on a net (rare)
       const db::Net *net_a = sc.net_for_pin (a.begin_pins ()->pin_id ());
       const db::Net *net_b = sc.net_for_pin (b.begin_pins ()->pin_id ());
+
       if (net_a == 0) {
-        db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s of circuit %s is not connected at all%s")), a.expanded_name (), c.name (), subcircuit_to_string (sc)));
+        db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s of circuit %s is not connected at all%s")), a_org.expanded_name (), c_org.name (), subcircuit_to_string (sc)) + path_msg (path, c_org));
         error.set_cell_name (sc.circuit ()->name ());
         error.set_geometry (subcircuit_geometry (sc, internal_layout ()));
         error.set_category_name ("must-connect");
         log_entry (error);
       }
+
       if (net_b == 0) {
-        db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s of circuit %s is not connected at all%s")), b.expanded_name (), c.name (), subcircuit_to_string (sc)));
+        db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect net %s of circuit %s is not connected at all%s")), b_org.expanded_name (), c_org.name (), subcircuit_to_string (sc)) + path_msg (path, c_org));
         error.set_cell_name (sc.circuit ()->name ());
         error.set_geometry (subcircuit_geometry (sc, internal_layout ()));
         error.set_category_name ("must-connect");
         log_entry (error);
       }
+
       if (net_a && net_b && net_a != net_b) {
-        if (a.expanded_name () == b.expanded_name ()) {
-          db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect nets %s of circuit %s are not connected%s")), a.expanded_name (), c.name (), subcircuit_to_string (sc)));
-          error.set_cell_name (sc.circuit ()->name ());
-          error.set_geometry (subcircuit_geometry (sc, internal_layout ()));
-          error.set_category_name ("must-connect");
-          log_entry (error);
-        } else {
-          db::LogEntryData error (db::Error, tl::sprintf (tl::to_string (tr ("Must-connect nets %s and %s of circuit %s are not connected%s")), a.expanded_name (), b.expanded_name (), c.name (), subcircuit_to_string (sc)));
-          error.set_cell_name (sc.circuit ()->name ());
-          error.set_geometry (subcircuit_geometry (sc, internal_layout ()));
-          error.set_category_name ("must-connect");
-          log_entry (error);
-        }
+        path.push_back (&sc);
+        check_must_connect_impl (*sc.circuit (), *net_a, *net_b, c_org, a_org, b_org, path);
+        path.pop_back ();
       }
+
     }
+
   }
 }
 

src/db/db/dbLayoutToNetlist.h

@@ -1041,6 +1041,7 @@ private:
   void join_nets_from_pattern (db::Circuit &c, const tl::GlobPattern &p);
   void join_nets_from_pattern (db::Circuit &c, const std::set<std::string> &p);
   void check_must_connect (const db::Circuit &c, const db::Net &a, const db::Net &b);
+  void check_must_connect_impl (const db::Circuit &c, const db::Net &a, const db::Net &b, const db::Circuit &c_org, const db::Net &a_org, const db::Net &b_org, std::vector<const db::SubCircuit *> &path);
 
   //  implementation of NetlistManipulationCallbacks
   virtual size_t link_net_to_parent_circuit (const Net *subcircuit_net, Circuit *parent_circuit, const DCplxTrans &trans);

src/db/db/dbNetlist.cc

@@ -126,20 +126,6 @@ void Netlist::set_case_sensitive (bool f)
   m_case_sensitive = f;
 }
 
-int Netlist::name_compare (bool case_sensitive, const std::string &n1, const std::string &n2)
-{
-  //  TODO: unicode support?
-  if (case_sensitive) {
-    return strcmp (n1.c_str (), n2.c_str ());
-  } else {
-#if defined(_WIN32)
-    return _stricmp (n1.c_str (), n2.c_str ());
-#else
-    return strcasecmp (n1.c_str (), n2.c_str ());
-#endif
-  }
-}
-
 std::string Netlist::normalize_name (bool case_sensitive, const std::string &n)
 {
   if (case_sensitive) {

src/db/db/dbNetlist.h

@@ -515,11 +515,6 @@ public:
    */
   void combine_devices ();
 
-  /**
-   *  @brief Compares two names with the given case sensitivity
-   */
-  static int name_compare (bool case_sensitive, const std::string &n1, const std::string &n2);
-
   /**
    *  @brief Normalizes a name with the given case sensitivity
    */

src/db/db/dbNetlistCompareCore.cc

@@ -1355,7 +1355,7 @@ static size_t distance3 (const NetGraphNode &a, const NetGraphNode &b1, const Ne
 static void
 analyze_nodes_for_close_matches (const std::multimap<size_t, const NetGraphNode *> &nodes_by_edges1, const std::multimap<size_t, const NetGraphNode *> &nodes_by_edges2, bool layout2ref, db::NetlistCompareLogger *logger, const db::NetGraph &g2)
 {
-  size_t max_search = 100;
+  size_t max_search = 100000;
   double max_fuzz_factor = 0.25;
   size_t max_fuzz_count = 3;
   size_t max_edges_split = 3;  //  by how many edges joining will reduce the edge count at max
@@ -1368,7 +1368,9 @@ analyze_nodes_for_close_matches (const std::multimap<size_t, const NetGraphNode
     msg = tl::to_string (tr ("Connecting nets %s and %s is making a better match to net %s from reference netlist (fuzziness %d nodes)"));
   }
 
-  for (auto i = nodes_by_edges1.begin (); i != nodes_by_edges1.end (); ++i) {
+  size_t tries = max_search;
+
+  for (auto i = nodes_by_edges1.begin (); i != nodes_by_edges1.end () && tries > 0; ++i) {
 
     if (i->first < min_edges) {
       continue;
@@ -1376,7 +1378,7 @@ analyze_nodes_for_close_matches (const std::multimap<size_t, const NetGraphNode
 
     std::set<const db::NetGraphNode *> seen;
 
-    for (auto j = nodes_by_edges2.begin (); j != nodes_by_edges2.end (); ++j) {
+    for (auto j = nodes_by_edges2.begin (); j != nodes_by_edges2.end () && tries > 0; ++j) {
 
       seen.insert (j->second);
 
@@ -1407,7 +1409,6 @@ analyze_nodes_for_close_matches (const std::multimap<size_t, const NetGraphNode
 
       auto k = nodes_by_edges2.lower_bound (ne);
 
-      size_t tries = max_search;
       for ( ; k != nodes_by_edges2.end () && j->first + k->first < i->first + max_fuzz_count + max_edges_split && tries > 0; ++k) {
 
         if (seen.find (k->second) != seen.end ()) {
@@ -1436,6 +1437,8 @@ analyze_nodes_for_close_matches (const std::multimap<size_t, const NetGraphNode
 void
 NetlistCompareCore::analyze_failed_matches () const
 {
+  tl::SelfTimer timer (tl::verbosity () >= 21, "Analyzing failed matches");
+
   //  Determine the range of nodes with same identity
 
   std::vector<NetGraphNode::edge_type> no_edges;

src/db/db/dbNetlistCompareUtils.cc

@@ -125,9 +125,40 @@ const std::string &extended_net_name (const db::Net *n)
   }
 }
 
+static int net_name_compare (bool case_sensitive, const std::string &n1, const std::string &n2)
+{
+  const char *n1p = n1.c_str ();
+  const char *n2p = n2.c_str ();
+
+  while (*n1p && *n2p) {
+
+    uint32_t c1 = tl::utf32_from_utf8 (n1p);
+    uint32_t c2 = tl::utf32_from_utf8 (n2p);
+
+    if (! case_sensitive) {
+      c1 = tl::utf32_downcase (c1);
+      c2 = tl::utf32_downcase (c2);
+    }
+
+    if (c1 != c2) {
+      return c1 < c2 ? -1 : 1;
+    }
+
+  }
+
+  //  colon terminates net name, such that NET:I is identical to NET.
+  if (*n2p && *n2p != ':') {
+    return -1;
+  } else if (*n1p && *n1p != ':') {
+    return 1;
+  } else {
+    return 0;
+  }
+}
+
 int name_compare (const db::Net *a, const db::Net *b)
 {
-  return db::Netlist::name_compare (combined_case_sensitive (a->netlist (), b->netlist ()), extended_net_name (a), extended_net_name (b));
+  return net_name_compare (combined_case_sensitive (a->netlist (), b->netlist ()), extended_net_name (a), extended_net_name (b));
 }
 
 bool net_names_are_different (const db::Net *a, const db::Net *b)

src/db/db/dbNetlistCompareUtils.h

@@ -92,30 +92,30 @@ std::string nets2string (const std::pair<const db::Net *, const db::Net *> &np);
 /**
  *  @brief Derives the common case sensitivity for two netlists
  */
-bool combined_case_sensitive (const db::Netlist *a, const db::Netlist *b);
+DB_PUBLIC bool combined_case_sensitive (const db::Netlist *a, const db::Netlist *b);
 
 /**
  *  @brief Gets the extended net name
  *  This name is used for comparing the net names and also employs the pin name if one is given
  */
-const std::string &extended_net_name (const db::Net *n);
+DB_PUBLIC const std::string &extended_net_name (const db::Net *n);
 
 /**
  *  @brief Compare two nets by name
  */
-int name_compare (const db::Net *a, const db::Net *b);
+DB_PUBLIC int name_compare (const db::Net *a, const db::Net *b);
 
 /**
  *  @brief Returns a value indicating whether two nets are different by name
  *  Two unnamed nets are never different.
  */
-bool net_names_are_different (const db::Net *a, const db::Net *b);
+DB_PUBLIC bool net_names_are_different (const db::Net *a, const db::Net *b);
 
 /**
  *  @brief Returns a value indicating whether two nets are equal by name
  *  Two unnamed nets are never equal.
  */
-bool net_names_are_equal (const db::Net *a, const db::Net *b);
+DB_PUBLIC bool net_names_are_equal (const db::Net *a, const db::Net *b);
 
 // --------------------------------------------------------------------------------------------------------------------
 //  DeviceCompare definition and implementation

src/db/unit_tests/dbNetlistCompareTests.cc

@@ -25,6 +25,7 @@
 #include "dbNetlistCompare.h"
 #include "dbNetlistCrossReference.h"
 #include "dbNetlistSpiceReader.h"
+#include "dbNetlistCompareUtils.h"
 
 class NetlistCompareTestLogger
   : public db::NetlistCompareLogger
@@ -437,6 +438,101 @@ TEST(0_EqualDeviceParameters)
   EXPECT_EQ (dc.less (d2, d1), false);
 }
 
+TEST(0_NetNameEquivalence)
+{
+  db::Netlist a, b;
+  a.set_case_sensitive (true);
+  b.set_case_sensitive (false);
+
+  EXPECT_EQ (db::combined_case_sensitive (&a, &b), false);
+
+  b.set_case_sensitive (true);
+  EXPECT_EQ (db::combined_case_sensitive (&a, &b), true);
+
+  a.set_case_sensitive (false);
+  EXPECT_EQ (db::combined_case_sensitive (&a, &b), false);
+
+  db::Circuit *ca = new db::Circuit ();
+  ca->set_name ("C");
+  a.add_circuit (ca);
+
+  db::Circuit *cb = new db::Circuit ();
+  cb->set_name ("C");
+  b.add_circuit (cb);
+
+  db::Net *na = new db::Net ("net1");
+  ca->add_net (na);
+
+  db::Net *nb = new db::Net ("net1");
+  cb->add_net (nb);
+
+  EXPECT_EQ (db::name_compare (na, nb), 0);
+
+  nb->set_name ("NET1");
+  EXPECT_EQ (db::name_compare (na, nb), 0);
+
+  nb->set_name ("NET2");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  nb->set_name ("NET11");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  nb->set_name ("net11");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  nb->set_name ("net0abc");
+  EXPECT_EQ (db::name_compare (na, nb), 1);
+
+  nb->set_name ("NET0");
+  EXPECT_EQ (db::name_compare (na, nb), 1);
+
+  a.set_case_sensitive (true);
+  b.set_case_sensitive (true);
+
+  nb->set_name ("net1");
+  EXPECT_EQ (db::name_compare (na, nb), 0);
+
+  nb->set_name ("net2");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  nb->set_name ("net11");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  nb->set_name ("net0");
+  EXPECT_EQ (db::name_compare (na, nb), 1);
+
+  nb->set_name ("NET1");
+  EXPECT_EQ (db::name_compare (na, nb), 1);
+
+  na->set_name ("NET1");
+  nb->set_name ("net1");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  b.set_case_sensitive (false);
+
+  //  colon terminates the net name, so that NET:I and NET and identical
+
+  na->set_name ("NET1:I");
+  nb->set_name ("net1");
+  EXPECT_EQ (db::name_compare (na, nb), 0);
+
+  na->set_name ("NET1:I");
+  nb->set_name ("net1:O");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+
+  na->set_name ("NET1");
+  nb->set_name ("net1:O");
+  EXPECT_EQ (db::name_compare (na, nb), 0);
+
+  na->set_name ("NET2");
+  nb->set_name ("net1:O");
+  EXPECT_EQ (db::name_compare (na, nb), 1);
+
+  na->set_name ("NET1");
+  nb->set_name ("net1abc:O");
+  EXPECT_EQ (db::name_compare (na, nb), -1);
+}
+
 TEST(1_SimpleInverter)
 {
   const char *nls1 =

src/drc/drc/built-in-macros/_drc_netter.rb

@@ -379,7 +379,8 @@ module DRC
           arg1.is_a?(String) || raise("The first argument has to be a string")
           @pre_extract_config << lambda { |l2n| l2n.join_nets(arg1, arg2) }
         else
-          arg1.is_a?(String) || raise("The argument has to be a string")
+          arg1.is_a?(Array) || raise("The argument has to be an array of strings")
+          arg1.find { |a| !a.is_a?(String) } && raise("The argument has to be an array of strings")
           @pre_extract_config << lambda { |l2n| l2n.join_nets(arg1) }
         end
 

src/lvs/unit_tests/lvsSimpleTests.cc

@@ -289,6 +289,12 @@ TEST(31_MustConnect2)
   run_test (_this, "must_connect2", "must_connect2.gds");
 }
 
+//  Intermediate cell propagates must-connect pins
+TEST(32_MustConnect3)
+{
+  run_test (_this, "must_connect3", "must_connect3.gds");
+}
+
 //  issue 1609
 TEST(40_DeviceExtractorErrors)
 {