From fc52fb90930eecb93cc69695045463fa38ebf3fb Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 8 Nov 2020 22:43:32 -0500 Subject: [PATCH] Fix arrays of modport interfaces (#2614). --- Changes | 2 + src/V3Inst.cpp | 3 ++ src/V3LinkDot.cpp | 61 +++++++++++---------------- src/V3LinkResolve.cpp | 10 +++++ src/V3Scope.cpp | 1 + test_regress/t/t_interface_ar2a.pl | 17 ++++++++ test_regress/t/t_interface_ar2a.v | 30 ++++++++++++++ test_regress/t/t_interface_ar2b.pl | 17 ++++++++ test_regress/t/t_interface_ar2b.v | 35 ++++++++++++++++ test_regress/t/t_interface_ar3.out | 9 ++++ test_regress/t/t_interface_ar3.pl | 23 +++++++++++ test_regress/t/t_interface_ar3.v | 66 ++++++++++++++++++++++++++++++ 12 files changed, 238 insertions(+), 36 deletions(-) create mode 100755 test_regress/t/t_interface_ar2a.pl create mode 100644 test_regress/t/t_interface_ar2a.v create mode 100755 test_regress/t/t_interface_ar2b.pl create mode 100644 test_regress/t/t_interface_ar2b.v create mode 100644 test_regress/t/t_interface_ar3.out create mode 100755 test_regress/t/t_interface_ar3.pl create mode 100644 test_regress/t/t_interface_ar3.v diff --git a/Changes b/Changes index ca1e6bff2..9dc1d5344 100644 --- a/Changes +++ b/Changes @@ -28,6 +28,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix queue poping wrong value when otherwise unused (#2512). [nanduraj1] +**** Fix arrays of modport interfaces (#2614). [Thierry Tambe] + * Verilator 4.102 2020-10-15 diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index a7dc03ee8..88e6899c3 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -377,7 +377,10 @@ private: return; } string index = AstNode::encodeNumber(constp->toSInt()); + if (VN_IS(arrselp->lhsp(), SliceSel)) + arrselp->lhsp()->v3error("Unsupported: interface slices"); AstVarRef* varrefp = VN_CAST(arrselp->lhsp(), VarRef); + UASSERT_OBJ(varrefp, arrselp, "No interface varref under array"); AstVarXRef* newp = new AstVarXRef(nodep->fileline(), varrefp->name() + "__BRA__" + index + "__KET__", "", VAccess::WRITE); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index a0af16b9c..8274b84fd 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -46,6 +46,8 @@ // #8: Insert modport's symbols under IfaceRefDType (after #7) // ResolveVisitor: // #9: Resolve general variables, which may point into the interface or modport (after #8) +// LinkResolve: +// #10: Unlink modports, not needed later except for XML/Lint //************************************************************************* // TOP // {name-of-top-modulename} @@ -1865,20 +1867,12 @@ private: m_ds.m_dotErr = true; } } - AstVar* makeIfaceModportVar(FileLine* fl, AstCell* cellp, AstIface* ifacep, - AstModport* modportp, AstRange* rangep) { - // Create iface variable, using duplicate var when under same module scope - string varName - = cellp->name() + "__Vmp__" + modportp->name() + "__Viftop" + cvtToStr(++m_modportNum); - AstIfaceRefDType* idtypep = new AstIfaceRefDType(fl, modportp->fileline(), cellp->name(), - ifacep->name(), modportp->name()); - idtypep->cellp(cellp); - AstNodeDType* vdtypep = idtypep; - if (rangep) vdtypep = new AstUnpackArrayDType(fl, VFlagChildDType(), vdtypep, rangep); - AstVar* varp = new AstVar(fl, AstVarType::IFACEREF, varName, VFlagChildDType(), vdtypep); - varp->isIfaceParent(true); - m_modp->addStmtp(varp); - return varp; + AstVar* findIfaceTopVarp(AstNode* nodep, VSymEnt* parentEntp, const string& name) { + string findName = name + "__Viftop"; + VSymEnt* ifaceSymp = parentEntp->findIdFallback(findName); + AstVar* ifaceTopVarp = ifaceSymp ? VN_CAST(ifaceSymp->nodep(), Var) : nullptr; + UASSERT_OBJ(ifaceTopVarp, nodep, "Can't find interface var ref: " << findName); + return ifaceTopVarp; } void markAndCheckPinDup(AstNode* nodep, AstNode* refp, const char* whatp) { if (refp->user5p() && refp->user5p() != nodep) { @@ -2173,12 +2167,7 @@ private: VSymEnt* parentEntp = cellEntp->parentp(); // Container of the var; probably a module or // generate begin - string findName = nodep->name() + "__Viftop"; - VSymEnt* ifaceSymp = parentEntp->findIdFallback(findName); - AstVar* ifaceRefVarp - = ifaceSymp ? VN_CAST(ifaceSymp->nodep(), Var) : nullptr; - UASSERT_OBJ(ifaceRefVarp, nodep, - "Can't find interface var ref: " << findName); + AstVar* ifaceRefVarp = findIfaceTopVarp(nodep, parentEntp, nodep->name()); // ok = true; m_ds.m_dotText = VString::dot(m_ds.m_dotText, ".", nodep->name()); @@ -2273,32 +2262,32 @@ private: } else { AstCell* cellp = VN_CAST(m_ds.m_dotSymp->nodep(), Cell); UASSERT_OBJ(cellp, nodep, "Modport not referenced from a cell"); - AstIface* ifacep = VN_CAST(cellp->modp(), Iface); - // string cellName = m_ds.m_dotText; // Use cellp->name - m_ds.m_dotText = VString::dot(m_ds.m_dotText, ".", nodep->name()); - m_ds.m_dotSymp = m_statep->getNodeSym(modportp); - m_ds.m_dotPos = DP_SCOPE; + VSymEnt* cellEntp = m_statep->getNodeSym(cellp); + UASSERT_OBJ(cellEntp, nodep, "No interface sym entry"); + VSymEnt* parentEntp = cellEntp->parentp(); // Container of the var; probably a + // module or generate begin + // We drop __BRA__??__KET__ as cells don't have that naming yet + AstVar* ifaceRefVarp = findIfaceTopVarp(nodep, parentEntp, cellp->name()); + // ok = true; + m_ds.m_dotText = VString::dot(m_ds.m_dotText, ".", nodep->name()); + m_ds.m_dotSymp = foundp; + m_ds.m_dotPos = DP_SCOPE; + UINFO(9, " cell -> iface varref " << foundp->nodep() << endl); + AstNode* newp + = new AstVarRef(ifaceRefVarp->fileline(), ifaceRefVarp, VAccess::READ); auto* cellarrayrefp = VN_CAST(m_ds.m_unlinkedScopep, CellArrayRef); - AstRange* rangep = nullptr; - if (cellarrayrefp) - rangep = new AstRange(nodep->fileline(), - cellarrayrefp->selp()->cloneTree(true), - cellarrayrefp->selp()->cloneTree(true)); - AstVar* varp - = makeIfaceModportVar(nodep->fileline(), cellp, ifacep, modportp, rangep); - AstNode* refp = new AstVarRef(varp->fileline(), varp, VAccess::READ); if (cellarrayrefp) { // iface[vec].modport became CellArrayRef(iface, lsb) // Convert back to SelBit(iface, lsb) UINFO(9, " Array modport to SelBit " << cellarrayrefp << endl); - refp = new AstSelBit(cellarrayrefp->fileline(), refp, + newp = new AstSelBit(cellarrayrefp->fileline(), newp, cellarrayrefp->selp()->unlinkFrBack()); - refp->user3(true); // Don't process again + newp->user3(true); // Don't process again VL_DO_DANGLING(cellarrayrefp->unlinkFrBack(), cellarrayrefp); m_ds.m_unlinkedScopep = nullptr; } - nodep->replaceWith(refp); + nodep->replaceWith(newp); VL_DO_DANGLING(pushDeletep(nodep), nodep); } } else if (AstEnumItem* valuep = VN_CAST(foundp->nodep(), EnumItem)) { diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp index 1f7fdaff7..bd7286e9e 100644 --- a/src/V3LinkResolve.cpp +++ b/src/V3LinkResolve.cpp @@ -502,6 +502,16 @@ private: iterateChildren(nodep); } + virtual void visit(AstIfaceRefDType* nodep) override { + // LinkDot checked modports, now remove references to them + // Keeping them later caused problems with InstDeArray, + // as it needed to make new modport arrays and such + nodep->modportp(nullptr); + iterateChildren(nodep); + } + // virtual void visit(AstModport* nodep) override { ... } + // We keep Modport's themselves around for XML dump purposes + virtual void visit(AstNode* nodep) override { iterateChildren(nodep); } public: diff --git a/src/V3Scope.cpp b/src/V3Scope.cpp index 7e54b0a30..9e3d19886 100644 --- a/src/V3Scope.cpp +++ b/src/V3Scope.cpp @@ -377,6 +377,7 @@ private: iterateChildren(nodep); } virtual void visit(AstModportFTaskRef* nodep) override { + // The modport persists only for xml dump // The crossrefs are dealt with in V3LinkDot nodep->ftaskp(nullptr); iterateChildren(nodep); diff --git a/test_regress/t/t_interface_ar2a.pl b/test_regress/t/t_interface_ar2a.pl new file mode 100755 index 000000000..552bd97db --- /dev/null +++ b/test_regress/t/t_interface_ar2a.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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 + +scenarios(simulator => 1); + +compile( + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_ar2a.v b/test_regress/t/t_interface_ar2a.v new file mode 100644 index 000000000..f0c406843 --- /dev/null +++ b/test_regress/t/t_interface_ar2a.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: SystemVerilog interface test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Thierry Tambe. +// SPDX-License-Identifier: CC0-1.0 + +module t (); + + ahb_slave_intf AHB_S[1](); + + AHB_MEM uMEM(.S(AHB_S[0].source)); +// AHB_MEM V_MEM(.S(AHB_S[0])); + +endmodule + +module AHB_MEM + ( + ahb_slave_intf.source S + ); + +endmodule + +interface ahb_slave_intf + (); + + logic [31:0] HADDR; + + modport source (input HADDR); + +endinterface diff --git a/test_regress/t/t_interface_ar2b.pl b/test_regress/t/t_interface_ar2b.pl new file mode 100755 index 000000000..552bd97db --- /dev/null +++ b/test_regress/t/t_interface_ar2b.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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 + +scenarios(simulator => 1); + +compile( + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_ar2b.v b/test_regress/t/t_interface_ar2b.v new file mode 100644 index 000000000..b14ae8cbc --- /dev/null +++ b/test_regress/t/t_interface_ar2b.v @@ -0,0 +1,35 @@ +// DESCRIPTION: Verilator: SystemVerilog interface test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Thierry Tambe. +// SPDX-License-Identifier: CC0-1.0 + +module t (); + + sub sub [1] (); + + ahb_slave_intf AHB_S[1](); + + AHB_MEM uMEM(.S(AHB_S[0])); +// AHB_MEM uMEM(.S(AHB_S[0].source)); + +endmodule + +module sub; +endmodule + +module AHB_MEM + ( + ahb_slave_intf.source S + ); + +endmodule + +interface ahb_slave_intf + (); + + logic [31:0] HADDR; + + modport source (input HADDR); + +endinterface diff --git a/test_regress/t/t_interface_ar3.out b/test_regress/t/t_interface_ar3.out new file mode 100644 index 000000000..e28530a56 --- /dev/null +++ b/test_regress/t/t_interface_ar3.out @@ -0,0 +1,9 @@ +%Error: t/t_interface_ar3.v:16:36: Unsupported: interface slices + : ... In instance t + 16 | sub sub01 [2] (.clk, .infc(iinst[0:1])); + | ^ +%Error: Internal Error: t/t_interface_ar3.v:16:36: ../V3Inst.cpp:#: No interface varref under array + : ... In instance t + 16 | sub sub01 [2] (.clk, .infc(iinst[0:1])); + | ^ + ... See the manual and https://verilator.org for more assistance. diff --git a/test_regress/t/t_interface_ar3.pl b/test_regress/t/t_interface_ar3.pl new file mode 100755 index 000000000..8d48ddb75 --- /dev/null +++ b/test_regress/t/t_interface_ar3.pl @@ -0,0 +1,23 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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 + +scenarios(simulator => 1); + +compile( + fails => $Self->{vlt_all}, # Verilator unsupported, bug546 + expect_filename => $Self->{golden_filename}, + ); + +execute( + check_finished => 1, + ) if !$Self->{vlt_all}; + +ok(1); +1; diff --git a/test_regress/t/t_interface_ar3.v b/test_regress/t/t_interface_ar3.v new file mode 100644 index 000000000..74416a300 --- /dev/null +++ b/test_regress/t/t_interface_ar3.v @@ -0,0 +1,66 @@ +// DESCRIPTION: Verilator: SystemVerilog interface test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Thierry Tambe. +// SPDX-License-Identifier: CC0-1.0 + +module t ( + input logic clk, + output logic HRESETn + ); + + int primsig[3]; + + ahb_slave_intf iinst[3] (primsig[2:0]); + + sub sub01 [2] (.clk, .infc(iinst[0:1])); + sub sub2 (.clk, .infc(iinst[2])); + + initial begin + primsig[0] = 30; + primsig[1] = 31; + primsig[2] = 32; + iinst[0].data = 10; + iinst[1].data = 11; + iinst[2].data = 12; + end + + int cyc = 0; + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc == 10) begin + if (iinst[0].primsig != 30) $stop; + if (iinst[1].primsig != 31) $stop; + if (iinst[2].primsig != 32) $stop; + if (iinst[0].data != 10) $stop; + if (iinst[1].data != 11) $stop; + if (iinst[2].data != 12) $stop; + if (sub01[0].internal != 10) $stop; + if (sub01[1].internal != 11) $stop; + if (sub2.internal != 12) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule + +module sub +( + input logic clk, + ahb_slave_intf infc + ); + + int internal; + + always_comb internal = infc.data; + +endmodule + +interface ahb_slave_intf + ( + input int primsig + ); + + int data; + +endinterface