Fix extra events in traces (#5405).

Changes

@@ -32,6 +32,7 @@ Verilator 5.029 devel
 * Fix dot fallback finding wrong symbols (#5394). [Arkadiusz Kozdra, Antmicro Ltd.]
 * Fix infinite recursion due to recursive functions/tasks (#5398). [Krzysztof Bieganski, Antmicro Ltd.]
 * Fix V3Randomize compile error on old GCC (#5403) (#5417). [Krzysztof Bieganski, Antmicro Ltd.]
+* Fix extra events in traces (#5405).
 * Fix empty `foreach` in `if` in constraints (#5408). [Krzysztof Bieganski, Antmicro Ltd.]
 * Fix queue `[$-i]` select as reference argument (#5411). [Krzysztof Bieganski, Antmicro Ltd.]
 * Fix `pre`/`post_randomize` on `randomize() with` (#5412). [Krzysztof Bieganski, Antmicro Ltd.]

include/verilated_fst_c.cpp

@@ -294,7 +294,7 @@ void VerilatedFst::configure(const VerilatedTraceConfig& config) {
 // so always inline them.
 
 VL_ATTR_ALWINLINE
-void VerilatedFstBuffer::emitEvent(uint32_t code, const VlEventBase* newvalp) {
+void VerilatedFstBuffer::emitEvent(uint32_t code) {
     VL_DEBUG_IFDEF(assert(m_symbolp[code]););
     fstWriterEmitValueChange(m_fst, m_symbolp[code], "1");
 }

include/verilated_fst_c.h

@@ -177,7 +177,7 @@ class VerilatedFstBuffer VL_NOT_FINAL {
 
     // Implementations of duck-typed methods for VerilatedTraceBuffer. These are
     // called from only one place (the full* methods), so always inline them.
-    VL_ATTR_ALWINLINE void emitEvent(uint32_t code, const VlEventBase* newvalp);
+    VL_ATTR_ALWINLINE void emitEvent(uint32_t code);
     VL_ATTR_ALWINLINE void emitBit(uint32_t code, CData newval);
     VL_ATTR_ALWINLINE void emitCData(uint32_t code, CData newval, int bits);
     VL_ATTR_ALWINLINE void emitSData(uint32_t code, SData newval, int bits);

include/verilated_trace.h

@@ -488,6 +488,7 @@ public:
     void fullWData(uint32_t* oldp, const WData* newvalp, int bits);
     void fullDouble(uint32_t* oldp, double newval);
     void fullEvent(uint32_t* oldp, const VlEventBase* newvalp);
+    void fullEventTriggered(uint32_t* oldp);
 
     // In non-offload mode, these are called directly by the trace callbacks,
     // and are called chg*. In offload mode, they are called by the worker
@@ -525,8 +526,9 @@ public:
         }
     }
     VL_ATTR_ALWINLINE void chgEvent(uint32_t* oldp, const VlEventBase* newvalp) {
-        fullEvent(oldp, newvalp);
+        if (newvalp->isTriggered()) fullEvent(oldp, newvalp);
     }
+    VL_ATTR_ALWINLINE void chgEventTriggered(uint32_t* oldp) { fullEventTriggered(oldp); }
     VL_ATTR_ALWINLINE void chgDouble(uint32_t* oldp, double newval) {
         double old;
         std::memcpy(&old, oldp, sizeof(old));
@@ -606,6 +608,9 @@ public:
         VL_DEBUG_IF(assert(m_offloadBufferWritep <= m_offloadBufferEndp););
     }
     void chgEvent(uint32_t code, const VlEventBase* newvalp) {
+        if (newvalp->isTriggered()) chgEventTriggered(code);
+    }
+    void chgEventTriggered(uint32_t code) {
         m_offloadBufferWritep[0] = VerilatedTraceOffloadCommand::CHG_EVENT;
         m_offloadBufferWritep[1] = code;
         m_offloadBufferWritep += 2;

include/verilated_trace_imp.h

@@ -169,7 +169,7 @@ void VerilatedTrace<VL_SUB_T, VL_BUF_T>::offloadWorkerThreadMain() {
                 continue;
             case VerilatedTraceOffloadCommand::CHG_EVENT:
                 VL_TRACE_OFFLOAD_DEBUG("Command CHG_EVENT " << top);
-                traceBufp->chgEvent(oldp, reinterpret_cast<const VlEventBase*>(readp));
+                traceBufp->chgEventTriggered(oldp);
                 continue;
 
                 //===
@@ -833,8 +833,15 @@ void VerilatedTraceBuffer<VL_BUF_T>::fullBit(uint32_t* oldp, CData newval) {
 template <>
 void VerilatedTraceBuffer<VL_BUF_T>::fullEvent(uint32_t* oldp, const VlEventBase* newvalp) {
     const uint32_t code = oldp - m_sigs_oldvalp;
-    *oldp = 1;  // Do we really store an "event" ?
-    emitEvent(code, newvalp);
+    // No need to update *oldp
+    if (newvalp->isTriggered()) emitEvent(code);
+}
+
+template <>
+void VerilatedTraceBuffer<VL_BUF_T>::fullEventTriggered(uint32_t* oldp) {
+    const uint32_t code = oldp - m_sigs_oldvalp;
+    // No need to update *oldp
+    emitEvent(code);
 }
 
 template <>

include/verilated_vcd_c.cpp

@@ -573,16 +573,11 @@ void VerilatedVcdBuffer::finishLine(uint32_t code, char* writep) {
 // so always inline them.
 
 VL_ATTR_ALWINLINE
-void VerilatedVcdBuffer::emitEvent(uint32_t code, const VlEventBase* newvalp) {
-    const bool triggered = newvalp->isTriggered();
-    // TODO : It seems that untriggered events are not filtered
-    // should be tested before this last step
-    if (triggered) {
-        // Don't prefetch suffix as it's a bit too late;
-        char* wp = m_writep;
-        *wp++ = '1';
-        finishLine(code, wp);
-    }
+void VerilatedVcdBuffer::emitEvent(uint32_t code) {
+    // Don't prefetch suffix as it's a bit too late;
+    char* wp = m_writep;
+    *wp++ = '1';
+    finishLine(code, wp);
 }
 
 VL_ATTR_ALWINLINE

include/verilated_vcd_c.h

@@ -214,7 +214,7 @@ class VerilatedVcdBuffer VL_NOT_FINAL {
     // Implementation of VerilatedTraceBuffer interface
     // Implementations of duck-typed methods for VerilatedTraceBuffer. These are
     // called from only one place (the full* methods), so always inline them.
-    VL_ATTR_ALWINLINE void emitEvent(uint32_t code, const VlEventBase* newvalp);
+    VL_ATTR_ALWINLINE void emitEvent(uint32_t code);
     VL_ATTR_ALWINLINE void emitBit(uint32_t code, CData newval);
     VL_ATTR_ALWINLINE void emitCData(uint32_t code, CData newval, int bits);
     VL_ATTR_ALWINLINE void emitSData(uint32_t code, SData newval, int bits);

test_regress/t/t_trace_event.out

@@ -0,0 +1,71 @@
+$version Generated by VerilatedVcd $end
+$timescale 1ps $end
+ $scope module TOP $end
+  $scope module t $end
+   $var event 1 # ev_test $end
+   $var wire 32 $ i [31:0] $end
+   $var wire 1 % toggle $end
+   $var wire 1 & clk $end
+  $upscope $end
+ $upscope $end
+$enddefinitions $end
+
+
+#0
+b00000000000000000000000000000000 $
+0%
+0&
+#10
+b00000000000000000000000000000001 $
+1&
+#20
+0&
+#30
+b00000000000000000000000000000010 $
+1&
+#40
+0&
+#50
+b00000000000000000000000000000011 $
+1&
+#60
+0&
+#70
+b00000000000000000000000000000100 $
+1&
+#80
+0&
+#90
+b00000000000000000000000000000101 $
+1&
+#100
+0&
+#110
+1#
+b00000000000000000000000000000110 $
+1%
+1&
+#120
+0&
+#130
+b00000000000000000000000000000111 $
+1&
+#140
+0&
+#150
+b00000000000000000000000000001000 $
+1&
+#160
+0&
+#170
+b00000000000000000000000000001001 $
+1&
+#180
+0&
+#190
+b00000000000000000000000000001010 $
+1&
+#200
+0&
+#210
+1&

test_regress/t/t_trace_event.py

@@ -0,0 +1,20 @@
+#!/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('simulator')
+
+test.compile(verilator_flags2=['--trace --binary'])
+
+test.execute()
+
+test.vcd_identical(test.trace_filename, test.golden_filename)
+
+test.passes()

test_regress/t/t_trace_event.v

@@ -0,0 +1,43 @@
+// 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
+
+`define STRINGIFY(x) `"x`"
+
+module t(/*AUTOARG*/);
+
+   event ev_test;
+
+   int i;
+
+   bit toggle = 1'b0;
+
+   bit clk;
+   always #10 clk = ~clk;
+
+   initial begin
+      @(posedge clk);
+
+      @(ev_test);
+      toggle = ~toggle;
+   end
+
+   initial begin
+      $dumpfile(`STRINGIFY(`TEST_DUMPFILE));
+      $dumpvars(0, top);
+      for(i=0; i < 10; i++) begin
+         @(posedge clk);
+
+         if (i == 5)
+           ->ev_test;
+      end
+
+      @(posedge clk);
+
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+
+endmodule

test_regress/t/t_trace_event_fst.out

@@ -0,0 +1,79 @@
+$date
+	Tue Sep 10 16:34:40 2024
+
+$end
+$version
+	fstWriter
+$end
+$timescale
+	1ps
+$end
+$scope module TOP $end
+$scope module t $end
+$var event 1 ! ev_test $end
+$var int 32 " i [31:0] $end
+$var bit 1 # toggle $end
+$var bit 1 $ clk $end
+$upscope $end
+$upscope $end
+$enddefinitions $end
+#0
+$dumpvars
+0$
+0#
+b00000000000000000000000000000000 "
+$end
+#10
+b00000000000000000000000000000001 "
+1$
+#20
+0$
+#30
+1$
+b00000000000000000000000000000010 "
+#40
+0$
+#50
+1$
+b00000000000000000000000000000011 "
+#60
+0$
+#70
+1$
+b00000000000000000000000000000100 "
+#80
+0$
+#90
+1$
+b00000000000000000000000000000101 "
+#100
+0$
+#110
+1$
+b00000000000000000000000000000110 "
+1#
+1!
+#120
+0$
+#130
+1$
+b00000000000000000000000000000111 "
+#140
+0$
+#150
+1$
+b00000000000000000000000000001000 "
+#160
+0$
+#170
+1$
+b00000000000000000000000000001001 "
+#180
+0$
+#190
+1$
+b00000000000000000000000000001010 "
+#200
+0$
+#210
+1$

test_regress/t/t_trace_event_fst.py

@@ -0,0 +1,21 @@
+#!/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.top_filename = "t/t_trace_event.v"
+
+test.compile(verilator_flags2=['--trace-fst --binary'])
+
+test.execute()
+
+test.fst_identical(test.trace_filename, test.golden_filename)
+
+test.passes()