Fix dynamic NBAs with automatic vars (#4696)

This patch addresses two issues with NBAs in non-inlined functions/tasks:
- If the NBA writes to a local automatic var, the var could cease to exist before the NBA executes. This is normally addressed by fork dynscopes (#4356), but NBA-to-fork transformation happens way after `V3Fork` (in `V3Timing`). To solve this, we put NBAs that write to locals under forks in `V3Fork` already. This way, such locals will be put in dynscopes, and will still exist after the task containing the NBA exits.
- The above change means that any writes in forks other than `fork..join` should be handled by `V3Fork`. Thus, in `V3SchedTiming`, we only have to worry about read references, so we can simply copy all remaining locals. Because we copy, lifetimes are not an issue. This fixes a bug that allowed assignment intravals to be overwritten if they go out of scope in the containing function.

src/V3Fork.cpp

@@ -256,6 +256,7 @@ class DynScopeVisitor final : public VNVisitor {
     // AstVar::user1()          -> int, timing-control fork nesting level of that variable
     // AstVarRef::user2()       -> bool, 1 = Node is a class handle reference. The handle gets
     //                                       modified in the context of this reference.
+    // AstAssignDly::user2()    -> bool, true if already visited
     const VNUser1InUse m_inuser1;
     const VNUser2InUse m_inuser2;
 
@@ -311,11 +312,8 @@ class DynScopeVisitor final : public VNVisitor {
     }
 
     static bool hasAsyncFork(AstNode* nodep) {
-        bool afork = false;
-        nodep->foreach([&](AstFork* forkp) {
-            if (!forkp->joinType().join()) afork = true;
-        });
-        return afork;
+        return nodep->exists([](AstFork* forkp) { return !forkp->joinType().join(); })
+               || nodep->exists([](AstAssignDly*) { return true; });
     }
 
     void bindNodeToDynScope(AstNode* nodep, ForkDynScopeFrame* frame) {
@@ -413,6 +411,20 @@ class DynScopeVisitor final : public VNVisitor {
         }
         visit(static_cast<AstNodeStmt*>(nodep));
     }
+    void visit(AstAssignDly* nodep) override {
+        if (m_procp && !nodep->user2()  // Unhandled AssignDly in function/task
+            && nodep->lhsp()->exists(  // And writes to a local variable
+                [](AstVarRef* refp) { return refp->varp()->isFuncLocal(); })) {
+            nodep->user2(true);
+            // Put it in a fork to prevent lifetime issues with the local
+            AstFork* const forkp = new AstFork{nodep->fileline(), "", nullptr};
+            forkp->joinType(VJoinType::JOIN_NONE);
+            nodep->replaceWith(forkp);
+            forkp->addStmtsp(nodep);
+        } else {
+            iterateChildren(nodep);
+        }
+    }
     void visit(AstNode* nodep) override {
         if (nodep->isTimingControl()) m_afterTimingControl = true;
         iterateChildren(nodep);

src/V3SchedTiming.cpp

@@ -286,7 +286,8 @@ void transformForks(AstNetlist* const netlistp) {
             funcp->foreach([&](AstNodeVarRef* refp) {
                 AstVar* const varp = refp->varp();
                 AstBasicDType* const dtypep = varp->dtypep()->basicp();
-                bool passByValue = false;
+                // If not a fork..join, copy. All write refs should've been handled by V3Fork
+                bool passByValue = !m_forkp->joinType().join();
                 if (!varp->isFuncLocal()) {
                     if (VString::startsWith(varp->name(), "__Vintra")) {
                         // Pass it by value to the new function, as otherwise there are issues with

src/V3Timing.cpp

@@ -1049,8 +1049,12 @@ class TimingControlVisitor final : public VNVisitor {
         // Special case for NBA
         if (inAssignDly) {
             // Put it in a fork so it doesn't block
-            auto* const forkp = new AstFork{flp, "", nullptr};
-            forkp->joinType(VJoinType::JOIN_NONE);
+            // Could already be the only thing directly under a fork, reuse that if possible
+            AstFork* forkp = !nodep->nextp() ? VN_CAST(nodep->firstAbovep(), Fork) : nullptr;
+            if (!forkp) {
+                forkp = new AstFork{flp, "", nullptr};
+                forkp->joinType(VJoinType::JOIN_NONE);
+            }
             if (!m_underProcedure) {
                 // If it's in a function, it won't be handled by V3Delayed
                 // Put it behind an additional named event that gets triggered in the NBA region

test_regress/t/t_assigndly_dynamic.v

@@ -25,8 +25,26 @@ class nba_waiter;
     endtask
 endclass
 
+class Foo;
+    task bar(logic a, logic b);
+        int x;
+        int y;
+        // bar's local vars and intravals could be overwritten by other locals
+        if (a) x <= `DELAY 'hDEAD;
+        if (b) y <= `DELAY 'hBEEF;
+        #2
+        if (x != 'hDEAD) $stop;
+    endtask
+
+   task qux();
+        int x[] = new[1];
+        x[0] <= `DELAY 'hBEEF;  // Segfault check
+    endtask
+endclass
+
 module t;
     nba_waiter waiter = new;
+    Foo foo = new;
     event e;
     int cnt = 0;
 
@@ -42,6 +60,9 @@ module t;
         waiter.wait_for_nba_region;
         if (cnt != 4) $stop;
         if ($time != `TIME_AFTER_SECOND_WAIT) $stop;
+        foo.bar(1, 1);
+        foo.qux();
+        #2
         $write("*-* All Finished *-*\n");
         $finish;
     end