From 0606a29f13c3fc272b46bc0b967f79330a217265 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 6 May 2023 21:41:17 -0400 Subject: [PATCH] Fix arrays of unpacked structs (#4173). --- Changes | 1 + src/V3AstNodeDType.h | 1 + src/V3AstNodes.cpp | 10 ++++++ src/V3Class.cpp | 6 ++-- src/V3Common.cpp | 1 + src/V3EmitCHeaders.cpp | 2 +- src/V3EmitCImp.cpp | 1 + test_regress/t/t_struct_nest_uarray.pl | 21 +++++++++++ test_regress/t/t_struct_nest_uarray.v | 48 ++++++++++++++++++++++++++ 9 files changed, 88 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_struct_nest_uarray.pl create mode 100644 test_regress/t/t_struct_nest_uarray.v diff --git a/Changes b/Changes index 6b0380417..98018b690 100644 --- a/Changes +++ b/Changes @@ -27,6 +27,7 @@ Verilator 5.011 devel * Fix initialization order of initial static after function/task (#4159). [Kamil Rakoczy, Antmicro Ltd] * Fix linking AstRefDType if it has parameterized class ref (#4164) (#4170). [Ryszard Rozak, Antmicro Ltd] * Fix crash caused by $display() optimization (#4165) (#4166). [Tudor Timi] +* Fix arrays of unpacked structs (#4173). [Risto Pejašinović] * Fix detection of wire/reg duplicates. * Fix false IMPLICITSTATIC on package functions. diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 10d90c33f..c06e761c0 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -887,6 +887,7 @@ public: if (m_refDTypep && m_refDTypep->clonep()) m_refDTypep = m_refDTypep->clonep(); } AstNodeDType* getChildDTypep() const override { return childDTypep(); } + AstNodeUOrStructDType* getChildStructp() const; AstNodeDType* subDTypep() const override VL_MT_STABLE { return m_refDTypep ? m_refDTypep : childDTypep(); } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index e0b2f1b4f..46f5e53d7 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1671,10 +1671,19 @@ void AstLogOr::dump(std::ostream& str) const { this->AstNodeExpr::dump(str); if (sideEffect()) str << " [SIDE]"; } + void AstMemberDType::dumpSmall(std::ostream& str) const { this->AstNodeDType::dumpSmall(str); str << "member"; } +AstNodeUOrStructDType* AstMemberDType::getChildStructp() const { + AstNodeDType* subdtp = skipRefp(); + while (AstNodeArrayDType* const asubdtp = VN_CAST(subdtp, NodeArrayDType)) { + subdtp = asubdtp->subDTypep(); + } + return VN_CAST(subdtp, NodeUOrStructDType); // Maybe nullptr +} + void AstMemberSel::dump(std::ostream& str) const { this->AstNodeExpr::dump(str); str << " -> "; @@ -1831,6 +1840,7 @@ void AstNodeUOrStructDType::dump(std::ostream& str) const { this->AstNodeDType::dump(str); if (packed()) str << " [PACKED]"; if (isFourstate()) str << " [4STATE]"; + if (classOrPackagep()) str << " pkg=" << nodeAddr(classOrPackagep()); } void AstNodeDType::dump(std::ostream& str) const { this->AstNode::dump(str); diff --git a/src/V3Class.cpp b/src/V3Class.cpp index 71cd260d3..f318048f8 100644 --- a/src/V3Class.cpp +++ b/src/V3Class.cpp @@ -62,6 +62,7 @@ private: AstClassPackage* const packagep = new AstClassPackage{nodep->fileline(), nodep->origName()}; packagep->name(nodep->name() + "__Vclpkg"); + nodep->editCountInc(); nodep->classOrPackagep(packagep); packagep->classp(nodep); v3Global.rootp()->addModulesp(packagep); @@ -177,14 +178,15 @@ private: } void setStructModulep(AstNodeUOrStructDType* const dtypep) { - // Give it a pointer to its package and a final name + // Give struct a pointer to its package and a final name + dtypep->editCountInc(); dtypep->classOrPackagep(m_classPackagep ? m_classPackagep : m_modp); dtypep->name(dtypep->name() + (VN_IS(dtypep, UnionDType) ? "__union" : "__struct") + cvtToStr(dtypep->uniqueNum())); for (const AstMemberDType* itemp = dtypep->membersp(); itemp; itemp = VN_AS(itemp->nextp(), MemberDType)) { - AstNodeUOrStructDType* const subp = VN_CAST(itemp->skipRefp(), NodeUOrStructDType); + AstNodeUOrStructDType* const subp = itemp->getChildStructp(); // Recurse only into anonymous unpacked structs inside this definition, // other unpacked structs will be reached from another typedefs if (subp && !subp->packed() && subp->name().empty()) setStructModulep(subp); diff --git a/src/V3Common.cpp b/src/V3Common.cpp index ed1570c51..25043d425 100644 --- a/src/V3Common.cpp +++ b/src/V3Common.cpp @@ -63,6 +63,7 @@ static void makeVlToString(AstIface* nodep) { } static void makeVlToString(AstNodeUOrStructDType* nodep) { AstNodeModule* const modp = nodep->classOrPackagep(); + UASSERT_OBJ(modp, nodep, "Unlinked struct package"); AstCFunc* const funcp = new AstCFunc{nodep->fileline(), "VL_TO_STRING", nullptr, "std::string"}; funcp->argTypes("const " + EmitCBase::prefixNameProtect(nodep) + "& obj"); diff --git a/src/V3EmitCHeaders.cpp b/src/V3EmitCHeaders.cpp index 798006653..04eac47d7 100644 --- a/src/V3EmitCHeaders.cpp +++ b/src/V3EmitCHeaders.cpp @@ -219,7 +219,7 @@ class EmitCHeader final : public EmitCConstInit { emitted.insert(sdtypep); for (const AstMemberDType* itemp = sdtypep->membersp(); itemp; itemp = VN_AS(itemp->nextp(), MemberDType)) { - AstNodeUOrStructDType* subp = VN_CAST(itemp->skipRefp(), NodeUOrStructDType); + AstNodeUOrStructDType* const subp = itemp->getChildStructp(); if (subp && !subp->packed()) { // Recurse if it belongs to the current module if (subp->classOrPackagep() == modp) { diff --git a/src/V3EmitCImp.cpp b/src/V3EmitCImp.cpp index 5574613d6..f47b5bcf9 100644 --- a/src/V3EmitCImp.cpp +++ b/src/V3EmitCImp.cpp @@ -54,6 +54,7 @@ class EmitCGatherDependencies final : VNVisitorConst { } else if (const AstNodeUOrStructDType* const dtypep = VN_CAST(nodep, NodeUOrStructDType)) { if (!dtypep->packed()) { + UASSERT_OBJ(dtypep->classOrPackagep(), nodep, "Unlinked struct package"); m_dependencies.insert(EmitCBase::prefixNameProtect(dtypep->classOrPackagep())); } } diff --git a/test_regress/t/t_struct_nest_uarray.pl b/test_regress/t/t_struct_nest_uarray.pl new file mode 100755 index 000000000..b46d46042 --- /dev/null +++ b/test_regress/t/t_struct_nest_uarray.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_struct_nest_uarray.v b/test_regress/t/t_struct_nest_uarray.v new file mode 100644 index 000000000..81cdeee3f --- /dev/null +++ b/test_regress/t/t_struct_nest_uarray.v @@ -0,0 +1,48 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`define stop $stop +`define checks(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); + +typedef struct { + struct { + struct { + logic [31:0] next; + } val; + } el[1]; +} pstr_t; + +module t (/*AUTOARG*/); + + typedef struct { + struct { + struct { + logic [31:0] next; + } val; + } el[1]; + } str_t; + + str_t str; + pstr_t pstr; + + initial begin + string s; + + str.el[0].val.next = 6; + s = $sformatf("%p", str); + $display("%s", s); + `checks(s, "'{el:'{'{val:'{next:'h6}}} }"); + + pstr.el[0].val.next = 6; + s = $sformatf("%p", pstr); + $display("%s", s); + `checks(s, "'{el:'{'{val:'{next:'h6}}} }"); + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule