Merge 0be75ae537 into 2fabf50801

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.

docs/guide/simulating.rst

@@ -431,8 +431,7 @@ Profiling ccache efficiency
 The Verilator-generated Makefile supports basic profiling of ccache
 behavior during the build. This can be used to track down files that might
 be unnecessarily rebuilt, though as of today, even minor code changes will
-usually require rebuilding a large number of files. Improving ccache
-efficiency during the edit/compile/test loop is an active development area.
+usually require rebuilding a large number of files.
 
 To get a basic report of how well ccache is doing, add the `ccache-report`
 target when invoking the generated Makefile:
@@ -445,8 +444,9 @@ This will print a report based on all executions of ccache during this
 invocation of Make. The report is also written to a file, in this example
 `obj_dir/Vout__cache_report.txt`.
 
-To use the `ccache-report` target, at least one other explicit build target
-must be specified, and OBJCACHE must be set to 'ccache'.
+To use the `ccache-report` target, OBJCACHE must be set to 'ccache' (this is
+done by `configure` if 'ccache' is installed). If no other explicit build
+targets are specified, `cchache-report` will build the `default` target.
 
 This feature is currently experimental and might change in subsequent
 releases.

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,

include/verilated.mk.in

@@ -311,30 +311,27 @@ ifneq ($(findstring ccache-report,$(MAKECMDGOALS)),)
   endif
   VK_OTHER_GOALS := $(strip $(subst ccache-report,,$(MAKECMDGOALS)))
   ifeq ($(VK_OTHER_GOALS),)
-    $(error ccache-report must be used with at least one other explicit target)
+    VK_OTHER_GOALS := default
   endif
 
   # Report ccache behaviour for this invocation of make
   VK_CCACHE_LOGDIR := ccache-logs
   VK_CCACHE_REPORT := $(VM_PREFIX)__ccache_report.txt
-  # Remove previous logfiles and report
+
+  # Remove previous logfiles and report, then create log directory
   $(shell rm -rf $(VK_CCACHE_LOGDIR) $(VK_CCACHE_REPORT))
+  $(shell mkdir -p $(VK_CCACHE_LOGDIR))
 
-  $(VK_CCACHE_LOGDIR):
-	mkdir -p $@
-
-  $(VK_OBJS): | $(VK_CCACHE_LOGDIR)
-
+  # Add ccache logging to compilation rules
   $(VK_OBJS): export CCACHE_LOGFILE=$(VK_CCACHE_LOGDIR)/$@.log
 
-  $(VK_CCACHE_REPORT): $(VK_OBJS)
+  # ccache-report runs last
+  $(VK_CCACHE_REPORT): $(VK_OBJS) $(VK_OTHER_GOALS)
 	$(VERILATOR_CCACHE_REPORT) -o $@ $(VK_CCACHE_LOGDIR)
 
-  # ccache-report runs last
   .PHONY: ccache-report
-  ccache-report: $(VK_CCACHE_REPORT) $(VK_OTHER_GOALS)
+  ccache-report: $(VK_CCACHE_REPORT)
 	@cat $<
-
 endif
 
 ######################################################################

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
-            const string& name = EmitCUtil::prefixNameProtect(modp);
-            putsDecoration(nullptr, "\n// CONSTRUCTORS\n");
-            putns(modp,
-                  name + "(" + EmitCUtil::symClassName() + "* symsp, const char* v__name);\n");
+        // Classes use CFuncs with isConstructor/isDestructor
+        if (VN_IS(modp, Class)) return;
+
+        // The root module needs a proper constuctor/destructor, everything
+        // 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()
-                        + "* symsp, const char* v__name)\n");
-        puts("    : VerilatedModule{v__name}\n");
+        const std::string ctorArgs = EmitCUtil::symClassName() + "* symsp, const char* namep";
 
-        ofp()->indentInc();
-        for (const AstNode* nodep = modp->stmtsp(); nodep; nodep = nodep->nextp()) {
-            if (const AstVar* const varp = VN_CAST(nodep, Var)) {
-                if (const AstBasicDType* const dtypep
-                    = VN_CAST(varp->dtypeSkipRefp(), BasicDType)) {
-                    if (dtypep->keyword().isMTaskState()) {
-                        puts(", ");
-                        putns(varp, varp->nameProtect());
-                        puts("(");
-                        iterateConst(varp->valuep());
-                        puts(")\n");
-                    } else if (varp->isIO() && varp->isSc()) {
-                        puts(", ");
-                        putns(varp, varp->nameProtect());
-                        puts("(");
-                        putsQuoted(varp->nameProtect());
-                        puts(")\n");
-                    } else if (dtypep->isDelayScheduler()) {
-                        puts(", ");
-                        putns(varp, varp->nameProtect());
-                        puts("{*symsp->_vm_contextp__}\n");
+        // The root module needs a proper constuctor, everything else uses a
+        // 'ctor' function in order to be able to split up constructors
+        if (modp->isTop()) {
+            putns(modp, modName + "::" + modName + "(" + ctorArgs + ")\n");
+
+            ofp()->indentInc();
+            const char* sepp = "  : ";
+            for (const AstNode* nodep = modp->stmtsp(); nodep; nodep = nodep->nextp()) {
+                if (const AstVar* const varp = VN_CAST(nodep, Var)) {
+                    if (const AstBasicDType* const dtypep
+                        = VN_CAST(varp->dtypeSkipRefp(), BasicDType)) {
+                        if (dtypep->keyword().isMTaskState()) {
+                            puts(sepp);
+                            putns(varp, varp->nameProtect());
+                            puts("(");
+                            iterateConst(varp->valuep());
+                            puts(")\n");
+                        } else if (varp->isIO() && varp->isSc()) {
+                            puts(sepp);
+                            putns(varp, varp->nameProtect());
+                            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(), "
-                 "count32p,");
+            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), vlNamep, 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(), "
-                 "count32p,");
+            puts("VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), vlNamep, 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)
-                        + "() {\n");
+        if (modp->isTop()) {
+            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

@@ -105,7 +105,7 @@ class EmitCSyms final : EmitCBaseVisitorConst {
     size_t m_funcNum = 0;  // CFunc split function number
     V3OutCFile* m_ofpBase = nullptr;  // Base (not split) C file
     AstCFile* m_ofpBaseFile = nullptr;  // Base (not split) AstCFile
-    std::unordered_map<int, bool> m_usesVfinal;  // Split method uses __Vfinal
+    std::vector<std::pair<size_t, bool>> m_usesVfinal;  // Split file index + uses __Vfinal
     VDouble0 m_statVarScopeBytes;  // Statistic tracking
     const std::string m_symsFileBase = v3Global.opt.makeDir() + "/" + symClassName();
 
@@ -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()) {
@@ -620,7 +620,7 @@ void EmitCSyms::checkSplit(bool usesVfinal) {
     puts(");\n");
 
     // Create new split file
-    m_usesVfinal[funcNum] = usesVfinal;
+    m_usesVfinal.emplace_back(funcNum, usesVfinal);
     const std::string filename = m_symsFileBase + "__" + std::to_string(funcNum) + ".cpp";
     AstCFile* const cfilep = newCFile(filename, true /*slow*/, true /*source*/);
     cfilep->support(true);
@@ -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");
-        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");
+        puts("{this, namep}\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()) {

src/V3Randomize.cpp

@@ -2633,7 +2633,6 @@ class RandomizeVisitor final : public VNVisitor {
                 AstWith* const withp = VN_CAST(pinp, With);
                 if (withp) {
                     FileLine* const fl = nodep->fileline();
-                    // Capture variables in 'with {}' (nullptr = no target class)
                     captured = new CaptureVisitor{withp->exprp(), m_modp, nullptr};
                     captured->addFunctionArguments(randomizeFuncp);
                     // Clear old constraints and variables for std::randomize with clause
@@ -2689,12 +2688,20 @@ class RandomizeVisitor final : public VNVisitor {
                                              VN_AS(randomizeFuncp->fvarp(), Var), VAccess::READ},
                                basicMethodp}});
             }
+            // Remove With nodes from pins as they have been processed
+            for (AstNode* pinp = nodep->pinsp(); pinp;) {
+                AstNode* const nextp = pinp->nextp();
+                if (VN_IS(pinp, With)) {
+                    VL_DO_DANGLING(pinp->unlinkFrBack()->deleteTree(), pinp);
+                }
+                pinp = nextp;
+            }
             // Replace the node with a call to that function
             nodep->name(randomizeFuncp->name());
             nodep->taskp(randomizeFuncp);
             nodep->dtypeFrom(randomizeFuncp->dtypep());
             if (VN_IS(m_modp, Class)) nodep->classOrPackagep(m_modp);
-            if (nodep->pinsp()) pushDeletep(nodep->pinsp()->unlinkFrBackWithNext());
+            // if (nodep->pinsp()) pushDeletep(nodep->pinsp()->unlinkFrBackWithNext());
             if (captured) nodep->addPinsp(captured->getArgs());
             UINFOTREE(9, nodep, "", "std::rnd-call");
             UINFOTREE(9, randomizeFuncp, "", "std::rnd-func");

test_regress/t/t_ccache_report.py

@@ -36,7 +36,7 @@ test.files_identical(report, "t/" + test.name + "__ccache_report_initial.out")
 test.run(logfile=test.obj_dir + "/rebuild.log",
          cmd=[
              os.environ["MAKE"], "-C " + test.obj_dir, "-f " + test.vm_prefix + ".mk",
-             test.vm_prefix, "ccache-report"
+             "ccache-report"
          ])
 
 test.files_identical(report, "t/" + test.name + "__ccache_report_rebuild.out")

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);");