From ff4923cf677db4e0b0e2a048f7ff1af4fe102189 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 8 Jul 2023 12:27:50 -0400 Subject: [PATCH] Internals: Make V3MemberMap towards removing member cache (#4350) --- src/V3AstNodeDType.h | 9 ----- src/V3AstNodeOther.h | 1 + src/V3AstNodes.cpp | 26 +++--------- src/V3LinkDot.cpp | 5 ++- src/V3MemberMap.h | 94 ++++++++++++++++++++++++++++++++++++++++++++ src/V3Randomize.cpp | 10 +++-- src/V3Timing.cpp | 7 ++-- src/V3Width.cpp | 18 ++++++--- src/V3WidthCommit.h | 1 - 9 files changed, 126 insertions(+), 45 deletions(-) create mode 100644 src/V3MemberMap.h diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index bb6599ca9..80e597e87 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -199,12 +199,9 @@ class AstNodeUOrStructDType VL_NOT_FINAL : public AstNodeDType { // A struct or union; common handling // @astgen op1 := membersp : List[AstMemberDType] private: - // TYPES - using MemberNameMap = std::map; // MEMBERS string m_name; // Name from upper typedef, if any AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy - MemberNameMap m_members; const int m_uniqueNum; bool m_packed; bool m_isFourstate = false; // V3Width computes @@ -250,12 +247,6 @@ public: static bool packedUnsup() { return true; } void isFourstate(bool flag) { m_isFourstate = flag; } bool isFourstate() const override VL_MT_SAFE { return m_isFourstate; } - void clearCache() { m_members.clear(); } - void repairMemberCache(); - AstMemberDType* findMember(const string& name) const { - const auto it = m_members.find(name); - return (it == m_members.end()) ? nullptr : it->second; - } static int lo() { return 0; } int hi() const { return dtypep()->width() - 1; } // Packed classes look like arrays VNumRange declRange() const { return VNumRange{hi(), lo()}; } diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index e139a5905..7889fe238 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2187,6 +2187,7 @@ public: static bool isClassExtendedFrom(const AstClass* refClassp, const AstClass* baseClassp); // Return the lowest class extended from, or this class AstClass* baseMostClassp(); + static bool isCacheableChild(const AstNode* nodep); }; class AstClassPackage final : public AstNodeModule { // The static information portion of a class (treated similarly to a package) diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 10297491b..28fc6c53f 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -103,29 +103,8 @@ int AstNodeSel::bitConst() const { return (constp ? constp->toSInt() : 0); } -void AstNodeUOrStructDType::repairMemberCache() { - clearCache(); - for (AstMemberDType* itemp = membersp(); itemp; itemp = VN_AS(itemp->nextp(), MemberDType)) { - if (m_members.find(itemp->name()) != m_members.end()) { - itemp->v3error("Duplicate declaration of member name: " << itemp->prettyNameQ()); - } else { - m_members.emplace(itemp->name(), itemp); - } - } -} - const char* AstNodeUOrStructDType::broken() const { BROKEN_RTN(m_classOrPackagep && !m_classOrPackagep->brokeExists()); - std::unordered_set exists; - for (AstMemberDType* itemp = membersp(); itemp; itemp = VN_AS(itemp->nextp(), MemberDType)) { - exists.insert(itemp); - } - for (MemberNameMap::const_iterator it = m_members.begin(); it != m_members.end(); ++it) { - if (VL_UNCOVERABLE(exists.find(it->second) == exists.end())) { - this->v3error("Internal: Structure member broken: " << it->first); - return "member broken"; - } - } return nullptr; } @@ -1427,6 +1406,11 @@ const char* AstClassPackage::broken() const { void AstClassPackage::cloneRelink() { if (m_classp && m_classp->clonep()) m_classp = m_classp->clonep(); } +bool AstClass::isCacheableChild(const AstNode* nodep) { + return (VN_IS(nodep, Var) || VN_IS(nodep, EnumItemRef) + || (VN_IS(nodep, NodeFTask) && !VN_AS(nodep, NodeFTask)->isExternProto()) + || VN_IS(nodep, CFunc)); +} void AstClass::insertCache(AstNode* nodep) { const bool doit = (VN_IS(nodep, Var) || VN_IS(nodep, EnumItemRef) || (VN_IS(nodep, NodeFTask) && !VN_AS(nodep, NodeFTask)->isExternProto()) diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index fac102020..78b22ca7a 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -69,6 +69,7 @@ #include "V3Ast.h" #include "V3Global.h" #include "V3Graph.h" +#include "V3MemberMap.h" #include "V3String.h" #include "V3SymTable.h" @@ -3492,14 +3493,16 @@ private: // so add members pointing to appropriate enum values { nodep->repairCache(); + VMemberMap memberMap; for (VSymEnt::const_iterator it = m_curSymp->begin(); it != m_curSymp->end(); ++it) { AstNode* const itemp = it->second->nodep(); - if (!nodep->findMember(it->first)) { + if (!memberMap.findMember(nodep, it->first)) { if (AstEnumItem* const aitemp = VN_CAST(itemp, EnumItem)) { AstEnumItemRef* const newp = new AstEnumItemRef{ aitemp->fileline(), aitemp, it->second->classOrPackagep()}; UINFO(8, "Class import noderef '" << it->first << "' " << newp << endl); nodep->addMembersp(newp); + memberMap.insert(nodep, newp); } } } diff --git a/src/V3MemberMap.h b/src/V3MemberMap.h new file mode 100644 index 000000000..6f6c8f008 --- /dev/null +++ b/src/V3MemberMap.h @@ -0,0 +1,94 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Member names for classes/structs +// +// Code available from: https://verilator.org +// +//************************************************************************* +// +// Copyright 2003-2023 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 +// +//************************************************************************* +// +// For a class or struct, create a map of names of each member. +// Used for faster lookup to avoid O(n^2) processing times. +// +//************************************************************************* + +#ifndef VERILATOR_V3MEMBERMAP_H_ +#define VERILATOR_V3MEMBERMAP_H_ + +#include "config_build.h" +#include "verilatedos.h" + +#include "V3Ast.h" +#include "V3Global.h" + +#include + +//###################################################################### + +class VMemberMap final { + // MEMBERS + using MemberMap = std::map; + using NodeMap = std::map; + NodeMap m_map; // Map of nodes being tracked +public: + void clear() { m_map.clear(); } + // Find 'name' under 'nodep', caching nodep's children if needed + AstNode* findMember(AstNode* nodep, const std::string& name) { + auto nit = m_map.find(nodep); + if (VL_UNLIKELY(nit == m_map.end())) { + scan(nodep); + nit = m_map.find(nodep); + } + const auto mit = nit->second.find(name); + if (mit == nit->second.end()) return nullptr; + return mit->second; + } + // Insert under the given parent node the child's name + void insert(AstNode* nodep, AstNode* childp) { + auto nit = m_map.find(nodep); + if (nit == m_map.end()) { + scan(nodep); + nit = m_map.find(nodep); + } + memberInsert(nit->second, childp, false); + } + +private: + void scan(AstNode* nodep) { + const auto nitPair = m_map.emplace(nodep, MemberMap{}); + MemberMap& mmapr = nitPair.first->second; + if (AstClass* const anodep = VN_CAST(nodep, Class)) { + for (AstNode* itemp = anodep->membersp(); itemp; itemp = itemp->nextp()) { + if (const AstScope* const scopep = VN_CAST(itemp, Scope)) { + for (AstNode* blockp = scopep->blocksp(); blockp; blockp = blockp->nextp()) { + if (AstClass::isCacheableChild(blockp)) memberInsert(mmapr, blockp); + } + } else { + if (AstClass::isCacheableChild(itemp)) memberInsert(mmapr, itemp); + } + } + } else if (AstNodeUOrStructDType* const anodep = VN_CAST(nodep, NodeUOrStructDType)) { + for (AstNode* itemp = anodep->membersp(); itemp; itemp = itemp->nextp()) { + memberInsert(mmapr, itemp); + } + } else { + nodep->v3fatalSrc("Unsupported node type"); + } + } + void memberInsert(MemberMap& mmapr, AstNode* childp, bool warn = true) { + const auto mitPair = mmapr.emplace(childp->name(), childp); + if (VL_UNCOVERABLE(!mitPair.second && warn)) { + // Probably an internal error, but we'll make it user friendly if happens + childp->v3error("Duplicate declaration of member name: " << childp->prettyNameQ()); + } + } +}; + +#endif // guard diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp index 6f88a2372..3bcf0278b 100644 --- a/src/V3Randomize.cpp +++ b/src/V3Randomize.cpp @@ -30,6 +30,7 @@ #include "V3Randomize.h" #include "V3Ast.h" +#include "V3MemberMap.h" VL_DEFINE_DEBUG_FUNCTIONS; @@ -138,6 +139,7 @@ private: const VNUser2InUse m_inuser2; // STATE + VMemberMap memberMap; // Member names cached for fast lookup AstNodeModule* m_modp = nullptr; // Current module const AstNodeFTask* m_ftaskp = nullptr; // Current function/task size_t m_enumValueTabCount = 0; // Number of tables with enum values created @@ -213,7 +215,7 @@ private: } } void addPrePostCall(AstClass* classp, AstFunc* funcp, const string& name) { - if (AstTask* userFuncp = VN_CAST(classp->findMember(name), Task)) { + if (AstTask* userFuncp = VN_CAST(memberMap.findMember(classp, name), Task)) { AstTaskRef* const callp = new AstTaskRef{userFuncp->fileline(), userFuncp->name(), nullptr}; callp->taskp(userFuncp); @@ -380,7 +382,8 @@ void V3Randomize::randomizeNetlist(AstNetlist* nodep) { } AstFunc* V3Randomize::newRandomizeFunc(AstClass* nodep) { - AstFunc* funcp = VN_AS(nodep->findMember("randomize"), Func); + VMemberMap memberMap; + AstFunc* funcp = VN_AS(memberMap.findMember(nodep, "randomize"), Func); if (!funcp) { v3Global.useRandomizeMethods(true); AstNodeDType* const dtypep @@ -403,8 +406,9 @@ AstFunc* V3Randomize::newRandomizeFunc(AstClass* nodep) { } AstFunc* V3Randomize::newSRandomFunc(AstClass* nodep) { + VMemberMap memberMap; AstClass* const basep = nodep->baseMostClassp(); - AstFunc* funcp = VN_AS(basep->findMember("srandom"), Func); + AstFunc* funcp = VN_AS(memberMap.findMember(basep, "srandom"), Func); if (!funcp) { v3Global.useRandomizeMethods(true); AstNodeDType* const dtypep diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 41846b721..e3b6ffaca 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -53,6 +53,7 @@ #include "V3Const.h" #include "V3EmitV.h" #include "V3Graph.h" +#include "V3MemberMap.h" #include "V3SenExprBuilder.h" #include "V3SenTree.h" #include "V3UniqueNames.h" @@ -114,6 +115,7 @@ private: const VNUser3InUse m_user3InUse; // STATE + VMemberMap memberMap; // Member names cached for fast lookup AstClass* m_classp = nullptr; // Current class AstNode* m_procp = nullptr; // NodeProcedure/CFunc/Begin we're under V3Graph m_depGraph; // Dependency graph where a node is a dependency of another if it being @@ -164,8 +166,6 @@ private: TimingDependencyVertex* const vxp = getDependencyVertex(nodep); if (nodep->needProcess()) nodep->user2(T_PROC); if (!m_classp) return; - // If class method (possibly overrides another method) - if (!m_classp->user1SetOnce()) m_classp->repairCache(); // Go over overridden functions @@ -182,9 +182,8 @@ private: // actually overridden by our method. If this causes a problem, traverse to // the root of the inheritance hierarchy and check if the original method is // virtual or not. - if (!cextp->classp()->user1SetOnce()) cextp->classp()->repairCache(); if (auto* const overriddenp - = VN_CAST(cextp->classp()->findMember(nodep->name()), CFunc)) { + = VN_CAST(memberMap.findMember(cextp->classp(), nodep->name()), CFunc)) { setTimingFlag(nodep, overriddenp->user2()); if (nodep->user2() < T_PROC) { // Add a vertex only if the flag can still change diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 578ba6570..0dcf90efc 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -70,6 +70,7 @@ #include "V3Const.h" #include "V3Global.h" +#include "V3MemberMap.h" #include "V3Number.h" #include "V3Randomize.h" #include "V3String.h" @@ -223,6 +224,7 @@ private: using DTypeMap = std::map; // STATE + VMemberMap memberMap; // Member names cached for fast lookup WidthVP* m_vup = nullptr; // Current node state AstClass* m_classp = nullptr; // Current class const AstCell* m_cellp = nullptr; // Current cell for arrayed instantiations @@ -2576,7 +2578,6 @@ private: // if (debug() >= 9) nodep->dumpTree("- class-in: "); if (!nodep->packed() && v3Global.opt.structsPacked()) nodep->packed(true); userIterateChildren(nodep, nullptr); // First size all members - nodep->repairMemberCache(); nodep->dtypep(nodep); nodep->isFourstate(false); // Error checks @@ -2629,7 +2630,7 @@ private: if (nodep->didWidthAndSet()) return; // If the package is std::process, set m_process type to VlProcessRef if (m_pkgp && m_pkgp->name() == "std" && nodep->name() == "process") { - if (AstVar* const varp = VN_CAST(nodep->findMember("m_process"), Var)) { + if (AstVar* const varp = VN_CAST(memberMap.findMember(nodep, "m_process"), Var)) { AstBasicDType* const dtypep = new AstBasicDType{ nodep->fileline(), VBasicDTypeKwd::PROCESS_REFERENCE, VSigning::UNSIGNED}; v3Global.rootp()->typeTablep()->addTypesp(dtypep); @@ -2807,7 +2808,6 @@ private: // No need to width-resolve the interface, as it was done when we did the child AstNodeModule* const ifacep = adtypep->ifacep(); UASSERT_OBJ(ifacep, nodep, "Unlinked"); - // if (AstNode* const foundp = ifacep->findMember(nodep->name())) return foundp; VSpellCheck speller; for (AstNode* itemp = ifacep->stmtsp(); itemp; itemp = itemp->nextp()) { if (itemp->name() == nodep->name()) return itemp; @@ -2824,7 +2824,8 @@ private: } bool memberSelStruct(AstMemberSel* nodep, AstNodeUOrStructDType* adtypep) { // Returns true if ok - if (AstMemberDType* const memberp = adtypep->findMember(nodep->name())) { + if (AstMemberDType* const memberp + = VN_CAST(memberMap.findMember(adtypep, nodep->name()), MemberDType)) { if (m_attrp) { // Looking for the base of the attribute nodep->dtypep(memberp); UINFO(9, " MEMBERSEL(attr) -> " << nodep << endl); @@ -3489,8 +3490,10 @@ private: AstClass* const first_classp = adtypep->classp(); if (nodep->name() == "randomize") { V3Randomize::newRandomizeFunc(first_classp); + memberMap.clear(); } else if (nodep->name() == "srandom") { V3Randomize::newSRandomFunc(first_classp); + memberMap.clear(); } UASSERT_OBJ(first_classp, nodep, "Unlinked"); for (AstClass* classp = first_classp; classp;) { @@ -3783,7 +3786,7 @@ private: classp = refp->classp(); UASSERT_OBJ(classp, nodep, "Unlinked"); - if (AstNodeFTask* const ftaskp = VN_CAST(classp->findMember("new"), Func)) { + if (AstNodeFTask* const ftaskp = VN_CAST(memberMap.findMember(classp, "new"), Func)) { nodep->taskp(ftaskp); nodep->classOrPackagep(classp); } else { @@ -3951,7 +3954,8 @@ private: // '{member:value} or '{data_type: default_value} if (const AstText* textp = VN_CAST(patp->keyp(), Text)) { // member: value - memp = vdtypep->findMember(textp->text()); + memp = VN_CAST(memberMap.findMember(vdtypep, textp->text()), + MemberDType); if (!memp) { patp->keyp()->v3error("Assignment pattern key '" << textp->text() << "' not found as member"); @@ -5596,8 +5600,10 @@ private: } if (nodep->name() == "randomize") { nodep->taskp(V3Randomize::newRandomizeFunc(m_classp)); + memberMap.clear(); } else if (nodep->name() == "srandom") { nodep->taskp(V3Randomize::newSRandomFunc(m_classp)); + memberMap.clear(); } else if (nodep->name() == "get_randstate") { methodOkArguments(nodep, 0, 0); m_classp->baseMostClassp()->needRNG(true); diff --git a/src/V3WidthCommit.h b/src/V3WidthCommit.h index 34cc4c689..3513f0cc7 100644 --- a/src/V3WidthCommit.h +++ b/src/V3WidthCommit.h @@ -190,7 +190,6 @@ private: void visit(AstNodeUOrStructDType* nodep) override { if (nodep->user1SetOnce()) return; // Process once visitIterateNodeDType(nodep); - nodep->clearCache(); } void visit(AstParamTypeDType* nodep) override { if (nodep->user1SetOnce()) return; // Process once