diff --git a/Changes b/Changes index 196c59353..d0e44651f 100644 --- a/Changes +++ b/Changes @@ -16,6 +16,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Add --no-debug-leak to reduce memory use under debug. [John Coiner] +**** Fix severe runtime performance bug in certain foreach loops. [John Coiner] + **** On convergence errors, show activity. [John Coiner] **** Fix GCC 8.0 issues, bug1273. diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index 05e283fa9..4f53618aa 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -340,6 +340,7 @@ private: FileLine* fl = varsp->fileline(); AstNode* varp = new AstVar(fl, AstVarType::BLOCKTEMP, varsp->name(), nodep->findSigned32DType()); + // These will be the left and right dimensions of the array: AstNode* leftp = new AstAttrOf(fl, AstAttrType::DIM_LEFT, new AstVarRef(fl, arrayp->name(), false), new AstConst(fl, dimension)); @@ -347,19 +348,26 @@ private: new AstVarRef(fl, arrayp->name(), false), new AstConst(fl, dimension)); AstNode* stmtsp = varp; - stmtsp->addNext(new AstAssign(fl, new AstVarRef(fl, varp->name(), true), leftp)); - AstNode* comparep = - new AstCond(fl, new AstLte(fl, leftp->cloneTree(true), rightp->cloneTree(true)), - // left increments up to right - new AstLte(fl, new AstVarRef(fl, varp->name(), false), rightp->cloneTree(true)), - // left decrements down to right - new AstGte(fl, new AstVarRef(fl, varp->name(), false), rightp)); - AstNode* incp = - new AstAssign(fl, new AstVarRef(fl, varp->name(), true), - new AstAdd(fl, new AstVarRef(fl, varp->name(), false), - new AstNegate(fl, new AstAttrOf(fl, AstAttrType::DIM_INCREMENT, - new AstVarRef(fl, arrayp->name(), false), - new AstConst(fl, dimension))))); + // Assign left-dimension into the loop var: + stmtsp->addNext(new AstAssign + (fl, new AstVarRef(fl, varp->name(), true), leftp)); + // This will turn into a bool constant, indicating whether + // we count the loop variable up or down: + AstNode* countupp = new AstLte(fl, leftp->cloneTree(true), + rightp->cloneTree(true)); + AstNode* comparep = new AstCond( + fl, countupp->cloneTree(true), + // Left increments up to right + new AstLte(fl, new AstVarRef(fl, varp->name(), false), + rightp->cloneTree(true)), + // Left decrements down to right + new AstGte(fl, new AstVarRef(fl, varp->name(), false), rightp)); + AstNode* incp = new AstAssign( + fl, new AstVarRef(fl, varp->name(), true), + new AstAdd(fl, new AstVarRef(fl, varp->name(), false), + new AstCond(fl, countupp, + new AstConst(fl, 1), + new AstConst(fl, -1)))); stmtsp->addNext(new AstWhile(fl, comparep, newp, incp)); newp = new AstBegin(nodep->fileline(),"",stmtsp); dimension--; diff --git a/test_regress/t/t_foreach.pl b/test_regress/t/t_foreach.pl index f91289753..60e20ab59 100755 --- a/test_regress/t/t_foreach.pl +++ b/test_regress/t/t_foreach.pl @@ -8,6 +8,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. compile ( + verilator_flags2 => ['--assert'] ); execute ( diff --git a/test_regress/t/t_foreach.v b/test_regress/t/t_foreach.v index 97fe8021e..4c9f4c649 100644 --- a/test_regress/t/t_foreach.v +++ b/test_regress/t/t_foreach.v @@ -13,12 +13,24 @@ module t (/*AUTOARG*/); reg [63:0] sum; reg [2:1] [4:3] array [5:6] [7:8]; reg [1:2] [3:4] larray [6:5] [8:7]; + bit [31:0] depth1_array [0:0]; function [63:0] crc (input [63:0] sum, input [31:0] a, input [31:0] b, input [31:0] c, input [31:0] d); crc = {sum[62:0],sum[63]} ^ {4'b0,a[7:0], 4'h0,b[7:0], 4'h0,c[7:0], 4'h0,d[7:0]}; endfunction initial begin + sum = 0; + foreach (depth1_array[index]) begin + sum = crc(sum, index, 0, 0, 0); + + // Ensure the index never goes out of bounds. + // We used to get this wrong for an array of depth 1. + assert (index != -1); + assert (index != 1); + end + `checkh(sum, 64'h0); + sum = 0; foreach (array[a]) begin sum = crc(sum, a, 0, 0, 0);