Fix forks capturing non-input ports in tasks (#5237) (#5343)

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>

src/V3Fork.cpp

@@ -107,7 +107,7 @@ public:
 
         // Move variables into the class
         for (AstVar* varp : m_captureOrder) {
-            if (varp->direction() == VDirection::INPUT) {
+            if (varp->direction().isAny()) {
                 varp = varp->cloneTree(false);
                 varp->direction(VDirection::NONE);
             } else {
@@ -161,16 +161,28 @@ public:
 
         AstNode* initsp = nullptr;  // Arguments need to be copied
         for (AstVar* varp : m_captureOrder) {
-            if (varp->direction() != VDirection::INPUT) continue;
+            if (!varp->direction().isAny()) continue;
 
-            AstMemberSel* const memberselp = new AstMemberSel{
-                varp->fileline(),
-                new AstVarRef{varp->fileline(), m_instance.m_handlep, VAccess::WRITE},
-                VN_AS(memberMap.findMember(m_instance.m_classp, varp->name()), Var)};
-            AstNode* initAsgnp
-                = new AstAssign{varp->fileline(), memberselp,
-                                new AstVarRef{varp->fileline(), varp, VAccess::READ}};
-            initsp = AstNode::addNext(initsp, initAsgnp);
+            if (varp->direction().isNonOutput()) {
+                AstMemberSel* const memberselp = new AstMemberSel{
+                    varp->fileline(),
+                    new AstVarRef{varp->fileline(), m_instance.m_handlep, VAccess::WRITE},
+                    VN_AS(memberMap.findMember(m_instance.m_classp, varp->name()), Var)};
+                AstNode* initAsgnp
+                    = new AstAssign{varp->fileline(), memberselp,
+                                    new AstVarRef{varp->fileline(), varp, VAccess::READ}};
+                initsp = AstNode::addNext(initsp, initAsgnp);
+            }
+            if (varp->direction().isWritable()) {
+                AstMemberSel* const memberselp = new AstMemberSel{
+                    varp->fileline(),
+                    new AstVarRef{varp->fileline(), m_instance.m_handlep, VAccess::READ},
+                    VN_AS(memberMap.findMember(m_instance.m_classp, varp->name()), Var)};
+                AstNode* writebackAsgnp = new AstAssign{
+                    varp->fileline(), new AstVarRef{varp->fileline(), varp, VAccess::WRITE},
+                    memberselp};
+                stmtp = AstNode::addNext(stmtp, writebackAsgnp);
+            }
         }
         if (initsp) AstNode::addNext(asgnp, initsp);
 
@@ -305,7 +317,7 @@ class DynScopeVisitor final : public VNVisitor {
         AstMemberSel* const membersel = new AstMemberSel{
             refp->fileline(), new AstVarRef{refp->fileline(), dynScope.m_handlep, refp->access()},
             refp->varp()};
-        if (refp->varp()->direction() == VDirection::INPUT) {
+        if (refp->varp()->direction().isAny()) {
             membersel->varp(
                 VN_AS(m_memberMap.findMember(dynScope.m_classp, refp->varp()->name()), Var));
         } else {
@@ -403,8 +415,17 @@ class DynScopeVisitor final : public VNVisitor {
     void visit(AstVarRef* nodep) override {
         ForkDynScopeFrame* const framep = frameOf(nodep->varp());
         if (!framep) return;
-
         if (needsDynScope(nodep)) {
+            if (m_afterTimingControl && nodep->varp()->isWritable()
+                && nodep->access().isWriteOrRW()) {
+                // The output variable may not exist after a delay, so we can't just write to it
+                nodep->v3warn(
+                    E_UNSUPPORTED,
+                    "Unsupported: Writing to a captured "
+                        << (nodep->varp()->isInoutish() ? "inout" : "output") << " variable in a "
+                        << (VN_IS(nodep->backp(), AssignDly) ? "non-blocking assignment" : "fork")
+                        << " after a timing control");
+            }
             if (!framep->instance().initialized()) framep->createInstancePrototype();
             framep->captureVarInsert(nodep->varp());
         }
@@ -430,7 +451,7 @@ class DynScopeVisitor final : public VNVisitor {
             forkp->addStmtsp(nodep);
             UINFO(9, "assign new fork " << forkp << endl);
         } else {
-            iterateChildren(nodep);
+            visit(static_cast<AstNodeStmt*>(nodep));
         }
     }
     void visit(AstNode* nodep) override {

test_regress/t/t_fork_dynscope_out.pl

@@ -0,0 +1,22 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2024 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ["--binary -Wno-INITIALDLY"],
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_fork_dynscope_out.v

@@ -0,0 +1,28 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Antmicro.
+// SPDX-License-Identifier: CC0-1.0
+
+module t;
+  bit p = 0, q = 0;
+
+  initial begin
+    t1(p);
+    t2(q);
+    if (p != 1) $stop;
+    if (q != 1) $stop;
+    $write("*-* All Finished *-*\n");
+    $finish;
+  end
+
+  task t1(inout p);
+    fork
+      p = 1;
+    join_none
+  endtask
+
+  task t2(output q);
+    q <= 1;
+  endtask
+endmodule

test_regress/t/t_fork_dynscope_unsup.out

@@ -0,0 +1,10 @@
+%Error-UNSUPPORTED: t/t_fork_dynscope_unsup.v:17:7: Unsupported: Writing to a captured inout variable in a fork after a timing control
+                                                  : ... note: In instance 't'
+   17 |       p = #1 1;
+      |       ^
+                    ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
+%Error-UNSUPPORTED: t/t_fork_dynscope_unsup.v:22:5: Unsupported: Writing to a captured output variable in a non-blocking assignment after a timing control
+                                                  : ... note: In instance 't'
+   22 |     q <= #1 1;
+      |     ^
+%Error: Exiting due to

test_regress/t/t_fork_dynscope_unsup.pl

@@ -0,0 +1,20 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2024 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+lint(
+    verilator_flags2 => ["--timing"],
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_fork_dynscope_unsup.v

@@ -0,0 +1,24 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Antmicro.
+// SPDX-License-Identifier: CC0-1.0
+
+module t;
+  bit p = 0, q = 0;
+
+  initial begin
+    t1(p);
+    t2(q);
+  end
+
+  task t1(inout p);
+    fork
+      p = #1 1;
+    join_none
+  endtask
+
+  task t2(output q);
+    q <= #1 1;
+  endtask
+endmodule