diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 74cc9d005..53c28d404 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -167,6 +167,7 @@ public: string cType(const string& name, bool forFunc, bool isRef, bool packed = false) const; // Represents a C++ LiteralType? (can be constexpr) bool isLiteralType() const VL_MT_STABLE; + virtual bool isDynamicallySized() const { return false; } private: class CTypeRecursed; @@ -383,6 +384,7 @@ public: int widthAlignBytes() const override { return subDTypep()->widthAlignBytes(); } int widthTotalBytes() const override { return subDTypep()->widthTotalBytes(); } bool isCompound() const override { return true; } + bool isDynamicallySized() const override { return true; } }; class AstBasicDType final : public AstNodeDType { // Builtin atomic/vectored data type @@ -756,6 +758,7 @@ public: int widthAlignBytes() const override { return subDTypep()->widthAlignBytes(); } int widthTotalBytes() const override { return subDTypep()->widthTotalBytes(); } bool isCompound() const override { return true; } + bool isDynamicallySized() const override { return true; } }; class AstEmptyQueueDType final : public AstNodeDType { // For EmptyQueue @@ -1124,6 +1127,7 @@ public: int widthAlignBytes() const override { return subDTypep()->widthAlignBytes(); } int widthTotalBytes() const override { return subDTypep()->widthTotalBytes(); } bool isCompound() const override { return true; } + bool isDynamicallySized() const override { return true; } }; class AstRefDType final : public AstNodeDType { // @astgen op1 := typeofp : Optional[AstNode] diff --git a/src/V3WidthCommit.cpp b/src/V3WidthCommit.cpp index 0f1c841ba..f94eb0788 100644 --- a/src/V3WidthCommit.cpp +++ b/src/V3WidthCommit.cpp @@ -42,6 +42,7 @@ class WidthCommitVisitor final : public VNVisitor { // STATE AstNodeModule* m_modp = nullptr; + std::string m_contNba; // In continuous- or non-blocking assignment VMemberMap m_memberMap; // Member names cached for fast lookup public: @@ -124,6 +125,26 @@ private: } } } + void varLifetimeCheck(AstNode* nodep, AstVar* varp) { + if (!m_contNba.empty()) { + std::string varType; + const AstNodeDType* const varDtp = varp->dtypep()->skipRefp(); + if (varp->lifetime().isAutomatic() && !VN_IS(varDtp, IfaceRefDType) + && !(varp->isFuncLocal() && varp->isIO())) + varType = "Automatic lifetime"; + else if (varp->isClassMember() && !varp->lifetime().isStatic() + && !VN_IS(varDtp, IfaceRefDType)) + varType = "Class non-static"; + else if (varDtp->isDynamicallySized()) + varType = "Dynamically-sized"; + if (!varType.empty()) { + UINFO(1, " Related var dtype: " << varDtp); + nodep->v3error(varType + << " variable not allowed in " << m_contNba + << " assignment (IEEE 1800-2023 6.21): " << varp->prettyNameQ()); + } + } + } // VISITORS void visit(AstNodeModule* nodep) override { @@ -278,6 +299,27 @@ private: iterateChildren(nodep); editDType(nodep); classEncapCheck(nodep, nodep->varp(), VN_CAST(nodep->classOrPackagep(), Class)); + if (nodep->access().isWriteOrRW()) varLifetimeCheck(nodep, nodep->varp()); + } + void visit(AstAssignDly* nodep) override { + iterateAndNextNull(nodep->timingControlp()); + iterateAndNextNull(nodep->rhsp()); + { + VL_RESTORER(m_contNba); + m_contNba = "nonblocking"; + iterateAndNextNull(nodep->lhsp()); + } + editDType(nodep); + } + void visit(AstAssignW* nodep) override { + iterateAndNextNull(nodep->timingControlp()); + iterateAndNextNull(nodep->rhsp()); + { + VL_RESTORER(m_contNba); + m_contNba = "continuous"; + iterateAndNextNull(nodep->lhsp()); + } + editDType(nodep); } void visit(AstNodeFTaskRef* nodep) override { iterateChildren(nodep); @@ -290,6 +332,7 @@ private: if (auto* const classrefp = VN_CAST(nodep->fromp()->dtypep(), ClassRefDType)) { classEncapCheck(nodep, nodep->varp(), classrefp->classp()); } // else might be struct, etc + varLifetimeCheck(nodep, nodep->varp()); } void visit(AstVar* nodep) override { iterateChildren(nodep); diff --git a/test_regress/t/t_assign_automatic_bad.out b/test_regress/t/t_assign_automatic_bad.out new file mode 100644 index 000000000..6fb3bcec9 --- /dev/null +++ b/test_regress/t/t_assign_automatic_bad.out @@ -0,0 +1,26 @@ +%Error: t/t_assign_automatic_bad.v:31:10: Automatic lifetime variable not allowed in continuous assignment (IEEE 1800-2023 6.21): 'bad_auto3' + : ... note: In instance 't' + 31 | assign bad_auto3 = 2; + | ^~~~~~~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_assign_automatic_bad.v:32:10: Dynamically-sized variable not allowed in continuous assignment (IEEE 1800-2023 6.21): 'bad_dyn5' + : ... note: In instance 't' + 32 | assign bad_dyn5 = empty_dyn; + | ^~~~~~~~ +%Error: t/t_assign_automatic_bad.v:33:12: Automatic lifetime variable not allowed in continuous assignment (IEEE 1800-2023 6.21): 'm_bad1' + : ... note: In instance 't' + 33 | assign c.m_bad1 = 2; + | ^~~~~~ +%Error: t/t_assign_automatic_bad.v:43:5: Automatic lifetime variable not allowed in nonblocking assignment (IEEE 1800-2023 6.21): 'bad_auto4' + : ... note: In instance 't' + 43 | bad_auto4 <= 2; + | ^~~~~~~~~ +%Error: t/t_assign_automatic_bad.v:44:5: Dynamically-sized variable not allowed in nonblocking assignment (IEEE 1800-2023 6.21): 'bad_dyn6' + : ... note: In instance 't' + 44 | bad_dyn6 <= empty_dyn; + | ^~~~~~~~ +%Error: t/t_assign_automatic_bad.v:46:7: Automatic lifetime variable not allowed in nonblocking assignment (IEEE 1800-2023 6.21): 'm_bad2' + : ... note: In instance 't' + 46 | c.m_bad2 <= 2; + | ^~~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_assign_automatic_bad.py b/test_regress/t/t_assign_automatic_bad.py new file mode 100755 index 000000000..55203b6c9 --- /dev/null +++ b/test_regress/t/t_assign_automatic_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_assign_automatic_bad.v b/test_regress/t/t_assign_automatic_bad.v new file mode 100644 index 000000000..98905ca97 --- /dev/null +++ b/test_regress/t/t_assign_automatic_bad.v @@ -0,0 +1,51 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +// 6.21 Scope and lifetime +// Automatic variables and elements of dynamically sized array variables shall +// not be written with nonblocking, continuous, or procedural continuous +// assignments. Non-static class properties shall not be written with continuous +// or procedural continuous assignments. + +class Cls; + static int s_ok1; + static int s_ok2; + int m_bad1; + int m_bad2; +endclass + +module t(clk); + input clk; + + Cls c; + + automatic int bad_auto3; + automatic int bad_auto4; + int bad_dyn5[]; + int bad_dyn6[]; + int empty_dyn[]; + + assign bad_auto3 = 2; // <--- Error: continuous automatic + assign bad_dyn5 = empty_dyn; // <--- Error: continuous dynarray + assign c.m_bad1 = 2; // <--- Error: continuous class non-static + // Only one simulator fails on this, probably not legal + // assign Cls::s_ok1 = 2; // OK: continuous class static + + logic ok_7; + task mt(output o); // OK: function output + o <= 1; + endtask + + always @(posedge clk) begin + bad_auto4 <= 2; // <--- Error: nonblocking automatic + bad_dyn6 <= empty_dyn; // <--- Error: nonblocking dynarray + Cls::s_ok2 <= 2; // OK: nonblocking class static + c.m_bad2 <= 2; // <--- Error: nonblocking class automatic + mt(ok_7); + $stop; + end + +endmodule diff --git a/test_regress/t/t_assigndly_dynamic.v b/test_regress/t/t_assigndly_dynamic.v index 7a403fa9b..391174165 100644 --- a/test_regress/t/t_assigndly_dynamic.v +++ b/test_regress/t/t_assigndly_dynamic.v @@ -17,7 +17,7 @@ class nba_waiter; // Task taken from UVM task wait_for_nba_region; - int nba; + static int nba; int next_nba; next_nba++; nba <= `DELAY next_nba; @@ -27,19 +27,14 @@ endclass class Foo; task bar(logic a, logic b); - int x; - int y; + static int x; + static int y; // bar's local vars and intravals could be overwritten by other locals if (a) x <= `DELAY 'hDEAD; if (b) y <= `DELAY 'hBEEF; #2 if (x != 'hDEAD) $stop; endtask - - task qux(); - int x[] = new[1]; - x[0] <= `DELAY 'hBEEF; // Segfault check - endtask endclass module t; @@ -61,7 +56,6 @@ module t; if (cnt != 4) $stop; if ($time != `TIME_AFTER_SECOND_WAIT) $stop; foo.bar(1, 1); - foo.qux(); #2 $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_assigndly_dynamic_notiming_bad.v b/test_regress/t/t_assigndly_dynamic_notiming_bad.v index b2f77420a..b439348c8 100644 --- a/test_regress/t/t_assigndly_dynamic_notiming_bad.v +++ b/test_regress/t/t_assigndly_dynamic_notiming_bad.v @@ -6,7 +6,7 @@ class Cls; task bar; - int qux; + static int qux; qux <= '1; endtask endclass diff --git a/test_regress/t/t_enum_type_pins.v b/test_regress/t/t_enum_type_pins.v index eaeb9d3d5..a7f2b899a 100644 --- a/test_regress/t/t_enum_type_pins.v +++ b/test_regress/t/t_enum_type_pins.v @@ -76,7 +76,7 @@ module test (/*AUTOARG*/ // Use the enumeration size to initialize a dynamic array t_pinid e; - int myarray1 [] = new [e.num]; + int myarray1 [] = new [e.num]; always @(posedge clk) begin @@ -87,7 +87,7 @@ module test (/*AUTOARG*/ e = e.first; forever begin - myarray1[e] <= e.prev; + myarray1[e] = e.prev; `ifdef TEST_VERBOSE $write ("myarray1[%d] (enum %s) = %d\n", e, e.name, myarray1[e]); diff --git a/test_regress/t/t_event_control_expr.v b/test_regress/t/t_event_control_expr.v index 74456d898..d544b419a 100644 --- a/test_regress/t/t_event_control_expr.v +++ b/test_regress/t/t_event_control_expr.v @@ -63,7 +63,7 @@ function int id(int x); return x; endfunction `define CLASS_TEST(name, expr) \ module t_``name(input int k); \ class Cls; \ - int k; \ + static int k; \ function int get_k(); return k; endfunction \ endclass \ Cls obj = new; \ diff --git a/test_regress/t/t_export_packed_struct2.cpp b/test_regress/t/t_export_packed_struct2.cpp index 6357902ef..c97e39392 100644 --- a/test_regress/t/t_export_packed_struct2.cpp +++ b/test_regress/t/t_export_packed_struct2.cpp @@ -92,7 +92,7 @@ int main(int argc, char** argv) { std::memset(reinterpret_cast(&tmp), 0xff, sizeof(tmp)); // `set` function should clear upper bits of `tmp.a` - tmp.set(adder->rootp->add__DOT__op2->__PVT__in); + tmp.set(adder->rootp->add__DOT__op2); for (int i = 0; i < 3; ++i) { for (int j = 0; j < 3; ++j) { diff --git a/test_regress/t/t_export_packed_struct2.v b/test_regress/t/t_export_packed_struct2.v index 885c23313..5a67b0842 100644 --- a/test_regress/t/t_export_packed_struct2.v +++ b/test_regress/t/t_export_packed_struct2.v @@ -6,10 +6,10 @@ // Packed struct in package package TEST_TYPES; - typedef union soft packed { - logic [64 : 0] a; - logic [2 : 0] b; - } sub_t; + typedef union soft packed { + logic [64 : 0] a; + logic [2 : 0] b; + } sub_t; typedef struct packed { struct packed { // Anonymous packed struct logic a; @@ -28,7 +28,6 @@ class cls_in; logic a; TEST_TYPES::sub_t [2:0][2:0][2:0] b; } in_t /*verilator public*/; - in_t in; endclass //cls module add ( @@ -36,31 +35,25 @@ module add ( //input cls_in op2, output TEST_TYPES::out_t out ); - cls_in op2 /*verilator public_flat*/; + cls_in::in_t op2 /*verilator public_flat*/; - initial begin - if(op2 != null) $stop; - op2 = new(); - if(!op2) $stop; - end - - assign op2.in.a = op1.anon.a; + assign op2.a = op1.anon.a; generate for (genvar i = 0; i < 3; ++i) begin for (genvar j = 0; j < 3; ++j) begin for (genvar k = 0; k < 3; ++k) begin - assign op2.in.b[i][j][k] = op1.b[i][j][k]; + assign op2.b[i][j][k] = op1.b[i][j][k]; end end end endgenerate - assign out.anon.a = op1.anon.a + op2.in.a; + assign out.anon.a = op1.anon.a + op2.a; generate for (genvar i = 0; i < 3; ++i) begin for (genvar j = 0; j < 3; ++j) begin for (genvar k = 0; k < 3; ++k) begin - assign out.b[i][j][k] = op1.b[i][j][k] + op2.in.b[i][j][k]; + assign out.b[i][j][k] = op1.b[i][j][k] + op2.b[i][j][k]; end end end diff --git a/test_regress/t/t_interface_array4.v b/test_regress/t/t_interface_array4.v index f16386c9b..f3042fa2d 100644 --- a/test_regress/t/t_interface_array4.v +++ b/test_regress/t/t_interface_array4.v @@ -29,7 +29,7 @@ endmodule module devB (Ifc s); endmodule -module top; +module t; Ifc s14[1:4] (); devA a1 (s14[1]); devB b1 (s14[2]); diff --git a/test_regress/t/t_savable.v b/test_regress/t/t_savable.v index 8b07a7814..e9acda757 100644 --- a/test_regress/t/t_savable.v +++ b/test_regress/t/t_savable.v @@ -69,12 +69,13 @@ module sub (/*AUTOARG*/ pvec[2][2] <= 32'h10202; r <= 1.234; s <= "hello"; - sarr[1] <= "sarr[1]"; - sarr[2] <= "sarr[2]"; - assoc["mapped"] <= "Is mapped"; + // Blocking to avoid delayed to dynamic var + sarr[1] = "sarr[1]"; + sarr[2] = "sarr[2]"; + assoc["mapped"] = "Is mapped"; end if (cyc==1) begin - if ($test$plusargs("save_restore")!=0) begin + if ($test$plusargs("save_restore") != 0) begin // Don't allow the restored model to run from time 0, it must run from a restore $write("%%Error: didn't really restore\n"); $stop; diff --git a/test_regress/t/t_sys_readmem_assoc.v b/test_regress/t/t_sys_readmem_assoc.v index 7d2487f31..d2a123837 100644 --- a/test_regress/t/t_sys_readmem_assoc.v +++ b/test_regress/t/t_sys_readmem_assoc.v @@ -7,9 +7,9 @@ `define STRINGIFY(x) `"x`" module t(/*AUTOARG*/ - // Inputs - clk - ); + // Inputs + clk + ); input clk; int cyc; @@ -20,7 +20,7 @@ module t(/*AUTOARG*/ always_ff @ (posedge clk) begin cyc <= cyc + 1; if (cyc == 1) begin - assoc_c[300] <= 10; // See if clearing must happen first + assoc_c[300] = 10; // See if clearing must happen first // Also checks no BLKANDNBLK due to readmem/writemem end else if (cyc == 2) begin diff --git a/test_regress/t/t_virtual_interface_member_trigger_realistic_case.v b/test_regress/t/t_virtual_interface_member_trigger_realistic_case.v index 2674d7a0a..617f94732 100755 --- a/test_regress/t/t_virtual_interface_member_trigger_realistic_case.v +++ b/test_regress/t/t_virtual_interface_member_trigger_realistic_case.v @@ -57,7 +57,7 @@ class intf_driver; endtask endclass -module t_virtual_interface_member_trigger_realistic_case(); +module t; logic clk; logic [7:0] data; logic valid;