From a2fcd37c08fc166e062cc2d744230c3bfd4e9c0d Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Tue, 23 Dec 2025 16:12:34 +0100 Subject: [PATCH] Fix unsupported `if` virtual interface warning (#6558) (#6861) --- src/V3AstNodeOther.h | 6 +++++ src/V3Gate.cpp | 4 +-- src/V3Localize.cpp | 1 + src/V3SchedVirtIface.cpp | 15 ++++++----- src/V3Width.cpp | 2 -- test_regress/t/t_interface_virtual_cond.py | 16 ++++++++++++ test_regress/t/t_interface_virtual_cond.v | 27 ++++++++++++++++++++ test_regress/t/t_interface_virtual_unsup.out | 12 ++++----- test_regress/t/t_interface_virtual_unsup.v | 2 ++ 9 files changed, 68 insertions(+), 17 deletions(-) create mode 100755 test_regress/t/t_interface_virtual_cond.py create mode 100644 test_regress/t/t_interface_virtual_cond.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 1009c3234..e91ad41da 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2185,6 +2185,12 @@ public: && !isSc() && !isPrimaryIO() && !isConst() && !isDouble() && !isString()); } bool isClassMember() const { return varType() == VVarType::MEMBER; } + bool isVirtIface() const { + if (AstIfaceRefDType* const dtp = VN_CAST(dtypep(), IfaceRefDType)) { + return dtp->isVirtual(); + } + return false; + } bool isStatementTemp() const { return varType() == VVarType::STMTTEMP; } bool isXTemp() const { return varType() == VVarType::XTEMP; } bool isParam() const { return varType().isParam(); } diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index f6e5a9198..67cb94f07 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -156,8 +156,8 @@ public: UINFO(6, "New vertex " << vscp); vVtxp = new GateVarVertex{this, vscp}; vscp->user1p(vVtxp); - if (vscp->varp()->sensIfacep()) { - // Can be used in a class method, which cannot be tracked statically + if (vscp->varp()->sensIfacep() || vscp->varp()->isVirtIface()) { + // Can be read/written to via the referenced actual interface vVtxp->clearReducibleAndDedupable("VirtIface"); vVtxp->setConsumed("VirtIface"); } diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index 331dd8549..9bb0dce59 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -192,6 +192,7 @@ class LocalizeVisitor final : public VNVisitor { && !nodep->varp()->isStatic() // Not a static variable && !nodep->varp()->isClassMember() // Statically exists in design hierarchy && !nodep->varp()->sensIfacep() // Not sensitive to an interface + && !nodep->varp()->isVirtIface() // Not interface pointer && !nodep->varp()->valuep() // Does not have an initializer ) { UINFO(4, "Consider for localization: " << nodep); diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 7c4821e3d..8a78aa8b1 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -104,12 +104,13 @@ private: }); } // Error on write across a virtual interface boundary - static void unsupportedWriteToVirtIface(AstNode* nodep, const char* locationp) { + static void unsupportedWriteToVirtIfaceMember(AstNode* nodep, const char* locationp) { if (!nodep) return; - foreachWrittenVirtIface(nodep, [locationp](AstVarRef* const selp, AstIface*) { - selp->v3warn(E_UNSUPPORTED, - "Unsupported: Write to virtual interface in " << locationp); - }); + foreachWrittenVirtIfaceMember( + nodep, [locationp](AstVarRef* const selp, AstIface*, AstVar* varp) { + selp->v3warn(E_UNSUPPORTED, + "Unsupported: Write to virtual interface in " << locationp); + }); } // Create trigger var for the given interface if it doesn't exist; return a write ref to it AstVarRef* createVirtIfaceTriggerRefp(FileLine* const flp, AstIface* ifacep) { @@ -164,7 +165,7 @@ private: iterateChildren(nodep); } void visit(AstNodeIf* nodep) override { - unsupportedWriteToVirtIface(nodep->condp(), "if condition"); + unsupportedWriteToVirtIfaceMember(nodep->condp(), "if condition"); { VL_RESTORER(m_trigAssignp); VL_RESTORER(m_trigAssignIfacep); @@ -201,7 +202,7 @@ private: } } void visit(AstLoopTest* nodep) override { - unsupportedWriteToVirtIface(nodep->condp(), "loop condition"); + unsupportedWriteToVirtIfaceMember(nodep->condp(), "loop condition"); } void visit(AstJumpBlock* nodep) override { { diff --git a/src/V3Width.cpp b/src/V3Width.cpp index ec931747f..b3b9f8afe 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3371,8 +3371,6 @@ class WidthVisitor final : public VNVisitor { nodep->varp(varp); AstIface* const ifacep = adtypep->ifacep(); varp->sensIfacep(ifacep); - nodep->fromp()->foreach( - [ifacep](AstVarRef* const refp) { refp->varp()->sensIfacep(ifacep); }); nodep->didWidth(true); return; } diff --git a/test_regress/t/t_interface_virtual_cond.py b/test_regress/t/t_interface_virtual_cond.py new file mode 100755 index 000000000..055e14291 --- /dev/null +++ b/test_regress/t/t_interface_virtual_cond.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.compile() + +test.passes() diff --git a/test_regress/t/t_interface_virtual_cond.v b/test_regress/t/t_interface_virtual_cond.v new file mode 100644 index 000000000..0f6824718 --- /dev/null +++ b/test_regress/t/t_interface_virtual_cond.v @@ -0,0 +1,27 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +interface Bus; + logic [15:0] data; +endinterface + +module t; + Bus intf(); + virtual Bus vif; + + function logic get_vif(inout virtual Bus vif); + vif = intf; + return 0; + endfunction + + initial begin + if (get_vif(vif)); + while (get_vif(vif)); + do ; while (get_vif(vif)); + for (int i = 0; get_vif(vif); i++); + end + +endmodule diff --git a/test_regress/t/t_interface_virtual_unsup.out b/test_regress/t/t_interface_virtual_unsup.out index ba1b8bf04..e4a69ebac 100644 --- a/test_regress/t/t_interface_virtual_unsup.out +++ b/test_regress/t/t_interface_virtual_unsup.out @@ -1,11 +1,11 @@ -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:22:22: Unsupported: Write to virtual interface in if condition - 22 | if (write_data(vif.data)) $write("dummy op"); +%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:24:22: Unsupported: Write to virtual interface in if condition + 24 | if (write_data(vif.data)) $write("dummy op"); | ^~~ ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:23:25: Unsupported: Write to virtual interface in loop condition - 23 | while (write_data(vif.data)); +%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:25:25: Unsupported: Write to virtual interface in loop condition + 25 | while (write_data(vif.data)); | ^~~ -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:24:30: Unsupported: Write to virtual interface in loop condition - 24 | do ; while (write_data(vif.data)); +%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:26:30: Unsupported: Write to virtual interface in loop condition + 26 | do ; while (write_data(vif.data)); | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_interface_virtual_unsup.v b/test_regress/t/t_interface_virtual_unsup.v index 30b1f13df..dd1f83774 100644 --- a/test_regress/t/t_interface_virtual_unsup.v +++ b/test_regress/t/t_interface_virtual_unsup.v @@ -4,6 +4,8 @@ // any use, without warranty, 2023 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 +// NOTE: Once this is supported, t_interface_virtual_cond is no longer needed + interface Bus; logic [15:0] data; endinterface