luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Internals: Remove stepCost use in MT scheduling (#8032) 

MT scheduling used to round MTask costs to the nearest 5% to supposedly
reduce the amount of work required for propagating critical path change
updates. Profiling shows this doesn't actually save any verilation time,
as the floating point arithmetic itself is costly, and the propagation
savings only kick in when merging small tasks into large ones, which is
not the common case. (The float arithmetic also made this step
non-reproducible across host architectures and libm versions  due to
numerical variance.)

Remove and use precise costs instead.
This commit is contained in:
Geza Lore 2026-08-03 17:34:44 +01:00 committed by GitHub
parent 1356c15e44
commit d58f512a85
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 17 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/V3OrderMTaskContraction.cpp
View File

@ -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<GraphWay::FORWARD>(edgep));
const uint64_t mergedCpCostRev = std::max(fromp->critPathCostWithout<GraphWay::REVERSE>(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<rev>());
#endif
const LogicMTask* const relativep = static_cast<LogicMTask*>(edge.furtherp<rev>());
cpCost = std::max(cpCost, (relativep->critPathCost(way)
+ static_cast<uint64_t>(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.

79
src/V3OrderMTaskGraph.h
View File

@ -42,51 +42,6 @@ class LogicMTask;
template <GraphWay::en N_Way>
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<uint64_t>(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<N_Way>();
// 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 <GraphWay::en N_Way>
@ -311,8 +246,8 @@ public:
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->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;
}