From 4a4db22cfce9fd41875ca60c13eb328f914866fb Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Tue, 23 Jun 2026 00:48:37 +0000 Subject: [PATCH] Address review feedback: VPI loader into Verilated class; autoconf link flags - Replace free vl_load_vpi_libs(argc, argv) with Verilated::loadVpiLib(const std::string&), taking a single library filename and living in the Verilated class (per review). - Parse +verilator+vpi+ in VerilatedContextImp::commandArgVl and load the library there, mirroring solverLogFilename; drop the generated-main call in V3EmitCMain so loading happens during commandArgs(). - Probe runtime-VPI link flags at configure time (CFG_LDFLAGS_DYNAMIC / CFG_LDLIBS_DYNAMIC) instead of hardcoding ifeq ($(UNAME_S),...) in V3EmitMk. - Drop the brittle .mk file_grep from t_flag_main_vpi.py; update t_flag_main_vpi_noexe to grep the new CFG_*_DYNAMIC variables. Signed-off-by: Matthew Ballance Co-Authored-By: Claude Opus 4.8 (1M context) --- configure.ac | 28 ++++++++ include/verilated.cpp | 95 +++++++++++++------------ include/verilated.h | 3 + include/verilated.mk.in | 3 + include/verilated_funcs.h | 5 -- src/V3EmitCMain.cpp | 4 +- src/V3EmitMk.cpp | 23 +++--- test_regress/t/t_flag_main_vpi.py | 14 ---- test_regress/t/t_flag_main_vpi_noexe.py | 14 ++-- 9 files changed, 99 insertions(+), 90 deletions(-) diff --git a/configure.ac b/configure.ac index 6917f0c4b..00d8ac713 100644 --- a/configure.ac +++ b/configure.ac @@ -589,6 +589,34 @@ m4_foreach([ldflag], [ AC_SUBST(CFG_LDLIBS_THREADS) AC_SUBST(CFG_LDFLAGS_THREADS_CMAKE) +# Find link flags for runtime VPI library loading (+verilator+vpi+). +# The model executable must export its VPI symbols so the dlopen'd library can +# resolve them: -rdynamic (GNU ld) or -Wl,-export_dynamic (Darwin); the first the +# linker accepts wins. -ldl provides dlopen/dlsym where it is a separate library. +_MY_LDLIBS_CHECK_SET(CFG_LDFLAGS_DYNAMIC, -rdynamic) +# -Wl,-export_dynamic contains a comma, so probe it directly rather than through +# the _MY_LDLIBS_CHECK_* macros (which re-expand their flag argument unquoted). +if test "$CFG_LDFLAGS_DYNAMIC" = ""; then + ACO_SAVE_LIBS="$LIBS" + LIBS="$LIBS -Wl,-export_dynamic" + AC_MSG_CHECKING([whether $CXX linker accepts -Wl,-export_dynamic]) + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[]])], + [_my_result=yes + if test -s conftest.err; then + if grep -e "-export_dynamic" conftest.err >/dev/null; then + _my_result=no + fi + fi], + [_my_result=no]) + AC_MSG_RESULT($_my_result) + LIBS="$ACO_SAVE_LIBS" + if test "$_my_result" = "yes"; then CFG_LDFLAGS_DYNAMIC="-Wl,-export_dynamic"; fi +fi +AC_SUBST(CFG_LDFLAGS_DYNAMIC) +_MY_LDLIBS_CHECK_OPT(CFG_LDLIBS_DYNAMIC, -ldl) +AC_SUBST(CFG_LDLIBS_DYNAMIC) + # If 'mold' is installed, use it to link for faster buildtimes _MY_LDLIBS_CHECK_OPT(CFG_LDFLAGS_SRC, -fuse-ld=mold) _MY_LDLIBS_CHECK_OPT(CFG_LDFLAGS_VERILATED, -fuse-ld=mold) diff --git a/include/verilated.cpp b/include/verilated.cpp index cc1464901..dde9c7b0c 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -92,7 +92,7 @@ #if VM_VPI # include # ifndef _WIN32 -# include // Used by vl_load_vpi_libs +# include // Used by Verilated::loadVpiLib # endif #endif @@ -269,55 +269,54 @@ void VL_WARN_MT(const char* filename, int linenum, const char* hier, const char* //=========================================================================== // Runtime VPI shared library loading (--vpi) +// Load one VPI shared library named by a +verilator+vpi+[:] argument. +// 'arg' is the payload after the prefix: either "" (invoke the library's +// vlog_startup_routines array) or ":" (invoke the named bootstrap). +void Verilated::loadVpiLib(const std::string& arg) VL_MT_UNSAFE { #if VM_VPI -void vl_load_vpi_libs(int argc, char** argv) VL_MT_UNSAFE { - const char* prefix = "+verilator+vpi+"; - const size_t pfx_len = std::strlen(prefix); - for (int i = 1; i < argc; ++i) { - if (std::strncmp(argv[i], prefix, pfx_len) != 0) continue; - const std::string arg(argv[i] + pfx_len); - if (arg.empty()) continue; -#ifdef _WIN32 - VL_FATAL_MT("", 0, "", - "+verilator+vpi+: runtime VPI library loading is not supported on" - " Windows; link the VPI code into the model instead"); -#else - using vlog_startup_t = void (*)(); - // Split : on the last ':' - const std::string::size_type colon_pos = arg.rfind(':'); - const bool has_entry = (colon_pos != std::string::npos); - const std::string libpath = has_entry ? arg.substr(0, colon_pos) : arg; - const std::string entry_name = has_entry ? arg.substr(colon_pos + 1) : std::string{}; - void* handle = dlopen(libpath.c_str(), RTLD_LAZY); - if (!handle) - // The library path is stable; the dlerror() text is platform-specific, so put it on - // a separate "- " line (test golden files strip "- " lines, keeping output portable). + if (arg.empty()) return; +# ifdef _WIN32 + VL_FATAL_MT("", 0, "", + "+verilator+vpi+: runtime VPI library loading is not supported on" + " Windows; link the VPI code into the model instead"); +# else + using vlog_startup_t = void (*)(); + // Split : on the last ':' + const std::string::size_type colon_pos = arg.rfind(':'); + const bool has_entry = (colon_pos != std::string::npos); + const std::string libpath = has_entry ? arg.substr(0, colon_pos) : arg; + const std::string entry_name = has_entry ? arg.substr(colon_pos + 1) : std::string{}; + void* handle = dlopen(libpath.c_str(), RTLD_LAZY); + if (!handle) + // The library path is stable; the dlerror() text is platform-specific, so put it on + // a separate "- " line (test golden files strip "- " lines, keeping output portable). + VL_FATAL_MT( + "", 0, "", + (std::string{"Cannot load VPI library: "} + libpath + "\n- dlerror: " + dlerror()) + .c_str()); + if (has_entry) { + vlog_startup_t bsp = reinterpret_cast(dlsym(handle, entry_name.c_str())); + if (!bsp) VL_FATAL_MT( "", 0, "", - (std::string{"Cannot load VPI library: "} + libpath + "\n- dlerror: " + dlerror()) + (std::string{"Cannot find VPI bootstrap '"} + entry_name + "' in: " + libpath) .c_str()); - if (has_entry) { - vlog_startup_t bsp - = reinterpret_cast(dlsym(handle, entry_name.c_str())); - if (!bsp) - VL_FATAL_MT( - "", 0, "", - (std::string{"Cannot find VPI bootstrap '"} + entry_name + "' in: " + libpath) - .c_str()); - bsp(); - } else { - vlog_startup_t* routinesp - = reinterpret_cast(dlsym(handle, "vlog_startup_routines")); - if (!routinesp) - VL_FATAL_MT( - "", 0, "", - (std::string{"Cannot find 'vlog_startup_routines' in: "} + libpath).c_str()); - for (int j = 0; routinesp[j]; ++j) routinesp[j](); - } -#endif + bsp(); + } else { + vlog_startup_t* routinesp + = reinterpret_cast(dlsym(handle, "vlog_startup_routines")); + if (!routinesp) + VL_FATAL_MT("", 0, "", + (std::string{"Cannot find 'vlog_startup_routines' in: "} + libpath) + .c_str()); + for (int j = 0; routinesp[j]; ++j) routinesp[j](); } -} +# endif +#else + // Never reached: the command-line handler only calls this when compiled with --vpi. + (void)arg; #endif +} //=========================================================================== // Debug prints @@ -3604,9 +3603,11 @@ void VerilatedContextImp::commandArgVl(const std::string& arg) { if (u64 == 0) u64 = pickRandomSeed(); randSeed(static_cast(u64)); } else if (commandArgVlString(arg, "+verilator+vpi+", str)) { - // With --vpi the generated --main (vl_load_vpi_libs) consumes this, so accept - // it silently here. Without --vpi there is no loader, so warn it is ignored. -#if !VM_VPI + // With --vpi, load the requested shared library now. Without --vpi there is + // no VPI runtime, so warn the argument is ignored. +#if VM_VPI + Verilated::loadVpiLib(str); +#else VL_WARN_MT( "COMMAND_LINE", 0, "", ("+verilator+vpi+ ignored: simulation was not compiled with --vpi '" + arg + "'") diff --git a/include/verilated.h b/include/verilated.h index 59ae1ad8c..73c7e5b2a 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -1036,6 +1036,9 @@ public: static void scTraceBeforeElaborationError() VL_ATTR_NORETURN VL_MT_SAFE; static void stackCheck(QData needSize) VL_MT_UNSAFE; + // Internal: Load a VPI shared library (+verilator+vpi+[:]) + static void loadVpiLib(const std::string& arg) VL_MT_UNSAFE; + // Internal: Get and set DPI context static const VerilatedScope* dpiScope() VL_MT_SAFE { return t_s.t_dpiScopep; } static void dpiScope(const VerilatedScope* scopep) VL_MT_SAFE { t_s.t_dpiScopep = scopep; } diff --git a/include/verilated.mk.in b/include/verilated.mk.in index 21c465258..a90907755 100644 --- a/include/verilated.mk.in +++ b/include/verilated.mk.in @@ -52,6 +52,9 @@ CFG_GCH_IF_CLANG = @CFG_GCH_IF_CLANG@ CFG_LDFLAGS_VERILATED = @CFG_LDFLAGS_VERILATED@ # Linker libraries for multithreading CFG_LDLIBS_THREADS = @CFG_LDLIBS_THREADS@ +# Linker flags/libraries for runtime VPI library loading (+verilator+vpi+) +CFG_LDFLAGS_DYNAMIC = @CFG_LDFLAGS_DYNAMIC@ +CFG_LDLIBS_DYNAMIC = @CFG_LDLIBS_DYNAMIC@ ###################################################################### # Programs diff --git a/include/verilated_funcs.h b/include/verilated_funcs.h index 71c0bf729..d69bca256 100644 --- a/include/verilated_funcs.h +++ b/include/verilated_funcs.h @@ -74,11 +74,6 @@ extern void VL_FATAL_MT(const char* filename, int linenum, const char* hier, extern void VL_WARN_MT(const char* filename, int linenum, const char* hier, const char* msg) VL_MT_SAFE; -#if VM_VPI -/// Load VPI shared libraries requested via +verilator+vpi+[:]. -extern void vl_load_vpi_libs(int argc, char** argv) VL_MT_UNSAFE; -#endif - /// Print a string, multithread safe. Eventually VL_PRINTF will get called. extern void VL_PRINTF_MT(const char* formatp, ...) VL_ATTR_PRINTF(1) VL_MT_SAFE; diff --git a/src/V3EmitCMain.cpp b/src/V3EmitCMain.cpp index 21d701c05..0d3fd0b6d 100644 --- a/src/V3EmitCMain.cpp +++ b/src/V3EmitCMain.cpp @@ -104,8 +104,8 @@ private: puts("\n"); if (v3Global.opt.vpi()) { - puts("// Load VPI shared libraries requested via +verilator+vpi+\n"); - puts("vl_load_vpi_libs(argc, argv);\n"); + // VPI shared libraries requested via +verilator+vpi+ are loaded by + // contextp->commandArgs() above, before the statically-linked startup routines. puts("// Hook VPI startup routines and invoke callback\n"); puts("if (vlog_startup_routines) {\n"); puts(/**/ "for (auto routinep = &vlog_startup_routines[0]; *routinep; routinep++)" diff --git a/src/V3EmitMk.cpp b/src/V3EmitMk.cpp index c6d6c629d..a63d328f3 100644 --- a/src/V3EmitMk.cpp +++ b/src/V3EmitMk.cpp @@ -571,8 +571,8 @@ public: of.puts("VM_VPI = "); of.puts(v3Global.opt.vpi() ? "1" : "0"); of.puts("\n"); - // Link flags for runtime VPI library loading are platform-specific and emitted by - // emitOverallMake() after verilated.mk is included (so $(UNAME_S) is defined). + // Link flags for runtime VPI library loading are emitted by emitOverallMake() after + // verilated.mk is included (so $(CFG_LDFLAGS_DYNAMIC)/$(CFG_LDLIBS_DYNAMIC) are defined). of.puts("\n### Object file lists...\n"); for (int support = 0; support < 3; ++support) { @@ -737,20 +737,13 @@ public: of.puts("\n"); if (v3Global.opt.vpi()) { - // Runtime VPI library loading (+verilator+vpi+) needs platform-specific - // link flags so the dlopen'd library can resolve the executable's VPI symbols. - // Resolve them with $(UNAME_S). + // Runtime VPI library loading (+verilator+vpi+) needs the executable to + // export its VPI symbols so the dlopen'd library can resolve them, plus the + // dl library for dlopen/dlsym. The exact flags are probed at configure time + // (CFG_LDFLAGS_DYNAMIC / CFG_LDLIBS_DYNAMIC in verilated.mk). of.puts("# Runtime VPI library loading (+verilator+vpi+) link requirements\n"); - // -rdynamic exports the executable's VPI symbols to the dlopen'd library; - // -ldl provides dlopen/dlsym. - of.puts("ifeq ($(UNAME_S),Linux)\n"); - of.puts(/**/ "LDFLAGS += -rdynamic\n"); - of.puts(/**/ "LDLIBS += -ldl\n"); - of.puts("endif\n"); - // macOS analog of -rdynamic; dlopen/dlsym live in libSystem so no -ldl needed. - of.puts("ifeq ($(UNAME_S),Darwin)\n"); - of.puts(/**/ "LDFLAGS += -Wl,-export_dynamic\n"); - of.puts("endif\n"); + of.puts("LDFLAGS += $(CFG_LDFLAGS_DYNAMIC)\n"); + of.puts("LDLIBS += $(CFG_LDLIBS_DYNAMIC)\n"); of.puts("\n"); } } diff --git a/test_regress/t/t_flag_main_vpi.py b/test_regress/t/t_flag_main_vpi.py index 3702f58f5..695b70e76 100755 --- a/test_regress/t/t_flag_main_vpi.py +++ b/test_regress/t/t_flag_main_vpi.py @@ -7,7 +7,6 @@ # SPDX-FileCopyrightText: 2025 Wilson Snyder # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -import platform import vltest_bootstrap test.scenarios('vlt') @@ -16,19 +15,6 @@ test.scenarios('vlt') # Also compile a VPI shared library to be loaded at runtime via +verilator+vpi+. test.compile(make_pli=True, verilator_flags2=["--binary --vpi --public-flat-rw"]) -# With --vpi and an executable (--binary implies --exe), the Makefile must export the -# executable's symbols so the generated loader's dlopen'd library can resolve them. -# The flags are gated on $(UNAME_S): -rdynamic/-ldl on Linux, -Wl,-export_dynamic on -# macOS (Windows is unaffected, runtime loading is unsupported there). -mk = test.obj_dir + "/V" + test.name + ".mk" -if platform.system() == 'Darwin': - test.file_grep(mk, r'ifeq \(\$\(UNAME_S\),Darwin\)') - test.file_grep(mk, r'LDFLAGS \+= -Wl,-export_dynamic') -else: - test.file_grep(mk, r'ifeq \(\$\(UNAME_S\),Linux\)') - test.file_grep(mk, r'LDFLAGS \+= -rdynamic') - test.file_grep(mk, r'LDLIBS \+= -ldl') - # Run the generated binary; load the VPI library via the +verilator+vpi+ plusarg. # The VPI library's output (observed 'count' reaching MAX_TICKS, then end-of-sim) is # checked against the golden .out file. diff --git a/test_regress/t/t_flag_main_vpi_noexe.py b/test_regress/t/t_flag_main_vpi_noexe.py index c8177a0eb..1f347105b 100755 --- a/test_regress/t/t_flag_main_vpi_noexe.py +++ b/test_regress/t/t_flag_main_vpi_noexe.py @@ -12,19 +12,19 @@ import vltest_bootstrap test.scenarios('vlt') # --vpi without --exe (no generated main, library-only output): the Makefile -# must still report VM_VPI = 1, but must NOT add '-rdynamic' since there is no -# executable to export VPI symbols from. Exercises the exe()==false branch of -# the VPI -rdynamic gate in V3EmitMk. +# must still report VM_VPI = 1, but must NOT add the runtime-VPI link flags since +# there is no executable to export VPI symbols from. Exercises the exe()==false +# branch of the VPI link-flag gate in V3EmitMk. test.top_filename = 't/t_flag_main_vpi.v' test.compile(make_main=False, verilator_make_gmake=False, verilator_flags2=["--vpi --timing"]) test.file_grep(test.obj_dir + "/V" + test.name + "_classes.mk", r'VM_VPI = 1') # Without --exe there is no executable to export symbols from or to dlopen into, -# so the runtime-VPI link flags must not be emitted into the Makefile on any platform. +# so the runtime-VPI link flags (CFG_LDFLAGS_DYNAMIC/CFG_LDLIBS_DYNAMIC, probed at +# configure time) must not be referenced in the generated Makefile. mk = test.obj_dir + "/V" + test.name + ".mk" -test.file_grep_not(mk, r'-rdynamic') # Linux -test.file_grep_not(mk, r'-ldl') # Linux -test.file_grep_not(mk, r'-export_dynamic') # macOS +test.file_grep_not(mk, r'CFG_LDFLAGS_DYNAMIC') +test.file_grep_not(mk, r'CFG_LDLIBS_DYNAMIC') test.passes()