diff --git a/Changes b/Changes index c4fc2df6f..f86b10cd2 100644 --- a/Changes +++ b/Changes @@ -33,6 +33,8 @@ Verilator 5.035 devel * Fix type_id package scope resolution (#5826). [Krzysztof Bieganski, Antmicro Ltd.] * Fix `rand_mode` method with cast (#5831). * Fix invalidating variable caches in SenExprBulider (#5834) (#5835). [Geza Lore] +* Fix assignment pattern as function argument (#5839) +* Fix checking built-in method arguments (#5839) Verilator 5.034 2025-02-24 diff --git a/src/V3Ast.h b/src/V3Ast.h index ba483fa94..6838be5ae 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -2359,6 +2359,7 @@ public: AstNodeDType* findBitDType() const { return findBasicDType(VBasicDTypeKwd::LOGIC); } AstNodeDType* findDoubleDType() const { return findBasicDType(VBasicDTypeKwd::DOUBLE); } AstNodeDType* findStringDType() const { return findBasicDType(VBasicDTypeKwd::STRING); } + AstNodeDType* findSigned8DType() const { return findBasicDType(VBasicDTypeKwd::BYTE); } AstNodeDType* findSigned32DType() const { return findBasicDType(VBasicDTypeKwd::INTEGER); } AstNodeDType* findUInt32DType() const { return findBasicDType(VBasicDTypeKwd::UINT32); } AstNodeDType* findUInt64DType() const { return findBasicDType(VBasicDTypeKwd::UINT64); } diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp index 1f1842e05..109bc5248 100644 --- a/src/V3Randomize.cpp +++ b/src/V3Randomize.cpp @@ -392,6 +392,7 @@ class RandomizeMarkVisitor final : public VNVisitor { exprp = nullptr; } if (randVarp == fromVarp) break; + UASSERT_OBJ(randVarp, nodep, "No rand variable found"); AstNode* backp = randVarp; while (backp && !VN_IS(backp, Class)) backp = backp->backp(); RandomizeMode randMode = {}; diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 54db85a50..46f8e43d8 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -392,7 +392,7 @@ class WidthVisitor final : public VNVisitor { // See similar handling in visit_cmp_eq_gt where created iterateCheckString(nodep, "LHS", nodep->lhsp(), BOTH); iterateCheckSigned32(nodep, "RHS", nodep->rhsp(), BOTH); - iterateCheckSigned32(nodep, "THS", nodep->thsp(), BOTH); + iterateCheckSigned8(nodep, "THS", nodep->thsp(), BOTH); nodep->dtypeSetString(); // AstPutcN returns the new string to be assigned by // AstAssign } @@ -3153,14 +3153,8 @@ class WidthVisitor final : public VNVisitor { if (debug() >= 9) nodep->dumpTree("- mts-in: "); // Should check types the method requires, but at present we don't do much userIterate(nodep->fromp(), WidthVP{SELF, BOTH}.p()); + // Args are checked within each particular method's decode // Any AstWith is checked later when know types, in methodWithArgument - for (AstNode* pinp = nodep->pinsp(); pinp; pinp = pinp->nextp()) { - if (AstArg* const argp = VN_CAST(pinp, Arg)) { - if (argp->exprp()) - userIterate(argp->exprp(), - WidthVP{nodep->fromp()->dtypep()->subDTypep(), BOTH}.p()); - } - } // Find the fromp dtype - should be a class UASSERT_OBJ(nodep->fromp() && nodep->fromp()->dtypep(), nodep, "Unsized expression"); AstNodeDType* const fromDtp = nodep->fromp()->dtypep()->skipRefToEnump(); @@ -3252,6 +3246,13 @@ class WidthVisitor final : public VNVisitor { } } + AstNodeExpr* methodArg(AstMethodCall* nodep, int arg) { + AstNode* argp = nodep->pinsp(); + for (int narg = 0; narg < arg; ++narg) argp = argp->nextp(); + UASSERT_OBJ(argp, nodep, "methodOkArguments() should have detected arg count error"); + return VN_AS(argp, Arg)->exprp(); + } + void methodCallEnum(AstMethodCall* nodep, AstEnumDType* adtypep) { // Method call on enum without following parenthesis, e.g. "ENUM.next" // Convert this into a method call, and let that visitor figure out what to do next @@ -3306,12 +3307,13 @@ class WidthVisitor final : public VNVisitor { } if (nodep->name() != "name" && nodep->pinsp()) { - if (!VN_IS(VN_AS(nodep->pinsp(), Arg)->exprp(), Const)) { - nodep->pinsp()->v3fatalSrc( - "Unsupported: enum next/prev with non-const argument"); + AstNodeExpr* const stepp = methodArg(nodep, 0); + nodep->fileline()->modifyWarnOff(V3ErrorCode::WIDTHEXPAND, true); + iterateCheckUInt32(nodep, "argument", stepp, BOTH); + if (!VN_IS(stepp, Const)) { + stepp->v3fatalSrc("Unsupported: enum next/prev with non-const argument"); } else { - const uint32_t stepWidth - = VN_AS(VN_AS(nodep->pinsp(), Arg)->exprp(), Const)->toUInt(); + const uint32_t stepWidth = VN_AS(stepp, Const)->toUInt(); if (stepWidth == 0) { // Step of 0 "legalizes" like $cast, use .next.prev AstMethodCall* const newp = new AstMethodCall{ @@ -3319,17 +3321,20 @@ class WidthVisitor final : public VNVisitor { new AstMethodCall{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "next", nullptr}, "prev", nullptr}; + // No dtype assigned, we will recurse the new method and replace nodep->replaceWith(newp); VL_DO_DANGLING(nodep->deleteTree(), nodep); return; } else if (stepWidth != 1) { // Unroll of enumVar.next(k) to enumVar.next(1).next(k - 1) + nodep->pinsp()->unlinkFrBack(); AstMethodCall* const clonep = nodep->cloneTree(false); - VN_AS(VN_AS(clonep->pinsp(), Arg)->exprp(), Const)->num().setLong(1); + VN_AS(stepp, Const)->num().setLong(1); AstConst* const constp = new AstConst(nodep->fileline(), stepWidth - 1); AstArg* const argp = new AstArg{nodep->fileline(), "", constp}; AstMethodCall* const newp = new AstMethodCall{nodep->fileline(), clonep, nodep->name(), argp}; + // No dtype assigned, we will recurse the new method and replace nodep->replaceWith(newp); VL_DO_DANGLING(nodep->deleteTree(), nodep); return; @@ -3364,6 +3369,7 @@ class WidthVisitor final : public VNVisitor { } else if (nodep->name() == "exists") { // function int exists(input index) // IEEE really should have made this a "bit" return methodOkArguments(nodep, 1, 1); + iterateCheckSizedSelf(nodep, "argument", methodArg(nodep, 0), SELF, BOTH); AstNodeExpr* const index_exprp = methodCallWildcardIndexExpr(nodep, adtypep); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "exists", index_exprp->unlinkFrBack()}; @@ -3376,6 +3382,7 @@ class WidthVisitor final : public VNVisitor { "clear"}; newp->dtypeSetVoid(); } else { + iterateCheckSizedSelf(nodep, "argument", methodArg(nodep, 0), SELF, BOTH); AstNodeExpr* const index_exprp = methodCallWildcardIndexExpr(nodep, adtypep); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "erase", index_exprp->unlinkFrBack()}; @@ -3443,6 +3450,7 @@ class WidthVisitor final : public VNVisitor { || nodep->name() == "next" // || nodep->name() == "prev") { methodOkArguments(nodep, 1, 1); + iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(), BOTH); AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep); methodCallLValueRecurse(nodep, index_exprp, VAccess::READWRITE); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), @@ -3453,6 +3461,7 @@ class WidthVisitor final : public VNVisitor { } else if (nodep->name() == "exists") { // function int exists(input index) // IEEE really should have made this a "bit" return methodOkArguments(nodep, 1, 1); + iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(), BOTH); AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "exists", index_exprp->unlinkFrBack()}; @@ -3465,6 +3474,8 @@ class WidthVisitor final : public VNVisitor { "clear"}; newp->dtypeSetVoid(); } else { + iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(), + BOTH); AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "erase", index_exprp->unlinkFrBack()}; @@ -3643,10 +3654,12 @@ class WidthVisitor final : public VNVisitor { AstCMethodHard* newp = nullptr; if (nodep->name() == "at") { // Created internally for [] methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at"}; newp->dtypeFrom(adtypep->subDTypep()); } else if (nodep->name() == "atWrite") { // Created internally for [] methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "atWrite"}; @@ -3689,12 +3702,14 @@ class WidthVisitor final : public VNVisitor { AstCMethodHard* newp = nullptr; if (nodep->name() == "at" || nodep->name() == "atBack") { // Created internally for [] methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), nodep->name()}; newp->dtypeFrom(adtypep->subDTypep()); } else if (nodep->name() == "atWriteAppend" || nodep->name() == "atWriteAppendBack") { // Created internally for [] methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), nodep->name()}; @@ -3712,6 +3727,7 @@ class WidthVisitor final : public VNVisitor { "clear"}; newp->dtypeSetVoid(); } else { + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); AstNodeExpr* const index_exprp = methodCallQueueIndexExpr(nodep); if (index_exprp->isZero()) { // delete(0) is a pop_front newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), @@ -3726,6 +3742,7 @@ class WidthVisitor final : public VNVisitor { } } else if (nodep->name() == "insert") { methodOkArguments(nodep, 2, 2); + iterateCheckSigned32(nodep, "index", methodArg(nodep, 0), BOTH); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); AstNodeExpr* const index_exprp = methodCallQueueIndexExpr(nodep); AstArg* const argp = VN_AS(nodep->pinsp()->nextp(), Arg); @@ -3750,6 +3767,7 @@ class WidthVisitor final : public VNVisitor { if (nodep->isStandaloneBodyStmt()) newp->dtypeSetVoid(); } else if (nodep->name() == "push_back" || nodep->name() == "push_front") { methodOkArguments(nodep, 1, 1); + iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->subDTypep(), BOTH); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); AstArg* const argp = VN_AS(nodep->pinsp(), Arg); iterateCheckTyped(nodep, "push value", argp->exprp(), adtypep->subDTypep(), BOTH); @@ -3910,11 +3928,17 @@ class WidthVisitor final : public VNVisitor { m_randomizeFromp = nodep->fromp(); withp = methodWithArgument(nodep, false, false, adtypep->findVoidDType(), adtypep->findBitDType(), adtypep); + for (AstNode* pinp = nodep->pinsp(); pinp; pinp = pinp->nextp()) { + if (AstArg* const argp = VN_CAST(pinp, Arg)) { + if (argp->exprp()) userIterate(argp->exprp(), WidthVP{SELF, BOTH}.p()); + } + } methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); V3Randomize::newRandomizeFunc(m_memberMap, first_classp); handleRandomizeArgs(nodep, first_classp); } else if (nodep->name() == "srandom") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); V3Randomize::newSRandomFunc(m_memberMap, first_classp); } @@ -4010,6 +4034,7 @@ class WidthVisitor final : public VNVisitor { // IEEE 1800-2023 18.9 methodOkArguments(nodep, 0, 1); if (nodep->pinsp()) { + iterateCheckBool(nodep, "argument", methodArg(nodep, 0), BOTH); nodep->dtypep(nodep->findBasicDType(VBasicDTypeKwd::INT)); } else { nodep->dtypeSetVoid(); @@ -4025,6 +4050,7 @@ class WidthVisitor final : public VNVisitor { methodOkArguments(nodep, 0, 1); // IEEE 1800-2023 18.8 if (nodep->pinsp()) { + iterateCheckBool(nodep, "argument", methodArg(nodep, 0), BOTH); nodep->dtypeSetVoid(); } else { nodep->dtypep(nodep->findBasicDType(VBasicDTypeKwd::INT)); @@ -4109,18 +4135,23 @@ class WidthVisitor final : public VNVisitor { VL_DO_DANGLING(pushDeletep(nodep), nodep); } else if (nodep->name() == "itoa") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); VL_DO_DANGLING(replaceWithSFormat(nodep, "%0d"), nodep); } else if (nodep->name() == "hextoa") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); VL_DO_DANGLING(replaceWithSFormat(nodep, "%0x"), nodep); } else if (nodep->name() == "octtoa") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); VL_DO_DANGLING(replaceWithSFormat(nodep, "%0o"), nodep); } else if (nodep->name() == "bintoa") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); VL_DO_DANGLING(replaceWithSFormat(nodep, "%0b"), nodep); } else if (nodep->name() == "realtoa") { methodOkArguments(nodep, 1, 1); + iterateCheckReal(nodep, "argument", methodArg(nodep, 0), BOTH); VL_DO_DANGLING(replaceWithSFormat(nodep, "%g"), nodep); } else if (nodep->name() == "tolower") { methodOkArguments(nodep, 0, 0); @@ -4137,6 +4168,7 @@ class WidthVisitor final : public VNVisitor { } else if (nodep->name() == "compare" || nodep->name() == "icompare") { const bool ignoreCase = nodep->name()[0] == 'i'; methodOkArguments(nodep, 1, 1); + iterateCheckString(nodep, "argument", methodArg(nodep, 0), BOTH); AstArg* const argp = VN_AS(nodep->pinsp(), Arg); AstNodeExpr* const lhs = nodep->fromp()->unlinkFrBack(); AstNodeExpr* const rhs = argp->exprp()->unlinkFrBack(); @@ -4145,6 +4177,8 @@ class WidthVisitor final : public VNVisitor { VL_DO_DANGLING(nodep->deleteTree(), nodep); } else if (nodep->name() == "putc") { methodOkArguments(nodep, 2, 2); + iterateCheckSigned32(nodep, "argument 0", methodArg(nodep, 0), BOTH); + iterateCheckSigned8(nodep, "argument 1", methodArg(nodep, 1), BOTH); AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg); AstArg* const arg1p = VN_AS(arg0p->nextp(), Arg); AstNodeVarRef* const fromp = VN_AS(nodep->fromp()->unlinkFrBack(), VarRef); @@ -4159,6 +4193,7 @@ class WidthVisitor final : public VNVisitor { VL_DO_DANGLING(nodep->backp()->replaceWith(newp), nodep); } else if (nodep->name() == "getc") { methodOkArguments(nodep, 1, 1); + iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH); AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg); AstNodeExpr* const lhsp = nodep->fromp()->unlinkFrBack(); AstNodeExpr* const rhsp = arg0p->exprp()->unlinkFrBack(); @@ -4167,6 +4202,8 @@ class WidthVisitor final : public VNVisitor { VL_DO_DANGLING(nodep->deleteTree(), nodep); } else if (nodep->name() == "substr") { methodOkArguments(nodep, 2, 2); + iterateCheckSigned32(nodep, "argument 0", methodArg(nodep, 0), BOTH); + iterateCheckSigned32(nodep, "argument 1", methodArg(nodep, 1), BOTH); AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg); AstArg* const arg1p = VN_AS(arg0p->nextp(), Arg); AstNodeExpr* const lhsp = nodep->fromp()->unlinkFrBack(); @@ -6942,10 +6979,18 @@ class WidthVisitor final : public VNVisitor { // otherwise self-determined was correct iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findDoubleDType(), stage); } + void iterateCheckSigned8(AstNode* nodep, const char* side, AstNode* underp, Stage stage) { + // Coerce child to signed8 if not already. Child is self-determined + iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findSigned8DType(), stage); + } void iterateCheckSigned32(AstNode* nodep, const char* side, AstNode* underp, Stage stage) { // Coerce child to signed32 if not already. Child is self-determined iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findSigned32DType(), stage); } + void iterateCheckUInt32(AstNode* nodep, const char* side, AstNode* underp, Stage stage) { + // Coerce child to unsigned32 if not already. Child is self-determined + iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findUInt32DType(), stage); + } void iterateCheckDelay(AstNode* nodep, const char* side, AstNode* underp, Stage stage) { // Coerce child to 64-bit delay if not already. Child is self-determined // underp may change as a result of replacement diff --git a/test_regress/t/t_process_notiming.out b/test_regress/t/t_process_notiming.out index b3934ae0b..4a663d81f 100644 --- a/test_regress/t/t_process_notiming.out +++ b/test_regress/t/t_process_notiming.out @@ -43,12 +43,12 @@ : ... note: In instance 't' 38 | p.srandom(0); | ^~~~~~~ -%Error-NOTIMING: t/t_process.v:39:25: process::get_randstate() requires --timing - : ... note: In instance 't' - 39 | p.set_randstate(p.get_randstate()); - | ^~~~~~~~~~~~~ %Error-NOTIMING: t/t_process.v:39:9: process::set_randstate() requires --timing : ... note: In instance 't' 39 | p.set_randstate(p.get_randstate()); | ^~~~~~~~~~~~~ +%Error-NOTIMING: t/t_process.v:39:25: process::get_randstate() requires --timing + : ... note: In instance 't' + 39 | p.set_randstate(p.get_randstate()); + | ^~~~~~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_queue_arg.py b/test_regress/t/t_queue_arg.py new file mode 100755 index 000000000..f989a35fb --- /dev/null +++ b/test_regress/t/t_queue_arg.py @@ -0,0 +1,18 @@ +#!/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('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_queue_arg.v b/test_regress/t/t_queue_arg.v new file mode 100644 index 000000000..f80aa7f39 --- /dev/null +++ b/test_regress/t/t_queue_arg.v @@ -0,0 +1,44 @@ +// 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 + +`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 { + string name1; + string name2; +} names_t; + +class uvm_queue; + names_t m_queue[$]; + + virtual function void push_back(names_t item); + m_queue.push_back(item); + endfunction +endclass + +module t(/*AUTOARG*/); + + uvm_queue q; + + initial begin + q = new; + // From uvm_queue#(uvm_acs_name_struct) __local_field_names__; + q.push_back('{"n1", "n2"}); + q.push_back('{"m1", "m2"}); + q.m_queue.push_back('{"o1", "o2"}); + $display("%p", q); + `checks(q.m_queue[0].name1, "n1"); + `checks(q.m_queue[0].name2, "n2"); + `checks(q.m_queue[1].name1, "m1"); + `checks(q.m_queue[1].name2, "m2"); + `checks(q.m_queue[2].name1, "o1"); + `checks(q.m_queue[2].name2, "o2"); + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule