From 8ad8d4f8077a0225ec626c64033d6d84ea05ef31 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 8 Nov 2025 15:56:15 +0000 Subject: [PATCH] Fix long C++ compilation due to VerilatedScope constructors (#6664) The Syms class can contain a very large number of VeriltedScope instances if `--vpi` is used, all of which need a call to the default constructor in the constructor of the Syms class. This can lead to very long compilation times, even without optimization on some compilers. To avoid the constructor calls, hold VeriltedScope via pointers in the Syms class, and explicitly new and delete them in the Syms constructor/destructor. These explicit new/delte can then be automatically split up into sub functions when the Syms constructor/destructor become large. Regarding run-time performance, this should have no significant effect, most interactions are either during construction/destruction of the Syms object, or are via pointers already. The one place where we used to refer to VerilatedScope instances is when emitting an AstScopeName for things like $display %m. For those there will be an extra load instruction at run-time, which should not make a big difference. Patch 3 of 3 to fix long compile times of the Syms module in some scenarios. --- include/verilated.cpp | 41 +++++++++++++------------- include/verilated.h | 21 +++++++------ src/V3AstNodeExpr.h | 6 ++-- src/V3EmitCFunc.h | 2 +- src/V3EmitCSyms.cpp | 27 ++++++++++------- test_regress/t/t_flag_csplit_groups.py | 2 +- 6 files changed, 52 insertions(+), 47 deletions(-) diff --git a/include/verilated.cpp b/include/verilated.cpp index 5f1b6bae0..0720f11bb 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -3454,37 +3454,36 @@ void* VerilatedVarProps::datapAdjustIndex(void* datap, int dim, int indx) const //====================================================================== // VerilatedScope:: Methods -VerilatedScope::~VerilatedScope() { - // Memory cleanup - not called during normal operation - Verilated::threadContextp()->impp()->scopeErase(this); - if (m_namep) VL_DO_CLEAR(delete[] m_namep, m_namep = nullptr); - if (m_callbacksp) VL_DO_CLEAR(delete[] m_callbacksp, m_callbacksp = nullptr); - if (m_varsp) VL_DO_CLEAR(delete m_varsp, m_varsp = nullptr); - m_funcnumMax = 0; // Force callback table to empty -} - -void VerilatedScope::configure(VerilatedSyms* symsp, const char* prefixp, const char* suffixp, +VerilatedScope::VerilatedScope(VerilatedSyms* symsp, const char* prefixp, const char* suffixp, const char* identifier, const char* defnamep, int8_t timeunit, - const Type& type) VL_MT_UNSAFE { - // Slowpath - called once/scope at construction - // We don't want the space and reference-count access overhead of strings. - m_symsp = symsp; - m_type = type; - m_timeunit = timeunit; - { + Type type) + : m_symsp{symsp} + , m_namep{[prefixp, suffixp]() { + // We don't want the space and reference-count access overhead of strings. char* const namep = new char[std::strlen(prefixp) + std::strlen(suffixp) + 2]; char* dp = namep; for (const char* sp = prefixp; *sp;) *dp++ = *sp++; if (*prefixp && *suffixp) *dp++ = '.'; for (const char* sp = suffixp; *sp;) *dp++ = *sp++; *dp++ = '\0'; - m_namep = namep; - } - m_identifierp = identifier; - m_defnamep = defnamep; + return namep; + }()} + , m_identifierp{identifier} + , m_defnamep{defnamep} + , m_timeunit{timeunit} + , m_type{type} { Verilated::threadContextp()->impp()->scopeInsert(this); } +VerilatedScope::~VerilatedScope() { + // Memory cleanup - not called during normal operation + Verilated::threadContextp()->impp()->scopeErase(this); + VL_DO_DANGLING(delete[] m_namep, m_namep); + VL_DO_DANGLING(delete[] m_callbacksp, m_callbacksp); + VL_DO_DANGLING(delete m_varsp, m_varsp); + VL_DEBUG_IFDEF(m_funcnumMax = 0;); +} + void VerilatedScope::exportInsert(int finalize, const char* namep, void* cb) VL_MT_UNSAFE { // Slowpath - called once/scope*export at construction // Insert a exported function into scope table diff --git a/include/verilated.h b/include/verilated.h index 5b7795652..314529656 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -695,23 +695,22 @@ public: }; // Type of a scope, currently only module and package are interesting private: // Fastpath: - VerilatedSyms* m_symsp = nullptr; // Symbol table + VerilatedSyms* const m_symsp; // Symbol table void** m_callbacksp = nullptr; // Callback table pointer (Fastpath) int m_funcnumMax = 0; // Maximum function number stored (Fastpath) // 4 bytes padding (on -m64), for rent. VerilatedVarNameMap* m_varsp = nullptr; // Variable map - const char* m_namep = nullptr; // Scope name (Slowpath) - const char* m_identifierp = nullptr; // Identifier of scope (with escapes removed) - const char* m_defnamep = nullptr; // Definition name (SCOPE_MODULE only) - int8_t m_timeunit = 0; // Timeunit in negative power-of-10 - Type m_type = SCOPE_OTHER; // Type of the scope + const char* const m_namep; // Scope name (Slowpath) + const char* const m_identifierp; // Identifier of scope (with escapes removed) + const char* const m_defnamep; // Definition name (SCOPE_MODULE only) + const int8_t m_timeunit; // Timeunit in negative power-of-10 + const Type m_type; // Type of the scope -public: // But internals only - called from verilated modules - VerilatedScope() = default; +public: // But internals only - called from verilated modules, VerilatedSyms + VerilatedScope(VerilatedSyms* symsp, const char* prefixp, const char* suffixp, + const char* identifier, const char* defnamep, int8_t timeunit, Type type); ~VerilatedScope(); - void configure(VerilatedSyms* symsp, const char* prefixp, const char* suffixp, - const char* identifier, const char* defnamep, int8_t timeunit, - const Type& type) VL_MT_UNSAFE; + void exportInsert(int finalize, const char* namep, void* cb) VL_MT_UNSAFE; void varInsert(const char* namep, void* datap, bool isParam, VerilatedVarType vltype, int vlflags, int udims, int pdims, ...) VL_MT_UNSAFE; diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 749b1e0a5..b1cccc769 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -2173,13 +2173,13 @@ public: bool dpiExport() const { return m_dpiExport; } void dpiExport(bool flag) { m_dpiExport = flag; } bool forFormat() const { return m_forFormat; } - // Name for __Vscope variable including children + // Name for __Vscopep variable including children string scopeSymName() const { return scopeNameFormatter(m_scopeAttr); } // Name for DPI import scope string scopeDpiName() const { return scopeNameFormatter(m_scopeEntr); } - // Name for __Vscope variable including children + // Name for __Vscopep variable including children string scopePrettySymName() const { return scopePrettyNameFormatter(m_scopeAttr); } - // Name for __Vscope variable including children + // Name for __Vscopep variable including children string scopePrettyDpiName() const { return scopePrettyNameFormatter(m_scopeEntr); } }; class AstSelLoopVars final : public AstNodeExpr { diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index f9763cfd2..f1538d230 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -919,7 +919,7 @@ public: if (!nodep->dpiExport()) { // this is where the DPI import context scope is set const string scope = nodep->scopeDpiName(); - putnbs(nodep, "(&(vlSymsp->" + protect("__Vscope_" + scope) + "))"); + putnbs(nodep, "(vlSymsp->" + protect("__Vscopep_" + scope) + ")"); } } void visit(AstSFormat* nodep) override { diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index eb53d0c4a..81e15bff3 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -535,7 +535,7 @@ void EmitCSyms::emitSymHdr() { puts("\n// SCOPE NAMES\n"); for (const auto& itpair : m_scopeNames) { const ScopeData& sd = itpair.second; - putns(sd.m_nodep, "VerilatedScope " + protect("__Vscope_" + sd.m_symName) + ";\n"); + putns(sd.m_nodep, "VerilatedScope* " + protect("__Vscopep_" + sd.m_symName) + ";\n"); } } @@ -632,16 +632,16 @@ void EmitCSyms::emitScopeHier(std::vector& stmts, bool destroy) { const std::string& scopeType = sd.m_type; if (name.find('.') != string::npos) continue; if (scopeType != "SCOPE_MODULE" && scopeType != "SCOPE_PACKAGE") continue; - const std::string id = protect("__Vscope_" + sd.m_symName); - stmts.emplace_back("__Vhier." + method + "(0, &" + id + ");"); + const std::string id = protect("__Vscopep_" + sd.m_symName); + stmts.emplace_back("__Vhier." + method + "(0, " + id + ");"); } for (const auto& itpair : m_vpiScopeHierarchy) { const std::string fromName = scopeSymString(itpair.first); - const std::string fromId = protect("__Vscope_" + m_scopeNames.at(fromName).m_symName); + const std::string fromId = protect("__Vscopep_" + m_scopeNames.at(fromName).m_symName); for (const std::string& name : itpair.second) { const std::string toName = scopeSymString(name); - const std::string toId = protect("__Vscope_" + m_scopeNames.at(toName).m_symName); - stmts.emplace_back("__Vhier." + method + "(&" + fromId + ", &" + toId + ");"); + const std::string toId = protect("__Vscopep_" + m_scopeNames.at(toName).m_symName); + stmts.emplace_back("__Vhier." + method + "(" + fromId + ", " + toId + ");"); } } } @@ -736,7 +736,7 @@ std::vector EmitCSyms::getSymCtorStmts() { for (const auto& itpair : m_scopeNames) { const ScopeData& sd = itpair.second; std::string stmt; - stmt += protect("__Vscope_" + sd.m_symName) + ".configure(this, name(), \""; + stmt += protect("__Vscopep_" + sd.m_symName) + " = new VerilatedScope{this, name(), \""; stmt += V3OutFormatter::quoteNameControls( VIdProtect::protectWordsIf(sd.m_prettyName, true)); stmt += "\", \""; @@ -745,7 +745,7 @@ std::vector EmitCSyms::getSymCtorStmts() { stmt += V3OutFormatter::quoteNameControls(sd.m_defName); stmt += "\", "; stmt += std::to_string(sd.m_timeunit); - stmt += ", VerilatedScope::" + sd.m_type + ");"; + stmt += ", VerilatedScope::" + sd.m_type + "};"; add(stmt); } @@ -762,7 +762,7 @@ std::vector EmitCSyms::getSymCtorStmts() { if (!funcp->dpiExportImpl()) continue; std::string stmt; - stmt += protect("__Vscope_" + scopep->scopeSymName()) + ".exportInsert("; + stmt += protect("__Vscopep_" + scopep->scopeSymName()) + "->exportInsert("; stmt += vfinal + ", \""; // Not protected - user asked for import/export stmt += V3OutFormatter::quoteNameControls(funcp->cname()); @@ -815,7 +815,7 @@ std::vector EmitCSyms::getSymCtorStmts() { } std::string stmt; - stmt += protect("__Vscope_" + svd.m_scopeName) + ".varInsert(\""; + stmt += protect("__Vscopep_" + svd.m_scopeName) + "->varInsert(\""; stmt += V3OutFormatter::quoteNameControls(protect(svd.m_varBasePretty)) + '"'; const std::string varName @@ -864,6 +864,13 @@ std::vector EmitCSyms::getSymDtorStmts() { add("_vm_pgoProfiler.write(\"" + topClassName() + "\", _vm_contextp__->profVltFilename());"); } + add("// Tear down scopes"); + for (const auto& itpair : m_scopeNames) { + const ScopeData& sd = itpair.second; + const std::string id = protect("__Vscopep_" + sd.m_symName); + add("VL_DO_CLEAR(delete " + id + ", " + id + " = nullptr);"); + } + add("// Tear down sub module instances"); for (const ScopeModPair& itpair : vlstd::reverse_view(m_scopes)) { const AstScope* const scopep = itpair.first; diff --git a/test_regress/t/t_flag_csplit_groups.py b/test_regress/t/t_flag_csplit_groups.py index bf39c13bb..a4b61ab1c 100755 --- a/test_regress/t/t_flag_csplit_groups.py +++ b/test_regress/t/t_flag_csplit_groups.py @@ -125,7 +125,7 @@ test.file_grep_not(test.obj_dir + "/" + test.vm_prefix + "_classes.mk", "vm_clas test.file_grep_not(test.obj_dir + "/" + test.vm_prefix + "_classes.mk", "vm_classes_2") # Check combine count -test.file_grep(test.stats, r'Node count, CFILE + (\d+)', (264 if test.vltmt else 247)) +test.file_grep(test.stats, r'Node count, CFILE + (\d+)', (271 if test.vltmt else 254)) test.file_grep(test.stats, r'Makefile targets, VM_CLASSES_FAST + (\d+)', 2) test.file_grep(test.stats, r'Makefile targets, VM_CLASSES_SLOW + (\d+)', 2)