From ed46878f7bbe7e84bb9799c7006cc3ebac7edb6c Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 4 Jun 2025 21:43:46 -0400 Subject: [PATCH] Fix virtual interface array typedef expressions (#6057). --- Changes | 1 + src/V3Inst.cpp | 72 +++++++++++++++------------- test_regress/t/t_interface_array3.py | 18 +++++++ test_regress/t/t_interface_array3.v | 54 +++++++++++++++++++++ 4 files changed, 111 insertions(+), 34 deletions(-) create mode 100755 test_regress/t/t_interface_array3.py create mode 100644 test_regress/t/t_interface_array3.v diff --git a/Changes b/Changes index a62177081..290b04282 100644 --- a/Changes +++ b/Changes @@ -51,6 +51,7 @@ Verilator 5.037 devel * Fix GCC 10 read-only linker error (#6040). [Todd Strader] * Fix WIDTHCONCAT on packed pattern assignment (#6045). [Dan Petrisko] * Fix V3OrderParallel scoring contraction hang (#6052). [Bartłomiej Chmiel, Antmicro Ltd.] +* Fix virtual interface array typedef expressions (#6057). Verilator 5.036 2025-04-27 diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 5b356f638..d9e320c7b 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -85,10 +85,11 @@ class InstVisitor final : public VNVisitor { m_cellp->addNextHere(assp); if (debug() >= 9) assp->dumpTree("- _new: "); } else if (nodep->modVarp()->isIfaceRef() - || (VN_IS(nodep->modVarp()->subDTypep(), UnpackArrayDType) - && VN_IS( - VN_AS(nodep->modVarp()->subDTypep(), UnpackArrayDType)->subDTypep(), - IfaceRefDType))) { + || (VN_IS(nodep->modVarp()->dtypep()->skipRefp(), UnpackArrayDType) + && VN_IS(VN_AS(nodep->modVarp()->dtypep()->skipRefp(), UnpackArrayDType) + ->subDTypep() + ->skipRefp(), + IfaceRefDType))) { // Create an AstAssignVarScope for Vars to Cells so we can // link with their scope later AstNodeExpr* const lhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(), @@ -133,7 +134,7 @@ private: // VISITORS void visit(AstVar* nodep) override { - if (VN_IS(nodep->dtypep(), IfaceRefDType)) { + if (VN_IS(nodep->dtypep()->skipRefp(), IfaceRefDType)) { UINFO(8, " dm-1-VAR " << nodep); insert(nodep); } @@ -185,20 +186,22 @@ private: // VISITORS void visit(AstVar* nodep) override { - if (VN_IS(nodep->dtypep(), UnpackArrayDType) - && VN_IS(VN_AS(nodep->dtypep(), UnpackArrayDType)->subDTypep(), IfaceRefDType)) { - if (VN_AS(VN_AS(nodep->dtypep(), UnpackArrayDType)->subDTypep(), IfaceRefDType) + AstNode* const dtp = nodep->dtypep()->skipRefp(); + if (VN_IS(dtp, UnpackArrayDType) + && VN_IS(VN_AS(dtp, UnpackArrayDType)->subDTypep()->skipRefp(), IfaceRefDType)) { + if (VN_AS(VN_AS(dtp, UnpackArrayDType)->subDTypep()->skipRefp(), IfaceRefDType) ->isVirtual()) return; UINFO(8, " dv-vec-VAR " << nodep); - AstUnpackArrayDType* const arrdtype = VN_AS(nodep->dtypep(), UnpackArrayDType); + AstUnpackArrayDType* const arrdtype = VN_AS(dtp, UnpackArrayDType); AstNode* prevp = nullptr; for (int i = arrdtype->lo(); i <= arrdtype->hi(); ++i) { const string varNewName = nodep->name() + "__BRA__" + cvtToStr(i) + "__KET__"; UINFO(8, "VAR name insert " << varNewName << " " << nodep); if (!m_deModVars.find(varNewName)) { AstIfaceRefDType* const ifaceRefp - = VN_AS(arrdtype->subDTypep(), IfaceRefDType)->cloneTree(false); + = VN_AS(arrdtype->subDTypep()->skipRefp(), IfaceRefDType) + ->cloneTree(false); arrdtype->addNextHere(ifaceRefp); ifaceRefp->cellp(nullptr); @@ -233,11 +236,13 @@ private: m_cellRangep = nodep->rangep(); AstVar* const ifaceVarp = VN_CAST(nodep->nextp(), Var); + AstNodeDType* const ifaceVarDtp + = ifaceVarp ? ifaceVarp->dtypep()->skipRefp() : nullptr; const bool isIface - = ifaceVarp && VN_IS(ifaceVarp->dtypep(), UnpackArrayDType) - && VN_IS(VN_AS(ifaceVarp->dtypep(), UnpackArrayDType)->subDTypep(), + = ifaceVarp && VN_IS(ifaceVarDtp, UnpackArrayDType) + && VN_IS(VN_AS(ifaceVarDtp, UnpackArrayDType)->subDTypep()->skipRefp(), IfaceRefDType) - && !VN_AS(VN_AS(ifaceVarp->dtypep(), UnpackArrayDType)->subDTypep(), + && !VN_AS(VN_AS(ifaceVarDtp, UnpackArrayDType)->subDTypep()->skipRefp(), IfaceRefDType) ->isVirtual(); @@ -261,10 +266,9 @@ private: // If this AstCell is actually an interface instantiation, also clone the IfaceRef // within the same parent module as the cell if (isIface) { - AstUnpackArrayDType* const arrdtype - = VN_AS(ifaceVarp->dtypep(), UnpackArrayDType); + AstUnpackArrayDType* const arrdtype = VN_AS(ifaceVarDtp, UnpackArrayDType); AstIfaceRefDType* const origIfaceRefp - = VN_AS(arrdtype->subDTypep(), IfaceRefDType); + = VN_AS(arrdtype->subDTypep()->skipRefp(), IfaceRefDType); origIfaceRefp->cellp(nullptr); AstVar* const varNewp = ifaceVarp->cloneTree(false); AstIfaceRefDType* const ifaceRefp = origIfaceRefp->cloneTree(false); @@ -306,14 +310,14 @@ private: void visit(AstPin* nodep) override { // Any non-direct pins need reconnection with a part-select if (!nodep->exprp()) return; // No-connect + const AstNodeDType* expDtp = nodep->exprp()->dtypep()->skipRefp(); if (m_cellRangep) { UINFO(4, " PIN " << nodep); const int modwidth = nodep->modVarp()->width(); const int expwidth = nodep->exprp()->width(); const std::pair pinDim - = nodep->modVarp()->dtypep()->dimensions(false); - const std::pair expDim - = nodep->exprp()->dtypep()->dimensions(false); + = nodep->modVarp()->dtypep()->skipRefp()->dimensions(false); + const std::pair expDim = expDtp->dimensions(false); UINFO(4, " PINVAR " << nodep->modVarp()); UINFO(4, " EXP " << nodep->exprp()); UINFO(4, " expwidth=" << expwidth << " modwidth=" << modwidth @@ -321,8 +325,7 @@ private: << " pinDim(p,u)=" << pinDim.first << "," << pinDim.second); if (expDim.second == pinDim.second + 1) { // Connection to array, where array dimensions match the instant dimension - const AstRange* const rangep - = VN_AS(nodep->exprp()->dtypep(), UnpackArrayDType)->rangep(); + const AstRange* const rangep = VN_AS(expDtp, UnpackArrayDType)->rangep(); const int arraySelNum = rangep->ascending() ? (rangep->elementsConst() - 1 - m_instSelNum) : m_instSelNum; @@ -358,9 +361,9 @@ private: } // end expanding ranged cell else if (AstArraySel* const arrselp = VN_CAST(nodep->exprp(), ArraySel)) { if (const AstUnpackArrayDType* const arrp - = VN_CAST(arrselp->fromp()->dtypep(), UnpackArrayDType)) { - if (!VN_IS(arrp->subDTypep(), IfaceRefDType)) return; - if (VN_AS(arrp->subDTypep(), IfaceRefDType)->isVirtual()) return; + = VN_CAST(arrselp->fromp()->dtypep()->skipRefp(), UnpackArrayDType)) { + if (!VN_IS(arrp->subDTypep()->skipRefp(), IfaceRefDType)) return; + if (VN_AS(arrp->subDTypep()->skipRefp(), IfaceRefDType)->isVirtual()) return; // Interface pin attaches to one element of arrayed interface V3Const::constifyParamsEdit(arrselp->bitp()); const AstConst* const constp = VN_CAST(arrselp->bitp(), Const); @@ -386,9 +389,9 @@ private: } else { AstVar* const pinVarp = nodep->modVarp(); const AstUnpackArrayDType* const pinArrp - = VN_CAST(pinVarp->dtypep(), UnpackArrayDType); - if (!pinArrp || !VN_IS(pinArrp->subDTypep(), IfaceRefDType)) return; - if (VN_AS(pinArrp->subDTypep(), IfaceRefDType)->isVirtual()) return; + = VN_CAST(pinVarp->dtypep()->skipRefp(), UnpackArrayDType); + if (!pinArrp || !VN_IS(pinArrp->subDTypep()->skipRefp(), IfaceRefDType)) return; + if (VN_AS(pinArrp->subDTypep()->skipRefp(), IfaceRefDType)->isVirtual()) return; // Arrayed pin/var attaches to arrayed submodule lower port/var, expand it AstNode* prevp = nullptr; AstNode* prevPinp = nullptr; @@ -403,7 +406,8 @@ private: if (!pinVarp->backp()) { varNewp = m_deModVars.find(varNewName); } else { - AstIfaceRefDType* const ifaceRefp = VN_AS(pinArrp->subDTypep(), IfaceRefDType); + AstIfaceRefDType* const ifaceRefp + = VN_AS(pinArrp->subDTypep()->skipRefp(), IfaceRefDType); ifaceRefp->cellp(nullptr); varNewp = pinVarp->cloneTree(false); varNewp->name(varNewName); @@ -435,13 +439,14 @@ private: UASSERT_OBJ(VN_IS(slicep->rhsp(), Const), slicep, "Slices should be constant"); const int slice_index = slicep->declRange().left() + in * slicep->declRange().leftToRightInc(); - const auto* const exprArrp = VN_AS(varrefp->dtypep(), UnpackArrayDType); + const auto* const exprArrp + = VN_AS(varrefp->dtypep()->skipRefp(), UnpackArrayDType); UASSERT_OBJ(exprArrp, slicep, "Slice of non-array"); expr_i = slice_index + exprArrp->lo(); } else if (!varrefp) { newp->exprp()->v3error("Unexpected connection to arrayed port"); } else if (const auto* const exprArrp - = VN_CAST(varrefp->dtypep(), UnpackArrayDType)) { + = VN_CAST(varrefp->dtypep()->skipRefp(), UnpackArrayDType)) { expr_i = exprArrp->left() + in * exprArrp->declRange().leftToRightInc(); } @@ -492,9 +497,9 @@ private: void visit(AstNodeAssign* nodep) override { if (AstSliceSel* const arrslicep = VN_CAST(nodep->rhsp(), SliceSel)) { if (const AstUnpackArrayDType* const arrp - = VN_CAST(arrslicep->fromp()->dtypep(), UnpackArrayDType)) { - if (!VN_IS(arrp->subDTypep(), IfaceRefDType)) return; - if (VN_AS(arrp->subDTypep(), IfaceRefDType)->isVirtual()) return; + = VN_CAST(arrslicep->fromp()->dtypep()->skipRefp(), UnpackArrayDType)) { + if (!VN_IS(arrp->subDTypep()->skipRefp(), IfaceRefDType)) return; + if (VN_AS(arrp->subDTypep()->skipRefp(), IfaceRefDType)->isVirtual()) return; arrslicep->v3warn(E_UNSUPPORTED, "Interface slices unsupported"); return; } @@ -557,7 +562,6 @@ private: } //-------------------- - void visit(AstNodeExpr*) override {} // Accelerate void visit(AstNode* nodep) override { iterateChildren(nodep); } void visit(AstNew* nodep) override { iterateChildren(nodep); } void visit(AstMethodCall* nodep) override { iterateChildren(nodep); } diff --git a/test_regress/t/t_interface_array3.py b/test_regress/t/t_interface_array3.py new file mode 100755 index 000000000..bd059b0f2 --- /dev/null +++ b/test_regress/t/t_interface_array3.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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('simulator') + +test.compile(verilator_flags2=['--binary']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_array3.v b/test_regress/t/t_interface_array3.v new file mode 100644 index 000000000..da93b3cf3 --- /dev/null +++ b/test_regress/t/t_interface_array3.v @@ -0,0 +1,54 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +interface my_ifc (); + logic sig; + modport master ( output sig ); + modport slave ( input sig ); +endinterface + +package my_pkg; + typedef virtual my_ifc my_vif; + function void my_func; + input my_vif in_vif; + begin + in_vif.sig = 1'b1; + end + endfunction +endpackage + +module dut (input logic clk, my_ifc.slave sif[2]); + generate + genvar i; + for (i=0; i<2; i++) begin + always_ff @( posedge clk ) begin + if (sif[i].sig == 1'b1) $display("Hello World %0d", i); + end + end + endgenerate +endmodule + +module t; + import my_pkg::*; + + logic clk; + my_ifc sif[2] (); + + dut DUT (.*); + + initial begin + clk = 0; + forever #(5) clk = ~clk; + end + + initial begin + repeat (4) @(posedge clk); + my_func(sif[0]); + my_func(sif[1]); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule