diff --git a/src/V3OrderMTaskContraction.cpp b/src/V3OrderMTaskContraction.cpp index 94220a4ce..a2897a01b 100644 --- a/src/V3OrderMTaskContraction.cpp +++ b/src/V3OrderMTaskContraction.cpp @@ -251,7 +251,7 @@ static uint64_t siblingScore(const SiblingMC* sibsp) { = std::max(ap->critPathCost(GraphWay::FORWARD), bp->critPathCost(GraphWay::FORWARD)); const uint64_t mergedCpCostRev = std::max(ap->critPathCost(GraphWay::REVERSE), bp->critPathCost(GraphWay::REVERSE)); - return mergedCpCostRev + mergedCpCostFwd + LogicMTask::stepCost(ap->cost() + bp->cost()); + return mergedCpCostRev + mergedCpCostFwd + ap->cost() + bp->cost(); } static uint64_t edgeScore(const MTaskEdge* edgep) { @@ -264,7 +264,7 @@ static uint64_t edgeScore(const MTaskEdge* edgep) { top->critPathCostWithout(edgep)); const uint64_t mergedCpCostRev = std::max(fromp->critPathCostWithout(edgep), top->critPathCost(GraphWay::REVERSE)); - return mergedCpCostRev + mergedCpCostFwd + LogicMTask::stepCost(fromp->cost() + top->cost()); + return mergedCpCostRev + mergedCpCostFwd + fromp->cost() + top->cost(); } void MergeCandidate::rescore() { @@ -446,11 +446,10 @@ static void partInitHalfCriticalPaths(V3Graph& mTaskGraph, bool checkOnly) { relatives.insert(edge.furtherp()); #endif const LogicMTask* const relativep = static_cast(edge.furtherp()); - cpCost = std::max(cpCost, (relativep->critPathCost(way) - + static_cast(relativep->stepCost()))); + cpCost = std::max(cpCost, (relativep->critPathCost(way) + relativep->cost())); } if (checkOnly) { - partCheckCachedScoreVsActual(mtaskp->critPathCost(way), cpCost); + UASSERT(mtaskp->critPathCost(way) == cpCost, "Calculation error in scoring"); } else { mtaskp->setCritPathCost(way, cpCost); } @@ -651,7 +650,7 @@ public: UASSERT_OBJ(first, mtaskp, "Set CP on node twice"); } mtaskp->setCritPathCost(way, newCp); - cpHasIncreased(mtaskp, newCp + mtaskp->stepCost()); + cpHasIncreased(mtaskp, newCp + mtaskp->cost()); } if (VL_UNLIKELY(m_slowAsserts)) m_seen.clear(); @@ -824,13 +823,12 @@ class Contraction final { newCp = std::max(otherp->critPathCost(way), mtaskp->critPathCost(way)); } - const uint64_t origRelativesCp = mtaskp->critPathCost(way) + mtaskp->stepCost(); - const uint64_t newRelativesCp - = newCp + LogicMTask::stepCost(mtaskp->cost() + otherp->cost()); + const uint64_t oldRelativesCp = mtaskp->critPathCost(way) + mtaskp->cost(); + const uint64_t newRelativesCp = newCp + mtaskp->cost() + otherp->cost(); NewCp result; result.cp = newCp; - result.propagate = (newRelativesCp > origRelativesCp); + result.propagate = (newRelativesCp > oldRelativesCp); result.propagateCp = newRelativesCp; return result; } @@ -919,7 +917,7 @@ class Contraction final { m_sb.removeSibling(mergeSibsp); } - // This also updates cost and stepCost on recipientp + // This also updates cost on recipientp recipientp->moveAllVerticesFrom(donorp); UINFO(9, "recipient = " << recipientp->id() << ", donor = " << donorp->id() @@ -1232,7 +1230,7 @@ class Contraction final { continue; } - partCheckCachedScoreVsActual(cachedScore, actualScore); + UASSERT(cachedScore == actualScore, "Calculation error in scoring"); // Finally there's no cycle risk, no need to rescore, we're // within m_scoreLimit and m_scoreLimitBeforeRescore. diff --git a/src/V3OrderMTaskGraph.h b/src/V3OrderMTaskGraph.h index 201011333..2db922fea 100644 --- a/src/V3OrderMTaskGraph.h +++ b/src/V3OrderMTaskGraph.h @@ -42,51 +42,6 @@ class LogicMTask; template class PropagateCp; -// When computing critical path costs, use a step function on the actual underlying vertex cost. -// -// If there are huge vertices, when a tiny vertex merges into a huge vertex, we can often avoid -// increasing the huge vertex's stepped cost. If the stepped cost hasn't increased, and the -// critical path into the huge vertex hasn't increased, we can avoid propagating a new critical -// path to vertices past the huge vertex. Since huge vertices tend to have huge lists of children -// and parents, this can be a substantial savings. -// -// Does not seem to reduce the quality of the partitioner's output. -// -// If you have huge vertices, leave this 'true', it is the major setting that allows the -// partitioner to handle such difficult graphs on anything like a human time scale. -// -// If you don't have huge vertices, the 'true' value doesn't help much but should cost almost -// nothing in terms of partitioner quality. -// -// If you want the most aggressive possible partition, set it "false" and be prepared to be -// disappointed when the improvement in the partition is negligible / in the noise. -// -// Q) Why retain the control, if there is really no downside? -// -// A) Cost stepping can lead to corner cases. A developer may wish to disable cost stepping to -// rule it out as the cause of unexpected behavior. -#define PART_STEPPED_COST true - -//###################################################################### -// Misc graph and assertion utilities - -inline void partCheckCachedScoreVsActual(uint64_t cached, uint64_t actual) { -#if PART_STEPPED_COST - // Cached CP might be a little bigger than actual, due to stepped CPs. - // Example: - // Let's say we have a parent with stepped_cost 40 and a grandparent - // with stepped_cost 27. Our forward-cp is 67. Then our parent and - // grandparent get merged, the merged node has stepped cost 66. We - // won't propagate that new CP to children as it hasn't grown. So, - // children may continue to think that the CP coming through this path - // is a little higher than it really is; permit that. - UASSERT((((cached * 10) <= (actual * 11)) && (cached * 11) >= (actual * 10)), - "Calculation error in scoring (approximate, may need tweak)"); -#else - UASSERT(cached == actual, "Calculation error in scoring"); -#endif -} - //============================================================================= // OrderMTaskGraph @@ -198,6 +153,8 @@ class LogicMTask final : public V3GraphVertex { OrderMoveVertex::List m_mVertices; // 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 @@ -235,28 +192,6 @@ public: 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; } - static uint64_t stepCost(uint64_t cost) { -#if PART_STEPPED_COST - // Round cost up to the nearest 5%. Use this when computing all critical paths. The idea is - // that critical path changes don't need to propagate when they don't exceed the next step, - // saving a lot of recursion. - if (cost == 0) return 0; - - double logcost = log(cost); - // log(1.05) is about 0.05, so round logcost up to the next 0.05 boundary - logcost *= 20.0; - logcost = ceil(logcost); - logcost = logcost / 20.0; - - const uint64_t sCost = static_cast(exp(logcost)); - UDEBUGONLY(UASSERT_STATIC(sCost >= cost, "stepped cost error exceeded");); - UDEBUGONLY(UASSERT_STATIC(sCost <= ((cost * 11 / 10)), "stepped cost error exceeded");); - return sCost; -#else - return cost; -#endif - } - uint64_t stepCost() const { return stepCost(m_cost); } uint64_t critPathCost(GraphWay way) const { return m_critPathCost[way]; } void setCritPathCost(GraphWay way, uint64_t cost) { m_critPathCost[way] = cost; } @@ -275,7 +210,7 @@ public: // Add to the edge heap LogicMTask* const relativep = edgep->furtherMTaskp(); // Value is !way cp to this edge - const uint64_t cp = relativep->stepCost() + relativep->critPathCost(inv); + const uint64_t cp = relativep->cost() + relativep->critPathCost(inv); m_edgeHeap[way].insert(&edgep->m_edgeHeapNode[way], {cp, relativep->id()}); } template @@ -311,8 +246,8 @@ public: const LogicMTask* const relativep = static_cast(edge.furtherp()); const uint64_t cachedCp = static_cast(edge).cachedCp(way); - const uint64_t cp = relativep->critPathCost(way.invert()) + relativep->stepCost(); - partCheckCachedScoreVsActual(cachedCp, cp); + const uint64_t cp = relativep->critPathCost(way.invert()) + relativep->cost(); + UASSERT(cachedCp == cp, "Calculation error in scoring"); } } @@ -354,10 +289,10 @@ private: // Base case: fromp is too late, cannot possibly be a prereq for top. if (fromp->critPathCost(GraphWay::REVERSE) - < (top->critPathCost(GraphWay::REVERSE) + top->stepCost())) { + < (top->critPathCost(GraphWay::REVERSE) + top->cost())) { return false; } - if ((fromp->critPathCost(GraphWay::FORWARD) + fromp->stepCost()) + if ((fromp->critPathCost(GraphWay::FORWARD) + fromp->cost()) > top->critPathCost(GraphWay::FORWARD)) { return false; }