From e44f34dde380d1b554485ac03c3623a4a04d45d3 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 30 Nov 2024 18:56:00 -0500 Subject: [PATCH] Improve concat lint error & cleanups for future commit. --- src/V3AstNodeExpr.h | 9 ++++++--- src/V3AstNodes.cpp | 2 ++ src/V3Width.cpp | 20 ++++++++++++-------- test_regress/t/t_unpacked_concat_bad.out | 4 ++++ test_regress/t/t_unpacked_concat_bad3.out | 9 +++++++-- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index cafe804b7..5237268e6 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -1738,7 +1738,8 @@ class AstPatMember final : public AstNodeExpr { // @astgen op2 := keyp : Optional[AstNode] // @astgen op3 := repp : Optional[AstNodeExpr] // replication count, or nullptr for count 1 // @astgen op4 := varrefp : Optional[AstNodeExpr] // Decoded variable if TEXT - bool m_default = false; + bool m_isDefault = false; // Has default + bool m_isConcat = false; // From concatenate public: AstPatMember(FileLine* fl, AstNodeExpr* lhssp, AstNode* keyp, AstNodeExpr* repp) @@ -1755,8 +1756,10 @@ public: int instrCount() const override { return widthInstrs() * 2; } void dump(std::ostream& str = std::cout) const override; void dumpJson(std::ostream& str = std::cout) const override; - bool isDefault() const { return m_default; } - void isDefault(bool flag) { m_default = flag; } + bool isConcat() const { return m_isConcat; } + void isConcat(bool flag) { m_isConcat = flag; } + bool isDefault() const { return m_isDefault; } + void isDefault(bool flag) { m_isDefault = flag; } }; class AstPattern final : public AstNodeExpr { // Verilog '{a,b,c,d...} diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 1685a5de5..7f4c57e0e 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2263,9 +2263,11 @@ void AstPackageImport::pkgNameFrom() { } void AstPatMember::dump(std::ostream& str) const { this->AstNodeExpr::dump(str); + if (isConcat()) str << " [CONCAT]"; if (isDefault()) str << " [DEFAULT]"; } void AstPatMember::dumpJson(std::ostream& str) const { + if (isConcat()) dumpJsonBoolFunc(str, isConcat); if (isDefault()) dumpJsonBoolFunc(str, isDefault); dumpJsonGen(str); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index f4a7c714a..51848261c 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -573,7 +573,6 @@ class WidthVisitor final : public VNVisitor { // to determine if value or push userIterateAndNext(nodep->lhsp(), WidthVP{vdtypep, PRELIM}.p()); userIterateAndNext(nodep->rhsp(), WidthVP{vdtypep, PRELIM}.p()); - // Queue "element 0" is lhsp, so we need to swap arguments const bool lhsIsValue = AstNode::computeCastable(adtypep->subDTypep(), nodep->lhsp()->dtypep(), nullptr) .isAssignable(); @@ -4537,6 +4536,7 @@ class WidthVisitor final : public VNVisitor { UINFO(9, "ent " << range.left() << " to " << range.right() << endl); AstNode* newp = nullptr; bool allConstant = true; + const bool isConcat = nodep->itemsp() && VN_AS(nodep->itemsp(), PatMember)->isConcat(); for (int entn = 0, ent = range.left(); entn < range.elements(); ++entn, ent += range.leftToRightInc()) { AstPatMember* newpatp = nullptr; @@ -4546,10 +4546,10 @@ class WidthVisitor final : public VNVisitor { if (defaultp) { newpatp = defaultp->cloneTree(false); patp = newpatp; - } else if (!(VN_IS(arrayDtp, UnpackArrayDType) && !allConstant)) { + } else if (!(VN_IS(arrayDtp, UnpackArrayDType) && !allConstant && isConcat)) { // If arrayDtp is an unpacked array and item is not constant, - // the number of elemnt cannot be determined here as the dtype of each element - // is not set yet. V3Slice checks for such cases. + // the number of elements cannot be determined here as the dtype of each + // element is not set yet. V3Slice checks for such cases. nodep->v3error("Assignment pattern missed initializing elements: " << ent); } } else { @@ -7855,14 +7855,18 @@ class WidthVisitor final : public VNVisitor { if (AstConcat* lhsp = VN_CAST(nodep->lhsp(), Concat)) { patConcatConvertRecurse(patternp, lhsp); } else { - patternp->addItemsp(new AstPatMember{nodep->lhsp()->fileline(), - nodep->lhsp()->unlinkFrBack(), nullptr, nullptr}); + AstPatMember* const newp = new AstPatMember{ + nodep->lhsp()->fileline(), nodep->lhsp()->unlinkFrBack(), nullptr, nullptr}; + newp->isConcat(true); + patternp->addItemsp(newp); } if (AstConcat* rhsp = VN_CAST(nodep->rhsp(), Concat)) { patConcatConvertRecurse(patternp, rhsp); } else { - patternp->addItemsp(new AstPatMember{nodep->rhsp()->fileline(), - nodep->rhsp()->unlinkFrBack(), nullptr, nullptr}); + AstPatMember* const newp = new AstPatMember{ + nodep->rhsp()->fileline(), nodep->rhsp()->unlinkFrBack(), nullptr, nullptr}; + newp->isConcat(true); + patternp->addItemsp(newp); } } diff --git a/test_regress/t/t_unpacked_concat_bad.out b/test_regress/t/t_unpacked_concat_bad.out index 6b736d415..051187a46 100644 --- a/test_regress/t/t_unpacked_concat_bad.out +++ b/test_regress/t/t_unpacked_concat_bad.out @@ -3,4 +3,8 @@ 12 | localparam bit_int_t count_bits [1:0] = {2{$bits(count_t)}}; | ^ ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_unpacked_concat_bad.v:12:46: Assignment pattern missed initializing elements: 0 + : ... note: In instance 't' + 12 | localparam bit_int_t count_bits [1:0] = {2{$bits(count_t)}}; + | ^ %Error: Exiting due to diff --git a/test_regress/t/t_unpacked_concat_bad3.out b/test_regress/t/t_unpacked_concat_bad3.out index 4bd30ce0c..f106250c0 100644 --- a/test_regress/t/t_unpacked_concat_bad3.out +++ b/test_regress/t/t_unpacked_concat_bad3.out @@ -1,4 +1,9 @@ -%Error: Internal Error: t/t_unpacked_concat_bad3.v:9:41: ../V3EmitCConstInit.h:#: Missing array init element +%Error: t/t_unpacked_concat_bad3.v:9:41: Assignment pattern missed initializing elements: 3 + : ... note: In instance 't' 9 | localparam logic [7:0] TOO_FEW [5] = '{0, 1, 2**8-1}; | ^~ - ... See the manual at https://verilator.org/verilator_doc.html for more assistance. +%Error: t/t_unpacked_concat_bad3.v:9:41: Assignment pattern missed initializing elements: 4 + : ... note: In instance 't' + 9 | localparam logic [7:0] TOO_FEW [5] = '{0, 1, 2**8-1}; + | ^~ +%Error: Exiting due to