From a8dca71ed05a66614093de96cfa62bf93faaeab3 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Mon, 21 Jul 2025 17:33:12 +0100 Subject: [PATCH] Support more complex combinational assignments in DFG. (#6205) Previously DFG was limited to having a Sel, or an ArraySel potentially under a Concat on the LHS of combinational assignments. Other forms or combinations were not representable in the graph. This adds support for arbitrary combinations of the above by combining DfgSplicePacked and DfgSpliceArray vertices introduced in #6176. In particular, Sel(ArraySel(VarRef,_),_) enables a lot more code to be represented in DFG. --- src/V3Dfg.cpp | 26 +- src/V3DfgAstToDfg.cpp | 665 ++++++++++++------- src/V3DfgDfgToAst.cpp | 103 ++- src/V3DfgPeephole.cpp | 13 +- src/V3DfgRegularize.cpp | 2 +- src/V3DfgVertices.h | 31 +- test_regress/t/t_dfg_break_cycles.py | 3 + test_regress/t/t_dfg_multidriver_dfg_bad.out | 98 ++- test_regress/t/t_dfg_multidriver_dfg_bad.v | 28 +- test_regress/t/t_dfg_peephole.cpp | 10 +- test_regress/t/t_dfg_peephole.py | 9 +- test_regress/t/t_dfg_peephole.v | 24 +- test_regress/t/t_inst_array_partial.v | 2 + 13 files changed, 658 insertions(+), 356 deletions(-) diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 613b29e7d..04679edd8 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -246,8 +246,9 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { AstNode* const nodep = varVtxp->nodep(); AstVar* const varp = varVtxp->varp(); os << toDotId(vtx); - os << " [label=\"" << nodep->name() << "\nW" << varVtxp->width() << " / F" - << varVtxp->fanout() << '"'; + os << " [label=\"" << nodep->name() << "\n"; + varVtxp->dtypep()->dumpSmall(os); + os << " / F" << varVtxp->fanout() << '"'; if (varp->direction() == VDirection::INPUT) { os << ", shape=box, style=filled, fillcolor=chartreuse2"; // Green @@ -273,9 +274,10 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { if (const DfgVarArray* const arrVtxp = vtx.cast()) { AstNode* const nodep = arrVtxp->nodep(); AstVar* const varp = arrVtxp->varp(); - const int elements = VN_AS(arrVtxp->dtypep(), UnpackArrayDType)->elementsConst(); os << toDotId(vtx); - os << " [label=\"" << nodep->name() << "[" << elements << "]\""; + os << " [label=\"" << nodep->name() << "\n"; + arrVtxp->dtypep()->dumpSmall(os); + os << " / F" << arrVtxp->fanout() << '"'; if (varp->direction() == VDirection::INPUT) { os << ", shape=box3d, style=filled, fillcolor=chartreuse2"; // Green } else if (varp->direction() == VDirection::OUTPUT) { @@ -318,8 +320,9 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { const uint32_t lsb = selVtxp->lsb(); const uint32_t msb = lsb + selVtxp->width() - 1; os << toDotId(vtx); - os << " [label=\"SEL\n_[" << msb << ":" << lsb << "]\nW" << vtx.width() << " / F" - << vtx.fanout() << '"'; + os << " [label=\"SEL\n_[" << msb << ":" << lsb << "]\n"; + vtx.dtypep()->dumpSmall(os); + os << " / F" << vtx.fanout() << '"'; if (vtx.hasMultipleSinks()) { os << ", shape=doublecircle"; } else { @@ -332,12 +335,7 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { if (vtx.is()) { os << toDotId(vtx); os << " [label=\"" << vtx.typeName() << "\n"; - if (const DfgSpliceArray* const sp = vtx.cast()) { - const int elements = VN_AS(sp->dtypep(), UnpackArrayDType)->elementsConst(); - os << "_[" << elements << "]"; - } else { - os << "W" << vtx.width(); - } + vtx.dtypep()->dumpSmall(os); os << " / F" << vtx.fanout() << '"'; if (vtx.hasMultipleSinks()) { os << ", shape=doubleoctagon"; @@ -349,7 +347,9 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { } os << toDotId(vtx); - os << " [label=\"" << vtx.typeName() << "\nW" << vtx.width() << " / F" << vtx.fanout() << '"'; + os << " [label=\"" << vtx.typeName() << "\n"; + vtx.dtypep()->dumpSmall(os); + os << " / F" << vtx.fanout() << '"'; if (vtx.hasMultipleSinks()) { os << ", shape=doublecircle"; } else { diff --git a/src/V3DfgAstToDfg.cpp b/src/V3DfgAstToDfg.cpp index dc121cf00..8691b1a53 100644 --- a/src/V3DfgAstToDfg.cpp +++ b/src/V3DfgAstToDfg.cpp @@ -28,9 +28,12 @@ #include "V3PchAstNoMT.h" // VL_MT_DISABLED_CODE_UNIT +#include "V3Const.h" #include "V3Dfg.h" #include "V3DfgPasses.h" +#include + VL_DEFINE_DEBUG_FUNCTIONS; namespace { @@ -42,6 +45,17 @@ T_Vertex* makeVertex(const T_Node* nodep, DfgGraph& dfg) { return new T_Vertex{dfg, nodep->fileline(), DfgVertex::dtypeFor(nodep)}; } +template <> +DfgArraySel* makeVertex(const AstArraySel* nodep, DfgGraph& dfg) { + // Some earlier passes create malformed ArraySels, just bail on those... + // See t_bitsel_wire_array_bad + if (VN_IS(nodep->fromp(), Const)) return nullptr; + AstUnpackArrayDType* const fromDtypep + = VN_CAST(nodep->fromp()->dtypep()->skipRefp(), UnpackArrayDType); + if (!fromDtypep) return nullptr; + return new DfgArraySel{dfg, nodep->fileline(), DfgVertex::dtypeFor(nodep)}; +} + //====================================================================== // Currently unhandled nodes // LCOV_EXCL_START @@ -77,17 +91,6 @@ class AstToDfgVisitor final : public VNVisitor { const VNUser1InUse m_user1InUse; // TYPES - // Represents a driver during canonicalization - struct Driver final { - FileLine* m_fileline; - DfgVertex* m_vtxp; - uint32_t m_lsb; - Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) - : m_fileline{flp} - , m_vtxp{vtxp} - , m_lsb{lsb} {} - }; - using RootType = std::conditional_t; using VariableType = std::conditional_t; @@ -183,93 +186,144 @@ class AstToDfgVisitor final : public VNVisitor { return m_foundUnhandled; } - DfgVertexSplice* convertLValue(AstNode* nodep) { - FileLine* const flp = nodep->fileline(); - + std::pair convertLValue(AstNode* nodep) { if (AstVarRef* const vrefp = VN_CAST(nodep, VarRef)) { m_foundUnhandled = false; visit(vrefp); - if (m_foundUnhandled) return nullptr; + if (m_foundUnhandled) return {nullptr, 0}; + + // Get the variable vertex DfgVertexVar* const vtxp = getVertex(vrefp)->template as(); - // Ensure driving splice vertex exists + // Ensure the Splice driver exists for this variable if (!vtxp->srcp()) { - if (VN_IS(vtxp->dtypep(), UnpackArrayDType)) { - vtxp->srcp(new DfgSpliceArray{*m_dfgp, flp, vtxp->dtypep()}); + FileLine* const flp = vtxp->fileline(); + AstNodeDType* const dtypep = vtxp->dtypep(); + if (vtxp->is()) { + vtxp->srcp(new DfgSplicePacked{*m_dfgp, flp, dtypep}); + } else if (vtxp->is()) { + vtxp->srcp(new DfgSpliceArray{*m_dfgp, flp, dtypep}); } else { - vtxp->srcp(new DfgSplicePacked{*m_dfgp, flp, vtxp->dtypep()}); + nodep->v3fatalSrc("Unhandled DfgVertexVar sub-type"); // LCOV_EXCL_LINE } } - return vtxp->srcp()->as(); + // Return the Splice driver + return {vtxp->srcp()->as(), 0}; + } + + if (AstSel* selp = VN_CAST(nodep, Sel)) { + // Only handle constant selects + const AstConst* const lsbp = VN_CAST(selp->lsbp(), Const); + if (!lsbp) { + ++m_ctx.m_nonRepLhs; + return {nullptr, 0}; + } + uint32_t lsb = lsbp->toUInt(); + + // Convert the 'fromp' sub-expression + const auto pair = convertLValue(selp->fromp()); + if (!pair.first) return {nullptr, 0}; + DfgSplicePacked* const splicep = pair.first->template as(); + // Adjust index. + lsb += pair.second; + + // AstSel doesn't change type kind (array vs packed), so we can use + // the existing splice driver with adjusted lsb + return {splicep, lsb}; + } + + if (AstArraySel* const aselp = VN_CAST(nodep, ArraySel)) { + // Only handle constant selects + const AstConst* const indexp = VN_CAST(aselp->bitp(), Const); + if (!indexp) { + ++m_ctx.m_nonRepLhs; + return {nullptr, 0}; + } + uint32_t index = indexp->toUInt(); + + // Convert the 'fromp' sub-expression + const auto pair = convertLValue(aselp->fromp()); + if (!pair.first) return {nullptr, 0}; + DfgSpliceArray* const splicep = pair.first->template as(); + // Adjust index. Note pair.second is always 0, but we might handle array slices later.. + index += pair.second; + + // Ensure the Splice driver exists for this element + if (!splicep->driverAt(index)) { + FileLine* const flp = nodep->fileline(); + AstNodeDType* const dtypep = DfgVertex::dtypeFor(nodep); + if (VN_IS(dtypep, BasicDType)) { + splicep->addDriver(flp, index, new DfgSplicePacked{*m_dfgp, flp, dtypep}); + } else if (VN_IS(dtypep, UnpackArrayDType)) { + splicep->addDriver(flp, index, new DfgSpliceArray{*m_dfgp, flp, dtypep}); + } else { + nodep->v3fatalSrc("Unhandled AstNodeDType sub-type"); // LCOV_EXCL_LINE + } + } + + // Return the splice driver + return {splicep->driverAt(index)->as(), 0}; } ++m_ctx.m_nonRepLhs; - return nullptr; + return {nullptr, 0}; } // Build DfgEdge representing the LValue assignment. Returns false if unsuccessful. - bool convertAssignment(FileLine* flp, AstNode* nodep, DfgVertex* vtxp) { + bool convertAssignment(FileLine* flp, AstNode* lhsp, DfgVertex* vtxp) { + // Simplify the LHS, to get rid of things like SEL(CONCAT(_, _), _) + lhsp = V3Const::constifyExpensiveEdit(lhsp); + // Concatenation on the LHS. Select parts of the driving 'vtxp' then convert each part - if (AstConcat* const concatp = VN_CAST(nodep, Concat)) { - AstNode* const lhsp = concatp->lhsp(); - AstNode* const rhsp = concatp->rhsp(); + if (AstConcat* const concatp = VN_CAST(lhsp, Concat)) { + AstNode* const cLhsp = concatp->lhsp(); + AstNode* const cRhsp = concatp->rhsp(); { // Convet LHS of concat - FileLine* const lFlp = lhsp->fileline(); - DfgSel* const lVtxp = new DfgSel{*m_dfgp, lFlp, DfgVertex::dtypeFor(lhsp)}; + FileLine* const lFlp = cLhsp->fileline(); + DfgSel* const lVtxp = new DfgSel{*m_dfgp, lFlp, DfgVertex::dtypeFor(cLhsp)}; lVtxp->fromp(vtxp); - lVtxp->lsb(rhsp->width()); - if (!convertAssignment(flp, lhsp, lVtxp)) return false; + lVtxp->lsb(cRhsp->width()); + if (!convertAssignment(flp, cLhsp, lVtxp)) return false; } { // Convert RHS of concat - FileLine* const rFlp = rhsp->fileline(); - DfgSel* const rVtxp = new DfgSel{*m_dfgp, rFlp, DfgVertex::dtypeFor(rhsp)}; + FileLine* const rFlp = cRhsp->fileline(); + DfgSel* const rVtxp = new DfgSel{*m_dfgp, rFlp, DfgVertex::dtypeFor(cRhsp)}; rVtxp->fromp(vtxp); rVtxp->lsb(0); - return convertAssignment(flp, rhsp, rVtxp); + return convertAssignment(flp, cRhsp, rVtxp); } } - if (AstSel* const selp = VN_CAST(nodep, Sel)) { - AstVarRef* const vrefp = VN_CAST(selp->fromp(), VarRef); - const AstConst* const lsbp = VN_CAST(selp->lsbp(), Const); - if (!vrefp || !lsbp) { - ++m_ctx.m_nonRepLhs; - return false; - } - if (DfgVertexSplice* const splicep = convertLValue(vrefp)) { - splicep->template as()->addDriver(flp, lsbp->toUInt(), vtxp); - return true; - } - } else 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; - } - if (DfgVertexSplice* const splicep = convertLValue(vrefp)) { - splicep->template as()->addDriver(flp, idxp->toUInt(), vtxp); - return true; - } - } else if (VN_IS(nodep, VarRef)) { - if (DfgVertexSplice* const splicep = convertLValue(nodep)) { - splicep->template as()->addDriver(flp, 0, vtxp); - return true; - } + // Construct LHS + const auto pair = convertLValue(lhsp); + if (!pair.first) return false; + + // If successful connect the driver + if (DfgSplicePacked* const sPackedp = pair.first->template cast()) { + sPackedp->addDriver(flp, pair.second, vtxp); + } else if (DfgSpliceArray* const sArrayp = pair.first->template cast()) { + sArrayp->addDriver(flp, pair.second, vtxp); } else { - ++m_ctx.m_nonRepLhs; + lhsp->v3fatalSrc("Unhandled DfgVertexSplice sub-type"); // LCOV_EXCL_LINE } - return false; + + return true; } bool convertEquation(AstNode* nodep, FileLine* flp, 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())) { + // Check data types are compatible. + if (!DfgVertex::isSupportedDType(lhsp->dtypep()) + || !DfgVertex::isSupportedDType(rhsp->dtypep())) { + markReferenced(nodep); + ++m_ctx.m_nonRepDType; + return false; + } + + // For now, only direct array assignment is supported (e.g. a = b, but not a = _ ? b : c) + if (VN_IS(rhsp->dtypep()->skipRefp(), UnpackArrayDType) && !VN_IS(rhsp, VarRef)) { markReferenced(nodep); ++m_ctx.m_nonRepDType; return false; @@ -312,180 +366,310 @@ class AstToDfgVisitor final : public VNVisitor { return true; } - // Sometime assignment ranges are coalesced by V3Const, - // so we unpack concatenations for better error reporting. - void addDriver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp, - std::vector& drivers) const { - if (DfgConcat* const concatp = vtxp->cast()) { - DfgVertex* const rhsp = concatp->rhsp(); - auto const rhs_width = rhsp->width(); - addDriver(rhsp->fileline(), lsb, rhsp, drivers); - DfgVertex* const lhsp = concatp->lhsp(); - addDriver(lhsp->fileline(), lsb + rhs_width, lhsp, drivers); - concatp->unlinkDelete(*m_dfgp); - } else { - drivers.emplace_back(flp, lsb, vtxp); + // Prune vertices potentially unused due to resolving multiple drivers. + // Having multiple drivers is an error and is hence assumed to be rare, + // so performance is not very important, set will suffice. + void removeUnused(std::set& prune) { + while (!prune.empty()) { + // Pop last vertex + const auto it = prune.begin(); + DfgVertex* const vtxp = *it; + prune.erase(it); + // If used (or a variable), then done + if (vtxp->hasSinks() || vtxp->is()) continue; + // If unused, then add sources to work list and delete + vtxp->forEachSource([&](DfgVertex& src) { prune.emplace(&src); }); + vtxp->unlinkDelete(*m_dfgp); } } - // Canonicalize packed variables - void canonicalizePacked() { - for (DfgVarPacked* const varp : m_varPackedps) { - // Delete variables with no sinks nor sources (this can happen due to reverting - // uncommitted vertices, which does not remove variables) - if (!varp->hasSinks() && !varp->srcp()) { - VL_DO_DANGLING(varp->unlinkDelete(*m_dfgp), varp); + // Normalize packed driver - return the normalized vertex and location for 'splicep' + std::pair // + normalizePacked(DfgVertexVar* varp, const std::string& sub, DfgSplicePacked* const splicep) { + // Represents a driver of 'splicep' + struct Driver final { + FileLine* m_fileline; + DfgVertex* m_vtxp; + uint32_t m_lsb; + Driver() = delete; + Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) + : m_fileline{flp} + , m_vtxp{vtxp} + , m_lsb{lsb} {} + }; + + // The drivers of 'splicep' + std::vector drivers; + drivers.reserve(splicep->arity()); + + // Sometime assignment ranges are coalesced by V3Const, + // so we unpack concatenations for better error reporting. + const std::function gather + = [&](FileLine* flp, uint32_t lsb, DfgVertex* vtxp) -> void { + if (DfgConcat* const concatp = vtxp->cast()) { + DfgVertex* const rhsp = concatp->rhsp(); + auto const rhs_width = rhsp->width(); + gather(rhsp->fileline(), lsb, rhsp); + DfgVertex* const lhsp = concatp->lhsp(); + gather(lhsp->fileline(), lsb + rhs_width, lhsp); + concatp->unlinkDelete(*m_dfgp); + } else { + drivers.emplace_back(flp, lsb, vtxp); + } + }; + + // Gather and unlink all drivers + splicep->forEachSourceEdge([&](DfgEdge& edge, size_t idx) { + DfgVertex* const driverp = edge.sourcep(); + UASSERT(driverp, "Should not have created undriven sources"); + UASSERT_OBJ(!driverp->is(), splicep, "Should not be DfgVertexSplice"); + gather(splicep->driverFileLine(idx), splicep->driverLsb(idx), driverp); + edge.unlinkSource(); + }); + + const auto cmp = [](const Driver& a, const Driver& b) { + if (a.m_lsb != b.m_lsb) return a.m_lsb < b.m_lsb; + return a.m_fileline->operatorCompare(*b.m_fileline) < 0; + }; + + // Sort drivers by LSB + std::stable_sort(drivers.begin(), drivers.end(), cmp); + + // Vertices that might have become unused due to multiple driver resolution. Having + // multiple drivers is an error and is hence assumed to be rare, so performance is + // not very important, set will suffice. + std::set prune; + + // Fix multiply driven ranges + for (auto it = drivers.begin(); it != drivers.end();) { + Driver& a = *it++; + const uint32_t aWidth = a.m_vtxp->width(); + const uint32_t aEnd = a.m_lsb + aWidth; + while (it != drivers.end()) { + Driver& b = *it; + // If no overlap, then nothing to do + if (b.m_lsb >= aEnd) break; + + const uint32_t bWidth = b.m_vtxp->width(); + const uint32_t bEnd = b.m_lsb + bWidth; + const uint32_t overlapEnd = std::min(aEnd, bEnd) - 1; + + if (a.m_fileline->operatorCompare(*b.m_fileline) != 0 + && !varp->varp()->isUsedLoopIdx() // Loop index often abused, so suppress + ) { + AstNode* const vp = varp->varScopep() + ? static_cast(varp->varScopep()) + : static_cast(varp->varp()); + + vp->v3warn( // + MULTIDRIVEN, + "Bits [" // + << overlapEnd << ":" << b.m_lsb << "] of signal '" << vp->prettyName() + << sub << "' have multiple combinational drivers\n" + << a.m_fileline->warnOther() << "... Location of first driver\n" + << a.m_fileline->warnContextPrimary() << '\n' + << b.m_fileline->warnOther() << "... Location of other driver\n" + << b.m_fileline->warnContextSecondary() << vp->warnOther() + << "... Only the first driver will be respected"); + } + + // If the first driver completely covers the range of the second driver, + // we can just delete the second driver completely, otherwise adjust the + // second driver to apply from the end of the range of the first driver. + if (aEnd >= bEnd) { + prune.emplace(b.m_vtxp); + it = drivers.erase(it); + } else { + const auto dtypep = DfgVertex::dtypeForWidth(bEnd - aEnd); + DfgSel* const selp = new DfgSel{*m_dfgp, b.m_vtxp->fileline(), dtypep}; + selp->fromp(b.m_vtxp); + selp->lsb(aEnd - b.m_lsb); + b.m_lsb = aEnd; + b.m_vtxp = selp; + std::stable_sort(it, drivers.end(), cmp); + } + } + } + + // 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.m_vtxp->width(); + const uint32_t bWidth = b.m_vtxp->width(); + if (a.m_lsb + aWidth == b.m_lsb) { + const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth); + DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.m_fileline, dtypep}; + concatp->rhsp(a.m_vtxp); + concatp->lhsp(b.m_vtxp); + a.m_vtxp = concatp; + b.m_vtxp = nullptr; // Mark as moved + ++m_ctx.m_coalescedAssignments; continue; } - // Nothing to do for un-driven (input) variables - if (!varp->srcp()) continue; + ++i; - DfgSplicePacked* const splicep = varp->srcp()->as(); - - // Gather (and unlink) all drivers - std::vector drivers; - drivers.reserve(splicep->arity()); - splicep->forEachSourceEdge([this, splicep, &drivers](DfgEdge& edge, size_t idx) { - DfgVertex* const driverp = edge.sourcep(); - UASSERT(driverp, "Should not have created undriven sources"); - addDriver(splicep->driverFileLine(idx), splicep->driverLsb(idx), driverp, drivers); - edge.unlinkSource(); - }); - - const auto cmp = [](const Driver& a, const Driver& b) { - if (a.m_lsb != b.m_lsb) return a.m_lsb < b.m_lsb; - return a.m_fileline->operatorCompare(*b.m_fileline) < 0; - }; - - // Sort drivers by LSB - std::stable_sort(drivers.begin(), drivers.end(), cmp); - - // Vertices that might have become unused due to multiple driver resolution. Having - // multiple drivers is an error and is hence assumed to be rare, so performance is - // not very important, set will suffice. - std::set prune; - - // Fix multiply driven ranges - for (auto it = drivers.begin(); it != drivers.end();) { - Driver& a = *it++; - const uint32_t aWidth = a.m_vtxp->width(); - const uint32_t aEnd = a.m_lsb + aWidth; - while (it != drivers.end()) { - Driver& b = *it; - // If no overlap, then nothing to do - if (b.m_lsb >= aEnd) break; - - const uint32_t bWidth = b.m_vtxp->width(); - const uint32_t bEnd = b.m_lsb + bWidth; - const uint32_t overlapEnd = std::min(aEnd, bEnd) - 1; - - if (a.m_fileline->operatorCompare(*b.m_fileline) != 0 - && !varp->varp()->isUsedLoopIdx() // Loop index often abused, so suppress - ) { - AstNode* const vp = varp->varScopep() - ? static_cast(varp->varScopep()) - : static_cast(varp->varp()); - vp->v3warn( // - MULTIDRIVEN, - "Bits [" // - << overlapEnd << ":" << b.m_lsb << "] of signal " - << vp->prettyNameQ() << " have multiple combinational drivers\n" - << a.m_fileline->warnOther() << "... Location of first driver\n" - << a.m_fileline->warnContextPrimary() << '\n' - << b.m_fileline->warnOther() << "... Location of other driver\n" - << b.m_fileline->warnContextSecondary() << vp->warnOther() - << "... Only the first driver will be respected"); - } - - // If the first driver completely covers the range of the second driver, - // we can just delete the second driver completely, otherwise adjust the - // second driver to apply from the end of the range of the first driver. - if (aEnd >= bEnd) { - prune.emplace(b.m_vtxp); - it = drivers.erase(it); - } else { - const auto dtypep = DfgVertex::dtypeForWidth(bEnd - aEnd); - DfgSel* const selp = new DfgSel{*m_dfgp, b.m_vtxp->fileline(), dtypep}; - selp->fromp(b.m_vtxp); - selp->lsb(aEnd - b.m_lsb); - b.m_lsb = aEnd; - b.m_vtxp = selp; - std::stable_sort(it, drivers.end(), cmp); - } - } - } - - // 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.m_vtxp->width(); - const uint32_t bWidth = b.m_vtxp->width(); - if (a.m_lsb + aWidth == b.m_lsb) { - const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth); - DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.m_fileline, dtypep}; - concatp->rhsp(a.m_vtxp); - concatp->lhsp(b.m_vtxp); - a.m_vtxp = concatp; - b.m_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.m_vtxp, c.m_fileline, "Should have been marked moved"); - c = b; - b.m_vtxp = nullptr; // Mark as moved - } - } - - // Reinsert drivers in order - splicep->resetSources(); - for (const Driver& driver : drivers) { - if (!driver.m_vtxp) break; // Stop at end of compacted list - splicep->addDriver(driver.m_fileline, driver.m_lsb, driver.m_vtxp); - } - - // Prune vertices potentially unused due to resolving multiple drivers. - while (!prune.empty()) { - // Pop last vertex - const auto it = prune.begin(); - DfgVertex* const vtxp = *it; - prune.erase(it); - // If used (or a variable), then done - if (vtxp->hasSinks() || vtxp->is()) continue; - // If unused, then add sources to work list and delete - vtxp->forEachSource([&](DfgVertex& src) { prune.emplace(&src); }); - vtxp->unlinkDelete(*m_dfgp); - } - - // If the whole variable is driven, remove the splice node - if (splicep->arity() == 1 // - && splicep->driverLsb(0) == 0 // - && splicep->source(0)->width() == varp->width()) { - varp->srcp(splicep->source(0)); - varp->driverFileLine(splicep->driverFileLine(0)); - splicep->unlinkDelete(*m_dfgp); + // Compact non-adjacent ranges within the vector + if (j != i) { + Driver& c = drivers[i]; + UASSERT_OBJ(!c.m_vtxp, c.m_fileline, "Should have been marked moved"); + c = b; + b.m_vtxp = nullptr; // Mark as moved } } + + // Reinsert drivers in order + splicep->resetSources(); + for (const Driver& driver : drivers) { + if (!driver.m_vtxp) break; // Stop at end of compacted list + splicep->addDriver(driver.m_fileline, driver.m_lsb, driver.m_vtxp); + } + + removeUnused(prune); + + // If the whole variable is driven whole, we can just use that driver + if (splicep->arity() == 1 // + && splicep->driverLsb(0) == 0 // + && splicep->source(0)->width() == splicep->width()) { + const auto result = std::make_pair(splicep->source(0), splicep->driverFileLine(0)); + VL_DO_DANGLING(splicep->unlinkDelete(*m_dfgp), splicep); + return result; + } + return {splicep, splicep->fileline()}; } - // Canonicalize array variables - void canonicalizeArray() { - for (DfgVarArray* const varp : m_varArrayps) { - // Delete variables with no sinks nor sources (this can happen due to reverting - // uncommitted vertices, which does not remove variables) - if (!varp->hasSinks() && !varp->srcp()) { - VL_DO_DANGLING(varp->unlinkDelete(*m_dfgp), varp); + // Normalize array driver - return the normalized vertex and location for 'splicep' + std::pair // + normalizeArray(DfgVertexVar* varp, const std::string& sub, DfgSpliceArray* const splicep) { + // Represents a driver of 'splicep' + struct Driver final { + FileLine* m_fileline; + DfgVertex* m_vtxp; + uint32_t m_idx; + Driver() = delete; + Driver(FileLine* flp, uint32_t idx, DfgVertex* vtxp) + : m_fileline{flp} + , m_vtxp{vtxp} + , m_idx{idx} {} + }; + + // The drivers of 'splicep' + std::vector drivers; + drivers.reserve(splicep->arity()); + + // Normalize, gather, and unlink all drivers + splicep->forEachSourceEdge([&](DfgEdge& edge, size_t i) { + DfgVertex* const driverp = edge.sourcep(); + UASSERT(driverp, "Should not have created undriven sources"); + const uint32_t idx = splicep->driverIndex(i); + if (DfgSplicePacked* const spp = driverp->cast()) { + const auto pair + = normalizePacked(varp, sub + "[" + std::to_string(idx) + "]", spp); + drivers.emplace_back(pair.second, idx, pair.first); + } else if (DfgSpliceArray* const sap = driverp->cast()) { + const auto pair = normalizeArray(varp, sub + "[" + std::to_string(idx) + "]", sap); + drivers.emplace_back(pair.second, idx, pair.first); + } else if (driverp->is()) { + driverp->v3fatalSrc("Unhandled DfgVertexSplice sub-type"); + } else { + drivers.emplace_back(splicep->driverFileLine(i), idx, driverp); + } + edge.unlinkSource(); + }); + + const auto cmp = [](const Driver& a, const Driver& b) { + if (a.m_idx != b.m_idx) return a.m_idx < b.m_idx; + return a.m_fileline->operatorCompare(*b.m_fileline) < 0; + }; + + // Sort drivers by index + std::stable_sort(drivers.begin(), drivers.end(), cmp); + + // Vertices that become unused due to multiple driver resolution + std::set prune; + + // Fix multiply driven ranges + for (auto it = drivers.begin(); it != drivers.end();) { + Driver& a = *it++; + AstUnpackArrayDType* aArrayDTypep = VN_CAST(a.m_vtxp->dtypep(), UnpackArrayDType); + const uint32_t aElements = aArrayDTypep ? aArrayDTypep->elementsConst() : 1; + const uint32_t aEnd = a.m_idx + aElements; + while (it != drivers.end()) { + Driver& b = *it; + // If no overlap, then nothing to do + if (b.m_idx >= aEnd) break; + + AstUnpackArrayDType* bArrayDTypep = VN_CAST(b.m_vtxp->dtypep(), UnpackArrayDType); + const uint32_t bElements = bArrayDTypep ? bArrayDTypep->elementsConst() : 1; + const uint32_t bEnd = b.m_idx + bElements; + const uint32_t overlapEnd = std::min(aEnd, bEnd) - 1; + + if (a.m_fileline->operatorCompare(*b.m_fileline) != 0) { + AstNode* const vp = varp->varScopep() + ? static_cast(varp->varScopep()) + : static_cast(varp->varp()); + + vp->v3warn( // + MULTIDRIVEN, + "Elements [" // + << overlapEnd << ":" << b.m_idx << "] of signal '" << vp->prettyName() + << sub << "' have multiple combinational drivers\n" + << a.m_fileline->warnOther() << "... Location of first driver\n" + << a.m_fileline->warnContextPrimary() << '\n' + << b.m_fileline->warnOther() << "... Location of other driver\n" + << b.m_fileline->warnContextSecondary() << vp->warnOther() + << "... Only the first driver will be respected"); + } + + // If the first driver completely covers the range of the second driver, + // we can just delete the second driver completely, otherwise adjust the + // second driver to apply from the end of the range of the first driver. + if (aEnd >= bEnd) { + prune.emplace(b.m_vtxp); + it = drivers.erase(it); + } else { + const auto distance = std::distance(drivers.begin(), it); + DfgVertex* const bVtxp = b.m_vtxp; + FileLine* const flp = b.m_vtxp->fileline(); + AstNodeDType* const elemDtypep = DfgVertex::dtypeFor( + VN_AS(splicep->dtypep(), UnpackArrayDType)->subDTypep()); + // Remove this driver + it = drivers.erase(it); + // Add missing items element-wise + for (uint32_t i = aEnd; i < bEnd; ++i) { + DfgArraySel* const aselp = new DfgArraySel{*m_dfgp, flp, elemDtypep}; + aselp->fromp(bVtxp); + aselp->bitp(new DfgConst{*m_dfgp, flp, 32, i}); + drivers.emplace_back(flp, i, aselp); + } + it = drivers.begin(); + std::advance(it, distance); + std::stable_sort(it, drivers.end(), cmp); + } } } + + // Reinsert drivers in order + splicep->resetSources(); + for (const Driver& driver : drivers) { + if (!driver.m_vtxp) break; // Stop at end of compacted list + splicep->addDriver(driver.m_fileline, driver.m_idx, driver.m_vtxp); + } + + removeUnused(prune); + + // If the whole variable is driven whole, we can just use that driver + if (splicep->arity() == 1 // + && splicep->driverIndex(0) == 0 // + && splicep->source(0)->dtypep()->isSame(splicep->dtypep())) { + const auto result = std::make_pair(splicep->source(0), splicep->driverFileLine(0)); + VL_DO_DANGLING(splicep->unlinkDelete(*m_dfgp), splicep); + return result; + } + return {splicep, splicep->fileline()}; } // VISITORS @@ -707,9 +891,32 @@ class AstToDfgVisitor final : public VNVisitor { iterate(&root); UASSERT_OBJ(m_uncommittedVertices.empty(), &root, "Uncommitted vertices remain"); - // Canonicalize variables - canonicalizePacked(); - canonicalizeArray(); + if (dumpDfgLevel() >= 8) m_dfgp->dumpDotFilePrefixed(ctx.prefix() + "ast2dfg"); + + // Normalize variable drivers (remove multiple drivers, remove unnecessary splice vertices) + for (DfgVertexVar* const varp : m_dfgp->varVertices().unlinkable()) { + // Delete variables with no sinks nor sources (this can happen due to reverting + // uncommitted vertices, which does not remove variables) + if (!varp->hasSinks() && !varp->srcp()) { + VL_DO_DANGLING(varp->unlinkDelete(*m_dfgp), varp); + continue; + } + + // Nothing to do for un-driven (input) variables + if (!varp->srcp()) continue; + + // The driver of a variable must always be a splice vertex, normalize it + std::pair normalizedDriver; + if (DfgSpliceArray* const sArrayp = varp->srcp()->cast()) { + normalizedDriver = normalizeArray(varp, "", sArrayp); + } else if (DfgSplicePacked* const sPackedp = varp->srcp()->cast()) { + normalizedDriver = normalizePacked(varp, "", sPackedp); + } else { + varp->v3fatalSrc("Unhandled DfgSplicePacked sub-type"); // LCOV_EXCL_LINE + } + varp->srcp(normalizedDriver.first); + varp->driverFileLine(normalizedDriver.second); + } } public: diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index 7c7ac3d88..520f42622 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -188,65 +188,57 @@ class DfgToAstVisitor final : DfgVisitor { return resultp; } - void addResultEquation(const DfgVertexVar* vtxp, FileLine* flp, AstNodeExpr* lhsp, - AstNodeExpr* rhsp) { - AstAssignW* const assignp = new AstAssignW{flp, lhsp, rhsp}; - if VL_CONSTEXPR_CXX17 (T_Scoped) { - // Add it to the scope holding the target variable - getCombActive(vtxp->varScopep()->scopep())->addStmtsp(assignp); - } else { - // Add it to the parend module of the DfgGraph - m_modp->addStmtsp(assignp); + void convertDriver(AstScope* scopep, FileLine* flp, AstNodeExpr* lhsp, DfgVertex* driverp) { + if (!driverp->is()) { + // Base case: assign vertex to current lhs + AstNodeExpr* const rhsp = convertDfgVertexToAstNodeExpr(driverp); + AstAssignW* const assignp = new AstAssignW{flp, lhsp, rhsp}; + lhsp->foreach([flp](AstNode* nodep) { nodep->fileline(flp); }); + if VL_CONSTEXPR_CXX17 (T_Scoped) { + // Add it to the scope holding the target variable + getCombActive(scopep)->addStmtsp(assignp); + } else { + // Add it to the parend module of the DfgGraph + m_modp->addStmtsp(assignp); + } + ++m_ctx.m_resultEquations; + return; } - ++m_ctx.m_resultEquations; - } - void convertPackedDriver(const DfgVarPacked* dfgVarp) { - if (DfgSplicePacked* const splicep = dfgVarp->srcp()->cast()) { - // Variable is driven partially. Render each driver as a separate assignment. - splicep->forEachSourceEdge([&](const DfgEdge& edge, size_t idx) { - UASSERT_OBJ(edge.sourcep(), dfgVarp, "Should have removed undriven sources"); - // Render the rhs expression - AstNodeExpr* const rhsp = convertDfgVertexToAstNodeExpr(edge.sourcep()); - // Create select LValue - FileLine* const flp = splicep->driverFileLine(idx); - AstVarRef* const refp = new AstVarRef{flp, getNode(dfgVarp), VAccess::WRITE}; - AstConst* const lsbp = new AstConst{flp, splicep->driverLsb(idx)}; + if (DfgSplicePacked* const sPackedp = driverp->cast()) { + // Partial assignment of packed value + sPackedp->forEachSourceEdge([&](const DfgEdge& edge, size_t i) { + UASSERT_OBJ(edge.sourcep(), sPackedp, "Should have removed undriven sources"); + // Create Sel + FileLine* const dflp = sPackedp->driverFileLine(i); + AstConst* const lsbp = new AstConst{dflp, sPackedp->driverLsb(i)}; const int width = static_cast(edge.sourcep()->width()); - AstSel* const lhsp = new AstSel{flp, refp, lsbp, width}; - // Add assignment of the value to the selected bits - addResultEquation(dfgVarp, flp, lhsp, rhsp); + AstSel* const nLhsp = new AstSel{dflp, lhsp->cloneTreePure(false), lsbp, width}; + // Convert source + convertDriver(scopep, dflp, nLhsp, edge.sourcep()); + // Delete Sel if not consumed + if (!nLhsp->backp()) VL_DO_DANGLING(nLhsp->deleteTree(), nLhsp); }); return; } - // Whole variable is driven. Render driver and assign directly to whole variable. - FileLine* const flp - = dfgVarp->driverFileLine() ? dfgVarp->driverFileLine() : dfgVarp->fileline(); - AstVarRef* const lhsp = new AstVarRef{flp, getNode(dfgVarp), VAccess::WRITE}; - AstNodeExpr* const rhsp = convertDfgVertexToAstNodeExpr(dfgVarp->srcp()); - addResultEquation(dfgVarp, flp, lhsp, rhsp); - } - - void convertArrayDiver(const DfgVarArray* dfgVarp) { - if (DfgSpliceArray* const splicep = dfgVarp->srcp()->cast()) { - // Variable is driven partially. Assign from parts of the canonical var. - splicep->forEachSourceEdge([&](const DfgEdge& edge, size_t idx) { - UASSERT_OBJ(edge.sourcep(), dfgVarp, "Should have removed undriven sources"); - // Render the rhs expression - AstNodeExpr* const rhsp = convertDfgVertexToAstNodeExpr(edge.sourcep()); - // Create select LValue - FileLine* const flp = splicep->driverFileLine(idx); - AstVarRef* const refp = new AstVarRef{flp, getNode(dfgVarp), VAccess::WRITE}; - AstConst* const idxp = new AstConst{flp, splicep->driverIndex(idx)}; - AstArraySel* const lhsp = new AstArraySel{flp, refp, idxp}; - // Add assignment of the value to the selected bits - addResultEquation(dfgVarp, flp, lhsp, rhsp); + if (DfgSpliceArray* const sArrayp = driverp->cast()) { + // Partial assignment of array variable + sArrayp->forEachSourceEdge([&](const DfgEdge& edge, size_t i) { + UASSERT_OBJ(edge.sourcep(), sArrayp, "Should have removed undriven sources"); + // Create ArraySel + FileLine* const dflp = sArrayp->driverFileLine(i); + AstConst* const idxp = new AstConst{dflp, sArrayp->driverIndex(i)}; + AstArraySel* const nLhsp = new AstArraySel{dflp, lhsp->cloneTreePure(false), idxp}; + // Convert source + convertDriver(scopep, dflp, nLhsp, edge.sourcep()); + // Delete ArraySel if not consumed + if (!nLhsp->backp()) VL_DO_DANGLING(nLhsp->deleteTree(), nLhsp); }); return; } - UASSERT_OBJ(false, dfgVarp, "Should not have wholly driven arrays in Dfg"); + driverp->v3fatalSrc("Unhandled DfgVertexSplice sub-type"); // LCOV_EXCL_LINE } // VISITORS @@ -294,14 +286,13 @@ class DfgToAstVisitor final : DfgVisitor { // If there is no driver (this vertex is an input to the graph), then nothing to do. if (!vtx.srcp()) continue; - // Render packed variable assignments - if (const DfgVarPacked* const dfgVarp = vtx.cast()) { - convertPackedDriver(dfgVarp); - continue; - } - - // Render array variable assignments - convertArrayDiver(vtx.as()); + // Render variable assignments + FileLine* const flp = vtx.driverFileLine() ? vtx.driverFileLine() : vtx.fileline(); + AstScope* const scopep = T_Scoped ? vtx.varScopep()->scopep() : nullptr; + AstVarRef* const lhsp = new AstVarRef{flp, getNode(&vtx), VAccess::WRITE}; + convertDriver(scopep, flp, lhsp, vtx.srcp()); + // convetDriver clones and might not use up the original lhsp + if (!lhsp->backp()) VL_DO_DANGLING(lhsp->deleteTree(), lhsp); } } diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 0e4f27967..b91e34071 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -1202,13 +1202,14 @@ class V3DfgPeephole final : public DfgVisitor { void visit(DfgArraySel* vtxp) override { if (DfgConst* const idxp = vtxp->bitp()->cast()) { if (DfgVarArray* const varp = vtxp->fromp()->cast()) { - if (varp->srcp()) { + if (varp->srcp() && !varp->varp()->isForced() && !varp->varp()->isSc()) { if (DfgSpliceArray* const splicep = varp->srcp()->cast()) { - const size_t idx = idxp->toSizeT(); - if (DfgVertex* const driverp = splicep->driverAt(idx)) { - APPLYING(INLINE_ARRAYSEL) { - replace(vtxp, driverp); - return; + if (DfgVertex* const driverp = splicep->driverAt(idxp->toSizeT())) { + if (!driverp->is()) { + APPLYING(INLINE_ARRAYSEL) { + replace(vtxp, driverp); + return; + } } } } diff --git a/src/V3DfgRegularize.cpp b/src/V3DfgRegularize.cpp index 0bfa62a08..d8de75898 100644 --- a/src/V3DfgRegularize.cpp +++ b/src/V3DfgRegularize.cpp @@ -50,7 +50,7 @@ class DfgRegularize final { if (vtx.is()) { const bool hasNonVarSink = vtx.findSink([](const DfgVertex& snk) { // - return !snk.is(); + return !snk.is() && !snk.is(); }); return hasNonVarSink; } diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index 50dd9a070..11a04e5a1 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -193,10 +193,6 @@ class DfgVarArray final : public DfgVertexVar { friend class DfgVertex; friend class DfgVisitor; - using DriverData = std::pair; - - std::vector m_driverData; // Additional data associate with each driver - public: DfgVarArray(DfgGraph& dfg, AstVar* varp) : DfgVertexVar{dfg, dfgType(), varp} { @@ -229,7 +225,14 @@ class DfgSpliceArray final : public DfgVertexSplice { friend class DfgVertex; friend class DfgVisitor; - using DriverData = std::pair; + struct DriverData final { + FileLine* m_flp; // Location of this driver + uint32_t m_index; // Array index driven by this driver (or low index of range) + DriverData() = delete; + DriverData(FileLine* flp, uint32_t index) + : m_flp{flp} + , m_index{index} {} + }; std::vector m_driverData; // Additional data associated with each driver @@ -253,8 +256,8 @@ public: 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; } + FileLine* driverFileLine(size_t i) const { return m_driverData.at(i).m_flp; } + uint32_t driverIndex(size_t i) const { return m_driverData.at(i).m_index; } DfgVertex* driverAt(size_t idx) const { const DfgEdge* const edgep = findSourceEdge([this, idx](const DfgEdge&, size_t i) { // @@ -271,8 +274,14 @@ class DfgSplicePacked final : public DfgVertexSplice { friend class DfgVertex; friend class DfgVisitor; - using DriverData = std::pair; - + struct DriverData final { + FileLine* m_flp; // Location of this driver + uint32_t m_lsb; // LSB of range driven by this driver + DriverData() = delete; + DriverData(FileLine* flp, uint32_t lsb) + : m_flp{flp} + , m_lsb{lsb} {} + }; std::vector m_driverData; // Additional data associated with each driver bool selfEquals(const DfgVertex& that) const override VL_MT_DISABLED; @@ -295,8 +304,8 @@ public: DfgVertexVariadic::resetSources(); } - FileLine* driverFileLine(size_t idx) const { return m_driverData[idx].first; } - uint32_t driverLsb(size_t idx) const { return m_driverData[idx].second; } + FileLine* driverFileLine(size_t i) const { return m_driverData.at(i).m_flp; } + uint32_t driverLsb(size_t i) const { return m_driverData.at(i).m_lsb; } const std::string srcName(size_t idx) const override { return std::to_string(driverLsb(idx)); } }; diff --git a/test_regress/t/t_dfg_break_cycles.py b/test_regress/t/t_dfg_break_cycles.py index 19b96d307..bd94a1b10 100755 --- a/test_regress/t/t_dfg_break_cycles.py +++ b/test_regress/t/t_dfg_break_cycles.py @@ -112,6 +112,9 @@ if coveredLines != expectedLines: for n in sorted(coveredLines - expectedLines): test.error_keep_going(f"V3DfgBreakCycles.cpp line {n} covered but not expected") +test.file_grep_not(test.obj_dir + "/obj_opt/Vopt__stats.txt", + r'DFG.*non-representable.*\s[1-9]\d*$') + # Execute test to check equivalence test.execute(executable=test.obj_dir + "/obj_opt/Vopt") diff --git a/test_regress/t/t_dfg_multidriver_dfg_bad.out b/test_regress/t/t_dfg_multidriver_dfg_bad.out index 62fea591c..9020d13ac 100644 --- a/test_regress/t/t_dfg_multidriver_dfg_bad.out +++ b/test_regress/t/t_dfg_multidriver_dfg_bad.out @@ -1,39 +1,75 @@ -%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [3:1] of signal 'a' have multiple combinational drivers - : ... note: In instance 't' - t/t_dfg_multidriver_dfg_bad.v:14:19: ... Location of first driver - 14 | assign a[3:0] = i[3:0]; - | ^ - t/t_dfg_multidriver_dfg_bad.v:15:19: ... Location of other driver - 15 | assign a[4:1] = ~i[4:1]; - | ^ - t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected - ... For warning description see https://verilator.org/warn/MULTIDRIVEN?v=latest - ... Use "/* verilator lint_off MULTIDRIVEN */" and lint_on around source to disable this message. -%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [3:3] of signal 'a' have multiple combinational drivers - : ... note: In instance 't' - t/t_dfg_multidriver_dfg_bad.v:14:19: ... Location of first driver - 14 | assign a[3:0] = i[3:0]; - | ^ - t/t_dfg_multidriver_dfg_bad.v:16:17: ... Location of other driver - 16 | assign a[3] = ~i[3]; - | ^ - t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected -%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [7:6] of signal 'a' have multiple combinational drivers +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:16:18: Bits [3:1] of signal 'a' have multiple combinational drivers : ... note: In instance 't' t/t_dfg_multidriver_dfg_bad.v:17:19: ... Location of first driver - 17 | assign a[8:5] = i[8:5]; + 17 | assign a[3:0] = i[3:0]; | ^ t/t_dfg_multidriver_dfg_bad.v:18:19: ... Location of other driver - 18 | assign a[7:6] = ~i[7:6]; + 18 | assign a[4:1] = ~i[4:1]; | ^ - t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected -%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [9:9] of signal 'a' have multiple combinational drivers + t/t_dfg_multidriver_dfg_bad.v:16:18: ... Only the first driver will be respected + ... For warning description see https://verilator.org/warn/MULTIDRIVEN?v=latest + ... Use "/* verilator lint_off MULTIDRIVEN */" and lint_on around source to disable this message. +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:16:18: Bits [3:3] of signal 'a' have multiple combinational drivers : ... note: In instance 't' - t/t_dfg_multidriver_dfg_bad.v:19:17: ... Location of first driver - 19 | assign a[9] = i[9]; - | ^ - t/t_dfg_multidriver_dfg_bad.v:20:19: ... Location of other driver - 20 | assign a[9] = ~i[9]; + t/t_dfg_multidriver_dfg_bad.v:17:19: ... Location of first driver + 17 | assign a[3:0] = i[3:0]; | ^ - t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected + t/t_dfg_multidriver_dfg_bad.v:19:17: ... Location of other driver + 19 | assign a[3] = ~i[3]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:16:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:16:18: Bits [7:6] of signal 'a' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:20:19: ... Location of first driver + 20 | assign a[8:5] = i[8:5]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:21:19: ... Location of other driver + 21 | assign a[7:6] = ~i[7:6]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:16:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:16:18: Bits [9:9] of signal 'a' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:22:17: ... Location of first driver + 22 | assign a[9] = i[9]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:23:19: ... Location of other driver + 23 | assign a[9] = ~i[9]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:16:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:26:18: Elements [3:0] of signal 'u' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:27:14: ... Location of first driver + 27 | assign u = j; + | ^ + t/t_dfg_multidriver_dfg_bad.v:28:14: ... Location of other driver + 28 | assign u = k; + | ^ + t/t_dfg_multidriver_dfg_bad.v:26:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:30:18: Elements [1:1] of signal 'v' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:31:14: ... Location of first driver + 31 | assign v = j; + | ^ + t/t_dfg_multidriver_dfg_bad.v:32:17: ... Location of other driver + 32 | assign v[1] = i; + | ^ + t/t_dfg_multidriver_dfg_bad.v:30:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:34:18: Elements [0:0] of signal 'w' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:35:17: ... Location of first driver + 35 | assign w[0] = i; + | ^ + t/t_dfg_multidriver_dfg_bad.v:36:14: ... Location of other driver + 36 | assign w = j; + | ^ + t/t_dfg_multidriver_dfg_bad.v:34:18: ... Only the first driver will be respected +%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:38:18: Bits [3:2] of signal 'x[3]' have multiple combinational drivers + : ... note: In instance 't' + t/t_dfg_multidriver_dfg_bad.v:39:17: ... Location of first driver + 39 | assign x[3] = i; + | ^ + t/t_dfg_multidriver_dfg_bad.v:40:22: ... Location of other driver + 40 | assign x[3][3:2] = ~i[1:0]; + | ^ + t/t_dfg_multidriver_dfg_bad.v:38:18: ... Only the first driver will be respected %Error: Exiting due to diff --git a/test_regress/t/t_dfg_multidriver_dfg_bad.v b/test_regress/t/t_dfg_multidriver_dfg_bad.v index 724b33539..84659c923 100644 --- a/test_regress/t/t_dfg_multidriver_dfg_bad.v +++ b/test_regress/t/t_dfg_multidriver_dfg_bad.v @@ -7,7 +7,10 @@ `default_nettype none module t( - input wire [10:0] i, + input wire [10:0] i, + input wire [10:0] j [4], + input wire [10:0] k [4], + output wire [10:0] o ); logic [10:0] a; @@ -19,5 +22,26 @@ module t( assign a[9] = i[9]; assign a[9] = ~i[9]; assign a[10] = i[10]; - assign o = a; + + logic [10:0] u [4]; + assign u = j; + assign u = k; + + logic [10:0] v [4]; + assign v = j; + assign v[1] = i; + + logic [10:0] w [4]; + assign w[0] = i; + assign w = j; + + logic [10:0] x [4]; + assign x[3] = i; + assign x[3][3:2] = ~i[1:0]; + // No warning for w[2]! + assign x[2][3:2] = ~i[1:0]; + assign x[2][1:0] = ~i[1:0]; + + assign o = a ^ u[3] ^ v[3] ^ w[3] ^ x[3]; + endmodule diff --git a/test_regress/t/t_dfg_peephole.cpp b/test_regress/t/t_dfg_peephole.cpp index 0664397e1..deb7da096 100644 --- a/test_regress/t/t_dfg_peephole.cpp +++ b/test_regress/t/t_dfg_peephole.cpp @@ -29,8 +29,10 @@ int main(int, char**) { uint64_t rand_a = 0x5aef0c8dd70a4497; uint64_t rand_b = 0xf0c0a8dd75ae4497; - uint64_t srand_a = 0x00fa8dcc7ae4957; - uint64_t srand_b = 0x0fa8dc7ae3c9574; + uint64_t srand_a = 0x000fa8dcc7ae4957; + uint64_t srand_b = 0x00fa8dc7ae3c9574; + uint64_t arand_a = 0x758c168d16c93a0f; + uint64_t arand_b = 0xbe01de017d87355d; for (size_t n = 0; n < 200000; ++n) { // Update rngs @@ -38,12 +40,16 @@ int main(int, char**) { rngUpdate(rand_b); rngUpdate(srand_a); rngUpdate(srand_b); + rngUpdate(arand_a); + rngUpdate(arand_b); // Assign inputs ref.rand_a = opt.rand_a = rand_a; ref.rand_b = opt.rand_b = rand_b; ref.srand_a = opt.srand_a = srand_a; ref.srand_b = opt.srand_b = srand_b; + ref.arand_a = opt.arand_a = arand_a; + ref.arand_b = opt.arand_b = arand_b; // Evaluate both models ref.eval(); diff --git a/test_regress/t/t_dfg_peephole.py b/test_regress/t/t_dfg_peephole.py index 0466a34e1..034ba591c 100755 --- a/test_regress/t/t_dfg_peephole.py +++ b/test_regress/t/t_dfg_peephole.py @@ -88,9 +88,6 @@ test.compile(verilator_flags2=[ "../../t/" + test.name + ".cpp" ]) # yapf:disable -# Execute test to check equivalence -test.execute(executable=test.obj_dir + "/obj_opt/Vopt") - def check(name): name = name.lower() @@ -103,4 +100,10 @@ def check(name): for opt in optimizations: check(opt) +test.file_grep_not(test.obj_dir + "/obj_opt/Vopt__stats.txt", + r'DFG.*non-representable.*\s[1-9]\d*$') + +# Execute test to check equivalence +test.execute(executable=test.obj_dir + "/obj_opt/Vopt") + test.passes() diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index fb4644387..cd2ed056a 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -8,7 +8,7 @@ module t ( `include "portlist.vh" // Boilerplate generated by t_dfg_peephole.py - rand_a, rand_b, srand_a, srand_b + rand_a, rand_b, srand_a, srand_b, arand_a, arand_b ); `include "portdecl.vh" // Boilerplate generated by t_dfg_peephole.py @@ -17,10 +17,16 @@ module t ( input rand_b; input srand_a; input srand_b; + input arand_a; + input arand_b; wire logic [63:0] rand_a; wire logic [63:0] rand_b; wire logic signed [63:0] srand_a; wire logic signed [63:0] srand_b; + // verilator lint_off ASCRANGE + wire logic [0:63] arand_a; + wire logic [0:63] arand_b; + // verilator lint_on ASCRANGE wire logic randbit_a = rand_a[0]; wire logic [127:0] rand_ba = {rand_b, rand_a}; @@ -29,9 +35,13 @@ module t ( wire logic [63:0] const_b; wire logic signed [63:0] sconst_a; wire logic signed [63:0] sconst_b; - wire logic [63:0] array [3:0]; + logic [63:0] array [3:0]; assign array[0] = (rand_a << 32) | (rand_a >> 32); assign array[1] = (rand_a << 16) | (rand_a >> 48); + assign array[2][3:0] = rand_a[3:0]; + always @(rand_b) begin // Intentional non-combinational partial driver + array[2][7:4] = rand_a[7:4]; + end `signal(FOLD_UNARY_CLog2, $clog2(const_a)); `signal(FOLD_UNARY_CountOnes, $countones(const_a)); @@ -184,6 +194,7 @@ module t ( `signal(REPLACE_COND_WITH_ELSE_BRANCH_ZERO, rand_a[0] ? rand_a[1] : 1'd0); `signal(REPLACE_COND_WITH_ELSE_BRANCH_ONES, rand_a[0] ? rand_a[1] : 1'd1); `signal(INLINE_ARRAYSEL, array[0]); + `signal(NO_INLINE_ARRAYSEL_PARTIAL, array[2]); `signal(PUSH_BITWISE_THROUGH_REDUCTION_AND, (&(rand_a + 64'd105)) & (&(rand_b + 64'd108))); `signal(PUSH_BITWISE_THROUGH_REDUCTION_OR, (|(rand_a + 64'd106)) | (|(rand_b + 64'd109))); `signal(PUSH_BITWISE_THROUGH_REDUCTION_XOR, (^(rand_a + 64'd107)) ^ (^(rand_b + 64'd110))); @@ -223,6 +234,15 @@ module t ( `signal(PUSH_SEL_THROUGH_SHIFTL, sel_from_shiftl[20:0]); `signal(REPLACE_SEL_FROM_SEL, sel_from_sel[4:3]); + // Asscending ranges + `signal(ASCENDNG_SEL, arand_a[0:4]); + // verilator lint_off ASCRANGE + wire [0:7] ascending_assign; + // verilator lint_on ASCRANGE + assign ascending_assign[0:3] = arand_a[4:7]; + assign ascending_assign[4:7] = arand_b[0:3]; + `signal(ASCENDING_ASSIGN, ascending_assign); + // Sel from not requires the operand to have a sinle sink, so can't use // the chekc due to the raw expression referencing the operand wire [63:0] sel_from_not_tmp = ~(rand_a >> rand_b[2:0] << rand_a[3:0]); diff --git a/test_regress/t/t_inst_array_partial.v b/test_regress/t/t_inst_array_partial.v index 35f2d2252..2470c714c 100644 --- a/test_regress/t/t_inst_array_partial.v +++ b/test_regress/t/t_inst_array_partial.v @@ -10,7 +10,9 @@ module t (/*AUTOARG*/ ); input clk; + // verilator lint_off MULTIDRIVEN wire [19:10] bitout; + // verilator lint_on MULTIDRIVEN wire [29:24] short_bitout; wire [7:0] allbits; wire [15:0] twobits;