Fix function calls inside loop bounds

git-svn-id: file://localhost/svn/verilator/trunk/verilator@776 77ca24e4-aefa-0310-84f0-b9a241c72d87

Changes

@@ -20,6 +20,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **** Fix 3.600 internal error with arrayed instances.  [David Hewson]
 
+**** Fix 3.600 internal error with non-unrolled function loops.  [David Hewson]
+
 **** Fix $display %m name not matching Verilog name inside SystemC modules.
 
 **** Declare optimized lookup tables as 'static', to reduce D-Cache miss rate.

src/V3Ast.h

@@ -774,14 +774,14 @@ struct AstNodeAssign : public AstNodeStmt {
 
 struct AstNodeFor : public AstNodeStmt {
     AstNodeFor(FileLine* fileline, AstNode* initsp, AstNode* condp,
-	       AstNode* assignsp, AstNode* bodysp)
+	       AstNode* incsp, AstNode* bodysp)
 	: AstNodeStmt(fileline) {
-	addNOp1p(initsp); setOp2p(condp); addNOp3p(assignsp); addNOp4p(bodysp);
+	addNOp1p(initsp); setOp2p(condp); addNOp3p(incsp); addNOp4p(bodysp);
     }
     virtual ~AstNodeFor() {}
-    AstNode*	initsp()	const { return op1p()->castNode(); }	// op1= initial statement
+    AstNode*	initsp()	const { return op1p()->castNode(); }	// op1= initial statements
     AstNode*	condp()		const { return op2p()->castNode(); }	// op2= condition to continue
-    AstNode*	assignsp()	const { return op3p()->castNode(); }	// op3= final statements
+    AstNode*	incsp()		const { return op3p()->castNode(); }	// op3= increment statements
     AstNode*	bodysp()	const { return op4p()->castNode(); }	// op4= body of loop
     virtual bool isGateOptimizable() const { return false; }
     virtual bool isPredictOptimizable() const { return false; }

src/V3AstNodes.h

@@ -1242,8 +1242,8 @@ struct AstFOpen : public AstNodeStmt {
 
 struct AstGenFor : public AstNodeFor {
     AstGenFor(FileLine* fileline, AstNode* initsp, AstNode* condp,
-	   AstNode* assignsp, AstNode* bodysp)
+	   AstNode* incsp, AstNode* bodysp)
-	: AstNodeFor(fileline, initsp, condp, assignsp, bodysp) {
+	: AstNodeFor(fileline, initsp, condp, incsp, bodysp) {
     }
     virtual ~AstGenFor() {}
     virtual AstType type() const { return AstType::GENFOR;}
@@ -1253,8 +1253,8 @@ struct AstGenFor : public AstNodeFor {
 
 struct AstFor : public AstNodeFor {
     AstFor(FileLine* fileline, AstNode* initsp, AstNode* condp,
-	   AstNode* assignsp, AstNode* bodysp)
+	   AstNode* incsp, AstNode* bodysp)
-	: AstNodeFor(fileline, initsp, condp, assignsp, bodysp) {
+	: AstNodeFor(fileline, initsp, condp, incsp, bodysp) {
     }
     virtual ~AstFor() {}
     virtual AstType type() const { return AstType::FOR;}

src/V3Begin.cpp

@@ -23,6 +23,7 @@
 // Each module:
 //	Look for BEGINs
 //	    BEGIN(VAR...) -> VAR ... {renamed}
+//	FOR -> WHILEs
 //
 //*************************************************************************
 
@@ -141,6 +142,22 @@ private:
 	    m_modp->addStmtp(nodep);
 	}
     }
+    virtual void visit(AstFor* nodep, AstNUser*) {
+	// So later optimizations don't need to deal with them,
+	//    FOR(init,cond,assign,body) -> init,WHILE(cond) { body, assign }
+	AstNode* initsp = nodep->initsp(); if (initsp) initsp->unlinkFrBackWithNext();
+	AstNode* condp = nodep->condp(); if (condp) condp->unlinkFrBackWithNext();
+	AstNode* incsp = nodep->incsp(); if (incsp) incsp->unlinkFrBackWithNext();
+	AstNode* bodysp = nodep->bodysp(); if (bodysp) bodysp->unlinkFrBackWithNext();
+	bodysp = bodysp->addNext(incsp);
+	AstNode* newp = new AstWhile(nodep->fileline(),
+				     condp,
+				     bodysp);
+	initsp = initsp->addNext(newp);
+	newp = initsp;
+	nodep->replaceWith(newp);
+	nodep->deleteTree(); nodep=NULL;
+    }
     virtual void visit(AstNode* nodep, AstNUser*) {
 	nodep->iterateChildren(*this);
     }

src/V3EmitV.cpp

@@ -199,7 +199,7 @@ public:
 	puts(";");
 	nodep->condp()->iterateAndNext(*this);
 	puts(";");
-	nodep->assignsp()->iterateAndNext(*this);
+	nodep->incsp()->iterateAndNext(*this);
 	m_suppressSemi = false;
 	puts(") {\n");
 	nodep->bodysp()->iterateAndNext(*this);

src/V3Task.cpp

@@ -52,6 +52,7 @@ private:
     typedef std::map<pair<AstScope*,AstVar*>,AstVarScope*> VarToScopeMap;
     // MEMBERS
     VarToScopeMap	m_varToScopeMap;	// Map for Var -> VarScope mappings
+    AstAssignW*		m_assignwp;		// Current assignment
 public:
     // METHODS
     AstScope* getScope(AstNodeFTask* nodep) {
@@ -85,10 +86,25 @@ private:
 		taskp->user3p(nodep);
 	    }
 	}
-	// No iterateChildren for speed
+	nodep->iterateChildren(*this);
+    }
+    virtual void visit(AstAssignW* nodep, AstNUser*) {
+	m_assignwp = nodep;
+	nodep->iterateChildren(*this);  // May delete nodep.
+	m_assignwp = NULL;
+    }
+    virtual void visit(AstNodeFTaskRef* nodep, AstNUser*) {
+	if (m_assignwp) {
+	    // Wire assigns must become always statements to deal with insertion
+	    // of multiple statements.  Perhaps someday make all wassigns into always's?
+	    UINFO(5,"     IM_WireRep  "<<m_assignwp<<endl);
+	    AstNode* lhsp = m_assignwp->lhsp()->unlinkFrBack();
+	    AstNode* rhsp = m_assignwp->rhsp()->unlinkFrBack();
+	    AstNode* assignp = new AstAssign (m_assignwp->fileline(), lhsp, rhsp);
+	    AstNode* alwaysp = new AstAlways (m_assignwp->fileline(), NULL, assignp);
+	    m_assignwp->replaceWith(alwaysp); pushDeletep(m_assignwp); m_assignwp=NULL;
+	}
     }
-    //--------------------
-    virtual void visit(AstNodeMath* nodep, AstNUser*) {}	// Speedup
     //--------------------
     // Default: Just iterate
     virtual void visit(AstNode* nodep, AstNUser*) {
@@ -97,6 +113,7 @@ private:
 public:
     // CONSTUCTORS
     TaskStateVisitor(AstNode* nodep) {
+	m_assignwp = NULL;
 	AstNode::user3ClearTree();
 	nodep->iterateAndNext(*this, NULL);
     }
@@ -148,11 +165,20 @@ private:
     //  AstNodeFTask::user	// True if its been expanded
     // Each funccall
     //  AstVar::user2p		// AstVarScope* to replace varref with
+
+    // TYPES
+    enum  InsertMode {
+	IM_BEFORE,		// Pointing at statement ref is in, insert before this
+	IM_AFTER,		// Pointing at last inserted stmt, insert after
+	IM_WHILE_PRECOND	// Pointing to for loop, add to body end
+    };
+
     // STATE
     TaskStateVisitor* m_statep;	// Common state between visitors
     AstModule*	m_modp;		// Current module
     AstScope*	m_scopep;	// Current scope
-    AstNode*	m_lastStmtp;	// Proceeding statement
+    InsertMode	m_insMode;	// How to insert
+    AstNode*	m_insStmtp;	// Where to insert statement
     int		m_modNCalls;	// Incrementing func # for making symbols
     //int debug() { return 9; }
 
@@ -344,18 +370,46 @@ private:
 	nodep->accept(*this);
 	m_scopep = oldscopep;
     }
+    void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
+	if (debug()>=9) { nodep->dumpTree(cout,"-newstmt:"); }
+	if (!m_insStmtp) nodep->v3fatalSrc("Function not underneath a statement");
+	if (m_insMode == IM_BEFORE) {
+	    // Add the whole thing before insertAt
+	    UINFO(5,"     IM_Before  "<<m_insStmtp<<endl);
+	    AstNRelinker handle;
+	    m_insStmtp->unlinkFrBackWithNext(&handle);
+	    if (debug()>=9) { newp->dumpTree(cout,"-newfunc:"); }
+	    newp->addNext(m_insStmtp);
+	    handle.relink(newp);
+	}
+	else if (m_insMode == IM_AFTER) {
+	    UINFO(5,"     IM_After   "<<m_insStmtp);
+	    m_insStmtp->addNextHere(newp);
+	}
+	else if (m_insMode == IM_WHILE_PRECOND) {
+	    UINFO(5,"     IM_While_Precond "<<m_insStmtp);
+	    AstWhile* whilep = m_insStmtp->castWhile();
+	    if (!whilep) nodep->v3fatalSrc("Insert should be under WHILE");
+	    whilep->addPrecondsp(newp);
+	}
+	else {
+	    nodep->v3fatalSrc("Unknown InsertMode");
+	}
+	m_insMode = IM_AFTER;
+	m_insStmtp = newp;
+    }
 
     // VISITORS
     virtual void visit(AstModule* nodep, AstNUser*) {
 	m_modp = nodep;
-	m_lastStmtp = NULL;
+	m_insStmtp = NULL;
 	m_modNCalls = 0;
 	nodep->iterateChildren(*this);
 	m_modp = NULL;
     }
     virtual void visit(AstScope* nodep, AstNUser*) {
 	m_scopep = nodep;
-	m_lastStmtp = NULL;
+	m_insStmtp = NULL;
 	nodep->iterateChildren(*this);
 	m_scopep = NULL;
     }
@@ -372,27 +426,12 @@ private:
     }
     virtual void visit(AstFuncRef* nodep, AstNUser*) {
 	UINFO(4," Func REF   "<<nodep<<endl);
-	if (debug()>=9) { m_lastStmtp->dumpTree(cout,"-prestmt:"); }
+	if (debug()>=9) { nodep->dumpTree(cout,"-preref:"); }
 	// First, do hierarchical funcs
 	AstFunc* funcp = nodep->taskp()->castFunc();
 	if (!funcp) nodep->v3fatalSrc("unlinked");
-	AstNode* insertAtp = m_lastStmtp;
-	if (AstAssignW* awp = insertAtp->castAssignW()) {
-	    // Wire assigns must become always statements to deal with insertion
-	    // of multiple statements.  Perhaps someday make all wassigns into always's?
-	    AstNode* lhsp = awp->lhsp()->unlinkFrBack();
-	    AstNode* rhsp = awp->rhsp()->unlinkFrBack();
-	    AstNode* assignp = new AstAssign (awp->fileline(), lhsp, rhsp);
-	    AstNode* alwaysp = new AstAlways (awp->fileline(), NULL, assignp);
-	    m_lastStmtp = assignp; insertAtp = assignp;
-	    awp->replaceWith(alwaysp); pushDeletep(awp); awp=NULL;
-	}
 	// Inline func refs in the function
 	iterateIntoFTask(funcp);
-	// Inline this reference
-	if (debug()>=9) { m_lastStmtp->dumpTree(cout,"-inlstmt:"); }
-	if (!insertAtp) nodep->v3fatalSrc("Function not underneath a statement");
-	UINFO(5,"     Under  "<<insertAtp<<endl);
 	// Create output variabls
 	string namePrefix = "__Vfunc_"+funcp->shortName()+"__"+cvtToStr(m_modNCalls++);
 	AstVarScope* outvscp = createVarScope (funcp->fvarp()->castVar(),
@@ -402,22 +441,20 @@ private:
 	if (!nodep->taskp()) nodep->v3fatalSrc("Unlinked?");
 
 	AstNode* beginp = createInlinedFTask(nodep, namePrefix, outvscp);
-	// Add the whole thing before insertAt
-	AstNRelinker handle;
-	insertAtp->unlinkFrBackWithNext(&handle);
-	if (debug()>=9) { beginp->dumpTree(cout,"-newfunc:"); }
-	beginp->addNext(insertAtp);
-	handle.relink(beginp);
 	// Replace the ref
 	AstVarRef* outrefp = new AstVarRef (nodep->fileline(), outvscp, false);
 	nodep->replaceWith(outrefp);
+	// Insert new statements
+	insertBeforeStmt(nodep, beginp);
+	// Cleanup
 	nodep->deleteTree(); nodep=NULL;
-	if (debug()>=9) { insertAtp->dumpTree(cout,"-newstmt:"); }
 	UINFO(4,"  Done.\n");
     }
     virtual void visit(AstNodeFTask* nodep, AstNUser*) {
-	AstNode* prevLastStmtp = m_lastStmtp;
+	InsertMode prevInsMode = m_insMode;
-	m_lastStmtp = nodep->stmtsp();
+	AstNode* prevInsStmtp = m_insStmtp;
+	m_insMode = IM_BEFORE;
+	m_insStmtp = nodep->stmtsp();  // Might be null if no statements, but we won't use it
 	if (!nodep->user()) {
 	    // Expand functions in it & Mark for later delete
 	    nodep->user(true);
@@ -452,11 +489,32 @@ private:
 	    // Just push, as other references to func may remain until visitor exits
 	    pushDeletep(nodep); nodep=NULL;
 	}
-	m_lastStmtp = prevLastStmtp;
+	m_insMode = prevInsMode;
+	m_insStmtp = prevInsStmtp;
+    }
+    virtual void visit(AstWhile* nodep, AstNUser*) {
+	// Special, as statements need to be put in different places
+	// Preconditions insert first just before themselves (the normal rule for other statement types)
+	m_insStmtp = NULL;	// First thing should be new statement
+	nodep->precondsp()->iterateAndNext(*this);
+	// Conditions insert first at end of precondsp.
+	m_insMode = IM_WHILE_PRECOND;
+	m_insStmtp = nodep;
+	nodep->condp()->iterateAndNext(*this);
+	// Body insert just before themselves
+	m_insStmtp = NULL;	// First thing should be new statement
+	nodep->bodysp()->iterateAndNext(*this);
+	// Done the loop
+	m_insStmtp = NULL;	// Next thing should be new statement
+    }
+    virtual void visit(AstNodeFor* nodep, AstNUser*) {
+	nodep->v3fatalSrc("For statements should have been converted to while statements in V3Begin.cpp\n");
     }
     virtual void visit(AstNodeStmt* nodep, AstNUser*) {
-	m_lastStmtp = nodep;
+	m_insMode = IM_BEFORE;
+	m_insStmtp = nodep;
 	nodep->iterateChildren(*this);
+	m_insStmtp = NULL;	// Next thing should be new statement
     }
     //--------------------
     // Default: Just iterate
@@ -470,6 +528,7 @@ public:
 	: m_statep(statep) {
 	m_modp = NULL;
 	m_scopep = NULL;
+	m_insStmtp = NULL;
 	AstNode::userClearTree();
 	nodep->accept(*this);
     }

src/V3Unroll.cpp

@@ -50,6 +50,7 @@ private:
     AstVar*		m_forVarp;		// Iterator variable
     AstVarScope*	m_forVscp;		// Iterator variable scope (NULL for generate pass)
     AstConst*		m_varValuep;		// Current value of loop
+    AstNode*		m_ignoreIncp;		// Increment node to ignore
     bool		m_varModeCheck;		// Just checking RHS assignments
     bool		m_varModeReplace;	// Replacing varrefs
     bool		m_varAssignHit;		// Assign var hit
@@ -66,7 +67,7 @@ private:
 	nodep->iterateChildren(*this);
     }
 
-    bool cantUnroll(AstNodeFor* nodep, const char* reason) {
+    bool cantUnroll(AstNode* nodep, const char* reason) {
 	if (m_generate) {
 	    nodep->v3error("Unsupported: Can't unroll generate for; "<<reason);
 	}
@@ -75,36 +76,44 @@ private:
 	return false;
     }
 
-    bool forUnrollCheck(AstNodeFor* nodep) {
+    bool forUnrollCheck(AstNode* nodep,
-	// Do only the body; ignore the loop variable as a dependency.
+			AstNode* initp,	// Maybe under nodep (no nextp), or standalone (ignore nextp)
-	// Return if we did the replacement or not
+			AstNode* precondsp, AstNode* condp,
-	if (m_varModeCheck || m_varModeReplace) return false;
+			AstNode* incp,		// Maybe under nodep or in bodysp
-	// See if we can make it simple enough to process
+			AstNode* bodysp) {
-	if (nodep->initsp()) V3Const::constifyTree(nodep->initsp());  // May change what is under init, leave here
+	// To keep the IF levels low, we return as each test fails.
-	V3Const::constifyTree(nodep->condp());
+	UINFO(4, " FOR Check "<<nodep<<endl);
-	if (nodep->assignsp()) V3Const::constifyTree(nodep->assignsp());
+	if (initp)	UINFO(6, "    Init "<<initp<<endl);
-
+	if (precondsp)	UINFO(6, "    Pcon "<<precondsp<<endl);
-	AstAssign* initp = nodep->initsp()->castAssign();
+	if (condp)	UINFO(6, "    Cond "<<condp<<endl);
-	if (!initp) nodep->v3fatalSrc("no initial assignment");
+	if (incp)	UINFO(6, "    Inc  "<<incp<<endl);
-	m_forVarp = initp->lhsp()->castVarRef()->varp();
+	// Initial value check
-	m_forVscp = initp->lhsp()->castVarRef()->varScopep();
+	AstAssign* initAssp = initp->castAssign();
+	if (!initAssp) return cantUnroll(nodep, "no initial assignment");
+	if (initp->nextp() && initp->nextp()!=nodep) nodep->v3fatalSrc("initial assignment shouldn't be a list");
+	m_forVarp = initAssp->lhsp()->castVarRef()->varp();
+	m_forVscp = initAssp->lhsp()->castVarRef()->varScopep();
 	if (nodep->castGenFor() && !m_forVarp->isGenVar()) {
 	    nodep->v3error("Non-genvar used in generate for: "<<m_forVarp->name()<<endl);
 	}
-	AstNodeBiop* condp = nodep->condp()->castNodeBiop();
+	if (m_generate) V3Const::constifyParam(initAssp->rhsp());
-	AstAssign* assignp = nodep->assignsp()->castAssign();
+	AstConst* constInitp = initAssp->rhsp()->castConst();
-	AstNodeBiop* incInstrp = assignp->rhsp()->castNodeBiop();
+	if (!constInitp) return cantUnroll(nodep, "non-constant initializer");
-	if (!assignp) nodep->v3fatalSrc("no increment assignment");
+	//
-	UINFO(4, " FOR Check "<<nodep<<endl);
+	// Condition check
+	if (condp->nextp()) nodep->v3fatalSrc("conditional shouldn't be a list");
+	//
+	// Assignment of next value check
+	AstAssign* incAssp = incp->castAssign();
+	if (!incAssp) return cantUnroll(nodep, "no increment assignment");
+	if (incAssp->nextp()) nodep->v3fatalSrc("increment shouldn't be a list");
+	AstNodeBiop* incInstrp = incAssp->rhsp()->castNodeBiop();
+	//
 	if (m_forVscp) { UINFO(8, "   Loop Variable: "<<m_forVscp<<endl); }
 	else	       { UINFO(8, "   Loop Variable: "<<m_forVarp<<endl); }
-	if (debug()>=9) nodep->dumpTree(cout,"   for: ");
+	if (debug()>=9) nodep->dumpTree(cout,"-   for: ");
 	//
 	// Extract the constant loop bounds
-	// To keep the IF levels low, we return as each test fails.
-	if (m_generate) V3Const::constifyParam(initp->rhsp());
-	AstConst* constInitp = initp->rhsp()->castConst();
-	if (!constInitp) return cantUnroll(nodep, "non-constant initializer");
 	bool subtract = incInstrp->castSub();
 	{
 	    if (!subtract && !incInstrp->castAdd()) return cantUnroll(nodep, "missing add/sub for incrementer");
@@ -134,13 +143,14 @@ private:
 	bool gte = condp->castGte() || condp->castGteS();
 	if (!lt && !lte && !gt && !gte)
 	    return cantUnroll(nodep, "condition not <= or <");
-	if (!condp->lhsp()->castVarRef())
+	AstNodeBiop* condBip = condp->castNodeBiop();
+	if (!condBip->lhsp()->castVarRef())
 	    return cantUnroll(nodep, "no variable on lhs of condition");
-	if (condp->lhsp()->castVarRef()->varp() != m_forVarp
+	if (condBip->lhsp()->castVarRef()->varp() != m_forVarp
-	    || condp->lhsp()->castVarRef()->varScopep() != m_forVscp)
+	    || condBip->lhsp()->castVarRef()->varScopep() != m_forVscp)
 	    return cantUnroll(nodep, "different variable in condition");
-	if (m_generate) V3Const::constifyParam(condp->rhsp());
+	if (m_generate) V3Const::constifyParam(condBip->rhsp());
-	AstConst* constStopp = condp->rhsp()->castConst();
+	AstConst* constStopp = condBip->rhsp()->castConst();
 	if (!constStopp) return cantUnroll(nodep, "non-constant final value");
 	UINFO(8, "   Stop expr ok: "<<constStopp<<endl);
 	//
@@ -162,7 +172,10 @@ private:
 
 	    // Less then 10 statements in the body?
 	    int bodySize = 0;
-	    for (AstNode* bodp = nodep->bodysp(); bodp; bodp=bodp->nextp()) {
+	    for (AstNode* bodp = precondsp; bodp; bodp=bodp->nextp()) {
+		bodySize++;
+	    }
+	    for (AstNode* bodp = bodysp; bodp; bodp=bodp->nextp()) {
 		bodySize++;
 	    }
 	    if (bodySize > v3Global.opt.unrollStmts())
@@ -172,30 +185,52 @@ private:
 	// Now, make sure there's no assignment to this variable in the loop
 	m_varModeCheck = true;
 	m_varAssignHit = false;
-	nodep->bodysp()->iterateAndNext(*this);
+	m_ignoreIncp = incp;
+	precondsp->iterateAndNext(*this);
+	bodysp->iterateAndNext(*this);
 	m_varModeCheck = false;
+	m_ignoreIncp = NULL;
 	if (m_varAssignHit) return cantUnroll(nodep, "genvar assigned *inside* loop");
 	//
 	// Finally, we can do it
-	forUnroller(nodep, constInitp->num(),
+	forUnroller(nodep, initp, precondsp, condp, incp, bodysp,
-		    condp, constStopp->num(),
+		    constInitp->num(),
+		    condBip, constStopp->num(),
 		    incInstrp, constIncp->num()); nodep = NULL;
 	return true;
     }
 
-    void forUnroller(AstNodeFor* nodep, const V3Number& numInit,
+    void forUnroller(AstNode* nodep,
+		     AstNode* initp,
+		     AstNode* precondsp, AstNode* condp,
+		     AstNode* incp, AstNode* bodysp,
+		     const V3Number& numInit,
 		     AstNodeBiop* cmpInstrp, const V3Number& numStop,
 		     AstNodeBiop* incInstrp, const V3Number& numInc) {
 	UINFO(4, "   Unroll for var="<<numInit<<"; var<"<<numStop<<"; var+="<<numInc<<endl);
-	AstNode* bodysp = nodep->bodysp();  // Maybe null if no body
+	UINFO(6, "    cmpI "<<cmpInstrp<<endl);
-	if (bodysp) bodysp->unlinkFrBackWithNext();
+	UINFO(6, "    IncI "<<incInstrp<<endl);
-
+	AstNode* stmtsp = NULL;
-	AstNode* newbodysp = NULL;
+	if (initp) {
+	    initp->unlinkFrBack();	// Always a single statement; nextp() may be nodep
+	    // Don't add to list, we do it once, and setting loop index isn't needed as we're constant propagating it
+	}
+	if (precondsp) {
+	    precondsp->unlinkFrBackWithNext();
+	    stmtsp = stmtsp->addNextNull(precondsp);
+	}
+	if (bodysp) {
+	    bodysp->unlinkFrBackWithNext();
+	    stmtsp = stmtsp->addNextNull(bodysp);  // Maybe null if no body
+	}
+	// If it's a While, then incp is already part of bodysp.
 	V3Number loopValue(nodep->fileline(), m_forVarp->width());  // May differ in size from numInitp
 	loopValue.opAssign(numInit);
 
+	AstNode* newbodysp = NULL;
+	AstNode* clonedIncsp = NULL;	// Last cloned incp() statements
 	m_statLoops++;
-	if (bodysp) {
+	if (stmtsp) {
 	    int times = 0;
 	    while (1) {
 		UINFO(8,"      Looping "<<loopValue<<endl);
@@ -205,7 +240,21 @@ private:
 		if (contin.isEqZero()) {
 		    break;  // Done with the loop
 		} else {
-		    AstNode* oneloopp = bodysp->cloneTree(true);
+		    // Replace iterator values with constant.
+		    AstNode* oneloopp = stmtsp->cloneTree(true);
+
+		    // A nicer way to propage the loop constant would be to set the variable to the value
+		    // and call a constant-propagator like V3Table, so temp values
+		    // that are calculated propagate down.
+		    // If we do this, we can remove the below
+		    if (nodep->castWhile() && incp) {
+			if (clonedIncsp) {
+			    // Previous iteration of loop set the variable.
+			    // This set is redundant with this next iteration and can be removed.
+			    clonedIncsp->unlinkFrBack()->deleteTree();
+			}
+			clonedIncsp = incp->clonep(); if (!clonedIncsp) nodep->v3fatalSrc("inc failed");
+		    }
 
 		    m_varValuep = new AstConst(nodep->fileline(), loopValue);
 		    m_varModeReplace = true;
@@ -228,55 +277,60 @@ private:
 		}
 	    }
 	}
-
+	// Leaving the iterator at the final value is handled by the increment statements being left the final body
-	// And, leave the iterator at the right final value.
-	if (!nodep->castGenFor()) {
-	    AstVarRef* newrefp = (m_forVscp
-				  ? new AstVarRef(nodep->fileline(), m_forVscp, true)
-				  : new AstVarRef(nodep->fileline(), m_forVarp, true));
-	    AstAssign* finalAssignp = new AstAssign
-		(nodep->fileline(),
-		 newrefp,
-		 new AstConst(nodep->fileline(), loopValue));
-	    if (newbodysp) newbodysp->addNext(finalAssignp);
-	    else newbodysp = finalAssignp;
-	}
-
 	// Replace the FOR()
 	if (newbodysp) nodep->replaceWith(newbodysp);
 	else nodep->unlinkFrBack();
-	if (debug()>=9) newbodysp->dumpTree(cout,"  _new: ");
+	if (debug()>=9) newbodysp->dumpTree(cout,"-  _new: ");
     }
 
-    virtual void visit(AstNodeFor* nodep, AstNUser*) {
+    virtual void visit(AstWhile* nodep, AstNUser*) {
-	if (!m_generate || m_varModeReplace) {
+	nodep->iterateChildren(*this);
-	    nodep->iterateChildren(*this);
-	}
 	if (m_varModeCheck || m_varModeReplace) {
 	} else {
-	    if (forUnrollCheck(nodep)) {
+	    // Constify before unroll call, as it may change what is underneath.
+	    if (nodep->precondsp()) V3Const::constifyTree(nodep->precondsp());
+	    if (nodep->condp()) V3Const::constifyTree(nodep->condp());
+	    // 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);
+	    initp = NULL; if (nodep->backp()->nextp() == nodep) initp=nodep->backp();
+	    // Grab assignment
+	    AstNode* incp = NULL;  // Should be last statement
+	    for (incp = nodep->bodysp(); incp->nextp(); incp = incp->nextp()) {}
+	    if (incp) V3Const::constifyTree(incp);
+	    for (incp = nodep->bodysp(); incp->nextp(); incp = incp->nextp()) {}  // Again, as may have changed
+	    // And check it
+	    if (forUnrollCheck(nodep, initp,
+			       nodep->precondsp(), nodep->condp(),
+			       incp, nodep->bodysp())) {
 		nodep=NULL; // Did replacement
-	    } else if (m_generate || nodep->castGenFor()) {
-		nodep->v3error("For loop doesn't have genvar index, or is misformed");
-	    } else {
-		// So later optimizations don't need to deal with them,
-		// convert leftover FOR's:
-		//    FOR(init,cond,assign,body) -> init,WHILE(cond) { body, assign }
-		AstNode* initsp = nodep->initsp(); if (initsp) initsp->unlinkFrBackWithNext();
-		AstNode* condp = nodep->condp(); if (condp) condp->unlinkFrBackWithNext();
-		AstNode* assignsp = nodep->assignsp(); if (assignsp) assignsp->unlinkFrBackWithNext();
-		AstNode* bodysp = nodep->bodysp(); if (bodysp) bodysp->unlinkFrBackWithNext();
-		bodysp = bodysp->addNext(assignsp);
-		AstNode* newp = new AstWhile(nodep->fileline(),
-					     condp,
-					     bodysp);
-		initsp = initsp->addNext(newp);
-		newp = initsp;
-		nodep->replaceWith(newp);
-		nodep->deleteTree(); nodep=NULL;
 	    }
 	}
     }
+    virtual void visit(AstGenFor* nodep, AstNUser*) {
+	if (!m_generate || m_varModeReplace) {
+	    nodep->iterateChildren(*this);
+	}  // else V3Param will recursively call each for loop to be unrolled for us
+	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->condp()) V3Const::constifyTree(nodep->condp());
+	    if (nodep->incsp()) V3Const::constifyTree(nodep->incsp());
+	    if (forUnrollCheck(nodep, nodep->initsp(),
+			       NULL, nodep->condp(),
+			       nodep->incsp(), nodep->bodysp())) {
+		nodep=NULL; // Did replacement
+	    } else {
+		nodep->v3error("For loop doesn't have genvar index, or is misformed");
+	    }
+	}
+    }
+    virtual void visit(AstNodeFor* nodep, AstNUser*) {
+	nodep->v3error("V3Task should have removed standard FORs");
+    }
 
     virtual void visit(AstBegin* nodep, AstNUser*) {
 	// Naming inside loop body; must have been a generate for.
@@ -307,7 +361,8 @@ private:
 	}
 	if (m_varModeReplace
 	    && nodep->varp() == m_forVarp
-	    && nodep->varScopep() == m_forVscp) {
+	    && nodep->varScopep() == m_forVscp
+	    && !nodep->lvalue()) {
 	    AstNode* newconstp = m_varValuep->cloneTree(false);
 	    nodep->replaceWith(newconstp);
 	}
@@ -316,7 +371,11 @@ private:
     //--------------------
     // Default: Just iterate
     virtual void visit(AstNode* nodep, AstNUser*) {
-	nodep->iterateChildren(*this);
+	if (m_varModeCheck && nodep == m_ignoreIncp) {
+	    // Ignore subtree that is the increment
+	} else {
+	    nodep->iterateChildren(*this);
+	}
     }
 
 public:
@@ -324,6 +383,7 @@ public:
     UnrollVisitor(AstNode* nodep, bool generate) {
 	m_forVarp = NULL;
 	m_forVscp = NULL;
+	m_ignoreIncp = NULL;
 	m_varModeCheck = false;
 	m_varModeReplace = false;
 	m_inBegin = false;

src/V3Width.cpp

@@ -492,7 +492,7 @@ private:
 	nodep->initsp()->iterateAndNext(*this);
 	nodep->condp()->iterateAndNext(*this,WidthVP(1,1,BOTH).p());
 	if (!nodep->castGenFor()) nodep->bodysp()->iterateAndNext(*this);
-	nodep->assignsp()->iterateAndNext(*this);
+	nodep->incsp()->iterateAndNext(*this);
 	widthCheckReduce(nodep,"For Test Condition",nodep->condp(),1,1);	// it's like a if() condition.
     }
     virtual void visit(AstWhile* nodep, AstNUser*) {

test_regress/driver.pl

@@ -123,7 +123,8 @@ sub one_test {
 	     } else {
 		 $test->oprint("FAILED: ","*"x60,"\n");
 		 push @fails, "\t#".$test->soprint("%Error: $test->{errors}\n");
-		 push @fails, "\t\tmake && ( cd test_regress ; "
+		 my $j = ($opt_jobs>1?" -j 2":"");
+		 push @fails, "\t\tmake$j && ( cd test_regress ; "
 		     .$test->{pl_filename}." ".join(' ',@Orig_ARGV_Sw)." )\n";
 		 $failcnt++;
 		 if ($opt_stop) { die "%Error: --stop and errors found\n"; }

test_regress/t/t_for_funcbound.pl

@@ -0,0 +1,22 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; }
+# $Id$
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2004 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# General Public License or the Perl Artistic License.
+
+compile (
+	 );
+
+execute (
+	 check_finished=>1,
+	 expect=>
+'[10] hello
+[20] world
+',
+     );
+
+ok(1);
+1;

test_regress/t/t_for_funcbound.v

@@ -0,0 +1,81 @@
+// $Id$
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2006 by Wilson Snyder.
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   integer j;
+   integer hit_count;
+   reg [63:0] cam_lookup_hit_vector;
+
+   strings strings ();
+
+   task show;
+      input [8*8-1:0] str;
+      reg [7:0]       char;
+      integer 	      loc;
+      begin
+	 $write("[%0t] ",$time);
+	 strings.stringStart(8*8-1);
+	 for (char = strings.stringByte(str); !strings.isNull(char); char = strings.stringByte(str)) begin
+	    $write("%c",char);
+	 end
+	 $write("\n");
+      end
+   endtask
+
+
+   integer cyc; initial cyc=1;
+   always @ (posedge clk) begin
+      if (cyc!=0) begin
+	 cyc <= cyc + 1;
+	 if (cyc==1) begin
+	    show("hello\000xx");
+	 end
+	 if (cyc==2) begin
+	    show("world\000xx");
+	 end
+	 if (cyc==4) begin
+	    $write("*-* All Finished *-*\n");
+	    $finish;
+	 end
+      end
+   end
+
+endmodule
+
+module strings;
+   // **NOT** reentrant, just a test!
+   integer index;
+   task stringStart;
+      input [31:0] bits;
+      begin
+	 index = (bits-1)/8;
+      end
+   endtask
+
+   function isNull;
+      input [7:0] chr;
+      isNull = (chr == 8'h0);
+   endfunction
+
+   function [7:0] stringByte;
+      input [8*8-1:0] str;
+      begin
+	 if (index<=0) stringByte=8'h0;
+	 else stringByte = str[index*8 +: 8];
+	 index = index - 1;
+      end
+   endfunction
+endmodule
+
+// Local Variables:
+// compile-command: "./vlint __FILE__"
+// End: