From 32fe65adc0bd6813d34960aba4b63ba67991db7a Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 1 Mar 2025 23:18:29 +0100 Subject: [PATCH] [consider merging] Fixed a bug when returning a Region into a layout with cells unselected. --- src/db/db/dbCellMapping.cc | 16 ++- src/db/db/dbCellMapping.h | 5 + src/db/db/dbDeepShapeStore.cc | 122 +++++++++++++-------- src/db/unit_tests/dbDeepShapeStoreTests.cc | 35 ++++++ testdata/algo/dss_bug.gds | Bin 0 -> 920 bytes testdata/algo/dss_bug_au.gds | Bin 0 -> 1208 bytes 6 files changed, 130 insertions(+), 48 deletions(-) create mode 100644 testdata/algo/dss_bug.gds create mode 100644 testdata/algo/dss_bug_au.gds diff --git a/src/db/db/dbCellMapping.cc b/src/db/db/dbCellMapping.cc index 90d9d6f97..3e0d30e37 100644 --- a/src/db/db/dbCellMapping.cc +++ b/src/db/db/dbCellMapping.cc @@ -284,8 +284,17 @@ std::vector CellMapping::source_cells () const return s; } +std::vector CellMapping::target_cells () const +{ + std::vector s; + s.reserve (m_b2a_mapping.size ()); + for (iterator m = begin (); m != end (); ++m) { + s.push_back (m->second); + } + return s; +} -void +void CellMapping::create_single_mapping (const db::Layout & /*layout_a*/, db::cell_index_type cell_index_a, const db::Layout & /*layout_b*/, db::cell_index_type cell_index_b) { clear (); @@ -374,7 +383,7 @@ CellMapping::do_create_missing_mapping (db::Layout &layout_a, const db::Layout & db::PropertyMapper pm (&layout_a, &layout_b); // Note: this avoids frequent cell index table rebuilds if source and target layout are identical - layout_a.start_changes (); + db::LayoutLocker locker (&layout_a); // Create instances for the new cells in layout A according to their instantiation in layout B double mag = layout_b.dbu () / layout_a.dbu (); @@ -405,9 +414,6 @@ CellMapping::do_create_missing_mapping (db::Layout &layout_a, const db::Layout & } - // Note: must be there because of start_changes - layout_a.end_changes (); - } } diff --git a/src/db/db/dbCellMapping.h b/src/db/db/dbCellMapping.h index 42e28fab6..4686e5f16 100644 --- a/src/db/db/dbCellMapping.h +++ b/src/db/db/dbCellMapping.h @@ -206,6 +206,11 @@ public: */ std::vector source_cells () const; + /** + * @brief Gets the target cells + */ + std::vector target_cells () const; + /** * @brief Access to the mapping table */ diff --git a/src/db/db/dbDeepShapeStore.cc b/src/db/db/dbDeepShapeStore.cc index a01df20c5..23a28ef30 100644 --- a/src/db/db/dbDeepShapeStore.cc +++ b/src/db/db/dbDeepShapeStore.cc @@ -1172,62 +1172,69 @@ DeepShapeStore::cell_mapping_to_original (unsigned int layout_index, db::Layout DeliveryMappingCacheKey key (layout_index, tl::id_of (into_layout), into_cell); std::map::iterator cm = m_delivery_mapping_cache.find (key); - if (cm == m_delivery_mapping_cache.end () || ! cm->second.is_valid (*into_layout, *source_layout)) { + if (cm != m_delivery_mapping_cache.end () && cm->second.is_valid (*into_layout, *source_layout)) { + return cm->second; + } - cm = m_delivery_mapping_cache.insert (std::make_pair (key, CellMappingWithGenerationIds ())).first; - cm->second.clear (); + cm = m_delivery_mapping_cache.insert (std::make_pair (key, CellMappingWithGenerationIds ())).first; + cm->second.clear (); - // collects the cell mappings we skip because they are variants (variant building or box variants) - std::map cm_skipped_variants; + // Not found in cache - compute a fresh mapping - if (into_layout == original_builder.source ().layout () && &into_layout->cell (into_cell) == original_builder.source ().top_cell () && original_builder.source ().global_trans ().is_unity ()) { + // collects the cell mappings we skip because they are variants (variant building or box variants) + std::map cm_skipped_variants; - // This is the case of mapping back to the original. In this case we can use the information - // provided inside the original hierarchy builders. They list the source cells and the target cells - // create from them. We need to consider however, that the hierarchy builder is allowed to create - // variants which we cannot map. + if (into_layout == original_builder.source ().layout () && &into_layout->cell (into_cell) == original_builder.source ().top_cell () && original_builder.source ().global_trans ().is_unity ()) { - for (HierarchyBuilder::cell_map_type::const_iterator m = original_builder.begin_cell_map (); m != original_builder.end_cell_map (); ) { + // This is the case of mapping back to the original. In this case we can use the information + // provided inside the original hierarchy builders. They list the source cells and the target cells + // create from them. We need to consider however, that the hierarchy builder is allowed to create + // variants which we cannot map. - HierarchyBuilder::cell_map_type::const_iterator mm = m; + for (HierarchyBuilder::cell_map_type::const_iterator m = original_builder.begin_cell_map (); m != original_builder.end_cell_map (); ) { + + HierarchyBuilder::cell_map_type::const_iterator mm = m; + ++mm; + bool skip = original_builder.is_variant (m->second); // skip variant cells + while (mm != original_builder.end_cell_map () && mm->first.original_cell == m->first.original_cell) { + // we have cell (box) variants and cannot simply map ++mm; - bool skip = original_builder.is_variant (m->second); // skip variant cells - while (mm != original_builder.end_cell_map () && mm->first.original_cell == m->first.original_cell) { - // we have cell (box) variants and cannot simply map - ++mm; - skip = true; - } - - if (! skip) { - cm->second.map (m->second, m->first.original_cell); - } else { - for (HierarchyBuilder::cell_map_type::const_iterator n = m; n != mm; ++n) { - tl_assert (cm_skipped_variants.find (n->second) == cm_skipped_variants.end ()); - cm_skipped_variants [n->second] = n->first; - } - } - - m = mm; - + skip = true; } - } else if (into_layout->cells () == 1) { + if (! skip) { + cm->second.map (m->second, m->first.original_cell); + } else { + for (HierarchyBuilder::cell_map_type::const_iterator n = m; n != mm; ++n) { + tl_assert (cm_skipped_variants.find (n->second) == cm_skipped_variants.end ()); + cm_skipped_variants [n->second] = n->first; + } + } - // Another simple case is mapping into an empty (or single-top-cell-only) layout, where we can use "create_from_single_full". - cm->second.create_single_mapping (*into_layout, into_cell, *source_layout, source_top); - - } else { - - cm->second.create_from_geometry (*into_layout, into_cell, *source_layout, source_top); + m = mm; } - // Add new cells for the variants and (possible) devices which are cells added during the device - // extraction process - std::vector > new_pairs = cm->second.create_missing_mapping2 (*into_layout, *source_layout, source_top, excluded_cells, included_cells); + } else if (into_layout->cells () == 1) { + + // Another simple case is mapping into an empty (or single-top-cell-only) layout, where we can use "create_from_single_full". + cm->second.create_single_mapping (*into_layout, into_cell, *source_layout, source_top); + + } else { + + cm->second.create_from_geometry (*into_layout, into_cell, *source_layout, source_top); + + } + + // Add new cells for the variants and (possible) devices which are cells added during the device + // extraction process + std::vector > new_pairs = cm->second.create_missing_mapping2 (*into_layout, *source_layout, source_top, excluded_cells, included_cells); + + if (! new_pairs.empty ()) { // the variant's originals we are going to delete std::set cells_to_delete; + std::vector > new_variants; // We now need to fix the cell map from the hierarchy builder, so we can import back from the modified layout. // This is in particular important if we created new cells for known variants. @@ -1240,6 +1247,7 @@ DeepShapeStore::cell_mapping_to_original (unsigned int layout_index, db::Layout // create the variant clone in the original layout too and delete this cell VariantsCollectorBase::copy_shapes (*into_layout, np->second, icm->second.original_cell); + new_variants.push_back (std::make_pair (np->second, icm->second.original_cell)); cells_to_delete.insert (icm->second.original_cell); // forget the original cell (now separated into variants) and map the variants back into the @@ -1259,15 +1267,43 @@ DeepShapeStore::cell_mapping_to_original (unsigned int layout_index, db::Layout } - // delete the variant's original cell + if (! new_variants.empty ()) { + + // copy cell instances for the new variants + + // collect the cells that are handled during cell mapping - + // we do not need to take care of them when creating variants, + // but there may be others inside "into_layout" which are + // not present in the DSS and for which we need to copy the + // instances. + std::vector mapped = cm->second.target_cells (); + std::sort (mapped.begin (), mapped.end ()); + + // Copy the variant instances - but only those for cells which are not going to be + // deleted and those not handled by the cell mapping object. + for (auto vv = new_variants.begin (); vv != new_variants.end (); ++vv) { + const db::Cell &from = into_layout->cell (vv->second); + db::Cell &to = into_layout->cell (vv->first); + for (db::Cell::const_iterator i = from.begin (); ! i.at_end (); ++i) { + if (cells_to_delete.find (i->cell_index ()) == cells_to_delete.end ()) { + auto m = std::lower_bound (mapped.begin (), mapped.end (), i->cell_index ()); + if (m == mapped.end () || *m != i->cell_index ()) { + to.insert (*i); + } + } + } + } + + } + if (! cells_to_delete.empty ()) { + // delete the variant original cells into_layout->delete_cells (cells_to_delete); } - cm->second.set_generation_ids (*into_layout, *source_layout); - } + cm->second.set_generation_ids (*into_layout, *source_layout); return cm->second; } diff --git a/src/db/unit_tests/dbDeepShapeStoreTests.cc b/src/db/unit_tests/dbDeepShapeStoreTests.cc index 0acce3658..c0a56db08 100644 --- a/src/db/unit_tests/dbDeepShapeStoreTests.cc +++ b/src/db/unit_tests/dbDeepShapeStoreTests.cc @@ -24,6 +24,8 @@ #include "dbDeepShapeStore.h" #include "dbRegion.h" #include "dbDeepRegion.h" +#include "dbReader.h" +#include "dbTestSupport.h" #include "tlUnitTest.h" #include "tlStream.h" @@ -266,3 +268,36 @@ TEST(5_State) EXPECT_EQ (store.breakout_cells (0)->find (5) != store.breakout_cells (0)->end (), true); EXPECT_EQ (store.breakout_cells (0)->find (3) != store.breakout_cells (0)->end (), true); } + +TEST(6_RestoreWithCellSelection) +{ + db::Layout ly; + + { + std::string fn (tl::testdata ()); + fn += "/algo/dss_bug.gds"; + tl::InputStream stream (fn); + db::Reader reader (stream); + reader.read (ly); + } + + unsigned int l1 = ly.get_layer (db::LayerProperties (1, 0)); + + db::Cell &top_cell = ly.cell (*ly.begin_top_down ()); + + db::RecursiveShapeIterator in_it (ly, top_cell, l1); + + std::set us; + us.insert (ly.cell_by_name ("X").second); + in_it.unselect_cells (us); + + db::DeepShapeStore dss; + + db::Region in_region (in_it, dss); + ly.clear_layer (l1); + + in_region.insert_into (&ly, top_cell.cell_index (), l1); + + db::compare_layouts (_this, ly, tl::testdata () + "/algo/dss_bug_au.gds"); +} + diff --git a/testdata/algo/dss_bug.gds b/testdata/algo/dss_bug.gds new file mode 100644 index 0000000000000000000000000000000000000000..4b55407481498e6a9f0efe859237c496a4e41444 GIT binary patch literal 920 zcmb7CJud`77=Ctkc5dU2;I0tagbK+?i2Fbz=V*kWxZj##L@oifbCuhNXHJ^C^S?F;wrLJo@^-SZa_XTl4&t$)Cw;L^$6 z!QH_@H^Egnzq`ws48{fo53aAdOTKsS-S;5C*Z`r!3U6@Xpn_G@(*NNzaoFns71!I{ zyV^Q<7`^Xw$LqJ(XMKcT_IE;v{Y`Km5Hk!6VwM4@&AIdoDL)q#n&G}$wCfuAJirV1 zl_emSXra)em3$ zQU4b=n&3s_Mg+0s41rSUb?#zKewl$&M}_2*vrU_7+_W{=XRSMSw4of8Piyo>acpjc zst4q?x!*r9Oz9zYv#qIGR63Lv+z(YBN%>ec{bZiMPweXx*E1Z+RVD5uC&`S;gPm~~ zPb}K?+?gT014>rxdsF)^}bJe^2A);~T;Dzr=%j;cuoQ=3?HO3If2&2&mq+JNP@ Wl+P1`A1}bghLjfVI-xxq=lBAUv7J-^ literal 0 HcmV?d00001