From 94ef1b76d02e149f0040122c6f5d8f6364652fa5 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 13 Feb 2023 20:58:49 -0500 Subject: [PATCH] Fix error text on packed struct init. --- src/V3AstNodeDType.h | 5 ++- src/V3Width.cpp | 35 +++++++++++++++------ src/verilog.y | 13 +++++--- test_regress/t/t_dist_warn_coverage.pl | 2 -- test_regress/t/t_struct_packed_init_bad.out | 5 +++ test_regress/t/t_struct_packed_init_bad.pl | 21 +++++++++++++ test_regress/t/t_struct_packed_init_bad.v | 21 +++++++++++++ test_regress/t/t_struct_unpacked_init.out | 6 ++++ test_regress/t/t_struct_unpacked_init.pl | 19 +++++++++++ test_regress/t/t_struct_unpacked_init.v | 25 +++++++++++++++ 10 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 test_regress/t/t_struct_packed_init_bad.out create mode 100755 test_regress/t/t_struct_packed_init_bad.pl create mode 100644 test_regress/t/t_struct_packed_init_bad.v create mode 100644 test_regress/t/t_struct_unpacked_init.out create mode 100755 test_regress/t/t_struct_unpacked_init.pl create mode 100644 test_regress/t/t_struct_unpacked_init.v diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index ce2115f65..fd652f595 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -845,6 +845,7 @@ class AstMemberDType final : public AstNodeDType { // A member of a struct/union // PARENT: AstNodeUOrStructDType // @astgen op1 := childDTypep : Optional[AstNodeDType] + // @astgen op3 := valuep : Optional[AstNode] private: AstNodeDType* m_refDTypep = nullptr; // Elements of this type (after widthing) string m_name; // Name of variable @@ -852,10 +853,12 @@ private: int m_lsb = -1; // Within this level's packed struct, the LSB of the first bit of the member // UNSUP: int m_randType; // Randomization type (IEEE) public: - AstMemberDType(FileLine* fl, const string& name, VFlagChildDType, AstNodeDType* dtp) + AstMemberDType(FileLine* fl, const string& name, VFlagChildDType, AstNodeDType* dtp, + AstNode* valuep) : ASTGEN_SUPER_MemberDType(fl) , m_name{name} { childDTypep(dtp); // Only for parser + this->valuep(valuep); dtypep(nullptr); // V3Width will resolve refDTypep(nullptr); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 4bed49e02..caeb19dfa 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2534,22 +2534,33 @@ private: nodep->repairMemberCache(); nodep->dtypep(nodep); nodep->isFourstate(false); + // Error checks + for (AstMemberDType* itemp = nodep->membersp(); itemp; + itemp = VN_AS(itemp->nextp(), MemberDType)) { + AstNodeDType* const dtp = itemp->subDTypep()->skipRefp(); + if (nodep->packed() + && !dtp->isIntegralOrPacked() + // Historically lax: + && !v3Global.opt.structsPacked()) + itemp->v3error("Unpacked data type " + << dtp->prettyDTypeNameQ() + << " in packed struct/union (IEEE 1800-2017 7.2.1)"); + if ((VN_IS(nodep, UnionDType) || nodep->packed()) && itemp->valuep()) { + itemp->v3error("Initial values not allowed in packed struct/union" + " (IEEE 1800-2017 7.2.1)"); + pushDeletep(itemp->valuep()->unlinkFrBack()); + } else if (itemp->valuep()) { + itemp->valuep()->v3warn(E_UNSUPPORTED, + "Unsupported: Initial values in struct/union members"); + pushDeletep(itemp->valuep()->unlinkFrBack()); + } + } // Determine bit assignments and width if (VN_IS(nodep, UnionDType) || nodep->packed()) { int lsb = 0; int width = 0; // Report errors on first member first AstMemberDType* itemp; - for (itemp = nodep->membersp(); itemp; itemp = VN_AS(itemp->nextp(), MemberDType)) { - AstNodeDType* const dtp = itemp->subDTypep()->skipRefp(); - if (nodep->packed() - && !dtp->isIntegralOrPacked() - // Historically lax: - && !v3Global.opt.structsPacked()) - itemp->v3error("Unpacked data type " - << dtp->prettyDTypeNameQ() - << " in packed struct/union (IEEE 1800-2017 7.2.1)"); - } // MSB is first, so loop backwards for (itemp = nodep->membersp(); itemp && itemp->nextp(); itemp = VN_AS(itemp->nextp(), MemberDType)) {} @@ -2615,6 +2626,10 @@ private: nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep())); nodep->dtypep(nodep); // The member itself, not subDtype nodep->widthFromSub(nodep->subDTypep()); + if (nodep->valuep()) { + userIterateAndNext(nodep->valuep(), WidthVP{nodep->dtypep(), PRELIM}.p()); + iterateCheckAssign(nodep, "Initial value", nodep->valuep(), FINAL, nodep->dtypep()); + } } void visit(AstStructSel* nodep) override { userIterateChildren(nodep, WidthVP{SELF, BOTH}.p()); diff --git a/src/verilog.y b/src/verilog.y index 9f708da32..3a05c499e 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -2191,13 +2191,16 @@ member_decl_assignment: // Derived from IEEE: variable_decl_assi // // So this is different from variable_decl_assignment id variable_dimensionListE { $$ = new AstMemberDType{$1, *$1, VFlagChildDType{}, - GRAMMARP->createArray(AstNodeDType::cloneTreeNull(GRAMMARP->m_memDTypep, true), $2, false)}; + GRAMMARP->createArray(AstNodeDType::cloneTreeNull(GRAMMARP->m_memDTypep, true), $2, false), + nullptr}; PARSEP->tagNodep($$); - } + } | id variable_dimensionListE '=' variable_declExpr - { BBUNSUP($4, "Unsupported: Initial values in struct/union members."); - // But still need error if packed according to IEEE 7.2.2 - $$ = nullptr; } + { $$ = new AstMemberDType{$1, *$1, VFlagChildDType{}, + GRAMMARP->createArray(AstNodeDType::cloneTreeNull(GRAMMARP->m_memDTypep, true), $2, false), + $4}; + PARSEP->tagNodep($$); + } | idSVKwd { $$ = nullptr; } // // // IEEE: "dynamic_array_variable_identifier '[' ']' [ '=' dynamic_array_new ]" diff --git a/test_regress/t/t_dist_warn_coverage.pl b/test_regress/t/t_dist_warn_coverage.pl index febbe85c2..b7eb7e1d7 100755 --- a/test_regress/t/t_dist_warn_coverage.pl +++ b/test_regress/t/t_dist_warn_coverage.pl @@ -26,7 +26,6 @@ foreach my $s ( 'Enum names without values only allowed on numeric types', # Hard to hit 'Enum ranges must be integral, per spec', # Hard to hit 'Return with return value isn\'t underneath a function', # Hard to hit, get other bad return messages - 'Select from non-array ', # Instead get type does not have a bit range 'Syntax error parsing real: \'', # Instead can't lex the number 'Unsupported: Ranges ignored in port-lists', # Hard to hit 'dynamic new() not expected in this context (expected under an assign)', # Instead get syntax error @@ -53,7 +52,6 @@ foreach my $s ( 'Exceeded limit of ', 'Extern declaration\'s scope is not a defined class', 'Format to $display-like function must have constant format string', - 'Forward typedef used as class/package does not resolve to class/package: ', 'Illegal +: or -: select; type already selected, or bad dimension: ', 'Illegal bit or array select; type already selected, or bad dimension: ', 'Illegal range select; type already selected, or bad dimension: ', diff --git a/test_regress/t/t_struct_packed_init_bad.out b/test_regress/t/t_struct_packed_init_bad.out new file mode 100644 index 000000000..90d48e049 --- /dev/null +++ b/test_regress/t/t_struct_packed_init_bad.out @@ -0,0 +1,5 @@ +%Error: t/t_struct_packed_init_bad.v:12:17: Initial values not allowed in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 12 | bit [3:0] m_lo = P; + | ^~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_struct_packed_init_bad.pl b/test_regress/t/t_struct_packed_init_bad.pl new file mode 100755 index 000000000..35096464e --- /dev/null +++ b/test_regress/t/t_struct_packed_init_bad.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(linter => 1); + +lint( + fails => $Self->{vlt_all}, + expect_filename => $Self->{golden_filename}, + ); + +#execute() + +ok(1); +1; diff --git a/test_regress/t/t_struct_packed_init_bad.v b/test_regress/t/t_struct_packed_init_bad.v new file mode 100644 index 000000000..98d81d2f0 --- /dev/null +++ b/test_regress/t/t_struct_packed_init_bad.v @@ -0,0 +1,21 @@ +// 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 + +module t(/*AUTOARG*/); + + parameter P = 4'h5; + + struct packed { + bit [3:0] m_lo = P; // Bad + bit [3:0] m_hi; + } s; + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_struct_unpacked_init.out b/test_regress/t/t_struct_unpacked_init.out new file mode 100644 index 000000000..9e3d7bc11 --- /dev/null +++ b/test_regress/t/t_struct_unpacked_init.out @@ -0,0 +1,6 @@ +%Error-UNSUPPORTED: t/t_struct_unpacked_init.v:12:24: Unsupported: Initial values in struct/union members + : ... In instance t + 12 | bit [3:0] m_lo = P; + | ^ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_struct_unpacked_init.pl b/test_regress/t/t_struct_unpacked_init.pl new file mode 100755 index 000000000..a60503a1f --- /dev/null +++ b/test_regress/t/t_struct_unpacked_init.pl @@ -0,0 +1,19 @@ +#!/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(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_struct_unpacked_init.v b/test_regress/t/t_struct_unpacked_init.v new file mode 100644 index 000000000..5518ab9e3 --- /dev/null +++ b/test_regress/t/t_struct_unpacked_init.v @@ -0,0 +1,25 @@ +// 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 + +module t(/*AUTOARG*/); + + parameter P = 4'h5; + + struct { // Can't legally be packed + bit [3:0] m_lo = P; + bit [3:0] m_hi; + } s; + + initial begin + s.m_hi = 4'ha; + if (s.m_lo != 4'h5) $stop; + if (s.m_hi != 4'ha) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule