github/verilator
1
0
Fork 1
mirror of https://github.com/verilator/verilator.git synced 2025-01-01 04:07:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add GENUNNAMED lint warning.

Browse Source 
Also fix generate-for blocks with empty statements getting lost.
This commit is contained in:
Wilson Snyder 2023-07-01 08:31:53 -04:00
parent 8b1cc3b303
commit cff37f0775
11 changed files with 159 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changes
View File

@ -15,6 +15,7 @@ Verilator 5.013 devel
* Deprecation planned for 32-bit pointer -m32 mode (#4268).
* Support some stream operations on queues (#4292). [Ryszard Rozak, Antmicro Ltd]
* Add GENUNNAMED lint warning. [Srinivasan Venkataramanan, Deepa Palaniappan]
* Fix 'VlForkSync' redeclaration (#4277). [Krzysztof Bieganski, Antmicro Ltd]
* Fix processes that can outlive their parents (#4253). [Krzysztof Boronski, Antmicro Ltd]
* Fix duplicate fork names (#4295). [Ryszard Rozak, Antmicro Ltd]

4
docs/guide/exe_verilator.rst
View File

@ -1539,7 +1539,7 @@ Summary:
Disable all code style related warning messages (note that by default, they are
already disabled). This is equivalent to ``-Wno-DECLFILENAME -Wno-DEFPARAM
-Wno-EOFNEWLINE -Wno-IMPORTSTAR -Wno-INCABSPATH -Wno-PINCONNECTEMPTY
-Wno-EOFNEWLINE -Wno-GENUNNAMED -Wno-IMPORTSTAR -Wno-INCABSPATH -Wno-PINCONNECTEMPTY
-Wno-PINNOCONNECT -Wno-SYNCASYNCNET -Wno-UNDRIVEN
-Wno-UNUSEDGENVAR -Wno-UNUSEDPARAM -Wno-UNUSEDSIGNAL
-Wno-VARHIDDEN``.
@ -1571,7 +1571,7 @@ Summary:
Enable all code style-related warning messages. This is equivalent to
``-Wwarn ASSIGNDLY -Wwarn-DECLFILENAME -Wwarn-DEFPARAM -Wwarn-EOFNEWLINE
-Wwarn-INCABSPATH -Wwarn-PINNOCONNECT -Wwarn-SYNCASYNCNET -Wwarn-UNDRIVEN
-Wwarn-GENUNNAMED -Wwarn-INCABSPATH -Wwarn-PINNOCONNECT -Wwarn-SYNCASYNCNET -Wwarn-UNDRIVEN
-Wwarn-UNUSEDGENVAR -Wwarn-UNUSEDPARAM -Wwarn-UNUSEDSIGNAL -Wwarn-VARHIDDEN``.
.. option:: --x-assign 0

46
docs/guide/warnings.rst
View File

@ -670,6 +670,52 @@ List Of Warnings
used as a clock.
.. option:: GENUNNAMED
Warns that a generate block was unnamed and "genblk" will be used per
IEEE.
The potential issue is that adding additional generate blocks will
renumber the assigned names, which may cause evental problems with
synthesis constraints or other tools that depend on hierarchical paths
remaining consistend.
Blocks that are empty may not be reported with this warning, as no
scopes are created for empty blocks, so there is no harm in having them
unnamed.
Disabled by default as this is a code-style warning; it will simulate
correctly.
.. code-block:: sv
:linenos:
:emphasize-lines: 2
generate
if (PARAM == 1) begin //<--- Warning
end
Results in:
.. code-block::
%Warning-GENUNNAMED: example.v:2:9: Unnamed generate block (IEEE 1800-2017 27.6)
To fix this assign a label (often with the naming convention prefix of
'gen_' or 'g_'), for example:
.. code-block:: sv
:linenos:
:emphasize-lines: 2
generate
if (PARAM == 1) begin : gen_param_1 //<--- Repaired
end
Other tools with similar warnings: Verible's generate-label, "All
generate block statements must have a label."
.. option:: HIERBLOCK
Warns that the top module is marked as a hierarchy block by the

12
src/V3Error.h
View File

@ -99,6 +99,7 @@ public:
ENUMVALUE, // Error: enum type needs explicit cast
EOFNEWLINE, // End-of-file missing newline
GENCLK, // Generated Clock. Historical, never issued.
GENUNNAMED, // Generate unnamed, without label
HIERBLOCK, // Ignored hierarchical block setting
IFDEPTH, // If statements too deep
IGNOREDRETURN, // Ignoring return value (function as task)
@ -192,7 +193,8 @@ public:
"CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CASTCONST", "CDCRSTLOGIC", "CLKDATA",
"CMPCONST", "COLONPLUS", "COMBDLY", "CONSTRAINTIGN", "CONTASSREG",
"DECLFILENAME", "DEFPARAM", "DEPRECATED",
"ENCAPSULATED", "ENDLABEL", "ENUMVALUE", "EOFNEWLINE", "GENCLK", "HIERBLOCK",
"ENCAPSULATED", "ENDLABEL", "ENUMVALUE", "EOFNEWLINE", "GENCLK", "GENUNNAMED",
"HIERBLOCK",
"IFDEPTH", "IGNOREDRETURN",
"IMPERFECTSCH", "IMPLICIT", "IMPLICITSTATIC", "IMPORTSTAR", "IMPURE",
"INCABSPATH", "INFINITELOOP", "INITIALDLY", "INSECURE",
@ -243,10 +245,10 @@ public:
bool styleError() const VL_MT_SAFE {
return (m_e == ASSIGNDLY // More than style, but for backward compatibility
|| m_e == BLKSEQ || m_e == DEFPARAM || m_e == DECLFILENAME || m_e == EOFNEWLINE
|| m_e == IMPORTSTAR || m_e == INCABSPATH || m_e == PINCONNECTEMPTY
|| m_e == PINNOCONNECT || m_e == SYNCASYNCNET || m_e == UNDRIVEN
|| m_e == UNUSEDGENVAR || m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL
|| m_e == VARHIDDEN);
|| m_e == GENUNNAMED || m_e == IMPORTSTAR || m_e == INCABSPATH
|| m_e == PINCONNECTEMPTY || m_e == PINNOCONNECT || m_e == SYNCASYNCNET
|| m_e == UNDRIVEN || m_e == UNUSEDGENVAR || m_e == UNUSEDPARAM
|| m_e == UNUSEDSIGNAL || m_e == VARHIDDEN);
}
// Warnings that are unused only
bool unusedError() const VL_MT_SAFE {

19
src/V3LinkParse.cpp
View File

@ -580,14 +580,25 @@ private:
// The genblk name will get attached to the if true/false LOWER begin block(s)
const bool nestedIf = nestedIfBegin(nodep);
// It's not FOR(BEGIN(...)) but we earlier changed it to BEGIN(FOR(...))
int assignGenBlkNum = -1;
if (nodep->genforp()) {
++m_genblkNum;
if (nodep->name() == "") nodep->name("genblk" + cvtToStr(m_genblkNum));
if (nodep->name() == "") assignGenBlkNum = m_genblkNum;
} else if (nodep->generate() && nodep->name() == "" && assignGenBlkNum == -1
&& (VN_IS(backp, CaseItem) || VN_IS(backp, GenIf)) && !nestedIf) {
assignGenBlkNum = m_genblkAbove;
}
if (nodep->generate() && nodep->name() == ""
&& (VN_IS(backp, CaseItem) || VN_IS(backp, GenIf)) && !nestedIf) {
nodep->name("genblk" + cvtToStr(m_genblkAbove));
if (assignGenBlkNum != -1) {
nodep->name("genblk" + cvtToStr(assignGenBlkNum));
if (nodep->stmtsp()) {
nodep->v3warn(GENUNNAMED,
"Unnamed generate block "
<< nodep->prettyNameQ() << " (IEEE 1800-2017 27.6)"
<< nodep->warnMore()
<< "... Suggest assign a label with 'begin : gen_<label_name>'");
}
}
if (nodep->name() != "") {
VL_RESTORER(m_genblkAbove);
VL_RESTORER(m_genblkNum);

8
src/verilog.y
View File

@ -2753,12 +2753,14 @@ genItemBegin<nodep>: // IEEE: part of generate_block
{ $$ = new AstBegin{$<fl>1, *$1, $4, true, false};
GRAMMARP->endLabel($<fl>6, *$1, $6); }
| id yP_COLON__BEGIN yBEGIN yEND endLabelE
{ $$ = nullptr; GRAMMARP->endLabel($<fl>5, *$1, $5); }
{ $$ = new AstBegin{$<fl>1, *$1, nullptr, true, false};
GRAMMARP->endLabel($<fl>5, *$1, $5); }
| yBEGIN ':' idAny ~c~genItemList yEND endLabelE
{ $$ = new AstBegin{$<fl>3, *$3, $4, true, false};
GRAMMARP->endLabel($<fl>6, *$3, $6); }
| yBEGIN ':' idAny yEND endLabelE
{ $$ = nullptr; GRAMMARP->endLabel($<fl>5, *$3, $5); }
{ $$ = new AstBegin{$<fl>3, *$3, nullptr, true, false};
GRAMMARP->endLabel($<fl>5, *$3, $5); }
;
c_genItemBegin<nodep>: // IEEE: part of generate_block (for checkers)
@ -5186,7 +5188,7 @@ let_port_list<nodep>: // IEEE: let_port_list
;
let_port_item<nodep>: // IEEE: let_port_Item
// // IEEE: Expanded let_formal_type
// // IEEE: Expanded let_formal_type
yUNTYPED idAny/*formal_port_identifier*/ variable_dimensionListE exprEqE
{ $$ = nullptr; BBUNSUP($<fl>1, "Unsupported: let untyped ports"); }
| data_type id/*formal_port_identifier*/ variable_dimensionListE exprEqE

2
test_regress/t/t_lint_declfilename_bbox.v
View File

@ -6,7 +6,7 @@
module t_lint_declfilename_bbox ();
parameter IN = 0;
if (IN) begin
if (IN) begin : gen_hasbbox
// Should not warn, see bug2430
BLACKBOXED bboxed ();
end

21
test_regress/t/t_lint_genunnamed_bad.out Normal file
View File

@ -0,0 +1,21 @@
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:14:6: Unnamed generate block 'genblk2' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
14 | begin
| ^~~~~
... For warning description see https://verilator.org/warn/GENUNNAMED?v=latest
... Use "/* verilator lint_off GENUNNAMED */" and lint_on around source to disable this message.
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:18:6: Unnamed generate block 'genblk2' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
18 | begin
| ^~~~~
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:22:4: Unnamed generate block 'genblk3' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
22 | for (genvar v = 0; v < P; ++v) ;
| ^~~
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:24:4: Unnamed generate block 'genblk4' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
24 | for (genvar v = 0; v < P; ++v)
| ^~~
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:30:9: Unnamed generate block 'genblk5' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
30 | 1: initial begin end
| ^~~~~~~
%Warning-GENUNNAMED: t/t_lint_genunnamed_bad.v:31:9: Unnamed generate block 'genblk5' (IEEE 1800-2017 27.6) : ... Suggest assign a label with 'begin : gen_<label_name>'
31 | 2: begin
| ^~~~~
%Error: Exiting due to

20
test_regress/t/t_lint_genunnamed_bad.pl Executable file
View File

@ -0,0 +1,20 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003-2009 by Wilson Snyder. This program is free software; you
# can redistribute it and/or modify it under the terms of either the GNU
# Lesser General Public License Version 3 or the Perl Artistic License
# Version 2.0.
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
scenarios(vlt => 1);
lint(
fails => 1,
verilator_flags2 => ["--lint-only -Wall -Wno-DECLFILENAME"],
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

36
test_regress/t/t_lint_genunnamed_bad.v Normal file
View File

@ -0,0 +1,36 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2023 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0
module t (/*AUTOARG*/);
parameter P = 1;
if (P) ;
if (P)
begin
initial $display;
end
else
begin
initial $display;
end
for (genvar v = 0; v < P; ++v) ;
for (genvar v = 0; v < P; ++v)
begin
initial $display;
end
case (P)
1: initial begin end
2: begin
initial begin end
end
endcase
endmodule

7
test_regress/t/t_lint_unused_bad.v
View File

@ -71,8 +71,11 @@ module sub;
end
generate
if (0)
for (ok_gv = 0; ok_gv < 1; ++ok_gv) begin end
if (0) begin : gen_gv_if0
for (ok_gv = 0; ok_gv < 1; ++ok_gv)
begin : gen_gv_if0_for
end
end
endgenerate
endmodule