diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 16a52b05d..78804e7ea 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -239,6 +239,9 @@ inline void AstNode::debugTreeChange(const char* prefix, int lineno, bool next) //if (debug()) cout<<"-treeChange: V3Ast.cpp:"<"<dumpTree(cout,"-treeChange: "); // if (next||1) this->dumpTreeAndNext(cout, prefix); // else this->dumpTree(cout, prefix); @@ -561,7 +564,7 @@ void AstNode::relink(AstNRelinker* linkerp) { if (linkerp->m_iterpp) { // If we're iterating over a next() link, we need to follow links off the // NEW node. Thus we pass iteration information via a pointer in the node. - // This adds a unfortunate 4 bytes to every AstNode, but is faster than passing + // This adds a unfortunate hot 8 bytes to every AstNode, but is faster than passing // across every function. // If anyone has a cleaner way, I'd be grateful. *(linkerp->m_iterpp) = newp; @@ -750,6 +753,20 @@ void AstNode::iterateChildren(AstNVisitor& v, AstNUser* vup) { if (m_op4p) m_op4p->iterateAndNext(v, vup); } +void AstNode::iterateChildrenConst(AstNVisitor& v, AstNUser* vup) { + // This is a very hot function + if (!this) return; + ASTNODE_PREFETCH(m_op1p); + ASTNODE_PREFETCH(m_op2p); + ASTNODE_PREFETCH(m_op3p); + ASTNODE_PREFETCH(m_op4p); + // if () not needed since iterateAndNext accepts null this, but faster with it. + if (m_op1p) m_op1p->iterateAndNextConst(v, vup); + if (m_op2p) m_op2p->iterateAndNextConst(v, vup); + if (m_op3p) m_op3p->iterateAndNextConst(v, vup); + if (m_op4p) m_op4p->iterateAndNextConst(v, vup); +} + void AstNode::iterateAndNext(AstNVisitor& v, AstNUser* vup) { // This is a very hot function // IMPORTANT: If you replace a node that's the target of this iterator, @@ -764,16 +781,18 @@ void AstNode::iterateAndNext(AstNVisitor& v, AstNUser* vup) { while (nodep) { AstNode* niterp = nodep; // This address may get stomped via m_iterpp if the node is edited ASTNODE_PREFETCH(nodep->m_nextp); + // Desirable check, but many places where multiple iterations are OK + //if (VL_UNLIKELY(niterp->m_iterpp)) niterp->v3fatalSrc("IterateAndNext under iterateAndNext may miss edits"); // cppcheck-suppress nullPointer niterp->m_iterpp = &niterp; niterp->accept(v, vup); // accept may do a replaceNode and change niterp on us... //if (niterp != nodep) UINFO(1,"iterateAndNext edited "<<(void*)nodep<<" now into "<<(void*)niterp<m_iterpp = NULL; - if (VL_UNLIKELY(niterp!=nodep)) { // Edited it + if (VL_UNLIKELY(niterp!=nodep)) { // Edited node inside accept nodep = niterp; - } else { // Same node, just loop + } else { // Unchanged node, just continue loop nodep = niterp->m_nextp; } } @@ -799,11 +818,12 @@ void AstNode::iterateChildrenBackwards(AstNVisitor& v, AstNUser* vup) { this->op4p()->iterateListBackwards(v,vup); } -void AstNode::iterateAndNextIgnoreEdit(AstNVisitor& v, AstNUser* vup) { +void AstNode::iterateAndNextConst(AstNVisitor& v, AstNUser* vup) { // Keep following the current list even if edits change it if (!this) return; for (AstNode* nodep=this; nodep; ) { AstNode* nnextp = nodep->m_nextp; + ASTNODE_PREFETCH(nnextp); nodep->accept(v, vup); nodep = nnextp; } diff --git a/src/V3Ast.h b/src/V3Ast.h index 42c53a82d..8dbaacb7f 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1186,9 +1186,11 @@ public: virtual void accept(AstNVisitor& v, AstNUser* vup=NULL) = 0; void iterate(AstNVisitor& v, AstNUser* vup=NULL) { this->accept(v,vup); } // Does this; excludes following this->next void iterateAndNext(AstNVisitor& v, AstNUser* vup=NULL); - void iterateAndNextIgnoreEdit(AstNVisitor& v, AstNUser* vup=NULL); + void iterateAndNextConst(AstNVisitor& v, AstNUser* vup=NULL); + void iterateAndNextIgnoreEdit(AstNVisitor& v, AstNUser* vup=NULL) { iterateAndNextConst(v, vup); } void iterateChildren(AstNVisitor& v, AstNUser* vup=NULL); // Excludes following this->next void iterateChildrenBackwards(AstNVisitor& v, AstNUser* vup=NULL); // Excludes following this->next + void iterateChildrenConst(AstNVisitor& v, AstNUser* vup=NULL); // Excludes following this->next AstNode* acceptSubtreeReturnEdits(AstNVisitor& v, AstNUser* vup=NULL); // Return edited nodep; see comments in V3Ast.cpp // CONVERSION diff --git a/src/V3AstConstOnly.h b/src/V3AstConstOnly.h new file mode 100644 index 000000000..67731f2a9 --- /dev/null +++ b/src/V3AstConstOnly.h @@ -0,0 +1,33 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Ast node structure +// +// Code available from: http://www.veripool.org/verilator +// +//************************************************************************* +// +// Copyright 2003-2014 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 _V3ASTCONSTONLY_H_ +#define _V3ASTCONSTONLY_H_ 1 + +// Include only in visitors that do not not edit nodes, so should use constant iterators +#define iterateAndNext error_use_iterateAndNextConst +#define iterateChildren error_use_iterateChildrenConst + +#define addNext error_no_addNext_in_ConstOnlyVisitor +#define replaceWith error_no_replaceWith_in_ConstOnlyVisitor +#define deleteTree error_no_deleteTree_in_ConstOnlyVisitor +#define unlinkFrBack error_no_unlinkFrBack_in_ConstOnlyVisitor + +#endif // Guard diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 97b10dd1d..5c3c7071d 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -37,6 +37,9 @@ #include "V3Broken.h" #include "V3Ast.h" +// This visitor does not edit nodes, and is called at error-exit, so should use constant iterators +#include "V3AstConstOnly.h" + //###################################################################### class BrokenTable : public AstNVisitor { @@ -185,7 +188,7 @@ private: // VISITORS virtual void visit(AstNode* nodep, AstNUser*) { BrokenTable::addInTree(nodep, nodep->maybePointedTo()); - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); } public: // CONSTUCTORS @@ -228,7 +231,7 @@ private: nodep->v3fatalSrc("Width != WidthMin"); } } - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); BrokenTable::setUnder(nodep,false); } public: diff --git a/src/V3Stats.cpp b/src/V3Stats.cpp index bfebe4d5e..5299bc052 100644 --- a/src/V3Stats.cpp +++ b/src/V3Stats.cpp @@ -31,6 +31,9 @@ #include "V3Ast.h" #include "V3File.h" +// This visitor does not edit nodes, and is called at error-exit, so should use constant iterators +#include "V3AstConstOnly.h" + //###################################################################### // Stats class functions @@ -74,14 +77,14 @@ private: virtual void visit(AstNodeModule* nodep, AstNUser*) { allNodes(nodep); if (!m_fast) { - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); } else { for (AstNode* searchp = nodep->stmtsp(); searchp; searchp=searchp->nextp()) { if (AstCFunc* funcp = searchp->castCFunc()) { if (funcp->name() == "_eval") { m_instrs=0; m_counting = true; - funcp->iterateChildren(*this); + funcp->iterateChildrenConst(*this); m_counting = false; } } @@ -90,7 +93,7 @@ private: } virtual void visit(AstVar* nodep, AstNUser*) { allNodes(nodep); - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); if (m_counting && nodep->dtypep()) { if (nodep->isUsedClock()) ++m_statVarClock; if (nodep->dtypeSkipRefp()->castUnpackArrayDType()) ++m_statVarArray; @@ -103,7 +106,7 @@ private: } virtual void visit(AstVarScope* nodep, AstNUser*) { allNodes(nodep); - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); if (m_counting) { if (nodep->varp()->dtypeSkipRefp()->castBasicDType()) { m_statVarScpBytes += nodep->varp()->dtypeSkipRefp()->widthTotalBytes(); @@ -114,13 +117,13 @@ private: UINFO(4," IF "<condp()->iterateAndNext(*this); + nodep->condp()->iterateAndNextConst(*this); // Track prediction if (m_counting) { ++m_statPred[nodep->branchPred()]; } if (!m_fast) { - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); } else { // See which path we want to take bool takeElse = false; @@ -135,11 +138,11 @@ private: m_counting = false; // Check if m_instrs = 0; - nodep->ifsp()->iterateAndNext(*this); + nodep->ifsp()->iterateAndNextConst(*this); double instrIf = m_instrs; // Check else m_instrs = 0; - nodep->elsesp()->iterateAndNext(*this); + nodep->elsesp()->iterateAndNextConst(*this); double instrElse = m_instrs; // Max of if or else condition takeElse = (instrElse > instrIf); @@ -150,9 +153,9 @@ private: // Count the block if (m_counting) { if (takeElse) { - nodep->elsesp()->iterateAndNext(*this); + nodep->elsesp()->iterateAndNextConst(*this); } else { - nodep->ifsp()->iterateAndNext(*this); + nodep->ifsp()->iterateAndNextConst(*this); } } } @@ -163,7 +166,7 @@ private: virtual void visit(AstCCall* nodep, AstNUser*) { //UINFO(4," CCALL "<iterateChildren(*this); + nodep->iterateChildrenConst(*this); if (m_fast) { // Enter the function and trace it nodep->funcp()->accept(*this); @@ -172,12 +175,12 @@ private: virtual void visit(AstCFunc* nodep, AstNUser*) { m_cfuncp = nodep; allNodes(nodep); - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); m_cfuncp = NULL; } virtual void visit(AstNode* nodep, AstNUser*) { allNodes(nodep); - nodep->iterateChildren(*this); + nodep->iterateChildrenConst(*this); } public: // CONSTRUCTORS