Bugfix/issue 2350

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<db::cell_index_type> 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 <const db::ColdProxy *> (cptr) && keep.empty ()) {
-          keep.insert (*c);
-        }
-      } else {
-        keep.clear ();
-        break;
-      }
-    }
-  }
-
-  layout.cleanup (keep);
+  layout.cleanup ();
 
   return layer_map_out ();
 }

src/db/db/dbDeepShapeStore.cc

@@ -1389,7 +1389,6 @@ namespace
   {
   public:
     DeepShapeStoreToShapeTransformer (const DeepShapeStore &dss, const db::Layout &layout)
-      : mp_layout (& layout)
     {
       //  gets the text annotation property ID -
       //  this is how the texts are passed for annotating the net names
@@ -1445,7 +1444,6 @@ namespace
 
   private:
     std::pair<bool, db::property_names_id_type> m_text_annot_name_id;
-    const db::Layout *mp_layout;
   };
 }
 

src/db/db/dbEdgePairFilters.h

@@ -119,7 +119,6 @@ class DB_PUBLIC InternalAngleEdgePairFilter
   virtual bool wants_variants () const { return false; }
 
 private:
-  bool m_inverted;
   db::EdgeAngleChecker m_checker;
 };
 

src/db/db/dbEdgesUtils.h

@@ -244,7 +244,6 @@ struct DB_PUBLIC EdgeOrientationFilter
   }
 
 private:
-  bool m_inverse;
   db::MagnificationAndOrientationReducer m_vars;
   EdgeAngleChecker m_checker;
 };

src/db/db/dbLayout.cc

@@ -1723,6 +1723,84 @@ Layout::refresh ()
   }
 }
 
+std::set<db::cell_index_type>
+Layout::cells_to_cleanup (const std::set<db::cell_index_type> &keep_always) const
+{
+  std::set<db::cell_index_type> 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<db::cell_index_type> 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 <const db::ColdProxy *> (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 <db::cell_index_type> 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 <db::cell_index_type> 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<db::cell_index_type> &keep)
 {
@@ -1801,28 +1879,17 @@ Layout::cleanup (const std::set<db::cell_index_type> &keep)
 
   }
 
-  std::set<cell_index_type> 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);
-      }
-    }
-
-    if (cells_to_delete.empty ()) {
-      break;
+  {
+    std::set<cell_index_type> cells_to_delete = cells_to_cleanup (keep);
+    if (! cells_to_delete.empty ()) {
+      delete_cells (cells_to_delete);
     }
-
-    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) {
@@ -2965,12 +3032,7 @@ Layout::has_context_info (cell_index_type cell_index) const
     }
   }
 
-  const db::Cell &cref = cell (cell_index);
-  if (cref.is_proxy () && ! cref.is_top ()) {
-    return true;
-  } else {
-    return false;
-  }
+  return cell (cell_index).is_proxy ();
 }
 
 bool

src/db/db/dbLayout.h

@@ -1682,6 +1682,19 @@ class DB_PUBLIC Layout
    */
   void cleanup (const std::set<db::cell_index_type> &keep = std::set<db::cell_index_type> ());
 
+  /**
+   *  @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<db::cell_index_type> cells_to_cleanup (const std::set<db::cell_index_type> &keep = std::set<db::cell_index_type> ()) const;
+
   /**
    *  @brief Calls "update" on all cells of the layout
    *

src/db/db/dbSaveLayoutOptions.cc

@@ -372,12 +372,19 @@ SaveLayoutOptions::get_cells (const db::Layout &layout, std::set <db::cell_index
 
   if (m_all_cells) {
 
+    //  issue #2350: don't write cells that are to be cleanup, but in a sensitive manner
+    //  - this does a kind of virtual cleanup, but with the same rules as applied
+    //  during the normal cleanup.
+    std::set<db::cell_index_type> 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,16 +393,18 @@ SaveLayoutOptions::get_cells (const db::Layout &layout, std::set <db::cell_index
     if (has_skipped_replica) {
 
       for (db::Layout::const_iterator cell = layout.begin (); cell != layout.end (); ++cell) {
-        if (cell->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);
         }
       }
 
     } else {
 
-      for (db::Layout::const_iterator cell = layout.begin (); cell != layout.end () && ! has_skipped_replica; ++cell) {
-        cells.insert (cell->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 ());
+        }
       }
 
     }

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<LIBT_L> 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");
+}