Fix output clockvar overwriting signal (#5320) (#5347)

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>

src/V3AssertPre.cpp

@@ -182,18 +182,47 @@ private:
         m_clockingp->addNextHere(varp->unlinkFrBack());
         varp->user1p(nodep);
         if (nodep->direction() == VDirection::OUTPUT) {
-            AstVarRef* const skewedRefp = new AstVarRef{flp, varp, VAccess::READ};
-            skewedRefp->user1(true);
-            AstAssign* const assignp = new AstAssign{flp, exprp->cloneTreePure(false), skewedRefp};
+            exprp->foreach([](AstNodeVarRef* varrefp) {
+                // Prevent confusing BLKANDNBLK warnings on clockvars due to generated assignments
+                varrefp->fileline()->warnOff(V3ErrorCode::BLKANDNBLK, true);
+            });
+            AstVarRef* const skewedReadRefp = new AstVarRef{flp, varp, VAccess::READ};
+            skewedReadRefp->user1(true);
+            // Initialize the clockvar
+            AstVarRef* const skewedWriteRefp = new AstVarRef{flp, varp, VAccess::WRITE};
+            skewedWriteRefp->user1(true);
+            AstInitialStatic* const initClockvarp = new AstInitialStatic{
+                flp, new AstAssign{flp, skewedWriteRefp, exprp->cloneTreePure(false)}};
+            m_modp->addStmtsp(initClockvarp);
+            // A var to keep the previous value of the clockvar
+            AstVar* const prevVarp = new AstVar{
+                flp, VVarType::MODULETEMP, "__Vclocking_prev__" + varp->name(), exprp->dtypep()};
+            prevVarp->lifetime(VLifetime::STATIC);
+            AstInitialStatic* const initPrevClockvarp = new AstInitialStatic{
+                flp, new AstAssign{flp, new AstVarRef{flp, prevVarp, VAccess::WRITE},
+                                   skewedReadRefp->cloneTreePure(false)}};
+            m_modp->addStmtsp(prevVarp);
+            m_modp->addStmtsp(initPrevClockvarp);
+            // Assign the clockvar to the actual var; only do it if the clockvar's value has
+            // changed
+            AstAssign* const assignp
+                = new AstAssign{flp, exprp->cloneTreePure(false), skewedReadRefp};
+            AstIf* const ifp
+                = new AstIf{flp,
+                            new AstNeq{flp, new AstVarRef{flp, prevVarp, VAccess::READ},
+                                       skewedReadRefp->cloneTreePure(false)},
+                            assignp};
+            ifp->addThensp(new AstAssign{flp, new AstVarRef{flp, prevVarp, VAccess::WRITE},
+                                         skewedReadRefp->cloneTree(false)});
             if (skewp->isZero()) {
                 // Drive the var in Re-NBA (IEEE 1800-2023 14.16)
                 m_clockingp->addNextHere(new AstAlwaysReactive{
-                    flp, new AstSenTree{flp, m_clockingp->sensesp()->cloneTree(false)}, assignp});
+                    flp, new AstSenTree{flp, m_clockingp->sensesp()->cloneTree(false)}, ifp});
             } else if (skewp->fileline()->timingOn()) {
                 // Create a fork so that this AlwaysObserved can be retriggered before the
                 // assignment happens. Also then it can be combo, avoiding the need for creating
                 // new triggers.
-                AstFork* const forkp = new AstFork{flp, "", assignp};
+                AstFork* const forkp = new AstFork{flp, "", ifp};
                 forkp->joinType(VJoinType::JOIN_NONE);
                 // Use Observed for this to make sure we do not miss the event
                 m_clockingp->addNextHere(new AstAlwaysObserved{

test_regress/t/t_clocking_out_on_change.pl

@@ -0,0 +1,22 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# 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
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ["--exe --main --timing"],
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_clocking_out_on_change.v

@@ -0,0 +1,61 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Antmicro.
+// SPDX-License-Identifier: CC0-1.0
+
+module t;
+   logic clk = 0;
+   initial forever #5 clk = ~clk;
+   int cyc = 0;
+   always @(posedge clk) begin
+      cyc <= cyc + 1;
+      if (cyc == 4) begin
+         $write("*-* All Finished *-*\n");
+         $finish();
+      end
+   end
+
+   // Skew 0
+   logic ok1 = 1;
+   always @(posedge clk)
+      if (cyc == 0) begin
+         if (!ok1) $stop;
+         #1 cb.ok1 <= 0;
+         #1 if (!ok1) $stop;
+      end else if (cyc == 1) begin
+         if (!ok1) $stop;
+         #1 if (ok1) $stop;
+      end
+      else if (cyc == 2) ok1 <= 1;
+      else if (!ok1) $stop;
+
+   // Skew > 0
+   logic ok2 = 1;
+   always @(posedge clk)
+      if (cyc == 0) begin
+         if (!ok2) $stop;
+         #1 cb.ok2 <= 0;
+         #2 if (!ok2) $stop;
+         #3 if (!ok2) $stop;
+      end else if (cyc == 1) begin
+         if (!ok2) $stop;
+         #1 if (!ok2) $stop;
+         #2 if (ok2) $stop;
+      end
+      else if (cyc == 2) ok2 <= 1;
+      else if (!ok2) $stop;
+
+   // No timing
+   logic ok3 = 0;
+   always @(posedge clk)
+      if (cyc == 0) ok3 <= 1;
+      else if (cyc == 1) if (!ok3) $stop;
+
+   // Clocking (used in all tests)
+   clocking cb @(posedge clk);
+      output ok1;
+      output #1 ok2;
+      output ok3;
+   endclocking
+endmodule