diff --git a/Changes b/Changes index cd044ae68..6b0380417 100644 --- a/Changes +++ b/Changes @@ -28,6 +28,7 @@ Verilator 5.011 devel * Fix linking AstRefDType if it has parameterized class ref (#4164) (#4170). [Ryszard Rozak, Antmicro Ltd] * Fix crash caused by $display() optimization (#4165) (#4166). [Tudor Timi] * Fix detection of wire/reg duplicates. +* Fix false IMPLICITSTATIC on package functions. Verilator 5.010 2023-04-30 diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index fa131af70..5dc90fba4 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -769,11 +769,14 @@ List Of Warnings then automatic as static prevents the function from being reentrant, which may be a source of bugs, and/or performance issues. - If the function does not require static behavior, change it to "function - automatic". + If the function is in a module, and does not require static behavior, + change it to "function automatic". - If the function requires static behavior, change it to "function - static". + If the function is in a module, and requires static behavior, change it + to "function static". + + If the function is in a package, it defaults to static, and label the + function's variables as static. Ignoring this warning will only suppress the lint check; it will simulate correctly. diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index bc4af9391..9da829f30 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -124,13 +124,6 @@ private: && (VN_IS(nodep->stmtsp(), GenIf)) // Begin has if underneath && !nodep->stmtsp()->nextp()); // Has only one item } - bool hasStaticDeclAssignments(AstNodeFTask* nodep) { - for (const AstNode* itemp = nodep->stmtsp(); itemp; itemp = itemp->nextp()) { - const AstVar* const varp = VN_CAST(itemp, Var); - if (varp && varp->valuep() && !varp->lifetime().isAutomatic()) return true; - } - return false; - } // VISITs void visit(AstNodeFTask* nodep) override { @@ -156,12 +149,30 @@ private: // DPI-imported functions and properties don't have lifetime specifiers m_lifetime = VLifetime::NONE; } - if (m_lifetime.isStatic() && hasStaticDeclAssignments(nodep)) { - nodep->v3warn( - IMPLICITSTATIC, - "Function/task's lifetime implicitly set to static\n" - << nodep->warnMore() - << "... Suggest use 'function automatic' or 'function static'"); + for (AstNode* itemp = nodep->stmtsp(); itemp; itemp = itemp->nextp()) { + AstVar* const varp = VN_CAST(itemp, Var); + if (varp && varp->valuep() && varp->lifetime().isNone() + && m_lifetime.isStatic() && !varp->isIO()) { + if (VN_IS(m_modp, Module)) { + nodep->v3warn(IMPLICITSTATIC, + "Function/task's lifetime implicitly set to static\n" + << nodep->warnMore() + << "... Suggest use 'function automatic' or " + "'function static'\n" + << nodep->warnContextPrimary() << '\n' + << varp->warnOther() + << "... Location of implicit static variable\n" + << varp->warnContextSecondary() << '\n' + << "... Suggest use 'function automatic' or " + "'function static'"); + } else { + varp->v3warn(IMPLICITSTATIC, + "Variable's lifetime implicitly set to static\n" + << nodep->warnMore() + << "... Suggest use 'static' before " + "variable declaration'"); + } + } } nodep->lifetime(m_lifetime); } diff --git a/test_regress/t/t_func_no_lifetime_bad.out b/test_regress/t/t_func_no_lifetime_bad.out index ac2132066..bc145b029 100644 --- a/test_regress/t/t_func_no_lifetime_bad.out +++ b/test_regress/t/t_func_no_lifetime_bad.out @@ -1,11 +1,27 @@ -%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:7:14: Function/task's lifetime implicitly set to static - : ... Suggest use 'function automatic' or 'function static' - 7 | function int f_implicit_static(); - | ^~~~~~~~~~~~~~~~~ +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:29:17: Function/task's lifetime implicitly set to static + : ... Suggest use 'function automatic' or 'function static' + 29 | function int f_implicit_static(); + | ^~~~~~~~~~~~~~~~~ + t/t_func_no_lifetime_bad.v:30:11: ... Location of implicit static variable + 30 | int cnt = 0; + | ^~~ +... Suggest use 'function automatic' or 'function static' ... 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:12:6: Function/task's lifetime implicitly set to static +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:34:9: Function/task's lifetime implicitly set to static : ... Suggest use 'function automatic' or 'function static' - 12 | task t_implicit_static(); - | ^~~~~~~~~~~~~~~~~ + 34 | task t_implicit_static(); + | ^~~~~~~~~~~~~~~~~ + t/t_func_no_lifetime_bad.v:35:11: ... Location of implicit static variable + 35 | int cnt = 0; + | ^~~ +... Suggest use 'function automatic' or 'function static' +%Warning-IMPLICITSTATIC: t/t_func_no_lifetime_bad.v:9:8: Variable's lifetime implicitly set to static + : ... 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 + : ... Suggest use 'static' before variable declaration' + 15 | int cnt = 0; + | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_func_no_lifetime_bad.v b/test_regress/t/t_func_no_lifetime_bad.v index e77979cf3..1c2fe0c2d 100644 --- a/test_regress/t/t_func_no_lifetime_bad.v +++ b/test_regress/t/t_func_no_lifetime_bad.v @@ -4,22 +4,38 @@ // any use, without warranty, 2023 by Antmicro Ltd. // SPDX-License-Identifier: CC0-1.0 -function int f_implicit_static(); - int cnt = 0; +// Not legal to put "static" here, so no warning +function int f_dunit_static(); + int cnt = 0; // Ok to require "static" here somehday return ++cnt; endfunction -task t_implicit_static(); - int cnt = 0; +// Not legal to put "static" here, so no warning +task t_dunit_static(); + int cnt = 0; // Ok to require "static" here somehday $display("%d", ++cnt); endtask +task t_dunit_static_ok(input int in_ok = 1); + static int cnt_ok = 0; // No warning here + $display("%d", ++cnt_ok); +endtask module t (/*AUTOARG*/ // Inputs clk ); + function int f_implicit_static(); + int cnt = 0; + return ++cnt; + endfunction + + task t_implicit_static(); + int cnt = 0; + $display("%d", ++cnt); + endtask + // verilator lint_off IMPLICITSTATIC function int f_implicit_but_lint_off(); int cnt = 0; @@ -30,6 +46,10 @@ module t (/*AUTOARG*/ int a, b; initial begin + a = f_dunit_static(); + t_dunit_static(); + t_dunit_static_ok(); + a = f_implicit_static(); t_implicit_static(); b = f_implicit_but_lint_off();