Merge pull request #10289 from The-OpenROAD-Project-staging/debug_write_verilog_bug

avoid duplicate ports/signals in hier verilogs

Author: maliberty

src/dbSta/include/db_sta/dbNetwork.hh

@@ -479,6 +479,12 @@ class dbNetwork : public ConcreteNetwork
   static constexpr unsigned DBIDTAG_WIDTH = 0x4;
 
  private:
+  // Strip the parent-instance prefix from a hierarchical name, treating
+  // backslash-escaped slashes (\/) as literal name characters rather than
+  // hierarchy separators.  Used to recover an in-module name from a
+  // path-qualified one stored in ODB.
+  static std::string stripParentPrefix(const std::string& name);
+
   std::set<const Cell*> hier_modules_;
   std::set<const Port*> concrete_ports_;
   std::unique_ptr<dbEditHierarchy> hierarchy_editor_;

src/dbSta/src/dbNetwork.cc

@@ -786,6 +786,21 @@ ObjectId dbNetwork::id(const Instance* instance) const
   return staToDb(instance)->getId();
 }
 
+std::string dbNetwork::stripParentPrefix(const std::string& name)
+{
+  size_t pos = name.length();
+  while ((pos = name.rfind('/', pos)) != std::string::npos) {
+    if (pos > 0 && name[pos - 1] == '\\') {
+      // Escaped slash inside a Verilog escaped identifier; not a
+      // hierarchy separator.  Keep searching to the left.
+      pos--;
+    } else {
+      return name.substr(pos + 1);
+    }
+  }
+  return name;
+}
+
 std::string dbNetwork::name(const Port* port) const
 {
   if (isConcretePort(port)) {
@@ -808,10 +823,7 @@ std::string dbNetwork::name(const Port* port) const
   }
 
   if (hasHierarchy()) {
-    size_t last_idx = name.find_last_of('/');
-    if (last_idx != std::string::npos) {
-      name = name.substr(last_idx + 1);
-    }
+    name = stripParentPrefix(name);
   }
   return name;
 }
@@ -855,21 +867,7 @@ std::string dbNetwork::name(const Instance* instance) const
   }
 
   if (hasHierarchy()) {
-    size_t last_idx = std::string::npos;
-    size_t pos = name.length();
-    while ((pos = name.rfind('/', pos)) != std::string::npos) {
-      if (pos > 0 && name[pos - 1] == '\\') {
-        // This is an escaped slash, so we should ignore it and continue
-        // searching.
-        pos--;
-      } else {
-        last_idx = pos;
-        break;
-      }
-    }
-    if (last_idx != std::string::npos) {
-      name = name.substr(last_idx + 1);
-    }
+    name = stripParentPrefix(name);
   }
   return name;
 }

src/dbSta/test/BUILD

@@ -23,6 +23,7 @@ ALL_TESTS = [
     "get_ports1_hier",
     "hier2",
     "hier3",
+    "hier_escape_port",
     "hierclock",
     "hierwrite",
     "hier_deep",

src/dbSta/test/CMakeLists.txt

@@ -13,6 +13,7 @@ or_integration_tests(
     get_ports1_hier
     hier2
     hier3
+    hier_escape_port
     hierclock
     hierwrite
     hier_deep

src/dbSta/test/hier_escape_port.ok

@@ -0,0 +1,25 @@
+[INFO ODB-0227] LEF file: Nangate45/Nangate45.lef, created 22 layers, 27 vias, 135 library cells
+[WARNING ORD-0011] Hierarchical flow (-hier) is currently in development and may cause multiple issues. Do not use in production environments.
+module top (clk);
+ input clk;
+
+ wire net_c;
+ wire net_b;
+
+ child h1 (.clk(clk),
+    .\pathA/midB/leaf (net_b),
+    .\pathA/midC/leaf (net_c));
+endmodule
+module child (clk,
+    \pathA/midB/leaf ,
+    \pathA/midC/leaf );
+ input clk;
+ output \pathA/midB/leaf ;
+ output \pathA/midC/leaf ;
+
+
+ DFF_X1 ff_b (.CK(clk),
+    .Q(\pathA/midB/leaf ));
+ DFF_X1 ff_c (.CK(clk),
+    .Q(\pathA/midC/leaf ));
+endmodule

src/dbSta/test/hier_escape_port.tcl

@@ -0,0 +1,12 @@
+# Two output ports in a child module whose Verilog escaped-identifier
+# names differ only in a middle path segment must round-trip through
+# write_verilog as two distinct ports, not collapse to a duplicate.
+source "helpers.tcl"
+read_lef Nangate45/Nangate45.lef
+read_liberty Nangate45/Nangate45_typ.lib
+read_verilog hier_escape_port.v
+link_design -hier top
+
+set verilog_file [make_result_file hier_escape_port.v]
+write_verilog $verilog_file
+report_file $verilog_file

src/dbSta/test/hier_escape_port.v

@@ -0,0 +1,28 @@
+// Regression test for dbNetwork::name(Port*) escaped-identifier handling.
+//
+// `child` has two output ports whose Verilog escaped-identifier names
+// (`\pathA/midB/leaf ` and `\pathA/midC/leaf `) differ only in a middle
+// segment.  Every `/` is a literal name character of the escaped
+// identifier (LRM 5.6.1) and is stored as `\/` in ODB.
+//
+// Before the fix, dbNetwork::name(Port*) stripped at the last `/` without
+// honoring the preceding `\`, collapsing both names to `leaf` and
+// producing a malformed child module with two identically-named output
+// ports on write_verilog round-trip.
+
+module top (clk);
+ input clk;
+ wire net_b;
+ wire net_c;
+ child h1 (.clk(clk),
+           .\pathA/midB/leaf (net_b),
+           .\pathA/midC/leaf (net_c));
+endmodule
+
+module child (clk, \pathA/midB/leaf , \pathA/midC/leaf );
+ input clk;
+ output \pathA/midB/leaf ;
+ output \pathA/midC/leaf ;
+ DFF_X1 ff_b (.CK(clk), .Q(\pathA/midB/leaf ));
+ DFF_X1 ff_c (.CK(clk), .Q(\pathA/midC/leaf ));
+endmodule

src/dbSta/test/hier_weird_port.ok

@@ -22,4 +22,4 @@ modinst 'u_sub' master 'sub':
   head modbterm: 'path\/with\/slash'
 --- pin-direction on u_sub ---
   u_sub/out dir=output
-  u_sub/slash dir=input
+  u_sub/path/with/slash dir=input