Merge 7c540b9b5f into 0a230da4df

This prevents the thundering herd problem and should increase the
scalability of the DispatchQueue significantly.

Additionally the code the DispatchQueue was taken from made this
improvement five years ago:
79ad8a539d

Signed-off-by: Drew Lewis <cannada@google.com>

doc/CodingGuidelines.txt

@@ -4,7 +4,7 @@ Naming conventions
 directory - lowercase (directory)
 filename - corresponding class name without prefix (Filename)
 class - upper camel case (ClassName)
-member function - upper camel case (memberFunction)
+member function - lower camel case (memberFunction)
 member variable - snake case with trailing underscore (member_variable_)
                   Trailing underscore prevents conflict with accessor 
                   member function name.
@@ -105,6 +105,9 @@ private order.
  friend class Frobulator;
  }
 
+Class member functions should not be defined inside the class unless they
+are simple accessors that return a member variable.
+
 Avoid using [] to lookup a map value because it creates a key/null value
 pair if the lookup fails.  Use map::find or sta::Map::findKey instead.
 
@@ -114,6 +117,11 @@ Avoid all use of global variables as "caches", even if they are thread local.
 OpenSTA goes to great lengths to minimize global state variable that prevent
 multiple instances of the Sta class from coexisting.
 
+Do not use thread_local variables. They are essentially global
+variables so they prevent multiple instances of an Sta object from
+existing concurrently, so they sbould also be avoided. Use stack state
+in each thread instead.
+
 Regression Tests
 ................
 
@@ -129,11 +137,17 @@ Tests log files and results are in test/results. The result/test.log
 is compared to test.ok to determine if a test passes.
 
 Test scripts are written in tcl and live in the /test directory.
-Compress large liberty, verilog, and spef, files and use existing
-libraries to prevent repository bloat.
+Compress large liberty, verilog, and spef, files., Use small or
+existing verilog and liberty files to prevent repository bloat.
 
 The test script should use a one line comment at the beginning of the
 file so head -1 can show what it is for. Use file names to roughly
 group regressions and use numeric suffixes to distinguish them.
 
 The script test/save_ok saves a test/results/<test>.log to test/<test>.okfile.
+
+To add a new regression:
+ add <test>.tcl to /tcl
+ add <test> name to test/regression_vars.tcl
+ run <test> with test/regression <test>
+ use save_ok <test> to save the log file to >test>.log

include/sta/Delay.hh

@@ -26,6 +26,7 @@
 
 #include "StaConfig.hh"
 
+// IWYU pragma: begin_exports
 #if (SSTA == 1)
   // Delays are Normal PDFs.
   #include "DelayNormal1.hh"
@@ -36,6 +37,7 @@
   // Delays are floats.
   #include "DelayFloat.hh"
 #endif
+// IWYU pragma: end_exports
 
 namespace sta {
 

search/ClkInfo.cc

@@ -166,8 +166,10 @@ ClkInfo::to_string(const StaState *sta) const
     const Pin *crpr_clk_pin = crpr_clk_path_.vertex(sta)->pin();
     result += " crpr ";
     result += network->pathName(crpr_clk_pin);
-    result += "/";
+    result += " ";
     result += std::to_string(crpr_clk_path_.tag(sta)->index());
+    result += "/";
+    result += crpr_clk_path_.minMax(sta)->to_string();
   }
 
   if (is_gen_clk_src_path_)

search/Crpr.cc

@@ -221,18 +221,21 @@ CheckCrpr::findCrpr(const Path *src_clk_path,
     int level_diff = src_level - tgt_level;
     if (level_diff >= 0) {
       src_clk_path2 = src_clk_path2->prevPath();
-      if (src_clk_path2 == nullptr)
+      if (src_clk_path2 == nullptr
+	  || src_clk_path2->isNull())
         break;
       src_level = src_clk_path2->vertex(this)->level();
     }
     if (level_diff <= 0) {
       tgt_clk_path2 = tgt_clk_path2->prevPath();
-      if (tgt_clk_path2 == nullptr)
+      if (tgt_clk_path2 == nullptr
+	  || tgt_clk_path2->isNull())
         break;
       tgt_level = tgt_clk_path2->vertex(this)->level();
     }
   }
-  if (src_clk_path2 && tgt_clk_path2
+  if (src_clk_path2 && !src_clk_path2->isNull()
+      && tgt_clk_path2 && !tgt_clk_path2->isNull()
       && (src_clk_path2->transition(this) == tgt_clk_path2->transition(this)
 	  || same_pin)) {
     debugPrint(debug_, "crpr", 2, "crpr pin %s",

search/Latches.cc

@@ -75,6 +75,8 @@ Latches::latchRequired(const Path *data_path,
       time_given_to_startpoint = 0.0;
   }
   else if (enable_path && disable_path) {
+    debugPrint(debug_, "latch", 1, "latch %s",
+               sdc_network_->pathName(data_path->pin(this)));
     Delay open_latency, latency_diff, max_borrow;
     float nom_pulse_width, open_uncertainty;
     Crpr open_crpr, crpr_diff;
@@ -102,8 +104,7 @@ Latches::latchRequired(const Path *data_path,
       + PathEnd::checkSetupMcpAdjustment(data_clk_edge, enable_clk_edge, mcp,
 					 1, sdc_)
       + open_crpr;
-    debugPrint(debug_, "latch", 1, "latch data %s %s enable %s",
-               network_->pathName(data_path->pin(this)),
+    debugPrint(debug_, "latch", 1, "data %s enable %s",
                delayAsString(data_arrival, this),
                delayAsString(enable_arrival, this));
     if (delayLessEqual(data_arrival, enable_arrival, this)) {
@@ -145,6 +146,11 @@ Latches::latchRequired(const Path *data_path,
     adjusted_data_arrival = data_arrival;
     time_given_to_startpoint = 0.0;
   }
+  debugPrint(debug_, "latch", 2, "req %s borrow %s time_given %s adj_arrival %s",
+	     delayAsString(required, this),
+	     delayAsString(borrow, this),
+	     delayAsString(time_given_to_startpoint, this),
+	     delayAsString(adjusted_data_arrival, this));
 }
 
 void
@@ -209,6 +215,16 @@ Latches::latchBorrowInfo(const Path *data_path,
     open_crpr = 0.0;
     crpr_diff = 0.0;
   }
+  debugPrint(debug_, "latch", 2, "nom_width %s open_lat %s lat_diff %s open_uncert %s",
+	     delayAsString(nom_pulse_width, this),
+	     delayAsString(open_latency, this),
+	     delayAsString(latency_diff, this),
+	     delayAsString(open_uncertainty, this));
+  debugPrint(debug_, "latch", 2, "open_crpr %s crpr_diff %s open_uncert %s max_borrow %s",
+	     delayAsString(open_crpr, this),
+	     delayAsString(crpr_diff, this),
+	     delayAsString(open_uncertainty, this),
+	     borrow_limit_exists ? delayAsString(max_borrow, this) : "none");
 }
 
 void

search/Search.cc

@@ -1382,7 +1382,11 @@ ArrivalVisitor::pruneCrprArrivals()
                    delayAsString(max_crpr, this),
                    delayAsString(max_arrival_max_crpr, this));
         Arrival arrival = tag_bldr_->arrival(path_index);
-	if (delayGreater(max_arrival_max_crpr, arrival, min_max, this)) {
+	// Latch D->Q path uses enable min so crpr clk path min/max
+	// does not match the path min/max.
+	if (delayGreater(max_arrival_max_crpr, arrival, min_max, this)
+	    && clk_info_no_crpr->crprClkPath(this)->minMax(this)
+	    == clk_info->crprClkPath(this)->minMax(this)) {
 	  debugPrint(debug_, "search", 3, "  pruned %s",
                      tag->to_string(this).c_str());
           path_itr = path_index_map.erase(path_itr);

search/Search.tcl

@@ -421,7 +421,7 @@ define_cmd_args "report_checks" \
      [-sort_by_slack]\
      [-path_group group_name]\
      [-format full|full_clock|full_clock_expanded|short|end|slack_only|summary|json]\
-     [-fields capacitance|slew|input_pin|net|src_attr]\
+     [-fields capacitance|slew|fanout|input_pin|net|src_attr]\
      [-digits digits]\
      [-no_line_splits]\
      [> filename] [>> filename]}

search/Tag.cc

@@ -138,11 +138,15 @@ Tag::to_string(bool report_index,
     result += network->pathName(clk_src);
   }
 
+  result += " crpr_pin ";
   const Path *crpr_clk_path = clk_info_->crprClkPath(sta);
-  if (crpr_clk_path != nullptr) {
-    result += " crpr_pin ";
+  if (crpr_clk_path) {
     result += network->pathName(crpr_clk_path->pin(sta));
+    result += " ";
+    result += crpr_clk_path->minMax(sta)->to_string();
   }
+  else
+    result += "null";
 
   if (input_delay_) {
     result += " input ";

test/package_require.tcl

@@ -0,0 +1,3 @@
+package require http
+package require msgcat
+package require opt

test/regression_vars.tcl

@@ -149,14 +149,16 @@ record_sta_tests {
   liberty_ccsn
   liberty_float_as_str
   liberty_latch3
+  package_require
   path_group_names
   prima3
+  report_checks_sorted
   report_checks_src_attr
   report_json1
   report_json2
   suppress_msg
   verilog_attribute
-  report_checks_sorted
+  verilog_specify
 }
 
 define_test_group fast [group_tests all]

test/verilog_specify.tcl

@@ -0,0 +1,2 @@
+# try to load verilog language file
+read_verilog verilog_specify.v

test/verilog_specify.v

@@ -0,0 +1,20 @@
+
+module counter(clk, reset, in, out);
+  input clk;
+  output out;
+  input reset;
+  input in;
+  wire mid;
+
+  parameter PARAM1=1;
+  parameter PARAM2="test";
+
+  specify
+    specparam SPARAM1=2;
+    specparam SPARAM2="test2";
+  endspecify
+
+  defparam _1415_.PARAM2 = 1;
+
+
+endmodule

util/DispatchQueue.cc

@@ -42,11 +42,12 @@ void
 DispatchQueue::setThreadCount(size_t thread_count)
 {
   terminateThreads();
-
+  std::unique_lock<std::mutex> lock(lock_);
   threads_.resize(thread_count);
   for(size_t i = 0; i < thread_count; i++) {
     threads_[i] = std::thread(&DispatchQueue::dispatch_thread_handler, this, i);
   }
+  quit_ = false;
 }
 
 void
@@ -66,7 +67,7 @@ DispatchQueue::dispatch(const fp_t& op)
   // Manual unlocking is done before notifying, to avoid waking up
   // the waiting thread only to block again (see notify_one for details)
   lock.unlock();
-  cv_.notify_all();
+  cv_.notify_one();
 }
 
 void
@@ -79,7 +80,7 @@ DispatchQueue::dispatch(fp_t&& op)
   // Manual unlocking is done before notifying, to avoid waking up
   // the waiting thread only to block again (see notify_one for details)
   lock.unlock();
-  cv_.notify_all();
+  cv_.notify_one();
 }
 
 void

util/StaConfig.hh.cmake

@@ -1,3 +1,5 @@
+#pragma once
+
 #define STA_VERSION "${STA_VERSION}"
 
 #define STA_GIT_SHA1 "${STA_GIT_SHA1}"

verilog/VerilogLex.ll

@@ -132,6 +132,9 @@ output { return token::OUTPUT; }
 parameter { return token::PARAMETER; }
 defparam { return token::DEFPARAM; }
 reg { return token::REG; }
+specify { return token::SPECIFY; }
+endspecify { return token::ENDSPECIFY; }
+specparam { return token::SPECPARAM; }
 supply0 { return token::SUPPLY0; }
 supply1 { return token::SUPPLY1; }
 tri { return token::TRI; }

verilog/VerilogParse.yy

@@ -83,6 +83,7 @@ sta::VerilogParse::error(const location_type &loc,
 }
 
 %token INT CONSTANT ID STRING MODULE ENDMODULE ASSIGN PARAMETER DEFPARAM
+%token SPECIFY ENDSPECIFY SPECPARAM
 %token WIRE WAND WOR TRI INPUT OUTPUT INOUT SUPPLY1 SUPPLY0 REG
 %token ATTR_OPEN ATTR_CLOSED
 
@@ -99,6 +100,8 @@ sta::VerilogParse::error(const location_type &loc,
 %type <stmt> stmt declaration instance parameter parameter_dcls parameter_dcl
 %type <stmt> defparam param_values param_value port_dcl
 %type <stmt_seq> stmts stmt_seq net_assignments continuous_assign port_dcls
+%type <stmt> specify_block
+%type <stmt_seq> specify_stmts
 %type <assign> net_assignment
 %type <dcl_arg> dcl_arg
 %type <dcl_arg_seq> dcl_args
@@ -232,6 +235,7 @@ stmt:
 |	defparam
 |	declaration
 |	instance
+|	specify_block
 |	error ';'
 	{ yyerrok; $$ = nullptr; }
 	;
@@ -240,6 +244,25 @@ stmt_seq:
 	continuous_assign
 	;
 
+/* specify blocks are used by some comercial tools to convey macro timing
+ * and other metadata.
+ * Their presence is not forbidden in structural verilog, this is a placeholder
+ * that just ignores them and allows verilog processing to proceed
+ * <<TODO>> if someone in the future wants implement support for timing info
+ * via specify blocks, implement proper parsing here
+ */
+specify_block:
+	SPECIFY specify_stmts ENDSPECIFY
+	{ $$ = nullptr; }
+	;
+
+specify_stmts:
+	SPECPARAM parameter_dcl ';'
+	{ $$ = nullptr; }
+|   specify_stmts SPECPARAM parameter_dcl ';'
+	{ $$ = nullptr; }
+	;
+
 /* Parameters are parsed and ignored. */
 parameter:
 	PARAMETER parameter_dcls ';'