From 442c9bc3168104aef1f268b6cf8ed9ff0b29d50f Mon Sep 17 00:00:00 2001 From: Arkadiusz Kozdra Date: Thu, 13 Jun 2024 14:38:20 +0200 Subject: [PATCH] Support parsing and otherwise ignoring inline constraints (#5126) --- src/V3AstNodeExpr.h | 10 +-- src/V3LinkDot.cpp | 64 +++++++++++++++++-- src/V3Width.cpp | 34 ++++++++-- src/verilog.y | 6 +- test_regress/t/t_package_local_bad.out | 7 ++ test_regress/t/t_package_local_bad.pl | 19 ++++++ test_regress/t/t_package_local_bad.v | 12 ++++ test_regress/t/t_randomize.out | 7 +- test_regress/t/t_randomize.v | 3 + .../t/t_randomize_method_complex_bad.out | 14 ++++ .../t/t_randomize_method_complex_bad.pl | 19 ++++++ .../t/t_randomize_method_complex_bad.v | 18 ++++++ .../t/t_randomize_method_with_unsup.out | 9 +-- 13 files changed, 193 insertions(+), 29 deletions(-) create mode 100644 test_regress/t/t_package_local_bad.out create mode 100755 test_regress/t/t_package_local_bad.pl create mode 100644 test_regress/t/t_package_local_bad.v create mode 100644 test_regress/t/t_randomize_method_complex_bad.out create mode 100755 test_regress/t/t_randomize_method_complex_bad.pl create mode 100644 test_regress/t/t_randomize_method_complex_bad.v diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 7f66a8bfc..55501bf45 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -2260,7 +2260,7 @@ class AstWith final : public AstNodeExpr { // @astgen op3 := exprp : List[AstNode] public: AstWith(FileLine* fl, AstLambdaArgRef* indexArgRefp, AstLambdaArgRef* valueArgRefp, - AstNodeExpr* exprp) + AstNode* exprp) : ASTGEN_SUPER_With(fl) { this->indexArgRefp(indexArgRefp); this->valueArgRefp(valueArgRefp); @@ -2283,13 +2283,13 @@ class AstWithParse final : public AstNodeExpr { // Replaced with AstWith // Parents: expr|stmt // Children: funcref, expr - // @astgen op1 := funcrefp : AstNode - // @astgen op2 := exprp : Optional[AstNodeExpr] + // @astgen op1 := funcrefp : AstNodeExpr + // @astgen op2 := exprsp : List[AstNode] public: - AstWithParse(FileLine* fl, AstNode* funcrefp, AstNodeExpr* exprp) + AstWithParse(FileLine* fl, AstNodeExpr* funcrefp, AstNode* exprsp) : ASTGEN_SUPER_WithParse(fl) { this->funcrefp(funcrefp); - this->exprp(exprp); + this->addExprsp(exprsp); } ASTGEN_MEMBERS_AstWithParse; bool same(const AstNode* /*samep*/) const override { return true; } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index d6bbb672e..a86918fc5 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1521,11 +1521,11 @@ class LinkDotFindVisitor final : public VNVisitor { VL_DO_DANGLING(argp->unlinkFrBackWithNext()->deleteTree(), argp); } // Type depends on the method used, let V3Width figure it out later - if (nodep->exprp()) { // Else empty expression and pretend no "with" + if (nodep->exprsp()) { // Else empty expression and pretend no "with" const auto indexArgRefp = new AstLambdaArgRef{argFl, name + "__DOT__index", true}; const auto valueArgRefp = new AstLambdaArgRef{argFl, name, false}; const auto newp = new AstWith{nodep->fileline(), indexArgRefp, valueArgRefp, - nodep->exprp()->unlinkFrBackWithNext()}; + nodep->exprsp()->unlinkFrBackWithNext()}; funcrefp->addPinsp(newp); } nodep->replaceWith(funcrefp->unlinkFrBack()); @@ -1545,6 +1545,7 @@ class LinkDotFindVisitor final : public VNVisitor { // Insert argref's name into symbol table m_statep->insertSym(m_curSymp, nodep->valueArgRefp()->name(), nodep->valueArgRefp(), nullptr); + iterateChildren(nodep); } } @@ -2213,16 +2214,21 @@ class LinkDotResolveVisitor final : public VNVisitor { return reinterpret_cast(clockingp->eventp()->user1p()); } + bool isParamedClassRefDType(const AstNode* classp) { + while (const AstRefDType* const refp = VN_CAST(classp, RefDType)) + classp = refp->subDTypep(); + return (VN_IS(classp, ClassRefDType) && VN_AS(classp, ClassRefDType)->paramsp()) + || VN_IS(classp, ParamTypeDType); + } bool isParamedClassRef(const AstNode* nodep) { // Is this a parameterized reference to a class, or a reference to class parameter if (const auto* classRefp = VN_CAST(nodep, ClassOrPackageRef)) { if (classRefp->paramsp()) return true; const auto* classp = classRefp->classOrPackageNodep(); while (const auto* typedefp = VN_CAST(classp, Typedef)) classp = typedefp->subDTypep(); - return (VN_IS(classp, ClassRefDType) && VN_AS(classp, ClassRefDType)->paramsp()) - || VN_IS(classp, ParamTypeDType); + return isParamedClassRefDType(classp); } - return false; + return isParamedClassRefDType(nodep); } VSymEnt* getThisClassSymp() { VSymEnt* classSymp = m_ds.m_dotSymp; @@ -2637,7 +2643,16 @@ class LinkDotResolveVisitor final : public VNVisitor { = VN_AS(m_ds.m_dotp->lhsp(), ClassOrPackageRef); classOrPackagep = cpackagerefp->classOrPackagep(); UASSERT_OBJ(classOrPackagep, m_ds.m_dotp->lhsp(), "Bad package link"); - m_ds.m_dotSymp = m_statep->getNodeSym(classOrPackagep); + if (cpackagerefp->name() == "local::") { + if (m_pinSymp) { + m_ds.m_dotSymp = m_curSymp->fallbackp(); + } else { + nodep->v3error("Illegal 'local::' outside 'randomize() with'"); + m_ds.m_dotErr = true; + } + } else { + m_ds.m_dotSymp = m_statep->getNodeSym(classOrPackagep); + } m_ds.m_dotPos = DP_SCOPE; } else if (m_ds.m_dotPos == DP_SCOPE) { // {a}.{b}, where {a} maybe a module name @@ -3096,8 +3111,42 @@ class LinkDotResolveVisitor final : public VNVisitor { void visit(AstMethodCall* nodep) override { // Created here so should already be resolved. VL_RESTORER(m_ds); + VL_RESTORER(m_pinSymp); { m_ds.init(m_curSymp); + if (nodep->name() == "randomize" && VN_IS(nodep->pinsp(), With)) { + const AstNodeDType* fromDtp = nodep->fromp()->dtypep(); + if (!fromDtp) { + if (const AstNodeVarRef* const varRefp = VN_CAST(nodep->fromp(), NodeVarRef)) { + fromDtp = varRefp->varp()->subDTypep(); + } else if (const AstNodeSel* const selp = VN_CAST(nodep->fromp(), NodeSel)) { + if (const AstNodeVarRef* const varRefp + = VN_CAST(selp->fromp(), NodeVarRef)) { + fromDtp = varRefp->varp()->dtypeSkipRefp()->subDTypep(); + } + } else if (const AstNodePreSel* const selp + = VN_CAST(nodep->fromp(), NodePreSel)) { + if (const AstNodeVarRef* const varRefp + = VN_CAST(selp->fromp(), NodeVarRef)) { + fromDtp = varRefp->varp()->dtypeSkipRefp()->subDTypep(); + } + } + if (!fromDtp) + nodep->v3warn(E_UNSUPPORTED, + "Unsupported: 'randomize() with' on complex expressions"); + } + if (m_statep->forPrimary() && isParamedClassRefDType(fromDtp)) { + m_ds.m_unresolvedClass = true; + } else if (fromDtp) { + const AstClassRefDType* const classDtp + = VN_CAST(fromDtp->skipRefp(), ClassRefDType); + if (!classDtp) + nodep->v3error("'randomize() with' on a non-class-instance " + << fromDtp->prettyNameQ()); + else + m_pinSymp = m_statep->getNodeSym(classDtp->classp()); + } + } iterateChildren(nodep); } } @@ -3113,7 +3162,7 @@ class LinkDotResolveVisitor final : public VNVisitor { if (nodep->user3SetOnce()) return; UINFO(8, " " << nodep << endl); UINFO(8, " " << m_ds.ascii() << endl); - { + if (m_ds.m_dotPos != DP_MEMBER || nodep->name() != "randomize") { // Visit arguments at the beginning. // They may be visitted even if the current node can't be linked now. VL_RESTORER(m_ds); @@ -3457,6 +3506,7 @@ class LinkDotResolveVisitor final : public VNVisitor { VL_RESTORER(m_curSymp); { m_ds.m_dotSymp = m_curSymp = m_statep->getNodeSym(nodep); + if (m_pinSymp) m_curSymp->importFromClass(m_statep->symsp(), m_pinSymp); iterateChildren(nodep); } m_ds.m_dotSymp = VL_RESTORER_PREV(m_curSymp); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 047683595..62d812b64 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3646,10 +3646,17 @@ class WidthVisitor final : public VNVisitor { void methodCallClass(AstMethodCall* nodep, AstClassRefDType* adtypep) { // No need to width-resolve the class, as it was done when we did the child AstClass* const first_classp = adtypep->classp(); + AstWith* withp = nullptr; if (nodep->name() == "randomize") { + withp = methodWithArgument(nodep, false, false, adtypep->findVoidDType(), + adtypep->findBitDType(), adtypep); + methodOkArguments(nodep, 0, 0); + methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); V3Randomize::newRandomizeFunc(first_classp); m_memberMap.clear(); } else if (nodep->name() == "srandom") { + methodOkArguments(nodep, 1, 1); + methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); V3Randomize::newSRandomFunc(first_classp); m_memberMap.clear(); } @@ -3695,6 +3702,10 @@ class WidthVisitor final : public VNVisitor { nodep->classOrPackagep(classp); if (VN_IS(ftaskp, Task)) nodep->dtypeSetVoid(); processFTaskRefArgs(nodep); + if (withp) { + withp->v3warn(CONSTRAINTIGN, "'with' constraint ignored (unsupported)"); + VL_DO_DANGLING(withp->deleteTree(), withp); + } } return; } else if (nodep->name() == "get_randstate" || nodep->name() == "set_randstate") { @@ -5993,14 +6004,23 @@ class WidthVisitor final : public VNVisitor { m_withp = nodep; userIterateChildren(nodep->indexArgRefp(), nullptr); userIterateChildren(nodep->valueArgRefp(), nullptr); - if (vdtypep) { - userIterateAndNext(nodep->exprp(), WidthVP{nodep->dtypep(), PRELIM}.p()); - } else { // 'sort with' allows arbitrary type - userIterateAndNext(nodep->exprp(), WidthVP{SELF, PRELIM}.p()); + if (!nodep->exprp()->hasDType()) { + userIterateAndNext(nodep->exprp(), nullptr); + } else { + if (vdtypep) { + userIterateAndNext(nodep->exprp(), WidthVP{nodep->dtypep(), PRELIM}.p()); + } else { // 'sort with' allows arbitrary type + userIterateAndNext(nodep->exprp(), WidthVP{SELF, PRELIM}.p()); + } + } + + if (!nodep->exprp()->hasDType()) { + nodep->dtypeSetVoid(); + } else { + nodep->dtypeFrom(nodep->exprp()); + iterateCheckAssign(nodep, "'with' return value", nodep->exprp(), FINAL, + nodep->dtypep()); } - nodep->dtypeFrom(nodep->exprp()); - iterateCheckAssign(nodep, "'with' return value", nodep->exprp(), FINAL, - nodep->dtypep()); } } void visit(AstLambdaArgRef* nodep) override { diff --git a/src/verilog.y b/src/verilog.y index e35565adb..b5e19970c 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -4111,8 +4111,7 @@ function_subroutine_callNoMethod: // IEEE: function_subroutine // // IEEE: randomize_call // // We implement randomize as a normal funcRef, since randomize isn't a keyword // // Note yNULL is already part of expressions, so they come for free - | funcRef yWITH__CUR constraint_block - { $$ = $1; BBUNSUP($2, "Unsupported: randomize() 'with' constraint"); } + | funcRef yWITH__CUR constraint_block { $$ = new AstWithParse{$2, $1, $3}; } | funcRef yWITH__CUR '{' '}' { $$ = new AstWithParse{$2, $1, nullptr}; } ; @@ -7198,8 +7197,7 @@ localNextId: // local // // Must call nextId without any additional tokens following yLOCAL__COLONCOLON { $$ = new AstClassOrPackageRef{$1, "local::", PARSEP->unitPackage($1), nullptr}; - SYMP->nextId(PARSEP->rootp()); - BBUNSUP($1, "Unsupported: Randomize 'local::'"); } + SYMP->nextId(PARSEP->rootp()); } ; //^^^========= diff --git a/test_regress/t/t_package_local_bad.out b/test_regress/t/t_package_local_bad.out new file mode 100644 index 000000000..d92fb60d7 --- /dev/null +++ b/test_regress/t/t_package_local_bad.out @@ -0,0 +1,7 @@ +%Error: t/t_package_local_bad.v:9:23: Illegal 'local::' outside 'randomize() with' + 9 | $display(local::x); + | ^ +%Error: t/t_package_local_bad.v:9:23: Can't find definition of scope/variable/func: 'x' + 9 | $display(local::x); + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_package_local_bad.pl b/test_regress/t/t_package_local_bad.pl new file mode 100755 index 000000000..27159da5b --- /dev/null +++ b/test_regress/t/t_package_local_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 2019 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_package_local_bad.v b/test_regress/t/t_package_local_bad.v new file mode 100644 index 000000000..a004a8cd0 --- /dev/null +++ b/test_regress/t/t_package_local_bad.v @@ -0,0 +1,12 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2024 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +module t(/*AUTOARG*/); + initial begin + $display(local::x); + $finish; + end +endmodule diff --git a/test_regress/t/t_randomize.out b/test_regress/t/t_randomize.out index 69f59261b..b7755620e 100644 --- a/test_regress/t/t_randomize.out +++ b/test_regress/t/t_randomize.out @@ -1,9 +1,12 @@ +%Warning-CONSTRAINTIGN: t/t_randomize.v:66:25: 'with' constraint ignored (unsupported) + 66 | v = p.randomize() with { if_4 == local::if_4; header == 2; }; + | ^~~~ + ... For warning description see https://verilator.org/warn/CONSTRAINTIGN?v=latest + ... Use "/* verilator lint_off CONSTRAINTIGN */" and lint_on around source to disable this message. %Warning-CONSTRAINTIGN: t/t_randomize.v:38:16: State-dependent constraint ignored (unsupported) : ... note: In instance 't' 38 | array[i] inside {2, 4, 6}; | ^ - ... For warning description see https://verilator.org/warn/CONSTRAINTIGN?v=latest - ... Use "/* verilator lint_off CONSTRAINTIGN */" and lint_on around source to disable this message. %Warning-CONSTRAINTIGN: t/t_randomize.v:26:7: Constraint expression ignored (unsupported) : ... note: In instance 't' 26 | if (header > 4) { diff --git a/test_regress/t/t_randomize.v b/test_regress/t/t_randomize.v index cfd56389a..d9633f9be 100644 --- a/test_regress/t/t_randomize.v +++ b/test_regress/t/t_randomize.v @@ -57,11 +57,14 @@ module t (/*AUTOARG*/); initial begin int v; + bit if_4 = '0; // TODO not testing constrained values v = p.randomize(); if (v != 1) $stop; v = p.randomize() with {}; if (v != 1) $stop; + v = p.randomize() with { if_4 == local::if_4; header == 2; }; + if (v != 1) $stop; // TODO not testing other randomize forms as unused in UVM $write("*-* All Finished *-*\n"); diff --git a/test_regress/t/t_randomize_method_complex_bad.out b/test_regress/t/t_randomize_method_complex_bad.out new file mode 100644 index 000000000..3275c55af --- /dev/null +++ b/test_regress/t/t_randomize_method_complex_bad.out @@ -0,0 +1,14 @@ +%Error-UNSUPPORTED: t/t_randomize_method_complex_bad.v:16:11: Unsupported: 'randomize() with' on complex expressions + 16 | x.f.randomize() with { r < 5; }, + | ^~~~~~~~~ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_randomize_method_complex_bad.v:16:30: Can't find definition of variable: 'r' + 16 | x.f.randomize() with { r < 5; }, + | ^ +%Error: t/t_randomize_method_complex_bad.v:17:9: 'randomize() with' on a non-class-instance 'int' + 17 | i.randomize() with { v < 5; }); + | ^~~~~~~~~ +%Error: t/t_randomize_method_complex_bad.v:17:28: Can't find definition of variable: 'v' + 17 | i.randomize() with { v < 5; }); + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_randomize_method_complex_bad.pl b/test_regress/t/t_randomize_method_complex_bad.pl new file mode 100755 index 000000000..66fa61649 --- /dev/null +++ b/test_regress/t/t_randomize_method_complex_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 2020 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}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_randomize_method_complex_bad.v b/test_regress/t/t_randomize_method_complex_bad.v new file mode 100644 index 000000000..92d21189f --- /dev/null +++ b/test_regress/t/t_randomize_method_complex_bad.v @@ -0,0 +1,18 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Cls; + Cls f; + rand int r; +endclass +module t (/*AUTOARG*/); + Cls x = new; + int i; + initial $display( + x.f.randomize(), + x.f.randomize() with { r < 5; }, + i.randomize() with { v < 5; }); +endmodule diff --git a/test_regress/t/t_randomize_method_with_unsup.out b/test_regress/t/t_randomize_method_with_unsup.out index d979ce6ea..d367d0db1 100644 --- a/test_regress/t/t_randomize_method_with_unsup.out +++ b/test_regress/t/t_randomize_method_with_unsup.out @@ -1,11 +1,12 @@ -%Error-UNSUPPORTED: t/t_randomize_method_with_unsup.v:47:40: Unsupported: randomize() 'with' constraint +%Warning-CONSTRAINTIGN: t/t_randomize_method_with_unsup.v:47:40: 'with' constraint ignored (unsupported) 47 | rand_result = obj.randomize() with { lb <= y && y <= ub; }; | ^~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_randomize_method_with_unsup.v:63:37: Unsupported: randomize() 'with' constraint + ... For warning description see https://verilator.org/warn/CONSTRAINTIGN?v=latest + ... Use "/* verilator lint_off CONSTRAINTIGN */" and lint_on around source to disable this message. +%Warning-CONSTRAINTIGN: t/t_randomize_method_with_unsup.v:63:37: 'with' constraint ignored (unsupported) 63 | rand_result = obj.randomize() with { 256 < y && y < 256; }; | ^~~~ -%Error-UNSUPPORTED: t/t_randomize_method_with_unsup.v:67:37: Unsupported: randomize() 'with' constraint +%Warning-CONSTRAINTIGN: t/t_randomize_method_with_unsup.v:67:37: 'with' constraint ignored (unsupported) 67 | rand_result = obj.randomize() with { 16 <= z && z <= 32; }; | ^~~~ %Error: Exiting due to