Support inout clocking items

Signed-off-by: Krzysztof Bieganski <[email protected]>
antmicro · Oct 26, 2023 · 12b44f2 · 12b44f2
1 parent 7660043
commit 12b44f2
Show file tree

Hide file tree

Showing 11 changed files with 162 additions and 51 deletions.
diff --git 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
@@ -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<en>(_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
@@ -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
@@ -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<AstClass*> 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
@@ -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
@@ -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
@@ -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<clockingItemp>:  // IEEE: [ clocking_item ]
         ;
 
 clocking_item<clockingItemp>:   // IEEE: clocking_item
-                yDEFAULT yINPUT clocking_skew ';'       { $$ = new AstClockingItem{$<fl>1, VDirection::INPUT, $3, nullptr}; }
-        |       yDEFAULT yOUTPUT clocking_skew ';'      { $$ = new AstClockingItem{$<fl>1, VDirection::OUTPUT, $3, nullptr}; }
+                yDEFAULT yINPUT clocking_skew ';'       { $$ = new AstClockingItem{$<fl>1, VSkewDirection::INPUT, $3, nullptr}; }
+        |       yDEFAULT yOUTPUT clocking_skew ';'      { $$ = new AstClockingItem{$<fl>1, VSkewDirection::OUTPUT, $3, nullptr}; }
         |       yDEFAULT yINPUT clocking_skew yOUTPUT clocking_skew ';'
-                        { $$ = new AstClockingItem{$<fl>1, VDirection::INPUT, $3, nullptr};
-                          $$->addNext(new AstClockingItem{$<fl>4, VDirection::OUTPUT, $5, nullptr}); }
+                        { $$ = new AstClockingItem{$<fl>1, VSkewDirection::INPUT, $3, nullptr};
+                          $$->addNext(new AstClockingItem{$<fl>4, VSkewDirection::OUTPUT, $5, nullptr}); }
         |       yINPUT clocking_skewE list_of_clocking_decl_assign ';'
-                        { $$ = GRAMMARP->makeClockingItemList($<fl>1, VDirection::INPUT, $2, $3); }
+                        { $$ = GRAMMARP->makeClockingItemList($<fl>1, VSkewDirection::INPUT, $2, $3); }
         |       yOUTPUT clocking_skewE list_of_clocking_decl_assign ';'
-                        { $$ = GRAMMARP->makeClockingItemList($<fl>1, VDirection::OUTPUT, $2, $3); }
+                        { $$ = GRAMMARP->makeClockingItemList($<fl>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($<fl>1, VSkewDirection::INPUT, $2, $5->cloneTree(true));
+                          $$->addNext(GRAMMARP->makeClockingItemList($<fl>1, VSkewDirection::OUTPUT, $4, $5)); }
         |       yINOUT list_of_clocking_decl_assign ';'
-                        { $$ = nullptr;
-                          BBUNSUP($1, "Unsupported: 'inout' clocking items"); }
+                        { $$ = GRAMMARP->makeClockingItemList($<fl>1, VSkewDirection::INPUT, nullptr, $2->cloneTree(true));
+                          $$->addNext(GRAMMARP->makeClockingItemList($<fl>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
@@ -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
@@ -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
@@ -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
@@ -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;