From d359fffcdc0c045d275cabcfc4027da0e76a01f9 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 27 Jul 2025 10:30:19 -0400 Subject: [PATCH] Internals: Refactor V3Task and add loop assert (#6218) --- src/V3Task.cpp | 34 +++++++++--------- test_regress/t/t_func_while2.py | 18 ++++++++++ test_regress/t/t_func_while2.v | 36 +++++++++++++++++++ ...uble_param.py => t_unroll_nested_param.py} | 0 ...double_param.v => t_unroll_nested_param.v} | 0 5 files changed, 72 insertions(+), 16 deletions(-) create mode 100755 test_regress/t/t_func_while2.py create mode 100644 test_regress/t/t_func_while2.v rename test_regress/t/{t_unroll_double_param.py => t_unroll_nested_param.py} (100%) rename test_regress/t/{t_unroll_double_param.v => t_unroll_nested_param.v} (100%) diff --git a/src/V3Task.cpp b/src/V3Task.cpp index c66dcb463..314ea0986 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -1435,9 +1435,9 @@ class TaskVisitor final : public VNVisitor { VL_RESTORER(m_modp); VL_RESTORER(m_modNCalls); m_modp = nodep; - m_insStmtp = nullptr; m_modNCalls = 0; iterateChildren(nodep); + UASSERT_OBJ(!m_insStmtp, nodep, "Didn't finish out last statement"); } void visit(AstWith* nodep) override { if (nodep->user1SetOnce()) { @@ -1451,8 +1451,8 @@ class TaskVisitor final : public VNVisitor { void visit(AstScope* nodep) override { VL_RESTORER(m_scopep); m_scopep = nodep; - m_insStmtp = nullptr; iterateChildren(nodep); + UASSERT_OBJ(!m_insStmtp, nodep, "Didn't finish out last statement"); } void visit(AstNodeFTaskRef* nodep) override { if (m_inSensesp) { @@ -1486,7 +1486,7 @@ class TaskVisitor final : public VNVisitor { ++m_statInlines; } - if (VN_IS(nodep, New)) { + if (VN_IS(nodep, New)) { // New not legal as while() condition insertBeforeStmt(nodep, beginp); UASSERT_OBJ(cnewp, nodep, "didn't create cnew for new"); nodep->replaceWith(cnewp); @@ -1508,7 +1508,7 @@ class TaskVisitor final : public VNVisitor { nodep->replaceWith(beginp); VL_DO_DANGLING(nodep->deleteTree(), nodep); VIsCached::clearCacheTree(); - } else { + } else { // VN_IS(nodep->backp(), StmtExpr) insertBeforeStmt(nodep, beginp); if (nodep->taskp()->isFunction()) { nodep->v3warn( @@ -1588,16 +1588,18 @@ class TaskVisitor final : public VNVisitor { } void visit(AstWhile* nodep) override { // Special, as statements need to be put in different places - // Conditions insert first at end of precondsp. - // TODO: is this right? This is how it used to be. - m_insStmtp = nodep; - iterateAndNextNull(nodep->condp()); - // Body insert just before themselves - m_insStmtp = nullptr; // First thing should be new statement - iterateAndNextNull(nodep->stmtsp()); - iterateAndNextNull(nodep->incsp()); - // Done the loop - m_insStmtp = nullptr; // Next thing should be new statement + { + // Conditions will create a StmtExpr + // Leave m_instStmtp = null, so will assert if not + iterateAndNextNull(nodep->condp()); + } + { + // Body insert just before themselves + VL_RESTORER(m_insStmtp); + m_insStmtp = nullptr; // First thing should be new statement + iterateAndNextNull(nodep->stmtsp()); + iterateAndNextNull(nodep->incsp()); + } } void visit(AstNodeForeach* nodep) override { // LCOV_EXCL_LINE nodep->v3fatalSrc( @@ -1608,15 +1610,15 @@ class TaskVisitor final : public VNVisitor { "For statements should have been converted to while statements in V3Begin.cpp"); } void visit(AstNodeStmt* nodep) override { + VL_RESTORER(m_insStmtp); m_insStmtp = nodep; iterateChildren(nodep); - m_insStmtp = nullptr; // Next thing should be new statement } void visit(AstStmtExpr* nodep) override { + VL_RESTORER(m_insStmtp); m_insStmtp = nodep; iterateChildren(nodep); if (!nodep->exprp()) VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); - m_insStmtp = nullptr; // Next thing should be new statement } void visit(AstSenItem* nodep) override { UASSERT_OBJ(!m_inSensesp, nodep, "Senitem under senitem?"); diff --git a/test_regress/t/t_func_while2.py b/test_regress/t/t_func_while2.py new file mode 100755 index 000000000..563b6fc6f --- /dev/null +++ b/test_regress/t/t_func_while2.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_st') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_func_while2.v b/test_regress/t/t_func_while2.v new file mode 100644 index 000000000..8cbb204a5 --- /dev/null +++ b/test_regress/t/t_func_while2.v @@ -0,0 +1,36 @@ +// 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 + +// verilog_format: off +`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); +// verilog_format: on + +module t; + + int i; + string value; + + function automatic int count(); + ++i; + value = {value, $sformatf(" count%0d", i)}; + return i; + endfunction + + initial begin + value = ""; + i = 0; + while (count() <= 2) begin + // verilator unroll_disable + value = {value, " loop"}; + end + `checks(value, " count1 loop count2 loop count3"); + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_unroll_double_param.py b/test_regress/t/t_unroll_nested_param.py similarity index 100% rename from test_regress/t/t_unroll_double_param.py rename to test_regress/t/t_unroll_nested_param.py diff --git a/test_regress/t/t_unroll_double_param.v b/test_regress/t/t_unroll_nested_param.v similarity index 100% rename from test_regress/t/t_unroll_double_param.v rename to test_regress/t/t_unroll_nested_param.v