With --Wall, add IFDEPTH warning on deep if statements.

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
 

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<dir>                    Directory to search for includes
+    --if-depth <value>          Tune IFDEPTH warning
      +incdir+<dir>              Directory to search for includes
     --inhibit-sim               Create function to turn off sim
     --inline-mult <value>       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<value>
+
+Rarely needed.  Set the depth at which the IFDEPTH warning will fire,
+defaults to 0 which disables this warning.
+
 =item +incdir+I<dir>
 
 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

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() {}

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"

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)<argc ) {
+		shift;
+		m_ifDepth = atoi(argv[i]);
+	    }
 	    else if ( !strcmp (sw, "-inline-mult") && (i+1)<argc ) {
 		shift;
 		m_inlineMult = atoi(argv[i]);
@@ -1061,6 +1065,7 @@ V3Options::V3Options() {
     m_underlineZero = false;
 
     m_errorLimit = 50;
+    m_ifDepth = 0;
     m_inlineMult = 2000;
     m_outputSplit = 0;
     m_outputSplitCFuncs = 0;

src/V3Options.h

@@ -127,6 +127,7 @@ class V3Options {
     bool	m_underlineZero;// main switch: --underline-zero; undocumented old Verilator 2
 
     int		m_errorLimit;	// main switch: --error-limit
+    int		m_ifDepth;	// main switch: --if-depth
     int		m_inlineMult;	// main switch: --inline-mult
     int		m_outputSplit;	// main switch: --output-split
     int		m_outputSplitCFuncs;// main switch: --output-split-cfuncs
@@ -245,6 +246,7 @@ class V3Options {
     bool inhibitSim() const { return m_inhibitSim; }
 
     int	   errorLimit() const { return m_errorLimit; }
+    int	   ifDepth() const { return m_ifDepth; }
     int	   inlineMult() const { return m_inlineMult; }
     int	   outputSplit() const { return m_outputSplit; }
     int	   outputSplitCFuncs() const { return m_outputSplitCFuncs; }

test_regress/t/t_lint_ifdepth_bad.pl

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

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