From 8c4aa8517e8952ecdb7e81350a17844d9204acae Mon Sep 17 00:00:00 2001 From: Johan Bjork Date: Thu, 12 May 2016 07:19:02 -0400 Subject: [PATCH] Fix --output-split of constructors, bug1035. Signed-off-by: Wilson Snyder --- Changes | 2 + src/Makefile_obj.in | 1 + src/V3AstNodes.h | 15 +++ src/V3Changed.cpp | 66 ++++++++-- src/V3EmitC.cpp | 169 +++++++++++++------------ src/V3VarResets.cpp | 94 ++++++++++++++ src/V3VarResets.h | 36 ++++++ src/Verilator.cpp | 5 + test_regress/t/t_unopt_array_csplit.pl | 22 ++++ 9 files changed, 318 insertions(+), 92 deletions(-) create mode 100644 src/V3VarResets.cpp create mode 100644 src/V3VarResets.h create mode 100755 test_regress/t/t_unopt_array_csplit.pl diff --git a/Changes b/Changes index 2b9f54535..4424d2aae 100644 --- a/Changes +++ b/Changes @@ -15,6 +15,8 @@ indicates the contributor was also the author of the fix; Thanks! *** Add --l2-name option for controlling "v" naming. +**** Fix --output-split of constructors, bug1035. [Johan Bjork] + **** Fix removal of empty packages, modules and cells, bug1034. [Johan Bjork] diff --git a/src/Makefile_obj.in b/src/Makefile_obj.in index ba02b5d88..87bb15d76 100644 --- a/src/Makefile_obj.in +++ b/src/Makefile_obj.in @@ -235,6 +235,7 @@ RAW_OBJS = \ V3Undriven.o \ V3Unknown.o \ V3Unroll.o \ + V3VarResets.o \ V3Width.o \ V3WidthSel.o \ diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 8c43f86bf..4f5ce4e5f 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -5157,6 +5157,21 @@ public: }; +class AstCReset : public AstNodeStmt { + //Reset variable at startup +public: + AstCReset(FileLine* fl, AstNode* exprsp) + : AstNodeStmt(fl) { + addNOp1p(exprsp); + } + ASTNODE_NODE_FUNCS(CReset, CRESET) + virtual bool isGateOptimizable() const { return false; } + virtual bool isPredictOptimizable() const { return false; } + virtual V3Hash sameHash() const { return V3Hash(); } + virtual bool same(AstNode* samep) const { return true; } + AstVarRef* varrefp() const { return op1p()->castVarRef(); } // op1= varref to reset +}; + class AstCStmt : public AstNodeStmt { // Emit C statement public: diff --git a/src/V3Changed.cpp b/src/V3Changed.cpp index 02ec2b8de..738d4241b 100644 --- a/src/V3Changed.cpp +++ b/src/V3Changed.cpp @@ -51,12 +51,54 @@ public: AstNodeModule* m_topModp; // Top module AstScope* m_scopetopp; // Scope under TOPSCOPE AstCFunc* m_chgFuncp; // Change function we're building + AstCFunc* m_tlChgFuncp; // Top level change function we're building + int m_numStmts; // Number of statements added to m_chgFuncp + int m_funcNum; // Number of change functions emitted + ChangedState() { m_topModp = NULL; m_chgFuncp = NULL; m_scopetopp = NULL; + m_tlChgFuncp = NULL; + m_numStmts = 0; + m_funcNum = 0; } ~ChangedState() {} + + void maybeCreateChgFuncp() { + // Don't create an extra function call if splitting is disabled + if (!v3Global.opt.outputSplitCFuncs()) { + m_chgFuncp = m_tlChgFuncp; + return; + } + if (!m_chgFuncp || v3Global.opt.outputSplitCFuncs() < m_numStmts) { + m_chgFuncp = new AstCFunc(m_scopetopp->fileline(), "_change_request_" + cvtToStr(++m_funcNum), m_scopetopp, "QData"); + m_chgFuncp->argTypes(EmitCBaseVisitor::symClassVar()); + m_chgFuncp->symProlog(true); + m_chgFuncp->declPrivate(true); + m_scopetopp->addActivep(m_chgFuncp); + + // Add a top call to it + AstCCall* callp = new AstCCall(m_scopetopp->fileline(), m_chgFuncp); + callp->argTypes("vlSymsp"); + + if (!m_tlChgFuncp->stmtsp()) { + m_tlChgFuncp->addStmtsp(new AstCReturn(m_scopetopp->fileline(), callp)); + } else { + AstCReturn* returnp = m_tlChgFuncp->stmtsp()->castCReturn(); + if (!returnp) m_scopetopp->v3fatalSrc("Lost CReturn in top change function"); + // This is currently using AstLogOr which will shortcut the evaluation if + // any function returns true. This is likely what we want and is similar to the logic already in use + // inside V3EmitC, however, it also means that verbose logging may miss to print change detect variables. + AstNode* newp = new AstCReturn(m_scopetopp->fileline(), + new AstLogOr(m_scopetopp->fileline(), callp, + returnp->lhsp()->unlinkFrBack())); + returnp->replaceWith(newp); + returnp->deleteTree(); VL_DANGLING(returnp); + } + m_numStmts = 0; + } + } }; //###################################################################### @@ -87,6 +129,8 @@ private: <<"... Could recompile with DETECTARRAY_MAX_INDEXES increased"<maybeCreateChgFuncp(); + AstChangeDet* changep = new AstChangeDet (m_vscp->fileline(), m_varEqnp->cloneTree(true), m_newRvEqnp->cloneTree(true), false); @@ -95,6 +139,8 @@ private: m_newLvEqnp->cloneTree(true), m_varEqnp->cloneTree(true)); m_statep->m_chgFuncp->addFinalsp(initp); + EmitCBaseCounterVisitor visitor(initp); + m_statep->m_numStmts += visitor.count(); } virtual void visit(AstBasicDType* nodep, AstNUser*) { @@ -200,6 +246,7 @@ private: } nodep->iterateChildren(*this); } + virtual void visit(AstTopScope* nodep, AstNUser*) { UINFO(4," TS "<scopep(); if (!scopep) nodep->v3fatalSrc("No scope found on top level, perhaps you have no statements?\n"); m_statep->m_scopetopp = scopep; - // Create change detection function - 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 + + // Create a wrapper change detection function that calls each change detection function + m_statep->m_tlChgFuncp = new AstCFunc(nodep->fileline(), "_change_request", scopep, "QData"); + m_statep->m_tlChgFuncp->argTypes(EmitCBaseVisitor::symClassVar()); + m_statep->m_tlChgFuncp->symProlog(true); + m_statep->m_tlChgFuncp->declPrivate(true); + m_statep->m_scopetopp->addActivep(m_statep->m_tlChgFuncp); + // Each change detection function needs at least one AstChangeDet + // to ensure that V3EmitC outputs the necessary code. + m_statep->maybeCreateChgFuncp(); m_statep->m_chgFuncp->addStmtsp(new AstChangeDet(nodep->fileline(), NULL, NULL, false)); - // + nodep->iterateChildren(*this); } virtual void visit(AstVarScope* nodep, AstNUser*) { diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 2368999d0..467fe5bf2 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -912,12 +912,17 @@ class EmitCImp : EmitCStmts { m_blkChangeDetVec.push_back(nodep); } + virtual void visit(AstCReset* nodep, AstNUser*) { + AstVar* varp = nodep->varrefp()->varp(); + emitVarReset(varp); + } + //--------------------------------------- // ACCESSORS // METHODS // Low level - void emitVarResets(AstNodeModule* modp); + void emitVarReset(AstVar* modp); void emitCellCtors(AstNodeModule* modp); void emitSensitives(); // Medium level @@ -1382,91 +1387,80 @@ void EmitCStmts::displayNode(AstNode* nodep, AstScopeName* scopenamep, //###################################################################### // Internal EmitC -void EmitCImp::emitVarResets(AstNodeModule* modp) { - puts("// Reset internal values\n"); - if (modp->isTop()) { - if (v3Global.opt.inhibitSim()) puts("__Vm_inhibitSim = false;\n"); - puts("\n"); +void EmitCImp::emitVarReset(AstVar* varp) { + if (varp->isIO() && m_modp->isTop() && optSystemC()) { + // System C top I/O doesn't need loading, as the lower level subinst code does it.} + } else if (varp->isParam()) { + if (!varp->valuep()) varp->v3fatalSrc("No init for a param?"); + // If a simple CONST value we initialize it using an enum + // If an ARRAYINIT we initialize it using an initial block similar to a signal + //puts("// parameter "+varp->name()+" = "+varp->valuep()->name()+"\n"); } - - puts("// Reset structure values\n"); - for (AstNode* nodep=modp->stmtsp(); nodep; nodep = nodep->nextp()) { - if (AstVar* varp = nodep->castVar()) { - if (varp->isIO() && modp->isTop() && optSystemC()) { - // System C top I/O doesn't need loading, as the lower level subinst code does it. - } - else if (varp->isParam()) { - if (!varp->valuep()) nodep->v3fatalSrc("No init for a param?"); - // If a simple CONST value we initialize it using an enum - // If an ARRAYINIT we initialize it using an initial block similar to a signal - //puts("// parameter "+varp->name()+" = "+varp->valuep()->name()+"\n"); - } - else if (AstInitArray* initarp = varp->valuep()->castInitArray()) { - AstConst* constsp = initarp->initsp()->castConst(); - if (AstUnpackArrayDType* arrayp = varp->dtypeSkipRefp()->castUnpackArrayDType()) { - for (int i=0; ielementsConst(); i++) { - if (!constsp) initarp->v3fatalSrc("Not enough values in array initalizement"); - emitSetVarConstant(varp->name()+"["+cvtToStr(i)+"]", constsp); - constsp = constsp->nextp()->castConst(); - } - } else { - varp->v3fatalSrc("InitArray under non-arrayed var"); - } - } - else if (varp->basicp() && varp->basicp()->keyword() == AstBasicDTypeKwd::STRING) { - // Constructor deals with it - } - else { - int vects = 0; - // This isn't very robust and may need cleanup for other data types - for (AstUnpackArrayDType* arrayp=varp->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { - int vecnum = vects++; - if (arrayp->msb() < arrayp->lsb()) varp->v3fatalSrc("Should have swapped msb & lsb earlier."); - string ivar = string("__Vi")+cvtToStr(vecnum); - // MSVC++ pre V7 doesn't support 'for (int ...)', so declare in sep block - puts("{ int __Vi"+cvtToStr(vecnum)+"="+cvtToStr(0)+";"); - puts(" for (; "+ivar+"<"+cvtToStr(arrayp->elementsConst())); - puts("; ++"+ivar+") {\n"); - } - bool zeroit = (varp->attrFileDescr() // Zero it out, so we don't core dump if never call $fopen - || (varp->basicp() && varp->basicp()->isZeroInit()) - || (varp->name().size()>=1 && varp->name()[0]=='_' && v3Global.opt.underlineZero())); - if (varp->isWide()) { - // DOCUMENT: We randomize everything. If the user wants a _var to be zero, - // there should be a initial statement. (Different from verilator2.) - if (zeroit) puts("VL_ZERO_RESET_W("); - else puts("VL_RAND_RESET_W("); - puts(cvtToStr(varp->widthMin())); - puts(","); - puts(varp->name()); - for (int v=0; vname()); - for (int v=0; visUsedClock())) { - puts(" = 0;\n"); - } else if (v3Global.opt.xInitialEdge() - && (0 == varp->name().find("__Vclklast__"))) { - puts(" = 1;\n"); - } else { - puts(" = VL_RAND_RESET_"); - emitIQW(varp); - puts("("); - puts(cvtToStr(varp->widthMin())); - puts(");\n"); - } - } - for (int v=0; vvaluep()->castInitArray()) { + AstConst* constsp = initarp->initsp()->castConst(); + if (AstUnpackArrayDType* arrayp = varp->dtypeSkipRefp()->castUnpackArrayDType()) { + for (int i=0; ielementsConst(); i++) { + if (!constsp) initarp->v3fatalSrc("Not enough values in array initalizement"); + emitSetVarConstant(varp->name()+"["+cvtToStr(i)+"]", constsp); + constsp = constsp->nextp()->castConst(); } + } else { + varp->v3fatalSrc("InitArray under non-arrayed var"); } } + else if (varp->basicp() && varp->basicp()->keyword() == AstBasicDTypeKwd::STRING) { + // Constructor deals with it + } + else { + int vects = 0; + // This isn't very robust and may need cleanup for other data types + for (AstUnpackArrayDType* arrayp=varp->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { + int vecnum = vects++; + if (arrayp->msb() < arrayp->lsb()) varp->v3fatalSrc("Should have swapped msb & lsb earlier."); + string ivar = string("__Vi")+cvtToStr(vecnum); + // MSVC++ pre V7 doesn't support 'for (int ...)', so declare in sep block + puts("{ int __Vi"+cvtToStr(vecnum)+"="+cvtToStr(0)+";"); + puts(" for (; "+ivar+"<"+cvtToStr(arrayp->elementsConst())); + puts("; ++"+ivar+") {\n"); + } + bool zeroit = (varp->attrFileDescr() // Zero it out, so we don't core dump if never call $fopen + || (varp->basicp() && varp->basicp()->isZeroInit()) + || (varp->name().size()>=1 && varp->name()[0]=='_' && v3Global.opt.underlineZero())); + if (varp->isWide()) { + // DOCUMENT: We randomize everything. If the user wants a _var to be zero, + // there should be a initial statement. (Different from verilator2.) + if (zeroit) puts("VL_ZERO_RESET_W("); + else puts("VL_RAND_RESET_W("); + puts(cvtToStr(varp->widthMin())); + puts(","); + puts(varp->name()); + for (int v=0; vname()); + for (int v=0; visUsedClock())) { + puts(" = 0;\n"); + } else if (v3Global.opt.xInitialEdge() + && (0 == varp->name().find("__Vclklast__"))) { + puts(" = 1;\n"); + } else { + puts(" = VL_RAND_RESET_"); + emitIQW(varp); + puts("("); + puts(cvtToStr(varp->widthMin())); + puts(");\n"); + } + } + for (int v=0; visTop()) { + if (v3Global.opt.inhibitSim()) puts("__Vm_inhibitSim = false;\n"); + puts("\n"); + } + puts("// Reset structure values\n"); + puts("_ctor_var_reset();\n"); emitTextSection(AstType::atSCCTOR); if (optSystemPerl()) puts("SP_AUTO_CTOR;\n"); puts("}\n"); @@ -2014,6 +2014,7 @@ void EmitCImp::emitInt(AstNodeModule* modp) { puts("static void traceChg ("+v3Global.opt.traceClassBase()+"* vcdp, void* userthis, uint32_t code);\n"); } if (v3Global.opt.savable()) { + ofp()->putsPrivate(false); // public: puts("void __Vserialize(VerilatedSerialize& os);\n"); puts("void __Vdeserialize(VerilatedDeserialize& os);\n"); puts("\n"); diff --git a/src/V3VarResets.cpp b/src/V3VarResets.cpp new file mode 100644 index 000000000..2c5cf0f65 --- /dev/null +++ b/src/V3VarResets.cpp @@ -0,0 +1,94 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Generate AstCReset nodes. +// +// Code available from: http://www.veripool.org/verilator +// +//************************************************************************* +// +// Copyright 2003-2016 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. +// +// Verilator is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +//************************************************************************* +// V3VarReset's Transformations: +// Iterates over all modules and creates a _ctor_var_reset AstCFunc +// For each variable that needs reset, add a AstCReset node. +// This transformation honors outputSplitCFuncs. +//************************************************************************* +#include "config_build.h" +#include "verilatedos.h" +#include +#include +#include +#include +#include +#include +#include + +#include "V3Global.h" +#include "V3VarResets.h" + +class V3VarReset { +private: + AstNodeModule* m_modp; // Current module + AstCFunc* m_tlFuncp; // Top level function being built + AstCFunc* m_funcp; // Current function + int m_numStmts; // Number of statements output + int m_funcNum; // Function number being built + + void initializeVar(AstVar* nodep) { + if (v3Global.opt.outputSplitCFuncs() + && v3Global.opt.outputSplitCFuncs() < m_numStmts) { + m_funcp = NULL; + } + if (!m_funcp) { + m_funcp = new AstCFunc(m_modp->fileline(), "_ctor_var_reset_" + cvtToStr(++m_funcNum), NULL, "void"); + m_funcp->isStatic(false); + m_funcp->declPrivate(true); + m_funcp->slow(true); + m_modp->addStmtp(m_funcp); + + // Add a top call to it + AstCCall* callp = new AstCCall(m_modp->fileline(), m_funcp); + + m_tlFuncp->addStmtsp(callp); + m_numStmts = 0; + } + m_funcp->addStmtsp(new AstCReset(nodep->fileline(), new AstVarRef(nodep->fileline(), nodep, true))); + m_numStmts += 1; + } + +public: + V3VarReset(AstNodeModule* nodep) { + m_modp = nodep; + m_numStmts = 0; + m_funcNum = 0; + m_tlFuncp = new AstCFunc(nodep->fileline(), "_ctor_var_reset", NULL, "void"); + m_tlFuncp->declPrivate(true); + m_tlFuncp->isStatic(false); + m_tlFuncp->slow(true); + m_funcp = m_tlFuncp; + m_modp->addStmtp(m_tlFuncp); + for (AstNode* np = m_modp->stmtsp(); np; np = np->nextp()) { + AstVar* varp = np->castVar(); + if (varp) initializeVar(varp); + } + } +}; + +//###################################################################### + +void V3VarResets::emitResets() { + UINFO(2,__FUNCTION__<<": "<modulesp(); nodep; nodep=nodep->nextp()->castNodeModule()) { + // Process each module in turn + V3VarReset v(nodep); + } +} diff --git a/src/V3VarResets.h b/src/V3VarResets.h new file mode 100644 index 000000000..0ff70037d --- /dev/null +++ b/src/V3VarResets.h @@ -0,0 +1,36 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Emit C++ code for module tree +// +// Code available from: http://www.veripool.org/verilator +// +//************************************************************************* +// +// Copyright 2003-2016 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. +// +// Verilator is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +//************************************************************************* + +#ifndef _V3VARRESETS_H_ +#define _V3VARRESETS_H_ 1 +#include "config_build.h" +#include "verilatedos.h" +#include "V3Error.h" +#include "V3Ast.h" + +//============================================================================ + +class V3VarResets { +public: + static void emitResets(); +}; + + +#endif // Guard diff --git a/src/Verilator.cpp b/src/Verilator.cpp index aaa8d86bb..89d9752f0 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -39,6 +39,7 @@ #include "V3Const.h" #include "V3Coverage.h" #include "V3CoverageJoin.h" +#include "V3VarResets.h" #include "V3Dead.h" #include "V3Delayed.h" #include "V3Depth.h" @@ -497,6 +498,10 @@ void process () { } V3Error::abortIfErrors(); + if (!v3Global.opt.lintOnly() + && !v3Global.opt.xmlOnly()) { + V3VarResets::emitResets(); + } // Output the text if (!v3Global.opt.lintOnly() diff --git a/test_regress/t/t_unopt_array_csplit.pl b/test_regress/t/t_unopt_array_csplit.pl new file mode 100755 index 000000000..42beadc2b --- /dev/null +++ b/test_regress/t/t_unopt_array_csplit.pl @@ -0,0 +1,22 @@ +#!/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. + +$Self->{vlt} or $Self->skip("Verilator only test"); + +top_filename("t/t_unopt_array.v"); +compile ( + v_flags2 => ["--trace --output-split 1 --output-split-cfuncs 1 -Wno-UNOPTFLAT"], + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1;