From 2ceea267e5fe3ab8dfaa0bf65ffb395870f28f79 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 28 Feb 2026 15:09:01 +0000 Subject: [PATCH] Fix eliminating assignments to DPI-read vaiables (#7158) --- src/V3AstNodeOther.h | 4 +++ src/V3AstNodes.cpp | 4 +++ src/V3Life.cpp | 1 + src/V3Task.cpp | 17 +++++++++---- test_regress/t/t_opt_life_dpi_read.cpp | 22 ++++++++++++++++ test_regress/t/t_opt_life_dpi_read.py | 18 +++++++++++++ test_regress/t/t_opt_life_dpi_read.v | 35 ++++++++++++++++++++++++++ 7 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 test_regress/t/t_opt_life_dpi_read.cpp create mode 100755 test_regress/t/t_opt_life_dpi_read.py create mode 100644 test_regress/t/t_opt_life_dpi_read.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 4c3558ba8..f2c25e094 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1933,6 +1933,7 @@ class AstVar final : public AstNode { bool m_isLatched : 1; // Not assigned in all control paths of combo always bool m_isForceable : 1; // May be forced/released externally from user C code bool m_isForcedByCode : 1; // May be forced/released from AstAssignForce/AstRelease + bool m_isReadByDpi : 1; // This variable can be read by a DPI Export bool m_isWrittenByDpi : 1; // This variable can be written by a DPI Export bool m_isWrittenBySuspendable : 1; // This variable can be written by a suspendable process bool m_ignorePostRead : 1; // Ignore reads in 'Post' blocks during ordering @@ -1987,6 +1988,7 @@ class AstVar final : public AstNode { m_isLatched = false; m_isForceable = false; m_isForcedByCode = false; + m_isReadByDpi = false; m_isWrittenByDpi = false; m_isWrittenBySuspendable = false; m_ignorePostRead = false; @@ -2152,6 +2154,8 @@ public: void setForceable() { m_isForceable = true; } void setForcedByCode() { m_isForcedByCode = true; } bool isForced() const { return m_isForceable || m_isForcedByCode; } + bool isReadByDpi() const { return m_isReadByDpi; } + void setReadByDpi() { m_isReadByDpi = true; } bool isWrittenByDpi() const { return m_isWrittenByDpi; } void setWrittenByDpi() { m_isWrittenByDpi = true; } bool isWrittenBySuspendable() const { return m_isWrittenBySuspendable; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 5c14d9b47..7c98ab18e 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2880,6 +2880,8 @@ void AstVar::dump(std::ostream& str) const { if (isSigPublic()) str << " [P]"; if (isSigUserRdPublic()) str << " [PRD]"; if (isSigUserRWPublic()) str << " [PWR]"; + if (isReadByDpi()) str << " [DPIRD]"; + if (isWrittenByDpi()) str << " [DPIWR]"; if (isInternal()) str << " [INTERNAL]"; if (isLatched()) str << " [LATCHED]"; if (isUsedLoopIdx()) str << " [LOOPIDX]"; @@ -2927,6 +2929,8 @@ void AstVar::dumpJson(std::ostream& str) const { if (dtypep()) dumpJsonStr(str, "dtypeName", dtypep()->name()); dumpJsonBoolFuncIf(str, isSigUserRdPublic); dumpJsonBoolFuncIf(str, isSigUserRWPublic); + dumpJsonBoolFuncIf(str, isReadByDpi); + dumpJsonBoolFuncIf(str, isWrittenByDpi); dumpJsonBoolFuncIf(str, isGParam); dumpJsonBoolFuncIf(str, isParam); dumpJsonBoolFuncIf(str, attrScBv); diff --git a/src/V3Life.cpp b/src/V3Life.cpp index 46fc197e9..52cca6bf3 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -129,6 +129,7 @@ public: const AstVar* const varp = vscp->varp(); // We don't optimize any public sigs if (varp->isSigPublic()) return; + if (varp->isReadByDpi()) return; if (varp->sensIfacep()) return; // Check the var entry, and remove if appropriate AstNodeStmt* const oldassp = entr.assignp(); diff --git a/src/V3Task.cpp b/src/V3Task.cpp index a40cade63..5f611d6be 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -1428,15 +1428,22 @@ class TaskVisitor final : public VNVisitor { // Mark non-local variables written by the exported function bool writesNonLocals = false; cfuncp->foreach([&writesNonLocals](AstVarRef* refp) { - if (refp->access().isReadOnly()) return; // Ignore read reference AstVar* const varp = refp->varScopep()->varp(); // We are ignoring function locals as they should not be referenced anywhere // outside the enclosing AstCFunc, hence they are irrelevant for code ordering. if (varp->isFuncLocal()) return; - // Mark it as written by DPI export - varp->setWrittenByDpi(); - // Remember we had some - writesNonLocals = true; + // Check if written + if (refp->access().isWriteOrRW()) { + // Mark it as written by DPI export + varp->setWrittenByDpi(); + // Remember we had some + writesNonLocals = true; + } + // Check if read + if (refp->access().isReadOrRW()) { + // Mark it as read by DPI export + varp->setReadByDpi(); + } }); // If this DPI export writes some non-local variables, set the DPI Export Trigger flag diff --git a/test_regress/t/t_opt_life_dpi_read.cpp b/test_regress/t/t_opt_life_dpi_read.cpp new file mode 100644 index 000000000..2fd744c6d --- /dev/null +++ b/test_regress/t/t_opt_life_dpi_read.cpp @@ -0,0 +1,22 @@ +// -*- 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-2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#include "svdpi.h" + +#include + +extern "C" int getDpi(); + +extern "C" int getViaDpi() { + const int value = getDpi(); + std::cout << "getDpi " << value << std::endl; + return value; +} diff --git a/test_regress/t/t_opt_life_dpi_read.py b/test_regress/t/t_opt_life_dpi_read.py new file mode 100755 index 000000000..16a308b26 --- /dev/null +++ b/test_regress/t/t_opt_life_dpi_read.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: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt_all') + +test.compile(verilator_flags2=["--binary", "--stats", test.top_filename.replace(".v", ".cpp")]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_opt_life_dpi_read.v b/test_regress/t/t_opt_life_dpi_read.v new file mode 100644 index 000000000..e35d61ccf --- /dev/null +++ b/test_regress/t/t_opt_life_dpi_read.v @@ -0,0 +1,35 @@ +// 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 + +// verilog_format: off +`define stop $stop +`define check(got,exp) do if ((got) !== (exp)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (got), (exp)); `stop; end while(0) +// verilog_format: on + +module t; + + int dpiGet = 0; + function automatic int getDpi(); + return dpiGet; + endfunction + export "DPI-C" function getDpi; + import "DPI-C" context function int getViaDpi(); // calls getDpi() + + int tmp1, tmp2, tmp3; + + initial begin + dpiGet = 13; + tmp1 = getViaDpi(); + dpiGet = 14; + tmp2 = getViaDpi(); + dpiGet = 15; + tmp3 = getViaDpi(); + `check(tmp1, 13); + `check(tmp2, 14); + `check(tmp3, 15); + end + +endmodule