Replace SenTreeSet with generic collection

Introduce VNRef that can be used to wrap AstNode keys in STL
collections, resulting in equality comparisons rather than identity
comparisons. This can then replace the SenTreeSet data-structure.

src/V3Active.cpp

@@ -32,9 +32,7 @@
 #include "V3Global.h"
 #include "V3Active.h"
 #include "V3Ast.h"
-#include "V3EmitCBase.h"
 #include "V3Const.h"
-#include "V3SenTree.h"  // for SenTreeSet
 #include "V3Graph.h"
 
 #include <unordered_map>
@@ -211,9 +209,8 @@ private:
     AstActive* m_iActivep = nullptr;  // For current scope, the IActive we're building
     AstActive* m_cActivep = nullptr;  // For current scope, the SActive(combo) we're building
 
-    SenTreeSet m_activeSens;  // Sen lists for each active we've made
+    // Map from AstSenTree (equivalence) to the corresponding AstActive created.
-    using ActiveMap = std::unordered_map<AstSenTree*, AstActive*>;
+    std::unordered_map<VNRef<AstSenTree>, AstActive*> m_activeMap;
-    ActiveMap m_activeMap;  // Map sentree to active, for folding.
 
     // METHODS
     void addActive(AstActive* nodep) {
@@ -225,7 +222,6 @@ private:
         m_scopep = nodep;
         m_iActivep = nullptr;
         m_cActivep = nullptr;
-        m_activeSens.clear();
         m_activeMap.clear();
         iterateChildren(nodep);
         // Don't clear scopep, the namer persists beyond this visit
@@ -259,29 +255,20 @@ public:
         }
         return m_iActivep;
     }
+
+    // Return an AstActive that is sensitive to a SenTree equivalent to the given sentreep.
     AstActive* getActive(FileLine* fl, AstSenTree* sensesp) {
-        // Return a sentree in this scope that matches given sense list.
 
-        AstActive* activep = nullptr;
+        auto it = m_activeMap.find(*sensesp);
-        AstSenTree* const activeSenp = m_activeSens.find(sensesp);
+        // If found matching AstActive, return it
-        if (activeSenp) {
+        if (it != m_activeMap.end()) return it->second;
-            const auto it = m_activeMap.find(activeSenp);
-            UASSERT(it != m_activeMap.end(), "Corrupt active map");
-            activep = it->second;
-        }
 
-        // Not found, form a new one
+        // No such AstActive yet, creat it, and add to map.
-        if (!activep) {
         AstSenTree* const newsenp = sensesp->cloneTree(false);
-            activep = new AstActive(fl, "sequent", newsenp);
+        AstActive* const activep = new AstActive(fl, "sequent", newsenp);
         activep->sensesStorep(activep->sensesp());
-            UINFO(8, "    New ACTIVE " << activep << endl);
-            // Form the sensitivity list
         addActive(activep);
-            m_activeMap[newsenp] = activep;
+        m_activeMap.emplace(*newsenp, activep);
-            m_activeSens.add(newsenp);
-            // Note actives may have also been added above in the Active visitor
-        }
         return activep;
     }
 

src/V3Ast.h

@@ -2043,6 +2043,47 @@ inline std::ostream& operator<<(std::ostream& os, const AstNode* rhs) {
 inline void VNRelinker::relink(AstNode* newp) { newp->AstNode::relink(this); }
 
 //######################################################################
+
+// VNRef is std::reference_wrapper that can only hold AstNode subtypes
+template <typename T_Node>  //
+class VNRef final : public std::reference_wrapper<T_Node> {
+    static_assert(std::is_base_of<AstNode, T_Node>::value,
+                  "Type parameter 'T_Node' must be a subtype of AstNode");
+
+public:
+    template <typename U>
+    VNRef(U&& x)
+        : std::reference_wrapper<T_Node>{x} {}
+
+    VNRef(const VNRef& other) noexcept
+        : std::reference_wrapper<T_Node>{other} {}
+};
+
+static_assert(sizeof(VNRef<AstNode>) == sizeof(std::reference_wrapper<AstNode>),
+              "VNRef should not contain extra members");
+
+// Specializations of std::hash and std::equal_to for VNRef. This in turn
+// enables us to use for example std::unordered_set<VNRef<AstNode>> for
+// sets using equality (AstNode::sameTree) rather than identity comparisons,
+// without having to copy nodes into the collections.
+
+// Forward declaration to avoid including V3Hasher.h which needs V3Ast.h (this file).
+size_t V3HasherUncachedHash(AstNode&);
+
+// Specialization of std::hash for VNRef
+template <typename T_Node>  //
+struct std::hash<VNRef<T_Node>> final {
+    size_t operator()(VNRef<T_Node> r) const { return V3HasherUncachedHash(r); }
+};
+
+// Specialization of std::equal_to for VNRef
+template <typename T_Node>  //
+struct std::equal_to<VNRef<T_Node>> final {
+    size_t operator()(VNRef<T_Node> ra, VNRef<T_Node> rb) const {
+        return ra.get().sameTree(&(rb.get()));
+    }
+};
+
 //######################################################################
 //=== AstNode* : Derived generic node types
 

src/V3Hasher.cpp

@@ -520,3 +520,11 @@ V3Hash V3Hasher::uncachedHash(const AstNode* nodep) {
     const HasherVisitor visitor{nodep, HasherVisitor::Uncached{}};
     return visitor.finalHash();
 }
+
+//######################################################################
+// This is used by the std::hash specialization for VNRef.
+// Declared separately to avoid a circular header dependency.
+
+size_t V3HasherUncachedHash(AstNode& node) {
+    return static_cast<size_t>(V3Hasher::uncachedHash(&node).value());
+}

src/V3SenTree.h

@@ -31,51 +31,11 @@
 // Collect SenTrees under the entire scope
 // And provide functions to find/add a new one
 
-class SenTreeSet final {
-    // Hash table of sensitive blocks.
-private:
-    // TYPES
-    struct HashSenTree {
-        size_t operator()(const AstSenTree* kp) const {
-            return V3Hasher::uncachedHash(kp).value();
-        }
-    };
-
-    struct EqSenTree {
-        bool operator()(const AstSenTree* ap, const AstSenTree* bp) const {
-            return ap->sameTree(bp);
-        }
-    };
-
-    // MEMBERS
-    using Set = std::unordered_set<AstSenTree*, HashSenTree, EqSenTree>;
-    Set m_trees;  // Set of sensitive blocks, for folding.
-
-public:
-    // CONSTRUCTORS
-    SenTreeSet() = default;
-
-    // METHODS
-    void add(AstSenTree* nodep) { m_trees.insert(nodep); }
-
-    AstSenTree* find(AstSenTree* likep) {
-        AstSenTree* resultp = nullptr;
-        const auto it = m_trees.find(likep);
-        if (it != m_trees.end()) resultp = *it;
-        return resultp;
-    }
-
-    void clear() { m_trees.clear(); }
-
-private:
-    VL_UNCOPYABLE(SenTreeSet);
-};
-
 class SenTreeFinder final {
 private:
     // STATE
     AstTopScope* const m_topScopep;  // Top scope to add global SenTrees to
-    SenTreeSet m_trees;  // Set of global SenTrees
+    std::unordered_set<VNRef<AstSenTree>> m_trees;  // Set of global SenTrees
 
     VL_UNCOPYABLE(SenTreeFinder);
 
@@ -87,25 +47,26 @@ public:
     explicit SenTreeFinder(AstNetlist* netlistp)
         : m_topScopep{netlistp->topScopep()} {
         // Gather existing global SenTrees
-        for (AstNode* nodep = m_topScopep->senTreesp(); nodep; nodep = nodep->nextp()) {
+        for (AstSenTree* senTreep = m_topScopep->senTreesp(); senTreep;
-            m_trees.add(VN_AS(nodep, SenTree));
+             senTreep = VN_AS(senTreep->nextp(), SenTree)) {
+            m_trees.emplace(*senTreep);
         }
     }
 
     // METHODS
 
-    // Return a global AstSenTree that matches given SenTree.
+    // Return a global AstSenTree equivalent to the given senTreep.
     // If no such global AstSenTree exists create one and add it to the stored AstTopScope.
     AstSenTree* getSenTree(AstSenTree* senTreep) {
-        AstSenTree* treep = m_trees.find(senTreep);
+        auto it = m_trees.find(*senTreep);
-        if (!treep) {
+        // If match found, return it.
-            // Not found, form a new one
+        if (it != m_trees.end()) return &(*it).get();
-            treep = senTreep->cloneTree(false);
+
-            m_topScopep->addSenTreep(treep);
+        // Not found, create a new one
-            UINFO(8, "    New SENTREE " << treep << endl);
+        AstSenTree* const newSenTreep = senTreep->cloneTree(false);
-            m_trees.add(treep);
+        m_topScopep->addSenTreep(newSenTreep);
-        }
+        m_trees.emplace(*newSenTreep);
-        return treep;
+        return newSenTreep;
     }
 
     // Return the global combinational AstSenTree.