From 37977d21e9b75f52f925a20c9d25f02fbd640a08 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 25 May 2026 00:09:38 +0200 Subject: [PATCH] Fixing issue #2350 (strm2oas writes empty OAS) Problem was that the OASIS writer was simply ignoring all top level proxy cells (PCells, Library references). The original bug #1835 fixed that by changing the reader behavior, so it would keep top level proxies. However, doing the spin through the writer got them removed and in addition, the cleanup happing during editing would also remove them. Solution is to centralize the strategy of cleaning cells. The cleanup now is changed to not remove proxy top cells if they are the only ones. This is consistent with the previous reader-only behavior. The writer implements the same behavior by means dropping cells marked for cleanup, instead of simply skipping all proxy cells. --- src/db/db/dbCommonReader.cc | 25 +--- src/db/db/dbLayout.cc | 101 +++++++++++--- src/db/db/dbLayout.h | 13 ++ src/db/db/dbSaveLayoutOptions.cc | 17 ++- src/db/unit_tests/dbLayoutTests.cc | 131 ++++++++++++++++++ .../gds2/db_plugin/dbGDS2WriterBase.cc | 2 +- .../oasis/db_plugin/dbOASISWriter.cc | 10 +- testdata/lstream/pcells_au.oas | Bin 366 -> 420 bytes 8 files changed, 245 insertions(+), 54 deletions(-) diff --git a/src/db/db/dbCommonReader.cc b/src/db/db/dbCommonReader.cc index 617ea3f1f..15cb0fb2e 100644 --- a/src/db/db/dbCommonReader.cc +++ b/src/db/db/dbCommonReader.cc @@ -600,30 +600,7 @@ CommonReader::read (db::Layout &layout, const db::LoadLayoutOptions &options) // A cleanup may be necessary because of the following scenario: if library proxies contain subcells // which are proxies themselves, the proxy update may make them orphans (the proxies are regenerated). // The cleanup will removed these. - - // Adressing issue #1835 (reading proxy-only GDS file renders empty layout) we do not delete - // the first (non-cold) proxy if there are only proxy top cells. - // We never clean up the top cell if there is a single one. This catches the case of having - // defunct proxies for top cells. - - std::set keep; - if (layout.end_top_cells () - layout.begin_top_down () == 1) { - keep.insert (*layout.begin_top_down ()); - } else { - for (auto c = layout.begin_top_down (); c != layout.end_top_cells (); ++c) { - const db::Cell *cptr = &layout.cell (*c); - if (cptr->is_proxy ()) { - if (! dynamic_cast (cptr) && keep.empty ()) { - keep.insert (*c); - } - } else { - keep.clear (); - break; - } - } - } - - layout.cleanup (keep); + layout.cleanup (); return layer_map_out (); } diff --git a/src/db/db/dbLayout.cc b/src/db/db/dbLayout.cc index fb9cbbc04..071f30aff 100644 --- a/src/db/db/dbLayout.cc +++ b/src/db/db/dbLayout.cc @@ -1723,6 +1723,84 @@ Layout::refresh () } } +std::set +Layout::cells_to_cleanup (const std::set &keep_always) const +{ + std::set to_clean; + + // Adressing issue #1835 (reading proxy-only GDS file renders empty layout) we do not delete + // the first (non-cold) proxy if there are only proxy top cells. + // We never clean up the top cell if there is a single one. This catches the case of having + // defunct proxies for top cells. + + if (end_top_cells () - begin_top_down () > 1) { + + std::set keep; + + for (auto c = begin_top_down (); c != end_top_cells (); ++c) { + const db::Cell *cptr = &cell (*c); + if (cptr->is_proxy ()) { + if (! dynamic_cast (cptr) && keep.empty ()) { + keep.insert (*c); + } + } else { + keep.clear (); + break; + } + + } + + for (auto c = begin_top_down (); c != end_top_cells (); ++c) { + if (cell (*c).is_proxy () && keep.find (*c) == keep.end () && keep_always.find (*c) == keep_always.end ()) { + to_clean.insert (*c); + } + } + + } + + // determine all cells that can be deleted as well because they are called + // by cells registered for cleanup and nowhere else + + if (! to_clean.empty ()) { + + // collect the called cells + std::set all_called; + for (auto c = to_clean.begin (); c != to_clean.end (); ++c) { + cell (*c).collect_called_cells (all_called, -1); + } + + // remove all called cells which are not proxies - we don't want to + // clean up those and their children. + std::set called; + for (auto c = all_called.begin (); c != all_called.end (); ++c) { + if (cell (*c).is_proxy ()) { + called.insert (*c); + } + } + + // From these cells erase all cells that have parents outside the subtree of our cell. + // Make sure this is done recursively by doing this top-down. + for (auto c = begin_top_down (); c != end_top_down (); ++c) { + if (called.find (*c) != called.end ()) { + const db::Cell &ccref = cell (*c); + for (db::Cell::parent_cell_iterator pc = ccref.begin_parent_cells (); pc != ccref.end_parent_cells (); ++pc) { + if (to_clean.find (*pc) == to_clean.end () && called.find (*pc) == called.end ()) { + // we have a parent outside the subset considered currently (either the cell was never in or + // it was removed itself already): remove this cell from the set of valid subcells. + called.erase (*c); + break; + } + } + } + } + + to_clean.insert (called.begin (), called.end ()); + + } + + return to_clean; +} + void Layout::cleanup (const std::set &keep) { @@ -1801,28 +1879,17 @@ Layout::cleanup (const std::set &keep) } - std::set cells_to_delete; - // deleting cells may create new top cells which need to be deleted as well, hence we iterate - // until there are no more cells to delete - while (true) { + // Remove all cells that are targeted for cleanup - // delete all cells that are top cells and are proxies. Those cells are proxies no longer required. - for (top_down_iterator c = begin_top_down (); c != end_top_cells (); ++c) { - if (cell (*c).is_proxy () && keep.find (*c) == keep.end ()) { - cells_to_delete.insert (*c); - } + { + std::set cells_to_delete = cells_to_cleanup (keep); + if (! cells_to_delete.empty ()) { + delete_cells (cells_to_delete); } - - if (cells_to_delete.empty ()) { - break; - } - - delete_cells (cells_to_delete); - cells_to_delete.clear (); - } + // Try to ensure that cell names reflect the library cell names. The latter is good for LVS for example. for (auto c = m_lib_proxy_map.begin (); c != m_lib_proxy_map.end (); ++c) { diff --git a/src/db/db/dbLayout.h b/src/db/db/dbLayout.h index b9bc0f1c9..36ec43dc5 100644 --- a/src/db/db/dbLayout.h +++ b/src/db/db/dbLayout.h @@ -1682,6 +1682,19 @@ public: */ void cleanup (const std::set &keep = std::set ()); + /** + * @brief Gets a list of cells which are cleanup candidates + * + * The returned list also includes cells that are indirectly cleaned up. + * The cleanup only removes top cells which represent unused proxies + * with some exceptions - for example proxies that are single top cells + * are retained. + * + * However, removing the top cells should not produce new top cells + * of proxies, so these cells need to be removed as well. + */ + std::set cells_to_cleanup (const std::set &keep = std::set ()) const; + /** * @brief Calls "update" on all cells of the layout * diff --git a/src/db/db/dbSaveLayoutOptions.cc b/src/db/db/dbSaveLayoutOptions.cc index 236a3f38c..33c14497b 100644 --- a/src/db/db/dbSaveLayoutOptions.cc +++ b/src/db/db/dbSaveLayoutOptions.cc @@ -372,12 +372,19 @@ SaveLayoutOptions::get_cells (const db::Layout &layout, std::set cleaned_cells = layout.cells_to_cleanup (); + bool has_skipped_replica = false; // check if we have skipped replicas if (has_context) { for (db::Layout::const_iterator cell = layout.begin (); cell != layout.end () && ! has_skipped_replica; ++cell) { - has_skipped_replica = cell->can_skip_replica (); + if (cell->can_skip_replica () && cleaned_cells.find (cell->cell_index ()) == cleaned_cells.end ()) { + has_skipped_replica = true; + } } } @@ -386,7 +393,7 @@ SaveLayoutOptions::get_cells (const db::Layout &layout, std::set is_top ()) { + if (cell->is_top () && cleaned_cells.find (cell->cell_index ()) == cleaned_cells.end ()) { cells.insert (cell->cell_index ()); collect_called_cells_unskipped (cell->cell_index (), layout, cells); } @@ -394,8 +401,10 @@ SaveLayoutOptions::get_cells (const db::Layout &layout, std::set cell_index ()); + for (db::Layout::const_iterator cell = layout.begin (); cell != layout.end (); ++cell) { + if (cleaned_cells.find (cell->cell_index ()) == cleaned_cells.end ()) { + cells.insert (cell->cell_index ()); + } } } diff --git a/src/db/unit_tests/dbLayoutTests.cc b/src/db/unit_tests/dbLayoutTests.cc index 9b6178ca8..c1dee66d1 100644 --- a/src/db/unit_tests/dbLayoutTests.cc +++ b/src/db/unit_tests/dbLayoutTests.cc @@ -1040,3 +1040,134 @@ TEST(101_CopyTreeDoesNotModifyPolygons) EXPECT_EQ (l2s (l), "begin_lib 0.001\nbegin_cell {TOP}\nboundary 1 0 {0 0} {0 1000} {500 1000} {1500 1000} {1000 1000} {1000 0} {0 0}\nend_cell\nend_lib\n"); } +namespace +{ + +class LIBT_L + : public db::Library +{ +public: + LIBT_L (tl::TestBase *_this) + : Library () + { + set_name("L"); + set_description("A test library."); + + layout ().dbu (0.001); + + db::LayerProperties p; + + p.layer = 23; + p.datatype = 0; + unsigned int l_cont = layout ().insert_layer (p); + + p.layer = 16; + p.datatype = 0; + unsigned int l_gate = layout ().insert_layer (p); + + db::Cell &cell_a = layout ().cell (layout ().add_cell ("A")); + cell_a.shapes(l_cont).insert(db::Box (50, 50, 150, 150)); + cell_a.shapes(l_gate).insert(db::Box (0, 0, 200, 1000)); + + db::Cell &top = layout ().cell (layout ().add_cell ("TOP")); + top.insert (db::CellInstArray (db::CellInst (cell_a.cell_index ()), db::Trans (db::Vector (0, 0)))); + } + + ~LIBT_L() + { + // .. nothing yet .. + } +}; + +} + +static std::string cells2string (const db::Layout &layout, bool top = true) +{ + std::string s; + auto cto = top ? layout.end_top_cells () : layout.end_top_down (); + for (auto c = layout.begin_top_down (); c != cto; ++c) { + if (! s.empty ()) { + s += ","; + } + if (layout.cell (*c).is_proxy ()) { + s += "*"; + } + s += layout.cell_name (*c); + } + return s; +} + +// issue #2350 +TEST(101_CleanupBehavior) +{ + std::unique_ptr lib (new LIBT_L (_this)); + db::LibraryManager::instance ().register_lib (lib.get ()); + + db::Manager m (true); + db::Layout layout (&m); + layout.do_cleanup (true); + layout.dbu (0.001); + + db::Cell &top = layout.cell (layout.add_cell ("TOPTOP")); + + db::cell_index_type lib_top = lib->layout ().cell_by_name ("TOP").second; + db::cell_index_type lp1 = layout.get_lib_proxy (lib.get (), lib_top); + + db::Instance i1 = top.insert (db::CellInstArray (db::CellInst (lp1), db::Trans (db::Vector (0, 0)))); + + db::Layout layout2 = layout; + layout2.do_cleanup (true); + + EXPECT_EQ (cells2string (layout2, false), "TOPTOP,*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "TOPTOP"); + + layout2.cleanup (); + + // cleanup does not change anything as the top cell is not a proxy + EXPECT_EQ (cells2string (layout2, false), "TOPTOP,*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "TOPTOP"); + + top.erase (i1); + + layout2 = layout; + layout2.do_cleanup (true); + + EXPECT_EQ (cells2string (layout2, false), "TOPTOP,*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "TOPTOP,*TOP"); + + layout2.cleanup (); + + // cleanup removes the proxy and subcell + EXPECT_EQ (cells2string (layout2, false), "TOPTOP"); + EXPECT_EQ (cells2string (layout2), "TOPTOP"); + + // now we borrow *A (LIB.A) and place it in TOPTOP + db::cell_index_type ci_a = layout.cell_by_name ("A").second; + EXPECT_EQ (layout.cell (ci_a).is_proxy (), true); + top.insert (db::CellInstArray (db::CellInst (ci_a), db::Trans (db::Vector (0, 0)))); + + layout2 = layout; + layout2.do_cleanup (true); + + EXPECT_EQ (cells2string (layout2, false), "TOPTOP,*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "TOPTOP,*TOP"); + + layout2.cleanup (); + + // cleanup removes the *TOP proxy, but not *A as it is used by TOPTOP + EXPECT_EQ (cells2string (layout2, false), "TOPTOP,*A"); + EXPECT_EQ (cells2string (layout2), "TOPTOP"); + + layout2 = layout; + layout2.delete_cell (top.cell_index ()); // layout and layout2 use the same cell indexes + layout2.do_cleanup (true); + + EXPECT_EQ (cells2string (layout2, false), "*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "*TOP"); + + layout2.cleanup (); + + // cleanup does not remove the proxy as it is the only top cell + EXPECT_EQ (cells2string (layout2, false), "*TOP,*A"); + EXPECT_EQ (cells2string (layout2), "*TOP"); +} diff --git a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc index 712ea2bd8..ce62968e2 100644 --- a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc +++ b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc @@ -582,7 +582,7 @@ GDS2WriterBase::write (db::Layout &layout, tl::OutputStream &stream, const db::S // don't write ghost cells unless they are not empty (any more) // also don't write proxy cells which are not employed - if (! cref.is_real_ghost_cell () && (! cref.is_proxy () || ! cref.is_top ())) { + if (! cref.is_real_ghost_cell ()) { try { bool skip_body = options.write_context_info () && cref.can_skip_replica (); diff --git a/src/plugins/streamers/oasis/db_plugin/dbOASISWriter.cc b/src/plugins/streamers/oasis/db_plugin/dbOASISWriter.cc index 93e0cafd0..bebf4d175 100644 --- a/src/plugins/streamers/oasis/db_plugin/dbOASISWriter.cc +++ b/src/plugins/streamers/oasis/db_plugin/dbOASISWriter.cc @@ -1437,12 +1437,6 @@ OASISWriter::write_layername_table (size_t &layernames_table_pos, const std::vec end_table (layernames_table_pos); } -static bool must_write_cell (const db::Cell &cref) -{ - // Don't write proxy cells which are not employed - return ! cref.is_proxy () || ! cref.is_top (); -} - void OASISWriter::create_cell_nstrings (const db::Layout &layout, const std::set &cell_set) { @@ -1511,13 +1505,13 @@ OASISWriter::write (db::Layout &layout, tl::OutputStream &stream, const db::Save cells_by_index.reserve (cell_set.size ()); for (db::Layout::bottom_up_const_iterator cell = layout.begin_bottom_up (); cell != layout.end_bottom_up (); ++cell) { - if (cell_set.find (*cell) != cell_set.end () && must_write_cell (layout.cell (*cell))) { + if (cell_set.find (*cell) != cell_set.end ()) { cells.push_back (*cell); } } for (db::Layout::const_iterator cell = layout.begin (); cell != layout.end (); ++cell) { - if (cell_set.find (cell->cell_index ()) != cell_set.end () && must_write_cell (layout.cell (cell->cell_index ()))) { + if (cell_set.find (cell->cell_index ()) != cell_set.end ()) { cells_by_index.push_back (cell->cell_index ()); } } diff --git a/testdata/lstream/pcells_au.oas b/testdata/lstream/pcells_au.oas index 88943799d812b233eab24fa7b84a1005ad322b89..3554c35e72dd92551d6ce77c6d74660080bcfe96 100644 GIT binary patch delta 88 zcmaFIw1jzr9IK2t3qQj|xd3}kHfPTuXCGHy22pKB1{T)E@-3W;CNM4pQVZoa{~hd09sxKEdT%j