diff --git a/Changes b/Changes index 189cbc055..03ac89d67 100644 --- a/Changes +++ b/Changes @@ -84,6 +84,7 @@ Verilator 5.006 2023-01-22 * Fix foreach unnamedblk duplicate error (#3885). [Ilya Barkov] * Fix elaboration of member selected classes (#3890). [Ilya Barkov] * Fix mismatched widths in DFG (#3872). [Geza Lore, Yike Zhou] +* Fix lint for non-integral types in packed structs. Verilator 5.004 2022-12-14 diff --git a/src/V3Ast.h b/src/V3Ast.h index c4f9e6dd4..63aa59ea8 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -566,7 +566,8 @@ public: } bool isIntNumeric() const { // Enum increment supported return (m_e == BIT || m_e == BYTE || m_e == INT || m_e == INTEGER || m_e == LOGIC - || m_e == LONGINT || m_e == SHORTINT || m_e == UINT32 || m_e == UINT64); + || m_e == LONGINT || m_e == SHORTINT || m_e == UINT32 || m_e == UINT64 + || m_e == TIME); } bool isBitLogic() const { // Bit/logic vector types; can form a packed array return (m_e == LOGIC || m_e == BIT); diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 6639b6c9d..ce2115f65 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -60,6 +60,8 @@ public: /// are compound when methods calls operate on object, or when /// under another compound-requiring object e.g. class virtual bool isCompound() const = 0; + // Integral or packed, allowed inside an unpacked union/struct + virtual bool isIntegralOrPacked() const { return !isCompound(); } // (Slow) recurse down to find basic data type virtual AstBasicDType* basicp() const = 0; // recurses over typedefs/const/enum to next non-typeref type @@ -473,6 +475,7 @@ public: VNumRange declRange() const { return isRanged() ? VNumRange{left(), right()} : VNumRange{}; } void cvtRangeConst(); // Convert to smaller representation bool isCompound() const override { return isString(); } + bool isIntegralOrPacked() const override { return keyword().isIntNumeric(); } }; class AstBracketArrayDType final : public AstNodeDType { // Associative/Queue/Normal array data type, ie "[dtype_or_expr]" @@ -1319,6 +1322,7 @@ public: std::vector unpackDimensions(); void isCompound(bool flag) { m_isCompound = flag; } bool isCompound() const override VL_MT_SAFE { return m_isCompound; } + bool isIntegralOrPacked() const override { return false; } }; // === AstNodeUOrStructDType === diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index acd53aa89..25cc94a67 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -675,7 +675,7 @@ AstVar* AstVar::scVarRecurse(AstNode* nodep) { return nullptr; } -bool AstNodeDType::isFourstate() const { return basicp()->isFourstate(); } +bool AstNodeDType::isFourstate() const { return basicp() && basicp()->isFourstate(); } class AstNodeDType::CTypeRecursed final { public: diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 194fb7cd9..2ffbec4ca 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2512,9 +2512,9 @@ private: } void visit(AstNodeUOrStructDType* nodep) override { if (nodep->didWidthAndSet()) return; // This node is a dtype & not both PRELIMed+FINALed - UINFO(5, " NODECLASS " << nodep << endl); + UINFO(5, " NODEUORS " << nodep << endl); // if (debug() >= 9) nodep->dumpTree("- class-in: "); - if (!nodep->packed() && v3Global.opt.structsPacked()) { nodep->packed(true); } + if (!nodep->packed() && v3Global.opt.structsPacked()) nodep->packed(true); userIterateChildren(nodep, nullptr); // First size all members nodep->repairMemberCache(); nodep->dtypep(nodep); @@ -2523,16 +2523,23 @@ private: if (VN_IS(nodep, UnionDType) || nodep->packed()) { int lsb = 0; int width = 0; - // MSB is first, so go backwards + // 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)) {} - for (AstMemberDType* backip; itemp; itemp = backip) { - if (itemp->skipRefp()->isCompound()) - itemp->v3error( - "Unpacked data type in packed struct/union (IEEE 1800-2017 7.2.1)"); + for (; itemp; itemp = VN_CAST(itemp->backp(), MemberDType)) { if (itemp->isFourstate()) nodep->isFourstate(true); - backip = VN_CAST(itemp->backp(), MemberDType); itemp->lsb(lsb); if (VN_IS(nodep, UnionDType)) { width = std::max(width, itemp->width()); diff --git a/test_regress/t/t_flag_structs_packed_bad.out b/test_regress/t/t_flag_structs_packed_bad.out index 3ca15d47c..43068e836 100644 --- a/test_regress/t/t_flag_structs_packed_bad.out +++ b/test_regress/t/t_flag_structs_packed_bad.out @@ -1,4 +1,4 @@ -%Error: t/t_flag_structs_packed.v:14:19: Unpacked data type in packed struct/union (IEEE 1800-2017 7.2.1) +%Error: t/t_flag_structs_packed.v:14:19: Unpacked data type 'STRUCTDTYPE 'x.notpacked_t'' in packed struct/union (IEEE 1800-2017 7.2.1) : ... In instance x 14 | notpacked_t b; | ^ diff --git a/test_regress/t/t_struct_contents_bad.out b/test_regress/t/t_struct_contents_bad.out new file mode 100644 index 000000000..c39da5643 --- /dev/null +++ b/test_regress/t/t_struct_contents_bad.out @@ -0,0 +1,41 @@ +%Error: t/t_struct_contents_bad.v:20:19: Unpacked data type 'real' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 20 | real r; + | ^ +%Error: t/t_struct_contents_bad.v:22:19: Unpacked data type 'real' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 22 | shortreal sr; + | ^~ +%Error: t/t_struct_contents_bad.v:23:19: Unpacked data type 'real' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 23 | realtime rt; + | ^~ +%Error: t/t_struct_contents_bad.v:24:19: Unpacked data type 'chandle' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 24 | chandle ch; + | ^~ +%Error: t/t_struct_contents_bad.v:25:19: Unpacked data type 'string' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 25 | string s; + | ^ +%Error: t/t_struct_contents_bad.v:26:19: Unpacked data type 'event' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 26 | event e; + | ^ +%Error: t/t_struct_contents_bad.v:27:25: Unpacked data type 'STRUCTDTYPE 't.struct_unpacked_t'' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 27 | struct_unpacked_t sp; + | ^~ +%Error: t/t_struct_contents_bad.v:28:24: Unpacked data type 'UNIONDTYPE 't.union_unpacked_t'' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 28 | union_unpacked_t up; + | ^~ +%Error: t/t_struct_contents_bad.v:29:11: Unpacked data type 'int$[0:1]' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 29 | int uarray[2]; + | ^~~~~~ +%Error: t/t_struct_contents_bad.v:30:11: Unpacked data type 'CLASSREFDTYPE 'Cls'' in packed struct/union (IEEE 1800-2017 7.2.1) + : ... In instance t + 30 | Cls c; + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_struct_contents_bad.pl b/test_regress/t/t_struct_contents_bad.pl new file mode 100755 index 000000000..a60503a1f --- /dev/null +++ b/test_regress/t/t_struct_contents_bad.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_contents_bad.v b/test_regress/t/t_struct_contents_bad.v new file mode 100644 index 000000000..a6c69763e --- /dev/null +++ b/test_regress/t/t_struct_contents_bad.v @@ -0,0 +1,37 @@ +// 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*/); + + typedef enum logic [1:0] { ZERO, ONE } enum_t; + + typedef struct { bit a; } struct_unpacked_t; + typedef union { bit a; } union_unpacked_t; + +class Cls; + bit a; +endclass + + // IEEE 1800-2017 7.2.1 + typedef struct packed { + real r; // BAD + // verilator lint_off SHORTREAL + shortreal sr; // BAD + realtime rt; // BAD + chandle ch; // BAD + string s; // BAD + event e; // BAD + struct_unpacked_t sp; // BAD + union_unpacked_t up; // BAD + int uarray[2]; // BAD + Cls c; // BAd + } illegal_t; + + initial begin + $stop; + end + +endmodule