From 26c85d449587e11debe8d802ccc0497de3d49975 Mon Sep 17 00:00:00 2001 From: Nick Brereton <85175726+nbstrike@users.noreply.github.com> Date: Wed, 17 Jun 2026 17:56:48 -0400 Subject: [PATCH] Fix `$bits` on unpacked structs (#4521) (#7796) Fixes #4521. --- src/V3LinkDot.cpp | 50 ++++++++++++++++++++++- src/V3Width.cpp | 3 +- test_regress/t/t_stream_unpacked_struct.v | 30 ++++++++++++++ test_regress/t/t_struct_type_bad.out | 7 ++++ test_regress/t/t_struct_type_bad.v | 6 +++ 5 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 1ada078ba..a6b038396 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1993,6 +1993,17 @@ class LinkDotFindVisitor final : public VNVisitor { // No need to insert, only the real typedef matters, but need to track for errors nodep->user1p(m_curSymp); } + void visit(AstNodeUOrStructDType* nodep) override { // FindVisitor:: + UASSERT_OBJ(m_curSymp, nodep, "Struct/union dtype not under module/package/$unit"); + VL_RESTORER(m_curSymp); + m_curSymp = m_statep->insertBlock(m_curSymp, "__Vdtype" + cvtToStr(nodep->uniqueNum()), + nodep, m_classOrPackagep); + iterateChildren(nodep); + } + void visit(AstMemberDType* nodep) override { // FindVisitor:: + iterateChildren(nodep); + m_statep->insertSym(m_curSymp, nodep->name(), nodep, m_classOrPackagep); + } void visit(AstParamTypeDType* nodep) override { // FindVisitor:: UASSERT_OBJ(m_curSymp, nodep, "Parameter type not under module/package/$unit"); @@ -3511,6 +3522,20 @@ class LinkDotResolveVisitor final : public VNVisitor { << declp->warnContextSecondary()); } } + void checkMemberDeclOrder(AstNode* nodep, AstMemberDType* declp) { + const uint32_t declTokenNum = declp->fileline()->tokenNum(); + if (nodep->fileline()->tokenNum() < declTokenNum) { + UINFO(1, "Related node " << nodep->fileline()->tokenNum() << " " << nodep); + UINFO(1, "Related decl " << declTokenNum << " " << declp); + nodep->v3error("Reference to " + << nodep->prettyNameQ() << " before declaration (IEEE 1800-2023 6.18)\n" + << nodep->warnMore() + << "... Suggest move the declaration before the reference\n" + << nodep->warnContextPrimary() << '\n' + << declp->warnOther() << "... Location of original declaration\n" + << declp->warnContextSecondary()); + } + } void replaceWithCheckBreak(AstNode* oldp, AstNodeDType* newp) { // Flag now to avoid V3Broken throwing an internal error @@ -3536,6 +3561,15 @@ class LinkDotResolveVisitor final : public VNVisitor { if (nodep && nodep->isParam()) nodep->usedParam(true); } + static VSymEnt* findIdFallbackSkipMemberDType(VSymEnt* lookp, const string& name) { + while (lookp) { + VSymEnt* const foundp = lookp->findIdFlat(name); + if (foundp && !VN_IS(foundp->nodep(), MemberDType)) return foundp; + lookp = lookp->fallbackp(); + } + return nullptr; + } + void symIterateChildren(AstNode* nodep, VSymEnt* symp) { // Iterate children, changing to given context, with restore to old context VL_RESTORER(m_ds); @@ -4532,6 +4566,16 @@ class LinkDotResolveVisitor final : public VNVisitor { } else { (void)defp; // Prevent unused variable warning } + } else if (AstMemberDType* const defp = VN_CAST(foundp->nodep(), MemberDType)) { + if (allowVar) { + checkMemberDeclOrder(nodep, defp); + AstRefDType* const refp = new AstRefDType{nodep->fileline(), nodep->name()}; + refp->refDTypep(defp); + replaceWithCheckBreak(nodep, refp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + ok = true; + m_ds.m_dotText = ""; + } } else if (AstEnumItem* const valuep = VN_CAST(foundp->nodep(), EnumItem)) { if (allowVar) { AstNode* const newp @@ -4967,6 +5011,10 @@ class LinkDotResolveVisitor final : public VNVisitor { refdtypep->v3error("Self-referential enumerated type definition"); } } + void visit(AstNodeUOrStructDType* nodep) override { + LINKDOT_VISIT_START(); + symIterateChildren(nodep, m_statep->getNodeSym(nodep)); + } void visit(AstEnumItemRef* nodep) override { // Resolve its reference // EnumItemRefs are created by the first pass, but V3Param may regenerate due to @@ -5984,7 +6032,7 @@ class LinkDotResolveVisitor final : public VNVisitor { if (nodep->classOrPackagep()) { foundp = m_statep->getNodeSym(nodep->classOrPackagep())->findIdFlat(nodep->name()); } else if (m_ds.m_dotPos == DP_FIRST || m_ds.m_dotPos == DP_NONE) { - foundp = m_curSymp->findIdFallback(nodep->name()); + foundp = findIdFallbackSkipMemberDType(m_curSymp, nodep->name()); } else { // Defensive: dotPos should be DP_FIRST/DP_NONE or classOrPackagep set. v3fatalSrc("Unexpected dotPos=" diff --git a/src/V3Width.cpp b/src/V3Width.cpp index f5e55d2a7..e11285aa6 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -9661,7 +9661,8 @@ class WidthVisitor final : public VNVisitor { continue; } else if (const AstNodeUOrStructDType* const adtypep = VN_CAST(dtypep, NodeUOrStructDType)) { - bits *= adtypep->width(); + bits *= adtypep->isStreamableFixedAggregate() ? adtypep->widthStream() + : adtypep->width(); break; } else if (const AstBasicDType* const adtypep = VN_CAST(dtypep, BasicDType)) { bits *= adtypep->width(); diff --git a/test_regress/t/t_stream_unpacked_struct.v b/test_regress/t/t_stream_unpacked_struct.v index 8a169c20e..6b0c6f2d6 100644 --- a/test_regress/t/t_stream_unpacked_struct.v +++ b/test_regress/t/t_stream_unpacked_struct.v @@ -83,6 +83,17 @@ module t; byte tail; } packed_union_struct_t; + localparam int WORD_WIDTH = 16; + + typedef union packed { + struct packed { + logic [15:0] temp_1; + logic [15:0] temp_2; + logic [15:0] temp_3; + } regs; + logic [($bits(regs) / WORD_WIDTH)-1:0][WORD_WIDTH-1:0] words; + } issue_4521_union_t; + simple_t simple; simple_t simple_out; simple_t simple_cont_out; @@ -106,6 +117,7 @@ module t; real_struct_t real_struct_out; packed_union_struct_t packed_union_struct; packed_union_struct_t packed_union_struct_out; + issue_4521_union_t issue_4521_union; logic [19:0] simple_bits; logic [31:0] wide_simple_bits; @@ -122,6 +134,7 @@ module t; logic [15:0] packed_union_bits; logic [11:0] narrow_bits; logic [19:0] simple_streaml_src; + logic [$bits(simple_t)-1:0] simple_bits_from_bits; byte byte_array_out[2]; assign {>>{simple_cont_out}} = 20'habcde; @@ -138,6 +151,13 @@ module t; `checkh(simple_bits, 20'h54321); simple = '{8'h12, 4'ha, '{4'hb, 4'hc}}; + `checkh($bits(simple_t), 20); + `checkh($bits(array_t), 32); + `checkh($bits(nested_t), 36); + `checkh($bits(struct_array_t), 56); + `checkh($bits(simple_array), 40); + simple_bits_from_bits = {>>{simple}}; + `checkh(simple_bits_from_bits, 20'h12abc); simple_bits = {>>{simple}}; `checkh(simple_bits, 20'h12abc); /* verilator lint_off WIDTHEXPAND */ @@ -341,6 +361,16 @@ module t; `checkh(packed_union_struct_out.u.byte_data, 8'hca); `checkh(packed_union_struct_out.tail, 8'hfe); + `checkh($bits(issue_4521_union_t), 48); + `checkh($bits(issue_4521_union.regs), 48); + `checkh($bits(issue_4521_union.words), 48); + issue_4521_union.regs.temp_1 = 16'h1234; + issue_4521_union.regs.temp_2 = 16'h5678; + issue_4521_union.regs.temp_3 = 16'h9abc; + `checkh(issue_4521_union.words[2], 16'h1234); + `checkh(issue_4521_union.words[1], 16'h5678); + `checkh(issue_4521_union.words[0], 16'h9abc); + $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_struct_type_bad.out b/test_regress/t/t_struct_type_bad.out index f57c1dd0c..2085d6ecd 100644 --- a/test_regress/t/t_struct_type_bad.out +++ b/test_regress/t/t_struct_type_bad.out @@ -2,4 +2,11 @@ 13 | i badi; | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_struct_type_bad.v:18:19: Reference to 'later' before declaration (IEEE 1800-2023 6.18) + : ... Suggest move the declaration before the reference + 18 | logic [($bits(later) / 8)-1:0][7:0] bad_forward; + | ^~~~~ + t/t_struct_type_bad.v:19:18: ... Location of original declaration + 19 | logic [15:0] later; + | ^~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_struct_type_bad.v b/test_regress/t/t_struct_type_bad.v index 622f2e8e8..1d977d7b7 100644 --- a/test_regress/t/t_struct_type_bad.v +++ b/test_regress/t/t_struct_type_bad.v @@ -13,4 +13,10 @@ module t; i badi; // Bad } struct_t; + typedef union packed { + // Bad + logic [($bits(later) / 8)-1:0][7:0] bad_forward; + logic [15:0] later; + } forward_member_t; + endmodule