From b0e796ca83cd05a6f72674c89e9af1eaa9c71366 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Fri, 15 Jul 2022 11:44:32 -0400 Subject: [PATCH] Public combo propagation issues (#2905) --- src/V3Order.cpp | 8 ++++++ test_regress/t/t_public_clk.cpp | 41 ++++++++++++++++++++++++++++++ test_regress/t/t_public_clk.pl | 27 ++++++++++++++++++++ test_regress/t/t_public_clk.v | 28 ++++++++++++++++++++ test_regress/t/t_public_seq.cpp | 45 +++++++++++++++++++++++++++++++++ test_regress/t/t_public_seq.pl | 27 ++++++++++++++++++++ test_regress/t/t_public_seq.v | 42 ++++++++++++++++++++++++++++++ 7 files changed, 218 insertions(+) create mode 100644 test_regress/t/t_public_clk.cpp create mode 100755 test_regress/t/t_public_clk.pl create mode 100644 test_regress/t/t_public_clk.v create mode 100644 test_regress/t/t_public_seq.cpp create mode 100755 test_regress/t/t_public_seq.pl create mode 100644 test_regress/t/t_public_seq.v diff --git a/src/V3Order.cpp b/src/V3Order.cpp index 4f2890be6..032f8ef3f 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -1601,11 +1601,19 @@ void OrderProcess::processDomainsIterate(OrderEitherVertex* vertexp) { // It should have already been copied into the settle domain. Presumably it has // inputs which we never trigger, or nothing it's sensitive to, so we can rip it out. if (!domainp && vertexp->scopep()) domainp = m_deleteDomainp; + // However, anything that is public RW must be added to the combo domain since the + // user may change it at any time + if (domainp && vvertexp && vvertexp->varScp()->varp()->isSigUserRWPublic()) + domainp = m_comboDomainp; } // vertexp->domainp(domainp); if (vertexp->domainp()) { UINFO(5, " done d=" << cvtToHex(vertexp->domainp()) + << (vertexp->domainp() == m_deleteDomainp ? " [DEL]" : "") + << (vertexp->domainp()->hasClocked() ? " [CLKD]" : "") + << (vertexp->domainp()->hasSettle() ? " [SETL]" : "") + << (vertexp->domainp()->hasInitial() ? " [INIT]" : "") << (vertexp->domainp()->hasCombo() ? " [COMB]" : "") << (vertexp->domainp()->isMulti() ? " [MULT]" : "") << " " << vertexp << endl); diff --git a/test_regress/t/t_public_clk.cpp b/test_regress/t/t_public_clk.cpp new file mode 100644 index 000000000..15ff2985b --- /dev/null +++ b/test_regress/t/t_public_clk.cpp @@ -0,0 +1,41 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Todd Strader. +// SPDX-License-Identifier: CC0-1.0 + +// Generated header +#include "Vt_public_clk.h" +#include "Vt_public_clk___024root.h" +// General headers +#include "verilated.h" + +std::unique_ptr topp; +int main(int argc, char** argv, char** env) { + vluint64_t sim_time = 1100; + const std::unique_ptr contextp{new VerilatedContext}; + contextp->commandArgs(argc, argv); + contextp->debug(0); + srand48(5); + topp.reset(new Vt_public_clk("top")); + + topp->rootp->t__DOT__clk = 0; + topp->eval(); + { contextp->timeInc(10); } + + while ((contextp->time() < sim_time) && !contextp->gotFinish()) { + topp->rootp->t__DOT__clk = !topp->rootp->t__DOT__clk; + topp->eval(); + contextp->timeInc(5); + } + + if (!contextp->gotFinish()) { + vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); + } + topp->final(); + + topp.reset(); + return 0; +} diff --git a/test_regress/t/t_public_clk.pl b/test_regress/t/t_public_clk.pl new file mode 100755 index 000000000..89f3e05aa --- /dev/null +++ b/test_regress/t/t_public_clk.pl @@ -0,0 +1,27 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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( + make_top_shell => 0, + make_main => 0, + verilator_flags2 => [ + "--exe", + "$Self->{t_dir}/$Self->{name}.cpp" + ], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_public_clk.v b/test_regress/t/t_public_clk.v new file mode 100644 index 000000000..b5f123f35 --- /dev/null +++ b/test_regress/t/t_public_clk.v @@ -0,0 +1,28 @@ +// DESCRIPTION: Verilator: public clock signal +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2022 by Todd Strader +// SPDX-License-Identifier: CC0-1.0 + +`ifdef VERILATOR +// The '$c1(1)' is there to prevent inlining of the signal by V3Gate +`define IMPURE_ONE ($c(1)) +`else +// Use standard $random (chaces of getting 2 consecutive zeroes is zero). +`define IMPURE_ONE (|($random | $random)) +`endif + +module t (); + + logic clk /* verilator public_flat_rw */; + int count; + wire other_clk = `IMPURE_ONE & clk; + + always_ff @(posedge other_clk) begin + count <= count + 1; + if (count == 10) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_public_seq.cpp b/test_regress/t/t_public_seq.cpp new file mode 100644 index 000000000..26906bd88 --- /dev/null +++ b/test_regress/t/t_public_seq.cpp @@ -0,0 +1,45 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Todd Strader. +// SPDX-License-Identifier: CC0-1.0 + +// Generated header +#include "Vt_public_seq.h" +#include "Vt_public_seq___024root.h" +// General headers +#include "verilated.h" + +std::unique_ptr topp; +int main(int argc, char** argv, char** env) { + vluint64_t sim_time = 1100; + const std::unique_ptr contextp{new VerilatedContext}; + contextp->commandArgs(argc, argv); + contextp->debug(0); + srand48(5); + topp.reset(new Vt_public_seq("top")); + + topp->clk = 0; + topp->eval(); + { contextp->timeInc(10); } + + int cyc = 0; + + while ((contextp->time() < sim_time) && !contextp->gotFinish()) { + if (cyc >= 5) ++topp->rootp->t__DOT__pub_byte; + topp->eval(); + topp->clk = !topp->clk; + topp->eval(); + contextp->timeInc(5); + if (topp->clk) cyc++; + } + if (!contextp->gotFinish()) { + vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); + } + topp->final(); + + topp.reset(); + return 0; +} diff --git a/test_regress/t/t_public_seq.pl b/test_regress/t/t_public_seq.pl new file mode 100755 index 000000000..89f3e05aa --- /dev/null +++ b/test_regress/t/t_public_seq.pl @@ -0,0 +1,27 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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( + make_top_shell => 0, + make_main => 0, + verilator_flags2 => [ + "--exe", + "$Self->{t_dir}/$Self->{name}.cpp" + ], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_public_seq.v b/test_regress/t/t_public_seq.v new file mode 100644 index 000000000..007e51ffe --- /dev/null +++ b/test_regress/t/t_public_seq.v @@ -0,0 +1,42 @@ +// DESCRIPTION: Verilator: public clock signal +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2022 by Todd Strader +// SPDX-License-Identifier: CC0-1.0 + +`ifdef VERILATOR +// The '$c1(1)' is there to prevent inlining of the signal by V3Gate +`define IMPURE_ONE ($c(1)) +`else +// Use standard $random (chaces of getting 2 consecutive zeroes is zero). +`define IMPURE_ONE (|($random | $random)) +`endif + +module t ( + input clk, + input dummy_clk // Never toggled from C++ +); + + int count; + + logic [7:0] pub_byte /* verilator public_flat_rw */ = 123; + logic [7:0] comb_byte; + + always_comb comb_byte = `IMPURE_ONE ? pub_byte : '0; + + always_ff @(posedge clk) begin + count <= count + 1; + if (comb_byte != pub_byte) begin + $display("%%Error: comb_byte (%0d) != pub_byte (%0d)", comb_byte, pub_byte); + $stop; + end + if (count == 10) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + + always_ff @(posedge dummy_clk) begin + comb_byte = ~pub_byte; + end +endmodule