From 740fee660eae220599a307408ed204acf8dd60e3 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 11 Dec 2021 18:38:23 -0500 Subject: [PATCH] Fix associative array foreach loop (#3229). --- Changes | 1 + src/V3AstNodes.cpp | 4 ++++ src/V3AstNodes.h | 13 +++++++++++++ src/V3Const.cpp | 4 ++-- src/V3Width.cpp | 37 +++++++++++++++++++++++++++++++++++++ test_regress/t/t_assoc.v | 10 ++++++++++ 6 files changed, 67 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 5e95bdf10..4b8796a49 100644 --- a/Changes +++ b/Changes @@ -24,6 +24,7 @@ Verilator 4.217 devel * 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] +* Fix associative array foreach loop (#3229). Verilator 4.216 2021-12-05 diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 13df5edd9..bbafbc259 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1392,6 +1392,10 @@ void AstJumpLabel::dump(std::ostream& str) const { str << "%Error:UNLINKED"; } } +void AstLogOr::dump(std::ostream& str) const { + this->AstNode::dump(str); + if (sideEffect()) str << " [SIDE]"; +} void AstMemberSel::dump(std::ostream& str) const { this->AstNodeMath::dump(str); str << " -> "; diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index accdcb154..db2fd7487 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -6590,6 +6590,11 @@ public: // Binary ops class AstLogOr final : public AstNodeBiop { + // LOGOR with optional side effects + // Side effects currently used in some V3Width code + // TBD if this concept is generally adopted for side-effect tracking + // versus V3Const tracking it itself + bool m_sideEffect = false; // Has side effect, relies on short-circuiting public: AstLogOr(FileLine* fl, AstNode* lhsp, AstNode* rhsp) : ASTGEN_SUPER_LogOr(fl, lhsp, rhsp) { @@ -6602,6 +6607,11 @@ public: virtual void numberOperate(V3Number& out, const V3Number& lhs, const V3Number& rhs) override { out.opLogOr(lhs, rhs); } + virtual bool same(const AstNode* samep) const override { + const AstLogOr* const sp = static_cast(samep); + return m_sideEffect == sp->m_sideEffect; + } + virtual void dump(std::ostream& str = std::cout) const override; virtual string emitVerilog() override { return "%k(%l %f|| %r)"; } virtual string emitC() override { return "VL_LOGOR_%nq%lq%rq(%nw,%lw,%rw, %P, %li, %ri)"; } virtual string emitSimpleOperator() override { return "||"; } @@ -6611,6 +6621,9 @@ public: virtual bool sizeMattersLhs() const override { return false; } virtual bool sizeMattersRhs() const override { return false; } virtual int instrCount() const override { return widthInstrs() + INSTR_COUNT_BRANCH; } + virtual bool isPure() const override { return !m_sideEffect; } + void sideEffect(bool flag) { m_sideEffect = flag; } + bool sideEffect() const { return m_sideEffect; } }; class AstLogAnd final : public AstNodeBiop { public: diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 5011b7711..3f5c707d5 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3209,7 +3209,7 @@ private: TREEOP ("AstAnd {$lhsp, $rhsp.isAllOnes}", "replaceWLhs(nodep)"); TREEOP ("AstLogAnd{$lhsp, $rhsp.isNeqZero}", "replaceWLhs(nodep)"); TREEOP ("AstOr {$lhsp, $rhsp.isAllOnes, isTPure($lhsp)}", "replaceWRhs(nodep)"); //->allOnes - TREEOP ("AstLogOr {$lhsp, $rhsp.isNeqZero, isTPure($lhsp)}", "replaceNum(nodep,1)"); + TREEOP ("AstLogOr {$lhsp, $rhsp.isNeqZero, isTPure($lhsp), nodep->isPure()}", "replaceNum(nodep,1)"); TREEOP ("AstXor {$lhsp.isAllOnes, $rhsp}", "AstNot{$rhsp}"); TREEOP ("AstMul {$lhsp.isOne, $rhsp}", "replaceWRhs(nodep)"); TREEOP ("AstMulS {$lhsp.isOne, $rhsp}", "replaceWRhs(nodep)"); @@ -3383,7 +3383,7 @@ private: TREEOPV("AstOneHot0{$lhsp.width1}", "replaceNum(nodep,1)"); // Binary AND/OR is faster than logical and/or (usually) TREEOPV("AstLogAnd{$lhsp.width1, $rhsp.width1, isTPure($lhsp), isTPure($rhsp)}", "AstAnd{$lhsp,$rhsp}"); - TREEOPV("AstLogOr {$lhsp.width1, $rhsp.width1, isTPure($lhsp), isTPure($rhsp)}", "AstOr{$lhsp,$rhsp}"); + TREEOPV("AstLogOr {$lhsp.width1, $rhsp.width1, nodep->isPure(), isTPure($lhsp), isTPure($rhsp)}", "AstOr{$lhsp,$rhsp}"); TREEOPV("AstLogNot{$lhsp.width1, isTPure($lhsp)}", "AstNot{$lhsp}"); // CONCAT(CONCAT({a},{b}),{c}) -> CONCAT({a},CONCAT({b},{c})) // CONCAT({const},CONCAT({const},{c})) -> CONCAT((constifiedCONC{const|const},{c})) diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 07def497c..e5a0ee2f1 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3812,6 +3812,7 @@ private: = 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->usedLoopIdx(true); varp->unlinkFrBack(); fromDtp = fromDtp->skipRefp(); if (!fromDtp) { @@ -3854,6 +3855,42 @@ private: loopp = createForeachLoop(nodep, bodyPointp, varp, leftp, condp, incp); // Prep for next fromDtp = fromDtp->subDTypep(); + } else if (const AstAssocArrayDType* const adtypep + = VN_CAST(fromDtp, AssocArrayDType)) { + // Make this: var KEY_TYPE index; + // bit index__Vfirst; + // index__Vfirst = 0; + // if (0 != array.first(index)) + // do body while (index__Vfirst || 0 != array.next(index)) + varp->dtypeFrom(adtypep->keyDTypep()); + AstVar* const first_varp = new AstVar{ + fl, AstVarType::BLOCKTEMP, varp->name() + "__Vfirst", VFlagBitPacked{}, 1}; + first_varp->usedLoopIdx(true); + AstNode* const firstp = new AstMethodCall{ + fl, fromp->cloneTree(false), "first", + new AstArg{fl, "", new AstVarRef{fl, varp, VAccess::READWRITE}}}; + AstNode* const nextp = new AstMethodCall{ + fl, fromp->cloneTree(false), "next", + new AstArg{fl, "", new AstVarRef{fl, varp, VAccess::READWRITE}}}; + AstNode* const first_clearp + = new AstAssign{fl, new AstVarRef{fl, first_varp, VAccess::WRITE}, + new AstConst{fl, AstConst::BitFalse{}}}; + auto* const orp = new AstLogOr{fl, new AstVarRef{fl, first_varp, VAccess::READ}, + new AstNeq{fl, new AstConst{fl, 0}, nextp}}; + orp->sideEffect(true); + AstNode* const whilep = new AstWhile{fl, orp, first_clearp}; + first_clearp->addNext(bodyPointp); + AstNode* const ifbodyp + = new AstAssign{fl, new AstVarRef{fl, first_varp, VAccess::WRITE}, + new AstConst{fl, AstConst::BitTrue{}}}; + ifbodyp->addNext(whilep); + AstNode* const stmtsp = varp; // New statements for under new Begin + stmtsp->addNext(first_varp); + stmtsp->addNext( + new AstIf{fl, new AstNeq{fl, new AstConst{fl, 0}, firstp}, ifbodyp}); + loopp = new AstBegin{nodep->fileline(), "", stmtsp, false, true}; + // Prep for next + fromDtp = fromDtp->subDTypep(); } else { argsp->v3error("Illegal to foreach loop on '" + fromDtp->prettyTypeName() + "'"); VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); diff --git a/test_regress/t/t_assoc.v b/test_regress/t/t_assoc.v index 278003ca4..1425573c5 100644 --- a/test_regress/t/t_assoc.v +++ b/test_regress/t/t_assoc.v @@ -99,6 +99,16 @@ module t (/*AUTOARG*/ `checkh(a[~65'hfe], ~ 90'hfee); end + begin + int a [string]; + int sum; + sum = 0; + a["one"] = 1; + a["two"] = 2; + foreach (a[i]) sum += a[i]; + `checkh(sum, 1 + 2); + end + $write("*-* All Finished *-*\n"); $finish; end