Internals: Remove `VlNow` (#4089)

`VlNow{}` is completely unnecessary, as coroutines are always on the
heap (unless optimized out). Also fix access of var ref passed to forked processes.

docs/internals.rst

@@ -628,16 +628,12 @@ coroutines ``co_await`` its ``join`` function, and forked ones call ``done``
 when they're finished. Once the required number of coroutines (set using
 ``setCounter``) finish execution, the forking coroutine is resumed.
 
-Awaitable Utilities
+``VlForever``
-~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~
 
-There are also two small utility awaitable types:
+A small utility awaitable type. It allows for blocking a coroutine forever. It
-
+is currently only used for ``wait`` statements that await a constant false
-* ``VlNow`` is an awaitable that suspends and immediately resumes coroutines.
+condition. See the `Timing Pass` section for more details.
-  It is used for forcing a coroutine to be moved onto the heap. See the `Forks`
-  section for more detail.
-* ``VlForever`` is used for blocking a coroutine forever. See the `Timing pass`
-  section for more detail.
 
 Timing Pass
 ~~~~~~~~~~~
@@ -749,9 +745,7 @@ doesn't suspend the forking process.
 
 In forked processes, references to local variables are only allowed in
 ``fork..join``, as this is the only case that ensures the lifetime of these
-locals are at least as long as the execution of the forked processes. This is
+locals are at least as long as the execution of the forked processes.
-where ``VlNow`` is used, to ensure the locals are moved to the heap before they
-are passed by reference to the forked processes.
 
 
 Multithreaded Mode

include/verilated_timing.h

@@ -312,16 +312,6 @@ public:
     }
 };
 
-//=============================================================================
-// VlNow is a helper awaitable type that always suspends, and then immediately resumes a coroutine.
-// Allows forcing the move of coroutine locals to the heap.
-
-struct VlNow {
-    bool await_ready() const { return false; }  // Always suspend
-    bool await_suspend(std::coroutine_handle<>) const { return false; }  // Resume immediately
-    void await_resume() const {}
-};
-
 //=============================================================================
 // VlForever is a helper awaitable type for suspending coroutines forever. Used for constant
 // wait statements.

src/V3SchedTiming.cpp

@@ -272,22 +272,20 @@ void transformForks(AstNetlist* const netlistp) {
                 // If it a fork sync or an intra-assignment variable, pass it by value
                 const bool passByValue = (dtypep && dtypep->isForkSync())
                                          || VString::startsWith(varp->name(), "__Vintra");
-                // Only handle vars passed by value or locals declared before the fork
-                if (!passByValue && (!varp->user1() || !varp->isFuncLocal())) return;
                 if (passByValue) {
                     // We can just pass it to the new function
+                } else if (!varp->user1() || !varp->isFuncLocal()) {
+                    // Not func local, or not declared before the fork. Their lifetime is longer
+                    // than the forked process. Skip
+                    return;
                 } else if (m_forkp->joinType().join()) {
                     // If it's fork..join, we can refer to variables from the parent process
-                    if (!m_funcp->user1SetOnce()) {  // Only do this once per function
-                        // Move all locals to the heap before the fork
-                        AstCExpr* const nowp
-                            = new AstCExpr{m_forkp->fileline(), "VlNow{}", 0, true};
-                        nowp->dtypeSetVoid();  // TODO: this is sloppy but harmless
-                        AstCAwait* const awaitp = new AstCAwait{m_forkp->fileline(), nowp};
-                        awaitp->dtypeSetVoid();
-                        m_forkp->addHereThisAsNext(awaitp->makeStmt());
-                    }
                 } else {
+                    // TODO: It is possible to relax this by allowing the use of such variables up
+                    // until the first await. Also, variables defined within a forked process
+                    // (inside a begin) are extracted out by V3Begin, so they also trigger this
+                    // error. Preventing this (or detecting such cases and moving the vars back)
+                    // would also allow for using them freely.
                     refp->v3warn(E_UNSUPPORTED, "Unsupported: variable local to a forking process "
                                                 "accessed in a fork..join_any or fork..join_none");
                     return;
@@ -305,7 +303,8 @@ void transformForks(AstNetlist* const netlistp) {
                         = new AstVarScope{newvarp->fileline(), funcp->scopep(), newvarp};
                     funcp->scopep()->addVarsp(newvscp);
                     vscp->user2p(newvscp);
-                    callp->addArgsp(new AstVarRef{refp->fileline(), vscp, VAccess::READ});
+                    callp->addArgsp(new AstVarRef{
+                        refp->fileline(), vscp, passByValue ? VAccess::READ : VAccess::READWRITE});
                 }
                 AstVarScope* const newvscp = VN_AS(vscp->user2p(), VarScope);
                 refp->varScopep(newvscp);