diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 56e0a8290..184fdb874 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -187,32 +187,59 @@ static const string toDotId(const DfgVertex& vtx) { return '"' + cvtToHex(&vtx) // Dump one DfgVertex in Graphviz format static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << toDotId(vtx); - if (const DfgVar* const varVtxp = vtx.cast()) { + + if (const DfgVarPacked* const varVtxp = vtx.cast()) { AstVar* const varp = varVtxp->varp(); os << " [label=\"" << varp->name() << "\nW" << varVtxp->width() << " / F" << varVtxp->fanout() << '"'; - if (varp->isIO()) { - if (varp->direction() == VDirection::INPUT) { - os << ", shape=house, orientation=270"; - } else if (varp->direction() == VDirection::OUTPUT) { - os << ", shape=house, orientation=90"; - } else { - os << ", shape=star"; - } + + if (varp->direction() == VDirection::INPUT) { + os << ", shape=box, style=filled, fillcolor=chartreuse2"; // Green + } else if (varp->direction() == VDirection::OUTPUT) { + os << ", shape=box, style=filled, fillcolor=cyan2"; // Cyan + } else if (varp->direction() == VDirection::INOUT) { + os << ", shape=box, style=filled, fillcolor=darkorchid2"; // Purple } else if (varVtxp->hasExtRefs()) { - os << ", shape=box, style=diagonals,filled, fillcolor=red"; + os << ", shape=box, style=filled, fillcolor=firebrick2"; // Red } else if (varVtxp->hasModRefs()) { - os << ", shape=box, style=diagonals"; + os << ", shape=box, style=filled, fillcolor=gold2"; // Yellow + } else if (varVtxp->keep()) { + os << ", shape=box, style=filled, fillcolor=grey"; } else { os << ", shape=box"; } - os << "]"; - } else if (const DfgConst* const constVtxp = vtx.cast()) { + os << "]" << endl; + return; + } + + if (const DfgVarArray* const arrVtxp = vtx.cast()) { + AstVar* const varp = arrVtxp->varp(); + os << " [label=\"" << varp->name() << "[]\""; + if (varp->direction() == VDirection::INPUT) { + os << ", shape=box3d, style=filled, fillcolor=chartreuse2"; // Green + } else if (varp->direction() == VDirection::OUTPUT) { + os << ", shape=box3d, style=filled, fillcolor=cyan2"; // Cyan + } else if (varp->direction() == VDirection::INOUT) { + os << ", shape=box3d, style=filled, fillcolor=darkorchid2"; // Purple + } else if (arrVtxp->hasExtRefs()) { + os << ", shape=box3d, style=filled, fillcolor=firebrick2"; // Red + } else if (arrVtxp->hasModRefs()) { + os << ", shape=box3d, style=filled, fillcolor=gold2"; // Yellow + } else if (arrVtxp->keep()) { + os << ", shape=box3d, style=filled, fillcolor=grey"; + } else { + os << ", shape=box3d"; + } + os << "]" << endl; + return; + } + + if (const DfgConst* const constVtxp = vtx.cast()) { const V3Number& num = constVtxp->constp()->num(); os << " [label=\""; if (num.width() <= 32 && !num.isSigned()) { const bool feedsSel = !constVtxp->findSink([](const DfgVertex& vtx) { // - return !vtx.is(); + return !vtx.is() && !vtx.is(); }); if (feedsSel) { os << num.toUInt(); @@ -225,17 +252,17 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { } os << '"'; os << ", shape=plain"; - os << "]"; - } else { - os << " [label=\"" << vtx.typeName() << "\nW" << vtx.width() << " / F" << vtx.fanout() - << '"'; - if (vtx.hasMultipleSinks()) - os << ", shape=doublecircle"; - else - os << ", shape=circle"; - os << "]"; + os << "]" << endl; + return; } - os << endl; + + os << " [label=\"" << vtx.typeName() << "\nW" << vtx.width() << " / F" << vtx.fanout() << '"'; + if (vtx.hasMultipleSinks()) { + os << ", shape=doublecircle"; + } else { + os << ", shape=circle"; + } + os << "]" << endl; } // Dump one DfgEdge in Graphviz format @@ -314,9 +341,9 @@ static void dumpDotUpstreamConeFromVertex(std::ostream& os, const DfgVertex& vtx dumpDotVertexAndSourceEdges(os, *itemp); } - // Emit all DfgVar vertices that have external references driven by this vertex + // Emit all DfgVarPacked vertices that have external references driven by this vertex vtx.forEachSink([&](const DfgVertex& dst) { - if (const DfgVar* const varVtxp = dst.cast()) { + if (const DfgVarPacked* const varVtxp = dst.cast()) { if (varVtxp->hasRefs()) dumpDotVertexAndSourceEdges(os, dst); } }); @@ -349,9 +376,10 @@ void DfgGraph::dumpDotAllVarConesPrefixed(const string& label) const { const string prefix = label.empty() ? name() + "-cone-" : name() + "-" + label + "-cone-"; forEachVertex([&](const DfgVertex& vtx) { // Check if this vertex drives a variable referenced outside the DFG. - const DfgVar* const sinkp = vtx.findSink([](const DfgVar& sink) { // - return sink.hasRefs(); - }); + const DfgVarPacked* const sinkp + = vtx.findSink([](const DfgVarPacked& sink) { // + return sink.hasRefs(); + }); // We only dump cones driving an externally referenced variable if (!sinkp) return; @@ -428,6 +456,11 @@ DfgVertex::DfgVertex(DfgGraph& dfg, FileLine* flp, AstNodeDType* dtypep, DfgType dfg.addVertex(*this); } +DfgVertex::~DfgVertex() { + // TODO: It would be best to intern these via AstTypeTable to save the effort + if (VN_IS(m_dtypep, UnpackArrayDType)) VL_DO_DANGLING(delete m_dtypep, m_dtypep); +} + bool DfgVertex::selfEquals(const DfgVertex& that) const { return this->m_type == that.m_type && this->dtypep() == that.dtypep(); } @@ -470,7 +503,7 @@ V3Hash DfgVertex::hash(HashCache& cache) const { result += selfHash(); // Variables are defined by themselves, so there is no need to hash the sources. This // enables sound hashing of graphs circular only through variables, which we rely on. - if (!is()) { + if (!is() && !is()) { forEachSource([&result, &cache](const DfgVertex& src) { result += src.hash(cache); }); } } @@ -502,18 +535,31 @@ void DfgVertex::replaceWith(DfgVertex* newSorucep) { // Vertex classes //------------------------------------------------------------------------------ -// DfgVar ---------- -void DfgVar::accept(DfgVisitor& visitor) { visitor.visit(this); } +// DfgVarPacked ---------- +void DfgVarPacked::accept(DfgVisitor& visitor) { visitor.visit(this); } -bool DfgVar::selfEquals(const DfgVertex& that) const { - if (const DfgVar* otherp = that.cast()) { +bool DfgVarPacked::selfEquals(const DfgVertex& that) const { + if (const DfgVarPacked* otherp = that.cast()) { UASSERT_OBJ(varp() != otherp->varp(), this, - "There should only be one DfgVar for a given AstVar"); + "There should only be one DfgVarPacked for a given AstVar"); } return false; } -V3Hash DfgVar::selfHash() const { return V3Hasher::uncachedHash(varp()); } +V3Hash DfgVarPacked::selfHash() const { return V3Hasher::uncachedHash(varp()); } + +// DfgVarPacked ---------- +void DfgVarArray::accept(DfgVisitor& visitor) { visitor.visit(this); } + +bool DfgVarArray::selfEquals(const DfgVertex& that) const { + if (const DfgVarArray* otherp = that.cast()) { + UASSERT_OBJ(varp() != otherp->varp(), this, + "There should only be one DfgVarArray for a given AstVar"); + } + return false; +} + +V3Hash DfgVarArray::selfHash() const { return V3Hasher::uncachedHash(varp()); } // DfgConst ---------- void DfgConst::accept(DfgVisitor& visitor) { visitor.visit(this); } @@ -531,8 +577,8 @@ V3Hash DfgConst::selfHash() const { return V3Hasher::uncachedHash(m_constp); } // DfgVisitor //------------------------------------------------------------------------------ -void DfgVisitor::visit(DfgVar* vtxp) { visit(static_cast(vtxp)); } - +void DfgVisitor::visit(DfgVarPacked* vtxp) { visit(static_cast(vtxp)); } +void DfgVisitor::visit(DfgVarArray* vtxp) { visit(static_cast(vtxp)); } void DfgVisitor::visit(DfgConst* vtxp) { visit(static_cast(vtxp)); } //------------------------------------------------------------------------------ diff --git a/src/V3Dfg.h b/src/V3Dfg.h index aedb1c838..e17dd1909 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -205,7 +205,7 @@ protected: DfgVertex(DfgGraph& dfg, FileLine* flp, AstNodeDType* dtypep, DfgType type); public: - virtual ~DfgVertex() = default; + virtual ~DfgVertex(); // METHODS private: @@ -219,15 +219,14 @@ private: virtual V3Hash selfHash() const; public: - // Returns true if an AstNode with the given 'dtype' can be represented as a DfgVertex - static bool isSupportedDType(const AstNodeDType* dtypep) { - // Conservatively only support bit-vector like basic types and packed arrays of the same + // Supported packed types + static bool isSupportedPackedDType(const AstNodeDType* dtypep) { dtypep = dtypep->skipRefp(); if (const AstBasicDType* const typep = VN_CAST(dtypep, BasicDType)) { return typep->keyword().isIntNumeric(); } if (const AstPackArrayDType* const typep = VN_CAST(dtypep, PackArrayDType)) { - return isSupportedDType(typep->subDTypep()); + return isSupportedPackedDType(typep->subDTypep()); } if (const AstNodeUOrStructDType* const typep = VN_CAST(dtypep, NodeUOrStructDType)) { return typep->packed(); @@ -235,6 +234,17 @@ public: return false; } + // Returns true if an AstNode with the given 'dtype' can be represented as a DfgVertex + static bool isSupportedDType(const AstNodeDType* dtypep) { + dtypep = dtypep->skipRefp(); + // Support unpacked arrays of packed types + if (const AstUnpackArrayDType* const typep = VN_CAST(dtypep, UnpackArrayDType)) { + return isSupportedPackedDType(typep->subDTypep()); + } + // Support packed types + return isSupportedPackedDType(dtypep); + } + // Return data type used to represent any packed value of the given 'width'. All packed types // of a given width use the same canonical data type, as the only interesting information is // the total width. @@ -245,7 +255,13 @@ public: // Return data type used to represent the type of 'nodep' when converted to a DfgVertex static AstNodeDType* dtypeFor(const AstNode* nodep) { UDEBUGONLY(UASSERT_OBJ(isSupportedDType(nodep->dtypep()), nodep, "Unsupported dtype");); - // Currently all supported types are packed, so this is simple + // For simplicity, all packed types are represented with a fixed type + if (AstUnpackArrayDType* const typep = VN_CAST(nodep->dtypep(), UnpackArrayDType)) { + // TODO: these need interning via AstTypeTable otherwise they leak + return new AstUnpackArrayDType{typep->fileline(), + dtypeForWidth(typep->subDTypep()->width()), + typep->rangep()->cloneTree(false)}; + } return dtypeForWidth(nodep->width()); } @@ -257,6 +273,7 @@ public: // Width of result uint32_t width() const { // Everything supported is packed now, so we can just do this: + UASSERT_OBJ(VN_IS(dtypep(), BasicDType), this, "'width()' called on unpacked value"); return dtypep()->width(); } @@ -325,6 +342,8 @@ public: inline void forEachSourceEdge(std::function f) const; // Calls given function 'f' for each sink vertex of this vertex + // Unlinking/deleting the given sink during iteration is safe, but not other sinks of this + // vertex. inline void forEachSink(std::function f); // Calls given function 'f' for each sink vertex of this vertex @@ -338,6 +357,10 @@ public: // Calls given function 'f' for each sink edge of this vertex. inline void forEachSinkEdge(std::function f) const; + // Returns first source edge which satisfies the given predicate 'p', or nullptr if no such + // sink vertex exists + inline DfgEdge* findSourceEdge(std::function p); + // Returns first sink vertex of type 'Vertex' which satisfies the given predicate 'p', // or nullptr if no such sink vertex exists template @@ -600,7 +623,7 @@ public: } }; -class DfgVar final : public DfgVertexLValue { +class DfgVarPacked final : public DfgVertexLValue { friend class DfgVertex; friend class DfgVisitor; @@ -614,7 +637,7 @@ class DfgVar final : public DfgVertexLValue { static constexpr DfgType dfgType() { return DfgType::atVar; }; public: - DfgVar(DfgGraph& dfg, AstVar* varp) + DfgVarPacked(DfgGraph& dfg, AstVar* varp) : DfgVertexLValue{dfg, dfgType(), varp, 1u} {} bool isDrivenByDfg() const { return arity() > 0; } @@ -638,6 +661,43 @@ public: } }; +class DfgVarArray final : public DfgVertexLValue { + friend class DfgVertex; + friend class DfgVisitor; + + using DriverData = std::pair; + + std::vector m_driverData; // Additional data associate with each driver + + void accept(DfgVisitor& visitor) override; + bool selfEquals(const DfgVertex& that) const override; + V3Hash selfHash() const override; + static constexpr DfgType dfgType() { return DfgType::atUnpackArrayDType; }; // TODO: gross + +public: + DfgVarArray(DfgGraph& dfg, AstVar* varp) + : DfgVertexLValue{dfg, dfgType(), varp, 4u} { + UASSERT_OBJ(VN_IS(varp->dtypeSkipRefp(), UnpackArrayDType), varp, "Non array DfgVarArray"); + } + + bool isDrivenByDfg() const { return arity() > 0; } + + void addDriver(FileLine* flp, uint32_t index, DfgVertex* vtxp) { + m_driverData.emplace_back(flp, index); + DfgVertexVariadic::addSource()->relinkSource(vtxp); + } + + void resetSources() { + m_driverData.clear(); + DfgVertexVariadic::resetSources(); + } + + FileLine* driverFileLine(size_t idx) const { return m_driverData[idx].first; } + uint32_t driverIndex(size_t idx) const { return m_driverData[idx].second; } + + const string srcName(size_t idx) const override { return cvtToStr(driverIndex(idx)); } +}; + class DfgConst final : public DfgVertex { friend class DfgVertex; friend class DfgVisitor; @@ -685,7 +745,8 @@ public: // Dispatch to most specific 'visit' method on 'vtxp' void iterate(DfgVertex* vtxp) { vtxp->accept(*this); } - virtual void visit(DfgVar* vtxp); + virtual void visit(DfgVarPacked* vtxp); + virtual void visit(DfgVarArray* vtxp); virtual void visit(DfgConst* vtxp); #include "V3Dfg__gen_visitor_decls.h" }; @@ -746,7 +807,10 @@ void DfgVertex::forEachSource(std::function f) const { } void DfgVertex::forEachSink(std::function f) { - for (const DfgEdge* edgep = m_sinksp; edgep; edgep = edgep->m_nextp) f(*edgep->m_sinkp); + for (const DfgEdge *edgep = m_sinksp, *nextp; edgep; edgep = nextp) { + nextp = edgep->m_nextp; + f(*edgep->m_sinkp); + } } void DfgVertex::forEachSink(std::function f) const { @@ -781,6 +845,17 @@ void DfgVertex::forEachSinkEdge(std::function f) const { } } +DfgEdge* DfgVertex::findSourceEdge(std::function p) { + const auto pair = sourceEdges(); + DfgEdge* const edgesp = pair.first; + const size_t arity = pair.second; + for (size_t i = 0; i < arity; ++i) { + DfgEdge& edge = edgesp[i]; + if (p(edge, i)) return &edge; + } + return nullptr; +} + template Vertex* DfgVertex::findSink(std::function p) const { static_assert(std::is_base_of::value, diff --git a/src/V3DfgAstToDfg.cpp b/src/V3DfgAstToDfg.cpp index 798686581..763a1656d 100644 --- a/src/V3DfgAstToDfg.cpp +++ b/src/V3DfgAstToDfg.cpp @@ -35,8 +35,6 @@ #include "V3Error.h" #include "V3Global.h" -#include - VL_DEFINE_DEBUG_FUNCTIONS; namespace { @@ -88,7 +86,8 @@ class AstToDfgVisitor final : public VNVisitor { bool m_foundUnhandled = false; // Found node not implemented as DFG or not implemented 'visit' std::vector m_uncommittedVertices; // Vertices that we might decide to revert bool m_converting = false; // We are trying to convert some logic at the moment - std::vector m_varps; // All the DfgVar vertices we created. + std::vector m_varPackedps; // All the DfgVarPacked vertices we created. + std::vector m_varArrayps; // All the DfgVarArray vertices we created. // METHODS void markReferenced(AstNode* nodep) { @@ -106,18 +105,25 @@ class AstToDfgVisitor final : public VNVisitor { m_uncommittedVertices.clear(); } - DfgVar* getNet(AstVar* varp) { + DfgVertexLValue* getNet(AstVar* varp) { if (!varp->user1p()) { - // Note DfgVar vertices are not added to m_uncommittedVertices, because we want to - // hold onto them via AstVar::user1p, and the AstVar which might be referenced via - // multiple AstVarRef instances, so we will never revert a DfgVar once created. This - // means we can end up with DfgVar vertices in the graph which have no connections at - // all (which is fine for later processing). - DfgVar* const vtxp = new DfgVar{*m_dfgp, varp}; - m_varps.push_back(vtxp); - varp->user1p(vtxp); + // Note DfgVertexLValue vertices are not added to m_uncommittedVertices, because we + // want to hold onto them via AstVar::user1p, and the AstVar might be referenced via + // multiple AstVarRef instances, so we will never revert a DfgVertexLValue once + // created. This means we can end up with DfgVertexLValue vertices in the graph which + // have no connections at all (which is fine for later processing). + if (VN_IS(varp->dtypep()->skipRefp(), UnpackArrayDType)) { + DfgVarArray* const vtxp = new DfgVarArray{*m_dfgp, varp}; + varp->user1p(); + m_varArrayps.push_back(vtxp); + varp->user1p(vtxp); + } else { + DfgVarPacked* const vtxp = new DfgVarPacked{*m_dfgp, varp}; + m_varPackedps.push_back(vtxp); + varp->user1p(vtxp); + } } - return varp->user1u().to(); + return varp->user1u().to(); } DfgVertex* getVertex(AstNode* nodep) { @@ -150,12 +156,12 @@ class AstToDfgVisitor final : public VNVisitor { m_foundUnhandled = false; visit(vrefp); if (m_foundUnhandled) return false; - getVertex(vrefp)->as()->addDriver(flp, 0, vtxp); + getVertex(vrefp)->as()->addDriver(flp, 0, vtxp); return true; } if (AstSel* const selp = VN_CAST(nodep, Sel)) { AstVarRef* const vrefp = VN_CAST(selp->fromp(), VarRef); - AstConst* const lsbp = VN_CAST(selp->lsbp(), Const); + const AstConst* const lsbp = VN_CAST(selp->lsbp(), Const); if (!vrefp || !lsbp || !VN_IS(selp->widthp(), Const)) { ++m_ctx.m_nonRepLhs; return false; @@ -163,7 +169,20 @@ class AstToDfgVisitor final : public VNVisitor { m_foundUnhandled = false; visit(vrefp); if (m_foundUnhandled) return false; - getVertex(vrefp)->as()->addDriver(flp, lsbp->toUInt(), vtxp); + getVertex(vrefp)->as()->addDriver(flp, lsbp->toUInt(), vtxp); + return true; + } + if (AstArraySel* const selp = VN_CAST(nodep, ArraySel)) { + AstVarRef* const vrefp = VN_CAST(selp->fromp(), VarRef); + const AstConst* const idxp = VN_CAST(selp->bitp(), Const); + if (!vrefp || !idxp) { + ++m_ctx.m_nonRepLhs; + return false; + } + m_foundUnhandled = false; + visit(vrefp); + if (m_foundUnhandled) return false; + getVertex(vrefp)->as()->addDriver(flp, idxp->toUInt(), vtxp); return true; } if (AstConcat* const concatp = VN_CAST(nodep, Concat)) { @@ -197,6 +216,15 @@ class AstToDfgVisitor final : public VNVisitor { bool convertEquation(AstNode* nodep, AstNode* lhsp, AstNode* rhsp) { UASSERT_OBJ(m_uncommittedVertices.empty(), nodep, "Should not nest"); + // Currently cannot handle direct assignments between unpacked types. These arise e.g. + // when passing an unpacked array through a module port. + if (!DfgVertex::isSupportedPackedDType(lhsp->dtypep()) + || !DfgVertex::isSupportedPackedDType(rhsp->dtypep())) { + markReferenced(nodep); + ++m_ctx.m_nonRepDType; + return false; + } + // Cannot handle mismatched widths. Mismatched assignments should have been fixed up in // earlier passes anyway, so this should never be hit, but being paranoid just in case. if (lhsp->width() != rhsp->width()) { // LCOV_EXCL_START @@ -233,6 +261,73 @@ class AstToDfgVisitor final : public VNVisitor { return true; } + // Canonicalize packed variables + void canonicalizePacked() { + for (DfgVarPacked* const varp : m_varPackedps) { + // Gather (and unlink) all drivers + struct Driver { + FileLine* flp; + uint32_t lsb; + DfgVertex* vtxp; + Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) + : flp{flp} + , lsb{lsb} + , vtxp{vtxp} {} + }; + std::vector drivers; + drivers.reserve(varp->arity()); + varp->forEachSourceEdge([varp, &drivers](DfgEdge& edge, size_t idx) { + UASSERT(edge.sourcep(), "Should not have created undriven sources"); + drivers.emplace_back(varp->driverFileLine(idx), varp->driverLsb(idx), + edge.sourcep()); + edge.unlinkSource(); + }); + + // Sort drivers by LSB + std::stable_sort(drivers.begin(), drivers.end(), + [](const Driver& a, const Driver& b) { return a.lsb < b.lsb; }); + + // TODO: bail on multidriver + + // Coalesce adjacent ranges + for (size_t i = 0, j = 1; j < drivers.size(); ++j) { + Driver& a = drivers[i]; + Driver& b = drivers[j]; + + // Coalesce adjacent range + const uint32_t aWidth = a.vtxp->width(); + const uint32_t bWidth = b.vtxp->width(); + if (a.lsb + aWidth == b.lsb) { + const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth); + DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.flp, dtypep}; + concatp->rhsp(a.vtxp); + concatp->lhsp(b.vtxp); + a.vtxp = concatp; + b.vtxp = nullptr; // Mark as moved + ++m_ctx.m_coalescedAssignments; + continue; + } + + ++i; + + // Compact non-adjacent ranges within the vector + if (j != i) { + Driver& c = drivers[i]; + UASSERT_OBJ(!c.vtxp, c.flp, "Should have been marked moved"); + c = b; + b.vtxp = nullptr; // Mark as moved + } + } + + // Reinsert sources in order + varp->resetSources(); + for (const Driver& driver : drivers) { + if (!driver.vtxp) break; // Stop at end of cmpacted list + varp->addDriver(driver.flp, driver.lsb, driver.vtxp); + } + } + } + // VISITORS void visit(AstNode* nodep) override { // Conservatively treat this node as unhandled @@ -298,7 +393,7 @@ class AstToDfgVisitor final : public VNVisitor { nodep->user1p(vtxp); } - // The rest of the 'visit' methods are generated by 'astgen' +// The rest of the 'visit' methods are generated by 'astgen' #include "V3Dfg__gen_ast_to_dfg.h" // CONSTRUCTOR @@ -309,70 +404,8 @@ class AstToDfgVisitor final : public VNVisitor { iterateChildren(&module); UASSERT_OBJ(m_uncommittedVertices.empty(), &module, "Uncommitted vertices remain"); - // Canonicalize variable assignments - for (DfgVar* const varp : m_varps) { - // Gather (and unlink) all drivers - struct Driver { - FileLine* flp; - uint32_t lsb; - DfgVertex* vtxp; - Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) - : flp{flp} - , lsb{lsb} - , vtxp{vtxp} {} - }; - std::vector drivers; - drivers.reserve(varp->arity()); - varp->forEachSourceEdge([varp, &drivers](DfgEdge& edge, size_t idx) { - UASSERT(edge.sourcep(), "Should not have created undriven sources"); - drivers.emplace_back(varp->driverFileLine(idx), varp->driverLsb(idx), - edge.sourcep()); - edge.unlinkSource(); - }); - - // Sort drivers by LSB - std::stable_sort(drivers.begin(), drivers.end(), - [](const Driver& a, const Driver& b) { return a.lsb < b.lsb; }); - - // TODO: bail on multidriver - - // Coalesce adjacent ranges - for (size_t i = 0, j = 1; j < drivers.size(); ++j) { - Driver& a = drivers[i]; - Driver& b = drivers[j]; - - // Coalesce adjacent range - const uint32_t aWidth = a.vtxp->width(); - const uint32_t bWidth = b.vtxp->width(); - if (a.lsb + aWidth == b.lsb) { - const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth); - DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.flp, dtypep}; - concatp->rhsp(a.vtxp); - concatp->lhsp(b.vtxp); - a.vtxp = concatp; - b.vtxp = nullptr; // Mark as moved - ++m_ctx.m_coalescedAssignments; - continue; - } - - ++i; - - // Compact non-adjacent ranges within the vector - if (j != i) { - Driver& c = drivers[i]; - UASSERT_OBJ(!c.vtxp, c.flp, "Should have been marked moved"); - c = b; - b.vtxp = nullptr; // Mark as moved - } - } - - // Reinsert sources in order - varp->resetSources(); - for (const Driver& driver : drivers) { - if (!driver.vtxp) break; // Stop at end of cmpacted list - varp->addDriver(driver.flp, driver.lsb, driver.vtxp); - } - } + // Canonicalize variables + canonicalizePacked(); } public: diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index 8a0c998c9..8c9e13b91 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -15,14 +15,14 @@ //************************************************************************* // // Convert DfgGraph back to AstModule. We recursively construct AstNodeMath expressions for each -// DfgVertex which represents a storage location (e.g.: DfgVar), or has multiple sinks without -// driving a storage location (and hence needs a temporary variable to duplication). The recursion -// stops when we reach a DfgVertex representing a storage location (e.g.: DfgVar), or a vertex that -// that has multiple sinks (as these nodes will have a [potentially new temporary] corresponding -// storage location). Redundant variables (those whose source vertex drives multiple variables) are -// eliminated when possible. Vertices driving multiple variables are rendered once, driving an -// arbitrarily (but deterministically) chosen canonical variable, and the corresponding redundant -// variables are assigned from the canonical variable. +// DfgVertex which represents a storage location (e.g.: DfgVarPacked), or has multiple sinks +// without driving a storage location (and hence needs a temporary variable to duplication). The +// recursion stops when we reach a DfgVertex representing a storage location (e.g.: DfgVarPacked), +// or a vertex that that has multiple sinks (as these nodes will have a [potentially new temporary] +// corresponding// storage location). Redundant variables (those whose source vertex drives +// multiple variables) are eliminated when possible. Vertices driving multiple variables are +// rendered once, driving an arbitrarily (but deterministically) chosen canonical variable, and the +// corresponding redundant variables are assigned from the canonical variable. // //************************************************************************* @@ -110,6 +110,11 @@ AstSliceSel* makeNode( } // namespace class DfgToAstVisitor final : DfgVisitor { + // NODE STATE + // AstVar::user1() bool: this is a temporary we are introducing + + const VNUser1InUse m_inuser1; + // STATE AstModule* const m_modp; // The parent/result module @@ -124,9 +129,9 @@ class DfgToAstVisitor final : DfgVisitor { // METHODS - // Given a DfgVar, return the canonical AstVar that can be used for this DfgVar. + // Given a DfgVarPacked, return the canonical AstVar that can be used for this DfgVarPacked. // Also builds the m_canonVars map as a side effect. - AstVar* getCanonicalVar(const DfgVar* vtxp) { + AstVar* getCanonicalVar(const DfgVarPacked* vtxp) { // If variable driven (at least partially) outside the DFG, then we have no choice if (!vtxp->isDrivenFullyByDfg()) return vtxp->varp(); @@ -135,24 +140,25 @@ class DfgToAstVisitor final : DfgVisitor { if (it != m_canonVars.end()) return it->second; // Not known yet, compute it (for all vars driven fully from the same driver) - std::vector varps; + std::vector varps; vtxp->source(0)->forEachSink([&](const DfgVertex& vtx) { - if (const DfgVar* const varVtxp = vtx.cast()) { + if (const DfgVarPacked* const varVtxp = vtx.cast()) { if (varVtxp->isDrivenFullyByDfg()) varps.push_back(varVtxp); } }); UASSERT_OBJ(!varps.empty(), vtxp, "The input vtxp is always available"); - std::stable_sort(varps.begin(), varps.end(), [](const DfgVar* ap, const DfgVar* bp) { - if (ap->hasExtRefs() != bp->hasExtRefs()) return ap->hasExtRefs(); - const FileLine& aFl = *(ap->fileline()); - const FileLine& bFl = *(bp->fileline()); - if (const int cmp = aFl.operatorCompare(bFl)) return cmp < 0; - return ap->varp()->name() < bp->varp()->name(); - }); + std::stable_sort(varps.begin(), varps.end(), + [](const DfgVarPacked* ap, const DfgVarPacked* bp) { + if (ap->hasExtRefs() != bp->hasExtRefs()) return ap->hasExtRefs(); + const FileLine& aFl = *(ap->fileline()); + const FileLine& bFl = *(bp->fileline()); + if (const int cmp = aFl.operatorCompare(bFl)) return cmp < 0; + return ap->varp()->name() < bp->varp()->name(); + }); AstVar* const canonVarp = varps.front()->varp(); // Add results to map - for (const DfgVar* const varp : varps) m_canonVars.emplace(varp->varp(), canonVarp); + for (const DfgVarPacked* const varp : varps) m_canonVars.emplace(varp->varp(), canonVarp); // Return it return canonVarp; @@ -164,18 +170,21 @@ class DfgToAstVisitor final : DfgVisitor { const auto pair = m_resultVars.emplace(vtxp, nullptr); AstVar*& varp = pair.first->second; if (pair.second) { - // If this vertex is a DfgVar, then we know the variable. If this node is not a DfgVar, - // then first we try to find a DfgVar driven by this node, and use that, otherwise we - // create a temporary - if (const DfgVar* const thisDfgVarp = vtxp->cast()) { - // This is a DfgVar - varp = getCanonicalVar(thisDfgVarp); - } else if (const DfgVar* const sinkDfgVarp = vtxp->findSink( - [](const DfgVar& var) { return var.isDrivenFullyByDfg(); })) { - // We found a DfgVar driven fully by this node - varp = getCanonicalVar(sinkDfgVarp); + // If this vertex is a DfgVarPacked, then we know the variable. If this node is not a + // DfgVarPacked, then first we try to find a DfgVarPacked driven by this node, and use + // that, otherwise we create a temporary + if (const DfgVarPacked* const thisDfgVarPackedp = vtxp->cast()) { + // This is a DfgVarPacked + varp = getCanonicalVar(thisDfgVarPackedp); + } else if (const DfgVarArray* const thisDfgVarArrayp = vtxp->cast()) { + // This is a DfgVarArray + varp = thisDfgVarArrayp->varp(); + } else if (const DfgVarPacked* const sinkDfgVarPackedp = vtxp->findSink( + [](const DfgVarPacked& var) { return var.isDrivenFullyByDfg(); })) { + // We found a DfgVarPacked driven fully by this node + varp = getCanonicalVar(sinkDfgVarPackedp); } else { - // No DfgVar driven fully by this node. Create a temporary. + // No DfgVarPacked driven fully by this node. Create a temporary. // TODO: should we reuse parts when the AstVar is used as an rvalue? const string name = m_tmpNames.get(vtxp->hash(m_hashCache).toString()); // Note: It is ok for these temporary variables to be always unsigned. They are @@ -184,6 +193,7 @@ class DfgToAstVisitor final : DfgVisitor { AstNodeDType* const dtypep = v3Global.rootp()->findBitDType( vtxp->width(), vtxp->width(), VSigning::UNSIGNED); varp = new AstVar{vtxp->fileline(), VVarType::MODULETEMP, name, dtypep}; + varp->user1(true); // Mark as temporary // Add temporary AstVar to containing module m_modp->addStmtsp(varp); } @@ -201,18 +211,30 @@ class DfgToAstVisitor final : DfgVisitor { return resultp; } + bool inlineVertex(DfgVertex& vtx) { + // Inline vertices that drive only a single node, or are special + if (!vtx.hasMultipleSinks()) return true; + if (vtx.is()) return true; + if (vtx.is()) return true; + if (vtx.is()) return true; + if (const DfgArraySel* const selp = vtx.cast()) { + return selp->bitp()->is(); + } + return false; + } + AstNodeMath* convertSource(DfgVertex* vtxp) { - if (vtxp->hasMultipleSinks()) { - // Vertices with multiple sinks need a temporary variable, just return a reference - return new AstVarRef{vtxp->fileline(), getResultVar(vtxp), VAccess::READ}; - } else { - // Vertex with single sink is simply recursively converted + if (inlineVertex(*vtxp)) { + // Inlined vertices are simply recursively converted UASSERT_OBJ(vtxp->hasSinks(), vtxp, "Must have one sink: " << vtxp->typeName()); return convertDfgVertexToAstNodeMath(vtxp); + } else { + // Vertices that are not inlined need a variable, just return a reference + return new AstVarRef{vtxp->fileline(), getResultVar(vtxp), VAccess::READ}; } } - void convertCanonicalVarDriver(const DfgVar* dfgVarp) { + void convertCanonicalVarDriver(const DfgVarPacked* dfgVarp) { const auto wRef = [dfgVarp]() { return new AstVarRef{dfgVarp->fileline(), dfgVarp->varp(), VAccess::WRITE}; }; @@ -237,7 +259,7 @@ class DfgToAstVisitor final : DfgVisitor { } } - void convertDuplicateVarDriver(const DfgVar* dfgVarp, AstVar* canonVarp) { + void convertDuplicateVarDriver(const DfgVarPacked* dfgVarp, AstVar* canonVarp) { const auto rRef = [canonVarp]() { return new AstVarRef{canonVarp->fileline(), canonVarp, VAccess::READ}; }; @@ -263,6 +285,22 @@ class DfgToAstVisitor final : DfgVisitor { } } + void convertArrayDiver(const DfgVarArray* dfgVarp) { + // Variable is driven partially. Asign from parts of the canonical var. + dfgVarp->forEachSourceEdge([&](const DfgEdge& edge, size_t idx) { + UASSERT_OBJ(edge.sourcep(), dfgVarp, "Should have removed undriven sources"); + // Render the rhs expression + AstNodeMath* const rhsp = convertDfgVertexToAstNodeMath(edge.sourcep()); + // Create select LValue + FileLine* const flp = dfgVarp->driverFileLine(idx); + AstVarRef* const refp = new AstVarRef{flp, dfgVarp->varp(), VAccess::WRITE}; + AstConst* const idxp = new AstConst{flp, dfgVarp->driverIndex(idx)}; + AstArraySel* const lhsp = new AstArraySel{flp, refp, idxp}; + // Add assignment of the value to the selected bits + addResultEquation(flp, lhsp, rhsp); + }); + } + void addResultEquation(FileLine* flp, AstNode* lhsp, AstNode* rhsp) { m_modp->addStmtsp(new AstAssignW{flp, lhsp, rhsp}); ++m_ctx.m_resultEquations; @@ -273,10 +311,14 @@ class DfgToAstVisitor final : DfgVisitor { vtxp->v3fatal("Unhandled DfgVertex: " << vtxp->typeName()); } // LCOV_EXCL_STOP - void visit(DfgVar* vtxp) override { + void visit(DfgVarPacked* vtxp) override { m_resultp = new AstVarRef{vtxp->fileline(), getCanonicalVar(vtxp), VAccess::READ}; } + void visit(DfgVarArray* vtxp) override { + m_resultp = new AstVarRef{vtxp->fileline(), vtxp->varp(), VAccess::READ}; + } + void visit(DfgConst* vtxp) override { // m_resultp = vtxp->constp()->cloneTree(false); } @@ -293,13 +335,13 @@ class DfgToAstVisitor final : DfgVisitor { // Convert vertices back to assignments dfg.forEachVertex([&](DfgVertex& vtx) { - // Render variable assignments - if (const DfgVar* const dfgVarp = vtx.cast()) { - // DfgVar instances (these might be driving the given AstVar variable) - // If there is no driver (i.e.: this DfgVar is an input to the Dfg), then + // Render packed variable assignments + if (const DfgVarPacked* const dfgVarp = vtx.cast()) { + // DfgVarPacked instances (these might be driving the given AstVar variable) + // If there is no driver (i.e.: this DfgVarPacked is an input to the Dfg), then // nothing to do if (!dfgVarp->isDrivenByDfg()) return; - // The driver of this DfgVar might drive multiple variables. Only emit one + // The driver of this DfgVarPacked might drive multiple variables. Only emit one // assignment from the driver to an arbitrarily chosen canonical variable, and // assign the other variables from that canonical variable AstVar* const canonVarp = getCanonicalVar(dfgVarp); @@ -318,23 +360,31 @@ class DfgToAstVisitor final : DfgVisitor { return; } - // Vertices driving a single vertex will be in-lined by 'convertDfgVertexToAstNodeMath' - if (!vtx.hasMultipleSinks()) return; + // Render array variable assignments + if (const DfgVarArray* dfgVarp = vtx.cast()) { + // If there is no driver, then there is nothing to do + if (!dfgVarp->isDrivenByDfg()) return; + // We don't canonicalize arrays, so just render the drivers + convertArrayDiver(dfgVarp); + // + return; + } - // Vertices with multiple sinks needs a temporary if they do not fully drive a DfgVar - const bool needsTemporary = !vtx.findSink([](const DfgVar& var) { // - return var.isDrivenFullyByDfg(); - }); - if (needsTemporary) { - // DfgVertex that has multiple sinks, but does not drive a DfgVar (needs temporary) + // If the vertex is known to be inlined, then nothing else to do + if (inlineVertex(vtx)) return; + + // Check if this uses a temporary, vs one of the vars rendered above + AstVar* const resultVarp = getResultVar(&vtx); + if (resultVarp->user1()) { + // We introduced a temporary for this DfgVertex ++m_ctx.m_intermediateVars; + FileLine* const flp = vtx.fileline(); // Just render the logic AstNodeMath* const rhsp = convertDfgVertexToAstNodeMath(&vtx); // The lhs is a temporary - AstNodeMath* const lhsp - = new AstVarRef{vtx.fileline(), getResultVar(&vtx), VAccess::WRITE}; + AstNodeMath* const lhsp = new AstVarRef{flp, resultVarp, VAccess::WRITE}; // Add assignment of the value to the variable - addResultEquation(vtx.fileline(), lhsp, rhsp); + addResultEquation(flp, lhsp, rhsp); } }); diff --git a/src/V3DfgPasses.cpp b/src/V3DfgPasses.cpp index 77e8392ca..0c975fa98 100644 --- a/src/V3DfgPasses.cpp +++ b/src/V3DfgPasses.cpp @@ -103,8 +103,8 @@ void V3DfgPasses::cse(DfgGraph& dfg, V3DfgCseContext& ctx) { void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { dfg.forEachVertex([&](DfgVertex& vtx) { - // We can eliminate certain redundant DfgVar vertices - DfgVar* const varp = vtx.cast(); + // We can eliminate certain redundant DfgVarPacked vertices + DfgVarPacked* const varp = vtx.cast(); if (!varp) return; // Can't remove if it has consumers @@ -125,9 +125,9 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { if (varp->isDrivenByDfg()) { DfgVertex* const driverp = varp->source(0); unsigned nonVarSinks = 0; - const DfgVar* firstSinkVarp = nullptr; + const DfgVarPacked* firstSinkVarp = nullptr; const bool keepFirst = driverp->findSink([&](const DfgVertex& sink) { - if (const DfgVar* const sinkVarp = sink.cast()) { + if (const DfgVarPacked* const sinkVarp = sink.cast()) { if (!firstSinkVarp) firstSinkVarp = sinkVarp; } else { ++nonVarSinks; @@ -135,11 +135,11 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { // We can stop as soon as we found the first var, and 2 non-var sinks return firstSinkVarp && nonVarSinks >= 2; }); - // Keep this DfgVar if needed + // Keep this DfgVarPacked if needed if (keepFirst && firstSinkVarp == varp) return; } - // OK, we can delete this DfgVar + // OK, we can delete this DfgVarPacked ++ctx.m_removed; // If not referenced outside the DFG, then also delete the referenced AstVar, @@ -154,7 +154,7 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { void V3DfgPasses::removeUnused(DfgGraph& dfg) { const auto processVertex = [&](DfgVertex& vtx) { // Keep variables - if (vtx.is()) return false; + if (vtx.is() || vtx.is()) return false; // Keep if it has sinks if (vtx.hasSinks()) return false; // Unlink and delete vertex diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 12804f137..19f98314c 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -120,9 +120,9 @@ class V3DfgPeephole final : public DfgVisitor { } // If both sides are variable references, order the side in some defined way. This allows // CSE to later merge 'a op b' with 'b op a'. - if (lhsp->is() && rhsp->is()) { - AstVar* const lVarp = lhsp->as()->varp(); - AstVar* const rVarp = rhsp->as()->varp(); + if (lhsp->is() && rhsp->is()) { + AstVar* const lVarp = lhsp->as()->varp(); + AstVar* const rVarp = rhsp->as()->varp(); if (lVarp->name() > rVarp->name()) { APPLYING(SWAP_VAR_IN_COMMUTATIVE_BINARY) { vtxp->lhsp(rhsp); @@ -1102,7 +1102,7 @@ class V3DfgPeephole final : public DfgVisitor { } } - void visit(DfgVar* vtxp) override { + void visit(DfgVarPacked* vtxp) override { // Inline variables fully driven by the logic represented by the DFG if (vtxp->hasSinks() && vtxp->isDrivenFullyByDfg()) { APPLYING(INLINE_VAR) { @@ -1122,7 +1122,7 @@ class V3DfgPeephole final : public DfgVisitor { // multiple sinks (otherwise we would need to introduce a temporary, but it is better for // debugging to keep the original variable name, if one is available), so we can't remove // redundant variables here. - const bool keep = vtx.is(); + const bool keep = vtx.is() || vtx.is(); // If it has no sinks (unused), we can remove it if (!keep && !vtx.hasSinks()) {