Support lower dimension looping in foreach loops (#3172).

Changes

@@ -19,6 +19,7 @@ Verilator 4.217 devel
 
 **Minor:**
 
+* Support lower dimension looping in foreach loops (#3172). [Ehab Ibrahim]
 * 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).

src/V3AstNodes.h

@@ -183,6 +183,15 @@ public:
     static AstConst* parseParamLiteral(FileLine* fl, const string& literal);
 };
 
+class AstEmpty final : public AstNode {
+    // Represents something missing, e.g. a missing argument in FOREACH
+public:
+    AstEmpty(FileLine* fl)
+        : ASTGEN_SUPER_Empty(fl) {}
+    ASTNODE_NODE_FUNCS(Empty)
+    virtual bool same(const AstNode* samep) const override { return true; }
+};
+
 class AstEmptyQueue final : public AstNodeMath {
 public:
     AstEmptyQueue(FileLine* fl)

src/V3LinkDot.cpp

@@ -1305,6 +1305,7 @@ class LinkDotFindVisitor final : public AstNVisitor {
                     argrefp = largrefp;
                     // Insert argref's name into symbol table
                     m_statep->insertSym(m_curSymp, argrefp->name(), argrefp, nullptr);
+                } else if (VN_IS(argp, Empty)) {
                 } else {
                     argp->v3error("'foreach' loop variable expects simple variable name");
                 }

src/V3Width.cpp

@@ -3810,16 +3810,17 @@ private:
         // Major dimension first
         while (AstNode* argsp
                = loopsp->elementsp()) {  // Loop advances due to below varp->unlinkFrBack()
+            const bool empty = VN_IS(argsp, Empty);
             AstVar* const varp = VN_CAST(argsp, Var);
-            UASSERT_OBJ(varp, argsp, "Missing foreach loop variable");
+            UASSERT_OBJ(varp || empty, argsp, "Missing foreach loop variable");
-            varp->usedLoopIdx(true);
+            if (varp) varp->usedLoopIdx(true);
-            varp->unlinkFrBack();
+            argsp->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;
             }
+            fromDtp = fromDtp->skipRefp();
             UINFO(9, "- foreachArg " << argsp << endl);
             UINFO(9, "-   from on  " << fromp << endl);
             UINFO(9, "-   from dtp " << fromDtp << endl);
@@ -3828,7 +3829,9 @@ private:
             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());
+                if (varp) {
+                    loopp = createForeachLoopRanged(nodep, bodyPointp, varp, adtypep->declRange());
+                }
                 // Prep for next
                 fromDtp = fromDtp->subDTypep();
             } else if (AstBasicDType* const adtypep = VN_CAST(fromDtp, BasicDType)) {
@@ -3838,21 +3841,25 @@ private:
                     VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
                     return;
                 }
-                loopp = createForeachLoopRanged(nodep, bodyPointp, varp, adtypep->declRange());
+                if (varp) {
+                    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};
+                if (varp) {
-                auto* const sizep
+                    auto* const leftp = new AstConst{fl, AstConst::Signed32{}, 0};
-                    = new AstCMethodHard{fl, fromp->cloneTree(false), "size", nullptr};
+                    auto* const sizep
-                sizep->dtypeSetSigned32();
+                        = new AstCMethodHard{fl, fromp->cloneTree(false), "size", nullptr};
-                sizep->didWidth(true);
+                    sizep->dtypeSetSigned32();
-                sizep->protect(false);
+                    sizep->didWidth(true);
-                AstNode* const condp
+                    sizep->protect(false);
-                    = new AstLt{fl, new AstVarRef{fl, varp, VAccess::READ}, sizep};
+                    AstNode* const condp
-                AstNode* const incp = new AstAdd{fl, new AstConst{fl, AstConst::Signed32{}, 1},
+                        = new AstLt{fl, new AstVarRef{fl, varp, VAccess::READ}, sizep};
-                                                 new AstVarRef{fl, varp, VAccess::READ}};
+                    AstNode* const incp = new AstAdd{fl, new AstConst{fl, AstConst::Signed32{}, 1},
-                loopp = createForeachLoop(nodep, bodyPointp, varp, leftp, condp, incp);
+                                                     new AstVarRef{fl, varp, VAccess::READ}};
+                    loopp = createForeachLoop(nodep, bodyPointp, varp, leftp, condp, incp);
+                }
                 // Prep for next
                 fromDtp = fromDtp->subDTypep();
             } else if (const AstAssocArrayDType* const adtypep
@@ -3897,13 +3904,16 @@ private:
                 return;
             }
             // New loop goes UNDER previous loop
-            if (!newp) {
+            if (varp) {
-                newp = loopp;
+                if (!newp) {
-            } else {
+                    newp = loopp;
-                lastBodyPointp->replaceWith(loopp);
+                } else {
+                    lastBodyPointp->replaceWith(loopp);
+                }
+                lastBodyPointp = bodyPointp;
             }
-            lastBodyPointp = bodyPointp;
         }
+        // The parser validates we don't have "foreach (array[,,,])"
         UASSERT_OBJ(newp, nodep, "foreach has no non-empty loop variable");
         if (bodyp) {
             lastBodyPointp->replaceWith(bodyp);

src/verilog.y

@@ -3523,7 +3523,8 @@ for_step_assignment<nodep>:  // ==IEEE: for_step_assignment
 
 loop_variables<nodep>:		// IEEE: loop_variables
 		parseRefBase				{ $$ = $1; }
-	|	loop_variables ',' parseRefBase		{ $$ = $1; $1->addNext($3); }
+	|	loop_variables ',' parseRefBase		{ $$ = $1; $$->addNext($3); }
+	|	',' parseRefBase			{ $$ = new AstEmpty{$1}; $$->addNext($2); }
 	;
 
 //************************************************
@@ -5007,6 +5008,8 @@ idArrayedForeach<nodep>:	// IEEE: id + select (under foreach expression)
 	//			// To avoid conflicts we allow expr as first element, must post-check
 	|	idArrayed '[' expr ',' loop_variables ']'
 			{ $3 = AstNode::addNextNull($3, $5); $$ = new AstSelLoopVars($2, $1, $3); }
+	|	idArrayed '[' ',' loop_variables ']'
+			{ $4 = AstNode::addNextNull(new AstEmpty{$3}, $4); $$ = new AstSelLoopVars($2, $1, $4); }
 	;
 
 // VarRef without any dots or vectorizaion

test_regress/t/t_foreach.v

@@ -71,6 +71,14 @@ module t (/*AUTOARG*/);
       end
       `checkh(sum, 64'h0030128ab2a8e557);
 
+      // comma syntax
+      sum = 0;
+      foreach (array[,index_b]) begin
+         $display(index_b);
+         sum = crc(sum, 0, index_b, 0, 0);
+      end
+      `checkh(sum, 64'h0000000006000000);
+
       //
       sum = 0;
       foreach (larray[index_a]) begin

test_regress/t/t_foreach_type_bad.out

@@ -7,4 +7,7 @@
 %Error: t/t_foreach_type_bad.v:23:21: Illegal to foreach loop on basic 'BASICDTYPE 'bit''
    23 |       foreach (b[i, j, k]);   
       |                     ^
+%Error: t/t_foreach_type_bad.v:25:18: Illegal to foreach loop on basic 'BASICDTYPE 'real''
+   25 |       foreach (r[, i]);   
+      |                  ^
 %Error: Exiting due to

test_regress/t/t_foreach_type_bad.v

@@ -22,6 +22,8 @@ module t (/*AUTOARG*/);
 
       foreach (b[i, j, k]);  // extra loop var
 
+      foreach (r[, i]);  // no loop var and extra
+
       $stop;
    end