From 9e5fb5467f534571b1950cfda029eb8134555460 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 22 Jul 2008 13:07:19 -0400 Subject: [PATCH] Add CASEZWITHX lint warning and if disabled fix handling of casez with Xs. --- Changes | 2 + bin/verilator | 16 ++++++-- src/V3Ast.h | 19 ++++++++++ src/V3AstNodes.h | 13 ++++--- src/V3Case.cpp | 65 ++++++++++++++++++++++++--------- src/V3Error.h | 6 ++- src/V3Number.cpp | 30 ++++++++------- src/V3Number.h | 1 + src/verilog.y | 6 +-- test_regress/t/t_case_x.pl | 18 +++++++++ test_regress/t/t_case_x.v | 65 +++++++++++++++++++++++++++++++++ test_regress/t/t_case_x_bad.pl | 3 +- test_regress/t/t_case_zx_bad.pl | 19 ++++++++++ test_regress/t/t_case_zx_bad.v | 20 ++++++++++ 14 files changed, 238 insertions(+), 45 deletions(-) create mode 100755 test_regress/t/t_case_x.pl create mode 100644 test_regress/t/t_case_x.v create mode 100755 test_regress/t/t_case_zx_bad.pl create mode 100644 test_regress/t/t_case_zx_bad.v diff --git a/Changes b/Changes index 163c81459..d474e0e8b 100644 --- a/Changes +++ b/Changes @@ -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] diff --git a/bin/verilator b/bin/verilator index f91fd48b2..d43d9a08e 100755 --- a/bin/verilator +++ b/bin/verilator @@ -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. Verilator is +two-state so interpret such items as always false. Note a common error is +to use a C 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. diff --git a/src/V3Ast.h b/src/V3Ast.h index b7e0f0c21..9580b80eb 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -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(_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 { diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index e32ad2775..9d6aea2e8 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -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; } }; diff --git a/src/V3Case.cpp b/src/V3Case.cpp index 4e5abc72f..d288e3de2 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -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<ignoreOverlap() && !bitched) { - itemp->v3warn(CASEOVERLAP,"Case values overlap (example pattern 0x"<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<ignoreOverlap() && !bitched) { + itemp->v3warn(CASEOVERLAP,"Case values overlap (example pattern 0x"<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); diff --git a/src/V3Error.h b/src/V3Error.h index 2b7d74b0b..d4e867751 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -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 diff --git a/src/V3Number.cpp b/src/V3Number.cpp index 21ef64400..563da725c 100644 --- a/src/V3Number.cpp +++ b/src/V3Number.cpp @@ -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; bit0; bit--) { @@ -625,10 +632,7 @@ V3Number& V3Number::opCountOnes (const V3Number& lhs) { return *this; } V3Number& V3Number::opIsUnknown (const V3Number& lhs) { - for(int bit=0; bitadd | 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*/ { } diff --git a/test_regress/t/t_case_x.pl b/test_regress/t/t_case_x.pl new file mode 100755 index 000000000..5a47a3bf7 --- /dev/null +++ b/test_regress/t/t_case_x.pl @@ -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; diff --git a/test_regress/t/t_case_x.v b/test_regress/t/t_case_x.v new file mode 100644 index 000000000..205261273 --- /dev/null +++ b/test_regress/t/t_case_x.v @@ -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 diff --git a/test_regress/t/t_case_x_bad.pl b/test_regress/t/t_case_x_bad.pl index 19dcf964b..f873379da 100755 --- a/test_regress/t/t_case_x_bad.pl +++ b/test_regress/t/t_case_x_bad.pl @@ -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.*', ); diff --git a/test_regress/t/t_case_zx_bad.pl b/test_regress/t/t_case_zx_bad.pl new file mode 100755 index 000000000..282e8adf5 --- /dev/null +++ b/test_regress/t/t_case_zx_bad.pl @@ -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; diff --git a/test_regress/t/t_case_zx_bad.v b/test_regress/t/t_case_zx_bad.v new file mode 100644 index 000000000..661602a0e --- /dev/null +++ b/test_regress/t/t_case_zx_bad.v @@ -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