Use the same serial ordering within MTasks as we use in serial mode (#4994)

The goal here is to use as single ordering heuristic (which can be
improved later) within MTasks as we do for serial code ordering. The
heuristic itself is factored out into the new OrderMoveGraphSerializer.
This also yields slightly nicer ordering than the previously use
GraphStream, so we end up with fewer trigger (domain) conditionals in
the MTasks, this can be worth a few percent speedup.

This has the somewhat nice side-effect of reusing OrderMoveGraphVertex
for both serial and parallel mode, so MTaskMoveGraphVertex can be
removed.

Serial mode yields identical output.

src/CMakeLists.txt

@@ -124,7 +124,7 @@ set(HEADERS
     V3Order.h
     V3OrderInternal.h
     V3OrderGraph.h
-    V3OrderMoveGraphBuilder.h
+    V3OrderMoveGraph.h
     V3Os.h
     V3PairingHeap.h
     V3Param.h
@@ -277,6 +277,7 @@ set(COMMON_SOURCES
     V3Options.cpp
     V3Order.cpp
     V3OrderGraphBuilder.cpp
+    V3OrderMoveGraph.cpp
     V3OrderParallel.cpp
     V3OrderProcessDomains.cpp
     V3OrderSerial.cpp

src/Makefile_obj.in

@@ -267,6 +267,7 @@ RAW_OBJS_PCH_ASTNOMT = \
 	V3Name.o \
 	V3Order.o \
 	V3OrderGraphBuilder.o \
+	V3OrderMoveGraph.o \
 	V3OrderParallel.o \
 	V3OrderProcessDomains.o \
 	V3OrderSerial.o \

src/V3ExecGraph.cpp

@@ -39,6 +39,8 @@ ExecMTask::ExecMTask(V3Graph* graphp, AstMTaskBody* bodyp) VL_MT_DISABLED  //
       m_id{s_nextId++},
       m_hashName{V3Hasher::uncachedHash(bodyp).toString()} {
     UASSERT_OBJ(bodyp->stmtsp(), bodyp, "AstMTaskBody should already be populated for hashing");
+    UASSERT_OBJ(!bodyp->execMTaskp(), bodyp, "AstMTaskBody already linked to an ExecMTask");
+    bodyp->execMTaskp(this);
 }
 
 void ExecMTask::dump(std::ostream& str) const {

src/V3Graph.h

@@ -317,9 +317,9 @@ public:
     void rank(uint32_t rank) { m_rank = rank; }
     double fanout() const { return m_fanout; }
     void user(uint32_t user) { m_user = user; }
-    uint32_t user() const { return m_user; }
+    uint32_t user() const VL_MT_STABLE { return m_user; }
     void userp(void* userp) { m_userp = userp; }
-    void* userp() const { return m_userp; }
+    void* userp() const VL_MT_STABLE { return m_userp; }
     // ITERATORS
     V3GraphVertex* verticesNextp() const { return m_vertices.nextp(); }
     V3GraphEdge* inBeginp() const { return m_ins.begin(); }

src/V3OrderMoveGraph.cpp

@@ -14,81 +14,72 @@
 //
 //*************************************************************************
 //
-//  Move graph builder for ordering
+//  OrderMoveGraph implementation and related
 //
 //*************************************************************************
 
-#ifndef VERILATOR_V3ORDERMOVEGRAPHBUILDER_H_
+#include "V3PchAstNoMT.h"  // VL_MT_DISABLED_CODE_UNIT
-#define VERILATOR_V3ORDERMOVEGRAPHBUILDER_H_
 
-#include "config_build.h"
+#include "V3OrderMoveGraph.h"
-#include "verilatedos.h"
 
-#include "V3Ast.h"
 #include "V3Graph.h"
-#include "V3Order.h"
-#include "V3OrderGraph.h"
 
-template <class T_MoveVertex>
+VL_DEFINE_DEBUG_FUNCTIONS;
-class V3OrderMoveGraphBuilder final {
-    // V3OrderMoveGraphBuilder takes as input the fine-grained bipartite OrderGraph of
-    // OrderLogicVertex and OrderVarVertex vertices. It produces a slightly coarsened graph to
-    // drive the code scheduling.
-    //
-    // * For the serial code scheduler, the new graph contains
-    //   nodes of type OrderMoveVertex.
-    //
-    // * For the threaded code scheduler, the new graph contains
-    //   nodes of type MTaskMoveVertex.
-    //
-    // * The difference in output type is abstracted away by the
-    //   'T_MoveVertex' template parameter; ProcessMoveBuildGraph otherwise
-    //   works the same way for both cases.
 
+//======================================================================
+// OrderMoveDomScope implementation
+
+OrderMoveDomScope::DomScopeMap OrderMoveDomScope::s_dsMap;
+
+//======================================================================
+// OrderMoveVertex implementation
+
+OrderMoveVertex::OrderMoveVertex(OrderMoveGraph& graph, OrderLogicVertex* lVtxp,
+                                 const AstSenTree* domainp) VL_MT_DISABLED
+    : V3GraphVertex{&graph},
+      m_logicp{lVtxp},
+      m_domScope{OrderMoveDomScope::getOrCreate(domainp, lVtxp ? lVtxp->scopep() : nullptr)} {
+    UASSERT_OBJ(!lVtxp || lVtxp->domainp() == domainp, lVtxp, "Wrong domain for Move vertex");
+}
+
+//======================================================================
+// OrderMoveGraphBuilder - for OrderMoveGraph::build
+
+class OrderMoveGraphBuilder final {
     // NODE STATE
     // AstSenTree::user1p()     -> AstSenTree:  Original AstSenTree for trigger
     const VNUser1InUse m_user1InUse;
 
     // TYPES
-    using DomainMap = std::map<const AstSenTree*, T_MoveVertex*>;
+    using DomainMap = std::map<const AstSenTree*, OrderMoveVertex*>;
 
     // MEMBERS
     const OrderGraph& m_orderGraph;  // Input OrderGraph
-    // Output graph of T_MoveVertex vertices
+    std::unique_ptr<OrderMoveGraph> m_moveGraphp{new OrderMoveGraph};  // Output OrderMoveGraph
-    std::unique_ptr<V3Graph> m_outGraphp{new V3Graph};
     // Map from Trigger reference AstSenItem to the original AstSenTree
     const V3Order::TrigToSenMap& m_trigToSen;
-    // Storage for domain -> T_MoveVertex, maps held in OrderVarVertex::userp()
+    // Storage for domain -> OrderMoveVertex, maps held in OrderVarVertex::userp()
     std::deque<DomainMap> m_domainMaps;
 
     // CONSTRUCTORS
-    V3OrderMoveGraphBuilder(const OrderGraph& orderGraph, const V3Order::TrigToSenMap& trigToSen)
+    OrderMoveGraphBuilder(const OrderGraph& orderGraph, const V3Order::TrigToSenMap& trigToSen)
         : m_orderGraph{orderGraph}
         , m_trigToSen{trigToSen} {
-        build();
-    }
-    virtual ~V3OrderMoveGraphBuilder() = default;
-    VL_UNCOPYABLE(V3OrderMoveGraphBuilder);
-    VL_UNMOVABLE(V3OrderMoveGraphBuilder);
-
-    // METHODS
-    void build() {
         // How this works:
-        //  - Create a T_MoveVertex for each OrderLogicVertex.
+        //  - Create a OrderMoveVertex for each OrderLogicVertex.
-        //  - Following each OrderLogicVertex, search forward in the context of
+        //  - Following each OrderLogicVertex, search forward in the context of its domain
-        //    its domain...
+        //    - If we encounter another OrderLogicVertex in non-exclusive domain, make the
-        //    * If we encounter another OrderLogicVertex in non-exclusive
+        //      OrderMoveVertex->OrderMoveVertex edge.
-        //      domain, make the T_MoveVertex->T_MoveVertex edge.
+        //    - If we encounter an OrderVarVertex, make a Vertex for the (OrderVarVertex, domain)
-        //    * If we encounter an OrderVarVertex, make a Vertex for the
+        //      pair and continue to search forward in the context of the same domain.  Unless we
-        //      (OrderVarVertex, domain) pair and continue to search
+        //      already created that pair, in which case, we've already done the forward search,
-        //      forward in the context of the same domain.  Unless we
+        //      so stop.
-        //      already created that pair, in which case, we've already
-        //      done the forward search, so stop.
 
-        // For each logic vertex, make a T_MoveVertex, for each variable vertex, allocate storage
+        // For each logic vertex, make a OrderMoveVertex, for each variable vertex, allocate
+        // storage
         for (V3GraphVertex* itp = m_orderGraph.verticesBeginp(); itp; itp = itp->verticesNextp()) {
             if (OrderLogicVertex* const lvtxp = itp->cast<OrderLogicVertex>()) {
-                lvtxp->userp(new T_MoveVertex{*m_outGraphp, lvtxp, lvtxp->domainp()});
+                lvtxp->userp(new OrderMoveVertex{*m_moveGraphp, lvtxp, lvtxp->domainp()});
             } else {
                 // This is an OrderVarVertex
                 m_domainMaps.emplace_back();
@@ -101,7 +92,14 @@ class V3OrderMoveGraphBuilder final {
                 iterateLogicVertex(lvtxp);
             }
         }
+        m_moveGraphp->removeRedundantEdgesSum(&V3GraphEdge::followAlwaysTrue);
+        m_moveGraphp->userClearVertices();
     }
+    virtual ~OrderMoveGraphBuilder() = default;
+    VL_UNCOPYABLE(OrderMoveGraphBuilder);
+    VL_UNMOVABLE(OrderMoveGraphBuilder);
+
+    // METHODS
 
     // Returns the AstSenItem that originally corresponds to this AstSenTree, or nullptr if no
     // original AstSenTree, or if the original AstSenTree had multiple AstSenItems.
@@ -161,9 +159,13 @@ class V3OrderMoveGraphBuilder final {
         return fromSenItemp->edgeType().exclusiveEdge(toSenItemp->edgeType());
     }
 
+    void addEdge(OrderMoveVertex* srcp, OrderMoveVertex* dstp) {
+        new V3GraphEdge{m_moveGraphp.get(), srcp, dstp, 1};
+    }
+
     void iterateLogicVertex(const OrderLogicVertex* lvtxp) {
         AstSenTree* const domainp = lvtxp->domainp();
-        T_MoveVertex* const lMoveVtxp = static_cast<T_MoveVertex*>(lvtxp->userp());
+        OrderMoveVertex* const lMoveVtxp = static_cast<OrderMoveVertex*>(lvtxp->userp());
         // Search forward from lvtxp, making new edges from lMoveVtxp forward
         for (V3GraphEdge* edgep = lvtxp->outBeginp(); edgep; edgep = edgep->outNextp()) {
             if (edgep->weight() == 0) continue;  // Was cut
@@ -171,11 +173,11 @@ class V3OrderMoveGraphBuilder final {
             // OrderGraph is a bipartite graph, so we know it's an OrderVarVertex
             const OrderVarVertex* const vvtxp = static_cast<const OrderVarVertex*>(edgep->top());
 
-            // Look up T_MoveVertex for this domain on this variable
+            // Look up OrderMoveVertex for this domain on this variable
             DomainMap& mapp = *static_cast<DomainMap*>(vvtxp->userp());
             const auto pair = mapp.emplace(domainp, nullptr);
-            // Reference to the mapped T_MoveVertex
+            // Reference to the mapped OrderMoveVertex
-            T_MoveVertex*& vMoveVtxp = pair.first->second;
+            OrderMoveVertex*& vMoveVtxp = pair.first->second;
 
             // On first encounter, visit downstream logic dependent on this (var, domain)
             if (pair.second) vMoveVtxp = iterateVarVertex(vvtxp, domainp);
@@ -185,14 +187,14 @@ class V3OrderMoveGraphBuilder final {
             if (!vMoveVtxp) continue;
 
             // Add this (variable, domain) as dependent of the logic that writes it.
-            new V3GraphEdge{m_outGraphp.get(), lMoveVtxp, vMoveVtxp, 1};
+            addEdge(lMoveVtxp, vMoveVtxp);
         }
     }
 
-    // Return the T_MoveVertex for this (var, domain) pair, iff it has downstream dependencies,
+    // Return the OrderMoveVertex for this (var, domain) pair, iff it has downstream dependencies,
     // otherwise return nullptr.
-    T_MoveVertex* iterateVarVertex(const OrderVarVertex* vvtxp, AstSenTree* domainp) {
+    OrderMoveVertex* iterateVarVertex(const OrderVarVertex* vvtxp, AstSenTree* domainp) {
-        T_MoveVertex* vMoveVtxp = nullptr;
+        OrderMoveVertex* vMoveVtxp = nullptr;
         // Search forward from vvtxp, making new edges from vMoveVtxp forward
         for (V3GraphEdge* edgep = vvtxp->outBeginp(); edgep; edgep = edgep->outNextp()) {
             if (edgep->weight() == 0) continue;  // Was cut
@@ -204,18 +206,24 @@ class V3OrderMoveGraphBuilder final {
             if (domainsExclusive(domainp, lVtxp->domainp())) continue;
 
             // there is a path from this vvtx to a logic vertex. Add the new edge.
-            if (!vMoveVtxp) vMoveVtxp = new T_MoveVertex{*m_outGraphp, nullptr, domainp};
+            if (!vMoveVtxp) vMoveVtxp = new OrderMoveVertex{*m_moveGraphp, nullptr, domainp};
-            T_MoveVertex* const lMoveVxp = static_cast<T_MoveVertex*>(lVtxp->userp());
+            OrderMoveVertex* const lMoveVxp = static_cast<OrderMoveVertex*>(lVtxp->userp());
-            new V3GraphEdge{m_outGraphp.get(), vMoveVtxp, lMoveVxp, 1};
+            addEdge(vMoveVtxp, lMoveVxp);
         }
         return vMoveVtxp;
     }
 
 public:
-    static std::unique_ptr<V3Graph> apply(const OrderGraph& orderGraph,
+    static std::unique_ptr<OrderMoveGraph> apply(const OrderGraph& orderGraph,
-                                          const V3Order::TrigToSenMap& trigToSen) {
+                                                 const V3Order::TrigToSenMap& trigToSen) {
-        return std::move(V3OrderMoveGraphBuilder{orderGraph, trigToSen}.m_outGraphp);
+        return std::move(OrderMoveGraphBuilder{orderGraph, trigToSen}.m_moveGraphp);
     }
 };
 
-#endif  // Guard
+//======================================================================
+// OrderMoveGraph implementation
+
+std::unique_ptr<OrderMoveGraph> OrderMoveGraph::build(const OrderGraph& orderGraph,
+                                                      const V3Order::TrigToSenMap& trigToSen) {
+    return OrderMoveGraphBuilder::apply(orderGraph, trigToSen);
+}

src/V3OrderMoveGraph.h

@@ -0,0 +1,271 @@
+// -*- mode: C++; c-file-style: "cc-mode" -*-
+//*************************************************************************
+// DESCRIPTION: Verilator: Block code ordering
+//
+// Code available from: https://verilator.org
+//
+//*************************************************************************
+//
+// Copyright 2003-2024 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
+//
+//*************************************************************************
+//
+//  Move graph builder for ordering
+//
+//*************************************************************************
+
+#ifndef VERILATOR_V3ORDERMOVEGRAPH_H_
+#define VERILATOR_V3ORDERMOVEGRAPH_H_
+
+#include "config_build.h"
+#include "verilatedos.h"
+
+#include "V3Ast.h"
+#include "V3Graph.h"
+#include "V3Order.h"
+#include "V3OrderGraph.h"
+
+class OrderMoveVertex;
+
+// Information stored for each unique (domain, scope) pair. Mainly a list of ready vertices under
+// that (domain, scope). OrderMoveDomScope instances are themselves organized into a global ready
+// list if they have ready vertices.
+class OrderMoveDomScope final {
+    // STATE
+    V3List<OrderMoveVertex*> m_readyVertices;  // Ready vertices in this domain/scope
+    V3ListEnt<OrderMoveDomScope*> m_listEnt;  // List entry to store this instance
+    bool m_isOnList = false;  // True if DomScope is already on a list through m_listEnt
+    const AstSenTree* const m_domainp;  // Domain the vertices belong to
+    const AstScope* const m_scopep;  // Scope the vertices belong to
+
+    // Key type for map below
+    class DomScopeMapKey final {
+        const AstSenTree* const m_domainp;
+        const AstScope* const m_scopep;
+
+    public:
+        DomScopeMapKey(const AstSenTree* domainp, const AstScope* scopep)
+            : m_domainp{domainp}
+            , m_scopep{scopep} {}
+
+        struct Hash final {
+            size_t operator()(const DomScopeMapKey& key) const {
+                V3Hash hash{reinterpret_cast<uintptr_t>(key.m_domainp)};
+                hash += reinterpret_cast<uintptr_t>(key.m_scopep);
+                return hash.value();
+            }
+        };
+
+        struct Equal final {
+            bool operator()(const DomScopeMapKey& a, const DomScopeMapKey& b) const {
+                return a.m_domainp == b.m_domainp && a.m_scopep == b.m_scopep;
+            }
+        };
+    };
+
+    using DomScopeMap = std::unordered_map<DomScopeMapKey, OrderMoveDomScope, DomScopeMapKey::Hash,
+                                           DomScopeMapKey::Equal>;
+    // Map from Domain/Scope to the corresponding DomScope instance
+    static DomScopeMap s_dsMap;
+
+public:
+    // STATIC MEMBERS
+    static OrderMoveDomScope& getOrCreate(const AstSenTree* domainp, const AstScope* scopep) {
+        return s_dsMap
+            .emplace(std::piecewise_construct,  //
+                     std::forward_as_tuple(domainp, scopep),  //
+                     std::forward_as_tuple(domainp, scopep))
+            .first->second;
+    }
+    static void clear() { s_dsMap.clear(); }
+
+    // CONSTRUCTOR
+    OrderMoveDomScope(const AstSenTree* domainp, const AstScope* scopep)
+        : m_domainp{domainp}
+        , m_scopep{scopep} {}
+    ~OrderMoveDomScope() = default;
+    VL_UNCOPYABLE(OrderMoveDomScope);
+    VL_UNMOVABLE(OrderMoveDomScope);
+
+    // MEMBERS
+    V3List<OrderMoveVertex*>& readyVertices() { return m_readyVertices; }
+    const AstSenTree* domainp() const { return m_domainp; }
+    const AstScope* scopep() const { return m_scopep; }
+
+    bool isOnList() const { return m_isOnList; }
+    void unlinkFrom(V3List<OrderMoveDomScope*>& list) {
+        UASSERT_OBJ(m_isOnList, m_domainp, "unlinkFrom, but DomScope is not on a list");
+        m_isOnList = false;
+        m_listEnt.unlink(list, this);
+    }
+    void appendTo(V3List<OrderMoveDomScope*>& list) {
+        UASSERT_OBJ(!m_isOnList, m_domainp, "appendTo, but DomScope is already on a list");
+        m_isOnList = true;
+        m_listEnt.pushBack(list, this);
+    }
+    void prependTo(V3List<OrderMoveDomScope*>& list) {
+        UASSERT_OBJ(!m_isOnList, m_domainp, "prependTo, but DomScope is already on a list");
+        m_isOnList = true;
+        m_listEnt.pushFront(list, this);
+    }
+    OrderMoveDomScope* nextp() const { return m_listEnt.nextp(); }
+};
+
+//======================================================================
+// Graph type
+
+// OrderMoveGraph is constructed from the fine-grained OrderGraph.
+// It is a slightly coarsened representation of dependencies used to drive serialization.
+class OrderMoveGraph final : public V3Graph {
+public:
+    // Build an OrderMoveGraph from an OrderGraph
+    static std::unique_ptr<OrderMoveGraph> build(const OrderGraph&, const V3Order::TrigToSenMap&);
+};
+
+//======================================================================
+// Vertex types
+
+class OrderMoveVertex final : public V3GraphVertex {
+    VL_RTTI_IMPL(OrderMoveVertex, V3GraphVertex)
+
+    // The corresponding logic vertex, or nullptr if this MoveVertex stands for a variable vertex.
+    OrderLogicVertex* const m_logicp;
+    OrderMoveDomScope& m_domScope;  // DomScope this vertex is under
+    V3ListEnt<OrderMoveVertex*> m_listEnt;  // List entry to store this Vertex
+
+    // METHODS
+    std::string dotColor() const override { return logicp() ? logicp()->dotColor() : "yellow"; }
+
+    std::string name() const override VL_MT_STABLE {
+        std::string nm;
+        if (!logicp()) {
+            nm = "var";
+        } else {
+            nm = logicp()->name() + "\\n";
+            nm += "MV:";
+            nm += +" d=" + cvtToHex(logicp()->domainp());
+            nm += +" s=" + cvtToHex(logicp()->scopep());
+        }
+        if (userp()) nm += "\nu=" + cvtToHex(userp());
+        return nm;
+    }
+
+public:
+    // CONSTRUCTORS
+    OrderMoveVertex(OrderMoveGraph& graph, OrderLogicVertex* lVtxp,
+                    const AstSenTree* domainp) VL_MT_DISABLED;
+    ~OrderMoveVertex() override = default;
+    VL_UNCOPYABLE(OrderMoveVertex);
+
+    OrderLogicVertex* logicp() const VL_MT_STABLE { return m_logicp; }
+    OrderMoveDomScope& domScope() const { return m_domScope; }
+
+    void unlinkFrom(V3List<OrderMoveVertex*>& list) { m_listEnt.unlink(list, this); }
+    void appendTo(V3List<OrderMoveVertex*>& list) { m_listEnt.pushBack(list, this); }
+    void moveAppend(V3List<OrderMoveVertex*>& src, V3List<OrderMoveVertex*>& dst) {
+        m_listEnt.moveAppend(src, dst, this);
+    }
+    OrderMoveVertex* nextp() const { return m_listEnt.nextp(); }
+};
+
+//======================================================================
+// Serializer for OrderMoveGraph
+
+class OrderMoveGraphSerializer final {
+    // STATE
+    V3List<OrderMoveDomScope*> m_readyDomScopeps;  // List of DomScopes which have ready vertices
+    OrderMoveDomScope* m_nextDomScopep = nullptr;  // Next DomScope to yield from
+
+    // METHODS
+
+    void ready(OrderMoveVertex* vtxp) {
+        UASSERT_OBJ(!vtxp->user(), vtxp, "'ready' called on vertex with pending dependencies");
+        if (vtxp->logicp()) {
+            // Add this vertex to the ready list of its DomScope
+            OrderMoveDomScope& domScope = vtxp->domScope();
+            vtxp->appendTo(domScope.readyVertices());
+            // Add the DomScope to the global ready list if not there yet
+            if (!domScope.isOnList()) domScope.appendTo(m_readyDomScopeps);
+        } else {  // This is a bit nonsense at this point, but equivalent to the old version
+            // Remove dependency on vertex we are returning. This might add vertices to
+            // currReadyList.
+            for (V3GraphEdge *edgep = vtxp->outBeginp(), *nextp; edgep; edgep = nextp) {
+                nextp = edgep->outNextp();
+                // The dependent variable
+                OrderMoveVertex* const dVtxp = edgep->top()->as<OrderMoveVertex>();
+                // Update number of dependencies
+                const uint32_t nDeps = dVtxp->user() - 1;
+                dVtxp->user(nDeps);
+                // If no more dependencies, mark it ready
+                if (!nDeps) ready(dVtxp);
+            }
+        }
+    }
+
+public:
+    // CONSTRUCTOR
+    OrderMoveGraphSerializer(OrderMoveGraph& moveGraph) {
+        // Set V3GraphVertex::user() to the number of incoming edges (upstream dependencies)
+        for (V3GraphVertex *vtxp = moveGraph.verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
+            nextp = vtxp->verticesNextp();
+            uint32_t nDeps = 0;
+            for (V3GraphEdge* edgep = vtxp->inBeginp(); edgep; edgep = edgep->inNextp()) ++nDeps;
+            vtxp->user(nDeps);
+        }
+    }
+    ~OrderMoveGraphSerializer() = default;
+    VL_UNCOPYABLE(OrderMoveGraphSerializer);
+
+    // Add a seed vertex to the ready lists
+    void addSeed(OrderMoveVertex* vtxp) { ready(vtxp); }
+
+    OrderMoveVertex* getNext() {
+        if (!m_nextDomScopep) m_nextDomScopep = m_readyDomScopeps.begin();
+        OrderMoveDomScope* const currDomScopep = m_nextDomScopep;
+        // If nothing is ready, we are done
+        if (!currDomScopep) return nullptr;
+
+        V3List<OrderMoveVertex*>& currReadyList = currDomScopep->readyVertices();
+        // This is the vertex we are returning now
+        OrderMoveVertex* const mVtxp = currReadyList.begin();
+        UASSERT(mVtxp, "DomScope on ready list, but has no ready vertices");
+        // Unlink vertex from ready list under the DomScope
+        mVtxp->unlinkFrom(currReadyList);
+
+        // Nonsesne, but what we used to do
+        if (currReadyList.empty()) currDomScopep->unlinkFrom(m_readyDomScopeps);
+
+        // Remove dependency on vertex we are returning. This might add vertices to currReadyList.
+        for (V3GraphEdge *edgep = mVtxp->outBeginp(), *nextp; edgep; edgep = nextp) {
+            nextp = edgep->outNextp();
+            // The dependent variable
+            OrderMoveVertex* const dVtxp = edgep->top()->as<OrderMoveVertex>();
+            // Update number of dependencies
+            const uint32_t nDeps = dVtxp->user() - 1;
+            dVtxp->user(nDeps);
+            // If no more dependencies, mark it ready
+            if (!nDeps) ready(dVtxp);
+        }
+
+        // If no more ready vertices in the current DomScope, prefer to continue with a new scope
+        // under the same domain.
+        if (currReadyList.empty()) {
+            m_nextDomScopep = nullptr;
+            for (OrderMoveDomScope* dsp = m_readyDomScopeps.begin(); dsp; dsp = dsp->nextp()) {
+                if (dsp->domainp() == currDomScopep->domainp()) {
+                    m_nextDomScopep = dsp;
+                    break;
+                }
+            }
+        }
+
+        // Finally yield the selected vertex
+        return mVtxp;
+    }
+};
+
+#endif  // Guard

src/V3OrderParallel.cpp

@@ -29,14 +29,13 @@
 #include "V3List.h"
 #include "V3OrderCFuncEmitter.h"
 #include "V3OrderInternal.h"
-#include "V3OrderMoveGraphBuilder.h"
+#include "V3OrderMoveGraph.h"
 #include "V3Os.h"
 #include "V3PairingHeap.h"
 #include "V3Scoreboard.h"
 #include "V3Stats.h"
 
 #include <array>
-#include <list>
 #include <memory>
 #include <type_traits>
 #include <unordered_map>
@@ -49,40 +48,6 @@ class MTaskEdge;
 class MergeCandidate;
 class SiblingMC;
 
-// Similar to OrderMoveVertex, but modified for threaded code generation.
-class MTaskMoveVertex final : public V3GraphVertex {
-    VL_RTTI_IMPL(MTaskMoveVertex, V3GraphVertex)
-    OrderLogicVertex* const m_logicp;  // Logic represented by this vertex, or nullptr if variable
-    const AstSenTree* const m_domainp;
-
-public:
-    MTaskMoveVertex(V3Graph& graph, OrderLogicVertex* logicp,
-                    const AstSenTree* domainp) VL_MT_DISABLED : V3GraphVertex{&graph},
-                                                                m_logicp{logicp},
-                                                                m_domainp{domainp} {}
-    ~MTaskMoveVertex() override = default;
-
-    // ACCESSORS
-    OrderLogicVertex* logicp() const { return m_logicp; }
-    const AstScope* scopep() const { return m_logicp ? m_logicp->scopep() : nullptr; }
-    const AstSenTree* domainp() const { return m_domainp; }
-
-    string dotColor() const override { return logicp() ? logicp()->dotColor() : "yellow"; }
-    string name() const override {
-        std::string nm;
-        if (!logicp()) {
-            nm = "var";
-        } else {
-            nm = logicp()->name() + "\\n";
-            nm += "MV:";
-            nm += +" d=" + cvtToHex(logicp()->domainp());
-            nm += +" s=" + cvtToHex(logicp()->scopep());
-        }
-        nm += "\nt=" + std::to_string(color());  // "color()" represents the mtask ID.
-        return nm;
-    }
-};
-
 // ######################################################################
 // Partitioner tunable settings:
 //
@@ -217,8 +182,6 @@ class LogicMTask final : public V3GraphVertex {
 
 public:
     // TYPES
-    using VxList = std::list<MTaskMoveVertex*>;
-
     struct CmpLogicMTask final {
         bool operator()(const LogicMTask* ap, const LogicMTask* bp) const {
             return ap->id() < bp->id();
@@ -228,10 +191,9 @@ public:
 private:
     // MEMBERS
 
-    // Set of MTaskMoveVertex's assigned to this mtask. LogicMTask does not
+    // List of OrderMoveVertex's assigned to this mtask. LogicMTask does not
-    // own the MTaskMoveVertex objects, we merely keep pointers to them
+    // own the OrderMoveVertex objects, we merely keep them in a list here.
-    // here.
+    V3List<OrderMoveVertex*> m_mVertices;
-    VxList m_mvertices;
 
     // Cost estimate for this LogicMTask, derived from V3InstrCount.
     // In abstract time units.
@@ -265,14 +227,14 @@ private:
 
 public:
     // CONSTRUCTORS
-    LogicMTask(V3Graph* graphp, MTaskMoveVertex* mtmvVxp)
+    LogicMTask(V3Graph* graphp, OrderMoveVertex* mVtxp)
         : V3GraphVertex{graphp}
         , m_id{s_nextId++} {
         UASSERT(s_nextId < 0xFFFFFFFFUL, "Too many mTaskGraphp");
         for (uint32_t& item : m_critPathCost) item = 0;
-        if (mtmvVxp) {  // Else null for test
+        if (mVtxp) {
-            m_mvertices.push_back(mtmvVxp);
+            mVtxp->appendTo(m_mVertices);
-            if (const OrderLogicVertex* const olvp = mtmvVxp->logicp()) {
+            if (const OrderLogicVertex* const olvp = mVtxp->logicp()) {
                 m_cost += V3InstrCount::count(olvp->nodep(), true);
             }
         }
@@ -283,12 +245,12 @@ public:
     V3List<SiblingMC*>& aSiblingMCs() { return m_aSiblingMCs; };
     V3List<SiblingMC*>& bSiblingMCs() { return m_bSiblingMCs; };
 
+    V3List<OrderMoveVertex*>& vertexList() { return m_mVertices; }
+    const V3List<OrderMoveVertex*>& vertexList() const { return m_mVertices; }
     void moveAllVerticesFrom(LogicMTask* otherp) {
-        // splice() is constant time
+        otherp->m_mVertices.begin()->moveAppend(otherp->m_mVertices, m_mVertices);
-        m_mvertices.splice(m_mvertices.end(), otherp->m_mvertices);
         m_cost += otherp->m_cost;
     }
-    const VxList& vertexList() const { return m_mvertices; }
     static uint64_t incGeneration() {
         static uint64_t s_generation = 0;
         ++s_generation;
@@ -675,7 +637,9 @@ void LogicMTask::dumpCpFilePrefixed(const V3Graph& graph, const string& nameComm
     // Dump
     for (const LogicMTask* mtaskp : path) {
         *osp << "begin mtask with cost " << mtaskp->cost() << '\n';
-        for (MTaskMoveVertex* const mVtxp : mtaskp->vertexList()) {
+        const V3List<OrderMoveVertex*>& vertexList = mtaskp->vertexList();
+        for (OrderMoveVertex *mVtxp = vertexList.begin(), *nextp; mVtxp; mVtxp = nextp) {
+            nextp = mVtxp->nextp();
             const OrderLogicVertex* const logicp = mVtxp->logicp();
             if (!logicp) continue;
             // Show nodes with hierarchical costs
@@ -1890,9 +1854,10 @@ class FixDataHazards final {
                 // Set up the OrderLogicVertex -> LogicMTask map
                 // Entry and exit MTasks have no MTaskMoveVertices under them, so move on
                 if (mtaskp->vertexList().empty()) continue;
-                // Otherwise there should be only one MTaskMoveVertex in each MTask at this stage
+                // Otherwise there should be only one OrderMoveVertex in each MTask at this stage
-                UASSERT_OBJ(mtaskp->vertexList().size() == 1, mtaskp, "Multiple MTaskMoveVertex");
+                const V3List<OrderMoveVertex*>& vertexList = mtaskp->vertexList();
-                const MTaskMoveVertex* const moveVtxp = mtaskp->vertexList().front();
+                UASSERT_OBJ(!vertexList.begin()->nextp(), mtaskp, "Multiple OrderMoveVertex");
+                const OrderMoveVertex* const moveVtxp = vertexList.begin();
                 // Set up mapping back to the MTask from the OrderLogicVertex
                 if (OrderLogicVertex* const lvtxp = moveVtxp->logicp()) lvtxp->userp(mtaskp);
             }
@@ -2017,8 +1982,10 @@ class FixDataHazards final {
                 // Merge donor into recipient.
                 if (donorp == recipientp) continue;
                 // Fix up the map, so donor's OLVs map to recipientp
-                for (const MTaskMoveVertex* const tmvp : donorp->vertexList()) {
+                const V3List<OrderMoveVertex*>& vtxList = donorp->vertexList();
-                    tmvp->logicp()->userp(recipientp);
+                for (const OrderMoveVertex *vtxp = vtxList.begin(), *nextp; vtxp; vtxp = nextp) {
+                    nextp = vtxp->nextp();
+                    vtxp->logicp()->userp(recipientp);
                 }
                 // Move all vertices from donorp to recipientp
                 recipientp->moveAllVerticesFrom(donorp);
@@ -2033,8 +2000,9 @@ class FixDataHazards final {
         }
     }
     bool hasDpiHazard(LogicMTask* mtaskp) {
-        for (const MTaskMoveVertex* const moveVtxp : mtaskp->vertexList()) {
+        const V3List<OrderMoveVertex*>& vertexList = mtaskp->vertexList();
-            if (OrderLogicVertex* const lvtxp = moveVtxp->logicp()) {
+        for (const OrderMoveVertex* mVtxp = vertexList.begin(); mVtxp; mVtxp = mVtxp->nextp()) {
+            if (OrderLogicVertex* const lvtxp = mVtxp->logicp()) {
                 // NOTE: We don't handle DPI exports. If testbench code calls a
                 // DPI-exported function at any time during eval() we may have
                 // a data hazard. (Likewise in non-threaded mode if an export
@@ -2153,7 +2121,7 @@ static void hashGraphDebug(const V3Graph& graph, const char* debugName) {
 
 class Partitioner final {
     // MEMBERS
-    const V3Graph& m_fineDepsGraph;  // Fine-grained dependency graph
+    OrderMoveGraph& m_moveGraph;  // Fine-grained dependency graph
     std::unique_ptr<V3Graph> m_mTaskGraphp{new V3Graph{}};  // The resulting MTask graph
 
     LogicMTask* m_entryMTaskp = nullptr;  // Singular source vertex of the dependency graph
@@ -2161,12 +2129,12 @@ class Partitioner final {
 
     // METHODS
 
-    // Predicate function to determine what MTaskMoveVertex to bypass when constructing the MTask
+    // Predicate function to determine what OrderMoveVertex to bypass when constructing the MTask
-    // graph. The fine-grained dependency graph of MTaskMoveVertex vertices is a bipartite graph
+    // graph. The fine-grained dependency graph of OrderMoveVertex vertices is a bipartite graph
     // of:
-    // - 1. MTaskMoveVertex instances containing logic via OrderLogicVertex
+    // - 1. OrderMoveVertex instances containing logic via OrderLogicVertex
-    //      (MTaskMoveVertex::logicp() != nullptr)
+    //      (OrderMoveVertex::logicp() != nullptr)
-    // - 2. MTaskMoveVertex instances containing an (OrderVarVertex, domain) pair
+    // - 2. OrderMoveVertex instances containing an (OrderVarVertex, domain) pair
     // Our goal is to order the logic vertices. The second type of variable/domain vertices only
     // carry dependencies and are eventually discarded. In order to reduce the working set size of
     // Contraction, we 'bypass' and not create LogicMTask vertices for the variable vertices,
@@ -2175,7 +2143,7 @@ class Partitioner final {
     // That is, we bypass a variable vertex if fanIn * fanOut <= fanIn + fanOut. This can only be
     // true if fanIn or fanOut are 1, or if they are both 2. This can cause significant reduction
     // in working set size.
-    static bool bypassOk(MTaskMoveVertex* mvtxp) {
+    static bool bypassOk(OrderMoveVertex* mvtxp) {
         // Need to keep all logic vertices
         if (mvtxp->logicp()) return false;
         // Count fan-in, up to 3
@@ -2210,10 +2178,10 @@ class Partitioner final {
         // node, to assert that we never count any node twice.
         const VNUser1InUse user1inUse;
 
-        // Create the LogicMTasks for each MTaskMoveVertex
+        // Create the LogicMTasks for each OrderMoveVertex
-        for (V3GraphVertex *vtxp = m_fineDepsGraph.verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
+        for (V3GraphVertex *vtxp = m_moveGraph.verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
             nextp = vtxp->verticesNextp();
-            MTaskMoveVertex* const mVtxp = static_cast<MTaskMoveVertex*>(vtxp);
+            OrderMoveVertex* const mVtxp = static_cast<OrderMoveVertex*>(vtxp);
             if (bypassOk(mVtxp)) {
                 mVtxp->userp(nullptr);  // Set to nullptr to mark as bypassed
             } else {
@@ -2228,7 +2196,7 @@ class Partitioner final {
         m_exitMTaskp = new LogicMTask{m_mTaskGraphp.get(), nullptr};
 
         // Create the mtask->mtask dependency edges based on the dependencies between
-        // MTaskMoveVertex vertices.
+        // OrderMoveVertex vertices.
         for (V3GraphVertex *vtxp = m_mTaskGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
             nextp = vtxp->verticesNextp();
             LogicMTask* const mtaskp = static_cast<LogicMTask*>(vtxp);
@@ -2236,10 +2204,11 @@ class Partitioner final {
             // Entry and exit vertices handled separately
             if (VL_UNLIKELY((mtaskp == m_entryMTaskp) || (mtaskp == m_exitMTaskp))) continue;
 
-            // At this point, there should only be one MTaskMoveVertex per LogicMTask
+            const V3List<OrderMoveVertex*>& vertexList = mtaskp->vertexList();
-            UASSERT_OBJ(mtaskp->vertexList().size() == 1, mtaskp, "Multiple MTaskMoveVertex");
+            OrderMoveVertex* const mvtxp = vertexList.begin();
-            MTaskMoveVertex* const mvtxp = mtaskp->vertexList().front();
+            // At this point, there should only be one OrderMoveVertex per LogicMTask
-            UASSERT_OBJ(mvtxp->userp(), mtaskp, "Bypassed MTaskMoveVertex should not have MTask");
+            UASSERT_OBJ(!mvtxp->nextp(), mtaskp, "Multiple OrderMoveVertex");
+            UASSERT_OBJ(mvtxp->userp(), mtaskp, "Bypassed OrderMoveVertex should not have MTask");
 
             // Function to add a edge to a dependent from 'mtaskp'
             const auto addEdge = [this, mtaskp](LogicMTask* otherp) {
@@ -2288,15 +2257,15 @@ class Partitioner final {
     }
 
     // CONSTRUCTORS
-    Partitioner(const OrderGraph& orderGraph, const V3Graph& fineDepsGraph)
+    Partitioner(const OrderGraph& orderGraph, OrderMoveGraph& moveGraph)
-        : m_fineDepsGraph{fineDepsGraph} {
+        : m_moveGraph{moveGraph} {
         // Fill in the m_mTaskGraphp with LogicMTask's and their interdependencies.
 
         // Called by V3Order
-        hashGraphDebug(m_fineDepsGraph, "v3partition initial fine-grained deps");
+        hashGraphDebug(m_moveGraph, "v3partition initial fine-grained deps");
 
         // Create the first MTasks. Initially, each MTask just wraps one
-        // MTaskMoveVertex. Over time, we'll merge MTasks together and
+        // OrderMoveVertex. Over time, we'll merge MTasks together and
         // eventually each MTask will wrap a large number of MTaskMoveVertices
         // (and the logic nodes therein.)
         const uint32_t totalGraphCost = setupMTaskDeps();
@@ -2361,22 +2330,35 @@ class Partitioner final {
         m_mTaskGraphp->removeTransitiveEdges();
         debugMTaskGraphStats(*m_mTaskGraphp, "transitive1");
 
-        // Set color to indicate the mtaskId on every underlying logic MTaskMoveVertex.
+        // Remove MTasks that have no logic in it rerouting the edges. Set user to indicate the
-        // Remove any MTasks that have no logic in it rerouting the edges.
+        // mtask on every underlying OrderMoveVertex. Clear vertex lists (used later).
+        m_moveGraph.userClearVertices();
         for (V3GraphVertex *vtxp = m_mTaskGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
             nextp = vtxp->verticesNextp();
-            const LogicMTask* const mtaskp = vtxp->as<LogicMTask>();
+            LogicMTask* const mtaskp = vtxp->as<LogicMTask>();
+            V3List<OrderMoveVertex*>& vertexList = mtaskp->vertexList();
+            // Check if MTask is empty
             bool empty = true;
-            for (MTaskMoveVertex* const mVtxp : mtaskp->vertexList()) {
+            for (OrderMoveVertex *mVtxp = vertexList.begin(), *nextp; mVtxp; mVtxp = nextp) {
-                if (!mVtxp->logicp()) continue;
+                nextp = mVtxp->nextp();
-                empty = false;
+                if (mVtxp->logicp()) {
-                mVtxp->color(mtaskp->id());
+                    empty = false;
+                    break;
+                }
             }
+            // If empty remove it now
             if (empty) {
-                vtxp->rerouteEdges(m_mTaskGraphp.get());
+                mtaskp->rerouteEdges(m_mTaskGraphp.get());
-                vtxp->unlinkDelete(m_mTaskGraphp.get());
+                VL_DO_DANGLING(mtaskp->unlinkDelete(m_mTaskGraphp.get()), mtaskp);
+                continue;
+            }
+            // Annotate the underlying OrderMoveVertex vertices and unlink them
+            while (OrderMoveVertex* mVtxp = vertexList.begin()) {
+                mVtxp->userp(mtaskp);
+                mVtxp->unlinkFrom(vertexList);
             }
         }
+        m_mTaskGraphp->removeRedundantEdgesSum(&V3GraphEdge::followAlwaysTrue);
     }
     ~Partitioner() = default;
     VL_UNCOPYABLE(Partitioner);
@@ -2384,31 +2366,8 @@ class Partitioner final {
 
 public:
     static std::unique_ptr<V3Graph> apply(const OrderGraph& orderGraph,
-                                          const V3Graph& fineDepsGraph) {
+                                          OrderMoveGraph& moveGraph) {
-        return std::move(Partitioner{orderGraph, fineDepsGraph}.m_mTaskGraphp);
+        return std::move(Partitioner{orderGraph, moveGraph}.m_mTaskGraphp);
-    }
-};
-
-// Sort MTaskMoveVertex vertices by domain, then by scope, based on teh order they are encountered
-class OrderVerticesByDomainThenScope final {
-    mutable uint64_t m_nextId = 0;  // Next id to use
-    mutable std::unordered_map<const void*, uint64_t> m_id;  // Map from ptr to id
-
-    // Map a pointer into an id, for deterministic results
-    uint64_t findId(const void* ptrp) const {
-        const auto pair = m_id.emplace(ptrp, m_nextId);
-        if (pair.second) ++m_nextId;
-        return pair.first->second;
-    }
-
-public:
-    bool operator()(const V3GraphVertex* lhsp, const V3GraphVertex* rhsp) const {
-        const MTaskMoveVertex* const l_vxp = lhsp->as<MTaskMoveVertex>();
-        const MTaskMoveVertex* const r_vxp = rhsp->as<MTaskMoveVertex>();
-        const uint64_t l_id = findId(l_vxp->domainp());
-        const uint64_t r_id = findId(r_vxp->domainp());
-        if (l_id != r_id) return l_id < r_id;
-        return findId(l_vxp->scopep()) < findId(r_vxp->scopep());
     }
 };
 
@@ -2426,92 +2385,119 @@ AstExecGraph* V3Order::createParallel(const OrderGraph& orderGraph, const std::s
     // For nondeterminism debug:
     hashGraphDebug(orderGraph, "V3OrderParallel's input OrderGraph");
 
-    // Starting from the orderGraph, make a slightly-coarsened graph representing
+    // Build the move graph
-    // only logic, and discarding edges we know we can ignore.
+    OrderMoveDomScope::clear();
-    // This is quite similar to the 'm_pomGraph' of the serial code gen:
+    const std::unique_ptr<OrderMoveGraph> moveGraphp
-    const std::unique_ptr<V3Graph> logicGraphp
+        = OrderMoveGraph::build(orderGraph, trigToSen);
-        = V3OrderMoveGraphBuilder<MTaskMoveVertex>::apply(orderGraph, trigToSen);
+    if (dumpGraphLevel() >= 9) moveGraphp->dumpDotFilePrefixed(tag + "_ordermv");
 
-    // Needed? We do this for m_pomGraph in serial mode, so do it here too:
+    // Partition moveGraphp into LogicMTask's. The partitioner will set userp() on each logic
-    logicGraphp->removeRedundantEdgesMax(&V3GraphEdge::followAlwaysTrue);
+    // vertex in the moveGraphp to the MTask it belongs to.
+    const std::unique_ptr<V3Graph> mTaskGraphp = Partitioner::apply(orderGraph, *moveGraphp);
+    if (dumpGraphLevel() >= 9) moveGraphp->dumpDotFilePrefixed(tag + "_ordermv_mtasks");
 
-    // Partition logicGraph into LogicMTask's. The partitioner will annotate
+    // Some variable OrderMoveVertices are not assigned to an MTask. Reroute and delete these.
-    // each vertex in logicGraph with a 'color' which is really an mtask ID
+    for (V3GraphVertex *vtxp = moveGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
-    // in this context.
+        nextp = vtxp->verticesNextp();
-    const std::unique_ptr<V3Graph> mTaskGraphp = Partitioner::apply(orderGraph, *logicGraphp);
+        OrderMoveVertex* const mVtxp = vtxp->as<OrderMoveVertex>();
-
+        if (!mVtxp->userp()) {
-    struct MTaskState final {
+            UASSERT_OBJ(!mVtxp->logicp(), mVtxp, "Logic OrderMoveVertex not assigned to mtask");
-        AstMTaskBody* m_mtaskBodyp = nullptr;
+            mVtxp->rerouteEdges(moveGraphp.get());
-        std::vector<const OrderLogicVertex*> m_logics;
+            VL_DO_DANGLING(vtxp->unlinkDelete(moveGraphp.get()), vtxp);
-        ExecMTask* m_execMTaskp = nullptr;
+        }
-    };
-
-    std::unordered_map<uint32_t /*mtask id*/, MTaskState> mtaskStates;
-
-    // Iterate through the entire logicGraph. For each logic node,
-    // attach it to a per-MTask ordered list of logic nodes.
-    // This is the order we'll execute logic nodes within the MTask.
-    //
-    // MTasks may span scopes and domains, so sort by both here:
-    GraphStream<OrderVerticesByDomainThenScope> logicStream{logicGraphp.get()};
-    while (const V3GraphVertex* const vtxp = logicStream.nextp()) {
-        const MTaskMoveVertex* const movep = vtxp->as<MTaskMoveVertex>();
-        // Only care about logic vertices
-        if (!movep->logicp()) continue;
-
-        const unsigned mtaskId = movep->color();
-        UASSERT(mtaskId > 0, "Every MTaskMoveVertex should have an mtask assignment >0");
-
-        // Add this logic to the per-mtask order
-        mtaskStates[mtaskId].m_logics.push_back(movep->logicp());
     }
 
-    // Create the AstExecGraph node which represents the execution
+    // Remove all edges from the move graph that cross between MTasks. Add logic to MTask lists.
-    // of the MTask graph.
+    for (V3GraphVertex *vtxp = moveGraphp->verticesBeginp(), *nextpVtxp; vtxp; vtxp = nextpVtxp) {
+        nextpVtxp = vtxp->verticesNextp();
+        OrderMoveVertex* const mVtxp = vtxp->as<OrderMoveVertex>();
+        LogicMTask* const mtaskp = static_cast<LogicMTask*>(mVtxp->userp());
+        // Add to list in MTask, in MoveGraph order. This should not be necessary, but see #4993.
+        mVtxp->appendTo(mtaskp->vertexList());
+        // Remove edges crossing between MTasks
+        for (V3GraphEdge *edgep = mVtxp->outBeginp(), *nextEdgep; edgep; edgep = nextEdgep) {
+            nextEdgep = edgep->outNextp();
+            const OrderMoveVertex* const toMVtxp = edgep->top()->as<OrderMoveVertex>();
+            if (mtaskp != toMVtxp->userp()) VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
+        }
+    }
+    if (dumpGraphLevel() >= 9) moveGraphp->dumpDotFilePrefixed(tag + "_ordermv_pruned");
+
+    // Create the AstExecGraph node which represents the execution of the MTask graph.
     FileLine* const rootFlp = v3Global.rootp()->fileline();
     AstExecGraph* const execGraphp = new AstExecGraph{rootFlp, tag};
     V3Graph* const depGraphp = execGraphp->depGraphp();
 
-    // Create CFuncs and bodies for each MTask.
+    // Translate the LogicMTask graph into the corresponding ExecMTask graph,
+    // which will outlive ordering.
+    std::unordered_map<const LogicMTask*, ExecMTask*> logicMTaskToExecMTask;
+    OrderMoveGraphSerializer serializer{*moveGraphp};
     V3OrderCFuncEmitter emitter{tag, slow};
     GraphStream<MTaskVxIdLessThan> mtaskStream{mTaskGraphp.get()};
     while (const V3GraphVertex* const vtxp = mtaskStream.nextp()) {
-        const LogicMTask* const mtaskp = vtxp->as<LogicMTask>();
+        const LogicMTask* const cMTaskp = vtxp->as<LogicMTask>();
+        LogicMTask* const mTaskp = const_cast<LogicMTask*>(cMTaskp);
 
-        // Create a body for this mtask
+        // Add initially ready vertices within this MTask to the serializer as seeds,
+        // and unlink them from the vertex list in the MTask as we go.
+        V3List<OrderMoveVertex*>& vertexList = mTaskp->vertexList();
+        while (OrderMoveVertex* vtxp = vertexList.begin()) {
+            // The serializer uses the list node in the vertex, so must unlink here
+            vtxp->unlinkFrom(vertexList);
+            if (vtxp->inEmpty()) serializer.addSeed(vtxp);
+        }
+
+        // Emit all logic within the MTask as they become ready
+        OrderMoveDomScope* prevDomScopep = nullptr;
+        while (OrderMoveVertex* const mVtxp = serializer.getNext()) {
+            // We only really care about logic vertices
+            if (OrderLogicVertex* const logicp = mVtxp->logicp()) {
+                // Force a new function if the domain or scope changed, for better combining.
+                OrderMoveDomScope* const domScopep = &mVtxp->domScope();
+                if (domScopep != prevDomScopep) emitter.forceNewFunction();
+                prevDomScopep = domScopep;
+                // Emit the logic under this vertex
+                emitter.emitLogic(logicp);
+            }
+            // Can delete the vertex now
+            VL_DO_DANGLING(mVtxp->unlinkDelete(moveGraphp.get()), mVtxp);
+        }
+
+        // We have 2 objects, because AstMTaskBody is an AstNode, and ExecMTask is a GraphVertex.
+        // To combine them would involve multiple inheritance.
+
+        // Construct the actual MTaskBody
         AstMTaskBody* const bodyp = new AstMTaskBody{rootFlp};
-        MTaskState& state = mtaskStates[mtaskp->id()];
+        execGraphp->addMTaskBodiesp(bodyp);
-        state.m_mtaskBodyp = bodyp;
-
-        // Emit functions with this MTaks's logic, and call them in the body.
-        for (const OrderLogicVertex* lVtxp : state.m_logics) emitter.emitLogic(lVtxp);
         for (AstActive* const activep : emitter.getAndClearActiveps()) bodyp->addStmtsp(activep);
         UASSERT_OBJ(bodyp->stmtsp(), bodyp, "Should not try to create empty MTask");
 
-        // Translate the LogicMTask graph into the corresponding ExecMTask
+        // Create the ExecMTask
-        // graph, which will outlive V3Order and persist for the remainder
+        ExecMTask* const execMTaskp = new ExecMTask{depGraphp, bodyp};
-        // of verilator's processing.
+        const bool newEntry = logicMTaskToExecMTask.emplace(mTaskp, execMTaskp).second;
-        // - The LogicMTask graph points to MTaskMoveVertex's
+        UASSERT_OBJ(newEntry, mTaskp, "LogicMTasks should be processed in dependencyorder");
-        //   and OrderLogicVertex's which are ephemeral to V3Order.
+        UINFO(3, "Final '" << tag << "' LogicMTask " << mTaskp->id() << " maps to ExecMTask"
-        // - The ExecMTask graph and the AstMTaskBody's produced here
+                           << execMTaskp->id() << std::endl);
-        //   persist until code generation time.
+
-        state.m_execMTaskp = new ExecMTask{depGraphp, bodyp};
+        // Add the dependency edges between ExecMTasks
-        UINFO(3, "Final '" << tag << "' LogicMTask " << mtaskp->id() << " maps to ExecMTask"
+        for (const V3GraphEdge* inp = mTaskp->inBeginp(); inp; inp = inp->inNextp()) {
-                           << state.m_execMTaskp->id() << std::endl);
-        // Cross-link each ExecMTask and MTaskBody
-        //  Q: Why even have two objects?
-        //  A: One is an AstNode, the other is a GraphVertex,
-        //     to combine them would involve multiple inheritance...
-        state.m_mtaskBodyp->execMTaskp(state.m_execMTaskp);
-        for (V3GraphEdge* inp = mtaskp->inBeginp(); inp; inp = inp->inNextp()) {
             const V3GraphVertex* fromVxp = inp->fromp();
             const LogicMTask* const fromp = fromVxp->as<const LogicMTask>();
-            const MTaskState& fromState = mtaskStates[fromp->id()];
+            new V3GraphEdge{depGraphp, logicMTaskToExecMTask.at(fromp), execMTaskp, 1};
-            new V3GraphEdge{depGraphp, fromState.m_execMTaskp, state.m_execMTaskp, 1};
         }
-        execGraphp->addMTaskBodiesp(bodyp);
     }
 
+    // Delete the remaining variable vertices
+    for (V3GraphVertex *vtxp = moveGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
+        nextp = vtxp->verticesNextp();
+        if (!vtxp->as<OrderMoveVertex>()->logicp()) {
+            VL_DO_DANGLING(vtxp->unlinkDelete(moveGraphp.get()), vtxp);
+        }
+    }
+
+    UASSERT(moveGraphp->empty(), "Waiting vertices remain, but none are ready");
+    OrderMoveDomScope::clear();
+
     return execGraphp;
 }
 

src/V3OrderSerial.cpp

@@ -20,283 +20,63 @@
 
 #include "V3PchAstNoMT.h"  // VL_MT_DISABLED_CODE_UNIT
 
-#include "V3Graph.h"
-#include "V3List.h"
 #include "V3OrderCFuncEmitter.h"
 #include "V3OrderInternal.h"
-#include "V3OrderMoveGraphBuilder.h"
+#include "V3OrderMoveGraph.h"
 
 #include <memory>
 
 VL_DEFINE_DEBUG_FUNCTIONS;
 
-//######################################################################
-
-class OrderMoveVertex;
-
-// Information stored for each unique (domain, scope) pair. Mainly a list of ready vertices under
-// that (domain, scope). OrderMoveDomScope instances are themselves organized into a global ready
-// list if they have ready vertices.
-class OrderMoveDomScope final {
-    // STATE
-    V3List<OrderMoveVertex*> m_readyVertices;  // Ready vertices in this domain/scope
-    V3ListEnt<OrderMoveDomScope*> m_listEnt;  // List entry to store this instance
-    bool m_isOnList = false;  // True if DomScope is already on a list through m_listEnt
-    const AstSenTree* const m_domainp;  // Domain the vertices belong to
-    const AstScope* const m_scopep;  // Scope the vertices belong to
-
-    // Key type for map below
-    class DomScopeMapKey final {
-        const AstSenTree* const m_domainp;
-        const AstScope* const m_scopep;
-
-    public:
-        DomScopeMapKey(const AstSenTree* domainp, const AstScope* scopep)
-            : m_domainp{domainp}
-            , m_scopep{scopep} {}
-
-        struct Hash final {
-            size_t operator()(const DomScopeMapKey& key) const {
-                V3Hash hash{reinterpret_cast<uintptr_t>(key.m_domainp)};
-                hash += reinterpret_cast<uintptr_t>(key.m_scopep);
-                return hash.value();
-            }
-        };
-
-        struct Equal final {
-            bool operator()(const DomScopeMapKey& a, const DomScopeMapKey& b) const {
-                return a.m_domainp == b.m_domainp && a.m_scopep == b.m_scopep;
-            }
-        };
-    };
-
-    using DomScopeMap = std::unordered_map<DomScopeMapKey, OrderMoveDomScope, DomScopeMapKey::Hash,
-                                           DomScopeMapKey::Equal>;
-    // Map from Domain/Scope to the corresponding DomScope instance
-    static DomScopeMap s_dsMap;
-
-public:
-    // STATIC MEMBERS
-    static OrderMoveDomScope& getOrCreate(const AstSenTree* domainp, const AstScope* scopep) {
-        return s_dsMap
-            .emplace(std::piecewise_construct,  //
-                     std::forward_as_tuple(domainp, scopep),  //
-                     std::forward_as_tuple(domainp, scopep))
-            .first->second;
-    }
-
-    static void clear() { s_dsMap.clear(); }
-
-    // CONSTRUCTOR
-    OrderMoveDomScope(const AstSenTree* domainp, const AstScope* scopep)
-        : m_domainp{domainp}
-        , m_scopep{scopep} {}
-    ~OrderMoveDomScope() = default;
-    VL_UNCOPYABLE(OrderMoveDomScope);
-    VL_UNMOVABLE(OrderMoveDomScope);
-
-    // MEMBERS
-    V3List<OrderMoveVertex*>& readyVertices() { return m_readyVertices; }
-    const AstSenTree* domainp() const { return m_domainp; }
-    const AstScope* scopep() const { return m_scopep; }
-
-    bool isOnList() const { return m_isOnList; }
-    void unlinkFrom(V3List<OrderMoveDomScope*>& list) {
-        UASSERT_OBJ(m_isOnList, m_domainp, "unlinkFrom, but DomScope is not on a list");
-        m_isOnList = false;
-        m_listEnt.unlink(list, this);
-    }
-    void appendTo(V3List<OrderMoveDomScope*>& list) {
-        UASSERT_OBJ(!m_isOnList, m_domainp, "appendTo, but DomScope is already on a list");
-        m_isOnList = true;
-        m_listEnt.pushBack(list, this);
-    }
-    OrderMoveDomScope* nextp() const { return m_listEnt.nextp(); }
-};
-
-OrderMoveDomScope::DomScopeMap OrderMoveDomScope::s_dsMap;
-
-// ######################################################################
-//  OrderMoveVertex constructor
-
-class OrderMoveVertex final : public V3GraphVertex {
-    VL_RTTI_IMPL(OrderMoveVertex, V3GraphVertex)
-
-    // The corresponding logic vertex, or nullptr if this MoveVertex stands for a variable vertex.
-    OrderLogicVertex* const m_logicp;
-    OrderMoveDomScope& m_domScope;  // DomScope this vertex is under
-    V3ListEnt<OrderMoveVertex*> m_listEnt;  // List entry for ready list under DomScope
-
-    // METHODS
-    std::string dotColor() const override { return logicp() ? logicp()->dotColor() : ""; }
-
-    std::string name() const override VL_MT_STABLE {
-        if (!logicp()) {
-            return "var";
-        } else {
-            std::string nm = logicp()->name() + "\\n";
-            nm += "MV:";
-            nm += +" d=" + cvtToHex(logicp()->domainp());
-            nm += +" s=" + cvtToHex(logicp()->scopep());
-            return nm;
-        }
-    }
-
-public:
-    // CONSTRUCTORS
-    OrderMoveVertex(V3Graph& graph, OrderLogicVertex* lVtxp,
-                    const AstSenTree* domainp) VL_MT_DISABLED
-        : V3GraphVertex{&graph},
-          m_logicp{lVtxp},
-          m_domScope{OrderMoveDomScope::getOrCreate(domainp, lVtxp ? lVtxp->scopep() : nullptr)} {
-        UASSERT_OBJ(!lVtxp || lVtxp->domainp() == domainp, lVtxp, "Wrong domain for Move vertex");
-    }
-    ~OrderMoveVertex() override = default;
-
-    OrderLogicVertex* logicp() const VL_MT_STABLE { return m_logicp; }
-    OrderMoveDomScope& domScope() const { return m_domScope; }
-
-    void unlinkFrom(V3List<OrderMoveVertex*>& list) { m_listEnt.unlink(list, this); }
-    void appendTo(V3List<OrderMoveVertex*>& list) { m_listEnt.pushBack(list, this); }
-};
-
 //######################################################################
 // OrderSerial class
 
-class OrderSerial final {
-    // STATE
-    std::unique_ptr<V3Graph> m_moveGraphp;  // Graph of logic elements to move
-    V3List<OrderMoveDomScope*> m_readyDomScopeps;  // List of DomScopes which have ready vertices
-    V3OrderCFuncEmitter m_emitter;  // Code emitter to construct the result
-
-    // METHODS
-
-    // Take the given waiting logic vertex, and move it to the ready list its DomScope
-    void logicReady(OrderMoveVertex* lVtxp) {
-        UASSERT_OBJ(lVtxp->logicp(), lVtxp, "logicReady called on variable vertex");
-        UASSERT_OBJ(lVtxp->inEmpty(), lVtxp, "logicReady called on vertex with incoming edge");
-        // Add this logic vertex to the ready list of its DomScope
-        OrderMoveDomScope& domScope = lVtxp->domScope();
-        lVtxp->appendTo(domScope.readyVertices());
-        // Add the DomScope to the global ready list if not there yet
-        if (!domScope.isOnList()) domScope.appendTo(m_readyDomScopeps);
-    }
-
-    // Remove the given variable vertex, and check if any of its dependents are ready
-    void varReady(OrderMoveVertex* vVtxp) {
-        UASSERT_OBJ(!vVtxp->logicp(), vVtxp, "varReady called on logic vertex");
-        UASSERT_OBJ(vVtxp->inEmpty(), vVtxp, "varReady called on vertex with incoming edge");
-        // Remove dependency of consumer logic on this variable, and mark them ready if this is
-        // the last dependency.
-        for (V3GraphEdge *edgep = vVtxp->outBeginp(), *nextp; edgep; edgep = nextp) {
-            // Pick up next as we are deleting it
-            nextp = edgep->outNextp();
-            // The dependent logic
-            OrderMoveVertex* const lVtxp = edgep->top()->as<OrderMoveVertex>();
-            UASSERT_OBJ(lVtxp->logicp(), lVtxp, "The move graph should be bipartite");
-            // Delete this edge
-            VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
-            // If this was the last dependency, the consumer logic is ready
-            if (lVtxp->inEmpty()) logicReady(lVtxp);
-        }
-
-        // Can delete the vertex now
-        VL_DO_DANGLING(vVtxp->unlinkDelete(m_moveGraphp.get()), vVtxp);
-    }
-
-    void process(const OrderGraph& orderGraph, const std::string& tag,
-                 const V3Order::TrigToSenMap& trigToSen) {
-        // Build the move graph
-        m_moveGraphp = V3OrderMoveGraphBuilder<OrderMoveVertex>::apply(orderGraph, trigToSen);
-        if (dumpGraphLevel() >= 9) m_moveGraphp->dumpDotFilePrefixed(tag + "_ordermv_start");
-        m_moveGraphp->removeRedundantEdgesMax(&V3GraphEdge::followAlwaysTrue);
-        if (dumpGraphLevel() >= 4) m_moveGraphp->dumpDotFilePrefixed(tag + "_ordermv_simpl");
-
-        // Mark initially ready vertices (those with no dependencies)
-        for (V3GraphVertex* vtxp = m_moveGraphp->verticesBeginp(); vtxp;
-             vtxp = vtxp->verticesNextp()) {
-            if (!vtxp->inEmpty()) continue;
-            OrderMoveVertex* const mVtxp = vtxp->as<OrderMoveVertex>();
-            if (mVtxp->logicp()) {
-                logicReady(mVtxp);
-            } else {
-                varReady(mVtxp);
-            }
-        }
-
-        // Emit all logic as they become ready
-        for (OrderMoveDomScope *currDomScopep = m_readyDomScopeps.begin(), *nextDomScopep;
-             currDomScopep; currDomScopep = nextDomScopep) {
-            m_emitter.forceNewFunction();
-
-            // Emit all logic ready under the current DomScope
-            V3List<OrderMoveVertex*>& currReadyList = currDomScopep->readyVertices();
-            UASSERT(!currReadyList.empty(), "DomScope on ready list, not has no ready vertices");
-            while (OrderMoveVertex* const lVtxp = currReadyList.begin()) {
-                UASSERT_OBJ(&lVtxp->domScope() == currDomScopep, lVtxp, "DomScope mismatch");
-                // Unlink vertex from ready list under the DomScope
-                lVtxp->unlinkFrom(currReadyList);
-                // Unlink DomScope from the global ready list if this is the last vertex
-                // TODO: should do this later
-                if (currReadyList.empty()) currDomScopep->unlinkFrom(m_readyDomScopeps);
-
-                // Actually emit the logic under this vertex
-                m_emitter.emitLogic(lVtxp->logicp());
-
-                // Remove dependency of produced variables on this logic, and mark them ready if
-                // this is the last producer.
-                for (V3GraphEdge *edgep = lVtxp->outBeginp(), *nextp; edgep; edgep = nextp) {
-                    // Pick up next as we are deleting it
-                    nextp = edgep->outNextp();
-                    // The dependent variable
-                    OrderMoveVertex* const vVtxp = edgep->top()->as<OrderMoveVertex>();
-                    UASSERT_OBJ(!vVtxp->logicp(), vVtxp, "The move graph should be bipartite");
-                    // Delete this edge
-                    VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
-                    // If this was the last producer, the produced variable is ready
-                    if (vVtxp->inEmpty()) varReady(vVtxp);
-                }
-
-                // Can delete the vertex now
-                VL_DO_DANGLING(lVtxp->unlinkDelete(m_moveGraphp.get()), lVtxp);
-            }
-
-            // Done with this DomScope, pick a new one to emit. Prefer a new scope under the
-            // same domain. If there isn't one, just pick teh head of the global ready list
-            nextDomScopep = m_readyDomScopeps.begin();
-            for (OrderMoveDomScope* huntp = nextDomScopep; huntp; huntp = huntp->nextp()) {
-                if (huntp->domainp() == currDomScopep->domainp()) {
-                    nextDomScopep = huntp;
-                    break;
-                }
-            }
-        }
-
-        UASSERT(m_moveGraphp->empty(), "Waiting vertices remain, but none are ready");
-    }
-
-    // CONSTRUCTOR
-    OrderSerial(const OrderGraph& orderGraph, const std::string& tag,
-                const V3Order::TrigToSenMap& trigToSen, bool slow)
-        : m_emitter{tag, slow} {
-        OrderMoveDomScope::clear();
-        process(orderGraph, tag, trigToSen);
-        OrderMoveDomScope::clear();
-    }
-
-    ~OrderSerial() = default;
-
-public:
-    // Order the logic
-    static std::vector<AstActive*> apply(const OrderGraph& graph, const std::string& tag,
-                                         const V3Order::TrigToSenMap& trigToSen, bool slow) {
-        return OrderSerial{graph, tag, trigToSen, slow}.m_emitter.getAndClearActiveps();
-    }
-};
-
 std::vector<AstActive*> V3Order::createSerial(const OrderGraph& graph, const std::string& tag,
                                               const TrigToSenMap& trigToSen, bool slow) {
 
     UINFO(2, "  Constructing serial code for '" + tag + "'");
-    return OrderSerial::apply(graph, tag, trigToSen, slow);
+
+    // Build the move graph
+    OrderMoveDomScope::clear();
+    const std::unique_ptr<OrderMoveGraph> moveGraphp = OrderMoveGraph::build(graph, trigToSen);
+    if (dumpGraphLevel() >= 9) moveGraphp->dumpDotFilePrefixed(tag + "_ordermv");
+
+    // Serializer
+    OrderMoveGraphSerializer serializer{*moveGraphp};
+
+    // Add initially ready vertices (those with no dependencies) to the serializer as seeds
+    for (V3GraphVertex *vtxp = moveGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
+        nextp = vtxp->verticesNextp();
+        if (vtxp->inEmpty()) serializer.addSeed(vtxp->as<OrderMoveVertex>());
+    }
+
+    // Emit all logic as they become ready
+    V3OrderCFuncEmitter emitter{tag, slow};
+    OrderMoveDomScope* prevDomScopep = nullptr;
+    while (OrderMoveVertex* const mVtxp = serializer.getNext()) {
+        // We only really care about logic vertices
+        if (OrderLogicVertex* const logicp = mVtxp->logicp()) {
+            // Force a new function if the domain or scope changed, for better combining.
+            OrderMoveDomScope* const domScopep = &mVtxp->domScope();
+            if (domScopep != prevDomScopep) emitter.forceNewFunction();
+            prevDomScopep = domScopep;
+            // Emit the logic under this vertex
+            emitter.emitLogic(logicp);
+        }
+        // Can delete the vertex now
+        VL_DO_DANGLING(mVtxp->unlinkDelete(moveGraphp.get()), mVtxp);
+    }
+
+    // Delete the remaining variable vertices
+    for (V3GraphVertex *vtxp = moveGraphp->verticesBeginp(), *nextp; vtxp; vtxp = nextp) {
+        nextp = vtxp->verticesNextp();
+        if (!vtxp->as<OrderMoveVertex>()->logicp()) {
+            VL_DO_DANGLING(vtxp->unlinkDelete(moveGraphp.get()), vtxp);
+        }
+    }
+
+    UASSERT(moveGraphp->empty(), "Waiting vertices remain, but none are ready");
+    OrderMoveDomScope::clear();
+
+    return emitter.getAndClearActiveps();
 }