Fix memory delayed assignments from multiple clock domains.

Changes

@@ -6,6 +6,10 @@ indicates the contributor was also the author of the fix; Thanks!
 
 * Verilator 3.832 devel
 
+***   Fix memory delayed assignments from multiple clock domains. [Andrew Ling]
+
+***   Report MULTIDRIVEN on memories set in multiple clock domains.
+
 
 * Verilator 3.831 2012/01/20
 

src/V3Delayed.cpp

@@ -74,14 +74,15 @@ private:
     // NODE STATE
     // Cleared each module:
     //  AstVarScope::user1p()	-> AstVarScope*.  Points to temp var created.
-    //  AstVarScope::user2p()	-> AstActive*.  Points to activity block of signal
+    //  AstVarScope::user2p()	-> AstActive*.  Points to activity block of signal (valid when AstVarScope::user1p is valid)
     //  AstVarScope::user4p()	-> AstAlwaysPost*.  Post block for this variable
     //  AstVarScope::user5()	-> VarUsage. Tracks delayed vs non-delayed usage
     //  AstVar::user2()		-> bool.  Set true if already made warning
     //  AstVar::user4()		-> int.   Vector number, for assignment creation
     //  AstVarRef::user2()	-> bool.  Set true if already processed
+    //  AstAlwaysPost::user2()	-> ActActive*.  Points to activity block of signal (valid when AstAlwaysPost::user4p is valid)
     //  AstAlwaysPost::user4()	-> AstIf*.  Last IF (__Vdlyvset__) created under this AlwaysPost
-    // Cleared each scope:
+    // Cleared each scope/active:
     //  AstAssignDly::user3()	-> AstVarScope*.  __Vdlyvset__ created for this assign
     //  AstAlwaysPost::user3()	-> AstVarScope*.  __Vdlyvset__ last referenced in IF
     AstUser1InUse	m_inuser1;
@@ -144,6 +145,40 @@ private:
 	return varscp;
     }
 
+    AstActive* createActivePost(AstVarRef* varrefp) {
+	AstActive* newactp = new AstActive (varrefp->fileline(), "sequentdly",
+					    m_activep->sensesp());
+	m_activep->addNext(newactp);
+	return newactp;
+    }
+    void checkActivePost(AstVarRef* varrefp, AstActive* oldactivep) {
+	// Check for MULTIDRIVEN, and if so make new sentree that joins old & new sentree
+	if (!oldactivep) varrefp->v3fatalSrc("<= old dly assignment not put under sensitivity block");
+	if (oldactivep->sensesp() != m_activep->sensesp()) {
+	    if (!varrefp->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)
+		&& !varrefp->varp()->user2()) {
+		varrefp->varp()->v3warn(MULTIDRIVEN,"Signal has multiple driving blocks: "<<varrefp->varp()->prettyName());
+		varrefp->v3warn(MULTIDRIVEN,"... Location of first driving block");
+		oldactivep->v3warn(MULTIDRIVEN,"... Location of other driving block");
+		varrefp->varp()->user2(true);
+	    }
+	    UINFO(4,"AssignDupDlyVar: "<<varrefp<<endl);
+	    UINFO(4,"  Act: "<<m_activep<<endl);
+	    UINFO(4,"  Act: "<<oldactivep<<endl);
+	    // Make a new sensitivity list, which is the combination of both blocks
+	    AstNodeSenItem* sena = m_activep->sensesp()->sensesp()->cloneTree(true);
+	    AstNodeSenItem* senb = oldactivep->sensesp()->sensesp()->cloneTree(true);
+	    AstSenTree* treep = new AstSenTree(m_activep->fileline(), sena);
+	    if (senb) treep->addSensesp(senb);
+	    if (AstSenTree* storep = oldactivep->sensesStorep()) {
+		storep->unlinkFrBack();
+		pushDeletep(storep);
+	    }
+	    oldactivep->sensesStorep(treep);
+	    oldactivep->sensesp(treep);
+	}
+    }
+
     AstNode* createDlyArray(AstAssignDly* nodep, AstNode* lhsp) {
 	// Create delayed assignment
 	// See top of this file for transformation
@@ -225,6 +260,7 @@ private:
 	//
 	//=== Setting/not setting boolean: __Vdlyvset__
 	AstVarScope* setvscp;
+	AstAssignPre* setinitp = NULL;
 
 	if (nodep->user3p()) {
 	    // Simplistic optimization.  If the previous statement in same scope was also a =>,
@@ -236,11 +272,9 @@ private:
 	} else {  // Create new one
 	    string setvarname = (string("__Vdlyvset__")+oldvarp->shortName()+"__v"+cvtToStr(modVecNum));
 	    setvscp = createVarSc(varrefp->varScopep(), setvarname, 1);
-	    AstAssignPre* setinitp
+	    setinitp = new AstAssignPre (nodep->fileline(),
-		= new AstAssignPre (nodep->fileline(),
 					 new AstVarRef(nodep->fileline(), setvscp, true),
 					 new AstConst(nodep->fileline(), 0));
-	    m_activep->addStmtsp(setinitp);
 	    AstAssign* setassignp
 		= new AstAssign (nodep->fileline(),
 				 new AstVarRef(nodep->fileline(), setvscp, true),
@@ -269,11 +303,18 @@ private:
 	UINFO(9,"   For "<<setvscp<<endl);
 	UINFO(9,"     & "<<varrefp<<endl);
 	AstAlwaysPost* finalp = varrefp->varScopep()->user4p()->castNode()->castAlwaysPost();
-	if (!finalp) {
+	if (finalp) {
+	    AstActive* oldactivep = finalp->user2p()->castNode()->castActive();
+	    checkActivePost(varrefp, oldactivep);
+	    if (setinitp) oldactivep->addStmtsp(setinitp);
+	} else { // first time we've dealt with this memory
 	    finalp = new AstAlwaysPost(nodep->fileline(), NULL/*sens*/, NULL/*body*/);
 	    UINFO(9,"     Created "<<finalp<<endl);
-	    m_activep->addStmtsp(finalp);
+	    AstActive* newactp = createActivePost(varrefp);
+	    newactp->addStmtsp(finalp);
 	    varrefp->varScopep()->user4p(finalp);
+	    finalp->user2p(newactp);
+	    if (setinitp) newactp->addStmtsp(setinitp);
 	}
 	AstIf* postLogicp;
 	if (finalp->user3p()->castNode() == setvscp) {
@@ -292,7 +333,6 @@ private:
 	    finalp->user4p(postLogicp);	// and the associated IF, as we may be able to reuse it
 	}
 	postLogicp->addIfsp(new AstAssign(nodep->fileline(), selectsp, valreadp));
-
 	return newlhsp;
     }
 
@@ -316,6 +356,7 @@ private:
 	m_activep = nodep;
 	bool oldinit = m_inInitial;
 	m_inInitial = nodep->hasInitial();
+	AstNode::user3ClearTree();  // Two sets to same variable in different actives must use different vars.
 	nodep->iterateChildren(*this);
 	m_inInitial = oldinit;
     }
@@ -342,6 +383,7 @@ private:
 	m_inDly = false;
 	m_nextDlyp = NULL;
     }
+
     virtual void visit(AstVarRef* nodep, AstNUser*) {
 	if (!nodep->user2Inc()) {  // Not done yet
 	    if (m_inDly && nodep->lvalue()) {
@@ -354,30 +396,7 @@ private:
 		AstVarScope* dlyvscp = oldvscp->user1p()->castNode()->castVarScope();
 		if (dlyvscp) {  // Multiple use of delayed variable
 		    AstActive* oldactivep = dlyvscp->user2p()->castNode()->castActive();
-		    if (!oldactivep) nodep->v3fatalSrc("<= old dly assignment not put under sensitivity block");
+		    checkActivePost(nodep, oldactivep);
-		    if (oldactivep->sensesp() != m_activep->sensesp()) {
-			if (!nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)
-			    && !nodep->varp()->user2()) {
-			    nodep->varp()->v3warn(MULTIDRIVEN,"Signal has multiple driving blocks: "<<nodep->varp()->prettyName());
-			    nodep->v3warn(MULTIDRIVEN,"... Location of first driving block");
-			    oldactivep->v3warn(MULTIDRIVEN,"... Location of other driving block");
-			    nodep->varp()->user2(true);
-			}
-			UINFO(4,"AssignDupDlyVar: "<<nodep<<endl);
-			UINFO(4,"  Act: "<<m_activep<<endl);
-			UINFO(4,"  Act: "<<oldactivep<<endl);
-			// Make a new sensitivity list, which is the combination of both blocks
-			AstNodeSenItem* sena = m_activep->sensesp()->sensesp()->cloneTree(true);
-			AstNodeSenItem* senb = oldactivep->sensesp()->sensesp()->cloneTree(true);
-			AstSenTree* treep = new AstSenTree(m_activep->fileline(), sena);
-			if (senb) treep->addSensesp(senb);
-			if (AstSenTree* storep = oldactivep->sensesStorep()) {
-			    storep->unlinkFrBack();
-			    pushDeletep(storep);
-			}
-			oldactivep->sensesStorep(treep);
-			oldactivep->sensesp(treep);
-		    }
 		}
 		if (!dlyvscp) {  // First use of this delayed variable
 		    string newvarname = (string("__Vdly__")+nodep->varp()->shortName());
@@ -393,12 +412,10 @@ private:
 		    postp->lhsp()->user2(true);	// Don't detect this assignment
 		    oldvscp->user1p(dlyvscp);  // So we can find it later
 		    // Make new ACTIVE with identical sensitivity tree
-		    AstActive* newactp = new AstActive (nodep->fileline(), "sequentdly",
+		    AstActive* newactp = createActivePost(nodep);
-							m_activep->sensesp());
+		    dlyvscp->user2p(newactp);
 		    newactp->addStmtsp(prep);	// Add to FRONT of statements
 		    newactp->addStmtsp(postp);
-		    m_activep->addNext(newactp);
-		    dlyvscp->user2p(newactp);
 		}
 		AstVarRef* newrefp = new AstVarRef(nodep->fileline(), dlyvscp, true);
 		newrefp->user2(true);  // No reason to do it again

test_regress/t/t_lint_multidriven_bad.pl

@@ -0,0 +1,30 @@
+#!/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.
+
+$Self->{vlt} or $Self->skip("Verilator only test");
+
+compile (
+    v_flags2 => ["--lint-only"],
+    fails=>1,
+    verilator_make_gcc => 0,
+    make_top_shell => 0,
+    make_main => 0,
+    expect=>
+'%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: Signal has multiple driving blocks: v.mem
+%Warning-MULTIDRIVEN: Use ".*" and lint_on around source to disable this message.
+%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: ... Location of first driving block
+%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: ... Location of other driving block
+%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: Signal has multiple driving blocks: out2
+%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: ... Location of first driving block
+%Warning-MULTIDRIVEN: t/t_lint_multidriven_bad.v:\d+: ... Location of other driving block
+%Error: Exiting due to.*',
+    );
+
+ok(1);
+1;

test_regress/t/t_lint_multidriven_bad.v

@@ -0,0 +1,37 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2012 by Wilson Snyder.
+
+module t (/*AUTOARG*/
+   // Outputs
+   out, out2,
+   // Inputs
+   clk, a0, d0, d1
+   );
+
+   input clk;
+   input [1:0] a0;
+   input [7:0] d0;
+   input [7:0] d1;
+   output reg [31:0] out;
+   output reg [15:0] out2;
+
+   reg [7:0] 	     mem [4];
+
+   always @(posedge clk) begin
+      mem[a0] <= d0;
+   end
+   always @(negedge clk) begin
+      mem[a0] <= d1;
+   end
+   assign out = {mem[3],mem[2],mem[1],mem[0]};
+
+   always @(posedge clk) begin
+      out2[7:0] <= d0;
+   end
+   always @(negedge clk) begin
+      out2[15:8] <= d0;
+   end
+   
+endmodule

test_regress/t/t_mem_twoedge.pl

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

test_regress/t/t_mem_twoedge.v

@@ -0,0 +1,117 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2012 by Wilson Snyder.
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   integer 	cyc=0;
+   reg [63:0] 	crc;
+   reg [63:0] 	sum;
+
+   // verilator lint_off MULTIDRIVEN
+   /*AUTOWIRE*/
+   // Beginning of automatic wires (for undeclared instantiated-module outputs)
+   wire [31:0]		out;			// From test of Test.v
+   wire [15:0]		out2;			// From test of Test.v
+   // End of automatics
+   // verilator lint_on MULTIDRIVEN
+
+   Test test (
+	      .en (crc[21:20]),
+	      .a1 (crc[19:18]),
+	      .a0 (crc[17:16]),
+	      .d1 (crc[15:8]),
+	      .d0 (crc[7:0]),
+	      /*AUTOINST*/
+	      // Outputs
+	      .out			(out[31:0]),
+	      .out2			(out2[15:0]),
+	      // Inputs
+	      .clk			(clk));
+
+   // Aggregate outputs into a single result vector
+   wire [63:0] result = {out2, 16'h0, out};
+
+   // Test loop
+`ifdef TEST_VERBOSE
+   always @ (negedge clk) begin
+      $write("[%0t] cyc==%0d crc=%x result=%x\n",$time, cyc, crc, result);
+   end
+`endif
+   always @ (posedge clk) begin
+`ifdef TEST_VERBOSE
+      $write("[%0t] cyc==%0d crc=%x result=%x\n",$time, cyc, crc, result);
+`endif
+      cyc <= cyc + 1;
+      crc <= {crc[62:0], crc[63]^crc[2]^crc[0]};
+      sum <= result ^ {sum[62:0],sum[63]^sum[2]^sum[0]};
+      if (cyc==0) begin
+	 // Setup
+	 crc <= 64'h5aef0c8d_d70a4497;
+	 sum <= 64'h0;
+	 test.clear();
+      end
+      else if (cyc<10) begin
+	 sum <= 64'h0;
+	 test.clear();
+      end
+      else if (cyc<90) begin
+      end
+      else if (cyc==99) begin
+	 $write("[%0t] cyc==%0d crc=%x sum=%x\n",$time, cyc, crc, sum);
+	 if (crc !== 64'hc77bb9b3784ea091) $stop;
+	 // What checksum will we end up with (above print should match)
+`define EXPECTED_SUM 64'hc68a94a34ec970aa
+	 if (sum !== `EXPECTED_SUM) $stop;
+	 $write("*-* All Finished *-*\n");
+	 $finish;
+      end
+   end
+
+endmodule
+
+module Test (/*AUTOARG*/
+   // Outputs
+   out, out2,
+   // Inputs
+   clk, en, a0, a1, d0, d1
+   );
+
+   input clk;
+   input [1:0] en;
+   input [1:0] a0;
+   input [1:0] a1;
+   input [7:0] d0;
+   input [7:0] d1;
+   output reg [31:0] out;
+   output reg [15:0] out2;
+
+   // verilator lint_off MULTIDRIVEN
+   reg [7:0] 	     mem [4];
+   // verilator lint_on MULTIDRIVEN
+
+   task clear();
+      for (int i=0; i<4; ++i) mem[i] = 0;
+   endtask
+
+   always @(posedge clk) begin
+      if (en[0]) begin
+	 mem[a0] <= d0;
+	 out2[7:0] <= d0;
+      end
+   end
+   always @(negedge clk) begin
+      if (en[1]) begin
+	 mem[a1] <= d1;
+	 out2[15:8] <= d0;
+      end
+   end
+
+   assign out = {mem[3],mem[2],mem[1],mem[0]};
+   
+endmodule