diff --git a/Changes b/Changes index 45c4102dd..3ac72afbd 100644 --- a/Changes +++ b/Changes @@ -35,6 +35,7 @@ Verilator 5.045 devel * Remove deprecated `--make cmake`. * Change JSON dumps to not include booleans that are false (#6977). * Change metacomment extra underscore error to BADVLTPRAGMA warning (#6968). [Geza Lore, Testorrent USA, Inc.] +* Change INITIALSTATIC to also report on processes, per IEEE. * Optimize string temporaries to not be localized (#6969). [Geza Lore, Testorrent USA, Inc.] * Optimize wide word shifts by multiple of word size (#6970). [Geza Lore, Testorrent USA, Inc.] * Optimize concatenations that produce unused bits in DFG (#6971). [Geza Lore, Testorrent USA, Inc.] diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index e63e07369..8c7ba7a80 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -1022,13 +1022,20 @@ List Of Warnings .. option:: IMPLICITSTATIC Warns that the lifetime of a task or a function was not provided and so - was implicitly set to static. The warning is suppressed when no - variables inside the task or a function are assigned to. + an enclosed variable was implicitly set to static. The warning is + suppressed when no variables inside the task or a function are assigned + to. + + Also warns that a process (e.g. "always" or "initial" statement) has + enclosed variables that were implicitly set to static. + + IEEE 1800-2023 6.21 requires this error, though Verilator treats it by + default as a warning. This is a warning because the static default differs from C++, differs from class member function/tasks. Static is a more dangerous default - then automatic as static prevents the function from being reentrant, - which may be a source of bugs, and/or performance issues. + then automatic as static prevents the function or process from being + reentrant, which may be a source of bugs, and/or performance issues. If the function is in a module, and does not require static behavior, change it to "function automatic". diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index 9f7bc2298..e16758430 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -325,13 +325,12 @@ class LinkParseVisitor final : public VNVisitor { && !nodep->isIO() && !nodep->isParam() // In task, or a procedure but not Initial/Final as executed only once - && ((m_ftaskp && !m_ftaskp->lifetime().isStaticExplicit()) - || (m_procedurep && !VN_IS(m_procedurep, Initial) - && !VN_IS(m_procedurep, Final)))) { + && ((m_ftaskp && !m_ftaskp->lifetime().isStaticExplicit()) || m_procedurep)) { if (VN_IS(m_modp, Module) && m_ftaskp) { m_ftaskp->v3warn( IMPLICITSTATIC, - "Function/task's lifetime implicitly set to static\n" + "Function/task's lifetime implicitly set to static;" + " variables made static (IEEE 1800-2023 6.21)\n" << m_ftaskp->warnMore() << "... Suggest use '" << m_ftaskp->verilogKwd() << " automatic' or '" << m_ftaskp->verilogKwd() << " static'\n" << m_ftaskp->warnContextPrimary() << '\n' @@ -339,12 +338,12 @@ class LinkParseVisitor final : public VNVisitor { << nodep->warnMore() << "... The initializer value will only be set once\n" << nodep->warnContextSecondary()); } else { - nodep->v3warn(IMPLICITSTATIC, - "Variable's lifetime implicitly set to static\n" - << nodep->warnMore() - << "... The initializer value will only be set once\n" - << nodep->warnMore() - << "... Suggest use 'static' before variable declaration'"); + nodep->v3warn( + IMPLICITSTATIC, + "Variable's lifetime implicitly set to static (IEEE 1800-2023 6.21)\n" + << nodep->warnMore() << "... The initializer value will only be set once\n" + << nodep->warnMore() + << "... Suggest use 'static' before variable declaration'"); } } if (!m_lifetimeAllowed && nodep->lifetime().isAutomatic()) { diff --git a/test_regress/t/t_func_no_lifetime_bad.out b/test_regress/t/t_func_no_lifetime_bad.out index 148325abf..5bf7797db 100644 --- a/test_regress/t/t_func_no_lifetime_bad.out +++ b/test_regress/t/t_func_no_lifetime_bad.out @@ -1,4 +1,4 @@ -%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:26:17: Function/task's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:26:17: Function/task's lifetime implicitly set to static; variables made static (IEEE 1800-2023 6.21) : ... Suggest use 'function automatic' or 'function static' 26 | function int f_implicit_static(); | ^~~~~~~~~~~~~~~~~ @@ -8,7 +8,7 @@ | ^~~ ... For warning description see https://verilator.org/warn/IMPLICITSTATIC?v=latest ... Use "/* verilator lint_off IMPLICITSTATIC */" and lint_on around source to disable this message. -%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:31:9: Function/task's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:31:9: Function/task's lifetime implicitly set to static; variables made static (IEEE 1800-2023 6.21) : ... Suggest use 'task automatic' or 'task static' 31 | task t_implicit_static(); | ^~~~~~~~~~~~~~~~~ @@ -16,12 +16,12 @@ : ... The initializer value will only be set once 32 | int cnt = 0; | ^~~ -%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:9:8: Variable's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:9:8: Variable's lifetime implicitly set to static (IEEE 1800-2023 6.21) : ... The initializer value will only be set once : ... Suggest use 'static' before variable declaration' 9 | int cnt = 0; | ^~~ -%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:15:8: Variable's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:15:8: Variable's lifetime implicitly set to static (IEEE 1800-2023 6.21) : ... The initializer value will only be set once : ... Suggest use 'static' before variable declaration' 15 | int cnt = 0; diff --git a/test_regress/t/t_lint_implicitstatic_bad.out b/test_regress/t/t_lint_implicitstatic_bad.out index 2af9e2775..0b3de4362 100644 --- a/test_regress/t/t_lint_implicitstatic_bad.out +++ b/test_regress/t/t_lint_implicitstatic_bad.out @@ -1,31 +1,29 @@ -%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:16:9: Variable's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:20:9: Variable's lifetime implicitly set to static (IEEE 1800-2023 6.21) : ... The initializer value will only be set once : ... Suggest use 'static' before variable declaration' - 16 | int implicit_warn = 1; + 20 | int implicit_warn = 1; | ^~~~~~~~~~~~~ ... For warning description see https://verilator.org/warn/IMPLICITSTATIC?v=latest ... Use "/* verilator lint_off IMPLICITSTATIC */" and lint_on around source to disable this message. -%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:20:16: Function/task's lifetime implicitly set to static - : ... Suggest use 'function automatic' or 'function static' - 20 | function int f_implicit_static(); - | ^~~~~~~~~~~~~~~~~ - t/t_lint_implicitstatic_bad.v:21:9: ... Location of implicit static variable - : ... The initializer value will only be set once - 21 | int cnt = 0; - | ^~~ -%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:25:8: Function/task's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:24:8: Function/task's lifetime implicitly set to static; variables made static (IEEE 1800-2023 6.21) : ... Suggest use 'task automatic' or 'task static' - 25 | task f_implicit_static(); + 24 | task t_implicit_static(); | ^~~~~~~~~~~~~~~~~ - t/t_lint_implicitstatic_bad.v:26:9: ... Location of implicit static variable + t/t_lint_implicitstatic_bad.v:25:9: ... Location of implicit static variable : ... The initializer value will only be set once - 26 | int cnt = 0; - | ^~~ -%Error: t/t_lint_implicitstatic_bad.v:25:8: Unsupported in C: Task has the same name as function: 'f_implicit_static' - 25 | task f_implicit_static(); - | ^~~~~~~~~~~~~~~~~ - t/t_lint_implicitstatic_bad.v:20:16: ... Location of original declaration - 20 | function int f_implicit_static(); + 25 | int t_no_has_init = 1; + | ^~~~~~~~~~~~~ +%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:32:16: Function/task's lifetime implicitly set to static; variables made static (IEEE 1800-2023 6.21) + : ... Suggest use 'function automatic' or 'function static' + 32 | function int f_implicit_static(); | ^~~~~~~~~~~~~~~~~ - ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. + t/t_lint_implicitstatic_bad.v:33:9: ... Location of implicit static variable + : ... The initializer value will only be set once + 33 | int f_no_has_init = 1; + | ^~~~~~~~~~~~~ +%Warning-IMPLICITSTATIC: t/t_lint_implicitstatic_bad.v:42:9: Variable's lifetime implicitly set to static (IEEE 1800-2023 6.21) + : ... The initializer value will only be set once + : ... Suggest use 'static' before variable declaration' + 42 | int i_no_has_init = 1; + | ^~~~~~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_lint_implicitstatic_bad.v b/test_regress/t/t_lint_implicitstatic_bad.v index e5521bbe4..e2d60dcc2 100644 --- a/test_regress/t/t_lint_implicitstatic_bad.v +++ b/test_regress/t/t_lint_implicitstatic_bad.v @@ -8,32 +8,41 @@ module t ( input clk ); - initial begin - int static_ok = 1; // Obvious as is in initial - end + class C; + task t; + int c_no_has_init = 1; // Ok + automatic int c_automatic_has_init = 1; // Ok + static int c_static_has_init = 1; // Ok + endtask + endclass always @(posedge clk) begin int implicit_warn = 1; // <--- Warning: IMPLICITSTATIC - localparam int NO_WARN = 2; // No warning here + localparam int OK = 2; // Ok end - function int f_implicit_static(); - int cnt = 0; // <--- Warning: IMPLICIT STATIC - return ++cnt; - endfunction - - task f_implicit_static(); - int cnt = 0; // <--- Warning: IMPLICIT STATIC - ++cnt; + task t_implicit_static(); + int t_no_has_init = 1; // <--- Warning: IMPLICIT STATIC + automatic int t_automatic_has_init = 1; // Ok + static int t_static_has_init = 1; // Ok + localparam int ONE = 1; // Ok + ++t_no_has_init; endtask - function int f_no_implicit_static(); - localparam int ONE = 1; // No warning here + function int f_implicit_static(); + int f_no_has_init = 1; // <--- Warning: IMPLICIT STATIC + automatic int f_automatic_has_init = 1; // Ok + static int f_static_has_init = 1; // Ok + localparam int ONE = 1; // Ok + ++f_no_has_init; return ONE; endfunction - task t_no_implicit_static(); - localparam TWO = 2; // No warning here - endtask + initial begin + int i_no_has_init = 1; // <--- Warning: want explicit lifetime + automatic int i_automatic_has_init = 1; // Ok + static int i_static_has_init = 1; // Ok + $finish; + end endmodule