Fix break under foreach loop (#3230).

Internals: Move Foreach handling into V3Width.
This commit is contained in:
Wilson Snyder 2021-12-11 15:06:33 -05:00
parent 00ef0519f5
commit 984ee624ed
14 changed files with 315 additions and 80 deletions

View File

@ -21,6 +21,7 @@ Verilator 4.217 devel
* Support up to 64 bit enums for .next/.prev/.name (#3244). [Alexander Grobman]
* Fix MSWIN compile error (#2681). [Unai Martinez-Corral]
* Fix break under foreach loop (#3230).
* Fix VL_STREAML_FAST_QQI with 64 bit left-hand-side (#3232) (#3235)
* Fix $sformat of inputs/outputs (#3236). [Adrien Le Masle]

View File

@ -83,6 +83,7 @@ private:
// STATE
AstNodeModule* m_modp = nullptr; // Current module
AstSelLoopVars* m_selloopvarsp = nullptr; // Current loop vars
// List of all encountered to avoid another loop through tree
std::vector<AstVar*> m_varsp;
std::vector<AstNode*> m_dtypesp;
@ -249,6 +250,13 @@ private:
}
checkAll(nodep);
}
virtual void visit(AstSelLoopVars* nodep) override {
// Var under a SelLoopVars means we haven't called V3Width to remove them yet
VL_RESTORER(m_selloopvarsp);
m_selloopvarsp = nodep;
iterateChildren(nodep);
checkAll(nodep);
}
virtual void visit(AstTypedef* nodep) override {
iterateChildren(nodep);
if (m_elimCells && !nodep->attrPublic()) {
@ -270,6 +278,7 @@ private:
iterateChildren(nodep);
checkAll(nodep);
if (nodep->isSigPublic() && m_modp && VN_IS(m_modp, Package)) m_modp->user1Inc();
if (m_selloopvarsp) nodep->user1Inc();
if (mightElimVar(nodep)) m_varsp.push_back(nodep);
}
virtual void visit(AstNodeAssign* nodep) override {

View File

@ -1270,6 +1270,48 @@ class LinkDotFindVisitor final : public AstNVisitor {
m_curSymp->exportStarStar(m_statep->symsp());
// No longer needed, but can't delete until any multi-instantiated modules are expanded
}
virtual void visit(AstForeach* nodep) override {
// Symbol table needs nodep->name() as the index variable's name
VL_RESTORER(m_curSymp);
VSymEnt* const oldCurSymp = m_curSymp;
{
++m_modWithNum;
m_curSymp = m_statep->insertBlock(m_curSymp, "__Vforeach" + cvtToStr(m_modWithNum),
nodep, m_classOrPackagep);
m_curSymp->fallbackp(oldCurSymp);
const auto loopvarsp = VN_CAST(nodep->arrayp(), SelLoopVars);
if (!loopvarsp) {
AstNode* const warnp = nodep->arrayp() ? nodep->arrayp() : nodep;
warnp->v3warn(E_UNSUPPORTED,
"Unsupported (or syntax error): Foreach on this array's construct");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
for (AstNode *nextp, *argp = loopvarsp->elementsp(); argp; argp = nextp) {
nextp = argp->nextp();
AstVar* argrefp = nullptr;
if (const auto parserefp = VN_CAST(argp, ParseRef)) {
// We use an int type, this might get changed in V3Width when types resolve
argrefp = new AstVar{parserefp->fileline(), AstVarType::BLOCKTEMP,
parserefp->name(), argp->findSigned32DType()};
parserefp->replaceWith(argrefp);
VL_DO_DANGLING(parserefp->deleteTree(), parserefp);
// Insert argref's name into symbol table
m_statep->insertSym(m_curSymp, argrefp->name(), argrefp, nullptr);
} else if (const auto largrefp = VN_CAST(argp, Var)) {
argrefp = largrefp;
// Insert argref's name into symbol table
m_statep->insertSym(m_curSymp, argrefp->name(), argrefp, nullptr);
} else {
argp->v3error("'foreach' loop variable expects simple variable name");
}
}
}
}
virtual void visit(AstWithParse* nodep) override {
// Change WITHPARSE(FUNCREF, equation) to FUNCREF(WITH(equation))
const auto funcrefp = VN_AS(nodep->funcrefp(), NodeFTaskRef);
@ -1593,6 +1635,11 @@ class LinkDotScopeVisitor final : public AstNVisitor {
symp->fallbackp(m_modSymp);
// No recursion, we don't want to pick up variables
}
virtual void visit(AstForeach* nodep) override {
VSymEnt* const symp = m_statep->insertBlock(m_modSymp, nodep->name(), nodep, nullptr);
symp->fallbackp(m_modSymp);
// No recursion, we don't want to pick up variables
}
virtual void visit(AstWith* nodep) override {
VSymEnt* const symp = m_statep->insertBlock(m_modSymp, nodep->name(), nodep, nullptr);
symp->fallbackp(m_modSymp);
@ -2834,6 +2881,16 @@ private:
m_ds.m_dotSymp = m_curSymp = oldCurSymp;
m_ftaskp = nullptr;
}
virtual void visit(AstForeach* nodep) override {
UINFO(5, " " << nodep << endl);
checkNoDot(nodep);
VSymEnt* const oldCurSymp = m_curSymp;
{
m_ds.m_dotSymp = m_curSymp = m_statep->getNodeSym(nodep);
iterateChildren(nodep);
}
m_ds.m_dotSymp = m_curSymp = oldCurSymp;
}
virtual void visit(AstWith* nodep) override {
UINFO(5, " " << nodep << endl);
checkNoDot(nodep);

View File

@ -107,6 +107,22 @@ private:
// Done the loop
m_insStmtp = nullptr; // Next thing should be new statement
}
virtual void visit(AstForeach* nodep) override {
// Special, as statements need to be put in different places
// Body insert just before themselves
m_insStmtp = nullptr; // First thing should be new statement
iterateChildren(nodep);
// Done the loop
m_insStmtp = nullptr; // Next thing should be new statement
}
virtual void visit(AstJumpBlock* nodep) override {
// Special, as statements need to be put in different places
// Body insert just before themselves
m_insStmtp = nullptr; // First thing should be new statement
iterateChildren(nodep);
// Done the loop
m_insStmtp = nullptr; // Next thing should be new statement
}
virtual void visit(AstNodeIf* nodep) override {
m_insStmtp = nodep;
iterateAndNextNull(nodep->condp());

View File

@ -46,7 +46,7 @@ private:
// STATE
AstNodeModule* m_modp = nullptr; // Current module
AstNodeFTask* m_ftaskp = nullptr; // Current function/task
AstWhile* m_loopp = nullptr; // Current loop
AstNode* m_loopp = nullptr; // Current loop
bool m_loopInc = false; // In loop increment
bool m_inFork = false; // Under fork
int m_modRepeatNum = 0; // Repeat counter
@ -66,6 +66,13 @@ private:
underp = VN_AS(nodep, NodeBlock)->stmtsp();
} else if (VN_IS(nodep, NodeFTask)) {
underp = VN_AS(nodep, NodeFTask)->stmtsp();
} else if (VN_IS(nodep, Foreach)) {
if (endOfIter) {
underp = VN_AS(nodep, Foreach)->bodysp();
} else {
underp = nodep;
under_and_next = false; // IE we skip the entire foreach
}
} else if (VN_IS(nodep, While)) {
if (endOfIter) {
// Note we jump to end of bodysp; a FOR loop has its
@ -192,6 +199,13 @@ private:
iterateAndNextNull(nodep->incsp());
}
}
virtual void visit(AstForeach* nodep) override {
VL_RESTORER(m_loopp);
{
m_loopp = nodep;
iterateAndNextNull(nodep->bodysp());
}
}
virtual void visit(AstReturn* nodep) override {
iterateChildren(nodep);
const AstFunc* const funcp = VN_CAST(m_ftaskp, Func);

View File

@ -413,13 +413,8 @@ private:
}
virtual void visit(AstForeach* nodep) override {
// FOREACH(array,loopvars,body)
// -> BEGIN(declare vars, loopa=lowest; WHILE(loopa<=highest, ... body))
// nodep->dumpTree(cout, "-foreach-old:");
// FOREACH(array, loopvars, body)
UINFO(9, "FOREACH " << nodep << endl);
// nodep->dumpTree(cout, "-foreach-in:");
AstNode* newp = nodep->bodysp();
if (newp) newp->unlinkFrBackWithNext();
// Separate iteration vars from base from variable
// Input:
// v--- arrayp
@ -433,81 +428,22 @@ private:
// 3. ASTSELLOOPVARS(first, var0..var1))
// 4. DOT(DOT(first, second), ASTSELBIT(third, var0))
AstNode* bracketp = nodep->arrayp();
AstNode* firstVarsp = nullptr;
while (AstDot* dotp = VN_CAST(bracketp, Dot)) { bracketp = dotp->rhsp(); }
while (AstDot* dotp = VN_CAST(bracketp, Dot)) bracketp = dotp->rhsp();
if (AstSelBit* const selp = VN_CAST(bracketp, SelBit)) {
firstVarsp = selp->rhsp()->unlinkFrBackWithNext();
selp->replaceWith(selp->fromp()->unlinkFrBack());
// Convert to AstSelLoopVars so V3LinkDot knows what's being defined
AstNode* const newp
= new AstSelLoopVars{selp->fileline(), selp->fromp()->unlinkFrBack(),
selp->rhsp()->unlinkFrBackWithNext()};
selp->replaceWith(newp);
VL_DO_DANGLING(selp->deleteTree(), selp);
} else if (AstSelLoopVars* const selp = VN_CAST(bracketp, SelLoopVars)) {
firstVarsp = selp->elementsp()->unlinkFrBackWithNext();
selp->replaceWith(selp->fromp()->unlinkFrBack());
VL_DO_DANGLING(selp->deleteTree(), selp);
// Ok
} else {
nodep->v3error(
"Syntax error; foreach missing bracketed index variable (IEEE 1800-2017 12.7.3)");
"Syntax error; foreach missing bracketed loop variable (IEEE 1800-2017 12.7.3)");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
AstNode* const arrayp = nodep->arrayp(); // Maybe different node since bracketp looked
if (!VN_IS(arrayp, ParseRef) && !VN_IS(arrayp, Dot)) {
// Code below needs to use other then attributes to figure out the bounds
// Also need to deal with queues, etc
arrayp->v3warn(E_UNSUPPORTED, "Unsupported: foreach on non-simple variable reference");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
// Split into for loop
// Must do innermost (last) variable first
int dimension = 1;
AstNode* lastVarsp = firstVarsp;
while (lastVarsp->nextp()) {
lastVarsp = lastVarsp->nextp();
dimension++;
}
for (AstNode* varsp = lastVarsp; varsp; varsp = varsp->backp()) {
UINFO(9, "foreachVar " << varsp << endl);
FileLine* const fl = varsp->fileline();
AstNode* const varp
= new AstVar(fl, AstVarType::BLOCKTEMP, varsp->name(), nodep->findSigned32DType());
// These will be the left and right dimensions and size of the array:
AstNode* const leftp = new AstAttrOf(
fl, AstAttrType::DIM_LEFT, arrayp->cloneTree(false), new AstConst(fl, dimension));
AstNode* const rightp = new AstAttrOf(
fl, AstAttrType::DIM_RIGHT, arrayp->cloneTree(false), new AstConst(fl, dimension));
AstNode* const sizep = new AstAttrOf(
fl, AstAttrType::DIM_SIZE, arrayp->cloneTree(false), new AstConst(fl, dimension));
AstNode* const stmtsp = varp;
// Assign left-dimension into the loop var:
stmtsp->addNext(
new AstAssign(fl, new AstVarRef(fl, varp->name(), VAccess::WRITE), leftp));
// This will turn into a constant bool for static arrays
AstNode* const notemptyp = new AstGt(fl, sizep, new AstConst(fl, 0));
// This will turn into a bool constant, indicating whether
// we count the loop variable up or down:
AstNode* const countupp
= new AstLte(fl, leftp->cloneTree(true), rightp->cloneTree(true));
AstNode* const comparep = new AstCond(
fl, countupp->cloneTree(true),
// Left increments up to right
new AstLte(fl, new AstVarRef(fl, varp->name(), VAccess::READ),
rightp->cloneTree(true)),
// Left decrements down to right
new AstGte(fl, new AstVarRef(fl, varp->name(), VAccess::READ), rightp));
// This will reduce to comparep for static arrays
AstNode* const condp = new AstAnd(fl, notemptyp, comparep);
AstNode* const incp = new AstAssign(
fl, new AstVarRef(fl, varp->name(), VAccess::WRITE),
new AstAdd(fl, new AstVarRef(fl, varp->name(), VAccess::READ),
new AstCond(fl, countupp, new AstConst(fl, 1), new AstConst(fl, -1))));
stmtsp->addNext(new AstWhile(fl, condp, newp, incp));
newp = new AstBegin(nodep->fileline(), "", stmtsp, false, true);
dimension--;
}
// newp->dumpTree(cout, "-foreach-new:");
VL_DO_DANGLING(firstVarsp->deleteTree(), firstVarsp);
nodep->replaceWith(newp);
VL_DO_DANGLING(nodep->deleteTree(), nodep);
}
virtual void visit(AstNodeModule* nodep) override {

View File

@ -3793,6 +3793,120 @@ private:
userIterateAndNext(nodep->resultp(), m_vup);
nodep->dtypeFrom(nodep->resultp());
}
virtual void visit(AstForeach* nodep) override {
const AstSelLoopVars* const loopsp = VN_CAST(nodep->arrayp(), SelLoopVars);
UASSERT_OBJ(loopsp, nodep, "No loop variables under foreach");
// if (debug()) nodep->dumpTree(cout, "-foreach-old: ");
AstNode* const fromp = loopsp->fromp();
userIterateAndNext(fromp, WidthVP(SELF, BOTH).p());
AstNodeDType* fromDtp = fromp->dtypep()->skipRefp();
// Split into for loop
AstNode* bodyp = nodep->bodysp(); // Might be null
if (bodyp) bodyp->unlinkFrBackWithNext();
// We record where the body needs to eventually go with bodyPointp
// (Can't use bodyp as might be null)
AstNode* lastBodyPointp = nullptr;
AstNode* newp = nullptr;
// Major dimension first
while (AstNode* argsp
= loopsp->elementsp()) { // Loop advances due to below varp->unlinkFrBack()
AstVar* const varp = VN_CAST(argsp, Var);
UASSERT_OBJ(varp, argsp, "Missing foreach loop variable");
varp->unlinkFrBack();
fromDtp = fromDtp->skipRefp();
if (!fromDtp) {
argsp->v3error("foreach loop variables exceed number of indices of array");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
UINFO(9, "- foreachArg " << argsp << endl);
UINFO(9, "- from on " << fromp << endl);
UINFO(9, "- from dtp " << fromDtp << endl);
FileLine* const fl = argsp->fileline();
AstNode* bodyPointp = new AstBegin{fl, "[EditWrapper]", nullptr};
AstNode* loopp = nullptr;
if (const AstNodeArrayDType* const adtypep = VN_CAST(fromDtp, NodeArrayDType)) {
loopp = createForeachLoopRanged(nodep, bodyPointp, varp, adtypep->declRange());
// Prep for next
fromDtp = fromDtp->subDTypep();
} else if (AstBasicDType* const adtypep = VN_CAST(fromDtp, BasicDType)) {
if (!adtypep->isRanged()) {
argsp->v3error("Illegal to foreach loop on basic '" + fromDtp->prettyTypeName()
+ "'");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
loopp = createForeachLoopRanged(nodep, bodyPointp, varp, adtypep->declRange());
// Prep for next
fromDtp = nullptr;
} else if (VN_IS(fromDtp, DynArrayDType) || VN_IS(fromDtp, QueueDType)) {
auto* const leftp = new AstConst{fl, AstConst::Signed32{}, 0};
auto* const sizep
= new AstCMethodHard{fl, fromp->cloneTree(false), "size", nullptr};
sizep->dtypeSetSigned32();
sizep->didWidth(true);
sizep->protect(false);
AstNode* const condp
= new AstLt{fl, new AstVarRef{fl, varp, VAccess::READ}, sizep};
AstNode* const incp = new AstAdd{fl, new AstConst{fl, AstConst::Signed32{}, 1},
new AstVarRef{fl, varp, VAccess::READ}};
loopp = createForeachLoop(nodep, bodyPointp, varp, leftp, condp, incp);
// Prep for next
fromDtp = fromDtp->subDTypep();
} else {
argsp->v3error("Illegal to foreach loop on '" + fromDtp->prettyTypeName() + "'");
VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
return;
}
// New loop goes UNDER previous loop
if (!newp) {
newp = loopp;
} else {
lastBodyPointp->replaceWith(loopp);
}
lastBodyPointp = bodyPointp;
}
UASSERT_OBJ(newp, nodep, "foreach has no non-empty loop variable");
if (bodyp) {
lastBodyPointp->replaceWith(bodyp);
} else {
lastBodyPointp->unlinkFrBack();
}
// if (debug()) newp->dumpTreeAndNext(cout, "-foreach-new: ");
nodep->replaceWith(newp);
VL_DO_DANGLING(lastBodyPointp->deleteTree(), lastBodyPointp);
VL_DO_DANGLING(nodep->deleteTree(), nodep);
}
AstNode* createForeachLoopRanged(AstForeach* nodep, AstNode* bodysp, AstVar* varp,
const VNumRange& declRange) {
FileLine* const fl = varp->fileline();
auto* const leftp = new AstConst{fl, AstConst::Signed32{}, declRange.left()};
auto* const rightp = new AstConst{fl, AstConst::Signed32{}, declRange.right()};
AstNode* condp;
AstNode* incp;
if (declRange.left() < declRange.right()) {
condp = new AstLte{fl, new AstVarRef{fl, varp, VAccess::READ}, rightp};
incp = new AstAdd{fl, new AstConst{fl, AstConst::Signed32{}, 1},
new AstVarRef{fl, varp, VAccess::READ}};
} else {
condp = new AstGte{fl, new AstVarRef{fl, varp, VAccess::READ}, rightp};
incp = new AstSub{fl, new AstVarRef{fl, varp, VAccess::READ},
new AstConst{fl, AstConst::Signed32{}, 1}};
}
return createForeachLoop(nodep, bodysp, varp, leftp, condp, incp);
}
AstNode* createForeachLoop(AstForeach* nodep, AstNode* bodysp, AstVar* varp, AstNode* leftp,
AstNode* condp, AstNode* incp) {
FileLine* const fl = varp->fileline();
auto* const whilep = new AstWhile{
fl, condp, bodysp, new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, incp}};
AstNode* const stmtsp = varp; // New statements for under new Begin
stmtsp->addNext(new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, leftp});
stmtsp->addNext(whilep);
AstNode* const newp = new AstBegin{nodep->fileline(), "", stmtsp, false, true};
return newp;
}
virtual void visit(AstNodeAssign* nodep) override {
// IEEE-2012 10.7, 11.8.2, 11.8.3, 11.5: (Careful of 11.8.1 which is

View File

@ -3522,8 +3522,8 @@ for_step_assignment<nodep>: // ==IEEE: for_step_assignment
;
loop_variables<nodep>: // IEEE: loop_variables
varRefBase { $$ = $1; }
| loop_variables ',' varRefBase { $$ = $1; $1->addNext($3); }
parseRefBase { $$ = $1; }
| loop_variables ',' parseRefBase { $$ = $1; $1->addNext($3); }
;
//************************************************
@ -5014,6 +5014,12 @@ varRefBase<varRefp>:
id { $$ = new AstVarRef($<fl>1, *$1, VAccess::READ); }
;
// ParseRef
parseRefBase<nodep>:
id
{ $$ = new AstParseRef{$<fl>1, VParseRefExp::PX_TEXT, *$1, nullptr, nullptr}; }
;
// yaSTRING shouldn't be used directly, instead via an abstraction below
str<strp>: // yaSTRING but with \{escapes} need decoded
yaSTRING { $$ = PARSEP->newString(GRAMMARP->deQuote($<fl>1,*$1)); }

View File

@ -133,6 +133,10 @@ module t (/*AUTOARG*/
`checkh(a[2], 3);
`checkh(a[3], 4);
i = 0;
foreach (a[j]) i += int'(a[j]);
`checkh(i, 1 + 2 + 3 + 4);
// test wide dynamic array
p256 = new [11];
`checkh(p256.size, 11);

View File

@ -72,7 +72,6 @@ module t (/*AUTOARG*/);
`checkh(sum, 64'h0030128ab2a8e557);
//
sum = 0;
foreach (larray[index_a]) begin
sum = crc(sum, index_a, 0, 0, 0);
@ -119,13 +118,31 @@ module t (/*AUTOARG*/);
end
`checkh(add, 1); // 9
add = 0;
foreach (oned[i]) begin
++add;
continue;
add += 100;
end
`checkh(add, 3); // 9, 8, 7
add = 0;
foreach (twod[i, j]) begin
++add;
break;
end
`checkh(add, 3); // 3,9 3,9, 3,9
// Although many simulators also do just "0,0". IEEE not clear - should we warn?.
// See https://www.accellera.org/images/eda/sv-bc/10303.html
`checkh(add, 1); // 3,9
add = 0;
foreach (twod[i, j]) begin
++add;
continue;
add += 100;
end
`checkh(add, 6);
foreach (twod[i, j]); // Null body check
$write("*-* All Finished *-*\n");
$finish;

View File

@ -1,4 +1,8 @@
%Error: t/t_foreach_bad.v:14:7: Syntax error; foreach missing bracketed index variable (IEEE 1800-2017 12.7.3)
%Error: t/t_foreach_bad.v:14:7: Syntax error; foreach missing bracketed loop variable (IEEE 1800-2017 12.7.3)
14 | foreach (array);
| ^~~~~~~
%Error-UNSUPPORTED: t/t_foreach_bad.v:16:21: Unsupported (or syntax error): Foreach on this array's construct
16 | foreach (array.array[a]);
| ^
... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
%Error: Exiting due to

View File

@ -0,0 +1,10 @@
%Error: t/t_foreach_type_bad.v:19:18: Illegal to foreach loop on 'CLASSREFDTYPE 'Cls''
19 | foreach (c[i]);
| ^
%Error: t/t_foreach_type_bad.v:21:18: Illegal to foreach loop on basic 'BASICDTYPE 'real''
21 | foreach (r[i]);
| ^
%Error: t/t_foreach_type_bad.v:23:21: Illegal to foreach loop on basic 'BASICDTYPE 'bit''
23 | foreach (b[i, j, k]);
| ^
%Error: Exiting due to

View File

@ -0,0 +1,19 @@
#!/usr/bin/env 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.
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
scenarios(linter => 1);
lint(
fails => 1,
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

View File

@ -0,0 +1,28 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2020 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0
class Cls;
endclass
module t (/*AUTOARG*/);
real r;
bit b[2];
Cls c;
initial begin
foreach (c[i]); // bad type
foreach (r[i]); // no loop var
foreach (b[i, j, k]); // extra loop var
$stop;
end
endmodule