From 48faf8d036db16573855d3eeb296b57de0ffca2a Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 21 Sep 2024 08:25:14 -0400 Subject: [PATCH] Fix tracing when name() is empty (#5470). --- Changes | 1 + include/verilated_fst_c.cpp | 19 ++++- include/verilated_trace.h | 10 +-- include/verilated_vcd_c.cpp | 20 +++++- src/V3EmitCModel.cpp | 4 +- test_regress/t/t_trace_dumpvars_dyn.cpp | 10 +-- test_regress/t/t_trace_no_top_name.out | 2 + test_regress/t/t_trace_no_top_name2.cpp | 53 ++++++++++++++ test_regress/t/t_trace_no_top_name2.v | 27 ++++++++ test_regress/t/t_trace_no_top_name2_fst.out | 77 +++++++++++++++++++++ test_regress/t/t_trace_no_top_name2_fst.py | 22 ++++++ test_regress/t/t_trace_no_top_name2_vcd.out | 69 ++++++++++++++++++ test_regress/t/t_trace_no_top_name2_vcd.py | 22 ++++++ 13 files changed, 319 insertions(+), 17 deletions(-) create mode 100644 test_regress/t/t_trace_no_top_name2.cpp create mode 100644 test_regress/t/t_trace_no_top_name2.v create mode 100644 test_regress/t/t_trace_no_top_name2_fst.out create mode 100755 test_regress/t/t_trace_no_top_name2_fst.py create mode 100644 test_regress/t/t_trace_no_top_name2_vcd.out create mode 100755 test_regress/t/t_trace_no_top_name2_vcd.py diff --git a/Changes b/Changes index 4e2a92845..c14c35625 100644 --- a/Changes +++ b/Changes @@ -55,6 +55,7 @@ Verilator 5.029 devel * Fix foreach colliding index names (#5444). [Arkadiusz Kozdra, Antmicro Ltd.] * Fix fault on defparam with UNSUPPORTED ignored (#5450). [Luiza de Melo] * Fix class reference with pin that is a class reference (#5454). +* Fix tracing when name() is empty (#5470). [Sam Shahrestani] * Fix timing mode not exiting on empty events (#5472). diff --git a/include/verilated_fst_c.cpp b/include/verilated_fst_c.cpp index 3fd38bf1a..348ffa001 100644 --- a/include/verilated_fst_c.cpp +++ b/include/verilated_fst_c.cpp @@ -127,6 +127,7 @@ void VerilatedFst::declDTypeEnum(int dtypenum, const char* name, uint32_t elemen // TODO: should return std::optional, but I can't have C++17 static std::pair toFstScopeType(VerilatedTracePrefixType type) { switch (type) { + case VerilatedTracePrefixType::ROOTIO_MODULE: return {true, FST_ST_VCD_MODULE}; case VerilatedTracePrefixType::SCOPE_MODULE: return {true, FST_ST_VCD_MODULE}; case VerilatedTracePrefixType::SCOPE_INTERFACE: return {true, FST_ST_VCD_INTERFACE}; case VerilatedTracePrefixType::STRUCT_PACKED: @@ -137,7 +138,20 @@ static std::pair toFstScopeType(VerilatedTracePrefixType typ } void VerilatedFst::pushPrefix(const std::string& name, VerilatedTracePrefixType type) { - const std::string newPrefix = m_prefixStack.back().first + name; + assert(!m_prefixStack.empty()); // Constructor makes an empty entry + std::string pname = name; + // An empty name means this is the root of a model created with name()=="". The + // tools get upset if we try to pass this as empty, so we put the signals under a + // new scope, but the signals further down will be peers, not children (as usual + // for name()!="") + // Terminate earlier $root? + if (m_prefixStack.back().second == VerilatedTracePrefixType::ROOTIO_MODULE) popPrefix(); + if (pname.empty()) { // Start new temporary root + pname = "$rootio"; // VCD names are not backslash escaped + m_prefixStack.emplace_back("", VerilatedTracePrefixType::ROOTIO_WRAPPER); + type = VerilatedTracePrefixType::ROOTIO_MODULE; + } + const std::string newPrefix = m_prefixStack.back().first + pname; const auto pair = toFstScopeType(type); const bool properScope = pair.first; const fstScopeType scopeType = pair.second; @@ -149,10 +163,11 @@ void VerilatedFst::pushPrefix(const std::string& name, VerilatedTracePrefixType } void VerilatedFst::popPrefix() { + assert(!m_prefixStack.empty()); const bool properScope = toFstScopeType(m_prefixStack.back().second).first; if (properScope) fstWriterSetUpscope(m_fst); m_prefixStack.pop_back(); - assert(!m_prefixStack.empty()); + assert(!m_prefixStack.empty()); // Always one left, the constructor's initial one } void VerilatedFst::declare(uint32_t code, const char* name, int dtypenum, diff --git a/include/verilated_trace.h b/include/verilated_trace.h index 5d7209754..87d5025b4 100644 --- a/include/verilated_trace.h +++ b/include/verilated_trace.h @@ -49,10 +49,12 @@ class VerilatedTraceOffloadBuffer; //============================================================================= // Common enumerations -enum class VerilatedTracePrefixType : uint32_t { +enum class VerilatedTracePrefixType : uint8_t { // Note: Entries must match VTracePrefixType (by name, not necessarily by value) ARRAY_PACKED, ARRAY_UNPACKED, + ROOTIO_MODULE, // $rootio, used when name()=="", other modules become peers + ROOTIO_WRAPPER, // "Above" ROOTIO_MODULE SCOPE_MODULE, SCOPE_INTERFACE, STRUCT_PACKED, @@ -61,7 +63,7 @@ enum class VerilatedTracePrefixType : uint32_t { }; // Direction attribute for ports -enum class VerilatedTraceSigDirection : uint32_t { +enum class VerilatedTraceSigDirection : uint8_t { NONE, INPUT, OUTPUT, @@ -69,7 +71,7 @@ enum class VerilatedTraceSigDirection : uint32_t { }; // Kind of signal. Similar to nettype but with a few more alternatives -enum class VerilatedTraceSigKind : uint32_t { +enum class VerilatedTraceSigKind : uint8_t { PARAMETER, SUPPLY0, SUPPLY1, @@ -81,7 +83,7 @@ enum class VerilatedTraceSigKind : uint32_t { }; // Base data type of signal -enum class VerilatedTraceSigType : uint32_t { +enum class VerilatedTraceSigType : uint8_t { DOUBLE, INTEGER, BIT, diff --git a/include/verilated_vcd_c.cpp b/include/verilated_vcd_c.cpp index 21e022178..9d45ae541 100644 --- a/include/verilated_vcd_c.cpp +++ b/include/verilated_vcd_c.cpp @@ -305,8 +305,22 @@ void VerilatedVcd::printIndent(int level_change) { } void VerilatedVcd::pushPrefix(const std::string& name, VerilatedTracePrefixType type) { - std::string newPrefix = m_prefixStack.back().first + name; + assert(!m_prefixStack.empty()); // Constructor makes an empty entry + std::string pname = name; + // An empty name means this is the root of a model created with name()=="". The + // tools get upset if we try to pass this as empty, so we put the signals under a + // new scope, but the signals further down will be peers, not children (as usual + // for name()!="") + // Terminate earlier $root? + if (m_prefixStack.back().second == VerilatedTracePrefixType::ROOTIO_MODULE) popPrefix(); + if (pname.empty()) { // Start new temporary root + pname = "$rootio"; // VCD names are not backslash escaped + m_prefixStack.emplace_back("", VerilatedTracePrefixType::ROOTIO_WRAPPER); + type = VerilatedTracePrefixType::ROOTIO_MODULE; + } + std::string newPrefix = m_prefixStack.back().first + pname; switch (type) { + case VerilatedTracePrefixType::ROOTIO_MODULE: case VerilatedTracePrefixType::SCOPE_MODULE: case VerilatedTracePrefixType::SCOPE_INTERFACE: case VerilatedTracePrefixType::STRUCT_PACKED: @@ -326,7 +340,9 @@ void VerilatedVcd::pushPrefix(const std::string& name, VerilatedTracePrefixType } void VerilatedVcd::popPrefix() { + assert(!m_prefixStack.empty()); switch (m_prefixStack.back().second) { + case VerilatedTracePrefixType::ROOTIO_MODULE: case VerilatedTracePrefixType::SCOPE_MODULE: case VerilatedTracePrefixType::SCOPE_INTERFACE: case VerilatedTracePrefixType::STRUCT_PACKED: @@ -338,7 +354,7 @@ void VerilatedVcd::popPrefix() { default: break; } m_prefixStack.pop_back(); - assert(!m_prefixStack.empty()); + assert(!m_prefixStack.empty()); // Always one left, the constructor's initial one } void VerilatedVcd::declare(uint32_t code, const char* name, const char* wirep, bool array, diff --git a/src/V3EmitCModel.cpp b/src/V3EmitCModel.cpp index dee84efb3..9baeb70e3 100644 --- a/src/V3EmitCModel.cpp +++ b/src/V3EmitCModel.cpp @@ -551,11 +551,11 @@ class EmitCModel final : public EmitCFunc { "0.\");\n"); puts("}\n"); puts("vlSymsp->__Vm_baseCode = code;\n"); - puts("if (strlen(vlSymsp->name())) tracep->pushPrefix(std::string{vlSymsp->name()}, " + puts("tracep->pushPrefix(std::string{vlSymsp->name()}, " "VerilatedTracePrefixType::SCOPE_MODULE);\n"); puts(topModNameProtected + "__" + protect("trace_decl_types") + "(tracep);\n"); puts(topModNameProtected + "__" + protect("trace_init_top") + "(vlSelf, tracep);\n"); - puts("if (strlen(vlSymsp->name())) tracep->popPrefix();\n"); + puts("tracep->popPrefix();\n"); puts("}\n"); // Forward declaration diff --git a/test_regress/t/t_trace_dumpvars_dyn.cpp b/test_regress/t/t_trace_dumpvars_dyn.cpp index 1d3fbfab3..4123e95b7 100644 --- a/test_regress/t/t_trace_dumpvars_dyn.cpp +++ b/test_regress/t/t_trace_dumpvars_dyn.cpp @@ -10,9 +10,11 @@ #if VM_TRACE_FST #include #define TRACE_FILE_NAME "simx.fst" +#define TRACE_CLASS VerilatedFstC #elif VM_TRACE_VCD #include #define TRACE_FILE_NAME "simx.vcd" +#define TRACE_CLASS VerilatedVcdC #endif #include @@ -31,13 +33,7 @@ int main(int argc, char** argv) { std::unique_ptr top{new VM_PREFIX{"top"}}; -#if defined(T_TRACE_DUMPVARS_DYN_VCD_0) || defined(T_TRACE_DUMPVARS_DYN_VCD_1) - std::unique_ptr tfp{new VerilatedVcdC}; -#elif defined(T_TRACE_DUMPVARS_DYN_FST_0) || defined(T_TRACE_DUMPVARS_DYN_FST_1) - std::unique_ptr tfp{new VerilatedFstC}; -#else -#error "Bad test" -#endif + std::unique_ptr tfp{new TRACE_CLASS}; #if defined(T_TRACE_DUMPVARS_DYN_VCD_0) || defined(T_TRACE_DUMPVARS_DYN_FST_0) tfp->dumpvars(0, ""); diff --git a/test_regress/t/t_trace_no_top_name.out b/test_regress/t/t_trace_no_top_name.out index 2905133f4..5eef24e61 100644 --- a/test_regress/t/t_trace_no_top_name.out +++ b/test_regress/t/t_trace_no_top_name.out @@ -1,5 +1,7 @@ $version Generated by VerilatedVcd $end $timescale 1ps $end + $scope module $rootio $end + $upscope $end $scope module another_top $end $var wire 1 # b $end $upscope $end diff --git a/test_regress/t/t_trace_no_top_name2.cpp b/test_regress/t/t_trace_no_top_name2.cpp new file mode 100644 index 000000000..ce05129eb --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2.cpp @@ -0,0 +1,53 @@ +// -*- 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 Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +#include +#if VM_TRACE_FST +#include +#define TRACE_FILE_NAME "simx.fst" +#define TRACE_CLASS VerilatedFstC +#elif VM_TRACE_VCD +#include +#define TRACE_FILE_NAME "simx.vcd" +#define TRACE_CLASS VerilatedVcdC +#endif + +#include + +#include VM_PREFIX_INCLUDE + +unsigned long long main_time = 0; +double sc_time_stamp() { return (double)main_time; } + +int main(int argc, char** argv) { + Verilated::debug(0); + Verilated::traceEverOn(true); + Verilated::commandArgs(argc, argv); + + // This test is to specifically check "" as the below upper model name + std::unique_ptr top{new VM_PREFIX{""}}; + + std::unique_ptr tfp{new TRACE_CLASS}; + + top->trace(tfp.get(), 99); + tfp->open(VL_STRINGIFY(TEST_OBJ_DIR) "/" TRACE_FILE_NAME); + top->clk = 0; + + while (main_time <= 20) { + top->eval(); + tfp->dump((unsigned int)(main_time)); + ++main_time; + top->clk = !top->clk; + } + tfp->close(); + top->final(); + tfp.reset(); + top.reset(); + printf("*-* All Finished *-*\n"); + return 0; +} diff --git a/test_regress/t/t_trace_no_top_name2.v b/test_regress/t/t_trace_no_top_name2.v new file mode 100644 index 000000000..b1f408cb6 --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2.v @@ -0,0 +1,27 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module sub; + int a = 1212; +endmodule + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + int cyc; + + sub sub(); + + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc == 10) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_trace_no_top_name2_fst.out b/test_regress/t/t_trace_no_top_name2_fst.out new file mode 100644 index 000000000..81393e41f --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2_fst.out @@ -0,0 +1,77 @@ +$date + Sat Sep 21 08:10:39 2024 + +$end +$version + fstWriter +$end +$timescale + 1ps +$end +$scope module $rootio $end +$var wire 1 ! clk $end +$upscope $end +$scope module t $end +$var wire 1 ! clk $end +$var int 32 " cyc [31:0] $end +$scope module sub $end +$var int 32 # a [31:0] $end +$upscope $end +$upscope $end +$enddefinitions $end +#0 +$dumpvars +b00000000000000000000010010111100 # +b00000000000000000000000000000000 " +0! +$end +#1 +1! +b00000000000000000000000000000001 " +#2 +0! +#3 +1! +b00000000000000000000000000000010 " +#4 +0! +#5 +1! +b00000000000000000000000000000011 " +#6 +0! +#7 +1! +b00000000000000000000000000000100 " +#8 +0! +#9 +1! +b00000000000000000000000000000101 " +#10 +0! +#11 +1! +b00000000000000000000000000000110 " +#12 +0! +#13 +1! +b00000000000000000000000000000111 " +#14 +0! +#15 +1! +b00000000000000000000000000001000 " +#16 +0! +#17 +1! +b00000000000000000000000000001001 " +#18 +0! +#19 +1! +b00000000000000000000000000001010 " +#20 +0! diff --git a/test_regress/t/t_trace_no_top_name2_fst.py b/test_regress/t/t_trace_no_top_name2_fst.py new file mode 100755 index 000000000..bedb57f89 --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2_fst.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python3 +# 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 + +import vltest_bootstrap + +test.scenarios('vlt') +test.pli_filename = "t/t_trace_no_top_name2.cpp" +test.top_filename = "t/t_trace_no_top_name2.v" + +test.compile(make_main=False, verilator_flags2=["--trace-fst --exe", test.pli_filename]) + +test.execute() + +test.fst_identical(test.trace_filename, test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_trace_no_top_name2_vcd.out b/test_regress/t/t_trace_no_top_name2_vcd.out new file mode 100644 index 000000000..45e6e891c --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2_vcd.out @@ -0,0 +1,69 @@ +$version Generated by VerilatedVcd $end +$timescale 1ps $end + $scope module $rootio $end + $var wire 1 # clk $end + $upscope $end + $scope module t $end + $var wire 1 # clk $end + $var wire 32 $ cyc [31:0] $end + $scope module sub $end + $var wire 32 % a [31:0] $end + $upscope $end + $upscope $end +$enddefinitions $end + + +#0 +0# +b00000000000000000000000000000000 $ +b00000000000000000000010010111100 % +#1 +1# +b00000000000000000000000000000001 $ +#2 +0# +#3 +1# +b00000000000000000000000000000010 $ +#4 +0# +#5 +1# +b00000000000000000000000000000011 $ +#6 +0# +#7 +1# +b00000000000000000000000000000100 $ +#8 +0# +#9 +1# +b00000000000000000000000000000101 $ +#10 +0# +#11 +1# +b00000000000000000000000000000110 $ +#12 +0# +#13 +1# +b00000000000000000000000000000111 $ +#14 +0# +#15 +1# +b00000000000000000000000000001000 $ +#16 +0# +#17 +1# +b00000000000000000000000000001001 $ +#18 +0# +#19 +1# +b00000000000000000000000000001010 $ +#20 +0# diff --git a/test_regress/t/t_trace_no_top_name2_vcd.py b/test_regress/t/t_trace_no_top_name2_vcd.py new file mode 100755 index 000000000..06a524d42 --- /dev/null +++ b/test_regress/t/t_trace_no_top_name2_vcd.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python3 +# 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 + +import vltest_bootstrap + +test.scenarios('vlt') +test.pli_filename = "t/t_trace_no_top_name2.cpp" +test.top_filename = "t/t_trace_no_top_name2.v" + +test.compile(make_main=False, verilator_flags2=["--trace --exe", test.pli_filename]) + +test.execute() + +test.vcd_identical(test.trace_filename, test.golden_filename) + +test.passes()