diff --git a/src/V3Dfg.h b/src/V3Dfg.h index 0849cd046..ecc4d421a 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -200,8 +200,6 @@ protected: m_inputps.emplace_back(new DfgEdge{this}); return m_inputps.back().get(); } - // Unlink all inputs and reset to no inputs - void resetInputs() { m_inputps.clear(); } public: // Get input 'i' @@ -210,6 +208,8 @@ public: void inputp(size_t i, DfgVertex* vtxp) { m_inputps[i]->relinkSrcp(vtxp); } // The number of inputs this vertex has. Some might be unconnected. size_t nInputs() const { return m_inputps.size(); } + // Unlink all inputs and reset to no inputs - use very carefully + void resetInputs() { m_inputps.clear(); } // The type of this vertex VDfgType type() const { return m_type; } diff --git a/src/V3DfgCache.cpp b/src/V3DfgCache.cpp index 83730f789..d6816bae7 100644 --- a/src/V3DfgCache.cpp +++ b/src/V3DfgCache.cpp @@ -26,13 +26,15 @@ namespace V3DfgCacheInternal { -void V3DfgCache::cache(DfgVertex* vtxp) { +DfgVertex* V3DfgCache::cache(DfgVertex* vtxp) { switch (vtxp->type()) { #define VERTEX_CACHE_ADD_CASE(t) \ - case t::dfgType(): V3DfgCacheInternal::cache(m_cache##t, reinterpret_cast(vtxp)); break; + case t::dfgType(): \ + return V3DfgCacheInternal::cache(m_cache##t, reinterpret_cast(vtxp)); \ + break; FOREACH_CACHED_VERTEX_TYPE(VERTEX_CACHE_ADD_CASE) #undef VERTEX_CACHE_ADD_CASE - default: break; + default: return nullptr; } } diff --git a/src/V3DfgCache.h b/src/V3DfgCache.h index 14822391c..252db7340 100644 --- a/src/V3DfgCache.h +++ b/src/V3DfgCache.h @@ -254,26 +254,35 @@ inline Vertex* get(DfgGraph& dfg, T_Cache& cache, const DfgDataType& dtype, Oper return it != cache.end() ? reinterpret_cast(it->second) : nullptr; } -// These add an existing vertex to the table, if an equivalent does not yet exist -inline void cache(CacheSel& cache, DfgSel* vtxp) { +// These add an existing vertex to the table. If an equivalent exists, +// it is returned and the cache is not updated. +inline DfgSel* cache(CacheSel& cache, DfgSel* vtxp) { DfgSel*& entrypr = getEntry(cache, vtxp->dtype(), vtxp->fromp(), vtxp->lsb()); - if (!entrypr) entrypr = vtxp; + if (entrypr && entrypr != vtxp) return entrypr; + entrypr = vtxp; + return nullptr; } -inline void cache(CacheUnary& cache, DfgVertexUnary* vtxp) { +inline DfgVertexUnary* cache(CacheUnary& cache, DfgVertexUnary* vtxp) { DfgVertexUnary*& entrypr = getEntry(cache, vtxp->dtype(), vtxp->inputp(0)); - if (!entrypr) entrypr = vtxp; + if (entrypr && entrypr != vtxp) return entrypr; + entrypr = vtxp; + return nullptr; } -inline void cache(CacheBinary& cache, DfgVertexBinary* vtxp) { +inline DfgVertexBinary* cache(CacheBinary& cache, DfgVertexBinary* vtxp) { DfgVertexBinary*& entrypr = getEntry(cache, vtxp->dtype(), vtxp->inputp(0), vtxp->inputp(1)); - if (!entrypr) entrypr = vtxp; + if (entrypr && entrypr != vtxp) return entrypr; + entrypr = vtxp; + return nullptr; } -inline void cache(CacheTernary& cache, DfgVertexTernary* vtxp) { +inline DfgVertexTernary* cache(CacheTernary& cache, DfgVertexTernary* vtxp) { DfgVertexTernary*& entrypr = getEntry(cache, vtxp->dtype(), vtxp->inputp(0), vtxp->inputp(1), vtxp->inputp(2)); - if (!entrypr) entrypr = vtxp; + if (entrypr && entrypr != vtxp) return entrypr; + entrypr = vtxp; + return nullptr; } // These remove an existing vertex from the cache, if it is the cached vertex @@ -357,8 +366,9 @@ public: template inline Vertex* get(const DfgDataType& dtype, Operands... operands); - // Add an existing vertex of the table. If an equivalent already exists, then nothing happens. - void cache(DfgVertex* vtxp); + // Add an existing vertex of the cache. If an equivalent already exists, + // it is returned and the cache is not updated. + DfgVertex* cache(DfgVertex* vtxp); // Remove an exiting vertex, it is the cached vertex. void invalidateByValue(DfgVertex* vtxp); diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 9ed5f586f..f439551cb 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -170,22 +170,21 @@ class V3DfgPeephole final : public DfgVisitor { } void deleteVertex(DfgVertex* vtxp) { - // Add all sources to the work list - addSourcesToWorkList(vtxp); + // Inputs should have been reset + UASSERT_OBJ(vtxp->is() || !vtxp->nInputs(), vtxp, + "Operation Vertx should not have sources when being deleted"); + // Should not have sinks + UASSERT_OBJ(!vtxp->hasSinks(), vtxp, "Should not delete used vertex"); // If in work list then we can't delete it just yet (as we can't remove from the middle of // the work list), but it will be deleted when the work list is processed. if (m_workList.contains(*vtxp)) return; // Otherwise we can delete it now. - // Remove from cache - m_cache.invalidateByValue(vtxp); // This pass only removes variables that are either not driven in this graph, // or are not observable outside the graph. If there is also no external write // to the variable and no references in other graph then delete the Ast var too. const DfgVertexVar* const varp = vtxp->cast(); AstNode* const nodep = varp && !varp->isVolatile() && !varp->hasDfgRefs() ? varp->nodep() : nullptr; - // Should not have sinks - UASSERT_OBJ(!vtxp->hasSinks(), vtxp, "Should not delete used vertex"); // Delete vertex and Ast variable if any VL_DO_DANGLING(vtxp->unlinkDelete(m_dfg), vtxp); if (nodep) VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); @@ -206,16 +205,30 @@ class V3DfgPeephole final : public DfgVisitor { addSinksToWorkList(vtxp); // Add replacement to the work list addToWorkList(replacementp); - // Replace vertex with the replacement + // Add all sources to the work list + addSourcesToWorkList(vtxp); + // Remove this and sinks from cache + m_cache.invalidateByValue(vtxp); vtxp->foreachSink([&](DfgVertex& sink) { m_cache.invalidateByValue(&sink); return false; }); + // Replace vertex with the replacement vtxp->replaceWith(replacementp); - replacementp->foreachSink([&](DfgVertex& sink) { - m_cache.cache(&sink); - return false; - }); + // Unlink and reset all inputs of the replaced vertex so it doesn't get iterated again + vtxp->resetInputs(); + // Re-cache all sinks of the replacement, remove duplicates + while (true) { + DfgVertex* sinkp = nullptr; + DfgVertex* samep = nullptr; + replacementp->foreachSink([&](DfgVertex& sink) -> bool { + sinkp = &sink; + samep = m_cache.cache(&sink); + return samep; + }); + if (!samep) break; + replace(sinkp, samep); + } // Vertex is now unused, so delete it deleteVertex(vtxp); } @@ -1855,6 +1868,7 @@ class V3DfgPeephole final : public DfgVisitor { // If undriven, or driven from another var, it is completely redundant. if (!vtxp->srcp() || vtxp->srcp()->is()) { APPLYING(REMOVE_VAR) { + addSourcesToWorkList(vtxp); deleteVertex(vtxp); return; } @@ -1873,6 +1887,7 @@ class V3DfgPeephole final : public DfgVisitor { }); if (!keep) { APPLYING(REMOVE_VAR) { + addSourcesToWorkList(vtxp); deleteVertex(vtxp); return; } @@ -1899,6 +1914,11 @@ class V3DfgPeephole final : public DfgVisitor { m_workList.foreach([&](DfgVertex& vtx) { // Remove unused operations as we go. Some vars may be removed in the visit method. if (!vtx.hasSinks() && !vtx.is()) { + if (vtx.nInputs()) { + addSourcesToWorkList(&vtx); + m_cache.invalidateByValue(&vtx); + vtx.resetInputs(); + } deleteVertex(&vtx); return; }