Improve MTask coarsening in multi-threaded scheduling (#8120)

This is a large refactor of the MTask graph and coarsening algorithm, in
prep for fixing the bug described in #7913, it can also improve the
resulting multi-threaded schedule.

Two major changes:

OrderMTaskGraph now maintains the critical paths of the MTasks through
mutation. There are 2 ways to mutate the graph, which are done via
methods on the graph itself: adding an edge (used during construction,
and will be used later during fixing data hazards), or merging an MTask
into another (used during contraction). All critical path measures are
automatically updated and propagated on any mutation, so no external
algorithm needs to maintain them explicitly.

The merge candidate scoreboard used during contraction is simplified to
remove deferral of updated scores. This simplifies the code and results
in a greedily more optimal schedule. (The previous tranched rescore was
an optimization to work around the previous std::set based scoreboard,
however since the algorithm now uses an efficient PairingHeap,
verilation time is not impacted by the more accurate scoring, while
yielding better results).

Combining these two into a single patch as the code is highly
interdependent and any one change without the other would be just a
noisy transit point with unclear performance implications. Together it
should be a clear improvement.

Also added a stronger validation step run with '--debug-partition',
which checks all invariants throughout the algorithms.

src/CMakeLists.txt

@@ -157,6 +157,7 @@ set(HEADERS
     V3ParseImp.h
     V3PchAstMT.h
     V3PchAstNoMT.h
+    V3PoolAllocator.h
     V3PreExpr.h
     V3PreLex.h
     V3PreProc.h

src/V3Graph.cpp

@@ -79,31 +79,6 @@ void V3GraphVertex::rerouteEdges(V3Graph* graphp) {
     unlinkEdges(graphp);
 }
 
-template <GraphWay::en N_Way>
-V3GraphEdge* V3GraphVertex::findConnectingEdgep(V3GraphVertex* waywardp) {
-    // O(edges) linear search. Searches search both nodes' edge lists in
-    // parallel.  The lists probably aren't _both_ huge, so this is
-    // unlikely to blow up even on fairly nasty graphs.
-    constexpr GraphWay way{N_Way};
-    constexpr GraphWay inv = way.invert();
-    auto& aEdges = this->edges<way>();
-    auto aIt = aEdges.begin();
-    auto aEnd = aEdges.end();
-    auto& bEdges = waywardp->edges<inv>();
-    auto bIt = bEdges.begin();
-    auto bEnd = bEdges.end();
-    while (aIt != aEnd && bIt != bEnd) {
-        V3GraphEdge& aedge = *aIt++;
-        if (aedge.furtherp<way>() == waywardp) return &aedge;
-        V3GraphEdge& bedge = *bIt++;
-        if (bedge.furtherp<inv>() == this) return &bedge;
-    }
-    return nullptr;
-}
-
-template V3GraphEdge* V3GraphVertex::findConnectingEdgep<GraphWay::FORWARD>(V3GraphVertex*);
-template V3GraphEdge* V3GraphVertex::findConnectingEdgep<GraphWay::REVERSE>(V3GraphVertex*);
-
 // cppcheck-has-bug-suppress constParameter
 void V3GraphVertex::v3errorEnd(const std::ostringstream& str) const  // LCOV_EXCL_START
     VL_RELEASE(V3Error::s().m_mutex) {

src/V3Graph.h

@@ -311,10 +311,6 @@ public:
         VL_RELEASE(V3Error::s().m_mutex) VL_MT_DISABLED;
     /// Edges are routed around this vertex to point from "from" directly to "to"
     void rerouteEdges(V3Graph* graphp) VL_MT_DISABLED;
-    // Find the edge connecting this vertex to the given vertex.
-    // If edge is not found returns nullptr. O(edges) performance.
-    template <GraphWay::en N_Way>
-    V3GraphEdge* findConnectingEdgep(V3GraphVertex* otherp) VL_MT_DISABLED;
 };
 
 std::ostream& operator<<(std::ostream& os, V3GraphVertex* vertexp) VL_MT_DISABLED;

src/V3OrderMTaskFixHazards.cpp

@@ -162,46 +162,6 @@ class FixDataHazards final {
 
     // METHODS
 
-    // Redirect all edges of 'donorp' onto 'recipientp'
-    static void redirectEdgesFrom(LogicMTask* recipientp, LogicMTask* donorp) {
-        // Process outgoing edges
-        while (MTaskEdge* const edgep = static_cast<MTaskEdge*>(donorp->outEdges().frontp())) {
-            LogicMTask* const top = edgep->toMTaskp();
-            top->removeRelativeEdge<GraphWay::REVERSE>(edgep);
-
-            // If an edge already exists between recipient and sink of donor, drop the duplicate.
-            if (recipientp->hasRelativeMTask(top)) {
-                VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
-                continue;
-            }
-
-            // Otherwise redirect the edge from donorp->top to recipientp->top.
-            edgep->relinkFromp(recipientp);
-            recipientp->addRelativeMTask(top);
-            recipientp->stealRelativeEdge<GraphWay::FORWARD>(edgep);
-            top->addRelativeEdge<GraphWay::REVERSE>(edgep);
-        }
-
-        // Process incoming edges
-        while (MTaskEdge* const edgep = static_cast<MTaskEdge*>(donorp->inEdges().frontp())) {
-            LogicMTask* const fromp = edgep->fromMTaskp();
-            fromp->removeRelativeMTask(donorp);
-            fromp->removeRelativeEdge<GraphWay::FORWARD>(edgep);
-
-            // If an edge already exists between recipient and source of donor, drop the duplicate.
-            if (fromp->hasRelativeMTask(recipientp)) {
-                VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
-                continue;
-            }
-
-            // Otherwise redirect the edge from fromp->donorp to fromp->recipientp.
-            edgep->relinkTop(recipientp);
-            fromp->addRelativeMTask(recipientp);
-            fromp->addRelativeEdge<GraphWay::FORWARD>(edgep);
-            recipientp->stealRelativeEdge<GraphWay::REVERSE>(edgep);
-        }
-    }
-
     void findAdjacentTasks(const OrderVarStdVertex* varVtxp, TasksByRank& tasksByRank) {
         // Find all writer tasks for this variable, group by rank.
         for (const V3GraphEdge& edge : varVtxp->inEdges()) {
@@ -219,8 +179,8 @@ class FixDataHazards final {
         LogicMTask* lastRecipientp = nullptr;
         for (const auto& pair : tasksByRank) {
             // Find the largest node at this rank, merge into it.  (If we
-            // happen to find a huge node, this saves time in
+            // happen to find a huge node, this saves time in the merge
-            // redirectEdgesFrom() versus merging into an arbitrary node.)
+            // versus merging into an arbitrary node.)
             LogicMTask* recipientp = nullptr;
             for (LogicMTask* const mtaskp : pair.second) {
                 if (!recipientp || (recipientp->cost() < mtaskp->cost())) recipientp = mtaskp;
@@ -231,20 +191,18 @@ class FixDataHazards final {
             for (LogicMTask* const donorp : pair.second) {
                 // Merge donor into recipient.
                 if (donorp == recipientp) continue;
-                // Fix up the map, so donor's OLVs map to recipientp
+                // Fix up the map, so donor's OLVs map to recipientp. Must do this while the donor
+                // still holds them.
                 for (const OrderMoveVertex& vtx : donorp->vertexList()) {
                     vtx.logicp()->userp(recipientp);
                 }
-                // Move all vertices from donorp to recipientp
+                // Merge donorp into recipientp, which also deletes donorp
-                recipientp->moveAllVerticesFrom(donorp);
+                m_mTaskGraph.mergeMTasks(recipientp, donorp);
-                // Redirect edges from donorp to recipientp
+                VL_DANGLING(donorp);
-                redirectEdgesFrom(recipientp, donorp);
-                // Remove donorp from the graph
-                VL_DO_DANGLING(donorp->unlinkDelete(&m_mTaskGraph), donorp);
             }
 
-            if (lastRecipientp && !lastRecipientp->hasRelativeMTask(recipientp)) {
+            if (lastRecipientp && !lastRecipientp->hasEdgeTo(recipientp)) {
-                new MTaskEdge{&m_mTaskGraph, lastRecipientp, recipientp, 1};
+                m_mTaskGraph.addEdge(lastRecipientp, recipientp);
             }
             lastRecipientp = recipientp;
         }
@@ -319,9 +277,8 @@ class FixDataHazards final {
         // given OVV.) Create edges across these remaining MTasks to ensure
         // they run in serial order (going along with the existing ranks.)
         //
-        // NOTE: we don't update the CP's stored in the LogicMTasks to
+        // NOTE: all graph mutations below go through OrderMTaskGraph (adding an edge, or merging
-        // reflect the changes we make to the graph. That's OK, as we
+        // two MTasks), so the CP's stored in the LogicMTasks are kept up to date throughout.
-        // haven't yet initialized CPs when we call this routine.
         for (const OrderVarStdVertex* const varVtxp : regularVars) {
             // Build a set of MTasks, per rank, which access this var.
             // Within a rank, sort by MTaskID to avoid nondeterminism.
@@ -387,4 +344,6 @@ public:
 
 void OrderMTaskGraph::fixDataHazards(OrderMTaskGraph& mtaskGraph) {
     FixDataHazards::apply(mtaskGraph);
+    // The critical paths are maintained as the graph is mutated, check them
+    mtaskGraph.validate();
 }

src/V3OrderMTaskGraph.cpp

@@ -21,28 +21,16 @@
 #include "V3Global.h"
 #include "V3InstrCount.h"
 
+#include <algorithm>
+#include <memory>
+#include <unordered_set>
+
 VL_DEFINE_DEBUG_FUNCTIONS;
 
-//######################################################################
-// OrderMTaskGraph
-
-OrderMTaskGraph::OrderMTaskGraph(OrderMoveGraph& moveGraph)
-    : m_moveGraph{moveGraph}
-    , m_entryp{new LogicMTask{*this, nullptr}}
-    , m_exitp{new LogicMTask{*this, nullptr}}
-    , m_forwardPropagator{v3Global.opt.debugPartition()}
-    , m_reversePropagator{v3Global.opt.debugPartition()} {}
-
-uint64_t OrderMTaskGraph::totalCost() const {
-    uint64_t cost = 0;
-    for (const V3GraphVertex& vtx : vertices()) cost += static_cast<const LogicMTask&>(vtx).cost();
-    return cost;
-}
-
 //######################################################################
 // LogicMTask
 
-uint32_t LogicMTask::s_nextId = 1;  // Start at 1, so that 0 indicates no mtask.
+uint32_t LogicMTask::s_nextId = 1;  // Start at 1, for historic reasons
 
 LogicMTask::LogicMTask(OrderMTaskGraph& graph, OrderMoveVertex* mVtxp)
     : V3GraphVertex{&graph} {
@@ -54,6 +42,281 @@ LogicMTask::LogicMTask(OrderMTaskGraph& graph, OrderMoveVertex* mVtxp)
     }
 }
 
+//######################################################################
+// OrderMTaskGraph
+
+OrderMTaskGraph::OrderMTaskGraph(OrderMoveGraph& moveGraph)
+    : m_moveGraph{moveGraph}
+    , m_entryp{new LogicMTask{*this, nullptr}}
+    , m_exitp{new LogicMTask{*this, nullptr}}
+    , m_slowAsserts{v3Global.opt.debugPartition()} {}
+
+bool OrderMTaskGraph::pathExistsImpl(LogicMTask* fromp, LogicMTask* top,
+                                     const MTaskEdge* excludedEdgep) {
+    UDEBUGONLY(UASSERT_OBJ(fromp->m_generation != m_currentGeneration, fromp,
+                           "Should not visit an MTask twice in the same search"););
+    // Mark visited.
+    fromp->m_generation = m_currentGeneration;
+
+    // Base case: we found a path.
+    if (fromp == top) return true;
+
+    // Base case: fromp is too late, cannot possibly be a prereq for top.
+    if (fromp->cpExclusive<GraphWay::REVERSE>() < top->cpInclusive<GraphWay::REVERSE>()) {
+        return false;
+    }
+    if (fromp->cpInclusive<GraphWay::FORWARD>() > top->cpExclusive<GraphWay::FORWARD>()) {
+        return false;
+    }
+
+    // Recursively look for a path
+    for (const V3GraphEdge& follow : fromp->outEdges()) {
+        if (&follow == excludedEdgep) continue;
+        LogicMTask* const nextp = static_cast<LogicMTask*>(follow.top());
+        // Don't visit the same MTask twice in the same search.
+        if (nextp->m_generation == m_currentGeneration) continue;
+        if (pathExistsImpl(nextp, top, nullptr)) return true;
+    }
+    return false;
+}
+
+template <GraphWay::en N_Way>
+void OrderMTaskGraph::propagatePush(LogicMTask* mtaskp) {
+    constexpr GraphWay way{N_Way};
+    constexpr GraphWay inv{way.invert()};
+    const uint64_t inclusiveCp = mtaskp->cpInclusive<way>();
+
+    for (V3GraphEdge& graphEdge : mtaskp->edges<way>()) {
+        MTaskEdge& edge = static_cast<MTaskEdge&>(graphEdge);
+
+        LogicMTask* const relativep = edge.furtherMTaskp<N_Way>();
+        EdgeHeap::Node& edgeHeapNode = edge.m_edgeHeapNode[inv];
+        if (inclusiveCp > edgeHeapNode.key().m_cp) {
+            relativep->m_edgeHeap[inv].increaseKey(&edgeHeapNode, inclusiveCp);
+        }
+
+        const uint64_t relativeCp = relativep->cpExclusive<way>();
+
+        if (relativeCp >= inclusiveCp) continue;
+
+        // relativep's critical path is out of step with its longest !wayward edge.
+        // Schedule that to be resolved.
+        const uint64_t increment = inclusiveCp - relativeCp;
+
+        PropagatePendingHeap::Node*& pendingNodepRef = relativep->m_propagateHeapNodep;
+        if (PropagatePendingHeap::Node* const nodep = pendingNodepRef) {
+            // Already in heap. Increase the increment if needed.
+            if (increment > nodep->key().m_increment) {
+                m_pendingHeap.increaseKey(nodep, increment);
+            }
+            continue;
+        }
+
+        // Add to heap
+        PropagatePendingHeap::Node* const nodep = m_pendingNodePool.alloc();
+        pendingNodepRef = nodep;
+        m_pendingHeap.insert(nodep, {increment, relativep->id(), relativep});
+    }
+}
+
+template <GraphWay::en N_Way>
+void OrderMTaskGraph::propagateResolve() {
+    constexpr GraphWay way{N_Way};
+    constexpr GraphWay inv{way.invert()};
+
+    // Each pending MTask is keyed on how much its critical path will grow by. Resolving them in
+    // decreasing order of that growth means each MTask needs resolving only once: the growth of a
+    // wayward MTask is never larger than the growth of the MTask it was pushed from, so once an
+    // MTask has been resolved, no larger growth can be pushed onto it later.
+    while (!m_pendingHeap.empty()) {
+        // Pop max element from heap
+        PropagatePendingHeap::Node* const maxp = m_pendingHeap.max();
+        m_pendingHeap.remove(maxp);
+        // Pick up values
+        LogicMTask* const mtaskp = maxp->key().m_mtaskp;
+        const uint64_t cpGrowBy = maxp->key().m_increment;
+        // Confirm that we only set each node's CP once. That's an important property of this
+        // algorithm, which allows it to be far faster than a recursive one.
+        UASSERT_OBJ(mtaskp->m_generation != m_currentGeneration, mtaskp, "Set CP on node twice");
+        mtaskp->m_generation = m_currentGeneration;
+        // Free the heap node, we are done with it
+        m_pendingNodePool.free(maxp);
+        mtaskp->m_propagateHeapNodep = nullptr;
+        // Update the critical path of mtaskp, that was out-of-date with respect to its edges
+        uint64_t& cpRef = mtaskp->m_cpExclusive[way];
+        const uint64_t newCp = cpRef + cpGrowBy;
+        // Check that CP matches that of the longest edge wayward of mtaskp.
+        if (VL_UNLIKELY(m_slowAsserts)) {
+            const uint64_t edgeCp = mtaskp->m_edgeHeap[inv].max()->key().m_cp;
+            UASSERT_OBJ(edgeCp == newCp, mtaskp, "CP doesn't match longest wayward edge");
+        }
+        cpRef = newCp;
+        propagatePush<N_Way>(mtaskp);
+    }
+}
+
+uint64_t OrderMTaskGraph::totalCost() const {
+    uint64_t cost = 0;
+    for (const V3GraphVertex& vtx : vertices()) cost += static_cast<const LogicMTask&>(vtx).cost();
+    return cost;
+}
+
+void OrderMTaskGraph::addEdge(LogicMTask* fromp, LogicMTask* top) {
+    UASSERT_OBJ(fromp != top, fromp, "Should not create self-edges");
+    UDEBUGONLY(UASSERT_OBJ(!fromp->hasEdgeTo(top), fromp, "Should not create redundant edges"););
+
+    // Create the edge. This inserts it into the edge heap of both endpoints with the correct
+    // critical path keys, as the critical paths of the endpoints are still unchanged here.
+    new MTaskEdge{this, fromp, top};
+
+    // The path through the new edge might be longer than the current critical path of its
+    // endpoints, in which case the critical paths need updating. Note each endpoint is the seed of
+    // one propagation, and is updated by the other: the inclusive critical paths of the endpoints
+    // themselves did not change (a new out-edge cannot lengthen a path into 'fromp', nor a new
+    // in-edge a path out of 'top'), so it is the new relative of each seed whose critical path
+    // might need to grow. That is, 'top' is updated wayward of 'fromp' below, and vice versa,
+    // together with the relatives of each, transitively.
+    //
+    // The guards below are an asymptotic optimization. The graph is consistent apart from the new
+    // edge, so the new relative is the only relative of either seed that can have a stale critical
+    // path, and if it does not need updating the propagation does nothing. It would however still
+    // walk all edges of the seed to discover that, which is expensive for a high degree seed.
+    if (fromp->cpInclusive<GraphWay::FORWARD>() > top->cpExclusive<GraphWay::FORWARD>()) {
+        propagate<GraphWay::FORWARD>(fromp);
+    }
+    if (top->cpInclusive<GraphWay::REVERSE>() > fromp->cpExclusive<GraphWay::REVERSE>()) {
+        propagate<GraphWay::REVERSE>(top);
+    }
+}
+
+void OrderMTaskGraph::mergeMTasks(LogicMTask* recipientp, LogicMTask* donorp) {
+    UASSERT_OBJ(recipientp != donorp, recipientp, "Should not merge an MTask with itself");
+
+    // Note we redirect the edges before updating the cost and critical paths of the recipient,
+    // which means the redirected edges are inserted into the edge heaps of the relatives using the
+    // pre-merge values of the recipient. The critical path propagation below then brings all of
+    // them up to date. This works because the keys in the edge heaps only ever need increasing:
+    // the inclusive critical path of the merged MTask is at least the inclusive critical path of
+    // either of the two MTasks it is made of, in both directions.
+
+    // Process outgoing edges of donor
+    while (MTaskEdge* const edgep = static_cast<MTaskEdge*>(donorp->outEdges().frontp())) {
+        LogicMTask* const relativep = edgep->toMTaskp();
+
+        relativep->removeRelativeEdge<GraphWay::REVERSE>(edgep);
+
+        if (relativep == recipientp || recipientp->hasEdgeTo(relativep)) {
+            // This is either the edge connecting the two MTasks, which becomes internal to the
+            // merged MTask, or is parallel with an existing edge of the recipient. Drop it.
+            VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
+        } else {
+            // No existing edge between recipient and relative of donor.
+            // Redirect the edge from donor -> relative to recipient -> relative.
+            edgep->relinkFromp(recipientp);
+            recipientp->addDependent(relativep);
+            recipientp->stealRelativeEdge<GraphWay::FORWARD>(edgep);
+            relativep->addRelativeEdge<GraphWay::REVERSE>(edgep);
+        }
+    }
+
+    // Process incoming edges of donor
+    while (MTaskEdge* const edgep = static_cast<MTaskEdge*>(donorp->inEdges().frontp())) {
+        LogicMTask* const relativep = edgep->fromMTaskp();
+
+        relativep->removeDependent(donorp);
+        relativep->removeRelativeEdge<GraphWay::FORWARD>(edgep);
+
+        if (relativep == recipientp || relativep->hasEdgeTo(recipientp)) {
+            // This is either the edge connecting the two MTasks, which becomes internal to the
+            // merged MTask, or is parallel with an existing edge of the recipient. Drop it.
+            VL_DO_DANGLING(edgep->unlinkDelete(), edgep);
+        } else {
+            // No existing edge between recipient and relative of donor.
+            // Redirect the edge from relative -> donor to relative -> recipient.
+            edgep->relinkTop(recipientp);
+            relativep->addDependent(recipientp);
+            relativep->addRelativeEdge<GraphWay::FORWARD>(edgep);
+            recipientp->stealRelativeEdge<GraphWay::REVERSE>(edgep);
+        }
+    }
+
+    // Move the contents of the donor into the recipient, update its cost
+    recipientp->m_mVertices.splice(recipientp->m_mVertices.end(), donorp->m_mVertices);
+    recipientp->m_cost += donorp->m_cost;
+
+    // The recipient now holds all edges of the merged MTask, and the critical paths of all its
+    // relatives are still up to date, so the critical paths implied by its edges are the critical
+    // paths of the merged MTask.
+    const uint64_t newCpFwd = recipientp->cpExclusiveFromEdges<GraphWay::FORWARD>();
+    const uint64_t newCpRev = recipientp->cpExclusiveFromEdges<GraphWay::REVERSE>();
+
+    // Set the new critical paths, then propagate the increases to the relatives. Note this also
+    // brings the keys of all edges of the merged MTask up to date in the relatives' edge heaps.
+    recipientp->cpExclusive<GraphWay::FORWARD>(newCpFwd);
+    propagate<GraphWay::FORWARD>(recipientp);
+    recipientp->cpExclusive<GraphWay::REVERSE>(newCpRev);
+    propagate<GraphWay::REVERSE>(recipientp);
+
+    // Remove the donor from the graph
+    VL_DO_DANGLING(donorp->unlinkDelete(this), donorp);
+}
+
+// Check the critical paths in the given direction, and the critical paths cached in the edge heaps
+// in the opposite direction, against those implied by the edges. Note this deliberately iterates
+// the edge lists, rather than consulting the edge heaps, so the heaps are validated, not trusted.
+template <GraphWay::en N_Way>
+void OrderMTaskGraph::validateWay() const {
+    constexpr GraphWay way{N_Way};
+    constexpr GraphWay inv = way.invert();
+    for (const V3GraphVertex& vtx : vertices()) {
+        const LogicMTask& mtask = *vtx.as<LogicMTask>();
+        uint64_t cpCost = 0;
+        std::unordered_set<const V3GraphVertex*> relatives;
+        for (const V3GraphEdge& graphEdge : mtask.edges<inv>()) {
+            const MTaskEdge& edge = *graphEdge.as<MTaskEdge>();
+            const LogicMTask& relative = *(edge.furtherp<inv>()->template as<LogicMTask>());
+            // Run a few asserts on the graph, while we are iterating through...
+            UASSERT_OBJ(edge.weight() != 0, &mtask, "Should be no cut edges in MTask graph");
+            UASSERT_OBJ(&relative != &mtask, &mtask, "Should be no self edges in MTask graph");
+            const bool first = relatives.insert(&relative).second;
+            UASSERT_OBJ(first, &mtask, "Should be no redundant edges in MTask graph");
+            const uint64_t inclusiveCp = relative.cpInclusive<way>();
+            // The critical path cached in the edge heap must match that of the relative
+            UASSERT_OBJ(edge.cachedCp(inv) == inclusiveCp, &mtask,
+                        "Cached critical path does not match the relative");
+            // As must the ID it is keyed on, which breaks ties between equal critical paths
+            UASSERT_OBJ(edge.cachedId(inv) == relative.id(), &mtask,
+                        "Cached ID does not match the relative");
+            cpCost = std::max(cpCost, inclusiveCp);
+        }
+        const uint64_t cp = mtask.cpExclusive<way>();
+        UASSERT_OBJ(cp == cpCost, &mtask, "Critical path does not match the edges");
+        // The edge heap must yield the same, that is: it must return the largest of its keys
+        UASSERT_OBJ(mtask.cpExclusiveFromEdges<N_Way>() == cpCost, &mtask,
+                    "Edge heap maximum does not match the edges");
+    }
+}
+
+void OrderMTaskGraph::validate() const {
+    if (!m_slowAsserts) return;
+
+    validateWay<GraphWay::FORWARD>();
+    validateWay<GraphWay::REVERSE>();
+
+    // Check the dependents set of each MTask agrees with its out-edges
+    for (const V3GraphVertex& vtx : vertices()) {
+        const LogicMTask& mtask = *vtx.as<LogicMTask>();
+        size_t nDependents = 0;
+        for (const V3GraphEdge& graphEdge : mtask.outEdges()) {
+            LogicMTask* const top = graphEdge.as<MTaskEdge>()->toMTaskp();
+            UASSERT_OBJ(mtask.hasEdgeTo(top), &mtask, "Dependent missing from the dependents set");
+            ++nDependents;
+        }
+        UASSERT_OBJ(mtask.m_dependents.size() == nDependents, &mtask,
+                    "Stale entry in the dependents set");
+    }
+}
+
 //######################################################################
 // OrderMTaskGraphBuilder
 
@@ -103,10 +366,9 @@ class OrderMTaskGraphBuilder final {
     }
 
     // Add an edge to the graph, if there is not already an edge between the two vertices.
-    void addEdge(LogicMTask& src, LogicMTask& dst) {
+    void addEdge(LogicMTask* srcp, LogicMTask* dstp) {
-        UASSERT_OBJ(&src != &dst, &src, "Should not create self-edges");
+        if (srcp->hasEdgeTo(dstp)) return;  // Don't create redundant edges.
-        if (src.hasRelativeMTask(&dst)) return;  // Don't create redundant edges.
+        m_mtaskGraph.addEdge(srcp, dstp);
-        new MTaskEdge{&m_mtaskGraph, &src, &dst, 1};
     }
 
     // CONSTRUCTORS
@@ -145,7 +407,7 @@ class OrderMTaskGraphBuilder final {
 
                 // If the opposite end of the edge is not a bypassed vertex, add direct dependency
                 if (LogicMTask* const otherp = static_cast<LogicMTask*>(top->userp())) {
-                    addEdge(mtask, *otherp);
+                    addEdge(&mtask, otherp);
                     continue;
                 }
 
@@ -155,7 +417,7 @@ class OrderMTaskGraphBuilder final {
                     // The Move graph is bipartite (logic <-> var), and logic is never
                     // bypassed, hence 'transp' must be non-nullptr.
                     UASSERT_OBJ(transp, mVtxp, "This cannot be a bypassed vertex");
-                    addEdge(mtask, *transp);
+                    addEdge(&mtask, transp);
                 }
             }
         }
@@ -166,8 +428,8 @@ class OrderMTaskGraphBuilder final {
             LogicMTask& mtask = static_cast<LogicMTask&>(vtx);
             if (VL_UNLIKELY((&mtask == &entry) || (&mtask == &exit))) continue;
             // Add the entry/exit edges if not otherwise connected
-            if (mtask.inEmpty()) addEdge(entry, mtask);
+            if (mtask.inEmpty()) addEdge(&entry, &mtask);
-            if (mtask.outEmpty()) addEdge(mtask, exit);
+            if (mtask.outEmpty()) addEdge(&mtask, &exit);
         }
     }
     ~OrderMTaskGraphBuilder() = default;
@@ -181,5 +443,6 @@ public:
 std::unique_ptr<OrderMTaskGraph> OrderMTaskGraph::build(OrderMoveGraph& moveGraph) {
     std::unique_ptr<OrderMTaskGraph> resp{new OrderMTaskGraph{moveGraph}};
     OrderMTaskGraphBuilder::apply(*resp);
+    resp->validate();
     return resp;
 }

src/V3OrderMTaskGraph.h

@@ -20,9 +20,9 @@
 //  candidate machinery: any auxiliary data the algorithms need is attached
 //  externally via the vertex/edge user pointers.
 //
-//  PropagateCp propagates increasing critical path costs through the graph.
+//  OrderMTaskGraph maintains the critical paths of the MTasks, and the ones
-//  OrderMTaskGraph owns one instance for each direction, which the algorithms
+//  cached in the edge heaps, as the graph is mutated via 'addEdge' and
-//  operating on the graph use to keep the critical paths up to date.
+//  'mergeMTasks'.
 //
 //*************************************************************************
 
@@ -35,38 +35,56 @@
 #include "V3Graph.h"
 #include "V3OrderMoveGraph.h"
 #include "V3PairingHeap.h"
+#include "V3PoolAllocator.h"
 
 #include <array>
-#include <cmath>
 #include <memory>
 #include <sstream>
 #include <unordered_set>
-#include <vector>
 
 class LogicMTask;
 class OrderMTaskGraph;
-template <GraphWay::en N_Way>
-class PropagateCp;
 
 //=============================================================================
-// We keep MTaskEdge graph edges in a PairingHeap, sorted by score and id
+// MTaskEdge graph edges are stored in a PairingHeap in each LogicMTask they
+// connect to, sorted by critical path through that edge (and id for stability).
 
 struct EdgeKey final {
-    uint64_t m_score;  // Score part of edge key
+    uint64_t m_cp;  // The inclusive critical path of the further MTask of the edge
-    uint64_t m_id;  // Unique ID part of edge key
+    uint32_t m_id;  // The ID of the further MTask, for stable comparison
-    void increase(uint64_t score) {
+    void increase(uint64_t cp) {
-        UDEBUGONLY(UASSERT(score >= m_score, "Must increase"););
+        UDEBUGONLY(UASSERT(cp >= m_cp, "Must increase"););
-        m_score = score;
+        m_cp = cp;
     }
-    // Sort first by Score then by ID
+    // Sort first by critical path, then by ID
     bool operator<(const EdgeKey& other) const {
-        if (m_score != other.m_score) return m_score < other.m_score;
+        if (m_cp != other.m_cp) return m_cp < other.m_cp;
         return m_id < other.m_id;
     }
 };
 
 using EdgeHeap = PairingHeap<EdgeKey>;
 
+//=============================================================================
+// LogicMTasks are stored in a PairingHeap during critical path update propagation.
+
+struct PropagatePendingKey final {
+    uint64_t m_increment;  // The amount the critical path of the MTask will grow by
+    uint32_t m_id;  // The ID of the MTask, for stable comparison
+    LogicMTask* m_mtaskp;  // The MTask the heap entry corresponds to
+    void increase(uint64_t increment) {
+        UDEBUGONLY(UASSERT(increment >= m_increment, "Must increase"););
+        m_increment = increment;
+    }
+    // Sort first by increment, then by ID
+    bool operator<(const PropagatePendingKey& other) const {
+        if (m_increment != other.m_increment) return m_increment < other.m_increment;
+        return m_id < other.m_id;
+    }
+};
+
+using PropagatePendingHeap = PairingHeap<PropagatePendingKey>;
+
 //=============================================================================
 // GraphEdge for the MTask graph
 
@@ -74,22 +92,19 @@ class MTaskEdge final : public V3GraphEdge {
     VL_RTTI_IMPL(MTaskEdge, V3GraphEdge)
 
     friend class LogicMTask;
-    template <GraphWay::en N_Way>
+    friend class OrderMTaskGraph;
-    friend class PropagateCp;
 
     // MEMBERS
     // This edge can be in 2 EdgeHeaps, one forward and one reverse. We allocate the heap nodes
     // directly within the edge as they are always required and this makes association cheap.
     std::array<EdgeHeap::Node, GraphWay::NUM_WAYS> m_edgeHeapNode;
 
-    // Note: The edge's contraction merge candidate (if any) is held in the inherited user pointer
+    // CONSTRUCTORS
-    // (V3GraphEdge::userp), managed entirely by the partitioner; see edgeMC() and
+    // Private, so edges can only be created via OrderMTaskGraph, which also updates the critical
-    // MergeCandidateScoreboard. Kept out of MTaskEdge so it does not depend on the MergeCandidate
+    // paths on graph mutation.
-    // hierarchy.
+    inline MTaskEdge(OrderMTaskGraph* graphp, LogicMTask* fromp, LogicMTask* top);
 
 public:
-    // CONSTRUCTORS
-    inline MTaskEdge(OrderMTaskGraph* graphp, LogicMTask* fromp, LogicMTask* top, int weight);
     VL_UNCOPYABLE(MTaskEdge);
     VL_UNMOVABLE(MTaskEdge);
 
@@ -99,11 +114,8 @@ public:
     inline LogicMTask* fromMTaskp() const;
     inline LogicMTask* toMTaskp() const;
 
-    // Following initial assignment of critical paths, clear this MTaskEdge
+    uint64_t cachedCp(GraphWay way) const { return m_edgeHeapNode[way].key().m_cp; }
-    // out of the edge-map for each node and reinsert at a new location
+    uint32_t cachedId(GraphWay way) const { return m_edgeHeapNode[way].key().m_id; }
-    // with updated critical path.
-    inline void resetCriticalPaths();
-    uint64_t cachedCp(GraphWay way) const { return m_edgeHeapNode[way].key().m_score; }
     // Convert from the address of the m_edgeHeapNode[way] in an MTaskEdge back to the MTaskEdge
     static const MTaskEdge* toMTaskEdge(GraphWay way, const EdgeHeap::Node* nodep) {
         const size_t offset = VL_OFFSETOF(MTaskEdge, m_edgeHeapNode[way]);
@@ -117,8 +129,8 @@ public:
 class LogicMTask final : public V3GraphVertex {
     VL_RTTI_IMPL(LogicMTask, V3GraphVertex)
 
-    template <GraphWay::en N_Way>
+    friend class MTaskEdge;
-    friend class PropagateCp;
+    friend class OrderMTaskGraph;
 
     // MEMBERS
 
@@ -126,18 +138,23 @@ class LogicMTask final : public V3GraphVertex {
     // OrderMoveVertex objects, we merely keep them in a list here.
     OrderMoveVertex::List m_mVertices;
 
+    static uint32_t s_nextId;  // Next ID number to use
+    const uint32_t m_id = s_nextId++;  // Unique LogicMTask ID number for stable comparison
+
     // Cost estimate for this LogicMTask, derived from V3InstrCount, in abstract time units.
     // Cost estimates and critical path lengths are bounded by number of AstNodes * constant,
     // will run out of host memory storing the Ast way before they can overflow.
     uint64_t m_cost = 0;
 
-    // Cost of critical paths going FORWARD from graph-start to the start
+    // Critical path in each direction: going FORWARD from graph-start to the start of this vertex,
-    // of this vertex, and also going REVERSE from the end of the graph to
+    // and going REVERSE from graph-exit to the end of this vertex. Exclusive of the cost of this
-    // the end of the vertex. Same units as m_cost.
+    // vertex itself, see cpInclusive() for the value including it.
-    std::array<uint64_t, GraphWay::NUM_WAYS> m_critPathCost = {};
+    std::array<uint64_t, GraphWay::NUM_WAYS> m_cpExclusive = {0, 0};
 
-    static uint32_t s_nextId;  // Next ID number to use
+    // The MTasks this MTask has an out-edge to, so checking for an existing edge is O(1)
-    const uint32_t m_id = s_nextId++;  // Unique LogicMTask ID number for stable comparison
+    std::unordered_set<LogicMTask*> m_dependents;
+    // Store the out/in edges in a heaps sorted by the critical path length through each edge
+    std::array<EdgeHeap, GraphWay::NUM_WAYS> m_edgeHeap;
 
     // Count "generations" which are just operations that scan through the
     // graph. We'll mark each node with the last generation that scanned
@@ -145,15 +162,9 @@ class LogicMTask final : public V3GraphVertex {
     // while searching for a path.
     uint64_t m_generation = 0;
 
-    // Store a set of forward relatives so we can quickly check if we have a given child
+    // Scratch pointer used only by the critical path propagation in OrderMTaskGraph: this MTask's
-    std::unordered_set<LogicMTask*> m_edgeSet;
+    // node in the pending heap, or nullptr if this MTask is not pending.
-    // Store the outgoing and incoming edges in a heap sorted by the critical path length
+    PropagatePendingHeap::Node* m_propagateHeapNodep = nullptr;
-    std::array<EdgeHeap, GraphWay::NUM_WAYS> m_edgeHeap;
-
-    // Scratch pointer used only by PropagateCp: this MTask's node in the pending heap, or nullptr
-    // if this MTask is not pending. Type erased, as the heap node type is private to PropagateCp,
-    // and differs between its two instantiations (which never run concurrently).
-    void* m_propagateHeapNodep = nullptr;
 
 public:
     // CONSTRUCTORS
@@ -161,20 +172,65 @@ public:
     VL_UNCOPYABLE(LogicMTask);
     VL_UNMOVABLE(LogicMTask);
 
-    // ACCESSORS
-    OrderMoveVertex::List& vertexList() { return m_mVertices; }
-    const OrderMoveVertex::List& vertexList() const { return m_mVertices; }
-    uint32_t id() const { return m_id; }
-    uint64_t cost() const VL_MT_SAFE { return m_cost; }
-    uint64_t critPathCost(GraphWay way) const { return m_critPathCost[way]; }
-    void setCritPathCost(GraphWay way, uint64_t cost) { m_critPathCost[way] = cost; }
-
     // METHODS
+    OrderMoveVertex::List& vertexList() { return m_mVertices; }
+    uint32_t id() const { return m_id; }
     bool operator<(const LogicMTask& rhs) const { return id() < rhs.id(); }
 
-    void moveAllVerticesFrom(LogicMTask* otherp) {
+    uint64_t cost() const VL_MT_SAFE { return m_cost; }
-        m_mVertices.splice(m_mVertices.end(), otherp->vertexList());
+    template <GraphWay::en N_Way>
-        m_cost += otherp->m_cost;
+    uint64_t cpExclusive() const {
+        return m_cpExclusive[N_Way];
+    }
+    template <GraphWay::en N_Way>
+    uint64_t cpInclusive() const {
+        return m_cpExclusive[N_Way] + m_cost;
+    }
+    // The critical path of this MTask without considering the given edge.
+    template <GraphWay::en N_Way>
+    uint64_t cpExclusiveWithout(const V3GraphEdge* edgep) const {
+        const GraphWay way{N_Way};
+        const GraphWay inv = way.invert();
+        UDEBUGONLY(UASSERT(edgep->furtherp<N_Way>() == this,
+                           "In cpExclusiveWithout(), 'edgep' must further to 'this'"););
+        // At most two edges need to be considered: the critical path, if that is not via 'edgep',
+        // or the second-worst path, if the critical path is via 'edgep'.
+        const EdgeHeap& edgeHeap = m_edgeHeap[inv];
+        // Pick up the critical path edge
+        const EdgeHeap::Node* const maxp = edgeHeap.max();
+        UDEBUGONLY(UASSERT(maxp, "Edge not in heap"););
+        // If 'edgep' is not the critical path edge, return its critical path
+        if (MTaskEdge::toMTaskEdge(inv, maxp) != edgep) return maxp->key().m_cp;
+        // Otherwise return the second-worst path, if there is one
+        const EdgeHeap::Node* const secp = edgeHeap.secondMax();
+        if (!secp) return 0;
+        return secp->key().m_cp;
+    }
+
+    bool hasEdgeTo(LogicMTask* dependentp) const { return m_dependents.count(dependentp); }
+
+    // For Graphviz dumps only
+    std::string name() const override VL_MT_STABLE {
+        std::ostringstream out;
+        out << "mt" << m_id  //
+            << " | cpFwd " << m_cpExclusive[GraphWay::FORWARD]  //
+            << " | cost " << cost()  //
+            << " | cpRev " << m_cpExclusive[GraphWay::REVERSE];
+        return out.str();
+    }
+
+private:
+    // Following only used by OrderMTaskGraph, which maintains cached CPs and graph invariants.
+
+    template <GraphWay::en N_Way>
+    uint64_t cpExclusiveFromEdges() const {
+        constexpr GraphWay inv = GraphWay{N_Way}.invert();
+        const EdgeHeap::Node* const maxp = m_edgeHeap[inv].max();
+        return maxp ? maxp->key().m_cp : 0;
+    }
+    template <GraphWay::en N_Way>
+    void cpExclusive(uint64_t cp) {
+        m_cpExclusive[N_Way] = cp;
     }
 
     template <GraphWay::en N_Way>
@@ -183,8 +239,8 @@ public:
         constexpr GraphWay inv = way.invert();
         // Add to the edge heap
         LogicMTask* const relativep = edgep->furtherMTaskp<N_Way>();
-        // Value is !way cp to this edge
+        // Value is the !way inclusive cp of the relative
-        const uint64_t cp = relativep->cost() + relativep->critPathCost(inv);
+        const uint64_t cp = relativep->cpInclusive<inv>();
         m_edgeHeap[way].insert(&edgep->m_edgeHeapNode[way], {cp, relativep->id()});
     }
     template <GraphWay::en N_Way>
@@ -202,283 +258,14 @@ public:
         m_edgeHeap[way].remove(&edgep->m_edgeHeapNode[way]);
     }
 
-    void addRelativeMTask(LogicMTask* relativep) {
+    void addDependent(LogicMTask* dependentp) {
-        // Add the relative to connecting edge map
+        const bool exists = !m_dependents.emplace(dependentp).second;
-        const bool exits = !m_edgeSet.emplace(relativep).second;
+        UDEBUGONLY(UASSERT(!exists, "Adding existing dependent"););
-        UDEBUGONLY(UASSERT(!exits, "Adding existing relative"););
     }
-    void removeRelativeMTask(LogicMTask* relativep) {
+    void removeDependent(LogicMTask* dependentp) {
-        const size_t removed = m_edgeSet.erase(relativep);
+        const size_t removed = m_dependents.erase(dependentp);
-        UDEBUGONLY(UASSERT(removed, "Relative should have been in set"););
+        UDEBUGONLY(UASSERT(removed, "Dependent should have been in set"););
     }
-    bool hasRelativeMTask(LogicMTask* relativep) const { return m_edgeSet.count(relativep); }
-
-    template <GraphWay::en N_Way>
-    void checkRelativesCp() const {
-        constexpr GraphWay way{N_Way};
-        for (const V3GraphEdge& edge : edges<N_Way>()) {
-            const LogicMTask* const relativep
-                = static_cast<const LogicMTask*>(edge.furtherp<N_Way>());
-            const uint64_t cachedCp = static_cast<const MTaskEdge&>(edge).cachedCp(way);
-            const uint64_t cp = relativep->critPathCost(way.invert()) + relativep->cost();
-            UASSERT(cachedCp == cp, "Calculation error in scoring");
-        }
-    }
-
-    template <GraphWay::en N_Way>
-    uint64_t critPathCostWithout(const V3GraphEdge* withoutp) const {
-        const GraphWay way{N_Way};
-        const GraphWay inv = way.invert();
-        // Compute the critical path cost wayward to this node, without considering edge
-        // 'withoutp'. We need to look at two edges at most, the critical path if that is not via
-        // 'withoutp', or the second-worst path, if the critical path is via 'withoutp'.
-        UDEBUGONLY(UASSERT(withoutp->furtherp<N_Way>() == this,
-                           "In critPathCostWithout(), edge 'withoutp' must further to 'this'"););
-        const EdgeHeap& edgeHeap = m_edgeHeap[inv];
-        const EdgeHeap::Node* const maxp = edgeHeap.max();
-        if (!maxp) return 0;
-        if (MTaskEdge::toMTaskEdge(inv, maxp) != withoutp) return maxp->key().m_score;
-        const EdgeHeap::Node* const secp = edgeHeap.secondMax();
-        if (!secp) return 0;
-        return secp->key().m_score;
-    }
-
-private:
-    // This takes LogicMTask instead of generic V3GraphVertex. We will use the critical
-    // paths known to LogicMTask to prune the recursion for speed. Also store 'generation' in
-    // LogicMTask::m_generation so we can prune the search and avoid recursing through the same
-    // node more than once in a single search.
-    static bool pathExistsFromInternal(LogicMTask* fromp, LogicMTask* top,
-                                       const MTaskEdge* excludedEdgep, uint64_t generation) {
-
-        // If already looked at this node in the current search, since we're back again,
-        // we must not have found a path on the first go.
-        if (fromp->m_generation == generation) return false;
-
-        // Mark visited
-        fromp->m_generation = generation;
-
-        // Base case: we found a path.
-        if (fromp == top) return true;
-
-        // Base case: fromp is too late, cannot possibly be a prereq for top.
-        if (fromp->critPathCost(GraphWay::REVERSE)
-            < (top->critPathCost(GraphWay::REVERSE) + top->cost())) {
-            return false;
-        }
-        if ((fromp->critPathCost(GraphWay::FORWARD) + fromp->cost())
-            > top->critPathCost(GraphWay::FORWARD)) {
-            return false;
-        }
-
-        // Recursively look for a path
-        for (const V3GraphEdge& follow : fromp->outEdges()) {
-            if (&follow == excludedEdgep) continue;
-            LogicMTask* const nextp = static_cast<LogicMTask*>(follow.top());
-            if (pathExistsFromInternal(nextp, top, nullptr, generation)) return true;
-        }
-        return false;
-    }
-
-public:
-    // True if there's a path from 'fromp' to 'top' excluding 'excludedEdgep', false otherwise.
-    // 'excludedEdgep' may be nullptr in which case no edge is excluded. If 'excludedEdgep' is
-    // non-nullptr it must connect fromp and top.
-    static bool pathExistsFrom(LogicMTask* fromp, LogicMTask* top,
-                               const MTaskEdge* excludedEdgep) {
-        static uint64_t s_generation = 0;
-        return pathExistsFromInternal(fromp, top, excludedEdgep, ++s_generation);
-    }
-
-    // For Graphviz dumps only
-    std::string name() const override VL_MT_STABLE {
-        std::ostringstream out;
-        out << "mt" << m_id  //
-            << " | fwdCP " << m_critPathCost[GraphWay::FORWARD]  //
-            << " | revCP " << m_critPathCost[GraphWay::REVERSE]  //
-            << " | cost " << cost();
-        return out.str();
-    }
-};
-
-//=============================================================================
-// PropagateCp
-
-template <GraphWay::en N_Way>
-class PropagateCp final {
-    // Propagate increasing critical path (CP) costs through a graph.
-    //
-    // Usage:
-    //  * Client increases the cost and/or CP at a node or small set of nodes
-    //    (often a pair in practice, eg. edge contraction.)
-    //  * Client calls PropagateCp::cpHasIncreased() one or more times.
-    //    Each call indicates that the inclusive CP of some "seed" vertex
-    //    has increased to a given value.
-    //    * NOTE: PropagateCp will neither read nor modify the cost
-    //      or CPs at the seed vertices, it only accesses and modifies
-    //      vertices wayward from the seeds.
-    //  * Client calls PropagateCp::go(). Internally, this iteratively
-    //    propagates the new CPs wayward through the graph.
-    //
-
-    // TYPES
-
-    // We keep pending vertices in a heap during critical path propagation
-    struct PendingKey final {
-        LogicMTask* m_mtaskp;  // The vertex in the heap
-        uint64_t m_score;  // The score of this entry
-        void increase(uint64_t score) {
-            UDEBUGONLY(UASSERT(score >= m_score, "Must increase"););
-            m_score = score;
-        }
-        bool operator<(const PendingKey& other) const {
-            if (m_score != other.m_score) return m_score < other.m_score;
-            return *m_mtaskp < *other.m_mtaskp;
-        }
-    };
-
-    using PendingHeap = PairingHeap<PendingKey>;
-    using PendingHeapNode = typename PendingHeap::Node;
-
-    // MEMBERS
-    PendingHeap m_pendingHeap;  // Heap of pending rescores
-
-    // We allocate this many heap nodes at once
-    static constexpr size_t ALLOC_CHUNK_SIZE = 128;
-    PendingHeapNode* m_freep = nullptr;  // List of free heap nodes
-    std::vector<std::unique_ptr<PendingHeapNode[]>> m_allocated;  // Allocated heap nodes
-
-    const bool m_slowAsserts;  // Enable nontrivial asserts
-    // Used only with slow asserts to check MTasks visited only once
-    std::unordered_set<LogicMTask*> m_seen;
-
-public:
-    // CONSTRUCTORS
-    explicit PropagateCp(bool slowAsserts)
-        : m_slowAsserts{slowAsserts} {}
-
-    // METHODS
-private:
-    // Allocate a HeapNode for the given element
-    PendingHeapNode* allocNode() {
-        // If no free nodes available, then make some
-        if (!m_freep) {
-            // Allocate in chunks for efficiency
-            m_allocated.emplace_back(new PendingHeapNode[ALLOC_CHUNK_SIZE]);
-            // Set up free list pointer
-            m_freep = m_allocated.back().get();
-            // Set up free list chain
-            for (size_t i = 1; i < ALLOC_CHUNK_SIZE; ++i) {
-                m_freep[i - 1].m_next.m_ptr = &m_freep[i];
-            }
-            // Clear the next pointer of the last entry
-            m_freep[ALLOC_CHUNK_SIZE - 1].m_next.m_ptr = nullptr;
-        }
-        // Free nodes are available, pick up the first one
-        PendingHeapNode* const resultp = m_freep;
-        m_freep = resultp->m_next.m_ptr;
-        resultp->m_next.m_ptr = nullptr;
-        return resultp;
-    }
-
-    // Release a heap node (make it available for future allocation)
-    void freeNode(PendingHeapNode* nodep) {
-        // Re-use the existing link pointers and simply prepend it to the free list
-        nodep->m_next.m_ptr = m_freep;
-        m_freep = nodep;
-    }
-
-public:
-    void cpHasIncreased(LogicMTask* vxp, uint64_t newInclusiveCp) {
-        constexpr GraphWay way{N_Way};
-        constexpr GraphWay inv{way.invert()};
-
-        // For *vxp, whose CP-inclusive has just increased to
-        // newInclusiveCp, iterate to all wayward nodes, update the edges
-        // of each, and add each to m_pending if its overall CP has grown.
-        for (V3GraphEdge& graphEdge : vxp->edges<way>()) {
-            MTaskEdge& edge = static_cast<MTaskEdge&>(graphEdge);
-
-            LogicMTask* const relativep = edge.furtherMTaskp<N_Way>();
-            EdgeHeap::Node& edgeHeapNode = edge.m_edgeHeapNode[inv];
-            if (newInclusiveCp > edgeHeapNode.key().m_score) {
-                relativep->m_edgeHeap[inv].increaseKey(&edgeHeapNode, newInclusiveCp);
-            }
-
-            const uint64_t critPathCost = relativep->critPathCost(way);
-
-            if (critPathCost >= newInclusiveCp) continue;
-
-            // relativep's critPathCost() is out of step with its longest !wayward edge.
-            // Schedule that to be resolved.
-            const uint64_t newVal = newInclusiveCp - critPathCost;
-
-            void*& pendingNodepRef = relativep->m_propagateHeapNodep;
-            if (PendingHeapNode* const nodep = static_cast<PendingHeapNode*>(pendingNodepRef)) {
-                // Already in heap. Increase score if needed.
-                if (newVal > nodep->key().m_score) m_pendingHeap.increaseKey(nodep, newVal);
-                continue;
-            }
-
-            // Add to heap
-            PendingHeapNode* const nodep = allocNode();
-            pendingNodepRef = nodep;
-            m_pendingHeap.insert(nodep, {relativep, newVal});
-        }
-    }
-
-    void go() {
-        constexpr GraphWay way{N_Way};
-        constexpr GraphWay inv{way.invert()};
-
-        // m_pending maps each pending vertex to the amount that it wayward
-        // CP will grow.
-        //
-        // We can iterate over the pending set in reverse order, always
-        // choosing the nodes with the largest pending CP-growth.
-        //
-        // The intuition is: if the original seed node had its CP grow by
-        // 50, the most any wayward node can possibly grow is also 50.  So
-        // for anything pending to grow by 50, we know we can process it
-        // once and we won't have to grow its CP again on the current pass.
-        // After we're done with all the grow-by-50s, nothing else will
-        // grow by 50 again on the current pass, and we can process the
-        // grow-by-49s and we know we'll only have to process each one
-        // once.  And so on.
-        //
-        // This generalizes to multiple seed nodes also.
-        while (!m_pendingHeap.empty()) {
-            // Pop max element from heap
-            PendingHeapNode* const maxp = m_pendingHeap.max();
-            m_pendingHeap.remove(maxp);
-            // Pick up values
-            LogicMTask* const mtaskp = maxp->key().m_mtaskp;
-            const uint64_t cpGrowBy = maxp->key().m_score;
-            // Free the heap node, we are done with it
-            freeNode(maxp);
-            mtaskp->m_propagateHeapNodep = nullptr;
-            // Update the critPathCost of mtaskp, that was out-of-date with respect to its edges
-            const uint64_t startCp = mtaskp->critPathCost(way);
-            const uint64_t newCp = startCp + cpGrowBy;
-            if (VL_UNLIKELY(m_slowAsserts)) {
-                // Check that CP matches that of the longest edge wayward of vxp.
-                const uint64_t edgeCp = mtaskp->m_edgeHeap[inv].max()->key().m_score;
-                UASSERT_OBJ(edgeCp == newCp, mtaskp, "CP doesn't match longest wayward edge");
-                // Confirm that we only set each node's CP once.  That's an
-                // important property of PropagateCp which allows it to be far
-                // faster than a recursive algorithm on some graphs.
-                const bool first = m_seen.insert(mtaskp).second;
-                UASSERT_OBJ(first, mtaskp, "Set CP on node twice");
-            }
-            mtaskp->setCritPathCost(way, newCp);
-            cpHasIncreased(mtaskp, newCp + mtaskp->cost());
-        }
-
-        if (VL_UNLIKELY(m_slowAsserts)) m_seen.clear();
-    }
-
-private:
-    VL_UNCOPYABLE(PropagateCp);
 };
 
 //=============================================================================
@@ -486,31 +273,87 @@ private:
 
 // The graph of LogicMTask vertices and MTaskEdge edges, used during multi-threaded scheduling.
 class OrderMTaskGraph final : public V3Graph {
+    // MEMBERS
     OrderMoveGraph& m_moveGraph;  // The OrderMoveGraph this graph is built from
     LogicMTask* const m_entryp;  // The singular entry point vertex
     LogicMTask* const m_exitp;  // The singular exit point vertex
 
-    // The critical path propagators, one for each direction. Owned here so the algorithms
+    const bool m_slowAsserts;  // Take extra time to validate the graph ('--debug-partition')
-    // operating on this graph (contraction, hazard fixing) share them.
+
-    PropagateCp<GraphWay::FORWARD> m_forwardPropagator;  // Forward propagator
+    // Critical path propagation state. Scratch only: the heap is empty, and no MTask is pending,
-    PropagateCp<GraphWay::REVERSE> m_reversePropagator;  // Reverse propagator
+    // between calls to 'propagate'. The node pool persists to recycle the heap nodes.
+    PropagatePendingHeap m_pendingHeap;  // Heap of MTasks pending a critical path update
+    PoolAllocator<PropagatePendingHeap::Node> m_pendingNodePool;  // Allocator for the heap nodes
+
+    // Generation counter, e.g. for marking the MTasks visited by algorithms
+    uint64_t m_currentGeneration = 0;
 
     // CONSTRUCTOR
     explicit OrderMTaskGraph(OrderMoveGraph& moveGraph);  // Used by build(), hence private
     VL_UNCOPYABLE(OrderMTaskGraph);
     VL_UNMOVABLE(OrderMTaskGraph);
 
+    // METHODS
+
+    bool pathExistsImpl(LogicMTask* fromp, LogicMTask* top, const MTaskEdge* excludedEdgep);
+
+    // Bring the critical paths of all MTasks wayward of 'mtaskp' in direction N_Way, and those
+    // cached in the edge heaps on the way, up to date. Call after mutating the graph such that
+    // only MTasks wayward of 'mtaskp' can have a stale critical path, and the critical path of
+    // 'mtaskp' itself is already correct. 'mtaskp' is read, but never modified.
+    //
+    // Note critical paths can only ever grow: those cached in the edge heaps are heap keys, and a
+    // heap key can be increased in place, but not decreased.
+    template <GraphWay::en N_Way>
+    void propagate(LogicMTask* mtaskp) {
+        ++m_currentGeneration;
+        propagatePush<N_Way>(mtaskp);
+        propagateResolve<N_Way>();
+    }
+    // Push the inclusive critical path of 'mtaskp' onto each of its wayward relatives, and add any
+    // relative left with a stale critical path to the pending heap (out of line below)
+    template <GraphWay::en N_Way>
+    void propagatePush(LogicMTask* mtaskp);
+    // Resolve all pending critical path increases (out of line below)
+    template <GraphWay::en N_Way>
+    void propagateResolve();
+    // Part of 'validate'
+    template <GraphWay::en N_Way>
+    void validateWay() const;
+
 public:
     // ACCESSORS
     OrderMoveGraph& moveGraph() const { return m_moveGraph; }
     LogicMTask* entryp() const { return m_entryp; }
     LogicMTask* exitp() const { return m_exitp; }
-    PropagateCp<GraphWay::FORWARD>& forwardPropagator() { return m_forwardPropagator; }
+    bool slowAsserts() const { return m_slowAsserts; }
-    PropagateCp<GraphWay::REVERSE>& reversePropagator() { return m_reversePropagator; }
 
     // METHODS
     uint64_t totalCost() const;  // O(V), called once
 
+    // True if there's a path from 'fromp' to 'top' excluding 'excludedEdgep', false otherwise.
+    // 'excludedEdgep' may be nullptr in which case no edge is excluded. If 'excludedEdgep' is
+    // non-nullptr it must connect fromp and top.
+    bool pathExists(LogicMTask* fromp, LogicMTask* top, const MTaskEdge* excludedEdgep) {
+        ++m_currentGeneration;
+        return pathExistsImpl(fromp, top, excludedEdgep);
+    }
+
+    // Add an edge to the graph, update impacted critical paths
+    void addEdge(LogicMTask* fromp, LogicMTask* top);
+
+    // Merge 'donorp' into 'recipientp': move the contents and all edges of 'donorp' onto
+    // 'recipientp', update impacted critical paths, then delete 'donorp'. The edge connecting the
+    // two (if any) becomes internal to the merged MTask and is deleted, as is one of each pair of
+    // edges the two have to a common relative. Note this deletes edges, so the caller must have
+    // released any auxiliary data it attached to them via their user pointer.
+    void mergeMTasks(LogicMTask* recipientp, LogicMTask* donorp);
+
+    // Do an EXPENSIVE check that the maintained critical paths, including the ones cached in the
+    // edge heaps, match those implied by the current edges of the graph, and that the graph itself
+    // is consistent. Does nothing unless 'slowAsserts', so it is safe to call unconditionally.
+    void validate() const;
+
     // STATIC METHODS
     // Build an MTask graph from 'moveGraph'
     static std::unique_ptr<OrderMTaskGraph> build(OrderMoveGraph& moveGraph) VL_MT_DISABLED;
@@ -523,9 +366,9 @@ public:
 //=============================================================================
 // MTaskEdge method definitions (need the full definition of LogicMTask)
 
-MTaskEdge::MTaskEdge(OrderMTaskGraph* graphp, LogicMTask* fromp, LogicMTask* top, int weight)
+MTaskEdge::MTaskEdge(OrderMTaskGraph* graphp, LogicMTask* fromp, LogicMTask* top)
-    : V3GraphEdge{graphp, fromp, top, weight} {
+    : V3GraphEdge{graphp, fromp, top, 1} {
-    fromp->addRelativeMTask(top);
+    fromp->addDependent(top);
     fromp->addRelativeEdge<GraphWay::FORWARD>(this);
     top->addRelativeEdge<GraphWay::REVERSE>(this);
 }
@@ -537,13 +380,4 @@ LogicMTask* MTaskEdge::furtherMTaskp() const {
 LogicMTask* MTaskEdge::fromMTaskp() const { return static_cast<LogicMTask*>(fromp()); }
 LogicMTask* MTaskEdge::toMTaskp() const { return static_cast<LogicMTask*>(top()); }
 
-void MTaskEdge::resetCriticalPaths() {
-    LogicMTask* const fromp = fromMTaskp();
-    LogicMTask* const top = toMTaskp();
-    fromp->removeRelativeEdge<GraphWay::FORWARD>(this);
-    top->removeRelativeEdge<GraphWay::REVERSE>(this);
-    fromp->addRelativeEdge<GraphWay::FORWARD>(this);
-    top->addRelativeEdge<GraphWay::REVERSE>(this);
-}
-
 #endif  // Guard

src/V3OrderParallel.cpp

@@ -71,6 +71,9 @@ static std::unique_ptr<OrderMTaskGraph> partition(OrderMoveGraph& moveGraph) {
         mTaskGraphp->hashGraphDebug("MTask graph after contract()");
     }
 
+    // Note the graph is only mutated by generic V3Graph algorithms from here on. These neither
+    // maintain the critical paths of the MTasks, nor create MTaskEdges when rerouting, so the
+    // critical paths are stale below, and the graph must not be handed back to OrderMTaskGraph.
     mTaskGraphp->removeTransitiveEdges();
     mTaskGraphp->hashGraphDebug("MTask graph after removeTransitiveEdges()");
 

src/V3PairingHeap.h

@@ -25,8 +25,8 @@
 //=============================================================================
 // Pairing heap (max-heap) with increase key and delete.
 //
-// While this is written as a generic data structure, it's interface and
+// While this is written as a generic data structure, its interface and
-// implementation is finely tuned for use by V3Partition, and is critical
+// implementation is finely tuned for use by parallel scheduling, and is critical
 // to Verilation performance, so be very careful changing anything or adding any
 // new operations that would impact either memory usage, or performance of the
 // existing operations. This data structure is fully deterministic, meaning

src/V3PoolAllocator.h

@@ -0,0 +1,85 @@
+// -*- mode: C++; c-file-style: "cc-mode" -*-
+//*************************************************************************
+// DESCRIPTION: Verilator: Chunked pool allocator
+//
+// Code available from: https://verilator.org
+//
+//*************************************************************************
+//
+// 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-FileCopyrightText: 2003-2026 Wilson Snyder
+// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+//
+//*************************************************************************
+
+#ifndef VERILATOR_V3POOLALLOCATOR_H_
+#define VERILATOR_V3POOLALLOCATOR_H_
+
+#include "config_build.h"
+#include "verilatedos.h"
+
+#include <cstddef>
+#include <memory>
+#include <new>
+#include <utility>
+#include <vector>
+
+// Hands out elements of a single type, allocated 'N_ChunkSize' at a time for
+// efficiency. Released elements are recycled via a free list stored in the
+// same storage. All memory is released when the pool is destroyed, so the
+// elements it handed out must not be used beyond its lifetime, and must all
+// have been released by then.
+template <typename T_Elem, size_t N_ChunkSize = 128>
+class PoolAllocator final {
+    static_assert(N_ChunkSize > 0, "Chunk size must be non-zero");
+
+    // A slot of storage, holding either a live element, or a link in the free list.
+    union Slot final {
+        T_Elem m_elem;  // Storage for the allocated element
+        Slot* m_nextFreep;  // Link to the next free slot
+        Slot() {}
+        ~Slot() {}
+    };
+
+    // MEMBERS
+    Slot* m_freep = nullptr;  // Head of the free list
+    std::vector<std::unique_ptr<Slot[]>> m_allocated;  // The allocated chunks
+
+public:
+    // CONSTRUCTORS
+    PoolAllocator() = default;
+    VL_UNCOPYABLE(PoolAllocator);
+    VL_UNMOVABLE(PoolAllocator);
+
+    // METHODS
+    // Allocate an element, constructed with the given arguments
+    template <typename... Args>
+    T_Elem* alloc(Args&&... args) {
+        // If no free slots available, then make some
+        if (!m_freep) {
+            // Allocate in chunks for efficiency
+            m_allocated.emplace_back(new Slot[N_ChunkSize]);
+            // Chain the new slots into the free list
+            Slot* const chunkp = m_allocated.back().get();
+            for (size_t i = 1; i < N_ChunkSize; ++i) chunkp[i - 1].m_nextFreep = &chunkp[i];
+            chunkp[N_ChunkSize - 1].m_nextFreep = nullptr;
+            m_freep = chunkp;
+        }
+        // Free slots are available, pick up the first one
+        Slot* const slotp = m_freep;
+        m_freep = slotp->m_nextFreep;
+        return new (&slotp->m_elem) T_Elem{std::forward<Args>(args)...};
+    }
+
+    // Destroy an element, and return its slot for future allocation
+    void free(T_Elem* elemp) {
+        elemp->~T_Elem();
+        Slot* const slotp = reinterpret_cast<Slot*>(elemp);
+        slotp->m_nextFreep = m_freep;
+        m_freep = slotp;
+    }
+};
+
+#endif  // Guard