From 63b2dbb827e9fc98e32ade8ed83328bebaec2f9f Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 20 Jun 2023 06:26:46 -0400 Subject: [PATCH] Fix error when multiple duplicate DPI exports (#4301). --- Changes | 1 + src/V3Task.cpp | 15 ++++++++++---- test_regress/t/t_dpi_2exparg_bad.out | 18 +++++++++++++++++ test_regress/t/t_dpi_2exparg_bad.pl | 19 ++++++++++++++++++ test_regress/t/t_dpi_2exparg_bad.v | 30 ++++++++++++++++++++++++++++ test_regress/t/t_dpi_dup_bad.out | 2 +- 6 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 test_regress/t/t_dpi_2exparg_bad.out create mode 100755 test_regress/t/t_dpi_2exparg_bad.pl create mode 100644 test_regress/t/t_dpi_2exparg_bad.v diff --git a/Changes b/Changes index 1e50c5310..b38c38206 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,7 @@ Verilator 5.013 devel **Minor:** * Deprecation planned for 32-bit pointer -m32 mode (#4268). +* Fix error when multiple duplicate DPI exports (#4301). Verilator 5.012 2023-06-13 diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 5f1e195ce..5fbd76ce3 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -657,8 +657,15 @@ private: args += ", "; dpiproto += ", "; } + // Include both the Verilator and C type names, as if either + // differ we may get C compilation problems later + const std::string dpiType = portp->dpiArgType(false, false); + dpiproto += dpiType; + const std::string vType = portp->dtypep()->prettyDTypeName(); + if (!portp->isDpiOpenArray() && dpiType != vType) { + dpiproto += " /* " + vType + " */ "; + } args += portp->name(); // Leftover so ,'s look nice - if (nodep->dpiImport()) dpiproto += portp->dpiArgType(false, false); } } } @@ -929,15 +936,15 @@ private: m_dpiNames.emplace(nodep->cname(), std::make_tuple(nodep, signature, funcp)); return funcp; } else { - // Seen this cname before. Check if it's the same prototype. + // Seen this cname import before. Check if it's the same prototype. const AstNodeFTask* firstNodep; string firstSignature; AstCFunc* firstFuncp; std::tie(firstNodep, firstSignature, firstFuncp) = it->second; if (signature != firstSignature) { // Different signature, so error. - nodep->v3error("Duplicate declaration of DPI function with different signature: " - << nodep->prettyNameQ() << '\n' + nodep->v3error("Duplicate declaration of DPI function with different signature: '" + << nodep->cname() << "'\n" << nodep->warnContextPrimary() << '\n' << nodep->warnMore() // << "... New signature: " << signature << '\n' // diff --git a/test_regress/t/t_dpi_2exparg_bad.out b/test_regress/t/t_dpi_2exparg_bad.out new file mode 100644 index 000000000..47496c569 --- /dev/null +++ b/test_regress/t/t_dpi_2exparg_bad.out @@ -0,0 +1,18 @@ +%Warning-WIDTHEXPAND: t/t_dpi_2exparg_bad.v:19:56: Operator NOT expects 64 bits on the LHS, but LHS's VARREF 'i' generates 32 bits. + : ... In instance t.b + 19 | task dpix_twice(input int i, output [63:0] o); o = ~i; endtask + | ^ + ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=latest + ... Use "/* verilator lint_off WIDTHEXPAND */" and lint_on around source to disable this message. +%Warning-WIDTHTRUNC: t/t_dpi_2exparg_bad.v:12:53: Operator ASSIGN expects 3 bits on the Assign RHS, but Assign RHS's NOT generates 32 bits. + : ... In instance t.a + 12 | task dpix_twice(input int i, output [2:0] o); o = ~i; endtask + | ^ +%Error: t/t_dpi_2exparg_bad.v:19:9: Duplicate declaration of DPI function with different signature: 'dpix_twice' + 19 | task dpix_twice(input int i, output [63:0] o); o = ~i; endtask + | ^~~~~~~~~~ + : ... New signature: void dpix_twice (int, svLogicVecVal* /* logic[63:0] */ ) + t/t_dpi_2exparg_bad.v:12:9: ... Original signature: void dpix_twice (int, svLogicVecVal* /* logic[2:0] */ ) + 12 | task dpix_twice(input int i, output [2:0] o); o = ~i; endtask + | ^~~~~~~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_dpi_2exparg_bad.pl b/test_regress/t/t_dpi_2exparg_bad.pl new file mode 100755 index 000000000..59ba0d6c6 --- /dev/null +++ b/test_regress/t/t_dpi_2exparg_bad.pl @@ -0,0 +1,19 @@ +#!/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(linter => 1); + +lint( + fails => $Self->{vlt_all}, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_dpi_2exparg_bad.v b/test_regress/t/t_dpi_2exparg_bad.v new file mode 100644 index 000000000..cbc2f44b3 --- /dev/null +++ b/test_regress/t/t_dpi_2exparg_bad.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// 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 + +module a; + import "DPI-C" task dpii_twice; // Legal + export "DPI-C" task dpix_twice; // Bad + task dpix_twice(input int i, output [2:0] o); o = ~i; endtask + initial dpii_twice(); +endmodule + +module b; + import "DPI-C" task dpii_twice; // Legal + export "DPI-C" task dpix_twice; // Bad + task dpix_twice(input int i, output [63:0] o); o = ~i; endtask + initial dpii_twice(); +endmodule + +module t; + a a(); + b b(); + + initial begin + $stop; + end +endmodule diff --git a/test_regress/t/t_dpi_dup_bad.out b/test_regress/t/t_dpi_dup_bad.out index 0450a8504..9015fb25d 100644 --- a/test_regress/t/t_dpi_dup_bad.out +++ b/test_regress/t/t_dpi_dup_bad.out @@ -1,4 +1,4 @@ -%Error: t/t_dpi_dup_bad.v:13:51: Duplicate declaration of DPI function with different signature: 't.oth_f_int2' +%Error: t/t_dpi_dup_bad.v:13:51: Duplicate declaration of DPI function with different signature: 'dpii_fa_bit' 13 | import "DPI-C" pure dpii_fa_bit = function int oth_f_int2(input int i, input int bad); | ^~~~~~~~~~ : ... New signature: pure int dpii_fa_bit (int, int)