diff --git a/Changes b/Changes index 864b05728..0aa1a1586 100644 --- a/Changes +++ b/Changes @@ -5,10 +5,14 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 4.107 devel +** Support randomize() class method and rand (#2607). [Krzysztof Bieganski] + *** Support $cast and new CASTCONST warning. *** Add --top option as alias of --top-module. +**** Report UNUSED on parameters, localparam and genvars (#2627). [Charles Eric LaForest] + **** Fix passing parameter type instantiations by position number. diff --git a/bin/verilator b/bin/verilator index f7f671968..e48d658d1 100755 --- a/bin/verilator +++ b/bin/verilator @@ -4973,10 +4973,10 @@ design simulate incorrectly; see the details under --bbox-unsup. =item UNUSED -Warns that the specified signal is never used/consumed. Verilator is -fairly liberal in the usage calculations; making a signal public, a signal -matching --unused-regexp ("*unused*") or accessing only a single array -element marks the entire signal as used. +Warns that the specified signal or parameter is never used/consumed. +Verilator is fairly liberal in the usage calculations; making a signal +public, a signal matching --unused-regexp ("*unused*") or accessing only a +single array element marks the entire signal as used. Disabled by default as this is a code style warning; it will simulate correctly. diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index d9602c913..3e8790c90 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -330,12 +330,6 @@ private: if (nodep->isSigPublic()) return false; // Can't elim publics! if (nodep->isIO() || nodep->isClassMember()) return false; if (nodep->isTemp() && !nodep->isTrace()) return true; - if (nodep->isParam()) { - const bool overriddenForHierBlock - = m_modp && m_modp->hierBlock() && nodep->overriddenParam(); - if (!nodep->isTrace() && !overriddenForHierBlock && !v3Global.opt.xmlOnly()) - return true; - } return m_elimUserVars; // Post-Trace can kill most anything } diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp index 4367d28a3..136be9994 100644 --- a/src/V3LinkResolve.cpp +++ b/src/V3LinkResolve.cpp @@ -105,7 +105,7 @@ private: virtual void visit(AstNodeVarRef* nodep) override { // VarRef: Resolve its reference - if (nodep->varp()) { nodep->varp()->usedParam(true); } + if (nodep->varp()) nodep->varp()->usedParam(true); iterateChildren(nodep); } diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index 50fbaae42..5bf4348ac 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -149,7 +149,7 @@ public: void reportViolations() { // Combine bits into overall state AstVar* nodep = m_varp; - if (!nodep->isParam() && !nodep->isGenVar()) { + { bool allU = true; bool allD = true; bool anyU = m_wholeFlags[FLAG_USED]; @@ -168,8 +168,12 @@ public: anyDnotU |= !used && driv; anynotDU |= !used && !driv; } + if ((nodep->isGenVar() || nodep->isParam()) && nodep->isUsedParam()) + allD = allU = true; if (allU) m_wholeFlags[FLAG_USED] = true; if (allD) m_wholeFlags[FLAG_DRIVEN] = true; + const char* const what + = nodep->isParam() ? "parameter" : nodep->isGenVar() ? "genvar" : "signal"; // Test results if (nodep->isIfaceRef()) { // For interface top level we don't do any tracking @@ -181,35 +185,39 @@ public: // UNDRIVEN is considered more serious - as is more likely a bug, // thus undriven+unused bits get UNUSED warnings, as they're not as buggy. if (!unusedMatch(nodep)) { - nodep->v3warn(UNUSED, - "Signal is not driven, nor used: " << nodep->prettyNameQ()); + nodep->v3warn(UNUSED, ucfirst(what) << " is not driven, nor used: " + << nodep->prettyNameQ()); nodep->fileline()->modifyWarnOff(V3ErrorCode::UNUSED, true); // Warn only once } } else if (allD && !anyU) { if (!unusedMatch(nodep)) { - nodep->v3warn(UNUSED, "Signal is not used: " << nodep->prettyNameQ()); + nodep->v3warn(UNUSED, ucfirst(what) + << " is not used: " << nodep->prettyNameQ()); nodep->fileline()->modifyWarnOff(V3ErrorCode::UNUSED, true); // Warn only once } } else if (!anyD && allU) { - nodep->v3warn(UNDRIVEN, "Signal is not driven: " << nodep->prettyNameQ()); + nodep->v3warn(UNDRIVEN, ucfirst(what) + << " is not driven: " << nodep->prettyNameQ()); nodep->fileline()->modifyWarnOff(V3ErrorCode::UNDRIVEN, true); // Warn only once } else { // Bits have different dispositions bool setU = false; bool setD = false; if (anynotDU && !unusedMatch(nodep)) { - nodep->v3warn(UNUSED, "Bits of signal are not driven, nor used: " - << nodep->prettyNameQ() << bitNames(BN_BOTH)); + nodep->v3warn(UNUSED, "Bits of " << what << " are not driven, nor used: " + << nodep->prettyNameQ() << bitNames(BN_BOTH)); setU = true; } if (anyDnotU && !unusedMatch(nodep)) { - nodep->v3warn(UNUSED, "Bits of signal are not used: " << nodep->prettyNameQ() - << bitNames(BN_UNUSED)); + nodep->v3warn(UNUSED, "Bits of " << what + << " are not used: " << nodep->prettyNameQ() + << bitNames(BN_UNUSED)); setU = true; } if (anyUnotD) { - nodep->v3warn(UNDRIVEN, "Bits of signal are not driven: " - << nodep->prettyNameQ() << bitNames(BN_UNDRIVEN)); + nodep->v3warn(UNDRIVEN, + "Bits of " << what << " are not driven: " << nodep->prettyNameQ() + << bitNames(BN_UNDRIVEN)); setD = true; } if (setU) { // Warn only once diff --git a/test_regress/t/t_delay.v b/test_regress/t/t_delay.v index 7434c1841..c391c5b61 100644 --- a/test_regress/t/t_delay.v +++ b/test_regress/t/t_delay.v @@ -8,7 +8,6 @@ module t (/*AUTOARG*/ // Inputs clk ); - parameter PAR = 3; input clk; integer cyc=1; diff --git a/test_regress/t/t_delay_stmtdly_bad.out b/test_regress/t/t_delay_stmtdly_bad.out index 9a680e713..a5beee6c4 100644 --- a/test_regress/t/t_delay_stmtdly_bad.out +++ b/test_regress/t/t_delay_stmtdly_bad.out @@ -1,26 +1,26 @@ -%Warning-ASSIGNDLY: t/t_delay.v:23:13: Unsupported: Ignoring delay on this assignment/primitive. - 23 | assign #(1.2000000000000000) dly1 = dly0 + 32'h1; +%Warning-ASSIGNDLY: t/t_delay.v:22:13: Unsupported: Ignoring delay on this assignment/primitive. + 22 | assign #(1.2000000000000000) dly1 = dly0 + 32'h1; | ^~~~~~~~~~~~~~~~~~ ... Use "/* verilator lint_off ASSIGNDLY */" and lint_on around source to disable this message. -%Warning-ASSIGNDLY: t/t_delay.v:28:19: Unsupported: Ignoring delay on this assignment/primitive. - 28 | dly0 <= #0 32'h11; +%Warning-ASSIGNDLY: t/t_delay.v:27:19: Unsupported: Ignoring delay on this assignment/primitive. + 27 | dly0 <= #0 32'h11; | ^ -%Warning-ASSIGNDLY: t/t_delay.v:31:19: Unsupported: Ignoring delay on this assignment/primitive. - 31 | dly0 <= #0.12 dly0 + 32'h12; +%Warning-ASSIGNDLY: t/t_delay.v:30:19: Unsupported: Ignoring delay on this assignment/primitive. + 30 | dly0 <= #0.12 dly0 + 32'h12; | ^~~~ -%Warning-ASSIGNDLY: t/t_delay.v:39:25: Unsupported: Ignoring delay on this assignment/primitive. - 39 | dly0 <= #(dly_s.dly) 32'h55; +%Warning-ASSIGNDLY: t/t_delay.v:38:25: Unsupported: Ignoring delay on this assignment/primitive. + 38 | dly0 <= #(dly_s.dly) 32'h55; | ^ -%Warning-STMTDLY: t/t_delay.v:44:11: Unsupported: Ignoring delay on this delayed statement. +%Warning-STMTDLY: t/t_delay.v:43:11: Unsupported: Ignoring delay on this delayed statement. : ... In instance t - 44 | #100 $finish; + 43 | #100 $finish; | ^~~ -%Warning-UNUSED: t/t_delay.v:21:12: Signal is not used: 'dly_s' +%Warning-UNUSED: t/t_delay.v:20:12: Signal is not used: 'dly_s' : ... In instance t - 21 | dly_s_t dly_s; + 20 | dly_s_t dly_s; | ^~~~~ -%Warning-BLKSEQ: t/t_delay.v:38:20: Blocking assignments (=) in sequential (flop or latch) block +%Warning-BLKSEQ: t/t_delay.v:37:20: Blocking assignments (=) in sequential (flop or latch) block : ... Suggest delayed assignments (<=) - 38 | dly_s.dly = 55; + 37 | dly_s.dly = 55; | ^ %Error: Exiting due to diff --git a/test_regress/t/t_lint_defparam.v b/test_regress/t/t_lint_defparam.v index 05bd79f41..a2b70f76e 100644 --- a/test_regress/t/t_lint_defparam.v +++ b/test_regress/t/t_lint_defparam.v @@ -13,4 +13,5 @@ endmodule module sub; parameter P = 6; + if (P != 0) ; // Prevent unused endmodule diff --git a/test_regress/t/t_lint_unused.v b/test_regress/t/t_lint_unused.v index 3042c127e..697706563 100644 --- a/test_regress/t/t_lint_unused.v +++ b/test_regress/t/t_lint_unused.v @@ -44,8 +44,6 @@ module sub; wire pub /*verilator public*/; // Ignore publics - localparam THREE = 3; - endmodule primitive udp_mux2 (q, a, b, s); diff --git a/test_regress/t/t_lint_unused_bad.out b/test_regress/t/t_lint_unused_bad.out index 9e888314f..745ca8336 100644 --- a/test_regress/t/t_lint_unused_bad.out +++ b/test_regress/t/t_lint_unused_bad.out @@ -23,4 +23,16 @@ : ... In instance t.sub 28 | wire [3:0] mixed; | ^~~~~ +%Warning-UNUSED: t/t_lint_unused_bad.v:37:14: Parameter is not used: 'UNUSED_P' + : ... In instance t.sub + 37 | parameter UNUSED_P = 1; + | ^~~~~~~~ +%Warning-UNUSED: t/t_lint_unused_bad.v:38:15: Parameter is not used: 'UNUSED_LP' + : ... In instance t.sub + 38 | localparam UNUSED_LP = 2; + | ^~~~~~~~~ +%Warning-UNUSED: t/t_lint_unused_bad.v:40:15: Genvar is not driven, nor used: 'unused_gv' + : ... In instance t.sub + 40 | genvar unused_gv; + | ^~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_lint_unused_bad.v b/test_regress/t/t_lint_unused_bad.v index 839527590..ade5152d3 100644 --- a/test_regress/t/t_lint_unused_bad.v +++ b/test_regress/t/t_lint_unused_bad.v @@ -34,9 +34,21 @@ module sub; localparam THREE = 3; + parameter UNUSED_P = 1; + localparam UNUSED_LP = 2; + + genvar unused_gv; + genvar ok_gv; + initial begin if (0 && assunu1[0] != 0 && udrb2 != 0) begin end if (0 && assunub2[THREE] && assunub2[1:0]!=0) begin end if (0 && mixed[1:0] != 0) begin end end + + generate + if (0) + for (ok_gv = 0; ok_gv < 1; ++ok_gv) begin end + endgenerate + endmodule