From 63f111b7f3aec5c10a9a29b2c82e5c5c03b1a0e3 Mon Sep 17 00:00:00 2001 From: Johan Bjork Date: Thu, 21 Jan 2016 19:00:19 -0500 Subject: [PATCH] Fix unrolling complicated for-loop bounds, bug677. Signed-off-by: Wilson Snyder --- Changes | 2 + src/V3Simulate.h | 6 +- src/V3Unroll.cpp | 273 ++++++++++++++----------- test_regress/t/t_unroll_complexcond.pl | 18 ++ test_regress/t/t_unroll_complexcond.v | 44 ++++ 5 files changed, 218 insertions(+), 125 deletions(-) create mode 100755 test_regress/t/t_unroll_complexcond.pl create mode 100644 test_regress/t/t_unroll_complexcond.v diff --git a/Changes b/Changes index fd26a615e..7ba22e9dd 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,8 @@ indicates the contributor was also the author of the fix; Thanks! **** Internal Verilation-time performance enhancements, bug1021. [Johan Bjork] +**** Fix unrolling complicated for-loop bounds, bug677. [Johan Bjork] + **** Fix stats file containing multiple unroll entries, bug1020. [Johan Bjork] **** Fix using short parameter names on negative params, bug1022. [Duraid Madina] diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 327409bde..603eb8226 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -831,7 +831,7 @@ public: // Move all allocated numbers to the free pool m_numFreeps = m_numAllps; } - void mainTableCheck (AstNode* nodep) { + void mainTableCheck (AstNode* nodep) { setMode(true/*scoped*/,true/*checking*/, false/*params*/); mainGuts(nodep); } @@ -839,6 +839,10 @@ public: setMode(true/*scoped*/,false/*checking*/, false/*params*/); mainGuts(nodep); } + void mainCheckTree (AstNode* nodep) { + setMode(false/*scoped*/,true/*checking*/, false/*params*/); + mainGuts(nodep); + } void mainParamEmulate (AstNode* nodep) { setMode(false/*scoped*/,false/*checking*/, true/*params*/); mainGuts(nodep); diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index 410ec14b8..0ca42dafc 100644 --- a/src/V3Unroll.cpp +++ b/src/V3Unroll.cpp @@ -40,6 +40,7 @@ #include "V3Stats.h" #include "V3Const.h" #include "V3Ast.h" +#include "V3Simulate.h" //###################################################################### // Unroll state, as a visitor of each AstNode @@ -107,19 +108,12 @@ private: if (precondsp) UINFO(6, " Pcon "<castAssign(); if (!initAssp) return cantUnroll(nodep, "no initial assignment"); if (initp->nextp() && initp->nextp()!=nodep) nodep->v3fatalSrc("initial assignment shouldn't be a list"); if (!initAssp->lhsp()->castVarRef()) return cantUnroll(nodep, "no initial assignment to simple variable"); - 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: "<prettyName()<rhsp()); // rhsp may change - AstConst* constInitp = initAssp->rhsp()->castConst(); - if (!constInitp) return cantUnroll(nodep, "non-constant initializer"); // // Condition check if (condp->nextp()) nodep->v3fatalSrc("conditional shouldn't be a list"); @@ -128,93 +122,21 @@ private: 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: "<=9) nodep->dumpTree(cout,"- for: "); - // - // Extract the constant loop bounds - bool subtract = incInstrp->castSub(); - { - if (!subtract && !incInstrp->castAdd()) return cantUnroll(nodep, "missing add/sub for incrementer"); - AstVarRef* incVarrp = (subtract ? incInstrp->lhsp()->castVarRef() - : incInstrp->rhsp()->castVarRef()); - if (!incVarrp) return cantUnroll(nodep, "missing variable in incrementer"); - if (incVarrp->varp() != m_forVarp - || incVarrp->varScopep() != m_forVscp) { - return cantUnroll(nodep, "different variables in incrementer"); - } - } - // - // Adds have the # on the lhsp because V3Const pushes rhs consts over to the lhs - // 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) preconstIncp = V3Const::constifyParamsEdit(preconstIncp)->castConst(); - AstConst* constIncp = (subtract ? incInstrp->rhsp()->castConst() - : incInstrp->lhsp()->castConst()); - UINFO(8, " Inc expr ok: "<isZero()) return cantUnroll(nodep, "zero increment"); // Or we could loop forever below... - bool lt = condp->castLt() || condp->castLtS(); - bool lte = condp->castLte() || condp->castLteS(); - bool gt = condp->castGt() || condp->castGtS(); - bool gte = condp->castGte() || condp->castGteS(); - if (!lt && !lte && !gt && !gte) - return cantUnroll(nodep, "condition not <=, <, >= or >"); - AstNodeBiop* cmpInstrp = condp->castNodeBiop(); - bool cmpVarLhs; - if (cmpInstrp->lhsp()->castVarRef() - && cmpInstrp->lhsp()->castVarRef()->varp() == m_forVarp - && cmpInstrp->lhsp()->castVarRef()->varScopep() == m_forVscp) { - cmpVarLhs = true; - } else if (cmpInstrp->rhsp()->castVarRef() - && cmpInstrp->rhsp()->castVarRef()->varp() == m_forVarp - && cmpInstrp->rhsp()->castVarRef()->varScopep() == m_forVscp) { - cmpVarLhs = false; - } else if (!cmpInstrp->rhsp()->castVarRef()) { - return cantUnroll(nodep, "no variable on rhs of condition"); - } else { - return cantUnroll(nodep, "different variable in condition"); + 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: "<prettyName()<rhsp()); // rhsp may change - if (m_generate) V3Const::constifyParamsEdit(cmpVarLhs ? cmpInstrp->rhsp() - : cmpInstrp->lhsp()); // rhsp/lhsp may change - AstConst* constStopp = (cmpVarLhs ? cmpInstrp->rhsp()->castConst() - : cmpInstrp->lhsp()->castConst()); - if (!constStopp) return cantUnroll(nodep, "non-constant final value"); - UINFO(8, " Stop expr ok: "<width()>32 || constInitp->num().isFourState() - || constStopp->width()>32 || constStopp->num().isFourState() - || constIncp->width()>32 || constIncp->num().isFourState()) - return cantUnroll(nodep, "init/final/increment too large or four state"); - vlsint32_t valInit = constInitp->num().toSInt(); - vlsint32_t valStop = constStopp->num().toSInt(); - if (gte) valStop++; if (lte) valStop--; // 23 >= a, handle as if 24 > a - vlsint32_t valInc = constIncp->num().toSInt(); - if (subtract) valInc = -valInc; - UINFO(8," In Numbers: for (v="<width()); } // Will roll around - UINFO(8, " ~Iters: "<rhsp()->castConst(); + if (!constInitp) return cantUnroll(nodep, "non-constant initializer"); - // Less than 10 statements in the body? - int bodySize = 0; - int bodyLimit = v3Global.opt.unrollStmts(); - if (loops>0) bodyLimit = v3Global.opt.unrollStmts() / loops; - if (bodySizeOverRecurse(precondsp, bodySize/*ref*/, bodyLimit) - || bodySizeOverRecurse(bodysp, bodySize/*ref*/, bodyLimit) - || bodySizeOverRecurse(incp, bodySize/*ref*/, bodyLimit)) { - return cantUnroll(nodep, "too many statements"); - } - } // // Now, make sure there's no assignment to this variable in the loop m_varModeCheck = true; @@ -226,26 +148,131 @@ private: m_varModeCheck = false; m_ignoreIncp = NULL; if (m_varAssignHit) return cantUnroll(nodep, "genvar assigned *inside* loop"); + // + if (m_forVscp) { UINFO(8, " Loop Variable: "<=9) nodep->dumpTree(cout,"- for: "); + + + if (!m_generate) { + AstAssign *incpAssign = incp->castAssign(); + if (!canSimulate(incpAssign->rhsp())) return cantUnroll(incp, "Unable to simulate increment"); + if (!canSimulate(condp)) return cantUnroll(condp, "Unable to simulate condition"); + + // Check whether to we actually want to try and unroll. + int loops; + if (!countLoops(initAssp, condp, incp, unrollCount(), loops)) + return cantUnroll(nodep, "Unable to simulate loop"); + + // Less than 10 statements in the body? + int bodySize = 0; + int bodyLimit = v3Global.opt.unrollStmts(); + if (loops>0) bodyLimit = v3Global.opt.unrollStmts() / loops; + if (bodySizeOverRecurse(precondsp, bodySize/*ref*/, bodyLimit) + || bodySizeOverRecurse(bodysp, bodySize/*ref*/, bodyLimit) + || bodySizeOverRecurse(incp, bodySize/*ref*/, bodyLimit)) { + return cantUnroll(nodep, "too many statements"); + } + } // Finally, we can do it - forUnroller(nodep, initp, precondsp, incp, bodysp, - constInitp->num(), - cmpInstrp, constStopp->num(), cmpVarLhs, - incInstrp, constIncp->num()); VL_DANGLING(nodep); + if (!forUnroller(nodep, initAssp, condp, precondsp, incp, bodysp)) { + return cantUnroll(nodep, "Unable to unroll loop"); + } + VL_DANGLING(nodep); // Cleanup return true; } - void forUnroller(AstNode* nodep, - AstNode* initp, + bool canSimulate(AstNode *nodep) { + SimulateVisitor simvis; + AstNode* clone = nodep->cloneTree(true); + simvis.mainCheckTree(clone); + return simvis.optimizable(); + } + + bool simulateTree(AstNode *nodep, const V3Number *loopValue, AstNode *dtypep, V3Number &outNum) { + AstNode* clone = nodep->cloneTree(true); + if (!clone) { + nodep->v3fatalSrc("Failed to clone tree"); + return false; + } + if (loopValue) { + m_varValuep = new AstConst (nodep->fileline(), *loopValue); + // Iteration requires a back, so put under temporary node + AstBegin* tempp = new AstBegin (nodep->fileline(), "[EditWrapper]", clone); + m_varModeReplace = true; + tempp->stmtsp()->iterateAndNext(*this); + m_varModeReplace = false; + clone = tempp->stmtsp()->unlinkFrBackWithNext(); + tempp->deleteTree(); + tempp = NULL; + pushDeletep(m_varValuep); m_varValuep = NULL; + } + SimulateVisitor simvis; + simvis.mainParamEmulate(clone); + if (!simvis.optimizable()) { + UINFO(3, "Unable to simulate" << endl); + if (debug()>=9) nodep->dumpTree(cout,"- _simtree: "); + return false; + } + // Fetch the result + V3Number* res = simvis.fetchNumberNull(clone); + if (!res) { + UINFO(3, "No number returned from simulation" << endl); + return false; + } + // Patch up datatype + if (dtypep) { + AstConst new_con (clone->fileline(), *res); + new_con.dtypeFrom(dtypep); + outNum = new_con.num(); + return true; + } + outNum = *res; + return true; + } + + bool countLoops(AstAssign *initp, AstNode *condp, AstNode *incp, int max, int &outLoopsr) { + outLoopsr = 0; + V3Number loopValue = V3Number(initp->fileline()); + if (!simulateTree(initp->rhsp(), NULL, initp, loopValue)) { + return false; + } + while (1) { + V3Number res = V3Number(initp->fileline()); + if (!simulateTree(condp, &loopValue, NULL, res)) { + return false; + } + if (!res.isEqOne()) { + break; + } + + outLoopsr++; + + // Run inc + AstAssign* incpass = incp->castAssign(); + V3Number newLoopValue = V3Number(initp->fileline()); + if (!simulateTree(incpass->rhsp(), &loopValue, incpass, newLoopValue)) { + return false; + } + loopValue.opAssign(newLoopValue); + if (outLoopsr > max) { + return false; + } + } + return true; + } + + bool forUnroller(AstNode* nodep, + AstAssign* initp, + AstNode* condp, AstNode* precondsp, - AstNode* incp, AstNode* bodysp, - const V3Number& numInit, - AstNodeBiop* cmpInstrp, const V3Number& numStop, bool cmpVarLhs, - AstNodeBiop* incInstrp, const V3Number& numInc) { - UINFO(4, " Unroll for var="<fileline()); + if (!simulateTree(initp->rhsp(), NULL, initp, loopValue)) { + return false; + } AstNode* stmtsp = NULL; if (initp) { initp->unlinkFrBack(); // Always a single statement; nextp() may be nodep @@ -269,26 +296,21 @@ private: // Mark variable to disable some later warnings m_forVarp->usedLoopIdx(true); - // 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; ++m_statLoops; if (stmtsp) { int times = 0; while (1) { UINFO(8," Looping "<fileline(), 1); - if (cmpVarLhs) { - cmpInstrp->numberOperate(contin, loopValue, numStop); - } else { - cmpInstrp->numberOperate(contin, numStop, loopValue); + V3Number res = V3Number(nodep->fileline()); + if (!simulateTree(condp, &loopValue, NULL, res)) { + nodep->v3error("Loop unrolling failed."); + return false; } - if (contin.isEqZero()) { + if (!res.isEqOne()) { break; // Done with the loop - } else { + } + else { // Replace iterator values with constant. AstNode* oneloopp = stmtsp->cloneTree(true); @@ -307,7 +329,7 @@ private: string nname = m_beginName + "__BRA__" + index + "__KET__"; oneloopp = new AstBegin(oneloopp->fileline(),nname,oneloopp,true); } - + pushDeletep(m_varValuep); m_varValuep=NULL; if (newbodysp) newbodysp->addNext(oneloopp); else newbodysp = oneloopp; @@ -317,12 +339,14 @@ private: break; } - //loopValue += valInc - V3Number newnum(nodep->fileline(), m_forVarp->width()); // Can't increment in-place - incInstrp->numberOperate(newnum, loopValue, numInc); - loopValue.opAssign(newnum); - - pushDeletep(m_varValuep); m_varValuep=NULL; + // loopValue += valInc + AstAssign *incpass = incp->castAssign(); + V3Number newLoopValue = V3Number(nodep->fileline()); + if (!simulateTree(incpass->rhsp(), &loopValue, incpass, newLoopValue)) { + nodep->v3error("Loop unrolling failed"); + return false; + } + loopValue.opAssign(newLoopValue); } } } @@ -334,6 +358,7 @@ private: if (initp) { pushDeletep(initp); VL_DANGLING(initp); } if (incp && !incp->backp()) { pushDeletep(incp); VL_DANGLING(incp); } if (debug()>=9) newbodysp->dumpTree(cout,"- _new: "); + return true; } virtual void visit(AstWhile* nodep, AstNUser*) { @@ -342,7 +367,7 @@ private: } else { // Constify before unroll call, as it may change what is underneath. if (nodep->precondsp()) V3Const::constifyEdit(nodep->precondsp()); // precondsp may change - if (nodep->condp()) V3Const::constifyEdit(nodep->condp()); //condp may change + 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(); diff --git a/test_regress/t/t_unroll_complexcond.pl b/test_regress/t/t_unroll_complexcond.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_unroll_complexcond.pl @@ -0,0 +1,18 @@ +#!/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. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_unroll_complexcond.v b/test_regress/t/t_unroll_complexcond.v new file mode 100644 index 000000000..49592a5c0 --- /dev/null +++ b/test_regress/t/t_unroll_complexcond.v @@ -0,0 +1,44 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This files is used to generated the BLKLOOPINIT error which +// is actually caused by not being able to unroll the for loop. +// +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2013 by Jie Xu. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + reg [3:0] tmp [3:0]; + + initial begin + tmp[0] = 4'b0000; + tmp[2] = 4'b0010; + tmp[3] = 4'b0011; + end + + // Test loop + always @ (posedge clk) begin + int i; + int j; + for (i = 0;(i < 4) && (i > 1); i++) begin + tmp[i] <= tmp[i-i]; + end + if (tmp[0] != 4'b0000) $stop; + if (tmp[3] != 4'b0011) $stop; + + j = 0; for (i=$c32("1"); i<3; ++i) j++; + if (j!=2) $stop; + j = 0; for (i=1; i<$c32("3"); ++i) j++; + if (j!=2) $stop; + j = 0; for (i=1; i<3; i=i+$c32("1")) j++; + if (j!=2) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule