Refining solution

- Properly handling copy constructor and initialization
- Simplify code
- Added test

src/plugins/streamers/lefdef/db_plugin/dbLEFDEFImporter.cc

@@ -597,6 +597,7 @@ LEFDEFReaderOptions::LEFDEFReaderOptions ()
     m_special_routing_datatype (0),
     m_separate_groups (false),
     m_joined_paths (false),
+    m_skip_duplicate_macros (false),
     m_macro_resolution_mode (0),
     m_read_lef_with_def (true),
     m_paths_relative_to_cwd (false),
@@ -682,6 +683,7 @@ LEFDEFReaderOptions &LEFDEFReaderOptions::operator= (const LEFDEFReaderOptions &
     m_macro_layout_files = d.m_macro_layout_files;
     m_read_lef_with_def = d.m_read_lef_with_def;
     m_paths_relative_to_cwd = d.m_paths_relative_to_cwd;
+    m_skip_duplicate_macros = d.m_skip_duplicate_macros;
     set_macro_layouts (d.macro_layouts ());
   }
   return *this;
@@ -1069,7 +1071,10 @@ void
 LEFDEFReaderState::ensure_lef_importer (int warn_level)
 {
   if (! mp_lef_importer.get ()) {
-    mp_lef_importer.reset (new db::LEFImporter (warn_level, true));
+    mp_lef_importer.reset (new db::LEFImporter (warn_level));
+    if (mp_tech_comp) {
+      mp_lef_importer->set_skip_duplicate_macros (mp_tech_comp->skip_duplicate_macros ());
+    }
   }
 }
 

src/plugins/streamers/lefdef/db_plugin/dbLEFDEFPlugin.cc

@@ -142,7 +142,7 @@ LEFDEFReader::read_lefdef (db::Layout &layout, const db::LoadLayoutOptions &opti
 
     tl::SelfTimer timer (tl::verbosity () >= 21, tl::to_string (tr ("Reading LEF file")));
 
-    db::LEFImporter importer (warn_level (), effective_options.skip_duplicate_macros ());
+    db::LEFImporter importer (warn_level ());
 
     for (std::vector<std::string>::const_iterator l = effective_options.begin_lef_files (); l != effective_options.end_lef_files (); ++l) {
 

src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.cc

@@ -33,9 +33,9 @@ namespace db
 // -----------------------------------------------------------------------------------
 //  LEFImporter implementation
 
-LEFImporter::LEFImporter (int warn_level, bool skip_duplicate_macros)
+LEFImporter::LEFImporter (int warn_level)
   : LEFDEFImporter (warn_level),
-    m_skip_duplicate_macros (skip_duplicate_macros)
+    m_skip_duplicate_macros (false)
 {
   //  .. nothing yet ..
 }
@@ -885,67 +885,21 @@ void
 LEFImporter::read_macro (Layout &layout)
 {
   std::string mn = get ();
+  set_cellname (mn);
+
+  GeometryBasedLayoutGenerator *mg = 0;
 
   if (m_macros.find (mn) != m_macros.end ()) {
     if (m_skip_duplicate_macros) {
-      // 1. Change error to warning
+      warn (tl::to_string (tr ("Skipping duplicate MACRO: : ")) + mn + tl::to_string (tr (" (skip option is enabled by user)")));
-      warn (tl::to_string (tr ("Duplicate MACRO name: ")) + mn + tl::to_string (tr (" (Skipping duplicate)")));
-
-      // Safe LEF block skipping
-      while (! at_end ()) {
-        
-        if (test ("END")) {
-          expect (mn);
-          break;
-        } else if (test ("PIN")) {
-            std::string pn = get (); // PIN always has a pin-name -> skip it
-            //tl::info << tl::to_string (tr ("Found PIN : ")) + pn;
-            while (! at_end ()) {
-              if (test ("PORT")) {
-                //tl::info << tl::to_string (tr ("Found PORT within ")) + pn;
-                while (! at_end ()) {
-                  if (test ("END")) {
-                    //tl::info << tl::to_string (tr ("Found PORT END within ")) + pn;
-                    break;
-                  } else {
-                    skip_entry ();
-                  }
-                }
-              } else if (test ("END")) {
-                std::string pn = get (); // PIN always has a pin-name -> skip it
-                //tl::info << tl::to_string (tr ("Found PIN END : ")) + pn;
-                break;
-              } else {
-                skip_entry ();
-              }
-            }
-        } else if (test ("OBS")) {
-          //tl::info << tl::to_string (tr ("Found OBS"));
-          while (! at_end ()) {
-            if (test ("END")) {
-              //tl::info << tl::to_string (tr ("Found END OBS"));
-              break;
-            } else {
-              skip_entry ();
-            }
-          }
-        } else {
-          skip_entry ();
-        }
-      }
-      tl::info << tl::to_string (tr ("Successfully skipped duplicate MACRO: ")) + mn;
-
-      return; // Exit early so we don't register or process this duplicate
     } else {
       error (tl::to_string (tr ("Duplicate MACRO name: ")) + mn);
     }
+  } else {
+    mg = new GeometryBasedLayoutGenerator ();
+    reader_state ()->register_macro_cell (mn, mg);
   }
 
-  set_cellname (mn);
-
-  GeometryBasedLayoutGenerator *mg = new GeometryBasedLayoutGenerator ();
-  reader_state ()->register_macro_cell (mn, mg);
-
   db::Trans foreign_trans;
   std::string foreign_name;
 
@@ -1017,7 +971,7 @@ LEFImporter::read_macro (Layout &layout)
             read_geometries (mg, layout.dbu (), LEFPins, &boxes_for_labels, prop_id);
 
             for (std::map <std::string, db::Box>::const_iterator b = boxes_for_labels.begin (); b != boxes_for_labels.end (); ++b) {
-              if (! b->second.empty ()) {
+              if (mg && ! b->second.empty ()) {
                 mg->add_text (b->first, LEFLabel, db::Text (label.c_str (), db::Trans (b->second.center () - db::Point ())), 0, 0);
               }
             }
@@ -1104,7 +1058,9 @@ LEFImporter::read_macro (Layout &layout)
 
     } else if (test ("FIXEDMASK")) {
 
-      mg->set_fixedmask (true);
+      if (mg) {
+        mg->set_fixedmask (true);
+      }
       expect (";");
 
     } else {
@@ -1127,8 +1083,10 @@ LEFImporter::read_macro (Layout &layout)
 
   }
 
-  mg->add_box (std::string (), Outline, db::Box (-origin, -origin + size), 0, 0);
+  if (mg) {
-  mg->subtract_overlap_from_outline (m_overlap_layers);
+    mg->add_box (std::string (), Outline, db::Box (-origin, -origin + size), 0, 0);
+    mg->subtract_overlap_from_outline (m_overlap_layers);
+  }
 
   MacroDesc macro_desc;
   macro_desc.foreign_name = foreign_name;

src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.h

@@ -50,7 +50,7 @@ public:
   /**
    *  @brief Default constructor
    */
-  LEFImporter (int warn_level, bool skip_duplicate_macros);
+  LEFImporter (int warn_level);
 
   /**
    *  @brief Destructor
@@ -147,6 +147,31 @@ public:
    */
   void finish_lef (db::Layout &layout);
 
+  /**
+   *  @brief Sets a flag indicating whether to skip duplicate macros
+   *
+   *  With this flag set to true, macros are ignored when there already
+   *  is a macro with the same name. Use this option with caution, as
+   *  skipping is only permitted if both macros are identical. The
+   *  LEF reader does not check this condition.
+   *
+   *  The default is "false" (raise an error on duplicate macros).
+   */
+  void set_skip_duplicate_macros (bool f)
+  {
+    m_skip_duplicate_macros = f;
+  }
+
+  /**
+   *  @brief Gets a flag indicating whether to skip duplicate macros
+   *
+   *  See set_skip_duplicate_macros for an explanation of this flag.
+   */
+  bool skip_duplicate_macros () const
+  {
+    return m_skip_duplicate_macros;
+  }
+
 protected:
   void do_read (db::Layout &layout);
 
@@ -159,6 +184,7 @@ private:
   std::map<std::string, ViaDesc> m_vias;
   std::set<std::string> m_routing_layers, m_cut_layers, m_overlap_layers;
   std::map<std::string, unsigned int> m_num_masks;
+  bool m_skip_duplicate_macros;
 
   std::vector <db::Trans> get_iteration (double dbu);
   void read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, LayerPurpose purpose, std::map<std::string, db::Box> *collect_bboxes = 0, properties_id_type prop_id = 0);
@@ -169,8 +195,6 @@ private:
   void read_layer (Layout &layout);
   void read_macro (Layout &layout);
   void skip_entry ();
-
-  bool m_skip_duplicate_macros;
 };
 
 }

src/plugins/streamers/lefdef/unit_tests/dbLEFDEFImportTests.cc

@@ -1199,3 +1199,54 @@ TEST(216_line_extensions)
   run_test (_this, "issue-2075", "map:test.map+lef:test.lef+def:test.def", "au.oas", default_options (), false);
 }
 
+//  issue-2374 (skip duplicate LEF macro option)
+TEST(217_skip_duplicate_macro)
+{
+  std::string fn_path (tl::testdata ());
+  fn_path += "/lefdef/issue-2374/";
+
+  db::LEFDEFReaderOptions lefdef_opt = default_options ();
+  lefdef_opt.set_single_map_file ("tech.map");
+  std::vector<std::string> lf;
+  lf.push_back ("tech.lef");
+  lf.push_back ("lib1.lef");  //  macros a,b
+  lf.push_back ("lib2.lef");  //  macros b,c
+  lefdef_opt.set_lef_files (lf);
+  lefdef_opt.set_read_lef_with_def (false);
+
+  db::Layout ly;
+
+  try {
+
+    db::LoadLayoutOptions opt;
+    opt.set_options (lefdef_opt);
+    EXPECT_EQ (lefdef_opt.skip_duplicate_macros (), false);
+
+    tl::InputStream is (fn_path + "top.def");
+    db::Reader reader (is);
+    reader.read (ly, opt);
+
+    //  must throw an exception because of duplicate macros
+    EXPECT_EQ (1, 0);
+
+  } catch (...) { }
+
+  ly = db::Layout ();
+
+  {
+
+    db::LEFDEFReaderOptions lo = lefdef_opt;
+    lo.set_skip_duplicate_macros (true);
+    EXPECT_EQ (lo.skip_duplicate_macros (), true);
+    db::LoadLayoutOptions opt;
+    opt.set_options (lo);
+
+    tl::InputStream is (fn_path + "top.def");
+    db::Reader reader (is);
+    reader.read (ly, opt);
+
+  }
+
+  db::compare_layouts (_this, ly, fn_path + "au.oas", db::WriteOAS);
+}
+

testdata/lefdef/issue-2374/lib1.lef

@@ -0,0 +1,20 @@
+VERSION 5.8 ;
+
+MACRO a
+  ORIGIN 0 0 ;
+  SIZE 600 BY 600 ;
+  OBS 
+    LAYER M1 ;
+      RECT 10 10 590 590 ;
+  END
+END a
+
+MACRO b
+  ORIGIN -600 0 ;
+  SIZE 400 BY 500 ;
+  OBS 
+    LAYER M1 ;
+      RECT 610 10 990 490 ;
+  END
+END b
+

testdata/lefdef/issue-2374/lib2.lef

@@ -0,0 +1,22 @@
+VERSION 5.8 ;
+
+MACRO b
+  ORIGIN -600 0 ;
+  SIZE 400 BY 500 ;
+  OBS 
+    LAYER M1 ;
+      RECT 610 10 990 590 ;
+  END
+END b
+
+MACRO c
+  ORIGIN -500 -500 ;
+  SIZE 500 BY 500 ;
+  OBS 
+    LAYER M1 ;
+      POLYGON 510 610 610 610 610 510 990 510 990 990 510 990 ;
+    LAYER overlap ;
+      RECT 500 700 1000 1000 ;
+      POLYGON 500 600  600 600  600 500  1000 500  1000 700  500 700 ;
+  END
+END c

testdata/lefdef/issue-2374/tech.lef

@@ -0,0 +1,12 @@
+VERSION 5.8 ;
+
+LAYER M1
+  TYPE ROUTING ;
+  DIRECTION HORIZONTAL ;
+  WIDTH 0.1 ;
+  PITCH 0.1 ;
+END M1
+
+LAYER overlap
+  TYPE OVERLAP ;
+END overlap

testdata/lefdef/issue-2374/tech.map

@@ -0,0 +1,6 @@
+DIEAREA ALL 1 0
+#BOUNDARY DIEAREA 1 0
+BOUNDARY MACRO 1 0
+#M1 LEFOBS 2 0
+M1 LEFOBS 3 0
+M1 BLOCKAGE 3 0

testdata/lefdef/issue-2374/top.def

@@ -0,0 +1,10 @@
+VERSION 5.8 ;
+DESIGN top ;
+UNITS DISTANCE MICRONS 1000 ;
+DIEAREA ( 0 0 ) ( 1000000 1000000 ) ;
+COMPONENTS 3 ;
+- a a + PLACED ( 0 0 ) N ;
+- b b + PLACED ( 600000 0 ) N ;
+- c c + PLACED ( 500000 500000 ) N ;
+END COMPONENTS
+END DESIGN