Fix associative array foreach loop (#3229).

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

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 << " -> ";

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<const AstLogOr*>(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:

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}))

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);

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