From 701fa5438ab107a9750b1f86f4cbbb36a5dcef86 Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Thu, 8 Aug 2024 22:48:25 +0200 Subject: [PATCH] Fix output clockvar overwriting signal (#5320) (#5347) Signed-off-by: Krzysztof Bieganski --- src/V3AssertPre.cpp | 39 ++++++++++++-- test_regress/t/t_clocking_out_on_change.pl | 22 ++++++++ test_regress/t/t_clocking_out_on_change.v | 61 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 5 deletions(-) create mode 100755 test_regress/t/t_clocking_out_on_change.pl create mode 100644 test_regress/t/t_clocking_out_on_change.v diff --git a/src/V3AssertPre.cpp b/src/V3AssertPre.cpp index 7632eea49..acd482f0e 100644 --- a/src/V3AssertPre.cpp +++ b/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{ diff --git a/test_regress/t/t_clocking_out_on_change.pl b/test_regress/t/t_clocking_out_on_change.pl new file mode 100755 index 000000000..bc08e4a19 --- /dev/null +++ b/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; diff --git a/test_regress/t/t_clocking_out_on_change.v b/test_regress/t/t_clocking_out_on_change.v new file mode 100644 index 000000000..a8730ca9f --- /dev/null +++ b/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