Speed up Dfg common sub-expression elimination

Added a DfgVertex::user() mechanism for storing data in vertices.
Similar in spirit to AstNode user data, but the generation counter is
stored in the DfgGraph the vertex is held under. Use this to cache
DfgVertex::hash results, and also speed up DfgVertex hashing in general.

Use these and additional improvements to speed up CSE.

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);
+    // Note: the recursive invocation can cause a re-hash of the cache which invalidates iterators
-    bool& result = pair.first->second;
+    uint8_t result = cache[key];
-    if (pair.second) {
+    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 {
+V3Hash DfgVertex::hash() {
-    const auto pair = cache.emplace(this, V3Hash{});
+    V3Hash& result = user<V3Hash>();
-    V3Hash& result = pair.first->second;
+    if (!result.value()) {
-    if (pair.second) {
+        V3Hash hash;
-        result += selfHash();
+        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<DfgVertexVar>()) {
-            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 {
+bool DfgVertexVar::selfEquals(const DfgVertex& that) const {
-    if (const DfgVarPacked* otherp = that.cast<DfgVarPacked>()) {
+    if (const DfgVertexVar* otherp = that.cast<DfgVertexVar>()) {
         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()); }
+V3Hash DfgVertexVar::selfHash() const {
-
+    V3Hash hash;
-// DfgVarPacked ----------
+    hash += m_varp->name();
-
+    hash += m_varp->varType();
-bool DfgVarArray::selfEquals(const DfgVertex& that) const {
+    return hash;
-    if (const DfgVarArray* otherp = that.cast<DfgVarArray>()) {
-        UASSERT_OBJ(varp() != otherp->varp(), this,
-                    "There should only be one DfgVarArray for a given AstVar");
-    }
-    return false;
 }
 
-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

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 <algorithm>
 #include <array>
 #include <functional>
+#include <new>
 #include <type_traits>
 #include <unordered_map>
 #include <vector>
@@ -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<DfgVertex*> 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<DfgVertex*> 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 <typename T>
+    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<T*>(&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<std::pair<const DfgVertex*, const DfgVertex*>, bool>;
+    using EqualsCache = std::unordered_map<std::pair<const DfgVertex*, const DfgVertex*>, 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<const DfgVertex*, V3Hash>;
-
     // 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
+    // Uses user data for caching hashes
-    // that the upstream sources of the vertex do not change between invocations.
+    V3Hash hash();
-    V3Hash hash(HashCache& cache) const;
-
-    // Uncached version of 'hash'
-    V3Hash hash() const {
-        HashCache cache;  // Still cache recursive calls within this invocation
-        return hash(cache);
-    }
 
     // Source edges of this vertex
     virtual std::pair<DfgEdge*, size_t> 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<void(const DfgVertex&)> 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<void(DfgVertex&)> f) {

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<AstVar*, AstVar*> 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<AstVar*> redundantVarps;
 

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<V3Hash, DfgVertex*> verticesWithEqualHashes;
+    std::unordered_map<V3Hash, std::vector<DfgVertex*>> 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<DfgConst>()) return;
+        if (vtxp->is<DfgConst>()) continue;
+
         // For everything else...
-        const V3Hash hash = vtx.hash(hashCache);
+        std::vector<DfgVertex*>& vec = verticesWithEqualHashes[vtxp->hash()];
-        auto pair = verticesWithEqualHashes.equal_range(hash);
+        bool replaced = false;
-        for (auto it = pair.first, end = pair.second; it != end; ++it) {
+        for (DfgVertex* const candidatep : vec) {
-            DfgVertex* const candidatep = it->second;
+            if (candidatep->equals(*vtxp, equalsCache)) {
-            if (candidatep->equals(vtx, equalsCache)) {
                 ++ctx.m_eliminated;
-                vtx.replaceWith(candidatep);
+                vtxp->replaceWith(candidatep);
-                vtx.unlinkDelete(dfg);
+                vtxp->unlinkDelete(dfg);
-                return;
+                replaced = true;
+                break;
             }
         }
-        verticesWithEqualHashes.emplace(hash, &vtx);
+        if (replaced) continue;
-    });
+        vec.push_back(vtxp);
+    }
 }
 
 void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) {

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<DriverData> 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<DriverData> 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} {}