From 77502aeb9733e58216cd66db88ff498f53cee54b Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Tue, 30 May 2023 15:00:10 +0200 Subject: [PATCH] Add warning that timing controls in DPI exports are unsupported (#4238) Signed-off-by: Krzysztof Bieganski --- src/V3Timing.cpp | 30 ++++++++++++------- test_regress/t/t_timing_dpi.pl | 28 ----------------- ..._timing_dpi.cpp => t_timing_dpi_unsup.cpp} | 0 test_regress/t/t_timing_dpi_unsup.out | 5 ++++ test_regress/t/t_timing_dpi_unsup.pl | 22 ++++++++++++++ .../{t_timing_dpi.v => t_timing_dpi_unsup.v} | 10 +++---- 6 files changed, 51 insertions(+), 44 deletions(-) delete mode 100755 test_regress/t/t_timing_dpi.pl rename test_regress/t/{t_timing_dpi.cpp => t_timing_dpi_unsup.cpp} (100%) create mode 100644 test_regress/t/t_timing_dpi_unsup.out create mode 100755 test_regress/t/t_timing_dpi_unsup.pl rename test_regress/t/{t_timing_dpi.v => t_timing_dpi_unsup.v} (79%) diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 8f4cf323c..6772f95f0 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -568,17 +568,25 @@ private: } void visit(AstCFunc* nodep) override { iterateChildren(nodep); - if (nodep->user2()) { - nodep->rtnType("VlCoroutine"); - // If in a class, create a shared pointer to 'this' - if (m_classp) nodep->addInitsp(new AstCStmt{nodep->fileline(), "VL_KEEP_THIS;\n"}); - if (!nodep->exists([](AstCAwait*) { return true; })) { - // It's a coroutine but has no awaits (a class method that overrides/is - // overridden by a suspendable, but doesn't have any awaits itself). Add a - // co_return at the end (either that or a co_await is required in a - // coroutine) - nodep->addStmtsp(new AstCStmt{nodep->fileline(), "co_return;\n"}); - } + if (!nodep->user2()) return; + nodep->rtnType("VlCoroutine"); + // If in a class, create a shared pointer to 'this' + if (m_classp) nodep->addInitsp(new AstCStmt{nodep->fileline(), "VL_KEEP_THIS;\n"}); + AstNode* firstCoStmtp = nullptr; // First co_* statement in the function + nodep->exists([&](AstCAwait* const awaitp) -> bool { return (firstCoStmtp = awaitp); }); + if (!firstCoStmtp) { + // It's a coroutine but has no awaits (a class method that overrides/is + // overridden by a suspendable, but doesn't have any awaits itself). Add a + // co_return at the end (either that or a co_await is required in a + // coroutine) + firstCoStmtp = new AstCStmt{nodep->fileline(), "co_return;\n"}; + nodep->addStmtsp(firstCoStmtp); + } + if (nodep->dpiExportImpl()) { + // A DPI-exported coroutine won't be able to block the calling code + // Error on the await node; fall back to the function node + firstCoStmtp->v3warn(E_UNSUPPORTED, + "Unsupported: Timing controls inside DPI-exported tasks"); } } void visit(AstNodeCCall* nodep) override { diff --git a/test_regress/t/t_timing_dpi.pl b/test_regress/t/t_timing_dpi.pl deleted file mode 100755 index 2c494cced..000000000 --- a/test_regress/t/t_timing_dpi.pl +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env perl -if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } -# DESCRIPTION: Verilator: Verilog Test driver/expect definition -# -# This file ONLY is placed under the Creative Commons Public Domain, for -# any use, without warranty, 2023 by Toru Niina. -# SPDX-License-Identifier: CC0-1.0 - - -scenarios(vlt => 1); - -if (!$Self->have_coroutines) { - skip("No coroutine support") -} -else { - compile( - v_flags2 => ["t/t_timing_dpi.cpp"], - verilator_flags2 => ["--exe --main --timing"], - make_main => 0, - ); - - execute( - check_finished => 1, - ); -} - -ok(1); -1 diff --git a/test_regress/t/t_timing_dpi.cpp b/test_regress/t/t_timing_dpi_unsup.cpp similarity index 100% rename from test_regress/t/t_timing_dpi.cpp rename to test_regress/t/t_timing_dpi_unsup.cpp diff --git a/test_regress/t/t_timing_dpi_unsup.out b/test_regress/t/t_timing_dpi_unsup.out new file mode 100644 index 000000000..c4cc70ed0 --- /dev/null +++ b/test_regress/t/t_timing_dpi_unsup.out @@ -0,0 +1,5 @@ +%Error-UNSUPPORTED: t/t_timing_dpi_unsup.v:28:19: Unsupported: Timing controls inside DPI-exported tasks + 28 | repeat(n) @(negedge clk); + | ^ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_timing_dpi_unsup.pl b/test_regress/t/t_timing_dpi_unsup.pl new file mode 100755 index 000000000..a6399f3f7 --- /dev/null +++ b/test_regress/t/t_timing_dpi_unsup.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 2023 by Antmicro Ltd. 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(linter => 1); + +top_filename("t/t_timing_dpi_unsup.v"); + +lint( + verilator_flags2 => ["--timing"], + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_timing_dpi.v b/test_regress/t/t_timing_dpi_unsup.v similarity index 79% rename from test_regress/t/t_timing_dpi.v rename to test_regress/t/t_timing_dpi_unsup.v index 63d3bd43d..996ef1e39 100644 --- a/test_regress/t/t_timing_dpi.v +++ b/test_regress/t/t_timing_dpi_unsup.v @@ -24,19 +24,19 @@ module t; export "DPI-C" task tb_sv_wait; task automatic tb_sv_wait(input int n); - `WRITE_VERBOSE("tb_sv_wait start..."); + `WRITE_VERBOSE("tb_sv_wait start...\n"); repeat(n) @(negedge clk); - `WRITE_VERBOSE("tb_sv_wait done!"); + `WRITE_VERBOSE("tb_sv_wait done!\n"); endtask always #halfcycle clk = ~clk; initial begin - `WRITE_VERBOSE("test start"); + `WRITE_VERBOSE("test start\n"); repeat(10) @(posedge clk); - `WRITE_VERBOSE("calling tb_c_wait..."); + `WRITE_VERBOSE("calling tb_c_wait...\n"); tb_c_wait(); - `WRITE_VERBOSE("tb_c_wait finish"); + `WRITE_VERBOSE("tb_c_wait finish\n"); repeat(10) @(posedge clk); $write("*-* All Finished *-*\n"); $finish;