From 716369de632e82375687d232eb290480034471ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Mon, 1 Aug 2022 18:50:07 +0200 Subject: [PATCH] Issue 1126 (#1128) * First attempt to fix. Rather experimental. * Debugging and bug fixing The basic issue was a missing break However, correct computation of cache results for instance-to-instance cluster interaction is implemented Plus: identical and overlapping instances are no longer ignored except in the case of exact duplicates. Otherwise these instance generate dead nets which are not connected elsewhere. * Added tests, fixed duplicate cells test, added missing files. * Code simplification (removed invariant from transformation in cluster-to-cluster interaction cache) * Skipping cell instance duplicates as some real-world testcases mandate so * Updated test data --- src/db/db/dbHierNetworkProcessor.cc | 19 ++++++-- .../unit_tests/dbHierNetworkProcessorTests.cc | 45 ++++++++++++++++++ src/db/unit_tests/dbLayoutToNetlistTests.cc | 1 + testdata/algo/issue-1126.gds.gz | Bin 0 -> 421 bytes testdata/algo/issue-1126_au.gds | Bin 0 -> 14700 bytes 5 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 testdata/algo/issue-1126.gds.gz create mode 100644 testdata/algo/issue-1126_au.gds diff --git a/src/db/db/dbHierNetworkProcessor.cc b/src/db/db/dbHierNetworkProcessor.cc index 46fb1e3c1..fd6554b5f 100644 --- a/src/db/db/dbHierNetworkProcessor.cc +++ b/src/db/db/dbHierNetworkProcessor.cc @@ -1582,8 +1582,7 @@ private: * @param interacting_clusters_out Receives the cluster interaction descriptors * * "interacting_clusters_out" will be cluster interactions in the parent instance space of i1 and i2 respectively. - * Cluster ID will be valid in the parent cells containing i1 and i2 and the cluster instances property ID will be set to p1 and p2 - * respectively. + * Cluster ID will be valid in the parent cells containing i1 and i2. */ void consider_instance_pair (const box_type &common, const db::Instance &i1, const db::ICplxTrans &t1, const db::CellInstArray::iterator &i1element, @@ -1629,6 +1628,7 @@ private: db::ICplxTrans tt2 = t2 * i2t; db::ICplxTrans cache_norm = tt1.inverted (); + ii_key = InstanceToInstanceInteraction ((! i1element.at_end () || i1.size () == 1) ? 0 : i1.cell_inst ().delegate (), (! i2element.at_end () || i2.size () == 1) ? 0 : i2.cell_inst ().delegate (), cache_norm, cache_norm * tt2); @@ -1695,9 +1695,14 @@ private: db::ICplxTrans i2t = i2.complex_trans (*ii2); db::ICplxTrans tt2 = t2 * i2t; + if (i1.cell_index () == i2.cell_index () && tt1 == tt2) { // skip interactions between identical instances (duplicate instance removal) - continue; + if (! i2element.at_end ()) { + break; + } else { + continue; + } } box_type ib2 = bb2.transformed (tt2); @@ -1903,6 +1908,7 @@ private: for (std::list >::iterator ii = ii_interactions.begin (); ii != ii_interactions.end (); ++ii) { propagate_cluster_inst (ii->second, i.cell_index (), tt2, i.prop_id ()); } + interacting_clusters.splice (interacting_clusters.end (), ii_interactions, ii_interactions.begin (), ii_interactions.end ()); } @@ -1974,6 +1980,7 @@ private: for (typename std::list::iterator ii = ci_interactions.begin (); ii != ci_interactions.end (); ++ii) { propagate_cluster_inst (ii->other_ci, i2.cell_index (), i2t, i2.prop_id ()); } + interactions_out.splice (interactions_out.end (), ci_interactions, ci_interactions.begin (), ci_interactions.end ()); } @@ -2088,6 +2095,10 @@ private: * * After calling this method, the cluster instance in ci is guaranteed to have connections from all * parent cells. One of these connections represents the instance ci. + * + * Returns false if the connection was already there in the same place (indicating duplicate instances). + * In this case, the cluster instance should be skipped. In the other case, the cluster instance is + * updated to reflect the connected cluster. */ void propagate_cluster_inst (ClusterInstance &ci, db::cell_index_type pci, const db::ICplxTrans &trans, db::properties_id_type prop_id) const { @@ -2178,7 +2189,7 @@ hier_clusters::propagate_cluster_inst (const db::Layout &layout, const db::Ce if (parent_cluster > 0) { - // taken parent + // take parent id_new = parent_cluster; } else { diff --git a/src/db/unit_tests/dbHierNetworkProcessorTests.cc b/src/db/unit_tests/dbHierNetworkProcessorTests.cc index 0c06713ec..d403c281a 100644 --- a/src/db/unit_tests/dbHierNetworkProcessorTests.cc +++ b/src/db/unit_tests/dbHierNetworkProcessorTests.cc @@ -1353,3 +1353,48 @@ TEST(200_issue609) EXPECT_EQ (root_nets (hc.clusters_per_cell (*td)), size_t (0)); } } + +// issue #1126 +TEST(201_issue1126) +{ + { + db::Layout ly; + unsigned int l1 = 0; + + { + db::LayerProperties p; + db::LayerMap lmap; + + p.layer = 1; + p.datatype = 0; + lmap.map (db::LDPair (p.layer, p.datatype), l1 = ly.insert_layer ()); + ly.set_properties (l1, p); + + db::LoadLayoutOptions options; + options.get_options ().layer_map = lmap; + options.get_options ().create_other_layers = false; + + std::string fn (tl::testdata ()); + fn += "/algo/issue-1126.gds.gz"; + tl::InputStream stream (fn); + db::Reader reader (stream); + reader.read (ly, options); + } + + std::vector strings; + normalize_layer (ly, strings, l1); + + // connect 1 to 1 + db::Connectivity conn; + conn.connect (l1, l1); + + db::hier_clusters hc; + hc.build (ly, ly.cell (*ly.begin_top_down ()), conn); + + // should not assert until here + } + + // detailed test: + run_hc_test (_this, "issue-1126.gds.gz", "issue-1126_au.gds"); +} + diff --git a/src/db/unit_tests/dbLayoutToNetlistTests.cc b/src/db/unit_tests/dbLayoutToNetlistTests.cc index 05d70654a..5f843ecdd 100644 --- a/src/db/unit_tests/dbLayoutToNetlistTests.cc +++ b/src/db/unit_tests/dbLayoutToNetlistTests.cc @@ -2846,6 +2846,7 @@ TEST(11_DuplicateInstances) // compare netlist as string CHECKPOINT (); db::compare_netlist (_this, *l2n.netlist (), + // suppressing duplicate cells: "circuit RINGO ();\n" " subcircuit INV2 $1 (IN=$I12,$2=FB,OUT=OSC,$4=VSS,$5=VDD);\n" " subcircuit INV2 $2 (IN=FB,$2=$I25,OUT=$I1,$4=VSS,$5=VDD);\n" diff --git a/testdata/algo/issue-1126.gds.gz b/testdata/algo/issue-1126.gds.gz new file mode 100644 index 0000000000000000000000000000000000000000..1cf858d49a1d23e06cb9da97a0ee4a3b09cc4e5d GIT binary patch literal 421 zcmV;W0b2eaiwFP!000001C5fuOF~f?hTn7U55?+oD!*lO@&+~ofzy}?i z?;^yw=M5eN5kM41_dC2?-^wHazoKo-JYHPcJ6~&adsFr8n+1fl!Je+`$tYAWppS7d z=mrO{Xe!O-iv`3@VkQP`op&hnxCwO}gZyT*R3qMh2(Lq#mul|*&pY)In?zrt%mvB! z;NM%uppLYhdk$s3 zq`Lb@^F(WtJV}`!&Aa>MH1X+xJV}|C)2=?}IifG$_8iKb=Z2oNWoOc2V$v*{E(WDN zUEZV(UM=>JeW!-5z5>-p)tCROkBXUXEybu(mG1w2b1wEy8}-g!=cN5-L~6E&%zxsX PQC0B^fp56(lmq|(1#{Ho literal 0 HcmV?d00001 diff --git a/testdata/algo/issue-1126_au.gds b/testdata/algo/issue-1126_au.gds new file mode 100644 index 0000000000000000000000000000000000000000..d1c79fdbbbefb75307ad87160112033f1e9dfa38 GIT binary patch literal 14700 zcmeHO&ubiY6n{Ir*=%DQqKOJ6)*sL++KXvWL{Zw1ltRUW7Z2VlRM0=52M;+|FyKkV zi>E3=vC{n)~;*<&-pT&@OL6zs;>R?Q-+v z(4G$!@uxMO2>wov)kR);MrsefeGd#Zb-D+?mqv`gTQH>7rWC_~1#&6npN zI1=)o`#9ti89#8OJb&?c$bbL4kWXZM@pyUu>PpDJFck8MjIXYg=GS&LEWh^f4$DuG z@m-Db{6pJA{`}sMPh|Yi_VWCtxsd66yIT-SLaN2b0 zv?(}kKDetUb#qs1W<`<{i;~CMrP+LS^}n(?l-AwPi%fqL@R7D``uwyAk#WW5eKx;4 zn=|V4Y$od$3tY*YP2)^4pw`3VSzWqD=dNkKu8GI~4mm(@9UadQT%F^+aIIIbKa1Mys$-nG z8kzP)#uc-=yl-Q2{aUC?S1YRvs}$-=k#WUtb>+Emoxo)XF1>!n1!UZD`RnMo^g3A% z5L|j4Qv_G%cve?;E@)oK9xJLBwKcrP+Mg|YRd+tSp1LENSBZ=(W}fgfl;<*!>SqnX z0gn1K$%F@3pRoT&B66cc6XVUnQRC2kqC{<1fY8*1J;km)@<~ zY!ew*>^9qZE}LontRXls+eF44m%ol1#s$4_ejw_>Y!eypwXTRayC(NTb{~En|t}xsBL}~uQlO$$4MaC7o&32v(I)Td&Tsqsv1!UZD`Pp_{ z+MDG7!KL$WlC_(bs3f_e4a-6}#2-dyz|fFi!0a-(Mv}#!Gta2d>qkUg{syOMPW} z5gB*>^7H7p7Cw*!IY8xTo&PLD#vPaUz2iEt6u5xkT0WH_xH`u(Pn2-MU&sNnHzMP% zu96ZLpVQB>#2Q;K;^J~o5;B1@@ur?!VDn?2oCJEM82GzIUi#Y}rWcWM#mx79o%pH1h4+2!K3d;jwfkryZ8jh5Ik$0F`51dd2K9CRa?oVsgdQJ|)Ue(`mYBb~@;#`Z3 literal 0 HcmV?d00001