From 9249ffdb84d8e31df335d94034ba9ec96463a689 Mon Sep 17 00:00:00 2001 From: Aleksander Kiryk Date: Mon, 3 Jul 2023 19:55:24 +0200 Subject: [PATCH] Fix queue slicing (#4329) --- src/V3Const.cpp | 46 ++++++++++++++++++++------ src/V3Const.h | 4 +++ src/V3WidthSel.cpp | 4 +-- test_regress/t/t_queue_var_slice.pl | 21 ++++++++++++ test_regress/t/t_queue_var_slice.v | 25 ++++++++++++++ test_regress/t/t_select_bad_range4.out | 8 ----- 6 files changed, 88 insertions(+), 20 deletions(-) create mode 100755 test_regress/t/t_queue_var_slice.pl create mode 100644 test_regress/t/t_queue_var_slice.v diff --git a/src/V3Const.cpp b/src/V3Const.cpp index f74282ffb..c8448ff6c 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3655,6 +3655,7 @@ private: public: // Processing Mode Enum enum ProcMode : uint8_t { + PROC_PARAMS_NOWARN, PROC_PARAMS, PROC_GENERATE, PROC_LIVE, @@ -3670,16 +3671,18 @@ public: , m_concswapNames{globalPass ? ("__Vconcswap_" + cvtToStr(s_globalPassNum++)) : ""} { // clang-format off switch (pmode) { - case PROC_PARAMS: m_doV = true; m_doNConst = true; m_params = true; - m_required = true; break; - case PROC_GENERATE: m_doV = true; m_doNConst = true; m_params = true; - m_required = true; m_doGenerate = true; break; - case PROC_LIVE: break; - case PROC_V_WARN: m_doV = true; m_doNConst = true; m_warn = true; break; - case PROC_V_NOWARN: m_doV = true; m_doNConst = true; break; - case PROC_V_EXPENSIVE: m_doV = true; m_doNConst = true; m_doExpensive = true; break; - case PROC_CPP: m_doV = false; m_doNConst = true; m_doCpp = true; break; - default: v3fatalSrc("Bad case"); break; + case PROC_PARAMS_NOWARN: m_doV = true; m_doNConst = true; m_params = true; + m_required = false; break; + case PROC_PARAMS: m_doV = true; m_doNConst = true; m_params = true; + m_required = true; break; + case PROC_GENERATE: m_doV = true; m_doNConst = true; m_params = true; + m_required = true; m_doGenerate = true; break; + case PROC_LIVE: break; + case PROC_V_WARN: m_doV = true; m_doNConst = true; m_warn = true; break; + case PROC_V_NOWARN: m_doV = true; m_doNConst = true; break; + case PROC_V_EXPENSIVE: m_doV = true; m_doNConst = true; m_doExpensive = true; break; + case PROC_CPP: m_doV = false; m_doNConst = true; m_doCpp = true; break; + default: v3fatalSrc("Bad case"); break; } // clang-format on } @@ -3727,6 +3730,29 @@ AstNode* V3Const::constifyParamsEdit(AstNode* nodep) { return nodep; } +//! Constify this cell node's parameter list if possible +//! @return Pointer to the edited node. +AstNode* V3Const::constifyParamsNoWarnEdit(AstNode* nodep) { + // if (debug() > 0) nodep->dumpTree("- forceConPRE : "); + // Resize even if the node already has a width, because buried in the tree + // we may have a node we just created with signing, etc, that isn't sized yet. + + // Make sure we've sized everything first + nodep = V3Width::widthParamsEdit(nodep); + ConstVisitor visitor{ConstVisitor::PROC_PARAMS_NOWARN, /* globalPass: */ false}; + if (AstVar* const varp = VN_CAST(nodep, Var)) { + // If a var wants to be constified, it's really a param, and + // we want the value to be constant. We aren't passed just the + // init value because we need widthing above to handle the var's type. + if (varp->valuep()) visitor.mainAcceptEdit(varp->valuep()); + } else { + nodep = visitor.mainAcceptEdit(nodep); + } + // Because we do edits, nodep links may get trashed and core dump this. + // if (debug() > 0) nodep->dumpTree("- forceConDONE: "); + return nodep; +} + //! Force this cell node's parameter list to become a constant inside generate. //! If we are inside a generated "if", "case" or "for", we don't want to //! trigger warnings when we deal with the width. It is possible that these diff --git a/src/V3Const.h b/src/V3Const.h index c171a5afe..a0c0cf797 100644 --- a/src/V3Const.h +++ b/src/V3Const.h @@ -30,6 +30,10 @@ public: static AstNodeExpr* constifyParamsEdit(AstNodeExpr* exprp) { return VN_AS(constifyParamsEdit(static_cast(exprp)), NodeExpr); } + static AstNode* constifyParamsNoWarnEdit(AstNode* nodep); + static AstNodeExpr* constifyParamsNoWarnEdit(AstNodeExpr* exprp) { + return VN_AS(constifyParamsNoWarnEdit(static_cast(exprp)), NodeExpr); + } static AstNode* constifyGenerateParamsEdit(AstNode* nodep); // Only do constant pushing, without removing dead logic static void constifyAllLive(AstNetlist* nodep); diff --git a/src/V3WidthSel.cpp b/src/V3WidthSel.cpp index c6308416b..f12893f40 100644 --- a/src/V3WidthSel.cpp +++ b/src/V3WidthSel.cpp @@ -348,8 +348,8 @@ private: UINFO(6, "SELEXTRACT " << nodep << endl); // if (debug() >= 9) nodep->dumpTree("- SELEX0: "); // Below 2 lines may change nodep->widthp() - V3Const::constifyParamsEdit(nodep->leftp()); // May relink pointed to node - V3Const::constifyParamsEdit(nodep->rightp()); // May relink pointed to node + V3Const::constifyParamsNoWarnEdit(nodep->leftp()); // May relink pointed to node + V3Const::constifyParamsNoWarnEdit(nodep->rightp()); // May relink pointed to node // if (debug() >= 9) nodep->dumpTree("- SELEX3: "); AstNodeExpr* const fromp = nodep->fromp()->unlinkFrBack(); const FromData fromdata = fromDataForArray(nodep, fromp); diff --git a/test_regress/t/t_queue_var_slice.pl b/test_regress/t/t_queue_var_slice.pl new file mode 100755 index 000000000..859050d63 --- /dev/null +++ b/test_regress/t/t_queue_var_slice.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2023 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(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_queue_var_slice.v b/test_regress/t/t_queue_var_slice.v new file mode 100644 index 000000000..da4e24877 --- /dev/null +++ b/test_regress/t/t_queue_var_slice.v @@ -0,0 +1,25 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer i = 0; + integer q[$] = {0, 1}; + + always @(posedge clk) begin + $display("%p", q[i:i+1]); + q.push_back(i+2); + i++; + if (i >= 3) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_select_bad_range4.out b/test_regress/t/t_select_bad_range4.out index cff72fd9a..6646fc383 100644 --- a/test_regress/t/t_select_bad_range4.out +++ b/test_regress/t/t_select_bad_range4.out @@ -52,14 +52,6 @@ : ... In instance t 23 | sel2 = mi[nonconst]; | ^ -%Error: t/t_select_bad_range4.v:24:17: Expecting expression to be constant, but variable isn't const: 'nonconst' - : ... In instance t - 24 | sel2 = mi[nonconst : nonconst]; - | ^~~~~~~~ -%Error: t/t_select_bad_range4.v:24:28: Expecting expression to be constant, but variable isn't const: 'nonconst' - : ... In instance t - 24 | sel2 = mi[nonconst : nonconst]; - | ^~~~~~~~ %Error: t/t_select_bad_range4.v:24:17: First value of [a:b] isn't a constant, maybe you want +: or -: : ... In instance t 24 | sel2 = mi[nonconst : nonconst];