diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index e353c5b5c..c5be6ba5f 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -864,9 +864,10 @@ bool DfgVertex::equals(const DfgVertex& that, EqualsCache& cache) const { const auto key = (this < &that) ? EqualsCache::key_type{this, &that} // : EqualsCache::key_type{&that, this}; - const auto pair = cache.emplace(key, true); - bool& result = pair.first->second; - if (pair.second) { + // Note: the recursive invocation can cause a re-hash of the cache which invalidates iterators + uint8_t result = cache[key]; + if (!result) { + result = 2; // Assume equals auto thisPair = this->sourceEdges(); const DfgEdge* const thisSrcEdgesp = thisPair.first; const size_t thisArity = thisPair.second; @@ -879,24 +880,30 @@ bool DfgVertex::equals(const DfgVertex& that, EqualsCache& cache) const { const DfgVertex* const thatSrcVtxp = thatSrcEdgesp[i].m_sourcep; if (thisSrcVtxp == thatSrcVtxp) continue; if (!thisSrcVtxp || !thatSrcVtxp || !thisSrcVtxp->equals(*thatSrcVtxp, cache)) { - result = false; + result = 1; // Mark not equal break; } } + cache[key] = result; } - return result; + return result >> 1; } -V3Hash DfgVertex::hash(HashCache& cache) const { - const auto pair = cache.emplace(this, V3Hash{}); - V3Hash& result = pair.first->second; - if (pair.second) { - result += selfHash(); +V3Hash DfgVertex::hash() { + V3Hash& result = user(); + if (!result.value()) { + V3Hash hash; + hash += selfHash(); // Variables are defined by themselves, so there is no need to hash the sources. This // enables sound hashing of graphs circular only through variables, which we rely on. if (!is()) { - forEachSource([&result, &cache](const DfgVertex& src) { result += src.hash(cache); }); + const auto pair = sourceEdges(); + const DfgEdge* const edgesp = pair.first; + const size_t arity = pair.second; + // Sources must always be connected in well-formed graphs + for (size_t i = 0; i < arity; ++i) hash += edgesp[i].m_sourcep->hash(); } + result = hash; } return result; } @@ -926,30 +933,23 @@ void DfgVertex::replaceWith(DfgVertex* newSorucep) { // Vertex classes //------------------------------------------------------------------------------ -// DfgVarPacked ---------- +// DfgVertexVar ---------- -bool DfgVarPacked::selfEquals(const DfgVertex& that) const { - if (const DfgVarPacked* otherp = that.cast()) { +bool DfgVertexVar::selfEquals(const DfgVertex& that) const { + if (const DfgVertexVar* otherp = that.cast()) { UASSERT_OBJ(varp() != otherp->varp(), this, "There should only be one DfgVarPacked for a given AstVar"); } return false; } -V3Hash DfgVarPacked::selfHash() const { return V3Hasher::uncachedHash(varp()); } - -// DfgVarPacked ---------- - -bool DfgVarArray::selfEquals(const DfgVertex& that) const { - if (const DfgVarArray* otherp = that.cast()) { - UASSERT_OBJ(varp() != otherp->varp(), this, - "There should only be one DfgVarArray for a given AstVar"); - } - return false; +V3Hash DfgVertexVar::selfHash() const { + V3Hash hash; + hash += m_varp->name(); + hash += m_varp->varType(); + return hash; } -V3Hash DfgVarArray::selfHash() const { return V3Hasher::uncachedHash(varp()); } - // DfgConst ---------- bool DfgConst::selfEquals(const DfgVertex& that) const { @@ -959,7 +959,7 @@ bool DfgConst::selfEquals(const DfgVertex& that) const { return false; } -V3Hash DfgConst::selfHash() const { return V3Hasher::uncachedHash(m_constp); } +V3Hash DfgConst::selfHash() const { return m_constp->num().toHash(); } //------------------------------------------------------------------------------ // DfgVisitor diff --git a/src/V3Dfg.h b/src/V3Dfg.h index e4a6d308f..e434c5b12 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -36,7 +36,6 @@ #include "V3Ast.h" #include "V3Error.h" #include "V3Hash.h" -#include "V3Hasher.h" #include "V3List.h" #include "V3Dfg__gen_forward_class_decls.h" // From ./astgen @@ -44,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -94,9 +94,36 @@ inline std::ostream& operator<<(std::ostream& os, const VDfgType& t) { return os class DfgGraph final { friend class DfgVertex; + // TYPES + + // RAII handle for DfgVertex user data + class UserDataInUse final { + DfgGraph* m_graphp; // The referenced graph + + public: + UserDataInUse(DfgGraph* graphp) + : m_graphp{graphp} {} + VL_UNCOPYABLE(UserDataInUse); + UserDataInUse(UserDataInUse&& that) { + UASSERT(that.m_graphp, "Moving from empty"); + m_graphp = vlstd::exchange(that.m_graphp, nullptr); + } + UserDataInUse& operator=(UserDataInUse&& that) { + UASSERT(that.m_graphp, "Moving from empty"); + m_graphp = vlstd::exchange(that.m_graphp, nullptr); + return *this; + } + + ~UserDataInUse() { + if (m_graphp) m_graphp->m_userCurrent = 0; + } + }; + // MEMBERS - size_t m_size = 0; // Number of vertices in the graph V3List m_vertices; // The vertices in the graph + size_t m_size = 0; // Number of vertices in the graph + uint32_t m_userCurrent = 0; // Vertex user data generation number currently in use + uint32_t m_userCnt = 0; // Vertex user data generation counter // Parent of the graph (i.e.: the module containing the logic represented by this graph). AstModule* const m_modulep; const string m_name; // Name of graph (for debugging) @@ -120,6 +147,19 @@ public: // Name of this graph const string& name() const { return m_name; } + // Reset Vertex user data + UserDataInUse userDataInUse() { + UASSERT(!m_userCurrent, "Conflicting use of DfgVertex user data"); + ++m_userCnt; + UASSERT(m_userCnt, "'m_userCnt' overflow"); + m_userCurrent = m_userCnt; + return UserDataInUse{this}; + } + + // Access to vertex list for faster iteration in important contexts + DfgVertex* verticesBegin() const { return m_vertices.begin(); } + DfgVertex* verticesRbegin() const { return m_vertices.rbegin(); } + // Calls given function 'f' for each vertex in the graph. It is safe to manipulate any vertices // in the graph, or to delete/unlink the vertex passed to 'f' during iteration. It is however // not safe to delete/unlink any vertex in the same graph other than the one passed to 'f'. @@ -221,13 +261,18 @@ class DfgVertex VL_NOT_FINAL { friend class DfgEdge; friend class DfgVisitor; + using UserDataStorage = void*; // Storage allocated for user data + // STATE V3ListEnt m_verticesEnt; // V3List handle of this vertex, kept under the DfgGraph protected: DfgEdge* m_sinksp = nullptr; // List of sinks of this vertex FileLine* const m_filelinep; // Source location AstNodeDType* m_dtypep; // Data type of the result of this vertex - mutable for efficiency - const VDfgType m_type; + DfgGraph* m_graphp; // The containing DfgGraph + const VDfgType m_type; // Vertex type tag + uint32_t m_userCnt = 0; // User data generation number + UserDataStorage m_userDataStorage; // User data storage // CONSTRUCTOR DfgVertex(DfgGraph& dfg, VDfgType type, FileLine* flp, AstNodeDType* dtypep); @@ -301,6 +346,23 @@ public: // The type of this vertex VDfgType type() const { return m_type; } + template + T& user() { + static_assert(sizeof(T) <= sizeof(UserDataStorage), + "Size of user data type 'T' is too large for allocated storage"); + static_assert(alignof(T) <= alignof(UserDataStorage), + "Alignment of user data type 'T' is larger than allocated storage"); + T* const storagep = reinterpret_cast(&m_userDataStorage); + const uint32_t userCurrent = m_graphp->m_userCurrent; + UDEBUGONLY(UASSERT_OBJ(userCurrent, this, "DfgVertex user data used without reserving");); + if (m_userCnt != userCurrent) { + m_userCnt = userCurrent; + VL_ATTR_UNUSED T* const resultp = new (storagep) T{}; + UDEBUGONLY(UASSERT_OBJ(resultp == storagep, this, "Something is odd");); + } + return *storagep; + } + // Width of result uint32_t width() const { // Everything supported is packed now, so we can just do this: @@ -309,7 +371,7 @@ public: } // Cache type for 'equals' below - using EqualsCache = std::unordered_map, bool>; + using EqualsCache = std::unordered_map, uint8_t>; // Vertex equality (based on this vertex and all upstream vertices feeding into this vertex). // Returns true, if the vertices can be substituted for each other without changing the @@ -324,19 +386,9 @@ public: return equals(that, cache); } - // Cache type for 'hash' below - using HashCache = std::unordered_map; - // Hash of vertex (depends on this vertex and all upstream vertices feeding into this vertex). - // The 'cache' argument is used to store results to avoid repeat evaluations, but it requires - // that the upstream sources of the vertex do not change between invocations. - V3Hash hash(HashCache& cache) const; - - // Uncached version of 'hash' - V3Hash hash() const { - HashCache cache; // Still cache recursive calls within this invocation - return hash(cache); - } + // Uses user data for caching hashes + V3Hash hash(); // Source edges of this vertex virtual std::pair sourceEdges() = 0; @@ -362,6 +414,10 @@ public: // Relink all sinks to be driven from the given new source void replaceWith(DfgVertex* newSourcep); + // Access to vertex list for faster iteration in important contexts + DfgVertex* verticesNext() const { return m_verticesEnt.nextp(); } + DfgVertex* verticesPrev() const { return m_verticesEnt.prevp(); } + // Calls given function 'f' for each source vertex of this vertex // Unconnected source edges are not iterated. inline void forEachSource(std::function f) const; @@ -488,11 +544,13 @@ public: void DfgGraph::addVertex(DfgVertex& vtx) { ++m_size; vtx.m_verticesEnt.pushBack(m_vertices, &vtx); + vtx.m_graphp = this; } void DfgGraph::removeVertex(DfgVertex& vtx) { --m_size; vtx.m_verticesEnt.unlink(m_vertices, &vtx); + vtx.m_graphp = nullptr; } void DfgGraph::forEachVertex(std::function f) { diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index de08f1060..2cab9eaec 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -126,7 +126,6 @@ class DfgToAstVisitor final : DfgVisitor { // Map from an AstVar, to the canonical AstVar that can be substituted for that AstVar std::unordered_map m_canonVars; V3UniqueNames m_tmpNames{"_VdfgTmp"}; // For generating temporary names - DfgVertex::HashCache m_hashCache; // For caching hashes // METHODS @@ -167,7 +166,7 @@ class DfgToAstVisitor final : DfgVisitor { // Given a DfgVertex, return an AstVar that will hold the value of the given DfgVertex once we // are done with converting this Dfg into Ast form. - AstVar* getResultVar(const DfgVertex* vtxp) { + AstVar* getResultVar(DfgVertex* vtxp) { const auto pair = m_resultVars.emplace(vtxp, nullptr); AstVar*& varp = pair.first->second; if (pair.second) { @@ -187,7 +186,7 @@ class DfgToAstVisitor final : DfgVisitor { } else { // No DfgVarPacked driven fully by this node. Create a temporary. // TODO: should we reuse parts when the AstVar is used as an rvalue? - const string name = m_tmpNames.get(vtxp->hash(m_hashCache).toString()); + const string name = m_tmpNames.get(vtxp->hash().toString()); // Note: It is ok for these temporary variables to be always unsigned. They are // read only by other expressions within the graph and all expressions interpret // their operands based on the expression type, not the operand type. @@ -330,6 +329,10 @@ class DfgToAstVisitor final : DfgVisitor { explicit DfgToAstVisitor(DfgGraph& dfg, V3DfgOptimizationContext& ctx) : m_modp{dfg.modulep()} , m_ctx{ctx} { + + // Used by DfgVertex::hash + const auto userDataInUse = dfg.userDataInUse(); + // We can eliminate some variables completely std::vector redundantVarps; diff --git a/src/V3DfgPasses.cpp b/src/V3DfgPasses.cpp index 3b08c3121..571bdb489 100644 --- a/src/V3DfgPasses.cpp +++ b/src/V3DfgPasses.cpp @@ -77,28 +77,36 @@ V3DfgOptimizationContext::~V3DfgOptimizationContext() { // Common subexpression elimination void V3DfgPasses::cse(DfgGraph& dfg, V3DfgCseContext& ctx) { - DfgVertex::HashCache hashCache; DfgVertex::EqualsCache equalsCache; - std::unordered_multimap verticesWithEqualHashes; + std::unordered_map> verticesWithEqualHashes; + verticesWithEqualHashes.reserve(dfg.size()); + + // Used by DfgVertex::hash + const auto userDataInUse = dfg.userDataInUse(); // In reverse, as the graph is sometimes in reverse topological order already - dfg.forEachVertexInReverse([&](DfgVertex& vtx) { + for (DfgVertex *vtxp = dfg.verticesRbegin(), *nextp; vtxp; vtxp = nextp) { + nextp = vtxp->verticesPrev(); + if (VL_LIKELY(nextp)) VL_PREFETCH_RW(nextp); + // Don't merge constants - if (vtx.is()) return; + if (vtxp->is()) continue; + // For everything else... - const V3Hash hash = vtx.hash(hashCache); - auto pair = verticesWithEqualHashes.equal_range(hash); - for (auto it = pair.first, end = pair.second; it != end; ++it) { - DfgVertex* const candidatep = it->second; - if (candidatep->equals(vtx, equalsCache)) { + std::vector& vec = verticesWithEqualHashes[vtxp->hash()]; + bool replaced = false; + for (DfgVertex* const candidatep : vec) { + if (candidatep->equals(*vtxp, equalsCache)) { ++ctx.m_eliminated; - vtx.replaceWith(candidatep); - vtx.unlinkDelete(dfg); - return; + vtxp->replaceWith(candidatep); + vtxp->unlinkDelete(dfg); + replaced = true; + break; } } - verticesWithEqualHashes.emplace(hash, &vtx); - }); + if (replaced) continue; + vec.push_back(vtxp); + } } void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index f504bcbd3..b9cd763fe 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -43,6 +43,9 @@ class DfgVertexVar VL_NOT_FINAL : public DfgVertexVariadic { bool m_hasModRefs = false; // This AstVar is referenced outside the DFG, but in the module bool m_hasExtRefs = false; // This AstVar is referenced from outside the module + bool selfEquals(const DfgVertex& that) const final; + V3Hash selfHash() const final; + public: DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVar* varp, uint32_t initialCapacity) : DfgVertexVariadic{dfg, type, varp->fileline(), dtypeFor(varp), initialCapacity} @@ -115,9 +118,6 @@ class DfgVarArray final : public DfgVertexVar { std::vector m_driverData; // Additional data associate with each driver - bool selfEquals(const DfgVertex& that) const override; - V3Hash selfHash() const override; - public: DfgVarArray(DfgGraph& dfg, AstVar* varp) : DfgVertexVar{dfg, dfgType(), varp, 4u} { @@ -177,9 +177,6 @@ class DfgVarPacked final : public DfgVertexVar { std::vector m_driverData; // Additional data associate with each driver - bool selfEquals(const DfgVertex& that) const override; - V3Hash selfHash() const override; - public: DfgVarPacked(DfgGraph& dfg, AstVar* varp) : DfgVertexVar{dfg, dfgType(), varp, 1u} {}