From d565906c2b050a58d35895a743c47316880d752f Mon Sep 17 00:00:00 2001 From: James Cherry Date: Mon, 29 Sep 2025 09:48:57 -0700 Subject: [PATCH 01/14] tag/glk_info debug Signed-off-by: James Cherry --- search/ClkInfo.cc | 4 +++- search/Latches.cc | 20 ++++++++++++++++++-- search/Tag.cc | 8 ++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) 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/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/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 "; From 8236a89ef6b2473f44a2c61bb25c7904f976b54f Mon Sep 17 00:00:00 2001 From: James Cherry Date: Mon, 29 Sep 2025 15:47:20 -0700 Subject: [PATCH 02/14] latch D->Q crpr path pruniing (eagle 20250923) Signed-off-by: James Cherry --- search/Search.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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); From 37e1d1543384fd8755e8cf51a186edbfada5e9f8 Mon Sep 17 00:00:00 2001 From: Matt Liberty Date: Tue, 30 Sep 2025 01:23:59 +0000 Subject: [PATCH 03/14] fix stray character typo (#302) Signed-off-by: Matt Liberty --- power/Power.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/power/Power.tcl b/power/Power.tcl index d38d4cbb..420a97a9 100644 --- a/power/Power.tcl +++ b/power/Power.tcl @@ -273,7 +273,7 @@ proc set_power_activity { args } { if { [info exists keys(-duty)] } { set duty $keys(-duty) check_float "duty" $duty - if { $duty < 0.0 || $duty > 1.0 } {i + if { $duty < 0.0 || $duty > 1.0 } { sta_error 309 "duty should be 0.0 to 1.0" } } From b4565890071400397269d72c58a1ac8d42a835c3 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Mon, 29 Sep 2025 19:45:04 -0700 Subject: [PATCH 04/14] PropActivityVisitor::visit null port check PR 301 Signed-off-by: James Cherry --- power/Power.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/power/Power.cc b/power/Power.cc index 5c1ebbc0..7dd2ada0 100644 --- a/power/Power.cc +++ b/power/Power.cc @@ -490,10 +490,11 @@ PropActivityVisitor::visit(Vertex *vertex) } if (network_->isDriver(pin)) { LibertyPort *port = network_->libertyPort(pin); - LibertyCell *test_cell = port->libertyCell()->testCell(); - if (test_cell) - port = test_cell->findLibertyPort(port->name()); if (port) { + LibertyCell *test_cell = port->libertyCell()->testCell(); + if (test_cell) + port = test_cell->findLibertyPort(port->name()); + FuncExpr *func = port->function(); if (func) { PwrActivity activity = power_->evalActivity(func, inst); From e7bffbfef5def47f333f365774d62c75d5a61503 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Mon, 29 Sep 2025 21:02:17 -0700 Subject: [PATCH 05/14] PropActivityVisitor::visit null port check PR 301 Signed-off-by: James Cherry --- power/Power.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/power/Power.cc b/power/Power.cc index 7dd2ada0..dbf55a27 100644 --- a/power/Power.cc +++ b/power/Power.cc @@ -494,7 +494,8 @@ PropActivityVisitor::visit(Vertex *vertex) LibertyCell *test_cell = port->libertyCell()->testCell(); if (test_cell) port = test_cell->findLibertyPort(port->name()); - + } + if (port) { FuncExpr *func = port->function(); if (func) { PwrActivity activity = power_->evalActivity(func, inst); From 76324bbabb39fff0fa86a6e81c1129676b34d947 Mon Sep 17 00:00:00 2001 From: Drew Lewis Date: Tue, 30 Sep 2025 17:08:05 -0400 Subject: [PATCH 06/14] Fix typo in CodingGuidelines.txt (#307) Signed-off-by: Drew Lewis --- doc/CodingGuidelines.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/CodingGuidelines.txt b/doc/CodingGuidelines.txt index 1053ffd2..5e2d1e48 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. From b553e636a019d3806625eedc09b1d543c9a15848 Mon Sep 17 00:00:00 2001 From: James Cherry Date: Sat, 4 Oct 2025 08:25:17 -0700 Subject: [PATCH 07/14] CodingGuildlines Signed-off-by: James Cherry --- doc/CodingGuidelines.txt | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/doc/CodingGuidelines.txt b/doc/CodingGuidelines.txt index 5e2d1e48..4e2e096e 100644 --- a/doc/CodingGuidelines.txt +++ b/doc/CodingGuidelines.txt @@ -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 From 745ee606f93f1f0b2a3d13629c644a2a573515f2 Mon Sep 17 00:00:00 2001 From: Henner Zeller Date: Sat, 4 Oct 2025 17:50:11 +0200 Subject: [PATCH 08/14] Mark choice of delay implementation with IWYU export pragma. (#300) The Delay header is meant to provide the Delay implementation to whoever is including it; it chooses the right implementation via an include which this PR marks as providing a symbol that is to be exported. Without that annotation, tools such as `clang-tidy` or the `clangd` language server (as well as many other tools) will complain about headers not directly providing a symbol if users just include Delay.hh; With this annotation, they know. Documentation about these IWYU pragmas: https://clangd.llvm.org/design/include-cleaner#iwyu-pragmas https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-begin_exportsend_exports Signed-off-by: Henner Zeller --- include/sta/Delay.hh | 2 ++ 1 file changed, 2 insertions(+) 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 { From 9550c99f0c260b26b94c02d7b0d5609c560b0bb6 Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Sat, 4 Oct 2025 13:36:22 -0400 Subject: [PATCH 09/14] Package require test (#303) --- test/package_require.ok | 0 test/package_require.tcl | 3 +++ test/regression_vars.tcl | 1 + 3 files changed, 4 insertions(+) create mode 100644 test/package_require.ok create mode 100644 test/package_require.tcl 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..e1cfbf71 100644 --- a/test/regression_vars.tcl +++ b/test/regression_vars.tcl @@ -149,6 +149,7 @@ record_sta_tests { liberty_ccsn liberty_float_as_str liberty_latch3 + package_require path_group_names prima3 report_checks_src_attr From 091d69385e0740693556e2a4b3c0633e615205eb Mon Sep 17 00:00:00 2001 From: James Cherry Date: Mon, 6 Oct 2025 08:34:44 -0700 Subject: [PATCH 10/14] CheckCrpr::findCrpr resolves orfs1253 Signed-off-by: James Cherry --- search/Crpr.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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", From c11bb38f58c34d4f804fd6f05debf644b733ce87 Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Tue, 7 Oct 2025 12:12:54 -0400 Subject: [PATCH 11/14] Include StaConfig.hh once (#305) * Include StaConfig.hh once * pragma once --- util/StaConfig.hh.cmake | 2 ++ 1 file changed, 2 insertions(+) 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}" From 36e516924f25e187af11baff2109c40c86313535 Mon Sep 17 00:00:00 2001 From: ambd161 <203089955+ambd161@users.noreply.github.com> Date: Sun, 12 Oct 2025 14:11:00 -0700 Subject: [PATCH 12/14] Recognize some basic specify blocks and ignore them (#309) * Add parser support for specify blocks and specparam Treated like regular parameters, and so ignored * Add regression test * Apply PR feedback * missed the verilog_lang --- test/regression_vars.tcl | 3 ++- test/verilog_specify.ok | 0 test/verilog_specify.tcl | 2 ++ test/verilog_specify.v | 20 ++++++++++++++++++++ verilog/VerilogLex.ll | 3 +++ verilog/VerilogParse.yy | 23 +++++++++++++++++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 test/verilog_specify.ok create mode 100644 test/verilog_specify.tcl create mode 100644 test/verilog_specify.v diff --git a/test/regression_vars.tcl b/test/regression_vars.tcl index e1cfbf71..d7a46617 100644 --- a/test/regression_vars.tcl +++ b/test/regression_vars.tcl @@ -152,12 +152,13 @@ record_sta_tests { 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/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 ';' From daeea4ab7e892aad762c8e50a9d19591d1d574d9 Mon Sep 17 00:00:00 2001 From: Drew Lewis Date: Sun, 12 Oct 2025 17:11:21 -0400 Subject: [PATCH 13/14] Change DispatchQueue::dispatch to use notify_one. (#308) 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: https://github.com/embeddedartistry/embedded-resources/commit/79ad8a539d7e264350464e434e03b5d910ae4a29 Signed-off-by: Drew Lewis --- util/DispatchQueue.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 1a22c68c6268c66d86dd48d1dd0dc5433a1211ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A2ris=20DOUADY?= Date: Mon, 13 Oct 2025 18:54:24 +0200 Subject: [PATCH 14/14] Update report_checks fields to include 'fanout' (which is already supported) (#315) --- search/Search.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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]}