From 7fd68b58ca979a9582c09b6d935b0f54e2650a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Chmiel?= Date: Wed, 23 Sep 2026 20:54:55 +0200 Subject: [PATCH] Fix VPI discoverability of forceable unpacked struct members (#8445) --- docs/CONTRIBUTORS | 1 + src/V3AstNodeOther.h | 2 +- src/V3AstNodes.cpp | 8 +- src/V3EmitCSyms.cpp | 26 ++-- src/V3Force.cpp | 2 +- .../t/t_forceable_unpacked_struct_unsup.out | 5 + .../t/t_forceable_unpacked_struct_unsup.py | 16 +++ .../t/t_forceable_unpacked_struct_unsup.v | 14 ++ .../t/t_vpi_forceable_unpacked_struct.cpp | 123 ++++++++++++++++++ .../t/t_vpi_forceable_unpacked_struct.py | 19 +++ .../t/t_vpi_forceable_unpacked_struct.v | 22 ++++ 11 files changed, 219 insertions(+), 19 deletions(-) create mode 100644 test_regress/t/t_forceable_unpacked_struct_unsup.out create mode 100755 test_regress/t/t_forceable_unpacked_struct_unsup.py create mode 100644 test_regress/t/t_forceable_unpacked_struct_unsup.v create mode 100644 test_regress/t/t_vpi_forceable_unpacked_struct.cpp create mode 100755 test_regress/t/t_vpi_forceable_unpacked_struct.py create mode 100644 test_regress/t/t_vpi_forceable_unpacked_struct.v diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 5fbe6dfbc..b10d19e5c 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -311,6 +311,7 @@ Steven Hugg Stuart Morris sumpster Szymon Gizler +Szymon Jędras Sören Tempel Sumanth Kadiyala Sunimali Rathnayake diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index c37796284..7746b7b15 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2485,7 +2485,7 @@ public: string vlArgType(bool named, bool forReturn, bool forFunc, const string& namespc = "", bool asRef = false, bool constRef = false) const; string vlEnumType() const; // Return VerilatorVarType: VLVT_UINT32, etc - string vlEnumDir() const; // Return VerilatorVarDir: VLVD_INOUT, etc + string vlEnumDir(bool forMember = false) const; // Return VerilatorVarDir: VLVD_INOUT, etc string vlPropDecl(const string& propName) const; // Return VerilatorVarProps declaration void combineType(VVarType type); AstNodeDType* getChildDTypep() const override { return childDTypep(); } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index da6c698b1..dda90078b 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -4102,7 +4102,7 @@ string AstVar::vlArgType(bool named, bool forReturn, bool forFunc, const string& } return ostatic + dtypep()->cType(oname, forFunc, asRef); } -string AstVar::vlEnumDir() const { +string AstVar::vlEnumDir(bool forMember) const { string out; if (isInout()) { out = "VLVD_INOUT"; @@ -4119,17 +4119,17 @@ string AstVar::vlEnumDir() const { } else if (isSigUserRdPublic()) { out += "|VLVF_PUB_RD"; } - if (isForceable()) out += "|VLVF_FORCEABLE"; + if (isForceable() && !forMember) out += "|VLVF_FORCEABLE"; if (isContinuously()) out += "|VLVF_CONTINUOUSLY"; // if (const AstBasicDType* const bdtypep = basicp()) { if (bdtypep->keyword().isDpiCLayout()) out += "|VLVF_DPI_CLAY"; } // - if (dtypep()->skipRefp()->isSigned()) out += "|VLVF_SIGNED"; + if (dtypep()->skipRefp()->isSigned() && !forMember) out += "|VLVF_SIGNED"; // if (AstBasicDType* const basicp = dtypep()->skipRefp()->basicp()) { - if (basicp->keyword() == VBasicDTypeKwd::BIT) out += "|VLVF_BITVAR"; + if (basicp->keyword() == VBasicDTypeKwd::BIT && !forMember) out += "|VLVF_BITVAR"; } if (isNet()) out += "|VLVF_NET"; return out; diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index dff8d6232..68a5d3888 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -309,13 +309,13 @@ class EmitCSyms final : EmitCBaseVisitorConst { } static std::string memberVlEnumDir(const AstVar* const varp, const AstNodeDType* const dtypep) { - std::string out = "((" + varp->vlEnumDir() + ") & ~(VLVF_SIGNED|VLVF_BITVAR))"; + std::string out = '(' + varp->vlEnumDir(/*forMember=*/true); const AstNodeDType* const skipDTypep = dtypep->skipRefp(); if (skipDTypep->isSigned()) out += "|VLVF_SIGNED"; if (const AstBasicDType* const basicp = skipDTypep->basicp()) { if (basicp->keyword() == VBasicDTypeKwd::BIT) out += "|VLVF_BITVAR"; } - return out; + return out + ')'; } static std::string insertVarStatement(const ScopeVarData& svd, const AstScope* const scopep, @@ -1408,21 +1408,21 @@ std::vector EmitCSyms::getSymCtorStmts() { const std::string bounds = boundsString(dims); residual.emplace_back( insertVarStatement(svd, scopep, varp, dims.udim, dims.pdim, bounds) + ";"); - if (const AstNodeUOrStructDType* const sdtypep - = VN_CAST(varp->dtypeSkipRefp(), NodeUOrStructDType)) { - if (!sdtypep->packed()) { - addUOrStructMemberVars(residual, svd, scopep, svd.m_varBasePretty, - protect(varp->name()), sdtypep); - } - } else if (VN_IS(varp->dtypeSkipRefp(), UnpackArrayDType)) { - addUnpackedArrayUOrStructMemberVars(residual, svd, scopep, - svd.m_varBasePretty, - protect(varp->name()), varp->dtypep()); - } break; } default: v3fatalSrc("Bad case"); } + if (kind == TableEntryKind::TABLE_ROW) continue; + if (const AstNodeUOrStructDType* const sdtypep + = VN_CAST(varp->dtypeSkipRefp(), NodeUOrStructDType)) { + if (!sdtypep->packed()) { + addUOrStructMemberVars(residual, svd, scopep, svd.m_varBasePretty, + protect(varp->name()), sdtypep); + } + } else if (VN_IS(varp->dtypeSkipRefp(), UnpackArrayDType)) { + addUnpackedArrayUOrStructMemberVars(residual, svd, scopep, svd.m_varBasePretty, + protect(varp->name()), varp->dtypep()); + } } if (!rows.empty()) { diff --git a/src/V3Force.cpp b/src/V3Force.cpp index 1d8610aa3..034109773 100644 --- a/src/V3Force.cpp +++ b/src/V3Force.cpp @@ -911,7 +911,7 @@ class ForceDiscoveryVisitor final : public VNVisitorConst { "buildForceableUnpackedArray called with non-unpacked dtype"); const AstNodeDType* const leafDtypep = dims.back()->subDTypep()->skipRefp(); const AstBasicDType* const innerBasicp = leafDtypep->basicp(); - if (!ForceState::isBitwiseDType(innerBasicp)) { + if (!innerBasicp || !ForceState::isBitwiseDType(innerBasicp)) { varp->v3warn(E_UNSUPPORTED, "Unsupported: Forcing unpacked arrays of non-bitwise inner type: " << varp->name()); // (#4735) diff --git a/test_regress/t/t_forceable_unpacked_struct_unsup.out b/test_regress/t/t_forceable_unpacked_struct_unsup.out new file mode 100644 index 000000000..ffd196b53 --- /dev/null +++ b/test_regress/t/t_forceable_unpacked_struct_unsup.out @@ -0,0 +1,5 @@ +%Error-UNSUPPORTED: t/t_forceable_unpacked_struct_unsup.v:13:14: Unsupported: Forcing unpacked arrays of non-bitwise inner type: array_struct_top__DOT__response + 13 | response_t response[2] /*verilator forceable*/; + | ^~~~~~~~ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_forceable_unpacked_struct_unsup.py b/test_regress/t/t_forceable_unpacked_struct_unsup.py new file mode 100755 index 000000000..18ef27714 --- /dev/null +++ b/test_regress/t/t_forceable_unpacked_struct_unsup.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint(fails=test.vlt_all, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_forceable_unpacked_struct_unsup.v b/test_regress/t/t_forceable_unpacked_struct_unsup.v new file mode 100644 index 000000000..6eb11c699 --- /dev/null +++ b/test_regress/t/t_forceable_unpacked_struct_unsup.v @@ -0,0 +1,14 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +typedef struct { + logic [31:0] a; + logic [31:0] b; +} response_t; + +module array_struct_top; + response_t response[2] /*verilator forceable*/; +endmodule diff --git a/test_regress/t/t_vpi_forceable_unpacked_struct.cpp b/test_regress/t/t_vpi_forceable_unpacked_struct.cpp new file mode 100644 index 000000000..a643f8e1e --- /dev/null +++ b/test_regress/t/t_vpi_forceable_unpacked_struct.cpp @@ -0,0 +1,123 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// +// 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-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#include "verilated.h" + +#include "TestCheck.h" +#include "TestSimulator.h" +#include "TestVpi.h" +#include "sv_vpi_user.h" +#include "vpi_user.h" + +#include +#include + +namespace { + +bool putValue(vpiHandle handle, PLI_INT32 val, PLI_INT32 flags = vpiNoDelay) { + s_vpi_value value{}; + value.format = vpiIntVal; + value.value.integer = val; + return vpi_put_value(handle, &value, nullptr, flags); +} + +PLI_INT32 getValue(vpiHandle handle) { + s_vpi_value value{}; + value.format = vpiIntVal; + vpi_get_value(handle, &value); + return value.value.integer; +} + +int errors = 0; + +bool mon_check() { + + TestVpiHandle forceable_response + = vpi_handle_by_name(const_cast("t.forceable_response"), nullptr); + TEST_CHECK_NZ(forceable_response); + if (errors) return true; + + std::unordered_set discoverable_by_iterate; + if (TestVpiHandle members = vpi_iterate(vpiMember, forceable_response)) { + while (TestVpiHandle member = vpi_scan(members)) { + discoverable_by_iterate.insert(vpi_get_str(vpiFullName, member)); + } + members.freed(); + } + const std::unordered_set expected_members + = {"t.forceable_response.a", "t.forceable_response.b", "t.forceable_response.nested"}; + TEST_CHECK(discoverable_by_iterate.size(), expected_members.size(), + discoverable_by_iterate == expected_members); + + TestVpiHandle a + = vpi_handle_by_name(const_cast("t.forceable_response.a"), nullptr); + TestVpiHandle b + = vpi_handle_by_name(const_cast("t.forceable_response.b"), nullptr); + TestVpiHandle nested + = vpi_handle_by_name(const_cast("t.forceable_response.nested"), nullptr); + TestVpiHandle c + = vpi_handle_by_name(const_cast("t.forceable_response.nested.c"), nullptr); + TEST_CHECK_NZ(a); + TEST_CHECK_NZ(b); + TEST_CHECK_NZ(nested); + TEST_CHECK_NZ(c); + if (errors) return true; + + std::unordered_set nested_members; + if (TestVpiHandle members = vpi_iterate(vpiMember, nested)) { + while (TestVpiHandle member = vpi_scan(members)) { + nested_members.insert(vpi_get_str(vpiFullName, member)); + } + members.freed(); + } + const std::unordered_set expected_nested_members + = {"t.forceable_response.nested.c"}; + TEST_CHECK(nested_members.size(), expected_nested_members.size(), + nested_members == expected_nested_members); + + TEST_CHECK_EQ(vpi_get(vpiSize, a), 32); + TEST_CHECK_EQ(vpi_get(vpiSize, b), 16); + TEST_CHECK_EQ(vpi_get(vpiSize, c), 8); + + putValue(a, 11); + putValue(b, 22); + putValue(c, 33); + TEST_CHECK_EQ(getValue(a), 11); + TEST_CHECK_EQ(getValue(b), 22); + TEST_CHECK_EQ(getValue(c), 33); + return errors; +} + +PLI_INT32 start_of_sim(t_cb_data* data) { + if (mon_check()) vpi_control(vpiStop); + return 0; +} + +void vpi_compat_bootstrap() { + static s_vpi_time vpi_time; + vpi_time.high = 0; + vpi_time.low = 0; + vpi_time.type = vpiSimTime; + + s_cb_data cb_data{}; + cb_data.reason = cbStartOfSimulation; + cb_data.cb_rtn = &start_of_sim; + cb_data.obj = NULL; + cb_data.time = &vpi_time; + cb_data.value = NULL; + cb_data.index = 0; + cb_data.user_data = NULL; + TestVpiHandle callback_h = vpi_register_cb(&cb_data); +} + +} // namespace + +void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; diff --git a/test_regress/t/t_vpi_forceable_unpacked_struct.py b/test_regress/t/t_vpi_forceable_unpacked_struct.py new file mode 100755 index 000000000..275c5c163 --- /dev/null +++ b/test_regress/t/t_vpi_forceable_unpacked_struct.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test module +# +# This file ONLY is placed under the Creative Commons Public Domain. +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: CC0-1.0 + +import vltest_bootstrap + +test.scenarios("vlt_all", "xrun") + +test.compile( + make_top_shell=False, + make_pli=True, + verilator_flags2=["--binary", "--vpi", "--no-l2name", "--public-flat-rw", test.pli_filename]) + +test.execute(use_libvpi=True) + +test.passes() diff --git a/test_regress/t/t_vpi_forceable_unpacked_struct.v b/test_regress/t/t_vpi_forceable_unpacked_struct.v new file mode 100644 index 000000000..64fd4f092 --- /dev/null +++ b/test_regress/t/t_vpi_forceable_unpacked_struct.v @@ -0,0 +1,22 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +typedef struct {logic [7:0] c;} nested_t; + +typedef struct { + logic [31:0] a; + logic [15:0] b; + nested_t nested; +} response_t; + +module t; + response_t forceable_response /* verilator forceable */; + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule