From c856380fac719d65aeb243483ced1d3186872e2f Mon Sep 17 00:00:00 2001 From: Ryszard Rozak Date: Tue, 16 Sep 2025 19:25:40 +0200 Subject: [PATCH] Support modports referencing clocking blocks (#4555) (#6436) --- src/V3AssertPre.cpp | 20 ++++++-- src/V3AstNodeOther.h | 20 ++++++++ src/V3AstNodes.cpp | 9 ++++ src/V3LinkDot.cpp | 17 +++++++ src/verilog.y | 3 +- test_regress/t/t_dist_warn_coverage.py | 3 -- test_regress/t/t_mod_interface_clocking.py | 18 +++++++ test_regress/t/t_mod_interface_clocking.v | 51 +++++++++++++++++++ .../t/t_mod_interface_clocking_bad.out | 8 ++- test_regress/t/t_mod_interface_clocking_bad.v | 2 +- 10 files changed, 141 insertions(+), 10 deletions(-) create mode 100755 test_regress/t/t_mod_interface_clocking.py create mode 100644 test_regress/t/t_mod_interface_clocking.v diff --git a/src/V3AssertPre.cpp b/src/V3AssertPre.cpp index b5ac1dacb..d789a8bbd 100644 --- a/src/V3AssertPre.cpp +++ b/src/V3AssertPre.cpp @@ -38,6 +38,7 @@ class AssertPreVisitor final : public VNVisitor { // We're not parsing the tree, or anything more complicated. private: // NODE STATE + // AstClockingItem::user1p() // AstVar*. varp() of ClockingItem after unlink const VNUser1InUse m_inuser1; // STATE // Current context: @@ -156,19 +157,32 @@ private: if (nodep->eventp()) nodep->addNextHere(nodep->eventp()->unlinkFrBack()); VL_DO_DANGLING(pushDeletep(nodep->unlinkFrBack()), nodep); } + void visit(AstModportClockingRef* const nodep) override { + // It has to be converted to a list of ModportClockingVarRefs, + // because clocking blocks are removed in this pass + for (AstClockingItem* itemp = nodep->clockingp()->itemsp(); itemp; + itemp = VN_AS(itemp->nextp(), ClockingItem)) { + AstVar* const varp = itemp->varp() ? itemp->varp() : VN_AS(itemp->user1p(), Var); + if (varp) { + AstModportVarRef* const modVarp + = new AstModportVarRef{nodep->fileline(), varp->name(), itemp->direction()}; + modVarp->varp(varp); + nodep->addNextHere(modVarp); + } + } + VL_DO_DANGLING(pushDeletep(nodep->unlinkFrBack()), nodep); + } void visit(AstClockingItem* const nodep) override { // Get a ref to the sampled/driven variable AstVar* const varp = nodep->varp(); if (!varp) { // Unused item - pushDeletep(nodep->unlinkFrBack()); return; } FileLine* const flp = nodep->fileline(); V3Const::constifyEdit(nodep->skewp()); if (!VN_IS(nodep->skewp(), Const)) { nodep->skewp()->v3error("Skew must be constant (IEEE 1800-2023 14.4)"); - VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); return; } AstConst* const skewp = VN_AS(nodep->skewp(), Const); @@ -176,6 +190,7 @@ private: AstNodeExpr* const exprp = nodep->exprp(); varp->name(m_clockingp->name() + "__DOT__" + varp->name()); m_clockingp->addNextHere(varp->unlinkFrBack()); + nodep->user1p(varp); varp->user1p(nodep); if (nodep->direction() == VDirection::OUTPUT) { exprp->foreach([](const AstNodeVarRef* varrefp) { @@ -292,7 +307,6 @@ private: } else { nodep->v3fatalSrc("Invalid direction"); } - VL_DO_DANGLING(pushDeletep(nodep->unlinkFrBack()), nodep); } void visit(AstDelay* nodep) override { // Only cycle delays are relevant in this stage; also only process once diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index f2f7ff1a7..e8ba13c46 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -826,6 +826,7 @@ public: addItemsp(itemsp); } ASTGEN_MEMBERS_AstClocking; + bool maybePointedTo() const override VL_MT_SAFE { return true; } void dump(std::ostream& str) const override; void dumpJson(std::ostream& str) const override; std::string name() const override VL_MT_STABLE { return m_name; } @@ -1198,6 +1199,25 @@ public: bool maybePointedTo() const override VL_MT_SAFE { return true; } ASTGEN_MEMBERS_AstModport; }; +class AstModportClockingRef final : public AstNode { + // A clocking block referenced under a modport + // The storage for the clocking block itself is inside the interface, thus this is a reference + // PARENT: AstModport + // + // @astgen ptr := m_clockingp : Optional[AstClocking] // Link to the actual clocking block + string m_name; // Name of the clocking block referenced +public: + AstModportClockingRef(FileLine* fl, const string& name) + : ASTGEN_SUPER_ModportClockingRef(fl) + , m_name{name} {} + ASTGEN_MEMBERS_AstModportClockingRef; + void dump(std::ostream& str) const override; + string name() const override VL_MT_STABLE { return m_name; } + AstClocking* clockingp() const VL_MT_STABLE { + return m_clockingp; + } // [After Link] Pointer to clocking block + void clockingp(AstClocking* clockingp) { m_clockingp = clockingp; } +}; class AstModportFTaskRef final : public AstNode { // An import/export referenced under a modport // The storage for the function itself is inside the diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 7b02d6f77..a2d8c59b6 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2082,6 +2082,15 @@ void AstMemberSel::dump(std::ostream& str) const { } } void AstMemberSel::dumpJson(std::ostream& str) const { dumpJsonGen(str); } +void AstModportClockingRef::dump(std::ostream& str) const { + this->AstNode::dump(str); + if (clockingp()) { + str << " -> "; + clockingp()->dump(str); + } else { + str << " -> UNLINKED"; + } +} void AstModportFTaskRef::dump(std::ostream& str) const { this->AstNode::dump(str); if (isExport()) str << " EXPORT"; diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index b97d192a9..d177208d8 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -2404,6 +2404,19 @@ class LinkDotIfaceVisitor final : public VNVisitor { VL_DO_DANGLING(pushDeletep(nodep), nodep); } } + void visit(AstModportClockingRef* nodep) override { // IfaceVisitor:: + UINFO(5, " fic: " << nodep); + iterateChildren(nodep); + VSymEnt* const symp = m_curSymp->findIdFallback(nodep->name()); + if (!symp) { + nodep->v3error("Modport item not found: " << nodep->prettyNameQ()); + } else if (AstClocking* const clockingp = VN_CAST(symp->nodep(), Clocking)) { + nodep->clockingp(clockingp); + m_statep->insertSym(m_curSymp, nodep->name(), nodep, nullptr /*package*/); + } else { + nodep->v3error("Modport item is not a clocking block: " << nodep->prettyNameQ()); + } + } void visit(AstNode* nodep) override { iterateChildren(nodep); } // IfaceVisitor:: public: @@ -3963,7 +3976,11 @@ class LinkDotResolveVisitor final : public VNVisitor { dotSymp = m_statep->getNodeSym(ifaceRefp->ifacep()); } } + } else if (const AstModportClockingRef* const clockingRefp + = VN_CAST(dotSymp->nodep(), ModportClockingRef)) { + dotSymp = m_statep->getNodeSym(clockingRefp->clockingp()); } + if (!m_statep->forScopeCreation()) { VSymEnt* foundp = nullptr; if (modport) { diff --git a/src/verilog.y b/src/verilog.y index f0d41d541..773b68964 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1655,8 +1655,7 @@ modportPortsDecl: port_direction modportSimplePortOrTFPort { $$ = new AstModportVarRef{$2, *$2, GRAMMARP->m_varIO}; GRAMMARP->m_modportImpExpActive = false;} // // IEEE: modport_clocking_declaration - | yCLOCKING idAny/*clocking_identifier*/ - { $$ = nullptr; BBUNSUP($1, "Unsupported: Modport clocking"); } + | yCLOCKING idAny/*clocking_identifier*/ { $$ = new AstModportClockingRef{$1, *$2}; } // // IEEE: yIMPORT modport_tf_port // // IEEE: yEXPORT modport_tf_port // // modport_tf_port expanded here diff --git a/test_regress/t/t_dist_warn_coverage.py b/test_regress/t/t_dist_warn_coverage.py index 8ba8ac79a..e43639dc6 100755 --- a/test_regress/t/t_dist_warn_coverage.py +++ b/test_regress/t/t_dist_warn_coverage.py @@ -51,7 +51,6 @@ for s in [ 'Interface port declaration ', 'Modport item is not a function/task: ', 'Modport item is not a variable: ', - 'Modport item not found: ', 'Modport not referenced as .', 'Modport not referenced from underneath an interface: ', 'Non-interface used as an interface: ', @@ -77,7 +76,6 @@ for s in [ 'Unsupported: 4-state numbers in this context', 'Unsupported: Bind with instance list', 'Unsupported: Concatenation to form ', - 'Unsupported: Modport clocking', 'Unsupported: Modport dotted port name', 'Unsupported: Modport export with prototype', 'Unsupported: Modport import with prototype', @@ -105,7 +103,6 @@ for s in [ 'Unsupported: repeat event control', 'Unsupported: static cast to ', 'Unsupported: super', - 'Unsupported: this.super', 'Unsupported: with[] stream expression', ]: Suppressed[s] = True diff --git a/test_regress/t/t_mod_interface_clocking.py b/test_regress/t/t_mod_interface_clocking.py new file mode 100755 index 000000000..c53b55262 --- /dev/null +++ b/test_regress/t/t_mod_interface_clocking.py @@ -0,0 +1,18 @@ +#!/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('simulator') + +test.compile(verilator_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_mod_interface_clocking.v b/test_regress/t/t_mod_interface_clocking.v new file mode 100644 index 000000000..22e6c7161 --- /dev/null +++ b/test_regress/t/t_mod_interface_clocking.v @@ -0,0 +1,51 @@ +// 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 axi_if; + logic clk; + wire rlast; + wire rvalid; + clocking cb @(posedge clk); + inout rlast, rvalid; + endclocking + modport md1(clocking cb, inout clk, rlast, rvalid); + modport md2(clocking cb); +endinterface + +module sub ( + axi_if.md1 axi1, + axi_if.md2 axi2 +); + initial begin + axi1.clk = 1'b0; + #1 axi1.clk = 1'b1; + #1 axi1.clk = 1'b0; + #1 axi1.clk = 1'b1; + end + initial begin + @(negedge axi1.rvalid); + $display("[%0t] rvalid==%b", $time, axi1.rvalid); + $display("[%0t] rlast is 1: ", $time, axi1.rlast === 1); + if (axi1.rlast === 1) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + initial begin + $display("[%0t] rvalid <= 1", $time); + axi1.cb.rvalid <= 1'b1; + @(posedge axi1.rvalid); + $display("[%0t] rvalid <= 0", $time); + axi1.cb.rvalid <= 1'b0; + @(negedge axi1.clk); + $display("[%0t] rlast <= 1", $time); + axi2.cb.rlast <= 1'b1; + end +endmodule + +module t; + axi_if axi_vi (); + sub i_sub (.axi1(axi_vi), .axi2(axi_vi)); +endmodule diff --git a/test_regress/t/t_mod_interface_clocking_bad.out b/test_regress/t/t_mod_interface_clocking_bad.out index a4dc0a653..017459e00 100644 --- a/test_regress/t/t_mod_interface_clocking_bad.out +++ b/test_regress/t/t_mod_interface_clocking_bad.out @@ -1,5 +1,11 @@ +%Error: t/t_mod_interface_clocking_bad.v:16:25: Modport item is not a clocking block: 'reset' + 16 | modport mp(input clk, clocking reset, clocking cx); + | ^~~~~~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_mod_interface_clocking_bad.v:16:41: Modport item not found: 'cx' + 16 | modport mp(input clk, clocking reset, clocking cx); + | ^~~~~~~~ %Error: t/t_mod_interface_clocking_bad.v:25:10: Can't find definition of 'cb' 25 | x.cb.reset <= 1; | ^~~~~ - ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. %Error: Exiting due to diff --git a/test_regress/t/t_mod_interface_clocking_bad.v b/test_regress/t/t_mod_interface_clocking_bad.v index 806977f2c..ee19b389a 100644 --- a/test_regress/t/t_mod_interface_clocking_bad.v +++ b/test_regress/t/t_mod_interface_clocking_bad.v @@ -13,7 +13,7 @@ interface mem_if ( output reset; endclocking - modport mp(input clk); + modport mp(input clk, clocking reset, clocking cx); endinterface