From 850de5913d7a13a6c221fb0794cd44e034e140a9 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 26 Dec 2010 09:31:09 -0500 Subject: [PATCH] With --Wall, add IFDEPTH warning on deep if statements. --- Changes | 16 ++++++----- bin/verilator | 16 +++++++++++ src/V3Begin.cpp | 17 ++++++++++++ src/V3Error.h | 10 ++++--- src/V3Options.cpp | 5 ++++ src/V3Options.h | 2 ++ test_regress/t/t_lint_ifdepth_bad.pl | 24 +++++++++++++++++ test_regress/t/t_lint_ifdepth_bad.v | 40 ++++++++++++++++++++++++++++ 8 files changed, 119 insertions(+), 11 deletions(-) create mode 100755 test_regress/t/t_lint_ifdepth_bad.pl create mode 100644 test_regress/t/t_lint_ifdepth_bad.v diff --git a/Changes b/Changes index 62d9c2aae..8372f482e 100644 --- a/Changes +++ b/Changes @@ -7,24 +7,26 @@ indicates the contributor was also the author of the fix; Thanks! ** Add limited support for VPI access to public signals, see docs. -** Add -Wall, -Wwarn-style, -Wno-style to enable code style warnings - that have been added to this release, and disabled by default. - *** Add -F option to read relative option files, bug297. [Neil Hamilton] *** Support ++,--,+= etc as standalone statements. [Alex Solomatnikov] -*** Suppress WIDTH warnings when adding/subtracting 1'b1. +**** When running with VERILATOR_ROOT, optionally find binaries under bin. + +**** Suppress WIDTH warnings when adding/subtracting 1'b1. + +** Add -Wall, -Wwarn-style, -Wno-style to enable code style warnings + that have been added to this release, and disabled by default: *** With --Wall, add DECLFILENAME warning on modules not matching filename. *** With --Wall, add DEFPARAM warning on deprecated defparam statements. +*** With --Wall, add IFDEPTH warning on deep if statements. + *** With --Wall, add INCABSPATH warning on `include with absolute paths. -**** When running with VERILATOR_ROOT, optionally find binaries under bin. - -**** The VARHIDDEN warning is now disabled by default, use -Wall to enable. +*** The VARHIDDEN warning is now disabled by default, use -Wall to enable. * Verilator 3.805 2010/11/02 diff --git a/bin/verilator b/bin/verilator index a0dcce80b..130fb6222 100755 --- a/bin/verilator +++ b/bin/verilator @@ -255,6 +255,7 @@ descriptions in the next sections for more information. --gdbbt Run Verilator under GDB for backtrace --help Display this help -I Directory to search for includes + --if-depth Tune IFDEPTH warning +incdir+ Directory to search for includes --inhibit-sim Create function to turn off sim --inline-mult Tune module inlining @@ -558,6 +559,11 @@ include directories or libraries. Same as +incdir and -y; +incdir and -y are fairly standard across Verilog tools while -I is an alias for GCC compatibility. +=item --if-depth I + +Rarely needed. Set the depth at which the IFDEPTH warning will fire, +defaults to 0 which disables this warning. + =item +incdir+I Add the directory to the list of directories that should be searched for @@ -2420,6 +2426,16 @@ additional ones.) Ignoring this warning may make Verilator simulations differ from other simulators. +=item IFDEPTH + +Warns that if/if else statements have exceeded the depth specified with +--if-depth, as they are likely to result in slow priority encoders. Unique +and priority if statements are ignored. Solutions include changing the +code to a case statement, or a SystemVerilog 'unique if' or 'priority if'. + +Disabled by default as this is a code style warning, it will simulate +correctly. + =item IMPERFECTSCH Warns that the scheduling of the model is not absolutely perfect, and some diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 218389b25..911af0052 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -53,6 +53,7 @@ private: string m_namedScope; // Name of begin blocks above us string m_unnamedScope; // Name of begin blocks, including unnamed blocks int m_repeatNum; // Repeat counter + int m_ifDepth; // Current if depth // METHODS static int debug() { @@ -152,6 +153,21 @@ private: // any BEGINs, but V3Coverage adds them all under the module itself. nodep->iterateChildren(*this); } + // VISITORS - LINT CHECK + virtual void visit(AstIf* nodep, AstNUser*) { // Note not AstNodeIf; other types don't get covered + // Check IFDEPTH warning - could be in other transform files if desire + int prevIfDepth = m_ifDepth; + if (m_ifDepth == -1 || v3Global.opt.ifDepth()<1) { // Turned off + } else if (nodep->uniquePragma() || nodep->unique0Pragma() || nodep->priorityPragma()) { + m_ifDepth = -1; + } else if (++m_ifDepth > v3Global.opt.ifDepth()) { + nodep->v3warn(IFDEPTH,"Deep 'if' statement; suggest unique/priority to avoid slow logic"); + nodep->fileline()->warnOn(V3ErrorCode::IFDEPTH, false); // Warn only once + m_ifDepth = -1; + } + nodep->iterateChildren(*this); + m_ifDepth = prevIfDepth; + } virtual void visit(AstNode* nodep, AstNUser*) { nodep->iterateChildren(*this); } @@ -161,6 +177,7 @@ public: m_modp = NULL; m_ftaskp = NULL; m_repeatNum = 0; + m_ifDepth = 0; nodep->accept(*this); } virtual ~BeginVisitor() {} diff --git a/src/V3Error.h b/src/V3Error.h index 67527999e..7ba5f8c61 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -63,9 +63,8 @@ public: COMBDLY, // Combinatorial delayed assignment DEFPARAM, // Style: Defparam DECLFILENAME, // Declaration doesn't match filename - STMTDLY, // Delayed statement - SYMRSVDWORD, // Symbol is Reserved Word GENCLK, // Generated Clock + IFDEPTH, // If statements too deep IMPERFECTSCH, // Imperfect schedule (disabled by default) IMPLICIT, // Implicit wire IMPURE, // Impure function not being inlined @@ -74,6 +73,8 @@ public: MODDUP, // Duplicate module MULTIDRIVEN, // Driven from multiple blocks REDEFMACRO, // Redefining existing define macro + STMTDLY, // Delayed statement + SYMRSVDWORD, // Symbol is Reserved Word UNDRIVEN, // No drivers UNOPT, // Unoptimizable block UNOPTFLAT, // Unoptimizable block after flattening @@ -104,10 +105,11 @@ public: "BLKANDNBLK", "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CMPCONST", "COMBDLY", "DEFPARAM", "DECLFILENAME", - "STMTDLY", "SYMRSVDWORD", "GENCLK", "IMPERFECTSCH", "IMPLICIT", "IMPURE", - "INCABSPATH", + "GENCLK", + "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE", "INCABSPATH", "LITENDIAN", "MODDUP", "MULTIDRIVEN", "REDEFMACRO", + "STMTDLY", "SYMRSVDWORD", "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED", "VARHIDDEN", "WIDTH", "WIDTHCONCAT", " MAX" diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 50d1ea338..1f521ab67 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -705,6 +705,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char else if ( !strncmp (sw, "-I", 2)) { addIncDir (parseFileArg(optdir, string (sw+strlen("-I")))); } + else if ( !strcmp (sw, "-if-depth") && (i+1) ["--lint-only -Wall -Wno-DECLFILENAME --if-depth 10"], + fails=>1, + verilator_make_gcc => 0, + make_top_shell => 0, + make_main => 0, + expect=> +'%Warning-IFDEPTH: t/t_lint_ifdepth_bad.v:\d+: Deep \'if\' statement; suggest unique/priority to avoid slow logic +%Warning-IFDEPTH: Use .* to disable this message. +%Error: Exiting due to.*', + ) if $Self->{v3}; + +ok(1); +1; + diff --git a/test_regress/t/t_lint_ifdepth_bad.v b/test_regress/t/t_lint_ifdepth_bad.v new file mode 100644 index 000000000..3ce9351d3 --- /dev/null +++ b/test_regress/t/t_lint_ifdepth_bad.v @@ -0,0 +1,40 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2010 by Wilson Snyder. + +module t; + + integer v = 19; + + initial begin + if (v==1) begin end + else if (v==2) begin end + else if (v==3) begin end + else if (v==4) begin end + else if (v==5) begin end + else if (v==6) begin end + else if (v==7) begin end + else if (v==8) begin end + else if (v==9) begin end + else if (v==10) begin end + else if (v==11) begin end // Warn about this one + else if (v==12) begin end + end + + initial begin + unique0 if (v==1) begin end + else if (v==2) begin end + else if (v==3) begin end + else if (v==4) begin end + else if (v==5) begin end + else if (v==6) begin end + else if (v==7) begin end + else if (v==8) begin end + else if (v==9) begin end + else if (v==10) begin end + else if (v==11) begin end // Warn about this one + else if (v==12) begin end + end + +endmodule