Improve Loop unrolling (#6480) (#6493)

This patch implements #6480. All loop statements are represented using
AstLoop and AstLoopTest.

This necessitates rework of the loop unroller to handle loops of
arbitrary form. To enable this, I have split the old unroller used for
'generate for' statements and moved it into V3Param, and subsequently
rewrote V3Unroll to handle the new representation. V3Unroll can now
unroll more complex loops, including with loop conditions containing
multiple variable references or inlined functions.

Handling the more generic code also requires some restrictions. If a
loop contains any of the following, it cannot be unrolled:
- A timing control that might suspend the loop
- A non-inlined call to a non-pure function

These constructs can change the values of variables in the loop, so are
generally not safe to unroll if they are present. (We could still unroll
if all the variables needed for unrolling are automatic, however we
don't do that right now.)

These restrictions seem ok in the benchmark suite, where the new
unroller can generally unroll many more loops than before.

src/V3Coverage.cpp

@@ -135,7 +135,7 @@ class CoverageVisitor final : public VNVisitor {
     // NODE STATE
     // Entire netlist:
     //  AstIf::user1()                  -> bool.  True indicates ifelse processed
-    //  AstIf::user2()                  -> bool.  True indicates coverage-generated
+    //  AstIf/AstLoopTest::user2()      -> bool.  True indicates coverage-generated
     const VNUser1InUse m_inuser1;
     const VNUser2InUse m_inuser2;
 
@@ -153,7 +153,6 @@ class CoverageVisitor final : public VNVisitor {
     bool m_objective = false;  // Expression objective
     bool m_ifCond = false;  // Visiting if condition
     bool m_inToggleOff = false;  // In function/task etc
-    bool m_inLoopNotBody = false;  // Inside a loop, but not in its body
     string m_beginHier;  // AstBegin hier name for user coverage points
 
     // STATE - cleared each module
@@ -285,23 +284,14 @@ class CoverageVisitor final : public VNVisitor {
     }
 
     void visit(AstNodeProcedure* nodep) override { iterateProcedure(nodep); }
-    // we can cover expressions in while loops, but the counting goes outside
-    // the while, see: "minimally-intelligent decision about ... clock domain"
-    // in the Toggle Coverage docs
-    void visit(AstWhile* nodep) override {
+    void visit(AstLoop* nodep) override {
+        UASSERT_OBJ(!nodep->contsp(), nodep, "'contsp' only used before LinkJump");
         VL_RESTORER(m_state);
         VL_RESTORER(m_inToggleOff);
         m_inToggleOff = true;
         createHandle(nodep);
-        {
-            VL_RESTORER(m_inLoopNotBody);
-            m_inLoopNotBody = true;
-            iterateNull(nodep->condp());
-            iterateAndNextNull(nodep->incsp());
-        }
         iterateAndNextNull(nodep->stmtsp());
         if (m_state.lineCoverageOn(nodep)) {
-            lineTrack(nodep);
             AstCoverOtherDecl* const declp
                 = new AstCoverOtherDecl{nodep->fileline(), "v_line/" + m_modp->prettyName(),
                                         "block", linesCov(m_state, nodep), 0};
@@ -311,6 +301,21 @@ class CoverageVisitor final : public VNVisitor {
             insertProcStatement(nodep, newp);
         }
     }
+    void visit(AstLoopTest* nodep) override {
+        if (nodep->user2SetOnce()) return;
+        lineTrack(nodep);
+        if (m_state.lineCoverageOn(nodep) && nodep->backp()->nextp() == nodep) {
+            AstCoverOtherDecl* const declp
+                = new AstCoverOtherDecl{nodep->fileline(), "v_line/" + m_modp->prettyName(),
+                                        "block", linesCov(m_state, nodep), 0};
+            m_modp->addStmtsp(declp);
+            AstNode* const newp
+                = newCoverInc(nodep->fileline(), declp, traceNameForLine(nodep, "block"));
+            nodep->addHereThisAsNext(newp);
+            createHandle(nodep);
+        }
+        iterateChildren(nodep);
+    }
 
     void visit(AstNodeFTask* nodep) override {
         if (!nodep->dpiImport()) iterateProcedure(nodep);
@@ -321,7 +326,7 @@ class CoverageVisitor final : public VNVisitor {
             itemp->addStmtsp(stmtp);
         } else if (AstNodeFTask* const itemp = VN_CAST(nodep, NodeFTask)) {
             itemp->addStmtsp(stmtp);
-        } else if (AstWhile* const itemp = VN_CAST(nodep, While)) {
+        } else if (AstLoop* const itemp = VN_CAST(nodep, Loop)) {
             itemp->addStmtsp(stmtp);
         } else if (AstIf* const itemp = VN_CAST(nodep, If)) {
             if (m_then) {
@@ -499,8 +504,7 @@ class CoverageVisitor final : public VNVisitor {
             return includeCondToBranchRecursive(backp);
         } else if (VN_IS(backp, Sel) && VN_AS(backp, Sel)->fromp() == nodep) {
             return includeCondToBranchRecursive(backp);
-        } else if (VN_IS(backp, NodeAssign) && VN_AS(backp, NodeAssign)->rhsp() == nodep
-                   && !m_inLoopNotBody) {
+        } else if (VN_IS(backp, NodeAssign) && VN_AS(backp, NodeAssign)->rhsp() == nodep) {
             return true;
         }
         return false;