From df5695254c12869b9139e73f8342b8675ea7e3f1 Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Tue, 14 Nov 2023 13:45:03 +0100 Subject: [PATCH] Fix dynamic NBAs with automatic vars Signed-off-by: Krzysztof Bieganski --- src/V3Fork.cpp | 19 ++++++++++---- src/V3SchedTiming.cpp | 3 ++- src/V3Timing.cpp | 8 ++++-- test_regress/t/t_assigndly_dynamic.v | 21 +++++++++++++++ test_regress/t/t_fork_repeat.pl | 18 +++++++++++++ test_regress/t/t_fork_repeat.v | 15 +++++++++++ test_regress/t/t_intraval_auto.pl | 23 +++++++++++++++++ test_regress/t/t_intraval_auto.v | 30 ++++++++++++++++++++++ test_regress/t/t_static_nonstatic_error.pl | 21 +++++++++++++++ test_regress/t/t_static_nonstatic_error.v | 18 +++++++++++++ 10 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 test_regress/t/t_fork_repeat.pl create mode 100644 test_regress/t/t_fork_repeat.v create mode 100755 test_regress/t/t_intraval_auto.pl create mode 100644 test_regress/t/t_intraval_auto.v create mode 100755 test_regress/t/t_static_nonstatic_error.pl create mode 100644 test_regress/t/t_static_nonstatic_error.v diff --git a/src/V3Fork.cpp b/src/V3Fork.cpp index 133e4352ff4..6d08aa153e1 100644 --- a/src/V3Fork.cpp +++ b/src/V3Fork.cpp @@ -313,11 +313,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) { @@ -415,6 +412,18 @@ class DynScopeVisitor final : public VNVisitor { } visit(static_cast(nodep)); } + void visit(AstAssignDly* nodep) override { + if (m_procp && !VN_IS(nodep->backp(), Fork) && nodep->lhsp()->exists([](AstVarRef* refp) { + return refp->varp()->isFuncLocal(); + })) { + auto* 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); diff --git a/src/V3SchedTiming.cpp b/src/V3SchedTiming.cpp index 368201f0552..75b5a67d778 100644 --- a/src/V3SchedTiming.cpp +++ b/src/V3SchedTiming.cpp @@ -288,7 +288,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 diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 6484a724752..4ee294711de 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -1029,8 +1029,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 diff --git a/test_regress/t/t_assigndly_dynamic.v b/test_regress/t/t_assigndly_dynamic.v index 9183eb767ff..7a403fa9bd9 100644 --- a/test_regress/t/t_assigndly_dynamic.v +++ b/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 diff --git a/test_regress/t/t_fork_repeat.pl b/test_regress/t/t_fork_repeat.pl new file mode 100644 index 00000000000..e23c975f957 --- /dev/null +++ b/test_regress/t/t_fork_repeat.pl @@ -0,0 +1,18 @@ +#!/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 --timing"], + ); + +ok(1); +1; diff --git a/test_regress/t/t_fork_repeat.v b/test_regress/t/t_fork_repeat.v new file mode 100644 index 00000000000..589ced97a41 --- /dev/null +++ b/test_regress/t/t_fork_repeat.v @@ -0,0 +1,15 @@ +module repeat_lifetime_bug; +initial begin + fork + begin + repeat(5) begin + $display("iteration"); + end + //for(int i=0; i<5; i++) begin + // $display("iteration"); + //end + end + join_any + $finish; +end +endmodule diff --git a/test_regress/t/t_intraval_auto.pl b/test_regress/t/t_intraval_auto.pl new file mode 100755 index 00000000000..bc632ee012e --- /dev/null +++ b/test_regress/t/t_intraval_auto.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 2019 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_intraval_auto.v b/test_regress/t/t_intraval_auto.v new file mode 100644 index 00000000000..04d9f4f1ccc --- /dev/null +++ b/test_regress/t/t_intraval_auto.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Foo; + task bar(logic b); + int x; + if (b) x <= 'hDEAD; + #1 + if (x != 'hDEAD) $stop; + endtask + + task qux(); + int x[] = new[1]; + x[0] <= 'hBEEF; // Segfault check + endtask +endclass + +module t; + Foo foo = new; + + initial begin + foo.bar(1); + foo.qux(); + #2 $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_static_nonstatic_error.pl b/test_regress/t/t_static_nonstatic_error.pl new file mode 100755 index 00000000000..b46d46042df --- /dev/null +++ b/test_regress/t/t_static_nonstatic_error.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_static_nonstatic_error.v b/test_regress/t/t_static_nonstatic_error.v new file mode 100644 index 00000000000..8a2c8b9bfdb --- /dev/null +++ b/test_regress/t/t_static_nonstatic_error.v @@ -0,0 +1,18 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2003 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +package my_pkg; + task tsk; + endtask +endpackage + +module t; + initial begin + my_pkg::tsk(); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule