Fix not reporting some duplicate signals/ports, bug1462.

This commit is contained in:
Wilson Snyder 2019-06-22 12:43:48 -04:00
parent b8eb6368e9
commit 5cb6474cc6
14 changed files with 252 additions and 51 deletions

View File

@ -4,6 +4,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
* Verilator 4.017 devel
**** Fix not reporting some duplicate signals/ports, bug1462. [Peter Gerst]
* Verilator 4.016 2016-06-16

View File

@ -1139,6 +1139,8 @@ private:
VDirection m_direction; // Direction input/output etc
VDirection m_declDirection; // Declared direction input/output etc
AstBasicDTypeKwd m_declKwd; // Keyword at declaration time
bool m_ansi:1; // ANSI port list variable (for dedup check)
bool m_declTyped:1; // Declared as type (for dedup check)
bool m_tristate:1; // Inout or triwire or trireg
bool m_primaryIO:1; // In/out to top level (or directly assigned from same)
bool m_sc:1; // SystemC variable
@ -1171,6 +1173,7 @@ private:
MTaskIdSet m_mtaskIds; // MTaskID's that read or write this var
void init() {
m_ansi = false; m_declTyped = false;
m_tristate = false; m_primaryIO = false;
m_sc = false; m_scClocked = false; m_scSensitive = false;
m_usedClock = false; m_usedParam = false; m_usedLoopIdx = false;
@ -1273,6 +1276,8 @@ public:
AstNode* attrsp() const { return op4p(); } // op4 = Attributes during early parse
void childDTypep(AstNodeDType* nodep) { setOp1p(nodep); }
virtual AstNodeDType* subDTypep() const { return dtypep() ? dtypep() : childDTypep(); }
void ansi(bool flag) { m_ansi = flag; }
void declTyped(bool flag) { m_declTyped = flag; }
void attrClockEn(bool flag) { m_attrClockEn = flag; }
void attrClocker(AstVarAttrClocker flag) { m_attrClocker = flag; }
void attrFileDescr(bool flag) { m_fileDescr = flag; }
@ -1306,6 +1311,8 @@ public:
virtual void name(const string& name) { m_name = name; }
virtual void tag(const string& text) { m_tag = text;}
virtual string tag() const { return m_tag; }
bool isAnsi() const { return m_ansi; }
bool isDeclTyped() const { return m_declTyped; }
bool isInoutish() const { return m_direction.isInoutish(); }
bool isNonOutput() const { return m_direction.isNonOutput(); }
bool isReadOnly() const { return m_direction.isReadOnly(); }

View File

@ -933,10 +933,23 @@ class LinkDotFindVisitor : public AstNVisitor {
<<" ;; parent=se"<<cvtToHex(foundp->parentp())<<endl);
if (foundp && foundp->parentp() == m_curSymp // Only when on same level
&& !foundp->imported()) { // and not from package
if (!(findvarp->isIO() && nodep->isIO()) // e.g. !(output && output)
&& ((findvarp->isIO() && nodep->isSignal()) // e.g. output && reg
|| (findvarp->isSignal() && nodep->isIO())) // e.g. reg && output
&& !(findvarp->isSignal() && !nodep->isSignal())) { // e.g. !(reg && reg)
bool nansiBad = ((findvarp->isDeclTyped() && nodep->isDeclTyped())
|| (findvarp->isIO() && nodep->isIO())); // e.g. !(output && output)
bool ansiBad = findvarp->isAnsi() || nodep->isAnsi(); // dup illegal with ANSI
if (ansiBad || nansiBad) {
static int didAnsiWarn = false;
bool ansiWarn = ansiBad && !nansiBad;
if (ansiWarn) { if (didAnsiWarn++) ansiWarn = false; }
nodep->v3error("Duplicate declaration of signal: "
<<nodep->prettyName()<<endl
<<findvarp->warnMore()<<"... Location of original declaration"<<endl
<<(ansiWarn
? findvarp->warnMore()+"... Note: ANSI ports must have type declared with the I/O (IEEE 2017 23.2.2.2)"
: ""));
// Combining most likely reduce other errors
findvarp->combineType(nodep);
findvarp->fileline()->modifyStateInherit(nodep->fileline());
} else {
findvarp->combineType(nodep);
findvarp->fileline()->modifyStateInherit(nodep->fileline());
AstBasicDType* bdtypep = VN_CAST(findvarp->childDTypep(), BasicDType);
@ -949,13 +962,6 @@ class LinkDotFindVisitor : public AstNVisitor {
newdtypep->unlinkFrBack();
findvarp->childDTypep(newdtypep);
}
} else {
nodep->v3error("Duplicate declaration of signal: "
<<nodep->prettyName()<<endl
<<findvarp->warnMore()<<"... Location of original declaration");
// Combining most likely reduce other errors
findvarp->combineType(nodep);
findvarp->fileline()->modifyStateInherit(nodep->fileline());
}
nodep->unlinkFrBack()->deleteTree(); VL_DANGLING(nodep);
} else {
@ -1221,6 +1227,10 @@ private:
} else if (!refp->isIO() && !refp->isIfaceRef()) {
nodep->v3error("Pin is not an in/out/inout/interface: "<<nodep->prettyName());
} else {
if (refp->user4()) {
nodep->v3error("Duplicate declaration of port: "<<nodep->prettyName()<<endl
<<refp->warnMore()<<"... Location of original declaration");
}
refp->user4(true);
VSymEnt* symp = m_statep->insertSym(m_statep->getNodeSym(m_modp),
"__pinNumber"+cvtToStr(nodep->pinNum()),

View File

@ -166,6 +166,8 @@ AstVar* V3ParseGrammar::createVariable(FileLine* fileline, string name,
AstVar* nodep = new AstVar(fileline, type, name, VFlagChildDType(), arrayDTypep);
nodep->addAttrsp(attrsp);
nodep->ansi(m_pinAnsi);
nodep->declTyped(m_varDeclTyped);
if (GRAMMARP->m_varDecl != AstVarType::UNKNOWN) nodep->combineType(GRAMMARP->m_varDecl);
if (GRAMMARP->m_varIO != VDirection::NONE) {
nodep->declDirection(GRAMMARP->m_varIO);

View File

@ -52,12 +52,14 @@ class V3ParseGrammar {
public:
bool m_impliedDecl; // Allow implied wire declarations
AstVarType m_varDecl; // Type for next signal declaration (reg/wire/etc)
bool m_varDeclTyped; // Var got reg/wire for dedup check
VDirection m_varIO; // Direction for next signal declaration (reg/wire/etc)
AstVar* m_varAttrp; // Current variable for attribute adding
AstRange* m_gateRangep; // Current range for gate declarations
AstCase* m_caseAttrp; // Current case statement for attribute adding
AstNodeDType* m_varDTypep; // Pointer to data type for next signal declaration
AstNodeDType* m_memDTypep; // Pointer to data type for next member declaration
bool m_pinAnsi; // In ANSI port list
int m_pinNum; // Pin number currently parsing
string m_instModule; // Name of module referenced for instantiations
AstPin* m_instParamp; // Parameters for instantiations
@ -69,10 +71,12 @@ public:
V3ParseGrammar() {
m_impliedDecl = false;
m_varDecl = AstVarType::UNKNOWN;
m_varDeclTyped = false;
m_varIO = VDirection::NONE;
m_varDTypep = NULL;
m_gateRangep = NULL;
m_memDTypep = NULL;
m_pinAnsi = false;
m_pinNum = -1;
m_instModule = "";
m_instParamp = NULL;
@ -115,6 +119,12 @@ public:
fl->v3warn(ENDLABEL,"End label '"<<*endnamep<<"' does not match begin label '"<<name<<"'");
}
}
void setVarDecl(AstVarType type) {
m_varDecl = type;
if (type != AstVarType::UNKNOWN && type != AstVarType::PORT) {
m_varDeclTyped = true;
}
}
void setDType(AstNodeDType* dtypep) {
if (m_varDTypep) { m_varDTypep->deleteTree(); m_varDTypep=NULL; } // It was cloned, so this is safe.
m_varDTypep = dtypep;
@ -181,12 +191,16 @@ int V3ParseGrammar::s_modTypeImpNum = 0;
#define CRELINE() (PARSEP->copyOrSameFileLine()) // Only use in empty rules, so lines point at beginnings
#define VARRESET_LIST(decl) { GRAMMARP->m_pinNum=1; VARRESET(); VARDECL(decl); } // Start of pinlist
#define VARRESET_NONLIST(decl) { GRAMMARP->m_pinNum=0; VARRESET(); VARDECL(decl); } // Not in a pinlist
#define VARRESET() { VARDECL(UNKNOWN); VARIO(NONE); VARDTYPE(NULL); }
#define VARDECL(type) { GRAMMARP->m_varDecl = AstVarType::type; }
#define VARRESET_LIST(decl) { GRAMMARP->m_pinNum=1; GRAMMARP->m_pinAnsi=false; \
VARRESET(); VARDECL(decl); } // Start of pinlist
#define VARRESET_NONLIST(decl) { GRAMMARP->m_pinNum=0; GRAMMARP->m_pinAnsi=false; \
VARRESET(); VARDECL(decl); } // Not in a pinlist
#define VARRESET() { VARDECL(UNKNOWN); VARIO(NONE); VARDTYPE_NDECL(NULL); \
GRAMMARP->m_varDeclTyped = false; }
#define VARDECL(type) { GRAMMARP->setVarDecl(AstVarType::type); }
#define VARIO(type) { GRAMMARP->m_varIO = VDirection::type; }
#define VARDTYPE(dtypep) { GRAMMARP->setDType(dtypep); }
#define VARDTYPE(dtypep) { GRAMMARP->setDType(dtypep); GRAMMARP->m_varDeclTyped = true; }
#define VARDTYPE_NDECL(dtypep) { GRAMMARP->setDType(dtypep); } // Port that is range or signed only (not a decl)
#define VARDONEA(fl,name,array,attrs) GRAMMARP->createVariable((fl),(name),(array),(attrs))
#define VARDONEP(portp,array,attrs) GRAMMARP->createVariable((portp)->fileline(),(portp)->name(),(array),(attrs))
@ -981,9 +995,9 @@ port<nodep>: // ==IEEE: port
| portDirNetE yVAR implicit_typeE portSig variable_dimensionListE sigAttrListE
{ $$=$4; VARDTYPE($3); $$->addNextNull(VARDONEP($$,$5,$6)); }
| portDirNetE signing portSig variable_dimensionListE sigAttrListE
{ $$=$3; VARDTYPE(new AstBasicDType($3->fileline(), LOGIC_IMPLICIT, $2)); $$->addNextNull(VARDONEP($$,$4,$5)); }
{ $$=$3; VARDTYPE_NDECL(new AstBasicDType($3->fileline(), LOGIC_IMPLICIT, $2)); $$->addNextNull(VARDONEP($$,$4,$5)); }
| portDirNetE signingE rangeList portSig variable_dimensionListE sigAttrListE
{ $$=$4; VARDTYPE(GRAMMARP->addRange(new AstBasicDType($3->fileline(), LOGIC_IMPLICIT, $2), $3,true)); $$->addNextNull(VARDONEP($$,$5,$6)); }
{ $$=$4; VARDTYPE_NDECL(GRAMMARP->addRange(new AstBasicDType($3->fileline(), LOGIC_IMPLICIT, $2), $3,true)); $$->addNextNull(VARDONEP($$,$5,$6)); }
| portDirNetE /*implicit*/ portSig variable_dimensionListE sigAttrListE
{ $$=$2; /*VARDTYPE-same*/ $$->addNextNull(VARDONEP($$,$3,$4)); }
//
@ -1001,8 +1015,8 @@ portDirNetE: // IEEE: part of port, optional net type and/or direction
/* empty */ { }
// // Per spec, if direction given default the nettype.
// // The higher level rule may override this VARDTYPE with one later in the parse.
| port_direction { VARDECL(PORT); VARDTYPE(NULL/*default_nettype*/); }
| port_direction { VARDECL(PORT); } net_type { VARDTYPE(NULL/*default_nettype*/); } // net_type calls VARDECL
| port_direction { VARDECL(PORT); VARDTYPE_NDECL(NULL/*default_nettype*/); }
| port_direction { VARDECL(PORT); } net_type { VARDTYPE_NDECL(NULL/*default_nettype*/); } // net_type calls VARDECL
| net_type { } // net_type calls VARDECL
;
@ -1327,11 +1341,12 @@ varLParamReset:
port_direction: // ==IEEE: port_direction + tf_port_direction
// // IEEE 19.8 just "input" FIRST forces type to wire - we'll ignore that here
yINPUT { VARIO(INPUT); }
| yOUTPUT { VARIO(OUTPUT); }
| yINOUT { VARIO(INOUT); }
| yREF { VARIO(REF); }
| yCONST__REF yREF { VARIO(CONSTREF); }
// // Only used for ANSI declarations
yINPUT { GRAMMARP->m_pinAnsi=true; VARIO(INPUT); }
| yOUTPUT { GRAMMARP->m_pinAnsi=true; VARIO(OUTPUT); }
| yINOUT { GRAMMARP->m_pinAnsi=true; VARIO(INOUT); }
| yREF { GRAMMARP->m_pinAnsi=true; VARIO(REF); }
| yCONST__REF yREF { GRAMMARP->m_pinAnsi=true; VARIO(CONSTREF); }
;
port_directionReset: // IEEE: port_direction that starts a port_declaraiton
@ -1344,7 +1359,7 @@ port_directionReset: // IEEE: port_direction that starts a port_declaraiton
;
port_declaration<nodep>: // ==IEEE: port_declaration
// // Used inside block; followed by ';'
// // Non-ANSI; used inside block followed by ';'
// // SIMILAR to tf_port_declaration
//
// // IEEE: inout_declaration
@ -1357,11 +1372,11 @@ port_declaration<nodep>: // ==IEEE: port_declaration
list_of_variable_decl_assignments { $$ = $6; }
| port_directionReset port_declNetE yVAR implicit_typeE { VARDTYPE($4); }
list_of_variable_decl_assignments { $$ = $6; }
| port_directionReset port_declNetE signingE rangeList { VARDTYPE(GRAMMARP->addRange(new AstBasicDType($4->fileline(), LOGIC_IMPLICIT, $3),$4,true)); }
| port_directionReset port_declNetE signingE rangeList { VARDTYPE_NDECL(GRAMMARP->addRange(new AstBasicDType($4->fileline(), LOGIC_IMPLICIT, $3), $4, true)); }
list_of_variable_decl_assignments { $$ = $6; }
| port_directionReset port_declNetE signing { VARDTYPE(new AstBasicDType($<fl>3, LOGIC_IMPLICIT, $3)); }
| port_directionReset port_declNetE signing { VARDTYPE_NDECL(new AstBasicDType($<fl>3, LOGIC_IMPLICIT, $3)); }
list_of_variable_decl_assignments { $$ = $5; }
| port_directionReset port_declNetE /*implicit*/ { VARDTYPE(NULL);/*default_nettype*/}
| port_directionReset port_declNetE /*implicit*/ { VARDTYPE_NDECL(NULL);/*default_nettype*/}
list_of_variable_decl_assignments { $$ = $4; }
// // IEEE: interface_declaration
// // Looks just like variable declaration unless has a period
@ -1373,7 +1388,7 @@ tf_port_declaration<nodep>: // ==IEEE: tf_port_declaration
// // SIMILAR to port_declaration
//
port_directionReset data_type { VARDTYPE($2); } list_of_tf_variable_identifiers ';' { $$ = $4; }
| port_directionReset implicit_typeE { VARDTYPE($2); } list_of_tf_variable_identifiers ';' { $$ = $4; }
| port_directionReset implicit_typeE { VARDTYPE_NDECL($2); } list_of_tf_variable_identifiers ';' { $$ = $4; }
| port_directionReset yVAR data_type { VARDTYPE($3); } list_of_tf_variable_identifiers ';' { $$ = $5; }
| port_directionReset yVAR implicit_typeE { VARDTYPE($3); } list_of_tf_variable_identifiers ';' { $$ = $5; }
;

16
test_regress/t/t_var_dup2.pl Executable file
View File

@ -0,0 +1,16 @@
#!/usr/bin/perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003 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.
scenarios(linter => 1);
lint(
);
ok(1);
1;

View File

@ -0,0 +1,14 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2019 by Wilson Snyder.
// Legal with ANSI Verilog 2001 style ports
module t
(
output wire ok_ow,
output reg ok_or);
wire ok_o_w;
reg ok_o_r;
endmodule

View File

@ -0,0 +1,6 @@
%Error: t/t_var_dup2_bad.v:12: Duplicate declaration of signal: bad_o_w
t/t_var_dup2_bad.v:9: ... Location of original declaration
t/t_var_dup2_bad.v:9: ... Note: ANSI ports must have type declared with the I/O (IEEE 2017 23.2.2.2)
%Error: t/t_var_dup2_bad.v:13: Duplicate declaration of signal: bad_o_r
t/t_var_dup2_bad.v:10: ... Location of original declaration
%Error: Exiting due to

View File

@ -0,0 +1,18 @@
#!/usr/bin/perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003 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.
scenarios(linter => 1);
lint(
fails => 1,
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

View File

@ -0,0 +1,14 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2019 by Wilson Snyder.
// Illegal with ANSI Verilog 2001 style ports
module t
(
output bad_o_w,
output bad_o_r);
wire bad_o_w;
reg bad_o_r;
endmodule

16
test_regress/t/t_var_dup3.pl Executable file
View File

@ -0,0 +1,16 @@
#!/usr/bin/perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003 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.
scenarios(linter => 1);
lint(
);
ok(1);
1;

View File

@ -0,0 +1,28 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2019 by Wilson Snyder.
// Legal with Verilog 1995 style ports
module t
(/*AUTOARG*/
// Outputs
ok_o_w, ok_o_r, ok_o_ra, ok_or, ok_ow, ok_owa
);
output ok_o_w;
wire ok_o_w;
output ok_o_r;
reg ok_o_r;
output [1:0] ok_o_ra;
reg [1:0] ok_o_ra;
output reg ok_or;
output wire ok_ow;
output wire [1:0] ok_owa;
endmodule

View File

@ -1,16 +1,36 @@
%Error: t/t_var_dup_bad.v:14: Duplicate declaration of signal: a
t/t_var_dup_bad.v:13: ... Location of original declaration
%Error: t/t_var_dup_bad.v:17: Duplicate declaration of signal: l
t/t_var_dup_bad.v:16: ... Location of original declaration
%Error: t/t_var_dup_bad.v:20: Duplicate declaration of signal: b
t/t_var_dup_bad.v:19: ... Location of original declaration
%Error: t/t_var_dup_bad.v:26: Duplicate declaration of signal: o
t/t_var_dup_bad.v:25: ... Location of original declaration
%Error: t/t_var_dup_bad.v:29: Duplicate declaration of signal: i
t/t_var_dup_bad.v:28: ... Location of original declaration
%Error: t/t_var_dup_bad.v:32: Duplicate declaration of signal: oi
t/t_var_dup_bad.v:31: ... Location of original declaration
%Error: t/t_var_dup_bad.v:39: Duplicate declaration of signal: org
t/t_var_dup_bad.v:38: ... Location of original declaration
%Error: t/t_var_dup_bad.v:31: Input/output/inout does not appear in port list: oi
%Error: t/t_var_dup_bad.v:16: Duplicate declaration of signal: a
t/t_var_dup_bad.v:15: ... Location of original declaration
%Error: t/t_var_dup_bad.v:19: Duplicate declaration of signal: l
t/t_var_dup_bad.v:18: ... Location of original declaration
%Error: t/t_var_dup_bad.v:22: Duplicate declaration of signal: b
t/t_var_dup_bad.v:21: ... Location of original declaration
%Error: t/t_var_dup_bad.v:25: Duplicate declaration of signal: o
t/t_var_dup_bad.v:24: ... Location of original declaration
%Error: t/t_var_dup_bad.v:28: Duplicate declaration of signal: i
t/t_var_dup_bad.v:27: ... Location of original declaration
%Error: t/t_var_dup_bad.v:31: Duplicate declaration of signal: oi
t/t_var_dup_bad.v:30: ... Location of original declaration
%Error: t/t_var_dup_bad.v:38: Duplicate declaration of signal: org
t/t_var_dup_bad.v:37: ... Location of original declaration
%Error: t/t_var_dup_bad.v:65: Duplicate declaration of signal: bad_reout_port
t/t_var_dup_bad.v:63: ... Location of original declaration
%Error: t/t_var_dup_bad.v:72: Duplicate declaration of signal: bad_rewire
t/t_var_dup_bad.v:69: ... Location of original declaration
%Error: t/t_var_dup_bad.v:73: Duplicate declaration of signal: bad_rereg
t/t_var_dup_bad.v:70: ... Location of original declaration
%Error: t/t_var_dup_bad.v:12: Duplicate declaration of port: oi
t/t_var_dup_bad.v:30: ... Location of original declaration
%Error: t/t_var_dup_bad.v:49: Duplicate declaration of port: bad_duport
t/t_var_dup_bad.v:51: ... Location of original declaration
%Error: t/t_var_dup_bad.v:57: Duplicate declaration of port: bad_mixport
t/t_var_dup_bad.v:57: ... Location of original declaration
%Error: t/t_var_dup_bad.v:40: Can't find definition of variable: bad_duport
%Error: t/t_var_dup_bad.v:40: Duplicate pin connection: bad_duport
t/t_var_dup_bad.v:40: ... Location of original pin connection
%Error: t/t_var_dup_bad.v:41: Can't find definition of variable: bad_mixport
%Error: t/t_var_dup_bad.v:41: Duplicate pin connection: bad_mixport
t/t_var_dup_bad.v:41: ... Location of original pin connection
%Error: t/t_var_dup_bad.v:42: Can't find definition of variable: bad_reout_port
%Error: t/t_var_dup_bad.v:43: Can't find definition of variable: bad_rewire
%Error: t/t_var_dup_bad.v:43: Can't find definition of variable: bad_rereg
%Error: Exiting due to

View File

@ -3,11 +3,13 @@
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2007 by Wilson Snyder.
module t (/*AUTOARG*/
module t
(
/*AUTOARG*/
// Outputs
ok, o, og, org,
o, oi, og, org,
// Inputs
i
i, oi
);
reg a;
@ -19,9 +21,6 @@ module t (/*AUTOARG*/
bit b;
bit b;
output ok;
reg ok;
output o;
output o;
@ -38,4 +37,38 @@ module t (/*AUTOARG*/
output reg org;
output reg org;
sub0 sub0(.*);
sub1 sub1(.*);
sub2 sub2(.*);
sub3 sub3(.*);
endmodule
module sub0
(
bad_duport,
bad_duport
);
output bad_duport;
endmodule
module sub1
(
bad_mixport,
output bad_mixport
);
endmodule
module sub2
(
output bad_reout_port
);
output bad_reout_port;
endmodule
module sub3
(output wire bad_rewire,
output reg bad_rereg
);
wire bad_rewire;
reg bad_rereg;
endmodule