Merge DFG components made acyclic into the original acyclic sub-graph. (#6261)

After making a cyclic DFG component acyclic, merge that component back
into the acyclic sub-graph of the input graph. This enables optimizing
through the components that were made acyclic in their larger context.

src/V3Dfg.cpp

@@ -171,25 +171,51 @@ std::unique_ptr<DfgGraph> DfgGraph::clone() const {
     return std::unique_ptr<DfgGraph>{clonep};
 }
 
-void DfgGraph::addGraph(DfgGraph& other) {
-    m_size += other.m_size;
-    other.m_size = 0;
+void DfgGraph::mergeGraphs(std::vector<std::unique_ptr<DfgGraph>>&& otherps) {
+    if (otherps.empty()) return;
 
-    for (DfgVertexVar& vtx : other.m_varVertices) {
-        vtx.m_userCnt = 0;
-        vtx.m_graphp = this;
+    // NODE STATE
+    // AstVar/AstVarScope::user2p() -> corresponding DfgVertexVar* in 'this' graph
+    const VNUser2InUse user2InUse;
+
+    // Set up Ast Variable -> DfgVertexVar map for 'this' graph
+    for (DfgVertexVar& vtx : m_varVertices) vtx.nodep()->user2p(&vtx);
+
+    // Merge in each of the other graphs
+    for (const std::unique_ptr<DfgGraph>& otherp : otherps) {
+        // Process variables
+        for (DfgVertexVar* const vtxp : otherp->m_varVertices.unlinkable()) {
+            // Variabels that are present in 'this', make them use the DfgVertexVar in 'this'.
+            if (DfgVertexVar* const altp = vtxp->nodep()->user2u().to<DfgVertexVar*>()) {
+                DfgVertex* const srcp = vtxp->srcp();
+                UASSERT_OBJ(!srcp || !altp->srcp(), vtxp, "At most one alias should be driven");
+                vtxp->replaceWith(altp);
+                if (srcp) altp->srcp(srcp);
+                VL_DO_DANGLING(vtxp->unlinkDelete(*otherp), vtxp);
+                continue;
+            }
+            // Otherwise they will be moved
+            vtxp->nodep()->user2p(vtxp);
+            vtxp->m_userCnt = 0;
+            vtxp->m_graphp = this;
+        }
+        m_varVertices.splice(m_varVertices.end(), otherp->m_varVertices);
+        // Process constants
+        for (DfgConst& vtx : otherp->m_constVertices) {
+            vtx.m_userCnt = 0;
+            vtx.m_graphp = this;
+        }
+        m_constVertices.splice(m_constVertices.end(), otherp->m_constVertices);
+        // Process operations
+        for (DfgVertex& vtx : otherp->m_opVertices) {
+            vtx.m_userCnt = 0;
+            vtx.m_graphp = this;
+        }
+        m_opVertices.splice(m_opVertices.end(), otherp->m_opVertices);
+        // Update graph sizes
+        m_size += otherp->m_size;
+        otherp->m_size = 0;
     }
-    m_varVertices.splice(m_varVertices.end(), other.m_varVertices);
-    for (DfgConst& vtx : other.m_constVertices) {
-        vtx.m_userCnt = 0;
-        vtx.m_graphp = this;
-    }
-    m_constVertices.splice(m_constVertices.end(), other.m_constVertices);
-    for (DfgVertex& vtx : other.m_opVertices) {
-        vtx.m_userCnt = 0;
-        vtx.m_graphp = this;
-    }
-    m_opVertices.splice(m_opVertices.end(), other.m_opVertices);
 }
 
 std::string DfgGraph::makeUniqueName(const std::string& prefix, size_t n) {

src/V3Dfg.h

@@ -714,8 +714,9 @@ public:
     // Return an identical, independent copy of this graph. Vertex and edge order might differ.
     std::unique_ptr<DfgGraph> clone() const VL_MT_DISABLED;
 
-    // Add contents of other graph to this graph. Leaves other graph empty.
-    void addGraph(DfgGraph& other) VL_MT_DISABLED;
+    // Merge contents of other graphs into this graph. Deletes the other graphs.
+    // DfgVertexVar instances representing the same Ast variable are unified.
+    void mergeGraphs(std::vector<std::unique_ptr<DfgGraph>>&& otherps) VL_MT_DISABLED;
 
     // Genarete a unique name. The provided 'prefix' and 'n' values will be part of the name, and
     // must be unique (as a pair) in each invocation for this graph.
@@ -920,6 +921,9 @@ DfgVertexVar::DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVar* varp)
     , m_varScopep{nullptr} {
     UASSERT_OBJ(dfg.modulep(), varp, "Un-scoped DfgVertexVar created in scoped DfgGraph");
     UASSERT_OBJ(!m_varp->isSc(), varp, "SystemC variable is not representable by DfgVertexVar");
+    // Increment reference count
+    varp->user1(varp->user1() + 0x10);
+    UASSERT_OBJ((varp->user1() >> 4) > 0, varp, "Reference count overflow");
 }
 DfgVertexVar::DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVarScope* vscp)
     : DfgVertexUnary{dfg, type, vscp->fileline(), dtypeFor(vscp)}
@@ -927,6 +931,16 @@ DfgVertexVar::DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVarScope* vscp)
     , m_varScopep{vscp} {
     UASSERT_OBJ(!dfg.modulep(), vscp, "Scoped DfgVertexVar created in un-scoped DfgGraph");
     UASSERT_OBJ(!m_varp->isSc(), vscp, "SystemC variable is not representable by DfgVertexVar");
+    // Increment reference count
+    vscp->user1(vscp->user1() + 0x10);
+    UASSERT_OBJ((vscp->user1() >> 4) > 0, vscp, "Reference count overflow");
+}
+
+DfgVertexVar::~DfgVertexVar() {
+    // Decrement reference count
+    AstNode* const nodep = this->nodep();
+    nodep->user1(nodep->user1() - 0x10);
+    UASSERT_OBJ((nodep->user1() >> 4) >= 0, nodep, "Reference count underflow");
 }
 
 //------------------------------------------------------------------------------
@@ -934,7 +948,7 @@ DfgVertexVar::DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVarScope* vscp)
 //------------------------------------------------------------------------------
 
 void DfgGraph::addVertex(DfgVertex& vtx) {
-    // Note: changes here need to be replicated in DfgGraph::addGraph
+    // Note: changes here need to be replicated in DfgGraph::mergeGraph
     ++m_size;
     if (DfgConst* const cVtxp = vtx.cast<DfgConst>()) {
         m_constVertices.linkBack(cVtxp);
@@ -948,7 +962,7 @@ void DfgGraph::addVertex(DfgVertex& vtx) {
 }
 
 void DfgGraph::removeVertex(DfgVertex& vtx) {
-    // Note: changes here need to be replicated in DfgGraph::addGraph
+    // Note: changes here need to be replicated in DfgGraph::mergeGraph
     --m_size;
     if (DfgConst* const cVtxp = vtx.cast<DfgConst>()) {
         m_constVertices.unlink(cVtxp);

src/V3DfgDecomposition.cpp

@@ -332,8 +332,6 @@ class ExtractCyclicComponents final {
             UASSERT_OBJ(clonep, &vtx, "Unhandled 'DfgVertexVar' sub-type");
             VertexState& cloneStatep = allocState(*clonep);
             cloneStatep.component = component;
-            // Mark variable as having references in other DFGs
-            vtx.setHasDfgRefs();
         }
         return *clonep;
     }

src/V3DfgOptimizer.cpp

@@ -243,11 +243,7 @@ static void process(DfgGraph& dfg, V3DfgContext& ctx) {
     std::vector<std::unique_ptr<DfgGraph>> cyclicComponents
         = dfg.extractCyclicComponents("cyclic");
 
-    // Split the remaining acyclic DFG into [weakly] connected components
-    std::vector<std::unique_ptr<DfgGraph>> acyclicComponents = dfg.splitIntoComponents("acyclic");
-
-    // Quick sanity check
-    UASSERT_OBJ(dfg.size() == 0, dfg.modulep(), "DfgGraph should have become empty");
+    std::vector<std::unique_ptr<DfgGraph>> madeAcyclicComponents;
 
     // Attempt to convert cyclic components into acyclic ones
     if (v3Global.opt.fDfgBreakCyckes()) {
@@ -261,21 +257,29 @@ static void process(DfgGraph& dfg, V3DfgContext& ctx) {
                 *it = std::move(result.first);
                 ++it;
             } else {
-                // Result became acyclic. Move to acyclicComponents, delete original.
-                acyclicComponents.emplace_back(std::move(result.first));
+                // Result became acyclic. Move to madeAcyclicComponents, delete original.
+                madeAcyclicComponents.emplace_back(std::move(result.first));
                 it = cyclicComponents.erase(it);
             }
         }
     }
+    // Merge those that were made acyclic back into the acyclic graph, this enables optimizing more
+    dfg.mergeGraphs(std::move(madeAcyclicComponents));
+
+    // Split the acyclic DFG into [weakly] connected components
+    std::vector<std::unique_ptr<DfgGraph>> acyclicComponents = dfg.splitIntoComponents("acyclic");
+
+    // Quick sanity check
+    UASSERT_OBJ(dfg.size() == 0, dfg.modulep(), "DfgGraph should have become empty");
 
     // For each acyclic component
     for (auto& component : acyclicComponents) {
         if (dumpDfgLevel() >= 7) component->dumpDotFilePrefixed(ctx.prefix() + "source");
         // Optimize the component
         V3DfgPasses::optimize(*component, ctx);
-        // Add back under the main DFG (we will convert everything back in one go)
-        dfg.addGraph(*component);
     }
+    // Merge back under the main DFG (we will convert everything back in one go)
+    dfg.mergeGraphs(std::move(acyclicComponents));
 
     // Eliminate redundant variables. Run this on the whole acyclic DFG. It needs to traverse
     // the module/netlist to perform variable substitutions. Doing this by component would do
@@ -287,9 +291,9 @@ static void process(DfgGraph& dfg, V3DfgContext& ctx) {
         if (dumpDfgLevel() >= 7) component->dumpDotFilePrefixed(ctx.prefix() + "source");
         // Converting back to Ast assumes the 'regularize' pass was run, so we must run it
         V3DfgPasses::regularize(*component, ctx.m_regularizeContext);
-        // Add back under the main DFG (we will convert everything back in one go)
-        dfg.addGraph(*component);
     }
+    // Merge back under the main DFG (we will convert everything back in one go)
+    dfg.mergeGraphs(std::move(cyclicComponents));
 }
 
 void V3DfgOptimizer::optimize(AstNetlist* netlistp, const string& label) {
@@ -301,7 +305,7 @@ void V3DfgOptimizer::optimize(AstNetlist* netlistp, const string& label) {
     // - bit1: Written via AstVarXRef (hierarchical reference)
     // - bit2: Read by logic in same module/netlist not represented in DFG
     // - bit3: Written by logic in same module/netlist not represented in DFG
-    // - bit4: Referenced by other DFG in same module/netlist
+    // - bit31-4: Reference count, how many DfgVertexVar represent this variable
     //
     // AstNode::user2/user3/user4 can be used by various DFG algorithms
     const VNUser1InUse user1InUse;

src/V3DfgVertices.h

@@ -50,6 +50,7 @@ class DfgVertexVar VL_NOT_FINAL : public DfgVertexUnary {
 public:
     inline DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVar* varp);
     inline DfgVertexVar(DfgGraph& dfg, VDfgType type, AstVarScope* vscp);
+    inline ~DfgVertexVar();
     ASTGEN_MEMBERS_DfgVertexVar;
 
     const std::string srcName(size_t) const override { return ""; }
@@ -82,8 +83,7 @@ public:
     void setHasModWrRefs() const { setHasModWrRefs(nodep()); }
 
     // Variable referenced from other DFG in the same module/netlist
-    bool hasDfgRefs() const { return nodep()->user1() & 0x10; }
-    void setHasDfgRefs() { nodep()->user1(nodep()->user1() | 0x10); }
+    bool hasDfgRefs() const { return nodep()->user1() >> 5; }  // I.e.: (nodep()->user1() >> 4) > 1
 
     // Variable referenced outside the containing module/netlist.
     bool hasExtRefs() const {