diff --git a/Changes b/Changes index 0bb0f137a..8ec76588d 100644 --- a/Changes +++ b/Changes @@ -5,14 +5,16 @@ indicates the contributor was also the author of the fix; Thanks! * Verilator 3.62** -**** Fix link error when using --exe with --trace. [Eugene Weber] - -**** Public functions now allow > 64 bit arguments. - -**** Don't core dump on errors when not under --debug. [Allan Cochrane] +*** Public functions now allow > 64 bit arguments. **** Remove .vpp intermediate files when not under --debug. +**** Fix link error when using --exe with --trace. [Eugene Weber] + +**** Fix mis-optimization of wide concats with constants. + +**** Fix core dump on printing error when not under --debug. [Allan Cochrane] + * Verilator 3.620 10/04/2006 *** Support simple inout task ports. [Eugene Weber] @@ -436,7 +438,7 @@ indicates the contributor was also the author of the fix; Thanks! **** Added NC Verilog as alternative to VCS for reference tests. **** Support implicit wire declarations on input-only signals. - (Dangerous, as leads to wires without drivers, but allowed by spec.) + (Dangerous, as leads to wires without drivers, but allowed by spec.) **** Fixed compile warnings on Suse 9.1 diff --git a/src/V3Expand.cpp b/src/V3Expand.cpp index 81eb77a35..3f099e166 100644 --- a/src/V3Expand.cpp +++ b/src/V3Expand.cpp @@ -660,6 +660,9 @@ private: // Lhs or Rhs may be word, long, or quad. // newAstWordSelClone nicely abstracts the difference. int rhsshift = rhsp->rhsp()->widthMin(); + // Sometimes doing the words backwards is preferrable. + // When we have x={x,foo} backwards is better, when x={foo,x} forward is better + // However V3Subst tends to rip this up, so not worth optimizing now. for (int w=0; wwidthWords(); w++) { addWordAssign(nodep, w, new AstOr (rhsp->fileline(), diff --git a/src/V3Subst.cpp b/src/V3Subst.cpp index b1d9ac1be..cc11e3c35 100644 --- a/src/V3Subst.cpp +++ b/src/V3Subst.cpp @@ -19,10 +19,11 @@ // //************************************************************************* // V3Subst's Transformations: -// +// // Each module: // Search all ASSIGN(WORDSEL(...)) and build what it's assigned to -// Later usages of that word may then be replaced +// Later usages of that word may then be replaced as long as +// the RHS hasn't changed value. // //************************************************************************* @@ -44,7 +45,7 @@ class SubstBaseVisitor : public AstNVisitor { public: - static int debug() { return 0; } + static int debug() { return V3Error::debugDefault(); } // static int debug() { return 9; } }; @@ -52,14 +53,17 @@ public: // Class for each word of a multi-word variable class SubstVarWord { -private: +protected: + // MEMBERS AstNodeAssign* m_assignp; // Last assignment to each word of this var + int m_step; // Step number of last assignment bool m_use; // True if each word was consumed bool m_complex; // True if each word is complex friend class SubstVarEntry; // METHODS void clear() { m_assignp = NULL; + m_step = 0; m_use = false; m_complex = false; } @@ -69,6 +73,7 @@ private: // Class for every variable we may process class SubstVarEntry { + // MEMBERS AstVar* m_varp; // Variable this tracks bool m_wordAssign; // True if any word assignments bool m_wordUse; // True if any individual word usage @@ -76,6 +81,7 @@ class SubstVarEntry { vector m_words; // Data for every word, if multi word variable int debug() { return SubstBaseVisitor::debug(); } public: + // CONSTRUCTORS SubstVarEntry (AstVar* varp) { // Construction for when a var is used m_varp = varp; m_whole.m_use = false; @@ -88,27 +94,34 @@ public: } } ~SubstVarEntry() {} - bool wordNumOk(int word) { +private: + // METHODS + bool wordNumOk(int word) const { return word < m_varp->widthWords(); } - AstNodeAssign* getWordAssignp(int word) { + AstNodeAssign* getWordAssignp(int word) const { if (!wordNumOk(word)) return NULL; else return m_words[word].m_assignp; } - void assignWhole (AstNodeAssign* assp) { +public: + void assignWhole (int step, AstNodeAssign* assp) { if (m_whole.m_assignp) m_whole.m_complex = true; m_whole.m_assignp = assp; + m_whole.m_step = step; } - void assignWord (int word, AstNodeAssign* assp) { + void assignWord (int step, int word, AstNodeAssign* assp) { if (!wordNumOk(word) || getWordAssignp(word) || m_words[word].m_complex) m_whole.m_complex = true; m_wordAssign = true; - if (wordNumOk(word)) m_words[word].m_assignp = assp; + if (wordNumOk(word)) { + m_words[word].m_assignp = assp; + m_words[word].m_step = step; + } } - void assignWordComplex (int word) { + void assignWordComplex (int step, int word) { if (!wordNumOk(word) || getWordAssignp(word) || m_words[word].m_complex) m_whole.m_complex = true; m_words[word].m_complex = true; } - void assignComplex() { + void assignComplex(int step) { m_whole.m_complex = true; } void consumeWhole() { //==consumeComplex as we don't know the difference @@ -138,6 +151,12 @@ public: return NULL; } } + int getWholeStep() const { + return m_whole.m_step; + } + int getWordStep(int word) const { + if (!wordNumOk(word)) return 0; else return m_words[word].m_step; + } void deleteAssign (AstNodeAssign* nodep) { UINFO(5, "Delete "<unlinkFrBack()->deleteTree(); nodep=NULL; @@ -155,18 +174,67 @@ public: } }; +//###################################################################### +// See if any variables have changed value since we determined subst value, as a visitor of each AstNode + +class SubstUseVisitor : public SubstBaseVisitor { +private: + // NODE STATE + // See SubstVisitor + // + // STATE + int m_origStep; // Step number where subst was recorded + bool m_ok; // No misassignments found + + // METHODS + SubstVarEntry* findEntryp(AstVarRef* nodep) { + return (SubstVarEntry*)(nodep->varp()->userp()); // Might be NULL + } + // VISITORS + virtual void visit(AstVarRef* nodep, AstNUser*) { + SubstVarEntry* entryp = findEntryp (nodep); + if (entryp) { + // Don't sweat it. We assign a new temp variable for every new assignment, + // so there's no way we'd ever replace a old value. + } else { + // A simple variable; needs checking. + if (m_origStep < nodep->varp()->user2()) { + if (m_ok) UINFO(9," RHS variable changed since subst recorded: "<iterateChildren(*this); + } +public: + // CONSTUCTORS + SubstUseVisitor(AstNode* nodep, int origStep) { + UINFO(9, " SubstUseVisitor "<accept(*this); + } + virtual ~SubstUseVisitor() {} + // METHODS + bool ok() const { return m_ok; } +}; + //###################################################################### // Subst state, as a visitor of each AstNode class SubstVisitor : public SubstBaseVisitor { private: // NODE STATE - // Passed to SubstElimVisitor + // Passed to SubstUseVisitor // AstVar::userp -> SubstVar* for usage var, 0=not set yet + // AstVar::user2 -> int step number for last assignment, 0=not set yet // // STATE vector m_entryps; // Nodes to delete when we are finished int m_ops; // Number of operators on assign rhs + int m_assignStep; // Assignment number to determine var lifetime V3Double0 m_statSubsts; // Statistic tracking enum { SUBST_MAX_OPS_SUBST = 30, // Maximum number of ops to substitute in @@ -188,6 +256,7 @@ private: // VISITORS virtual void visit(AstNodeAssign* nodep, AstNUser*) { m_ops = 0; + m_assignStep++; nodep->rhsp()->iterateAndNext(*this); bool hit=false; if (AstVarRef* varrefp = nodep->lhsp()->castVarRef()) { @@ -196,10 +265,10 @@ private: hit = true; if (m_ops > SUBST_MAX_OPS_SUBST) { UINFO(8," ASSIGNtooDeep "<assignComplex(); + entryp->assignComplex(m_assignStep); } else { UINFO(8," ASSIGNwhole "<assignWhole(nodep); + entryp->assignWhole(m_assignStep, nodep); } } } @@ -212,10 +281,10 @@ private: hit = true; if (m_ops > SUBST_MAX_OPS_SUBST) { UINFO(8," ASSIGNtooDeep "<assignWordComplex(word); + entryp->assignWordComplex(m_assignStep, word); } else { UINFO(8," ASSIGNword"<assignWord(word, nodep); + entryp->assignWord(m_assignStep, word, nodep); } } } @@ -248,7 +317,13 @@ private: UINFO(8," USEword"<substWord (nodep, word)) { - replaceSubstEtc(nodep, substp); nodep=NULL; + // Check that the RHS hasn't changed value since we recorded it. + SubstUseVisitor visitor (substp, entryp->getWordStep(word)); + if (visitor.ok()) { + replaceSubstEtc(nodep, substp); nodep=NULL; + } else { + entryp->consumeWord(word); + } } else { entryp->consumeWord(word); } @@ -257,14 +332,27 @@ private: } } virtual void visit(AstVarRef* nodep, AstNUser*) { + // Any variable + if (nodep->lvalue()) { + m_assignStep++; + nodep->varp()->user2(m_assignStep); + UINFO(9, " ASSIGNstep u2="<varp()->user2()<<" "<varp()->isStatementTemp()) { SubstVarEntry* entryp = getEntryp (nodep); if (nodep->lvalue()) { UINFO(8," ASSIGNcpx "<assignComplex(); + entryp->assignComplex(m_assignStep); } else if (AstNode* substp = entryp->substWhole(nodep)) { - UINFO(8," USEwhole "<getWholeStep()); + if (visitor.ok()) { + UINFO(8," USEwhole "<consumeWhole(); + } } else { // Consumed w/o substitute UINFO(8," USEwtf "<consumeWhole(); @@ -272,6 +360,7 @@ private: } } virtual void visit(AstVar* nodep, AstNUser*) {} + virtual void visit(AstConst* nodep, AstNUser*) {} virtual void visit(AstNode* nodep, AstNUser*) { m_ops++; if (!nodep->isSubstOptimizable()) { @@ -283,7 +372,9 @@ public: // CONSTUCTORS SubstVisitor(AstNode* nodep) { AstNode::userClearTree(); // userp() used on entire tree + AstNode::user2ClearTree(); // user2p() used on entire tree m_ops = 0; + m_assignStep = 0; nodep->accept(*this); } virtual ~SubstVisitor() { diff --git a/test_regress/driver.pl b/test_regress/driver.pl index ce095598d..b430e7a09 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -673,8 +673,8 @@ sub verilator_version { sub files_identical { my $fn1 = shift; my $fn2 = shift; - my $f1 = IO::File->new ("<$fn1") or die "%Error: $! $fn1,"; - my $f2 = IO::File->new ("<$fn2") or die "%Error: $! $fn2,"; + my $f1 = IO::File->new ("<$fn1"); if (!$f1) { warn "%Error: $! $fn1\n"; return 0; } + my $f2 = IO::File->new ("<$fn2"); if (!$f2) { warn "%Error: $! $fn2\n"; return 0; } my @l1 = $f1->getlines(); my @l2 = $f2->getlines(); my $nl = $#l1; $nl = $#l2 if ($#l2 > $nl);