From 2dba76a7c26eabccb31667f87c84b3fec56ba9fd Mon Sep 17 00:00:00 2001
From: Ryszard Rozak <rrozak@antmicro.com>
Date: Tue, 14 Nov 2023 23:02:13 +0100
Subject: [PATCH] Fix access type of function arguments (#4692) (#4694)

---
 src/V3Task.cpp                      | 14 +++-----------
 src/V3Width.cpp                     |  2 ++
 test_regress/t/t_fork_output_arg.pl | 23 +++++++++++++++++++++++
 test_regress/t/t_fork_output_arg.v  | 29 +++++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 11 deletions(-)
 create mode 100755 test_regress/t/t_fork_output_arg.pl
 create mode 100644 test_regress/t/t_fork_output_arg.v

diff --git a/src/V3Task.cpp b/src/V3Task.cpp
index 6e5dcb5246..c3cf4ed09e 100644
--- a/src/V3Task.cpp
+++ b/src/V3Task.cpp
@@ -30,7 +30,6 @@
 #include "V3Const.h"
 #include "V3EmitCBase.h"
 #include "V3Graph.h"
-#include "V3LinkLValue.h"
 
 #include <tuple>
 
@@ -503,7 +502,6 @@ class TaskVisitor final : public VNVisitor {
                 }
             } else if (portp->isInoutish()) {
                 // if (debug() >= 9) pinp->dumpTree("-pinrsize- ");
-                V3LinkLValue::linkLValueSet(pinp);
 
                 AstVarScope* const newvscp
                     = createVarScope(portp, namePrefix + "__" + portp->shortName());
@@ -524,12 +522,6 @@ class TaskVisitor final : public VNVisitor {
                 beginp->addNext(postassp);
                 // if (debug() >= 9) beginp->dumpTreeAndNext(cout, "-pinrsize-out- ");
             } else if (portp->isWritable()) {
-                // Make output variables
-                // Correct lvalue; we didn't know when we linked
-                // This is slightly scary; are we sure no decisions were made
-                // before here based on this not being a lvalue?
-                // Doesn't seem so; V3Unknown uses it earlier, but works ok.
-                V3LinkLValue::linkLValueSet(pinp);
                 // Even if it's referencing a varref, we still make a temporary
                 // Else task(x,x,x) might produce incorrect results
                 AstVarScope* const newvscp
@@ -1859,9 +1851,9 @@ AstNodeFTask* V3Task::taskConnectWrapNew(AstNodeFTask* taskp, const string& newn
             newTaskp->addStmtsp(newAssignp);
         }
         oldNewVars.emplace(portp, newPortp);
-        AstArg* const newArgp
-            = new AstArg{portp->fileline(), portp->name(),
-                         new AstVarRef{portp->fileline(), newPortp, VAccess::READ}};
+        const VAccess pinAccess = portp->isWritable() ? VAccess::WRITE : VAccess::READ;
+        AstArg* const newArgp = new AstArg{portp->fileline(), portp->name(),
+                                           new AstVarRef{portp->fileline(), newPortp, pinAccess}};
         newCallp->addPinsp(newArgp);
     }
     // Create wrapper call to original, passing arguments, adding setting of return value
diff --git a/src/V3Width.cpp b/src/V3Width.cpp
index 8291e0fe1e..b5186cc4d6 100644
--- a/src/V3Width.cpp
+++ b/src/V3Width.cpp
@@ -70,6 +70,7 @@
 #include "V3Const.h"
 #include "V3Error.h"
 #include "V3Global.h"
+#include "V3LinkLValue.h"
 #include "V3MemberMap.h"
 #include "V3Number.h"
 #include "V3Randomize.h"
@@ -5687,6 +5688,7 @@ class WidthVisitor final : public VNVisitor {
                     AstNodeExpr* const newp = new AstResizeLValue{pinp->fileline(), pinp};
                     relinkHandle.relink(newp);
                 }
+                if (portp->isWritable()) V3LinkLValue::linkLValueSet(pinp);
                 if (!portp->basicp() || portp->basicp()->isOpaque()) {
                     checkClassAssign(nodep, "Function Argument", pinp, portDTypep);
                     userIterate(pinp, WidthVP{portDTypep, FINAL}.p());
diff --git a/test_regress/t/t_fork_output_arg.pl b/test_regress/t/t_fork_output_arg.pl
new file mode 100755
index 0000000000..b8493bd062
--- /dev/null
+++ b/test_regress/t/t_fork_output_arg.pl
@@ -0,0 +1,23 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 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 => ["--exe --main --timing"],
+    make_main => 0,
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_fork_output_arg.v b/test_regress/t/t_fork_output_arg.v
new file mode 100644
index 0000000000..e5c5169e5c
--- /dev/null
+++ b/test_regress/t/t_fork_output_arg.v
@@ -0,0 +1,29 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2023 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+class Cls;
+   int x = 100;
+   task get_x(output int arg);
+      arg = x;
+   endtask
+endclass
+
+task automatic test;
+   int o;
+   Cls c = new;
+   fork
+      c.get_x(o);
+   join_any
+   if (o != 100) $stop;
+endtask
+
+module t();
+   initial begin
+      test();
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule