From 9bc88ff1bc75fdc53d346c75d8f21b544e76e52e Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 24 Feb 2026 19:06:05 -0500 Subject: [PATCH] Fix finding single DPI exports from other scopes --- include/verilated.cpp | 24 +++++++++++++-- include/verilated.h | 10 +----- include/verilated_imp.h | 36 +++++++++++++++++++--- test_regress/t/t_dpi_export_scope_bad.v | 22 ++++++++----- test_regress/t/t_dpi_export_scope_flat.cpp | 25 +++++++++++++++ test_regress/t/t_dpi_export_scope_flat.py | 18 +++++++++++ test_regress/t/t_dpi_export_scope_flat.v | 21 +++++++++++++ 7 files changed, 132 insertions(+), 24 deletions(-) create mode 100644 test_regress/t/t_dpi_export_scope_flat.cpp create mode 100755 test_regress/t/t_dpi_export_scope_flat.py create mode 100644 test_regress/t/t_dpi_export_scope_flat.v diff --git a/include/verilated.cpp b/include/verilated.cpp index 9bba77cd9..86a891df8 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -3464,11 +3464,10 @@ void Verilated::mkdir(const char* dirname) VL_MT_UNSAFE { void Verilated::quiesce() VL_MT_SAFE { // Wait until all threads under this evaluation are quiet - // THREADED-TODO } int Verilated::exportFuncNum(const char* namep) VL_MT_SAFE { - return VerilatedImp::exportFind(namep); + return VerilatedImp::exportFindNum(namep); } void Verilated::endOfThreadMTaskGuts(VerilatedEvalMsgQueue* evalMsgQp) VL_MT_SAFE { @@ -3573,7 +3572,7 @@ VerilatedScope::~VerilatedScope() { void VerilatedScope::exportInsert(int finalize, const char* namep, void* cb) VL_MT_UNSAFE { // Slowpath - called once/scope*export at construction // Insert a exported function into scope table - const int funcnum = VerilatedImp::exportInsert(namep); + const int funcnum = VerilatedImp::exportInsert(namep, cb); if (!finalize) { // Need two passes so we know array size to create // Alternative is to dynamically stretch the array, which is more code, and slower. @@ -3630,6 +3629,25 @@ VerilatedVar* VerilatedScope::varFind(const char* namep) const VL_MT_SAFE_POSTIN return nullptr; } +void* VerilatedScope::exportFind(const VerilatedScope* scopep, int funcnum) VL_MT_SAFE { + if (VL_UNLIKELY(!scopep)) return exportFindNullError(funcnum); + // If function is registered only once across all scopes, fast path it. + // UVM for example expects to find uvm_polling_value_change_notify + // from a different scope than where decared. + VL_DEBUG_IFDEF(assert(funcnum < VerilatedImp::exportFlatCbs().size());); + { + void* const cbp = VerilatedImp::exportFlatCbs()[funcnum]; + if (VL_LIKELY(cbp)) return cbp; + } + // Else specific scope-based export call + if (VL_LIKELY(funcnum < scopep->m_funcnumMax)) { + // m_callbacksp must be declared, as Max'es are > 0 + void* const cbp = scopep->m_callbacksp[funcnum]; + if (VL_LIKELY(cbp)) return cbp; + } + return scopep->exportFindError(funcnum); // LCOV_EXCL_LINE +} + void* VerilatedScope::exportFindNullError(int funcnum) VL_MT_SAFE { // Slowpath - Called only when find has failed const std::string msg = ("Testbench C called '"s + VerilatedImp::exportName(funcnum) diff --git a/include/verilated.h b/include/verilated.h index 5fbc6ddd8..15fdab267 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -744,15 +744,7 @@ public: // But internals only - called from verilated modules, VerilatedSyms void scopeDump() const; void* exportFindError(int funcnum) const VL_MT_SAFE; static void* exportFindNullError(int funcnum) VL_MT_SAFE; - static void* exportFind(const VerilatedScope* scopep, int funcnum) VL_MT_SAFE { - if (VL_UNLIKELY(!scopep)) return exportFindNullError(funcnum); - if (VL_LIKELY(funcnum < scopep->m_funcnumMax)) { - // m_callbacksp must be declared, as Max'es are > 0 - return scopep->m_callbacksp[funcnum]; - } else { // LCOV_EXCL_LINE - return scopep->exportFindError(funcnum); // LCOV_EXCL_LINE - } - } + static void* exportFind(const VerilatedScope* scopep, int funcnum) VL_MT_SAFE; Type type() const { return m_type; } }; diff --git a/include/verilated_imp.h b/include/verilated_imp.h index ab5ac0e17..6ee4e580e 100644 --- a/include/verilated_imp.h +++ b/include/verilated_imp.h @@ -433,6 +433,10 @@ protected: ExportNameMap m_exportMap VL_GUARDED_BY(m_exportMutex); int m_exportNext VL_GUARDED_BY(m_exportMutex) = 0; // Next export funcnum + // No guard, as init-time loaded + std::vector m_exportFlatCbs; // Exports when only single scope registered + std::vector m_exportFlatMulti; // Multiple scopes registerd; cannot use m_exportScopes + // CONSTRUCTORS VerilatedImpData() = default; }; @@ -542,8 +546,8 @@ public: // in the design that also happen to have our same callback function. // Rather than a 2D map, the integer scheme saves 500ish ns on a likely // miss at the cost of a multiply, and all lookups move to slowpath. - static int exportInsert(const char* namep) VL_MT_SAFE { - // Slow ok - called once/function at creation +private: + static int exportInsertName(const char* namep) VL_MT_SAFE { const VerilatedLockGuard lock{s().m_exportMutex}; const auto it = s().m_exportMap.find(namep); if (it == s().m_exportMap.end()) { @@ -553,15 +557,37 @@ public: return it->second; } } - static int exportFind(const char* namep) VL_MT_SAFE { + +public: + static int exportInsert(const char* namep, void* cb) VL_MT_SAFE { + const int funcnum = VerilatedImp::exportInsertName(namep); + const VerilatedLockGuard lock{s().m_exportMutex}; + // Slow ok - called once/function at creation + if (funcnum >= s().m_exportFlatCbs.size()) { + s().m_exportFlatCbs.resize(funcnum + 1); + s().m_exportFlatMulti.resize(funcnum + 1); + } + if (!s().m_exportFlatMulti[funcnum]) { + if (s().m_exportFlatCbs[funcnum] == cb) { // Duplicate + } else if (!s().m_exportFlatCbs[funcnum]) { // First + s().m_exportFlatCbs[funcnum] = cb; + } else { // Multiple registrants + s().m_exportFlatCbs[funcnum] = nullptr; + s().m_exportFlatMulti[funcnum] = true; + } + } + return funcnum; + } + static int exportFindNum(const char* namep) VL_MT_SAFE { const VerilatedLockGuard lock{s().m_exportMutex}; const auto& it = s().m_exportMap.find(namep); if (VL_LIKELY(it != s().m_exportMap.end())) return it->second; - const std::string msg = ("%Error: Testbench C called "s + namep - + " but no such DPI export function name exists in ANY model"); + const std::string msg = "%Error: Testbench C called "s + namep + + " but no such DPI export function name exists in ANY model"; VL_FATAL_MT("unknown", 0, "", msg.c_str()); return -1; } + static const std::vector& exportFlatCbs() VL_MT_SAFE { return s().m_exportFlatCbs; } static const char* exportName(int funcnum) VL_MT_SAFE { // Slowpath; find name for given export; errors only so no map to reverse-map it const VerilatedLockGuard lock{s().m_exportMutex}; diff --git a/test_regress/t/t_dpi_export_scope_bad.v b/test_regress/t/t_dpi_export_scope_bad.v index 36b91e6b3..3bd725bb9 100644 --- a/test_regress/t/t_dpi_export_scope_bad.v +++ b/test_regress/t/t_dpi_export_scope_bad.v @@ -7,15 +7,23 @@ // SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 module t; - s s(); + s s (); + other other (); - import "DPI-C" context function void dpix_run_tests(); - initial dpix_run_tests(); + import "DPI-C" context function void dpix_run_tests(); + initial dpix_run_tests(); endmodule module s; - export "DPI-C" task dpix_task; - task dpix_task(); - $write("Hello in %m\n"); - endtask + export "DPI-C" task dpix_task; + task dpix_task(); + $write("Hello in %m\n"); + endtask +endmodule + +module other; + export "DPI-C" task dpix_task; + task dpix_task(); + $write("Hello in %m\n"); + endtask endmodule diff --git a/test_regress/t/t_dpi_export_scope_flat.cpp b/test_regress/t/t_dpi_export_scope_flat.cpp new file mode 100644 index 000000000..d444e9fda --- /dev/null +++ b/test_regress/t/t_dpi_export_scope_flat.cpp @@ -0,0 +1,25 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2010 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +#include + +//====================================================================== + +#include "Vt_dpi_export_scope_flat__Dpi.h" + +#ifdef NEED_EXTERNS +extern "C" { +extern void dpix_task(); +} +#endif + +//====================================================================== + +void dpix_run_tests() { + dpix_task(); // Wrong scope +} diff --git a/test_regress/t/t_dpi_export_scope_flat.py b/test_regress/t/t_dpi_export_scope_flat.py new file mode 100755 index 000000000..e9528ed81 --- /dev/null +++ b/test_regress/t/t_dpi_export_scope_flat.py @@ -0,0 +1,18 @@ +#!/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: 2024 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(v_flags2=["--binary", test.pli_filename]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_dpi_export_scope_flat.v b/test_regress/t/t_dpi_export_scope_flat.v new file mode 100644 index 000000000..75d304d57 --- /dev/null +++ b/test_regress/t/t_dpi_export_scope_flat.v @@ -0,0 +1,21 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// 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: 2020 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +module t; + s s (); + + import "DPI-C" context function void dpix_run_tests(); + initial dpix_run_tests(); +endmodule + +module s; + export "DPI-C" task dpix_task; + task dpix_task(); + $write("Hello in %m\n"); + endtask +endmodule