With --Wall, add BLKSEQ warning on blocking assignments in seq blocks.

Changes

@@ -18,6 +18,8 @@ indicates the contributor was also the author of the fix; Thanks!
 **   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 BLKSEQ warning on blocking assignments in seq blocks.
+
 ***  With --Wall, add DECLFILENAME warning on modules not matching filename.
 
 ***  With --Wall, add DEFPARAM warning on deprecated defparam statements.

bin/verilator

@@ -1248,7 +1248,7 @@ for debugging and selecting between multiple operating system builds.
 Specifies the directory containing the distribution kit.  This is used to
 find the executable, Perl library, and include files.  If not specified, it
 will come from a default optionally specified at configure time (before
-Verilator was compiled).  It should not be spefified if using a precompiled
+Verilator was compiled).  It should not be specified if using a pre-compiled
 Verilator RPM as the hardcoded value should be correct.
 
 =back
@@ -1420,7 +1420,7 @@ Instead of DPI exporting, there's also Verilator public functions, which
 are slightly faster, but less compatible.
 
 
-=head1 VERIFICATION PROCEDURAL INTERACE (VPI)
+=head1 VERIFICATION PROCEDURAL INTERFACE (VPI)
 
 Verilator supports a very limited subset of the VPI.  This subset allows
 inspection, examination, value change callbacks, and depositing of values
@@ -2329,6 +2329,20 @@ inside a public task.
 Ignoring this warning may make Verilator simulations differ from other
 simulators.
 
+=item BLKSEQ
+
+This indicates that a blocking assignment (=) is used in a sequential
+block.  Generally non-blocking/delayed assignments (<=) are used in
+sequential blocks, to avoid the possibility of simulator races.  It can be
+reasonable to do this if the generated signal is used ONLY later in the
+same block, however this style is generally discouraged as it is error
+prone.
+
+      always @ (posedge clk)  foo = ...
+
+Disabled by default as this is a code style warning; it will simulate
+correctly.
+
 =item BLKLOOPINIT
 
 This indicates that the initialization of an array needs to use non-delayed

src/V3Active.cpp

@@ -160,16 +160,25 @@ public:
 
 class ActiveDlyVisitor : public ActiveBaseVisitor {
 private:
+    bool m_combo;	// Combo logic
     // VISITORS
     virtual void visit(AstAssignDly* nodep, AstNUser*) {
-	// Convert to a non-delayed assignment
-	UINFO(5,"    ASSIGNDLY "<<nodep<<endl);
-	nodep->v3warn(COMBDLY,"Delayed assignments (<=) in non-clocked (non flop or latch) blocks should be non-delayed assignments (=).");
-	AstNode* newp = new AstAssign (nodep->fileline(),
-				       nodep->lhsp()->unlinkFrBack(),
-				       nodep->rhsp()->unlinkFrBack());
-	nodep->replaceWith(newp);
-	nodep->deleteTree(); nodep = NULL;
+	if (m_combo) {
+	    // Convert to a non-delayed assignment
+	    UINFO(5,"    ASSIGNDLY "<<nodep<<endl);
+	    nodep->v3warn(COMBDLY,"Delayed assignments (<=) in non-clocked (non flop or latch) block; suggest blocking assignments (=).");
+	    AstNode* newp = new AstAssign (nodep->fileline(),
+					   nodep->lhsp()->unlinkFrBack(),
+					   nodep->rhsp()->unlinkFrBack());
+	    nodep->replaceWith(newp);
+	    nodep->deleteTree(); nodep = NULL;
+	}
+    }
+    virtual void visit(AstAssign* nodep, AstNUser*) {
+	if (!m_combo) {
+	    // Convert to a non-delayed assignment
+	    nodep->v3warn(BLKSEQ,"Blocking assignments (=) in sequential (flop or latch) block; suggest delayed assignments (<=).");
+	}
     }
     // Empty visitors, speed things up
     virtual void visit(AstNodeMath* nodep, AstNUser*) {}
@@ -179,7 +188,8 @@ private:
     }
 public:
     // CONSTUCTORS
-    ActiveDlyVisitor(AstNode* nodep) {
+    ActiveDlyVisitor(AstNode* nodep, bool combo) {
+	m_combo = combo;
 	nodep->accept(*this);
     }
     virtual ~ActiveDlyVisitor() {}
@@ -317,7 +327,10 @@ private:
 
 	// Warn and/or convert any delayed assignments
 	if (combo && !sequent) {
-	    ActiveDlyVisitor dlyvisitor (nodep);
+	    ActiveDlyVisitor dlyvisitor (nodep, true);
+	}
+	else if (!combo && sequent) {
+	    ActiveDlyVisitor dlyvisitor (nodep, false);
 	}
     }
     virtual void visit(AstAlways* nodep, AstNUser*) {

src/V3Error.h

@@ -54,6 +54,7 @@ public:
 	EC_FIRST_WARN,	// Just a code so the program knows where to start warnings
 	//
 	BLKANDNBLK,	// Blocked and non-blocking assignments to same variable
+	BLKSEQ,		// Blocking assignments in sequential block
 	CASEINCOMPLETE,	// Case statement has missing values
 	CASEOVERLAP,	// Case statements overlap
 	CASEWITHX,	// Case with X values
@@ -103,13 +104,14 @@ public:
 	    "MULTITOP", "TASKNSVAR", "BLKLOOPINIT",
 	    // Warnings
 	    " EC_FIRST_WARN",
-	    "BLKANDNBLK",
+	    "BLKANDNBLK", "BLKSEQ",
 	    "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CMPCONST",
 	    "COMBDLY", "DEFPARAM", "DECLFILENAME",
 	    "GENCLK",
 	    "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE", "INCABSPATH",
 	    "LITENDIAN", "MODDUP",
-	    "MULTIDRIVEN", "REDEFMACRO",
+	    "MULTIDRIVEN",
+	    "REDEFMACRO",
 	    "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET",
 	    "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED",
 	    "VARHIDDEN", "WIDTH", "WIDTHCONCAT",
@@ -136,7 +138,8 @@ public:
 				      || m_e==UNSIGNED
 				      || m_e==WIDTH); }
     // Warnings that are style only
-    bool styleError() const { return ( m_e==DEFPARAM
+    bool styleError() const { return ( m_e==BLKSEQ
+				       || m_e==DEFPARAM
 				       || m_e==DECLFILENAME
 				       || m_e==INCABSPATH
 				       || m_e==SYNCASYNCNET

test_regress/t/t_lint_blksync_bad.pl

@@ -0,0 +1,26 @@
+#!/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 -Wwarn-BLKSEQ -Wwarn-COMBDLY"],
+    fails=>1,
+    verilator_make_gcc => 0,
+    make_top_shell => 0,
+    make_main => 0,
+    expect=>
+'%Warning-BLKSEQ: t/t_lint_blksync_bad.v:\d+: Blocking assignments \(=\) in sequential \(flop or latch\) block; suggest delayed assignments \(<=\).
+%Warning-BLKSEQ: Use .* to disable this message.
+%Warning-COMBDLY: t/t_lint_blksync_bad.v:\d+: Delayed assignments \(<=\) in non-clocked \(non flop or latch\) block; suggest blocking assignments \(=\).
+%Warning-COMBDLY: \*\*\* See the manual before disabling this,
+%Warning-COMBDLY: else you may end up with different sim results.
+%Error: Exiting due to.*',
+    ) if $Self->{v3};
+
+ok(1);
+1;

test_regress/t/t_lint_blksync_bad.v

@@ -0,0 +1,28 @@
+// 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 (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   reg 	 sync_blk;
+   reg 	 sync_nblk;
+   reg 	 combo_blk;
+   reg 	 combo_nblk;
+
+   always @(posedge clk) begin
+      sync_blk = 1'b1; 
+      sync_nblk <= 1'b1; 
+   end
+
+   always @* begin
+      combo_blk = 1'b1; 
+      combo_nblk <= 1'b1; 
+   end
+
+endmodule
+