From 12b44f2796bb3924a4234ff286d2eb43d5be2f56 Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Wed, 25 Oct 2023 19:34:48 +0200 Subject: [PATCH] Support `inout` clocking items Signed-off-by: Krzysztof Bieganski --- src/V3AssertPre.cpp | 23 ++++++++------- src/V3Ast.h | 36 +++++++++++++++++++++++ src/V3AstNodeOther.h | 14 ++++----- src/V3LinkDot.cpp | 15 ++++++++++ src/V3LinkParse.cpp | 10 +++---- src/V3Width.cpp | 2 +- src/verilog.y | 22 +++++++------- test_regress/t/t_clocking_inout.pl | 23 +++++++++++++++ test_regress/t/t_clocking_inout.v | 44 ++++++++++++++++++++++++++++ test_regress/t/t_clocking_unsup1.out | 20 +++++-------- test_regress/t/t_clocking_unsup1.v | 4 +-- 11 files changed, 162 insertions(+), 51 deletions(-) create mode 100755 test_regress/t/t_clocking_inout.pl create mode 100644 test_regress/t/t_clocking_inout.v diff --git a/src/V3AssertPre.cpp b/src/V3AssertPre.cpp index 92eda85e5dd..7e26e1dddc9 100644 --- a/src/V3AssertPre.cpp +++ b/src/V3AssertPre.cpp @@ -157,6 +157,13 @@ class AssertPreVisitor final : public VNVisitor { iterateChildren(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)) { @@ -167,11 +174,9 @@ class AssertPreVisitor final : public VNVisitor { AstConst* const skewp = VN_AS(nodep->skewp(), Const); if (skewp->num().isNegative()) skewp->v3error("Skew cannot be negative"); AstNodeExpr* const exprp = nodep->exprp(); - // Get a ref to the sampled/driven variable - AstVar* const varp = nodep->varp()->unlinkFrBack(); - m_clockingp->addVarsp(varp); + m_clockingp->addVarsp(varp->unlinkFrBack()); varp->user1p(nodep); - if (nodep->direction() == VDirection::OUTPUT) { + if (nodep->direction() == VSkewDirection::OUTPUT) { AstVarRef* const skewedRefp = new AstVarRef{flp, varp, VAccess::READ}; skewedRefp->user1(true); AstAssign* const assignp = new AstAssign{flp, exprp->cloneTreePure(false), skewedRefp}; @@ -199,7 +204,7 @@ class AssertPreVisitor final : public VNVisitor { "clocking output skew greater than #0 should be handled"); } } - } else if (nodep->direction() == VDirection::INPUT) { + } else { // Ref to the clockvar AstVarRef* const refp = new AstVarRef{flp, varp, VAccess::WRITE}; refp->user1(true); @@ -245,8 +250,6 @@ class AssertPreVisitor final : public VNVisitor { new AstSenTree{flp, m_clockingp->sensesp()->cloneTree(false)}, popp->makeStmt()}); } - } else { - nodep->v3fatal("Invalid direction"); } pushDeletep(nodep->unlinkFrBack()); } @@ -309,17 +312,17 @@ class AssertPreVisitor final : public VNVisitor { void visit(AstNodeVarRef* nodep) override { if (AstClockingItem* const itemp = VN_CAST(nodep->varp()->user1p(), ClockingItem)) { if (nodep->access().isReadOrRW() && !nodep->user1() - && itemp->direction() == VDirection::OUTPUT) { + && itemp->direction() == VSkewDirection::OUTPUT) { nodep->v3error("Cannot read from output clockvar (IEEE 1800-2017 14.3)"); } if (nodep->access().isWriteOrRW()) { - if (itemp->direction() == VDirection::OUTPUT) { + if (itemp->direction() == VSkewDirection::OUTPUT) { if (!m_inAssignDlyLhs) { nodep->v3error("Only non-blocking assignments can write " "to clockvars (IEEE 1800-2017 14.16)"); } if (m_inAssign) m_inSynchDrive = true; - } else if (!nodep->user1() && itemp->direction() == VDirection::INPUT) { + } else if (!nodep->user1() && itemp->direction() == VSkewDirection::INPUT) { nodep->v3error("Cannot write to input clockvar (IEEE 1800-2017 14.3)"); } } diff --git a/src/V3Ast.h b/src/V3Ast.h index 75cd1e15101..a3585fc3ed1 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -766,6 +766,42 @@ inline std::ostream& operator<<(std::ostream& os, const VDirection& rhs) { return os << rhs.ascii(); } +//###################################################################### + +class VSkewDirection final { +public: + enum en : uint8_t { INPUT, OUTPUT }; + enum en m_e; + // cppcheck-suppress noExplicitConstructor + constexpr VSkewDirection(en _e) + : m_e{_e} {} + explicit VSkewDirection(int _e) + : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning + constexpr operator en() const VL_MT_SAFE { return m_e; } + const char* ascii() const { + static const char* const names[] = {"INPUT", "OUTPUT"}; + return names[m_e]; + } + string verilogKwd() const { + static const char* const names[] = {"input", "output"}; + return names[m_e]; + } + string xmlKwd() const { + static const char* const names[] = {"in", "out"}; + return names[m_e]; + } + string prettyName() const { return verilogKwd(); } + const char* varPrefix() const { return m_e == VSkewDirection::INPUT ? "__Vin_" : "__Vout_"; } +}; +constexpr bool operator==(const VSkewDirection& lhs, const VSkewDirection& rhs) { + return lhs.m_e == rhs.m_e; +} +constexpr bool operator==(const VSkewDirection& lhs, VSkewDirection::en rhs) { return lhs.m_e == rhs; } +constexpr bool operator==(VSkewDirection::en lhs, const VSkewDirection& rhs) { return lhs == rhs.m_e; } +inline std::ostream& operator<<(std::ostream& os, const VSkewDirection& rhs) { + return os << rhs.ascii(); +} + // ###################################################################### /// Boolean or unknown diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index d3f0aee1e8f..d37f3e9f6f3 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -914,21 +914,21 @@ class AstClockingItem final : public AstNode { // @astgen op2 := exprp : Optional[AstNodeExpr] // @astgen op3 := assignp : Optional[AstAssign] // @astgen op4 := varp : Optional[AstVar] - VDirection m_direction; + VSkewDirection m_direction; public: - AstClockingItem(FileLine* fl, VDirection direction, AstNodeExpr* skewp, AstNode* clockingDeclp) - : ASTGEN_SUPER_ClockingItem(fl) { - m_direction = direction; + AstClockingItem(FileLine* fl, VSkewDirection direction, AstNodeExpr* skewp, AstNode* declp) + : ASTGEN_SUPER_ClockingItem(fl) + , m_direction{direction} { this->skewp(skewp); - if (AstAssign* const clkAssignp = VN_CAST(clockingDeclp, Assign)) { + if (AstAssign* const clkAssignp = VN_CAST(declp, Assign)) { this->assignp(clkAssignp); } else { - exprp(VN_AS(clockingDeclp, NodeExpr)); + exprp(VN_AS(declp, NodeExpr)); } } ASTGEN_MEMBERS_AstClockingItem; - VDirection direction() const { return m_direction; } + VSkewDirection direction() const { return m_direction; } }; class AstConstPool final : public AstNode { // Container for const static data diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index fd6854980f3..fd72800bc07 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1182,6 +1182,7 @@ class LinkDotFindVisitor final : public VNVisitor { varname = refp->name(); dtypep = VN_AS(foundp->nodep(), Var)->childDTypep()->cloneTree(false); } + varname = nodep->direction().varPrefix() + varname; AstVar* const newvarp = new AstVar{nodep->fileline(), VVarType::MODULETEMP, varname, VFlagChildDType{}, dtypep}; newvarp->lifetime(VLifetime::STATIC); @@ -2032,6 +2033,7 @@ class LinkDotResolveVisitor final : public VNVisitor { std::set m_extendsParam; // Classes that have a parameterized super class // (except the default instances) // They are added to the set only in linkDotPrimary. + bool m_inAssignLhs = false; // Inside assignment left hand side bool m_insideClassExtParam = false; // Inside a class from m_extendsParam bool m_explicitSuperNew = false; // Hit a "super.new" call inside a "new" function @@ -2045,6 +2047,7 @@ class LinkDotResolveVisitor final : public VNVisitor { AstNode* m_unlinkedScopep; // Unresolved scope, needs corresponding VarXRef bool m_dotErr; // Error found in dotted resolution, ignore upwards string m_dotText; // String of dotted names found in below parseref + string m_refPrefix; // Prefix for parseref name DotStates() { init(nullptr); } ~DotStates() = default; void init(VSymEnt* curSymp) { @@ -2533,8 +2536,18 @@ class LinkDotResolveVisitor final : public VNVisitor { m_inSens = true; iterateChildren(nodep); } + void visit(AstNodeAssign* nodep) override { + { + VL_RESTORER(m_inAssignLhs); + m_inAssignLhs = true; + iterate(nodep->lhsp()); + } + iterateNull(nodep->timingControlp()); + iterate(nodep->rhsp()); + } void visit(AstParseRef* nodep) override { if (nodep->user3SetOnce()) return; + nodep->name(m_ds.m_refPrefix + nodep->name()); UINFO(9, " linkPARSEREF " << m_ds.ascii() << " n=" << nodep << endl); if (m_ds.m_unresolvedClass) return; // m_curSymp is symbol table of outer expression @@ -2827,6 +2840,8 @@ class LinkDotResolveVisitor final : public VNVisitor { m_ds.m_dotText = ""; } } else if (VN_IS(foundp->nodep(), Clocking)) { + VSkewDirection direction = m_inAssignLhs ? VSkewDirection::OUTPUT : VSkewDirection::INPUT; + m_ds.m_refPrefix = direction.varPrefix(); m_ds.m_dotSymp = foundp; ok = m_ds.m_dotPos == DP_SCOPE; } else if (const AstNodeFTask* const ftaskp = VN_CAST(foundp->nodep(), NodeFTask)) { diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index 9c9b349e2f3..e3355925c9f 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -829,13 +829,13 @@ class LinkParseVisitor final : public VNVisitor { nextItemp = VN_AS(itemp->nextp(), ClockingItem); if (itemp->exprp() || itemp->assignp()) continue; if (itemp->skewp()) { - if (itemp->direction() == VDirection::INPUT) { + if (itemp->direction() == VSkewDirection::INPUT) { // Disallow default redefinition; note some simulators allow this if (m_defaultInSkewp) { itemp->skewp()->v3error("Multiple default input skews not allowed"); } m_defaultInSkewp = itemp->skewp(); - } else if (itemp->direction() == VDirection::OUTPUT) { + } else { if (AstConst* const constp = VN_CAST(itemp->skewp(), Const)) { if (constp->num().is1Step()) { itemp->skewp()->v3error("1step not allowed as output skew"); @@ -846,8 +846,6 @@ class LinkParseVisitor final : public VNVisitor { itemp->skewp()->v3error("Multiple default output skews not allowed"); } m_defaultOutSkewp = itemp->skewp(); - } else { - itemp->v3fatalSrc("Incorrect direction"); } } pushDeletep(itemp->unlinkFrBack()); @@ -856,7 +854,7 @@ class LinkParseVisitor final : public VNVisitor { } void visit(AstClockingItem* nodep) override { cleanFileline(nodep); - if (nodep->direction() == VDirection::OUTPUT) { + if (nodep->direction() == VSkewDirection::OUTPUT) { if (!nodep->skewp()) { if (m_defaultOutSkewp) { nodep->skewp(m_defaultOutSkewp->cloneTree(false)); @@ -869,7 +867,7 @@ class LinkParseVisitor final : public VNVisitor { nodep->skewp()->v3error("1step not allowed as output skew"); } } - } else if (nodep->direction() == VDirection::INPUT) { + } else { if (!nodep->skewp()) { if (m_defaultInSkewp) { nodep->skewp(m_defaultInSkewp->cloneTree(false)); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 30407d90d65..376b297e318 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -5698,7 +5698,7 @@ class WidthVisitor final : public VNVisitor { } void visit(AstClockingItem* nodep) override { nodep->exprp()->foreach([nodep](AstVarRef* const refp) { - refp->access(nodep->direction().isWritable() ? VAccess::WRITE : VAccess::READ); + refp->access(nodep->direction() == VSkewDirection::OUTPUT ? VAccess::WRITE : VAccess::READ); }); userIterateChildren(nodep, WidthVP{SELF, PRELIM}.p()); } diff --git a/src/verilog.y b/src/verilog.y index 3d553f66756..a282ba3320c 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -262,7 +262,7 @@ public: } } // Given a list of clocking declarations, put them in clocking items - AstClockingItem* makeClockingItemList(FileLine* flp, const VDirection direction, + AstClockingItem* makeClockingItemList(FileLine* flp, const VSkewDirection direction, AstNodeExpr* skewp, AstNode* const clockingDeclps) { AstClockingItem* itemsp = nullptr; for (AstNode *nodep = clockingDeclps, *nextp; nodep; nodep = nextp) { @@ -5754,21 +5754,21 @@ clocking_itemList: // IEEE: [ clocking_item ] ; clocking_item: // IEEE: clocking_item - yDEFAULT yINPUT clocking_skew ';' { $$ = new AstClockingItem{$1, VDirection::INPUT, $3, nullptr}; } - | yDEFAULT yOUTPUT clocking_skew ';' { $$ = new AstClockingItem{$1, VDirection::OUTPUT, $3, nullptr}; } + yDEFAULT yINPUT clocking_skew ';' { $$ = new AstClockingItem{$1, VSkewDirection::INPUT, $3, nullptr}; } + | yDEFAULT yOUTPUT clocking_skew ';' { $$ = new AstClockingItem{$1, VSkewDirection::OUTPUT, $3, nullptr}; } | yDEFAULT yINPUT clocking_skew yOUTPUT clocking_skew ';' - { $$ = new AstClockingItem{$1, VDirection::INPUT, $3, nullptr}; - $$->addNext(new AstClockingItem{$4, VDirection::OUTPUT, $5, nullptr}); } + { $$ = new AstClockingItem{$1, VSkewDirection::INPUT, $3, nullptr}; + $$->addNext(new AstClockingItem{$4, VSkewDirection::OUTPUT, $5, nullptr}); } | yINPUT clocking_skewE list_of_clocking_decl_assign ';' - { $$ = GRAMMARP->makeClockingItemList($1, VDirection::INPUT, $2, $3); } + { $$ = GRAMMARP->makeClockingItemList($1, VSkewDirection::INPUT, $2, $3); } | yOUTPUT clocking_skewE list_of_clocking_decl_assign ';' - { $$ = GRAMMARP->makeClockingItemList($1, VDirection::OUTPUT, $2, $3); } + { $$ = GRAMMARP->makeClockingItemList($1, VSkewDirection::OUTPUT, $2, $3); } | yINPUT clocking_skewE yOUTPUT clocking_skewE list_of_clocking_decl_assign ';' - { $$ = nullptr; - BBUNSUP($5, "Unsupported: Mixed input/output clocking items"); } + { $$ = GRAMMARP->makeClockingItemList($1, VSkewDirection::INPUT, $2, $5->cloneTree(true)); + $$->addNext(GRAMMARP->makeClockingItemList($1, VSkewDirection::OUTPUT, $4, $5)); } | yINOUT list_of_clocking_decl_assign ';' - { $$ = nullptr; - BBUNSUP($1, "Unsupported: 'inout' clocking items"); } + { $$ = GRAMMARP->makeClockingItemList($1, VSkewDirection::INPUT, nullptr, $2->cloneTree(true)); + $$->addNext(GRAMMARP->makeClockingItemList($1, VSkewDirection::OUTPUT, nullptr, $2)); } | assertion_item_declaration { $$ = nullptr; BBUNSUP($1, "Unsupported: assertion items in clocking blocks"); } diff --git a/test_regress/t/t_clocking_inout.pl b/test_regress/t/t_clocking_inout.pl new file mode 100755 index 00000000000..b8493bd062a --- /dev/null +++ b/test_regress/t/t_clocking_inout.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 2023 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( + verilator_flags2 => ["--exe --main --timing"], + make_main => 0, + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_clocking_inout.v b/test_regress/t/t_clocking_inout.v new file mode 100644 index 00000000000..885c26e0a3d --- /dev/null +++ b/test_regress/t/t_clocking_inout.v @@ -0,0 +1,44 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +module t; + logic clk = 0, foo = 0, bar = 0; + + always #5 clk = ~clk; + + clocking cb @(posedge clk); + input #11 output #2 foo; + inout bar; + endclocking + + initial begin + cb.foo <= 1; + cb.bar <= 1; + if (foo != 0 || cb.foo != 0) $stop; + if (bar != 0 || cb.bar != 0) $stop; + + @(posedge bar) + if ($time != 5) $stop; + if (foo != 0 || cb.foo != 0) $stop; + if (cb.bar != 0) $stop; + + #1 + if (foo != 0 || cb.foo != 0) $stop; + if (cb.bar != 1) $stop; + + @(posedge foo) + if ($time != 7) $stop; + if (cb.foo != 0) $stop; + + #9 // $time == 16 + if (cb.foo != 0) $stop; + + #10 // $time == 26 + if (cb.foo != 1) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_clocking_unsup1.out b/test_regress/t/t_clocking_unsup1.out index 164271f819e..9904a7a65bd 100644 --- a/test_regress/t/t_clocking_unsup1.out +++ b/test_regress/t/t_clocking_unsup1.out @@ -1,17 +1,11 @@ -%Error-UNSUPPORTED: t/t_clocking_unsup1.v:14:26: Unsupported: Mixed input/output clocking items - 14 | input #1 output #1step x; - | ^ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_clocking_unsup1.v:15:8: Unsupported: 'inout' clocking items - 15 | inout y; - | ^~~~~ -%Error-UNSUPPORTED: t/t_clocking_unsup1.v:16:15: Unsupported: clocking event edge override - 16 | output posedge #1 a; +%Error-UNSUPPORTED: t/t_clocking_unsup1.v:14:15: Unsupported: clocking event edge override + 14 | output posedge #1 a; | ^~~~~~~ -%Error-UNSUPPORTED: t/t_clocking_unsup1.v:17:15: Unsupported: clocking event edge override - 17 | output negedge #1 b; + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error-UNSUPPORTED: t/t_clocking_unsup1.v:15:15: Unsupported: clocking event edge override + 15 | output negedge #1 b; | ^~~~~~~ -%Error-UNSUPPORTED: t/t_clocking_unsup1.v:18:15: Unsupported: clocking event edge override - 18 | output edge #1 b; +%Error-UNSUPPORTED: t/t_clocking_unsup1.v:16:15: Unsupported: clocking event edge override + 16 | output edge #1 b; | ^~~~ %Error: Exiting due to diff --git a/test_regress/t/t_clocking_unsup1.v b/test_regress/t/t_clocking_unsup1.v index 7315e2e636c..a15a9212d81 100644 --- a/test_regress/t/t_clocking_unsup1.v +++ b/test_regress/t/t_clocking_unsup1.v @@ -10,9 +10,7 @@ module t(/*AUTOARG*/ ); input clk; - global clocking cb @(posedge clk); - input #1 output #1step x; - inout y; + clocking cb @(posedge clk); output posedge #1 a; output negedge #1 b; output edge #1 b;