From 4bdfc653a3d8895597715ef154d98a661c535afa Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 3 Oct 2024 18:54:30 -0400 Subject: [PATCH] Add error on misused genvar (#408). --- Changes | 1 + src/V3LinkResolve.cpp | 9 ++ test_regress/t/t_genvar_for_bad.out | 6 +- test_regress/t/t_genvar_for_bad.v | 9 +- test_regress/t/t_genvar_misuse_bad.out | 7 ++ test_regress/t/t_genvar_misuse_bad.py | 5 +- test_regress/t/t_json_only_debugcheck.out | 124 +++++++++++----------- 7 files changed, 89 insertions(+), 72 deletions(-) create mode 100644 test_regress/t/t_genvar_misuse_bad.out diff --git a/Changes b/Changes index 5ea791963..b3e10fc57 100644 --- a/Changes +++ b/Changes @@ -30,6 +30,7 @@ Verilator 5.029 devel * Support inside array constraints (#5448). [Arkadiusz Kozdra, Antmicro Ltd.] * Support DPI imports and exports with double underscores (#5481). * Support ccache when compiling Verilated files with cmake. +* Add error on misused genvar (#408). [Alex Solomatnikov] * Add error on instances without parenthesis. * Add Docker pre-commit hook (#5238) (#5452). [Chris Bachhuber] * Add partial coverage symbol and branch data in lcov info files (#5388). [Andrew Nolte] diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp index 4ed741904..2dd3fe9c9 100644 --- a/src/V3LinkResolve.cpp +++ b/src/V3LinkResolve.cpp @@ -50,6 +50,7 @@ class LinkResolveVisitor final : public VNVisitor { AstNodeFTask* m_ftaskp = nullptr; // Function or task we're inside AstNodeCoverOrAssert* m_assertp = nullptr; // Current assertion int m_senitemCvtNum = 0; // Temporary signal counter + bool m_underGenFor = false; // Under GenFor bool m_underGenerate = false; // Under GenFor/GenIf // VISITs @@ -105,6 +106,12 @@ class LinkResolveVisitor final : public VNVisitor { void visit(AstNodeVarRef* nodep) override { // VarRef: Resolve its reference if (nodep->varp()) nodep->varp()->usedParam(true); + // TODO should look for where genvar is valid, but for now catch + // just gross errors of using genvar outside any generate + if (nodep->varp() && nodep->varp()->isGenVar() && !m_underGenFor) { + nodep->v3error("Genvar " << nodep->prettyNameQ() + << " used outside generate for loop (IEEE 1800-2024 27.4)"); + } iterateChildren(nodep); } @@ -450,7 +457,9 @@ class LinkResolveVisitor final : public VNVisitor { // We keep Modport's themselves around for XML dump purposes void visit(AstGenFor* nodep) override { + VL_RESTORER(m_underGenFor); VL_RESTORER(m_underGenerate); + m_underGenFor = true; m_underGenerate = true; iterateChildren(nodep); } diff --git a/test_regress/t/t_genvar_for_bad.out b/test_regress/t/t_genvar_for_bad.out index 5088dc104..0ecb3be58 100644 --- a/test_regress/t/t_genvar_for_bad.out +++ b/test_regress/t/t_genvar_for_bad.out @@ -1,5 +1,5 @@ -%Error: t/t_genvar_for_bad.v:23:10: Genvar not legal in non-generate for (IEEE 1800-2023 27.4): 't.i' +%Error: t/t_genvar_for_bad.v:25:13: Genvar not legal in non-generate for (IEEE 1800-2023 27.4): 't.i' : ... Suggest move for loop upwards to generate-level scope. - 23 | for (i=0; i