diff --git a/Changes b/Changes index 8b78d1108..a84e08d96 100644 --- a/Changes +++ b/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 diff --git a/src/V3Delayed.cpp b/src/V3Delayed.cpp index ab3296b12..5469e04fd 100644 --- a/src/V3Delayed.cpp +++ b/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: "<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: "<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 - = new AstAssignPre (nodep->fileline(), - new AstVarRef(nodep->fileline(), setvscp, true), - new AstConst(nodep->fileline(), 0)); - m_activep->addStmtsp(setinitp); + setinitp = new AstAssignPre (nodep->fileline(), + new AstVarRef(nodep->fileline(), setvscp, true), + new AstConst(nodep->fileline(), 0)); AstAssign* setassignp = new AstAssign (nodep->fileline(), new AstVarRef(nodep->fileline(), setvscp, true), @@ -269,11 +303,18 @@ private: UINFO(9," For "<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 "<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"); - 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: "<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: "<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); - } + checkActivePost(nodep, oldactivep); } 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", - m_activep->sensesp()); + AstActive* newactp = createActivePost(nodep); + 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 diff --git a/test_regress/t/t_lint_multidriven_bad.pl b/test_regress/t/t_lint_multidriven_bad.pl new file mode 100755 index 000000000..7b3a3aad3 --- /dev/null +++ b/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; diff --git a/test_regress/t/t_lint_multidriven_bad.v b/test_regress/t/t_lint_multidriven_bad.v new file mode 100644 index 000000000..4806102f8 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_mem_twoedge.pl b/test_regress/t/t_mem_twoedge.pl new file mode 100755 index 000000000..7058e622f --- /dev/null +++ b/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; diff --git a/test_regress/t/t_mem_twoedge.v b/test_regress/t/t_mem_twoedge.v new file mode 100644 index 000000000..c16fc825b --- /dev/null +++ b/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