From 87108ff943b729271828ecfddc32fe54570cf40a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 5 Sep 2026 16:13:30 +0100 Subject: [PATCH] Optimize redundant vertex cache lookups in DFG peephole (#8283) --- src/V3DfgCache.h | 18 ++++++------ src/V3DfgPeephole.cpp | 64 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/V3DfgCache.h b/src/V3DfgCache.h index aa9cb050a..d3cce4c85 100644 --- a/src/V3DfgCache.h +++ b/src/V3DfgCache.h @@ -328,21 +328,23 @@ class V3DfgCache final { } public: + // Note: the cache starts out empty. If the caller wants existing vertices + // to be found, it must add them itself by calling 'cache' on each. explicit V3DfgCache(DfgGraph& dfg) : m_dfg{dfg} { - // Initialize the type to cache lookup table + // Initialize the type to cache lookup table #define VERTEX_CACHE_DECLARE_CACHE_PTR(t) m_vtxType2Cachep[t::dfgType()] = &m_cache##t; - FOREACH_DFG_VERTEX_TYPE(VERTEX_CACHE_DECLARE_CACHE_PTR) + FOREACH_DFG_VERTEX_TYPE(VERTEX_CACHE_DECLARE_CACHE_PTR) #undef VERTEX_CACHE_DECLARE_CACHE_PTR + } - // Add all operation vertices to the cache - for (DfgVertex& vtx : m_dfg.opVertices()) cache(&vtx); + // Add an existing vertex to the cache. If an equivalent (but different) already exists, + // it is returned and the cache is not updated. + DfgVertex + * cache(DfgVertex * vtxp) { + return m_vtxType2Cachep[vtxp->type()]->cache(vtxp); } - // Add an existing vertex to the cache. If an equivalent (but different) already exists, - // it is returned and the cache is not updated. - DfgVertex* cache(DfgVertex* vtxp) { return m_vtxType2Cachep[vtxp->type()]->cache(vtxp); } - // Remove an exiting vertex, it is the cached vertex. void invalidate(DfgVertex* vtxp) { m_vtxType2Cachep[vtxp->type()]->invalidate(vtxp); } diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 5b584cd6b..c96a3b49e 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -195,6 +195,7 @@ class V3DfgPeephole final : public DfgVisitor { size_t m_iterListIndex = 0; // Position of this vertx m_iterList (0 means not in list) size_t m_generation = 0; // Generation number of this vertex - for uniqueness check size_t m_id = 0; // Unique vertex ID (0 means unassigned) - for sorting + bool m_isCachedVertex = false; // This vertex is the vertex cached for its operation }; // STATE @@ -226,6 +227,41 @@ class V3DfgPeephole final : public DfgVisitor { return true; } + // Add vertex to the cache. If an equivalent (but different) vertex is + // already cached, it is returned and the cache is not updated. + DfgVertex* cacheVertex(DfgVertex* vtxp) { + DfgVertex* const equivp = m_cache.cache(vtxp); + UASSERT_OBJ(!m_vInfo[vtxp].m_isCachedVertex || !equivp, vtxp, + "Vertex marked 'm_isCachedVertex' has a cached equivalent"); + m_vInfo[vtxp].m_isCachedVertex = !equivp; + return equivp; + } + + // Remove vertex from the cache (no-op if it is not the cached vertex) + void invalidateVertex(DfgVertex* vtxp) { + m_cache.invalidate(vtxp); + m_vInfo[vtxp].m_isCachedVertex = false; + } + + // Find the vertex of the given type with the given operands in the + // cache, or create a new one and add it to the cache. + template + Vertex* getOrCreateVertex(FileLine* flp, const DfgDataType& dtype, Operands... operands) { + Vertex* const vtxp = m_cache.getOrCreate(flp, dtype, operands...); + m_vInfo[vtxp].m_isCachedVertex = true; + return vtxp; + } + + // Find the vertex of the given type with the given operands in the + // cache, or nullptr if there is no such vertex. + template + Vertex* getVertex(const DfgDataType& dtype, Operands... operands) { + Vertex* const vtxp = m_cache.get(dtype, operands...); + UASSERT_OBJ(!vtxp || m_vInfo[vtxp].m_isCachedVertex, vtxp, + "Cached vertex not marked 'm_isCachedVertex'"); + return vtxp; + } + void incrementGeneration() { ++m_currentGeneration; // TODO: could sweep on overflow @@ -270,7 +306,7 @@ class V3DfgPeephole final : public DfgVisitor { UASSERT_OBJ(!varp || !varp->hasPrev(), vtxp, "Deleting variable consumed via DfgPrev"); // Invalidate cache entry - m_cache.invalidate(vtxp); + invalidateVertex(vtxp); // It might be in the iter list, remove it removeFromIterList(vtxp); @@ -344,14 +380,14 @@ class V3DfgPeephole final : public DfgVisitor { // Remove sinks of the original vertex from the cache - their inputs are changing m_vtxp->foreachSink([&](DfgVertex& dst) { - m_cache.invalidate(&dst); + invalidateVertex(&dst); return false; }); // Replace vertex with the replacement m_vtxp->replaceWith(resp); // Re-cache all sinks of the replacement resp->foreachSink([&](DfgVertex& dst) { - m_cache.cache(&dst); + cacheVertex(&dst); return false; }); @@ -390,7 +426,7 @@ class V3DfgPeephole final : public DfgVisitor { template Vertex* make(FileLine* flp, const DfgDataType& dtype, Operands... operands) { // Find or create an equivalent vertex - Vertex* const vtxp = m_cache.getOrCreate(flp, dtype, operands...); + Vertex* const vtxp = getOrCreateVertex(flp, dtype, operands...); // Sanity check UASSERT_OBJ(vtxp->dtype() == dtype, vtxp, "Vertex dtype mismatch"); if (VL_UNLIKELY(v3Global.opt.debugCheck())) vtxp->typeCheck(m_dfg); @@ -594,7 +630,7 @@ class V3DfgPeephole final : public DfgVisitor { // '(a OP (b OP c))' -> '(a OP b) OP c' if (Vertex* const existingp - = m_cache.get(resultDType(lhsp, rlVtxp), lhsp, rlVtxp)) { + = getVertex(resultDType(lhsp, rlVtxp), lhsp, rlVtxp)) { UASSERT_OBJ(existingp->hasSinks(), vtxp, "Existing vertex should be used"); if (existingp != rhsp) { APPLYING(REUSE_ASSOC_BINARY_LHS_WITH_LHS_OF_RHS) { @@ -607,7 +643,7 @@ class V3DfgPeephole final : public DfgVisitor { // '(a OP (b OP c))' -> '(a OP c) OP b' iff also commutative if VL_CONSTEXPR_CXX17 (IsCommutative::value) { if (Vertex* const existingp - = m_cache.get(resultDType(lhsp, rrVtxp), lhsp, rrVtxp)) { + = getVertex(resultDType(lhsp, rrVtxp), lhsp, rrVtxp)) { UASSERT_OBJ(existingp->hasSinks(), vtxp, "Existing vertex should be used"); if (existingp != rhsp) { APPLYING(REUSE_ASSOC_BINARY_LHS_WITH_RHS_OF_RHS) { @@ -3127,6 +3163,9 @@ class V3DfgPeephole final : public DfgVisitor { // Assign vertex IDs m_dfg.forEachVertex([&](DfgVertex& vtx) { m_vInfo[vtx].m_id = ++m_lastId; }); + // Add all operation vertices to the cache + for (DfgVertex& vtx : m_dfg.opVertices()) cacheVertex(&vtx); + // Initialize the work list and iter list. They can't get bigger than // m_dfg.size(), but new vertices are created in the loop, so over alloacte m_workList.reserve(m_dfg.size() * 2); @@ -3168,11 +3207,14 @@ class V3DfgPeephole final : public DfgVisitor { // Unsued vertices should have been removed immediately UASSERT_OBJ(m_vtxp->hasSinks(), m_vtxp, "Operation vertex should have sinks"); - // Check if an equivalent vertex exists, if so replace this vertex with it - if (DfgVertex* const sampep = m_cache.cache(m_vtxp)) { - APPLYING(REPLACE_WITH_EQUIVALENT) { - replace(sampep); - continue; + // Check if an equivalent vertex exists, if so replace this vertex with it. + // Can skip the lookup if the vertex is known to be the cached one + if (!m_vInfo[m_vtxp].m_isCachedVertex) { + if (DfgVertex* const sampep = cacheVertex(m_vtxp)) { + APPLYING(REPLACE_WITH_EQUIVALENT) { + replace(sampep); + continue; + } } }