luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add CASEZWITHX lint warning and if disabled fix handling of casez with Xs.

This commit is contained in:
Wilson Snyder 2008-07-22 13:07:19 -04:00
parent fb34bf7222
commit 9e5fb5467f
14 changed files with 238 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Changes
View File

@ -10,6 +10,8 @@ indicates the contributor was also the author of the fix; Thanks!
** Add --autoflush, for flushing streams after $display. [Steve Tong]
** Add CASEZWITHX lint warning and if disabled fix handling of casez with Xs.
*** Add $feof, $fgetc, $fgets, $fflush, $fscanf, $sscanf. [Holger Waechtler]
*** Add $stime. [Holger Waechtler]

16
bin/verilator
View File

@ -563,9 +563,9 @@ Disable the specified warning message.
=item -Wno-lint
Disable all lint related warning messages. This is equivelent to
"-Wno-CASEINCOMPLETE -Wno-CASEOVERLAP -Wno-CASEX -Wno-CMPCONST
-Wno-IMPLICIT -Wno-UNDRIVEN -Wno-UNSIGNED -Wno-UNUSED -Wno-VARHIDDEN
-Wno-WIDTH".
"-Wno-CASEINCOMPLETE -Wno-CASEOVERLAP -Wno-CASEX -Wno-CASEZWITHX
-Wno-CMPCONST -Wno-IMPLICIT -Wno-UNDRIVEN -Wno-UNSIGNED -Wno-UNUSED
-Wno-VARHIDDEN -Wno-WIDTH".
It is strongly recommended you cleanup your code rather than using this
option, it is only intended to be use when running test-cases of code
@ -1676,6 +1676,16 @@ L<http://www.sunburst-design.com/papers/CummingsSNUG1999Boston_FullParallelCase_
Ignoring this warning will only suppress the lint check, it will simulate
correctly.
=item CASEWITHX
Warns that a case statement contains a constant with a C<x>. Verilator is
two-state so interpret such items as always false. Note a common error is
to use a C<X> in a case or casez statement item; often what the user
instead insteaded is to use a casez with C<?>.
Ignoring this warning will only suppress the lint check, it will simulate
correctly.
=item CMPCONST
Warns that you are comparing a value in a way that will always be constant.

19
src/V3Ast.h
View File

@ -251,6 +251,25 @@ public:
//######################################################################
class AstCaseType {
public:
enum en {
CASE,
CASEX,
CASEZ
};
enum en m_e;
inline AstCaseType () {};
inline AstCaseType (en _e) : m_e(_e) {};
explicit inline AstCaseType (int _e) : m_e(static_cast<en>(_e)) {};
operator en () const { return m_e; };
};
inline bool operator== (AstCaseType lhs, AstCaseType rhs) { return (lhs.m_e == rhs.m_e); }
inline bool operator== (AstCaseType lhs, AstCaseType::en rhs) { return (lhs.m_e == rhs); }
inline bool operator== (AstCaseType::en lhs, AstCaseType rhs) { return (lhs == rhs.m_e); }
//######################################################################
class AstDisplayType {
public:
enum en {

13
src/V3AstNodes.h
View File

@ -1195,11 +1195,11 @@ struct AstCase : public AstNodeCase {
// exprp Children: MATHs
// casesp Children: CASEITEMs
private:
bool m_casex; // True if casex instead of normal case.
AstCaseType m_casex; // 0=case, 1=casex, 2=casez
bool m_fullPragma; // Synthesis full_case
bool m_parallelPragma; // Synthesis parallel_case
public:
AstCase(FileLine* fileline, bool casex, AstNode* exprp, AstNode* casesp)
AstCase(FileLine* fileline, AstCaseType casex, AstNode* exprp, AstNode* casesp)
: AstNodeCase(fileline, exprp, casesp) {
m_casex=casex;
m_fullPragma=false; m_parallelPragma=false;
@ -1208,11 +1208,14 @@ public:
virtual AstType type() const { return AstType::CASE;}
virtual AstNode* clone() { return new AstCase(*this); }
virtual void accept(AstNVisitor& v, AstNUser* vup=NULL) { v.visit(this,vup); }
virtual string verilogKwd() const { return casex()?"casex":"case"; }
bool casex() const { return m_casex; }
virtual string verilogKwd() const { return casez()?"casez":casex()?"casex":"case"; }
virtual bool same(AstNode* samep) const {
return m_casex==samep->castCase()->m_casex; }
bool casex() const { return m_casex==AstCaseType::CASEX; }
bool casez() const { return m_casex==AstCaseType::CASEZ; }
bool fullPragma() const { return m_fullPragma; }
bool parallelPragma() const { return m_parallelPragma; }
void fullPragma(bool flag) { m_fullPragma=flag; }
bool parallelPragma() const { return m_parallelPragma; }
void parallelPragma(bool flag) { m_parallelPragma=flag; }
};

65
src/V3Case.cpp
View File

@ -73,7 +73,7 @@ private:
}
// Check for X/Z in non-casex statements
if (!nodep->castCase() || !nodep->castCase()->casex()) {
{
m_caseExprp = nodep;
nodep->exprp()->accept(*this);
for (AstCaseItem* itemp = nodep->itemsp(); itemp; itemp=itemp->nextp()->castCaseItem()) {
@ -83,11 +83,18 @@ private:
}
}
virtual void visit(AstConst* nodep, AstNUser*) {
// See also neverItem
if (m_caseExprp && nodep->num().isFourState()) {
if (m_caseExprp->castGenCase()) {
nodep->v3error("Use of x/? constant in generate case statement, (no such thing as 'generate casez')");
} else if (m_caseExprp->castCase() && m_caseExprp->castCase()->casex()) {
// Don't sweat it, we already complained about casex in general
} else if (m_caseExprp->castCase() && m_caseExprp->castCase()->casez()) {
if (nodep->num().isUnknown()) {
nodep->v3warn(CASEWITHX, "Use of x constant in casez statement, (perhaps intended ?/z in constant)");
}
} else {
nodep->v3error("Use of x/? constant in case statement, (perhaps intended casex/casez)");
nodep->v3warn(CASEWITHX, "Use of x/? constant in case statement, (perhaps intended casex/casez)");
}
}
}
@ -152,20 +159,24 @@ private:
//if (debug()>=9) icondp->dumpTree(cout," caseitem: ");
AstConst* iconstp = icondp->castConst();
if (!iconstp) nodep->v3fatalSrc("above 'can't parse' should have caught this\n");
V3Number nummask (itemp->fileline(), iconstp->width());
nummask.opBitsNonX(iconstp->num());
uint32_t mask = nummask.asInt();
V3Number numval (itemp->fileline(), iconstp->width());
numval.opBitsOne(iconstp->num());
uint32_t val = numval.asInt();
for (uint32_t i=0; i<(1UL<<m_caseWidth); i++) {
if ((i & mask) == val) {
if (!m_valueItem[i]) {
m_valueItem[i] = itemp;
} else if (!itemp->ignoreOverlap() && !bitched) {
itemp->v3warn(CASEOVERLAP,"Case values overlap (example pattern 0x"<<hex<<i<<")");
bitched = true;
m_caseNoOverlapsAllCovered = false;
if (neverItem(nodep, iconstp)) {
// X in casez can't ever be executed
} else {
V3Number nummask (itemp->fileline(), iconstp->width());
nummask.opBitsNonX(iconstp->num());
uint32_t mask = nummask.asInt();
V3Number numval (itemp->fileline(), iconstp->width());
numval.opBitsOne(iconstp->num());
uint32_t val = numval.asInt();
for (uint32_t i=0; i<(1UL<<m_caseWidth); i++) {
if ((i & mask) == val) {
if (!m_valueItem[i]) {
m_valueItem[i] = itemp;
} else if (!itemp->ignoreOverlap() && !bitched) {
itemp->v3warn(CASEOVERLAP,"Case values overlap (example pattern 0x"<<hex<<i<<")");
bitched = true;
m_caseNoOverlapsAllCovered = false;
}
}
}
}
@ -296,8 +307,15 @@ private:
AstNode* and1p;
AstNode* and2p;
AstConst* iconstp = icondp->castConst();
if (iconstp && iconstp->num().isFourState()
&& nodep->casex()) {
if (iconstp && neverItem(nodep, iconstp)) {
// X in casez can't ever be executed
icondp->deleteTree(); icondp=NULL; iconstp=NULL;
// For simplicity, make expression that is not equal, and let later
// optimizations remove it
and1p = new AstConst(itemp->fileline(), V3Number(itemp->fileline(),1,0));
and2p = new AstConst(itemp->fileline(), V3Number(itemp->fileline(),1,1));
} else if (iconstp && iconstp->num().isFourState()
&& (nodep->casex() || nodep->casez())) {
V3Number nummask (itemp->fileline(), iconstp->width());
nummask.opBitsNonX(iconstp->num());
V3Number numval (itemp->fileline(), iconstp->width());
@ -394,6 +412,17 @@ private:
}
}
bool neverItem(AstCase* casep, AstConst* itemp) {
// Xs in case or casez are impossible due to two state simulations
if (casep->casex()) {
} else if (casep->casez()) {
if (itemp->num().isUnknown()) return true;
} else {
if (itemp->num().isFourState()) return true;
}
return false;
}
// VISITORS
virtual void visit(AstCase* nodep, AstNUser*) {
V3Case::caseLint(nodep);

6
src/V3Error.h
View File

@ -46,6 +46,7 @@ public:
BLKANDNBLK, // Blocked and non-blocking assignments to same variable
CASEINCOMPLETE, // Case statement has missing values
CASEOVERLAP, // Case statements overlap
CASEWITHX, // Case with X values
CASEX, // Casex
CMPCONST, // Comparison is constant due to limited range
COMBDLY, // Combinatorial delayed assignment
@ -79,7 +80,7 @@ public:
"MULTITOP", "TASKNSVAR",
" FIRST_WARN",
"BLKANDNBLK",
"CASEINCOMPLETE", "CASEOVERLAP", "CASEX", "CMPCONST",
"CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CMPCONST",
"COMBDLY", "STMTDLY", "GENCLK", "IMPLICIT", "IMPURE",
"MULTIDRIVEN", "REDEFMACRO",
"UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED",
@ -95,7 +96,8 @@ public:
bool pretendError() const { return ( m_e==BLKANDNBLK || m_e==IMPURE); };
// Warnings that are lint only
bool lintError() const { return ( m_e==CASEINCOMPLETE || m_e==CASEOVERLAP
|| m_e==CASEX || m_e==CMPCONST
|| m_e==CASEWITHX || m_e==CASEX
|| m_e==CMPCONST
|| m_e==IMPLICIT
|| m_e==UNDRIVEN || m_e==UNSIGNED
|| m_e==UNUSED || m_e==VARHIDDEN

30
src/V3Number.cpp
View File

@ -143,14 +143,14 @@ V3Number::V3Number (FileLine* fileline, const char* sourcep) {
olen++;
break;
}
case 'z': {
case 'z': case '?': {
if (olen) m_fileline->v3error("Multi-digit X/Z/? not legal in decimal constant: "<<*cp);
if (!m_sized) m_fileline->v3error("Unsized X/Z/? not legal in decimal constant: "<<*cp);
olen++;
setAllBitsZ();
break;
}
case 'x': case '?': {
case 'x': {
if (olen) m_fileline->v3error("Multi-digit X/Z/? not legal in decimal constant: "<<*cp);
if (!m_sized) m_fileline->v3error("Unsized X/Z/? not legal in decimal constant: "<<*cp);
olen++;
@ -181,9 +181,8 @@ V3Number::V3Number (FileLine* fileline, const char* sourcep) {
switch(tolower(*cp)) {
case '0': setBit(obit++, 0); break;
case '1': setBit(obit++, 1); break;
case 'z': setBit(obit++, 'z'); break;
case 'x': case '?':
setBit(obit++, 'x'); break;
case 'z': case '?': setBit(obit++, 'z'); break;
case 'x': setBit(obit++, 'x'); break;
case '_': break;
default:
m_fileline->v3error("Illegal character in binary constant: "<<*cp);
@ -202,8 +201,9 @@ V3Number::V3Number (FileLine* fileline, const char* sourcep) {
case '5': setBit(obit++, 1); setBit(obit++, 0); setBit(obit++, 1); break;
case '6': setBit(obit++, 0); setBit(obit++, 1); setBit(obit++, 1); break;
case '7': setBit(obit++, 1); setBit(obit++, 1); setBit(obit++, 1); break;
case 'z': setBit(obit++, 'z'); setBit(obit++, 'z'); setBit(obit++, 'z'); break;
case 'x': case '?':
case 'z': case '?':
setBit(obit++, 'z'); setBit(obit++, 'z'); setBit(obit++, 'z'); break;
case 'x':
setBit(obit++, 'x'); setBit(obit++, 'x'); setBit(obit++, 'x'); break;
case '_': break;
default:
@ -230,8 +230,9 @@ V3Number::V3Number (FileLine* fileline, const char* sourcep) {
case 'd': setBit(obit++,1); setBit(obit++,0); setBit(obit++,1); setBit(obit++,1); break;
case 'e': setBit(obit++,0); setBit(obit++,1); setBit(obit++,1); setBit(obit++,1); break;
case 'f': setBit(obit++,1); setBit(obit++,1); setBit(obit++,1); setBit(obit++,1); break;
case 'z': setBit(obit++,'z'); setBit(obit++,'z'); setBit(obit++,'z'); setBit(obit++,'z'); break;
case 'x': case '?':
case 'z': case '?':
setBit(obit++,'z'); setBit(obit++,'z'); setBit(obit++,'z'); setBit(obit++,'z'); break;
case 'x':
setBit(obit++,'x'); setBit(obit++,'x'); setBit(obit++,'x'); setBit(obit++,'x'); break;
case '_': break;
default:
@ -522,6 +523,12 @@ bool V3Number::isEqAllOnes(int optwidth) const {
}
return true;
}
bool V3Number::isUnknown() const {
for(int bit=0; bit<width(); bit++) {
if (bitIsX(bit)) return true;
}
return false;
}
int V3Number::minWidth() const {
for(int bit=width()-1; bit>0; bit--) {
@ -625,10 +632,7 @@ V3Number& V3Number::opCountOnes (const V3Number& lhs) {
return *this;
}
V3Number& V3Number::opIsUnknown (const V3Number& lhs) {
for(int bit=0; bit<lhs.width(); bit++) {
if (lhs.bitIsX(bit)) return setSingleBits(1);
}
return setSingleBits(0);
return setSingleBits(lhs.isUnknown());
}
V3Number& V3Number::opOneHot (const V3Number& lhs) {
if (lhs.isFourState()) return setAllBitsX();

1
src/V3Number.h
View File

@ -134,6 +134,7 @@ public:
bool isCaseEq(const V3Number& rhsp) const; // operator==
void width(int width, bool sized=true);
void isSigned(bool ssigned) { m_signed=ssigned; }
bool isUnknown() const;
uint32_t asInt() const;
vlsint32_t asSInt() const;
vluint64_t asQuad() const;

6
src/verilog.y
View File

@ -952,9 +952,9 @@ stateCaseForIf: caseStmt caseAttrE caseListE yENDCASE { $$ = $1; if ($3) $1->add
| yDO stmtBlock yWHILE '(' expr ')' { $$ = $2->cloneTree(true); $$->addNext(new AstWhile($1,$5,$2));}
;
caseStmt: yCASE '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,false,$3,NULL); }
| yCASEX '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,true,$3,NULL); $1->v3warn(CASEX,"Suggest casez (with ?'s) in place of casex (with X's)\n"); }
| yCASEZ '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,true,$3,NULL); }
caseStmt: yCASE '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,AstCaseType::CASE,$3,NULL); }
| yCASEX '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,AstCaseType::CASEX,$3,NULL); $1->v3warn(CASEX,"Suggest casez (with ?'s) in place of casex (with X's)\n"); }
| yCASEZ '(' expr ')' { $$ = V3Parse::s_caseAttrp = new AstCase($1,AstCaseType::CASEZ,$3,NULL); }
;
caseAttrE: /*empty*/ { }

18
test_regress/t/t_case_x.pl Executable file
View File

@ -0,0 +1,18 @@
#!/usr/bin/perl
if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003-2007 by Wilson Snyder. This program is free software; you can
# redistribute it and/or modify it under the terms of either the GNU
# General Public License or the Perl Artistic License.
compile (
v_flags2 => ["--x-assign 0"],
);
execute (
check_finished=>1,
);
ok(1);
1;

65
test_regress/t/t_case_x.v Normal file
View File

@ -0,0 +1,65 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2005-2007 by Wilson Snyder.
module t (/*AUTOARG*/);
reg [3:0] value;
reg [3:0] valuex;
// verilator lint_off CASEOVERLAP
// verilator lint_off CASEWITHX
// verilator lint_off CASEX
// Note for Verilator Xs must become zeros, or the Xs may match.
initial begin
value = 4'b1001;
valuex = 4'b1xxx;
case (value)
4'b1xxx: $stop;
4'b1???: $stop;
4'b1001: ;
default: $stop;
endcase
case (valuex)
4'b1???: $stop;
4'b1xxx: ;
4'b1001: ;
4'b1000: ; // 1xxx is mapped to this by Verilator -x-assign 0
default: $stop;
endcase
//
casex (value)
4'b100x: ;
default: $stop;
endcase
casex (value)
4'b100?: ;
default: $stop;
endcase
casex (valuex)
4'b100x: ;
default: $stop;
endcase
casex (valuex)
4'b100?: ;
default: $stop;
endcase
//
casez (value)
4'bxxxx: $stop;
4'b100?: ;
default: $stop;
endcase
casez (valuex)
4'b1xx?: ;
4'b100?: ; // 1xxx is mapped to this by Verilator -x-assign 0
default: $stop;
endcase
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

3
test_regress/t/t_case_x_bad.pl
View File

@ -10,7 +10,8 @@ compile (
v_flags2 => ["--lint-only"],
fails=>1,
expect=>
'%Error: t/t_case_x_bad.v:\d+: Use of x/. constant in case statement, \(perhaps intended casex/casez\)
'%Warning-CASEWITHX: t/t_case_x_bad.v:\d+: Use of x/\? constant in case statement, \(perhaps intended casex/casez\)
.*
%Error: Exiting due to.*',
);

19
test_regress/t/t_case_zx_bad.pl Executable file
View File

@ -0,0 +1,19 @@
#!/usr/bin/perl
if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003-2007 by Wilson Snyder. This program is free software; you can
# redistribute it and/or modify it under the terms of either the GNU
# General Public License or the Perl Artistic License.
compile (
v_flags2 => ["--lint-only"],
fails=>1,
expect=>
'%Warning-CASEWITHX: t/t_case_zx_bad.v:\d+: Use of x constant in casez statement, \(perhaps intended \?/z in constant\)
.*
%Error: Exiting due to.*',
);
ok(1);
1;

20
test_regress/t/t_case_zx_bad.v Normal file
View File

@ -0,0 +1,20 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2005-2007 by Wilson Snyder.
module t (/*AUTOARG*/
// Inputs
value
);
input [3:0] value;
always @ (/*AS*/value) begin
casez (value)
4'b0000: $stop;
4'b1xxx: $stop;
default: $stop;
endcase
end
endmodule