Use explicit ctor/dtor functions for VerilatedModules (#6660)

In order to avoid long compile times of the Syms constructor due to
having a very large number of member constructor sto call, move to using
explicit ctor/dtor functions for all but the root VerilatedModule. The
root module needs a constructor as it has non-default-constructible
members. The other modules don't.

This is only part of the fix, as in order to avoid having a default
constructor call the VerilatedModule needs to be default constructible.
I think this is now true for modules that do not contain strings or
other non trivially constructible/destructible variables.

Patch 1 of 3 to fix long compile times of the Syms module in some
scenarios.

include/verilated.cpp

@@ -3415,18 +3415,6 @@ VerilatedModel::VerilatedModel(VerilatedContext& context)
 
 std::unique_ptr<VerilatedTraceConfig> VerilatedModel::traceConfig() const { return nullptr; }
 
-//===========================================================================
-// VerilatedModule:: Methods
-
-VerilatedModule::VerilatedModule(const char* namep)
-    : m_namep{strdup(namep)} {}
-
-VerilatedModule::~VerilatedModule() {
-    // Memory cleanup - not called during normal operation
-    // cppcheck-suppress cstyleCast  // NOLINTNEXTLINE(google-readability-casting)
-    if (m_namep) VL_DO_CLEAR(free((void*)(m_namep)), m_namep = nullptr);
-}
-
 //======================================================================
 // VerilatedVar:: Methods
 

include/verilated.h

@@ -308,20 +308,6 @@ private:
     virtual std::unique_ptr<VerilatedTraceConfig> traceConfig() const;
 };
 
-//=========================================================================
-/// Base class for all Verilated module classes.
-
-class VerilatedModule VL_NOT_FINAL {
-    VL_UNCOPYABLE(VerilatedModule);
-
-private:
-    const char* m_namep;  // Module name
-public:
-    explicit VerilatedModule(const char* namep);  // Create module with given hierarchy name
-    ~VerilatedModule();
-    const char* name() const VL_MT_SAFE_POSTINIT { return m_namep; }  ///< Return name of module
-};
-
 //=========================================================================
 // Functions overridable by user defines
 // (Internals however must use VL_PRINTF_MT, which calls these.)
@@ -720,7 +706,7 @@ private:
     int8_t m_timeunit = 0;  // Timeunit in negative power-of-10
     Type m_type = SCOPE_OTHER;  // Type of the scope
 
-public:  // But internals only - called from VerilatedModule's
+public:  // But internals only - called from verilated modules
     VerilatedScope() = default;
     ~VerilatedScope();
     void configure(VerilatedSyms* symsp, const char* prefixp, const char* suffixp,

src/V3Common.cpp

@@ -71,7 +71,7 @@ static void makeVlToString(AstIface* nodep) {
     funcp->isConst(false);
     funcp->isStatic(false);
     funcp->protect(false);
-    AstNodeExpr* const exprp = new AstCExpr{nodep->fileline(), "obj ? obj->name() : \"null\""};
+    AstNodeExpr* const exprp = new AstCExpr{nodep->fileline(), "obj ? obj->vlNamep : \"null\""};
     exprp->dtypeSetString();
     funcp->addStmtsp(new AstCReturn{nodep->fileline(), exprp});
     nodep->addStmtsp(funcp);

src/V3EmitCFunc.cpp

@@ -732,7 +732,7 @@ void EmitCFunc::emitVarResetScopeHash() {
             = std::to_string(VString::hashMurmur(m_classOrPackage->name())) + "ULL";
     } else {
         puts(string("const uint64_t __VscopeHash = VL_MURMUR64_HASH(")
-             + (m_useSelfForThis ? "vlSelf" : "this") + "->name());\n");
+             + (m_useSelfForThis ? "vlSelf" : "this") + "->vlNamep);\n");
     }
     m_createdScopeHash = true;
 }

src/V3EmitCHeaders.cpp

@@ -130,7 +130,8 @@ class EmitCHeader final : public EmitCConstInit {
             }
         } else {  // not class
             putsDecoration(nullptr, "\n// INTERNAL VARIABLES\n");
-            puts(EmitCUtil::symClassName() + "* const vlSymsp;\n");
+            puts(EmitCUtil::symClassName() + "* vlSymsp;\n");
+            puts("const char* vlNamep;\n");
         }
     }
     void emitParamDecls(const AstNodeModule* modp) {
@@ -155,14 +156,25 @@ class EmitCHeader final : public EmitCConstInit {
         }
     }
     void emitCtorDtorDecls(const AstNodeModule* modp) {
-        if (!VN_IS(modp, Class)) {  // Classes use CFuncs with isConstructor/isDestructor
+        // Classes use CFuncs with isConstructor/isDestructor
-            const string& name = EmitCUtil::prefixNameProtect(modp);
+        if (VN_IS(modp, Class)) return;
-            putsDecoration(nullptr, "\n// CONSTRUCTORS\n");
+
-            putns(modp,
+        // The root module needs a proper constuctor/destructor, everything
-                  name + "(" + EmitCUtil::symClassName() + "* symsp, const char* v__name);\n");
+        // else uses a 'ctor'/'dtor' function in order to be able to split up
+        // construction/destruction code
+        const std::string name = EmitCUtil::prefixNameProtect(modp);
+        putsDecoration(nullptr, "\n// CONSTRUCTORS\n");
+        const std::string ctorArgs = EmitCUtil::symClassName() + "* symsp, const char* namep";
+        if (modp->isTop()) {
+            putns(modp, name + "(" + ctorArgs + ");\n");
             putns(modp, "~" + name + "();\n");
-            putns(modp, "VL_UNCOPYABLE(" + name + ");\n");
+        } else {
+            putns(modp, name + "() = default;\n");
+            putns(modp, "~" + name + "() = default;\n");
+            putns(modp, "void ctor(" + ctorArgs + ");\n");
+            putns(modp, "void dtor();\n");
         }
+        putns(modp, "VL_UNCOPYABLE(" + name + ");\n");
     }
     void emitInternalMethodDecls(const AstNodeModule* modp) {
         bool first = true;
@@ -590,7 +602,7 @@ class EmitCHeader final : public EmitCConstInit {
                 puts("public virtual VlClass");
             }
         } else {
-            puts(" final : public VerilatedModule");
+            puts(" final");
         }
         puts(" {\n");
         ofp()->resetPrivate();

src/V3EmitCImp.cpp

@@ -113,46 +113,61 @@ class EmitCImp final : EmitCFunc {
         if (!first) puts("\n");
     }
     void emitCtorImp(const AstNodeModule* modp) {
-        const string modName = EmitCUtil::prefixNameProtect(modp);
+        const std::string modName = EmitCUtil::prefixNameProtect(modp);
 
         puts("\n");
         m_lazyDecls.emit("void " + modName + "__", protect("_ctor_var_reset"),
                          "(" + modName + "* vlSelf);");
         puts("\n");
 
-        putns(modp, modName + "::" + modName + "(" + EmitCUtil::symClassName()
+        const std::string ctorArgs = EmitCUtil::symClassName() + "* symsp, const char* namep";
-                        + "* symsp, const char* v__name)\n");
-        puts("    : VerilatedModule{v__name}\n");
 
-        ofp()->indentInc();
+        // The root module needs a proper constuctor, everything else uses a
-        for (const AstNode* nodep = modp->stmtsp(); nodep; nodep = nodep->nextp()) {
+        // 'ctor' function in order to be able to split up constructors
-            if (const AstVar* const varp = VN_CAST(nodep, Var)) {
+        if (modp->isTop()) {
-                if (const AstBasicDType* const dtypep
+            putns(modp, modName + "::" + modName + "(" + ctorArgs + ")\n");
-                    = VN_CAST(varp->dtypeSkipRefp(), BasicDType)) {
+
-                    if (dtypep->keyword().isMTaskState()) {
+            ofp()->indentInc();
-                        puts(", ");
+            const char* sepp = "  : ";
-                        putns(varp, varp->nameProtect());
+            for (const AstNode* nodep = modp->stmtsp(); nodep; nodep = nodep->nextp()) {
-                        puts("(");
+                if (const AstVar* const varp = VN_CAST(nodep, Var)) {
-                        iterateConst(varp->valuep());
+                    if (const AstBasicDType* const dtypep
-                        puts(")\n");
+                        = VN_CAST(varp->dtypeSkipRefp(), BasicDType)) {
-                    } else if (varp->isIO() && varp->isSc()) {
+                        if (dtypep->keyword().isMTaskState()) {
-                        puts(", ");
+                            puts(sepp);
-                        putns(varp, varp->nameProtect());
+                            putns(varp, varp->nameProtect());
-                        puts("(");
+                            puts("(");
-                        putsQuoted(varp->nameProtect());
+                            iterateConst(varp->valuep());
-                        puts(")\n");
+                            puts(")\n");
-                    } else if (dtypep->isDelayScheduler()) {
+                        } else if (varp->isIO() && varp->isSc()) {
-                        puts(", ");
+                            puts(sepp);
-                        putns(varp, varp->nameProtect());
+                            putns(varp, varp->nameProtect());
-                        puts("{*symsp->_vm_contextp__}\n");
+                            puts("(");
+                            putsQuoted(varp->nameProtect());
+                            puts(")\n");
+                        } else if (dtypep->isDelayScheduler()) {
+                            puts(sepp);
+                            putns(varp, varp->nameProtect());
+                            puts("{*symsp->_vm_contextp__}\n");
+                        } else {
+                            continue;
+                        }
+                        sepp = ", ";
                     }
                 }
             }
+            ofp()->indentDec();
+            puts(" {\n");
+        } else {
+            putns(modp, "void " + modName + "::ctor(" + ctorArgs + ") {\n");
         }
-        puts(", vlSymsp{symsp}\n");
-        ofp()->indentDec();
 
-        puts(" {\n");
+        puts("vlSymsp = symsp;\n");
+        if (modp->isTop()) {
+            puts("vlNamep = strdup(namep);\n");
+        } else {
+            puts("vlNamep = strdup(Verilated::catName(vlSymsp->name(), namep));\n");
+        }
 
         putsDecoration(modp, "// Reset structure values\n");
         puts(modName + "__" + protect("_ctor_var_reset") + "(this);\n");
@@ -195,13 +210,12 @@ class EmitCImp final : EmitCFunc {
             }
             // static doesn't need save-restore as is constant
             puts("static uint32_t fake_zero_count = 0;\n");
-            puts("std::string fullhier = std::string{VerilatedModule::name()} + hierp;\n");
+            puts("std::string fullhier = std::string{vlNamep} + hierp;\n");
             puts("if (!fullhier.empty() && fullhier[0] == '.') fullhier = fullhier.substr(1);\n");
             // Used for second++ instantiation of identical bin
             puts("if (!enable) count32p = &fake_zero_count;\n");
             puts("*count32p = 0;\n");
-            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), VerilatedModule::name(), "
+            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), vlNamep, count32p,");
-                 "count32p,");
             puts("  \"filename\",filenamep,");
             puts("  \"lineno\",lineno,");
             puts("  \"column\",column,\n");
@@ -232,7 +246,7 @@ class EmitCImp final : EmitCFunc {
             }
             // static doesn't need save-restore as is constant
             puts("static uint32_t fake_zero_count = 0;\n");
-            puts("std::string fullhier = std::string{VerilatedModule::name()} + hierp;\n");
+            puts("std::string fullhier = std::string{vlNamep} + hierp;\n");
             puts("if (!fullhier.empty() && fullhier[0] == '.') fullhier = fullhier.substr(1);\n");
             puts("std::string commentWithIndex = commentp;\n");
             puts("if (ranged) commentWithIndex += '[' + std::to_string(i) + ']';\n");
@@ -240,8 +254,7 @@ class EmitCImp final : EmitCFunc {
             // Used for second++ instantiation of identical bin
             puts("if (!enable) count32p = &fake_zero_count;\n");
             puts("*count32p = 0;\n");
-            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), VerilatedModule::name(), "
+            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), vlNamep, count32p,");
-                 "count32p,");
             puts("  \"filename\",filenamep,");
             puts("  \"lineno\",lineno,");
             puts("  \"column\",column,\n");
@@ -257,9 +270,14 @@ class EmitCImp final : EmitCFunc {
         }
     }
     void emitDestructorImp(const AstNodeModule* modp) {
+        const std::string modName = EmitCUtil::prefixNameProtect(modp);
         puts("\n");
-        putns(modp, EmitCUtil::prefixNameProtect(modp) + "::~" + EmitCUtil::prefixNameProtect(modp)
+        if (modp->isTop()) {
-                        + "() {\n");
+            putns(modp, modName + "::~" + modName + "() {\n");
+        } else {
+            putns(modp, "void " + modName + "::dtor() {\n");
+        }
+        putns(modp, "VL_DO_DANGLING(free(const_cast<char*>(vlNamep)), vlNamep);\n");
         emitSystemCSection(modp, VSystemCSectionType::DTOR);
         puts("}\n");
     }

src/V3EmitCSyms.cpp

@@ -544,7 +544,7 @@ void EmitCSyms::emitSymHdr() {
     }
 
     puts("\n// METHODS\n");
-    puts("const char* name() { return TOP.name(); }\n");
+    puts("const char* name() { return TOP.vlNamep; }\n");
 
     if (v3Global.hasEvents()) {
         if (v3Global.assignsEvents()) {
@@ -738,6 +738,15 @@ void EmitCSyms::emitSymImp() {
         puts("_vm_pgoProfiler.write(\"" + topClassName()
              + "\", _vm_contextp__->profVltFilename());\n");
     }
+    puts("// Tear down sub module instances\n");
+    for (const ScopeModPair& itpair : vlstd::reverse_view(m_scopes)) {
+        const AstScope* const scopep = itpair.first;
+        const AstNodeModule* const modp = itpair.second;
+        if (modp->isTop()) continue;
+        putns(scopep, protect(scopep->nameDotless()));
+        puts(".dtor();\n");
+        ++m_numStmts;
+    }
     puts("}\n");
 
     if (v3Global.needTraceDumper()) {
@@ -788,23 +797,16 @@ void EmitCSyms::emitSymImp() {
         puts("    , _vm_pgoProfiler{" + std::to_string(v3Global.currentHierBlockCost()) + "}\n");
     }
 
-    puts("    // Setup module instances\n");
+    puts("    // Setup top module instance\n");
     for (const ScopeModPair& itpair : m_scopes) {
         const AstScope* const scopep = itpair.first;
         const AstNodeModule* const modp = itpair.second;
+        if (!modp->isTop()) continue;
         puts("    , ");
         putns(scopep, protect(scopep->nameDotless()));
-        puts("{this");
+        puts("{this, namep}\n");
-        if (modp->isTop()) {
-            puts(", namep");
-        } else {
-            // The "." is added by catName
-            puts(", Verilated::catName(namep, ");
-            putsQuoted(VIdProtect::protectWordsIf(scopep->prettyName(), scopep->protect()));
-            puts(")");
-        }
-        puts("}\n");
         ++m_numStmts;
+        break;
     }
     puts("{\n");
 
@@ -819,6 +821,18 @@ void EmitCSyms::emitSymImp() {
         V3Stats::addStat(V3Stats::STAT_MODEL_SIZE, stackSize + m_statVarScopeBytes);
     }
 
+    puts("// Setup sub module instances\n");
+    for (const ScopeModPair& itpair : m_scopes) {
+        const AstScope* const scopep = itpair.first;
+        const AstNodeModule* const modp = itpair.second;
+        if (modp->isTop()) continue;
+        putns(scopep, protect(scopep->nameDotless()));
+        puts(".ctor(this, ");
+        putsQuoted(VIdProtect::protectWordsIf(scopep->prettyName(), scopep->protect()));
+        puts(");\n");
+        ++m_numStmts;
+    }
+
     if (v3Global.opt.profPgo()) {
         puts("// Configure profiling for PGO\n");
         if (!v3Global.opt.hierChild()) {

test_regress/t/t_dpi_var.v

@@ -111,7 +111,7 @@ module sub (/*AUTOARG*/
 
    initial begin
       // Test the naming
-      $c("mon_class_name(this->name());");
+      $c("mon_class_name(this->vlNamep);");
       mon_scope_name("%m");
       // Scheme A - pass pointer directly
       $c("mon_register_a(\"in\", &", in, ", false, 0, 1);");