diff --git a/doc/CodingGuidelines.txt b/doc/CodingGuidelines.txt index 1053ffd2..4e2e096e 100644 --- a/doc/CodingGuidelines.txt +++ b/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/.log to test/.okfile. + +To add a new regression: + add .tcl to /tcl + add name to test/regression_vars.tcl + run with test/regression + use save_ok to save the log file to >test>.log diff --git a/include/sta/Delay.hh b/include/sta/Delay.hh index 32491141..236158c9 100644 --- a/include/sta/Delay.hh +++ b/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 { diff --git a/search/ClkInfo.cc b/search/ClkInfo.cc index 3e343b26..76e21287 100644 --- a/search/ClkInfo.cc +++ b/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_) diff --git a/search/Crpr.cc b/search/Crpr.cc index 46b5917f..5420f720 100644 --- a/search/Crpr.cc +++ b/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", diff --git a/search/Latches.cc b/search/Latches.cc index f9328082..7d746e25 100644 --- a/search/Latches.cc +++ b/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 diff --git a/search/Search.cc b/search/Search.cc index 21d8e6d7..812c98d9 100644 --- a/search/Search.cc +++ b/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); diff --git a/search/Search.tcl b/search/Search.tcl index e11ac07e..10480194 100644 --- a/search/Search.tcl +++ b/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]} diff --git a/search/Tag.cc b/search/Tag.cc index 0ebfde38..32a29420 100644 --- a/search/Tag.cc +++ b/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 "; diff --git a/test/package_require.ok b/test/package_require.ok new file mode 100644 index 00000000..e69de29b diff --git a/test/package_require.tcl b/test/package_require.tcl new file mode 100644 index 00000000..6b783bac --- /dev/null +++ b/test/package_require.tcl @@ -0,0 +1,3 @@ +package require http +package require msgcat +package require opt diff --git a/test/regression_vars.tcl b/test/regression_vars.tcl index 02bdadcd..d7a46617 100644 --- a/test/regression_vars.tcl +++ b/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] diff --git a/test/verilog_specify.ok b/test/verilog_specify.ok new file mode 100644 index 00000000..e69de29b diff --git a/test/verilog_specify.tcl b/test/verilog_specify.tcl new file mode 100644 index 00000000..f7a8cb21 --- /dev/null +++ b/test/verilog_specify.tcl @@ -0,0 +1,2 @@ +# try to load verilog language file +read_verilog verilog_specify.v \ No newline at end of file diff --git a/test/verilog_specify.v b/test/verilog_specify.v new file mode 100644 index 00000000..3e8f9ce6 --- /dev/null +++ b/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 diff --git a/util/DispatchQueue.cc b/util/DispatchQueue.cc index e8a31e5f..c5f85e62 100644 --- a/util/DispatchQueue.cc +++ b/util/DispatchQueue.cc @@ -66,7 +66,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 +79,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 diff --git a/util/StaConfig.hh.cmake b/util/StaConfig.hh.cmake index 572588af..78d2e304 100644 --- a/util/StaConfig.hh.cmake +++ b/util/StaConfig.hh.cmake @@ -1,3 +1,5 @@ +#pragma once + #define STA_VERSION "${STA_VERSION}" #define STA_GIT_SHA1 "${STA_GIT_SHA1}" diff --git a/verilog/VerilogLex.ll b/verilog/VerilogLex.ll index af6a01be..e55801b6 100644 --- a/verilog/VerilogLex.ll +++ b/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; } diff --git a/verilog/VerilogParse.yy b/verilog/VerilogParse.yy index c25de12a..530e9cf2 100644 --- a/verilog/VerilogParse.yy +++ b/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 declaration instance parameter parameter_dcls parameter_dcl %type defparam param_values param_value port_dcl %type stmts stmt_seq net_assignments continuous_assign port_dcls +%type specify_block +%type specify_stmts %type net_assignment %type dcl_arg %type 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 + * <> 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 ';'