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;