From a6743588b6e4103819fc62df5d2d8e74f9b11cc6 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 21 Jan 2015 20:43:21 -0500 Subject: [PATCH] Fix UNOPTFLAT change detect on multidim arrays, bug872. --- Changes | 2 + src/V3AstNodes.cpp | 2 +- src/V3Changed.cpp | 217 +++++++++++++++++++++++------------ test_regress/t/t_EXAMPLE.v | 2 +- test_regress/t/t_order_2d.pl | 18 +++ test_regress/t/t_order_2d.v | 79 +++++++++++++ 6 files changed, 242 insertions(+), 78 deletions(-) create mode 100755 test_regress/t/t_order_2d.pl create mode 100644 test_regress/t/t_order_2d.v diff --git a/Changes b/Changes index ac10303a9..11208e97c 100644 --- a/Changes +++ b/Changes @@ -17,6 +17,8 @@ indicates the contributor was also the author of the fix; Thanks! **** Fix non-ANSI modport instantiations, bug868. [Kevin Thompson] +**** Fix UNOPTFLAT change detect on multidim arrays, bug872. [Andrew Bardsley] + * Verilator 3.868 2014-12-20 diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 40df6b800..e747b51a5 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -863,7 +863,7 @@ void AstNodeDType::dumpSmall(ostream& str) { void AstNodeArrayDType::dumpSmall(ostream& str) { this->AstNodeDType::dumpSmall(str); if (castPackArrayDType()) str<<"p"; else str<<"u"; - str<<" "<AstNodeDType::dump(str); diff --git a/src/V3Changed.cpp b/src/V3Changed.cpp index 1d77345c3..f2e637a74 100644 --- a/src/V3Changed.cpp +++ b/src/V3Changed.cpp @@ -43,6 +43,130 @@ #include "V3Changed.h" #include "V3EmitCBase.h" +//###################################################################### + +class ChangedState { +public: + // STATE + AstNodeModule* m_topModp; // Top module + AstScope* m_scopetopp; // Scope under TOPSCOPE + AstCFunc* m_chgFuncp; // Change function we're building + ChangedState() { + m_topModp = NULL; + m_chgFuncp = NULL; + m_scopetopp = NULL; + } + ~ChangedState() {} +}; + +//###################################################################### +// Utility visitor to find elements to be compared + +class ChangedInsertVisitor : public AstNVisitor { +private: + // STATE + ChangedState* m_statep; // Shared state across visitors + AstVarScope* m_vscp; // Original (non-change) variable we're change-detecting + AstVarScope* m_newvscp; // New (change detect) variable we're change-detecting + AstNode* m_varEqnp; // Original var's equation to get var value + AstNode* m_newLvEqnp; // New var's equation to read value + AstNode* m_newRvEqnp; // New var's equation to set value + uint32_t m_detects; // # detects created + + // CONSTANTS + enum MiscConsts { + DETECTARRAY_MAX_INDEXES = 256 // How many indexes before error + // Ok to increase this, but may result in much slower model + }; + + void newChangeDet() { + if (++m_detects > DETECTARRAY_MAX_INDEXES) { + m_vscp->v3warn(E_DETECTARRAY, "Unsupported: Can't detect more than "<prettyName()<warnMore() + <<"... Could recompile with DETECTARRAY_MAX_INDEXES increased"<fileline(), + m_varEqnp->cloneTree(true), + m_newRvEqnp->cloneTree(true), false); + m_statep->m_chgFuncp->addStmtsp(changep); + AstAssign* initp = new AstAssign (m_vscp->fileline(), + m_newLvEqnp->cloneTree(true), + m_varEqnp->cloneTree(true)); + m_statep->m_chgFuncp->addFinalsp(initp); + } + + virtual void visit(AstBasicDType* nodep, AstNUser*) { + newChangeDet(); + } + virtual void visit(AstPackArrayDType* nodep, AstNUser*) { + newChangeDet(); + } + virtual void visit(AstUnpackArrayDType* nodep, AstNUser*) { + for (int index=0; index < nodep->elementsConst(); ++index) { + AstNode* origVEp = m_varEqnp; + AstNode* origNLEp = m_newLvEqnp; + AstNode* origNREp = m_newRvEqnp; + + m_varEqnp = new AstArraySel(nodep->fileline(), m_varEqnp->cloneTree(true), index); + m_newLvEqnp = new AstArraySel(nodep->fileline(), m_newLvEqnp->cloneTree(true), index); + m_newRvEqnp = new AstArraySel(nodep->fileline(), m_newRvEqnp->cloneTree(true), index); + + nodep->subDTypep()->skipRefp()->accept(*this); + + m_varEqnp->deleteTree(); + m_newLvEqnp->deleteTree(); + m_newRvEqnp->deleteTree(); + + m_varEqnp = origVEp; + m_newLvEqnp = origNLEp; + m_newRvEqnp = origNREp; + } + } + virtual void visit(AstNodeClassDType* nodep, AstNUser*) { + if (nodep->packedUnsup()) { + newChangeDet(); + } else { + if (debug()) nodep->dumpTree(cout,"-DETECTARRAY-class-"); + m_vscp->v3warn(E_DETECTARRAY, "Unsupported: Can't detect changes on complex variable (probably with UNOPTFLAT warning suppressed): "<varp()->prettyName()); + } + } + virtual void visit(AstNode* nodep, AstNUser*) { + nodep->iterateChildren(*this); + if (debug()) nodep->dumpTree(cout,"-DETECTARRAY-general-"); + m_vscp->v3warn(E_DETECTARRAY, "Unsupported: Can't detect changes on complex variable (probably with UNOPTFLAT warning suppressed): "<varp()->prettyName()); + } +public: + // CONSTUCTORS + ChangedInsertVisitor(AstVarScope* vscp, ChangedState* statep) { + m_statep = statep; + m_vscp = vscp; + m_detects = 0; + { + AstVar* varp = m_vscp->varp(); + string newvarname = "__Vchglast__"+m_vscp->scopep()->nameDotless()+"__"+varp->shortName(); + // Create: VARREF(_last) + // ASSIGN(VARREF(_last), VARREF(var)) + // ... + // CHANGEDET(VARREF(_last), VARREF(var)) + AstVar* newvarp = new AstVar (varp->fileline(), AstVarType::MODULETEMP, newvarname, varp); + m_statep->m_topModp->addStmtp(newvarp); + m_newvscp = new AstVarScope(m_vscp->fileline(), m_statep->m_scopetopp, newvarp); + m_statep->m_scopetopp->addVarp(m_newvscp); + + m_varEqnp = new AstVarRef(m_vscp->fileline(), m_vscp, false); + m_newLvEqnp = new AstVarRef(m_vscp->fileline(), m_newvscp, true); + m_newRvEqnp = new AstVarRef(m_vscp->fileline(), m_newvscp, false); + } + vscp->dtypep()->skipRefp()->accept(*this); + m_varEqnp->deleteTree(); + m_newLvEqnp->deleteTree(); + m_newRvEqnp->deleteTree(); + } + virtual ~ChangedInsertVisitor() {} +}; + //###################################################################### // Changed state, as a visitor of each AstNode @@ -54,15 +178,7 @@ private: AstUser1InUse m_inuser1; // STATE - AstNodeModule* m_topModp; // Top module - AstScope* m_scopetopp; // Scope under TOPSCOPE - AstCFunc* m_chgFuncp; // Change function we're building - - // CONSTANTS - enum MiscConsts { - DETECTARRAY_MAX_INDEXES = 256 // How many indexes before error - // Ok to increase this, but may result in much slower model - }; + ChangedState* m_statep; // Shared state across visitors // METHODS static int debug() { @@ -71,69 +187,16 @@ private: return level; } - AstNode* aselIfNeeded(bool isArray, int index, AstNode* childp) { - if (isArray) { - return new AstArraySel(childp->fileline(), childp, - new AstConst(childp->fileline(), index)); - } else { - return childp; - } - } - void genChangeDet(AstVarScope* vscp) { - AstVar* varp = vscp->varp(); vscp->v3warn(IMPERFECTSCH,"Imperfect scheduling of variable: "<dtypeSkipRefp()->castUnpackArrayDType(); - AstPackArrayDType* parrayp = varp->dtypeSkipRefp()->castPackArrayDType(); - AstNodeClassDType *classp = varp->dtypeSkipRefp()->castNodeClassDType(); - bool isUnpackArray = uarrayp; - bool isPackArray = parrayp; - bool isClass = classp && classp->packedUnsup(); - int elements = isUnpackArray ? uarrayp->elementsConst() : 1; - if (isUnpackArray && (elements > DETECTARRAY_MAX_INDEXES)) { - vscp->v3warn(E_DETECTARRAY, "Unsupported: Can't detect more than "<prettyName()<warnMore() - <<"... Could recompile with DETECTARRAY_MAX_INDEXES increased to at least "<dtypeSkipRefp()->castBasicDType()) { - if (debug()) varp->dumpTree(cout,"-DETECTARRAY-"); - vscp->v3warn(E_DETECTARRAY, "Unsupported: Can't detect changes on complex variable (probably with UNOPTFLAT warning suppressed): "<prettyName()); - } else { - string newvarname = "__Vchglast__"+vscp->scopep()->nameDotless()+"__"+varp->shortName(); - // Create: VARREF(_last) - // ASSIGN(VARREF(_last), VARREF(var)) - // ... - // CHANGEDET(VARREF(_last), VARREF(var)) - AstVar* newvarp = new AstVar (varp->fileline(), AstVarType::MODULETEMP, newvarname, varp); - m_topModp->addStmtp(newvarp); - AstVarScope* newvscp = new AstVarScope(vscp->fileline(), m_scopetopp, newvarp); - m_scopetopp->addVarp(newvscp); - for (int index=0; indexfileline(), - aselIfNeeded(isUnpackArray, index, - new AstVarRef(vscp->fileline(), vscp, false)), - aselIfNeeded(isUnpackArray, index, - new AstVarRef(vscp->fileline(), newvscp, false)), - false); - m_chgFuncp->addStmtsp(changep); - AstAssign* initp - = new AstAssign (vscp->fileline(), - aselIfNeeded(isUnpackArray, index, - new AstVarRef(vscp->fileline(), newvscp, true)), - aselIfNeeded(isUnpackArray, index, - new AstVarRef(vscp->fileline(), vscp, false))); - m_chgFuncp->addFinalsp(initp); - } - } + ChangedInsertVisitor visitor (vscp, m_statep); } // VISITORS virtual void visit(AstNodeModule* nodep, AstNUser*) { UINFO(4," MOD "<isTop()) { - m_topModp = nodep; + m_statep->m_topModp = nodep; } nodep->iterateChildren(*this); } @@ -144,15 +207,15 @@ private: // Create the change detection function AstScope* scopep = nodep->scopep(); if (!scopep) nodep->v3fatalSrc("No scope found on top level, perhaps you have no statements?\n"); - m_scopetopp = scopep; + m_statep->m_scopetopp = scopep; // Create change detection function - m_chgFuncp = new AstCFunc(nodep->fileline(), "_change_request", scopep, "QData"); - m_chgFuncp->argTypes(EmitCBaseVisitor::symClassVar()); - m_chgFuncp->symProlog(true); - m_chgFuncp->declPrivate(true); - m_scopetopp->addActivep(m_chgFuncp); + m_statep->m_chgFuncp = new AstCFunc(nodep->fileline(), "_change_request", scopep, "QData"); + m_statep->m_chgFuncp->argTypes(EmitCBaseVisitor::symClassVar()); + m_statep->m_chgFuncp->symProlog(true); + m_statep->m_chgFuncp->declPrivate(true); + m_statep->m_scopetopp->addActivep(m_statep->m_chgFuncp); // We need at least one change detect so we know to emit the correct code - m_chgFuncp->addStmtsp(new AstChangeDet(nodep->fileline(), NULL, NULL, false)); + m_statep->m_chgFuncp->addStmtsp(new AstChangeDet(nodep->fileline(), NULL, NULL, false)); // nodep->iterateChildren(*this); } @@ -164,6 +227,9 @@ private: } } } + virtual void visit(AstNodeMath* nodep, AstNUser*) { + // Short-circuit + } //-------------------- // Default: Just iterate virtual void visit(AstNode* nodep, AstNUser*) { @@ -172,10 +238,8 @@ private: public: // CONSTUCTORS - ChangedVisitor(AstNetlist* nodep) { - m_topModp = NULL; - m_chgFuncp = NULL; - m_scopetopp = NULL; + ChangedVisitor(AstNetlist* nodep, ChangedState* statep) { + m_statep = statep; nodep->accept(*this); } virtual ~ChangedVisitor() {} @@ -186,5 +250,6 @@ public: void V3Changed::changedAll(AstNetlist* nodep) { UINFO(2,__FUNCTION__<<": "<1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_order_2d.v b/test_regress/t/t_order_2d.v new file mode 100644 index 000000000..35f0c35dd --- /dev/null +++ b/test_regress/t/t_order_2d.v @@ -0,0 +1,79 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2015 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc=0; + reg [63:0] crc; + reg [63:0] sum; + + // Take CRC data and apply to testblock inputs + wire input_signal = crc[0]; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + wire output_signal; // From test of Test.v + // End of automatics + + Test test (/*AUTOINST*/ + // Outputs + .output_signal (output_signal), + // Inputs + .input_signal (input_signal)); + + // Aggregate outputs into a single result vector + wire [63:0] result = {63'h0, output_signal}; + + // Test loop + 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 <= '0; + end + else if (cyc<10) begin + sum <= '0; + 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'h765b2e12b25ec97b + if (sum !== `EXPECTED_SUM) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module Test ( + input input_signal, + output output_signal + ); + + // bug872 + + // verilator lint_off UNOPTFLAT + wire some_signal[1:0][1:0]; + assign some_signal[0][0] = input_signal; + assign some_signal[0][1] = some_signal[0][0]; + assign some_signal[1][0] = some_signal[0][1]; + assign some_signal[1][1] = some_signal[1][0]; + assign output_signal = some_signal[1][1]; + +endmodule