Add custom memory management for verilated classes (#3595)

This change introduces a custom reference-counting pointer class that
allows creating such pointers from 'this'. This lets us keep the
receiver object around even if all references to it outside of a class
method no longer exist. Useful for coroutine methods, which may outlive
all external references to the object.

The deletion of objects is deferred until the next time slot. This is to
make clearing the triggered flag on named events in classes safe
(otherwise freed memory could be accessed).

include/verilated.cpp

@@ -3095,3 +3095,18 @@ void VerilatedAssertOneThread::fatal_different() VL_MT_SAFE {
 #endif
 
 //===========================================================================
+// VlDeleter:: Methods
+
+void VlDeleter::deleteAll() {
+    while (true) {
+        VerilatedLockGuard lock{m_mutex};
+        if (m_newGarbage.empty()) break;
+        VerilatedLockGuard deleteLock{m_deleteMutex};
+        std::swap(m_newGarbage, m_toDelete);
+        lock.unlock();  // So destuctors can enqueue new objects
+        for (VlClass* const objp : m_toDelete) delete objp;
+        m_toDelete.clear();
+    }
+}
+
+//===========================================================================

include/verilated_types.h

@@ -30,6 +30,7 @@
 
 #include <algorithm>
 #include <array>
+#include <atomic>
 #include <deque>
 #include <map>
 #include <set>
@@ -1003,12 +1004,161 @@ std::string VL_TO_STRING(const VlUnpacked<T_Value, T_Depth>& obj) {
     return obj.to_string();
 }
 
+class VlClass;  // See below
+
+//===================================================================
+// Class providing delayed deletion of garbage objects. Objects get deleted only when 'deleteAll()'
+// is called, or the deleter itself is destroyed.
+
+class VlDeleter final {
+    // MEMBERS
+    // Queue of new objects that should be deleted
+    std::vector<VlClass*> m_newGarbage VL_GUARDED_BY(m_mutex);
+    // Queue of objects currently being deleted (only for deleteAll())
+    std::vector<VlClass*> m_toDelete VL_GUARDED_BY(m_deleteMutex);
+    mutable VerilatedMutex m_mutex;  // Mutex protecting the 'new garbage' queue
+    mutable VerilatedMutex m_deleteMutex;  // Mutex protecting the delete queue
+
+public:
+    // CONSTRUCTOR
+    VlDeleter() = default;
+    ~VlDeleter() { deleteAll(); }
+
+private:
+    VL_UNCOPYABLE(VlDeleter);
+
+public:
+    // METHODS
+    // Adds a new object to the 'new garbage' queue.
+    void put(VlClass* const objp) VL_MT_SAFE {
+        const VerilatedLockGuard lock{m_mutex};
+        m_newGarbage.push_back(objp);
+    }
+
+    // Deletes all queued garbage objects.
+    void deleteAll() VL_MT_SAFE;
+};
+
+//===================================================================
+// Base class for all verilated classes. Includes a reference counter, and a pointer to the deleter
+// object that should destroy it after the counter reaches 0. This allows for easy construction of
+// VlClassRefs from 'this'. Also declares a virtual constructor, so that the object can be deleted
+// using a base pointer.
+
+class VlClass VL_NOT_FINAL {
+    // TYPES
+    template <typename T_Class>
+    friend class VlClassRef;  // Needed for access to the ref counter and deleter
+
+    // MEMBERS
+    std::atomic<size_t> m_counter{0};  // Reference count for this object
+    VlDeleter* m_deleter = nullptr;  // The deleter that will delete this object
+
+    // METHODS
+    // Atomically increments the reference counter
+    void refCountInc() VL_MT_SAFE { ++m_counter; }
+    // Atomically decrements the reference counter. Assuming VlClassRef semantics are sound, it
+    // should never get called at m_counter == 0.
+    void refCountDec() VL_MT_SAFE {
+        if (!--m_counter) m_deleter->put(this);
+    }
+
+public:
+    // CONSTRUCTORS
+    VlClass() = default;
+    VlClass(const VlClass& copied) {}
+    virtual ~VlClass() {}
+};
+
 //===================================================================
 // Verilog class reference container
 // There are no multithreaded locks on this; the base variable must
 // be protected by other means
 
-#define VlClassRef std::shared_ptr
+template <typename T_Class>
+class VlClassRef final {
+private:
+    // TYPES
+    template <typename T_OtherClass>
+    friend class VlClassRef;  // Needed for template copy/move assignments
+
+    // MEMBERS
+    T_Class* m_objp = nullptr;  // Object pointed to
+
+    // METHODS
+    // Increase reference counter with null check
+    void refCountInc() const VL_MT_SAFE {
+        if (m_objp) m_objp->refCountInc();
+    }
+    // Decrease reference counter with null check
+    void refCountDec() const VL_MT_SAFE {
+        if (m_objp) m_objp->refCountDec();
+    }
+
+public:
+    // CONSTRUCTORS
+    VlClassRef() = default;
+    template <typename... T_Args>
+    VlClassRef(VlDeleter& deleter, T_Args&&... args)
+        : m_objp{new T_Class{std::forward<T_Args>(args)...}} {
+        m_objp->m_deleter = &deleter;
+        refCountInc();
+    }
+    VlClassRef(T_Class* objp)
+        : m_objp{objp} {
+        refCountInc();
+    }
+    VlClassRef(const VlClassRef& copied)
+        : m_objp{copied.m_objp} {
+        refCountInc();
+    }
+    VlClassRef(VlClassRef&& moved)
+        : m_objp{std::exchange(moved.m_objp, nullptr)} {}
+    ~VlClassRef() { refCountDec(); }
+
+    // METHODS
+    // Copy and move assignments
+    VlClassRef& operator=(const VlClassRef& copied) {
+        refCountDec();
+        m_objp = copied.m_objp;
+        refCountInc();
+        return *this;
+    }
+    VlClassRef& operator=(VlClassRef&& moved) {
+        refCountDec();
+        m_objp = std::exchange(moved.m_objp, nullptr);
+        return *this;
+    }
+    template <typename T_OtherClass>
+    VlClassRef& operator=(const VlClassRef<T_OtherClass>& copied) {
+        refCountDec();
+        m_objp = copied.m_objp;
+        refCountInc();
+        return *this;
+    }
+    template <typename T_OtherClass>
+    VlClassRef& operator=(VlClassRef<T_OtherClass>&& moved) {
+        refCountDec();
+        m_objp = std::exchange(moved.m_objp, nullptr);
+        return *this;
+    }
+    // Dynamic caster
+    template <typename T_OtherClass>
+    VlClassRef<T_OtherClass> dynamicCast() const {
+        return dynamic_cast<T_OtherClass*>(m_objp);
+    }
+    // Dereference operators
+    T_Class& operator*() const { return *m_objp; }
+    T_Class* operator->() const { return m_objp; }
+    // For 'if (ptr)...'
+    operator bool() const { return m_objp; }
+};
+
+#define VL_NEW(Class, ...) \
+    VlClassRef<Class> { vlSymsp->__Vm_deleter, __VA_ARGS__ }
+
+#define VL_KEEP_THIS \
+    VlClassRef<std::remove_pointer<decltype(this)>::type> __Vthisref { this }
 
 template <class T>  // T typically of type VlClassRef<x>
 inline T VL_NULL_CHECK(T t, const char* filename, int linenum) {
@@ -1018,7 +1168,7 @@ inline T VL_NULL_CHECK(T t, const char* filename, int linenum) {
 
 template <typename T, typename U>
 static inline bool VL_CAST_DYNAMIC(VlClassRef<T> in, VlClassRef<U>& outr) {
-    VlClassRef<U> casted = std::dynamic_pointer_cast<U>(in);
+    VlClassRef<U> casted = in.template dynamicCast<U>();
     if (VL_LIKELY(casted)) {
         outr = casted;
         return true;

src/V3EmitCFunc.h

@@ -419,7 +419,7 @@ public:
     }
     void visit(AstCNew* nodep) override {
         bool comma = false;
-        puts("std::make_shared<" + prefixNameProtect(nodep->dtypep()) + ">(");
+        puts("VL_NEW(" + prefixNameProtect(nodep->dtypep()) + ", ");
         puts("vlSymsp");  // TODO make this part of argsp, and eliminate when unnecessary
         if (nodep->argsp()) comma = true;
         for (AstNode* subnodep = nodep->argsp(); subnodep; subnodep = subnodep->nextp()) {
@@ -1054,7 +1054,7 @@ public:
         puts(")");
     }
     void visit(AstNewCopy* nodep) override {
-        puts("std::make_shared<" + prefixNameProtect(nodep->dtypep()) + ">(");
+        puts("VL_NEW(" + prefixNameProtect(nodep->dtypep()) + ", ");
         puts("*");  // i.e. make into a reference
         iterateAndNextNull(nodep->rhsp());
         puts(")");

src/V3EmitCHeaders.cpp

@@ -256,9 +256,11 @@ class EmitCHeader final : public EmitCConstInit {
         puts("\nclass ");
         puts(prefixNameProtect(modp));
         if (const AstClass* const classp = VN_CAST(modp, Class)) {
-            if (classp->extendsp()) {
             puts(" : public ");
+            if (classp->extendsp()) {
                 puts(prefixNameProtect(classp->extendsp()->classp()));
+            } else {
+                puts("VlClass");
             }
         } else {
             puts(" final : public VerilatedModule");

src/V3EmitCImp.cpp

@@ -75,6 +75,7 @@ class EmitCGatherDependencies final : VNVisitor {
         iterateChildrenConst(nodep);
     }
     void visit(AstCNew* nodep) override {
+        addSymsDependency();
         addDTypeDependency(nodep->dtypep());
         iterateChildrenConst(nodep);
     }
@@ -83,6 +84,7 @@ class EmitCGatherDependencies final : VNVisitor {
         iterateChildrenConst(nodep);
     }
     void visit(AstNewCopy* nodep) override {
+        addSymsDependency();
         addDTypeDependency(nodep->dtypep());
         iterateChildrenConst(nodep);
     }

src/V3EmitCModel.cpp

@@ -387,6 +387,7 @@ class EmitCModel final : public EmitCFunc {
         if (v3Global.opt.trace()) puts("vlSymsp->__Vm_activity = true;\n");
 
         if (v3Global.hasEvents()) puts("vlSymsp->clearTriggeredEvents();\n");
+        if (v3Global.hasClasses()) puts("vlSymsp->__Vm_deleter.deleteAll();\n");
 
         puts("if (VL_UNLIKELY(!vlSymsp->__Vm_didInit)) {\n");
         puts("vlSymsp->__Vm_didInit = true;\n");

src/V3EmitCSyms.cpp

@@ -446,6 +446,7 @@ void EmitCSyms::emitSymHdr() {
              "  ///< Used by trace routines when tracing multiple models\n");
     }
     if (v3Global.hasEvents()) puts("std::vector<VlEvent*> __Vm_triggeredEvents;\n");
+    if (v3Global.hasClasses()) puts("VlDeleter __Vm_deleter;\n");
     puts("bool __Vm_didInit = false;\n");
 
     if (v3Global.opt.mtasks()) {

src/V3Global.h

@@ -106,6 +106,7 @@ class V3Global final {
     bool m_needTraceDumper = false;  // Need __Vm_dumperp in symbols
     bool m_dpi = false;  // Need __Dpi include files
     bool m_hasEvents = false;  // Design uses SystemVerilog named events
+    bool m_hasClasses = false;  // Design uses SystemVerilog classes
     bool m_usesTiming = false;  // Design uses timing constructs
     bool m_hasForceableSignals = false;  // Need to apply V3Force pass
     bool m_hasSCTextSections = false;  // Has `systemc_* sections that need to be emitted
@@ -149,6 +150,8 @@ public:
     void dpi(bool flag) { m_dpi = flag; }
     bool hasEvents() const { return m_hasEvents; }
     void setHasEvents() { m_hasEvents = true; }
+    bool hasClasses() const { return m_hasClasses; }
+    void setHasClasses() { m_hasClasses = true; }
     bool usesTiming() const { return m_usesTiming; }
     void setUsesTiming() { m_usesTiming = true; }
     bool hasForceableSignals() const { return m_hasForceableSignals; }

src/V3SchedTiming.cpp

@@ -347,6 +347,7 @@ void transformForks(AstNetlist* const netlistp) {
                 nodep->replaceWith(callp);
                 // If we're in a class, add a vlSymsp arg
                 if (m_inClass) {
+                    newfuncp->addInitsp(new AstCStmt{nodep->fileline(), "VL_KEEP_THIS;\n"});
                     newfuncp->argTypes(EmitCBaseVisitor::symClassVar());
                     callp->argTypes("vlSymsp");
                 }

src/V3Timing.cpp

@@ -393,6 +393,8 @@ private:
         }
         if (nodep->user2() && !nodep->isCoroutine()) {  // If first marked as suspendable
             nodep->rtnType("VlCoroutine");
+            // If in a class, create a shared pointer to 'this'
+            if (m_classp) nodep->addInitsp(new AstCStmt{nodep->fileline(), "VL_KEEP_THIS;\n"});
             // Revisit dependent nodes if needed
             for (V3GraphEdge* edgep = vxp->inBeginp(); edgep; edgep = edgep->inNextp()) {
                 auto* const depVxp = static_cast<DependencyVertex*>(edgep->fromp());

src/verilog.y

@@ -6202,7 +6202,8 @@ classFront<classp>:             // IEEE: part of class_declaration
                         { $$ = new AstClass($2, *$4);
                           $$->isVirtual($1);
                           $$->lifetime($3);
-                          SYMP->pushNew($<classp>$); }
+                          SYMP->pushNew($<classp>$);
+                          v3Global.setHasClasses(); }
         //                      // IEEE: part of interface_class_declaration
         |       yINTERFACE yCLASS lifetimeE idAny/*class_identifier*/
                         { $$ = new AstClass($2, *$4);

test_regress/t/t_class_member_sens.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 2022 by Antmicro Ltd. 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(vlt => 1);
+
+compile(
+    sanitize => 1,
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_class_member_sens.v

@@ -0,0 +1,30 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+module t (/*AUTOARG*/
+    // Inputs
+    clk
+    );
+    input clk;
+
+    class EventClass;
+        event e;
+    endclass
+
+    EventClass ec = new;
+    int cyc = 0;
+
+    always @ec.e ec = new;
+
+    always @(posedge clk) begin
+       cyc <= cyc + 1;
+       if (cyc == 1) ->ec.e;
+       else if (cyc == 2) begin
+          $write("*-* All Finished *-*\n");
+          $finish;
+       end
+   end
+endmodule

test_regress/t/t_timing_class.v

@@ -84,6 +84,7 @@ module t;
         logic y;
         task do_assign;
             y = #10 x;
+            `WRITE_VERBOSE(("Did assignment with delay\n"));
         endtask
     endclass
 
@@ -121,7 +122,10 @@ module t;
         if ($time != 80) $stop;
         if (event_trig_count != 2) $stop;
         if (dAsgn.y != 1) $stop;
-        $write("*-* All Finished *-*\n");
+        // Test if the object is deleted before do_assign finishes:
+        fork dAsgn.do_assign; join_none
+        #5 dAsgn = null;
+        #15 $write("*-* All Finished *-*\n");
         $finish;
     end
 
@@ -162,5 +166,5 @@ module t;
         if (fc.done != 4 || $time != 70) $stop;
     end
 
-    initial #81 $stop; // timeout
+    initial #101 $stop; // timeout
 endmodule