Fix tracing when name() is empty (#5470).

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).
 
 

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<fstScopeType>, but I can't have C++17
 static std::pair<bool, fstScopeType> 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<bool, fstScopeType> 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,

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,

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,

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

test_regress/t/t_trace_dumpvars_dyn.cpp

@@ -10,9 +10,11 @@
 #if VM_TRACE_FST
 #include <verilated_fst_c.h>
 #define TRACE_FILE_NAME "simx.fst"
+#define TRACE_CLASS VerilatedFstC
 #elif VM_TRACE_VCD
 #include <verilated_vcd_c.h>
 #define TRACE_FILE_NAME "simx.vcd"
+#define TRACE_CLASS VerilatedVcdC
 #endif
 
 #include <memory>
@@ -31,13 +33,7 @@ int main(int argc, char** argv) {
 
     std::unique_ptr<VM_PREFIX> top{new VM_PREFIX{"top"}};
 
-#if defined(T_TRACE_DUMPVARS_DYN_VCD_0) || defined(T_TRACE_DUMPVARS_DYN_VCD_1)
-    std::unique_ptr<VerilatedVcdC> tfp{new VerilatedVcdC};
-#elif defined(T_TRACE_DUMPVARS_DYN_FST_0) || defined(T_TRACE_DUMPVARS_DYN_FST_1)
-    std::unique_ptr<VerilatedFstC> tfp{new VerilatedFstC};
-#else
-#error "Bad test"
-#endif
+    std::unique_ptr<TRACE_CLASS> tfp{new TRACE_CLASS};
 
 #if defined(T_TRACE_DUMPVARS_DYN_VCD_0) || defined(T_TRACE_DUMPVARS_DYN_FST_0)
     tfp->dumpvars(0, "");

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

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 <verilated.h>
+#if VM_TRACE_FST
+#include <verilated_fst_c.h>
+#define TRACE_FILE_NAME "simx.fst"
+#define TRACE_CLASS VerilatedFstC
+#elif VM_TRACE_VCD
+#include <verilated_vcd_c.h>
+#define TRACE_FILE_NAME "simx.vcd"
+#define TRACE_CLASS VerilatedVcdC
+#endif
+
+#include <memory>
+
+#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<VM_PREFIX> top{new VM_PREFIX{""}};
+
+    std::unique_ptr<TRACE_CLASS> 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;
+}

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

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!

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()

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#

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()