Internals: Rework V3Param, V3Width and V3Const to return a AstNode* representing

any replacement edit they made to the passed in node.  Assure all callers use it
and/or comment on non-use.  Hopefully no function changed.

src/V3Active.cpp

@@ -84,7 +84,7 @@ private:
     }
     virtual void visit(AstSenTree* nodep, AstNUser*) {
 	// Simplify sensitivity list
-	V3Const::constifyTreeExpensive(nodep);
+	V3Const::constifyExpensiveEdit(nodep); nodep=NULL;
     }
     // Empty visitors, speed things up
     virtual void visit(AstNodeStmt* nodep, AstNUser*) { }

src/V3ActiveTop.cpp

@@ -80,9 +80,9 @@ private:
     }
     virtual void visit(AstActive* nodep, AstNUser*) {
 	UINFO(4,"   ACTIVE "<<nodep<<endl);
+	V3Const::constifyExpensiveEdit(nodep); // Remove duplicate clocks and such; sensesp() may change!
 	AstSenTree* sensesp = nodep->sensesp();
 	if (!sensesp) nodep->v3fatalSrc("NULL");
-	V3Const::constifyTreeExpensive(nodep);	// Remove duplicate clocks and such
 	if (sensesp->sensesp()
 	    && sensesp->sensesp()->castSenItem()
 	    && sensesp->sensesp()->castSenItem()->isNever()) {

src/V3Ast.cpp

@@ -778,6 +778,42 @@ void AstNode::iterateAndNextIgnoreEdit(AstNVisitor& v, AstNUser* vup) {
     }
 }
 
+AstNode* AstNode::acceptSubtreeReturnEdits(AstNVisitor& v, AstNUser* vup) {
+    // Some visitors perform tree edits (such as V3Const), and may even
+    // replace/delete the exact nodep that the visitor is called with.  If
+    // this happens, the parent will loose the handle to the node that was
+    // processed.
+    // To solve this, this function returns the pointer to the replacement node,
+    // which in many cases is just the same node that was passed in.
+    AstNode* nodep = this;  // Note "this" may point to bogus point later in this function
+    if (nodep->castNetlist()) {
+	// Calling on top level; we know the netlist won't get replaced
+	nodep->accept(v, vup);
+    } else if (!nodep->backp()) {
+	// Calling on standalone tree; insert a shim node so we can keep track, then delete it on completion
+	AstBegin* tempp = new AstBegin(nodep->fileline(),"[EditWrapper]",nodep);
+	{
+	    tempp->stmtsp()->accept(v, vup);  nodep=NULL; // nodep to null as may be replaced
+	}
+	nodep = tempp->stmtsp()->unlinkFrBackWithNext();
+	tempp->deleteTree(); tempp=NULL;
+    } else {
+	// Use back to determine who's pointing at us (IE assume new node grafts into same place as old one)
+	AstNode** nextnodepp = NULL;
+	if (this->m_backp->m_op1p == this) nextnodepp = &(this->m_backp->m_op1p);
+	else if (this->m_backp->m_op2p == this) nextnodepp = &(this->m_backp->m_op2p);
+	else if (this->m_backp->m_op3p == this) nextnodepp = &(this->m_backp->m_op3p);
+	else if (this->m_backp->m_op4p == this) nextnodepp = &(this->m_backp->m_op4p);
+	else if (this->m_backp->m_nextp == this) nextnodepp = &(this->m_backp->m_nextp);
+	if (!nextnodepp) this->v3fatalSrc("Node's back doesn't point to forward to node itself");
+	{
+	    nodep->accept(v, vup); nodep=NULL; // nodep to null as may be replaced
+	}
+	nodep = *nextnodepp;  // Grab new node from point where old was connected
+    }
+    return nodep;
+}
+
 //======================================================================
 
 void AstNode::cloneRelinkTree() {
@@ -958,6 +994,7 @@ void AstNode::v3errorEnd(ostringstream& str) const {
 }
 
 //######################################################################
+// AstNVisitor
 
 void AstNVisitor::doDeletes() {
     for (vector<AstNode*>::iterator it = m_deleteps.begin(); it != m_deleteps.end(); ++it) {

src/V3Ast.h

@@ -791,6 +791,7 @@ public:
     void	iterateAndNextIgnoreEdit(AstNVisitor& v, AstNUser* vup=NULL);
     void	iterateChildren(AstNVisitor& v, AstNUser* vup=NULL);  // Excludes following this->next
     void	iterateChildrenBackwards(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
     AstNode*	castNode() { return this; }

src/V3Const.cpp

@@ -1717,8 +1717,7 @@ private:
 	} else {
 	    // Calculate the width of this operation
 	    if (m_params && !nodep->width()) {
-		V3Width::widthParams(nodep);
+		nodep = V3Width::widthParamsEdit(nodep);
-		V3Signed::signedParams(nodep);
 	    }
 	    nodep->iterateChildren(*this);
 	}
@@ -1736,57 +1735,60 @@ public:
 	m_scopep = NULL;
     }
     virtual ~ConstVisitor() {}
-    void main(AstNode* nodep) {
+    AstNode* mainAcceptEdit(AstNode* nodep) {
 	// Operate starting at a random place
-	nodep->accept(*this);
+	return nodep->acceptSubtreeReturnEdits(*this);
     }
 };
 
 //######################################################################
 // Const class functions
 
-void V3Const::constifyParam(AstNode* nodep) {
+AstNode* V3Const::constifyParamsEdit(AstNode* nodep) {
     //if (debug()>0) nodep->dumpTree(cout,"  forceConPRE : ");
-    V3Width::widthSignedIfNotAlready(nodep); // Make sure we've sized everything first
+    nodep = V3Width::widthParamsEditIfNeed(nodep); // Make sure we've sized everything first
     ConstVisitor visitor (true,false,false,false);
     if (AstVar* varp=nodep->castVar()) {
 	// If a var wants to be constified, it's really a param, and
 	// we want the value to be constant.  We aren't passed just the
 	// init value because we need widthing above to handle the var's type.
-	if (varp->initp()) visitor.main(varp->initp());
+	if (varp->initp()) visitor.mainAcceptEdit(varp->initp());
     } else {
-	visitor.main(nodep);
+	nodep = visitor.mainAcceptEdit(nodep);
     }
     // Because we do edits, nodep links may get trashed and core dump this.
     //if (debug()>0) nodep->dumpTree(cout,"  forceConDONE: ");
+    return nodep;
 }
 
 void V3Const::constifyAll(AstNetlist* nodep) {
     // Only call from Verilator.cpp, as it uses user#'s
     UINFO(2,__FUNCTION__<<": "<<endl);
     ConstVisitor visitor (false,true,false,false);
-    visitor.main(nodep);
+    (void)visitor.mainAcceptEdit(nodep);
 }
 
 void V3Const::constifyAllLint(AstNetlist* nodep) {
     // Only call from Verilator.cpp, as it uses user#'s
     UINFO(2,__FUNCTION__<<": "<<endl);
     ConstVisitor visitor (false,false,true,false);
-    visitor.main(nodep);
+    (void)visitor.mainAcceptEdit(nodep);
 }
 
 void V3Const::constifyCpp(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
     ConstVisitor visitor (false,false,false,true);
-    visitor.main(nodep);
+    (void)visitor.mainAcceptEdit(nodep);
 }
 
-void V3Const::constifyTree(AstNode* nodep) {
+AstNode* V3Const::constifyEdit(AstNode* nodep) {
     ConstVisitor visitor (false,false,false,false);
-    visitor.main(nodep);
+    nodep = visitor.mainAcceptEdit(nodep);
+    return nodep;
 }
 
-void V3Const::constifyTreeExpensive(AstNode* nodep) {
+AstNode* V3Const::constifyExpensiveEdit(AstNode* nodep) {
     ConstVisitor visitor (false,true,false,false);
-    visitor.main(nodep);
+    nodep = visitor.mainAcceptEdit(nodep);
+    return nodep;
 }

src/V3Const.h

@@ -32,7 +32,8 @@
 class V3Const {
 public:
     // Force this cell node's parameter list to become a constant
-    static void constifyParam(AstNode* nodep);
+    // Return new node that may have replaced nodep
+    static AstNode* constifyParamsEdit(AstNode* nodep);
     // Everything that's possible
     static void constifyAll(AstNetlist* nodep);
     // Also, warn
@@ -40,9 +41,11 @@ public:
     // C++ datatypes
     static void constifyCpp(AstNetlist* nodep);
     // Only the current node and lower
-    static void constifyTree(AstNode* nodep);
+    // Return new node that may have replaced nodep
+    static AstNode* constifyEdit(AstNode* nodep);
     // Only the current node and lower, with special SenTree optimization
-    static void constifyTreeExpensive(AstNode* nodep);
+    // Return new node that may have replaced nodep
+    static AstNode* constifyExpensiveEdit(AstNode* nodep);
 };
 
 #endif // Guard

src/V3Gate.cpp

@@ -688,7 +688,7 @@ void GateVisitor::optimizeElimVar(AstVarScope* varscp, AstNode* substp, AstNode*
 	if (debug()>=9) consumerp->dumpTree(cout,"\telimUseCns: ");
 	//Caution: Can't let V3Const change our handle to consumerp, such as by
 	// optimizing away this assignment, etc.
-	V3Const::constifyTree(consumerp);
+	consumerp = V3Const::constifyEdit(consumerp);
 	if (debug()>=5) consumerp->dumpTree(cout,"\telimUseDne: ");
     }
 }

src/V3Life.cpp

@@ -318,7 +318,7 @@ private:
 	if (lastEdit != AstNode::editCountGbl()) {
 	    // We changed something, try to constant propagate, but don't delete the
 	    // assignment as we still need nodep to remain.
-	    V3Const::constifyTree(nodep->rhsp());
+	    V3Const::constifyEdit(nodep->rhsp());  // rhsp may change
 	}
 	// Has to be direct assignment without any EXTRACTing.
 	if (nodep->lhsp()->castVarRef() && !m_sideEffect) {

src/V3Order.cpp

@@ -1096,7 +1096,8 @@ void OrderVisitor::processDomainsIterate(OrderEitherVertex* vertexp) {
 		    AstNodeSenItem* newtree2p = fromVertexp->domainp()->sensesp()->cloneTree(true);
 		    if (!newtree2p) fromVertexp->domainp()->v3fatalSrc("No senitem found under clocked domain");
 		    newtreep->addSensesp(newtree2p);
-		    V3Const::constifyTreeExpensive(newtreep);	// Remove duplicates
+		    newtree2p=NULL; // Below edit may replace it
+		    V3Const::constifyExpensiveEdit(newtreep);	// Remove duplicates
 		    newtreep->multi(true);	// Comment that it was made from 2 clock domains
 		    domainp = m_finder.getSenTree(domainp->fileline(), newtreep);
 		    if (ddebug) {

src/V3Param.cpp

@@ -131,7 +131,7 @@ private:
     virtual void visit(AstVar* nodep, AstNUser*) {
 	if (nodep->isParam()) {
 	    if (!nodep->hasSimpleInit()) { nodep->v3fatalSrc("Parameter without initial value"); }
-	    V3Const::constifyParam(nodep);  // The variable, not just the var->init()
+	    V3Const::constifyParamsEdit(nodep);  // The variable, not just the var->init()
 	}
     }
 
@@ -151,9 +151,8 @@ private:
 	nodep->deleteTree(); nodep=NULL;
     }
     virtual void visit(AstGenIf* nodep, AstNUser*) {
-	V3Width::widthParams(nodep);  // Param typed widthing will NOT recurse the body
+	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body
-	V3Signed::signedParams(nodep);
+	V3Const::constifyParamsEdit(nodep->condp());  // condp may change
-	V3Const::constifyParam(nodep->condp());
 	if (AstConst* constp = nodep->condp()->castConst()) {
 	    AstNode* keepp = (constp->isZero()
 			      ? nodep->elsesp()
@@ -172,22 +171,20 @@ private:
     virtual void visit(AstGenFor* nodep, AstNUser*) {
 	// We parse a very limited form of FOR, so we don't need to do a full
 	// simulation to unroll the loop
-	V3Width::widthParams(nodep);  // Param typed widthing will NOT recurse the body
+	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body
-	V3Signed::signedParams(nodep);
 	V3Unroll::unrollGen(nodep); nodep=NULL;
     }
     virtual void visit(AstGenCase* nodep, AstNUser*) {
 	AstNode* keepp = NULL;
 	V3Case::caseLint(nodep);
-	V3Width::widthParams(nodep);  // Param typed widthing will NOT recurse the body
+	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body
-	V3Signed::signedParams(nodep);
+	V3Const::constifyParamsEdit(nodep->exprp());  // exprp may change
-	V3Const::constifyParam(nodep->exprp());
 	AstConst* exprp = nodep->exprp()->castConst();
 	// Constify
 	for (AstCaseItem* itemp = nodep->itemsp(); itemp; itemp=itemp->nextp()->castCaseItem()) {
 	    for (AstNode* ep = itemp->condsp(); ep; ) {
 		AstNode* nextp = ep->nextp(); //May edit list
-		V3Const::constifyParam(ep);
+		V3Const::constifyParamsEdit(ep); ep=NULL; // ep may change
 		ep = nextp;
 	    }
 	}
@@ -248,7 +245,7 @@ void ParamVisitor::visit(AstCell* nodep, AstNUser*) {
 	// Create new module name with _'s between the constants
 	if (debug()>9) nodep->dumpTree(cout,"cell:\t");
 	// Evaluate all module constants
-	V3Const::constifyParam(nodep);
+	V3Const::constifyParamsEdit(nodep);
 
 	// Make sure constification worked
 	// Must be a separate loop, as constant conversion may have changed some pointers.

src/V3Signed.cpp

@@ -359,12 +359,14 @@ private:
 
 public:
     // CONSTRUCTORS
-    SignedVisitor(AstNode* nodep, bool paramsOnly) {
+    SignedVisitor(bool paramsOnly) {
 	m_paramsOnly = paramsOnly;
 	m_taskDepth = 0;
-	nodep->accept(*this);
     }
     virtual ~SignedVisitor() {}
+    AstNode* mainAcceptEdit(AstNode* nodep) {
+	return nodep->acceptSubtreeReturnEdits(*this);
+    }
 };
 
 //######################################################################
@@ -391,10 +393,11 @@ private:
     }
 public:
     // CONSTRUCTORS
-    SignedRemoveVisitor(AstNode* nodep) {
+    SignedRemoveVisitor() {}
-	nodep->accept(*this);
-    }
     virtual ~SignedRemoveVisitor() {}
+    AstNode* mainAcceptEdit(AstNode* nodep) {
+	return nodep->acceptSubtreeReturnEdits(*this);
+    }
 };
 
 //######################################################################
@@ -402,12 +405,15 @@ public:
 
 void V3Signed::signedAll(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
-    SignedVisitor visitor (nodep, false);
+    (void)signedParamsEdit(nodep);
-    SignedRemoveVisitor rvisitor (nodep);
 }
 
-void V3Signed::signedParams(AstNode* nodep) {
+AstNode* V3Signed::signedParamsEdit(AstNode* nodep) {
+    // Only called from V3Width::widthParamsEdit
     UINFO(4,__FUNCTION__<<": "<<endl);
-    SignedVisitor visitor (nodep, true);
+    SignedVisitor visitor (true);
-    SignedRemoveVisitor rvisitor (nodep);
+    nodep = visitor.mainAcceptEdit(nodep);
+    SignedRemoveVisitor rvisitor;
+    nodep = rvisitor.mainAcceptEdit(nodep);
+    return nodep;
 }

src/V3Signed.h

@@ -32,7 +32,9 @@
 class V3Signed {
 public:
     static void signedAll(AstNetlist* nodep);
-    static void signedParams(AstNode* nodep);
+protected:
+    friend class V3Width;  // Use widthParamsEdit instead of signedParamsEdit
+    static AstNode* signedParamsEdit(AstNode* nodep); // May replace nodep
 };
 
 #endif // Guard

src/V3Simulate.h

@@ -503,7 +503,8 @@ private:
 	UINFO(5,"   FUNCREF "<<nodep<<endl);
 	if (!m_params) { badNodeType(nodep); return; }
 	AstFunc* funcp = nodep->taskp()->castFunc(); if (!funcp) nodep->v3fatalSrc("Not linked");
-	V3Width::widthSignedIfNotAlready(funcp); // Make sure we've sized the function
+	V3Width::widthParamsEditIfNeed(funcp); funcp=NULL; // Make sure we've sized the function
+	funcp = nodep->taskp()->castFunc(); if (!funcp) nodep->v3fatalSrc("Not linked");
 	// Apply function call values to function
 	// Note we'd need a stack if we allowed recursive functions!
 	AstNode* pinp = nodep->pinsp();  AstNode* nextpinp = NULL;

src/V3Unknown.cpp

@@ -71,19 +71,6 @@ private:
 	return level;
     }
 
-    AstNode* constify(AstNode* nodep) {
-	// Put the new nodes under a temporary XOR operator we'll rip up in a moment.
-	// This way when we constify, if the expression
-	// is a constant it will still be under the XOR.
-	AstXor* newp = new AstXor (nodep->fileline(), nodep,
-				   new AstConst (nodep->fileline(), 0)); // Just so it's a valid XOR
-	V3Const::constifyTree(newp->lhsp()); nodep=NULL; // Just the conditional
-	// Nodep may now be a changed object like a constant
-	nodep = newp->lhsp()->unlinkFrBack();
-	newp->deleteTree(); newp=NULL;
-	return nodep;
-    }
-
     void replaceBoundLvalue(AstNode* nodep, AstNode* condp) {
 	// Spec says a out-of-range LHS SEL results in a NOP.
 	// This is a PITA.  We could:
@@ -184,11 +171,11 @@ private:
     }
     void visitEqNeqCase(AstNodeBiop* nodep) {
 	UINFO(4," N/EQCASE->EQ "<<nodep<<endl);
-	V3Const::constifyTree(nodep->lhsp());
+	V3Const::constifyEdit(nodep->lhsp());  // lhsp may change
-	V3Const::constifyTree(nodep->rhsp());
+	V3Const::constifyEdit(nodep->rhsp());  // rhsp may change
 	if (nodep->lhsp()->castConst() && nodep->rhsp()->castConst()) {
 	    // Both sides are constant, node can be constant
-	    V3Const::constifyTree(nodep); nodep=NULL;
+	    V3Const::constifyEdit(nodep); nodep=NULL;
 	    return;
 	} else {
 	    AstNode* lhsp = nodep->lhsp()->unlinkFrBack();
@@ -214,11 +201,11 @@ private:
     }
     void visitEqNeqWild(AstNodeBiop* nodep) {
 	UINFO(4," N/EQWILD->EQ "<<nodep<<endl);
-	V3Const::constifyTree(nodep->lhsp());
+	V3Const::constifyEdit(nodep->lhsp());  // lhsp may change
-	V3Const::constifyTree(nodep->rhsp());
+	V3Const::constifyEdit(nodep->rhsp());  // rhsp may change
 	if (nodep->lhsp()->castConst() && nodep->rhsp()->castConst()) {
 	    // Both sides are constant, node can be constant
-	    V3Const::constifyTree(nodep); nodep=NULL;
+	    V3Const::constifyEdit(nodep); nodep=NULL;
 	    return;
 	} else {
 	    AstNode* lhsp = nodep->lhsp()->unlinkFrBack();
@@ -356,7 +343,8 @@ private:
 	    AstNode* condp = new AstLte (nodep->fileline(),
 					 nodep->lsbp()->cloneTree(false),
 					 new AstConst(nodep->fileline(), maxlsbnum));
-	    condp = constify(condp);
+	    // Note below has null backp(); the Edit function knows how to deal with that.
+	    condp = V3Const::constifyEdit(condp);
 	    if (condp->isOne()) {
 		// We don't need to add a conditional; we know the existing expression is ok
 		condp->deleteTree();
@@ -367,9 +355,6 @@ private:
 		nodep->unlinkFrBack(&replaceHandle);
 		V3Number xnum (nodep->fileline(), nodep->width());
 		xnum.setAllBitsX();
-		// Put the new nodes under a temporary XOR operator we'll rip up in a moment.
-		// This way when we constify, if the expression
-		// is a constant it will still be under the XOR.
 		AstNode* newp = new AstCondBound (nodep->fileline(),
 						  condp,
 						  nodep,
@@ -412,7 +397,8 @@ private:
 	    AstNode* condp = new AstLte (nodep->fileline(),
 					 nodep->bitp()->cloneTree(false),
 					 new AstConst(nodep->fileline(), widthnum));
-	    condp = constify(condp);
+	    // Note below has null backp(); the Edit function knows how to deal with that.
+	    condp = V3Const::constifyEdit(condp);
 	    if (condp->isOne()) {
 		// We don't need to add a conditional; we know the existing expression is ok
 		condp->deleteTree();
@@ -424,9 +410,6 @@ private:
 		nodep->unlinkFrBack(&replaceHandle);
 		V3Number xnum (nodep->fileline(), nodep->width());
 		xnum.setAllBitsX();
-		// Put the new nodes under a temporary XOR operator we'll rip up in a moment.
-		// This way when we constify, if the expression
-		// is a constant it will still be under the XOR.
 		AstNode* newp = new AstCondBound (nodep->fileline(),
 						  condp,
 						  nodep,

src/V3Unroll.cpp

@@ -108,7 +108,7 @@ private:
 	if (nodep->castGenFor() && !m_forVarp->isGenVar()) {
 	    nodep->v3error("Non-genvar used in generate for: "<<m_forVarp->name()<<endl);
 	}
-	if (m_generate) V3Const::constifyParam(initAssp->rhsp());
+	if (m_generate) V3Const::constifyParamsEdit(initAssp->rhsp());  // rhsp may change
 	AstConst* constInitp = initAssp->rhsp()->castConst();
 	if (!constInitp) return cantUnroll(nodep, "non-constant initializer");
 	//
@@ -142,7 +142,7 @@ private:
 	// Subtracts have it on the rhs, because you write i=i-1; i=1-i is non-sensible.
 	AstConst* preconstIncp = (subtract ? incInstrp->rhsp()->castConst()
 				  : incInstrp->lhsp()->castConst());
-	if (m_generate) V3Const::constifyParam(preconstIncp);
+	if (m_generate) preconstIncp = V3Const::constifyParamsEdit(preconstIncp)->castConst();
 	AstConst* constIncp = (subtract ? incInstrp->rhsp()->castConst()
 			       : incInstrp->lhsp()->castConst());
 	UINFO(8, "   Inc expr ok:  "<<constIncp<<endl);
@@ -161,7 +161,7 @@ private:
 	if (condBip->lhsp()->castVarRef()->varp() != m_forVarp
 	    || condBip->lhsp()->castVarRef()->varScopep() != m_forVscp)
 	    return cantUnroll(nodep, "different variable in condition");
-	if (m_generate) V3Const::constifyParam(condBip->rhsp());
+	if (m_generate) V3Const::constifyParamsEdit(condBip->rhsp());  // rhsp may change
 	AstConst* constStopp = condBip->rhsp()->castConst();
 	if (!constStopp) return cantUnroll(nodep, "non-constant final value");
 	UINFO(8, "   Stop expr ok: "<<constStopp<<endl);
@@ -299,17 +299,17 @@ private:
 	if (m_varModeCheck || m_varModeReplace) {
 	} else {
 	    // Constify before unroll call, as it may change what is underneath.
-	    if (nodep->precondsp()) V3Const::constifyTree(nodep->precondsp());
+	    if (nodep->precondsp()) V3Const::constifyEdit(nodep->precondsp());  // precondsp may change
-	    if (nodep->condp()) V3Const::constifyTree(nodep->condp());
+	    if (nodep->condp()) V3Const::constifyEdit(nodep->condp()); //condp may change
 	    // Grab initial value
 	    AstNode* initp = NULL;  // Should be statement before the while.
 	    if (nodep->backp()->nextp() == nodep) initp=nodep->backp();
-	    if (initp) V3Const::constifyTree(initp);
+	    if (initp) { V3Const::constifyEdit(initp); initp=NULL; }
-	    initp = NULL; if (nodep->backp()->nextp() == nodep) initp=nodep->backp();
+	    if (nodep->backp()->nextp() == nodep) initp=nodep->backp();
 	    // Grab assignment
 	    AstNode* incp = NULL;  // Should be last statement
 	    for (incp = nodep->bodysp(); incp && incp->nextp(); incp = incp->nextp()) {}
-	    if (incp) V3Const::constifyTree(incp);
+	    if (incp) { V3Const::constifyEdit(incp); incp=NULL; }
 	    for (incp = nodep->bodysp(); incp && incp->nextp(); incp = incp->nextp()) {}  // Again, as may have changed
 	    // And check it
 	    if (forUnrollCheck(nodep, initp,
@@ -326,9 +326,9 @@ private:
 	if (m_varModeCheck || m_varModeReplace) {
 	} else {
 	    // Constify before unroll call, as it may change what is underneath.
-	    if (nodep->initsp()) V3Const::constifyTree(nodep->initsp());
+	    if (nodep->initsp()) V3Const::constifyEdit(nodep->initsp());  // initsp may change
-	    if (nodep->condp()) V3Const::constifyTree(nodep->condp());
+	    if (nodep->condp()) V3Const::constifyEdit(nodep->condp());  // condp may change
-	    if (nodep->incsp()) V3Const::constifyTree(nodep->incsp());
+	    if (nodep->incsp()) V3Const::constifyEdit(nodep->incsp());  // incsp may change
 	    if (nodep->condp()->isZero()) {
 		// We don't need to do any loops.  Remove the GenFor,
 		// Genvar's don't care about any initial assignments.

src/V3Width.cpp

@@ -231,7 +231,7 @@ private:
 	if (vup->c()->prelim()) {
 	    nodep->lhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p());
 	    nodep->rhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p());
-	    V3Const::constifyParam(nodep->rhsp());
+	    V3Const::constifyParamsEdit(nodep->rhsp()); // rhsp may change
 	    AstConst* constp = nodep->rhsp()->castConst();
 	    if (!constp) { nodep->v3error("Replication value isn't a constant."); return; }
 	    uint32_t times = constp->toUInt();
@@ -252,8 +252,8 @@ private:
 	if (vup->c()->prelim()) {
 	    nodep->msbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p());
 	    nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p());
-	    V3Const::constifyParam(nodep->msbp());
+	    V3Const::constifyParamsEdit(nodep->msbp()); // msbp may change
-	    V3Const::constifyParam(nodep->lsbp());
+	    V3Const::constifyParamsEdit(nodep->lsbp()); // lsbp may change
 	    AstConst* msbConstp = nodep->msbp()->castConst();
 	    AstConst* lsbConstp = nodep->lsbp()->castConst();
 	    if (!msbConstp || !lsbConstp) {
@@ -287,7 +287,7 @@ private:
 	    nodep->fromp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p());
 	    nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p());
 	    nodep->widthp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p());
-	    V3Const::constifyParam(nodep->widthp());
+	    V3Const::constifyParamsEdit(nodep->widthp()); // widthp may change
 	    AstConst* widthConstp = nodep->widthp()->castConst();
 	    if (!widthConstp) {
 		nodep->v3error("Width of bit extract isn't a constant");
@@ -849,12 +849,14 @@ private:
 
 public:
     // CONSTUCTORS
-    WidthVisitor(AstNode* nodep, bool paramsOnly) {
+    WidthVisitor(bool paramsOnly) {
 	m_paramsOnly = paramsOnly;
 	m_taskDepth = 0;
 	m_cellRangep = NULL;
 	m_casep = NULL;
-	nodep->accept(*this, WidthVP(ANYSIZE,0,BOTH).p());
+    }
+    AstNode* mainAcceptEdit(AstNode* nodep) {
+	return nodep->acceptSubtreeReturnEdits(*this, WidthVP(ANYSIZE,0,BOTH).p());
     }
     virtual ~WidthVisitor() {}
 };
@@ -1205,24 +1207,27 @@ public:
 void V3Width::width(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
     // We should do it in bottom-up module order, but it works in any order.
-    WidthVisitor visitor (nodep, false);
+    WidthVisitor visitor (false);
+    (void)visitor.mainAcceptEdit(nodep);
 }
 
-void V3Width::widthParams(AstNode* nodep) {
+AstNode* V3Width::widthParamsEdit(AstNode* nodep) {
     UINFO(4,__FUNCTION__<<": "<<endl);
     // We should do it in bottom-up module order, but it works in any order.
-    WidthVisitor visitor (nodep, true);
+    WidthVisitor visitor (true);
+    nodep = visitor.mainAcceptEdit(nodep);
+    nodep = V3Signed::signedParamsEdit(nodep);
+    return nodep;
 }
 
-void V3Width::widthSignedIfNotAlready(AstNode* nodep) {
+AstNode* V3Width::widthParamsEditIfNeed(AstNode* nodep) {
     if (!nodep->width()) {
-	V3Width::widthParams(nodep);
+	nodep = V3Width::widthParamsEdit(nodep);
-	V3Signed::signedParams(nodep);
     }
+    return nodep;
 }
 
 void V3Width::widthCommit(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
     WidthCommitVisitor visitor (nodep);
 }
-

src/V3Width.h

@@ -33,8 +33,8 @@ class V3Width {
 public:
     static void width(AstNetlist* nodep);
     // Smaller step... Only do a single node for parameter propagation
-    static void widthParams(AstNode* nodep);
+    static AstNode* widthParamsEdit(AstNode* nodep);
-    static void widthSignedIfNotAlready(AstNode* nodep);
+    static AstNode* widthParamsEditIfNeed(AstNode* nodep);
     // Final step... Mark all widths as equal
     static void widthCommit(AstNetlist* nodep);
 };