From 7e5a7b65a00a6f160d8c154b674706b1aa18f7b8 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 2 Apr 2008 12:53:53 +0000 Subject: [PATCH] Unsized concatenates now give WIDTHCONCAT warnings. git-svn-id: file://localhost/svn/verilator/trunk/verilator@1020 77ca24e4-aefa-0310-84f0-b9a241c72d87 --- Changes | 4 ++++ bin/verilator | 18 ++++++++++++++++++ src/V3Error.h | 3 ++- src/V3Number.cpp | 9 ++++++--- src/V3Width.cpp | 10 ++++++++-- test_regress/t/t_param_concat.pl | 19 +++++++++++++++++++ test_regress/t/t_param_concat.v | 28 ++++++++++++++++++++++++++++ test_regress/t/t_param_concat_bad.pl | 24 ++++++++++++++++++++++++ 8 files changed, 109 insertions(+), 6 deletions(-) create mode 100755 test_regress/t/t_param_concat.pl create mode 100644 test_regress/t/t_param_concat.v create mode 100755 test_regress/t/t_param_concat_bad.pl diff --git a/Changes b/Changes index 43dde1928..445ef1020 100644 --- a/Changes +++ b/Changes @@ -12,6 +12,10 @@ indicates the contributor was also the author of the fix; Thanks! *** Add --top-module option to select between multiple tops. [Stefan Thiede] +*** Unsized concatenates now give WIDTHCONCAT warnings. [Jonathan Kimmitt] + Previously they threw fatal errors, which in most cases is correct + according to spec, but can be incorrect in presence of parameter values. + **** Ignore delays attached to gate UDPs. [Stefan Thiede] **** Fix SystemVerilog parameterized defines with `` expansion, diff --git a/bin/verilator b/bin/verilator index 93ef494f5..4a1bdfbc7 100755 --- a/bin/verilator +++ b/bin/verilator @@ -1893,6 +1893,24 @@ The best fix, which clarifies intent and will also make all tools happy is: Ignoring this warning will only suppress the lint check, it will simulate correctly. +=item WIDTHCONCAT + +Warns that based on width rules of Verilog, a concatenate or replication +has a undeterminate width. In most cases this violates the Verilog rule +that widths inside concatenates and replicates must be sized, and should be +fixed in the code. + + wire [63:0] concat = {1,2}; + +An example where this is technically legal (though still bad form) is: + + parameter PAR = 1; + wire [63:0] concat = {PAR,PAR}; + +The correct fix is to either size the 1 ("32'h1"), or add the width to the +parameter definition ("parameter [31:0]"), or add the width to the +parameter usage ("{PAR[31:0],PAR[31:0]}". + =back The following describes the less obvious errors: diff --git a/src/V3Error.h b/src/V3Error.h index c40f0aca5..f7bc9df81 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -61,6 +61,7 @@ public: UNUSED, // No receivers VARHIDDEN, // Hiding variable WIDTH, // Width mismatch + WIDTHCONCAT, // Unsized numbers/parameters in concatenations MAX // ***Add new elements below also*** }; @@ -81,7 +82,7 @@ public: "COMBDLY", "STMTDLY", "GENCLK", "IMPLICIT", "IMPURE", "MULTIDRIVEN", "REDEFMACRO", "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED", - "VARHIDDEN", "WIDTH", + "VARHIDDEN", "WIDTH", "WIDTHCONCAT", " MAX" }; return names[m_e]; diff --git a/src/V3Number.cpp b/src/V3Number.cpp index 656b28ae8..466abb1c6 100644 --- a/src/V3Number.cpp +++ b/src/V3Number.cpp @@ -698,8 +698,10 @@ V3Number& V3Number::opXnor (const V3Number& lhs, const V3Number& rhs) { V3Number& V3Number::opConcat (const V3Number& lhs, const V3Number& rhs) { setZero(); - if (!lhs.sized()) m_fileline->v3error("Unsized constants not allowed in concatenations: "<v3error("Unsized constants not allowed in concatenations: "<v3warn(WIDTHCONCAT,"Unsized numbers/parameters not allowed in concatenations."); + } int obit = 0; for(int bit=0; bitv3error("Unsized constants not allowed in concatenations: "<v3warn(WIDTHCONCAT,"Unsized numbers/parameters not allowed in replications."); return opRepl(lhs, rhs.asInt()); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 9cababb29..6e80879ae 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -201,7 +201,10 @@ private: nodep->lhsp()->widthMin() + nodep->rhsp()->widthMin()); } if (vup->c()->final()) { - if (!nodep->widthSized()) nodep->v3error("Concat argument must have specific size. (No unsized numbers/parameters)."); + if (!nodep->widthSized()) { + // See also error in V3Number + nodep->v3warn(WIDTHCONCAT,"Unsized numbers/parameters not allowed in concatenations."); + } } } virtual void visit(AstReplicate* nodep, AstNUser* vup) { @@ -217,7 +220,10 @@ private: (nodep->lhsp()->widthMin() * times)); } if (vup->c()->final()) { - if (!nodep->widthSized()) nodep->v3error("Replication expression must have specific size."); + if (!nodep->widthSized()) { + // See also error in V3Number + nodep->v3warn(WIDTHCONCAT,"Unsized numbers/parameters not allowed in replications."); + } } } virtual void visit(AstRange* nodep, AstNUser* vup) { diff --git a/test_regress/t/t_param_concat.pl b/test_regress/t/t_param_concat.pl new file mode 100755 index 000000000..d272045ca --- /dev/null +++ b/test_regress/t/t_param_concat.pl @@ -0,0 +1,19 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; } +# $Id$ +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# General Public License or the Perl Artistic License. + +compile ( + verilator_flags2 => ["--Wno-WIDTHCONCAT"], + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_param_concat.v b/test_regress/t/t_param_concat.v new file mode 100644 index 000000000..8f70dbf8d --- /dev/null +++ b/test_regress/t/t_param_concat.v @@ -0,0 +1,28 @@ +// $Id$ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2005 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + parameter UNSIZED = 10; + + integer cyc=1; + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc==1) begin + if ({UNSIZED,UNSIZED+1} != {32'd10, 32'd11}) $stop; + if ({2{UNSIZED}} != {32'd10, 32'd10}) $stop; + end + if (cyc==9) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule diff --git a/test_regress/t/t_param_concat_bad.pl b/test_regress/t/t_param_concat_bad.pl new file mode 100755 index 000000000..9ec4860fa --- /dev/null +++ b/test_regress/t/t_param_concat_bad.pl @@ -0,0 +1,24 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; } +# $Id$ +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# General Public License or the Perl Artistic License. + +top_filename("t/t_param_concat.v"); + +compile ( + fails=>1, + expect=> +'%Warning-WIDTHCONCAT: t/t_param_concat.v:\d+: Unsized numbers/parameters not allowed in concatenations. +%Warning-WIDTHCONCAT: Use "/\* verilator lint_off WIDTHCONCAT \*/" and lint_on around source to disable this message. +%Warning-WIDTHCONCAT: t/t_param_concat.v:\d+: Unsized numbers/parameters not allowed in replications. +%Warning-WIDTHCONCAT: t/t_param_concat.v:\d+: Unsized numbers/parameters not allowed in concatenations. +%Warning-WIDTHCONCAT: t/t_param_concat.v:\d+: Unsized numbers/parameters not allowed in replications. +%Error: Exiting due to.*', + ); + +ok(1); +1;