From cb5d03ff0bae3b1f7120547667626b94a8c26a63 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 24 Jul 2024 06:38:56 -0400 Subject: [PATCH] Fix WIDTHEXPAND on left shift of intuitive amount (#5284). --- Changes | 1 + src/V3Width.cpp | 11 +++++++- test_regress/t/t_lint_width_shift_bad.out | 23 +++++++++++++++++ test_regress/t/t_lint_width_shift_bad.pl | 19 ++++++++++++++ test_regress/t/t_lint_width_shift_bad.v | 31 +++++++++++++++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 test_regress/t/t_lint_width_shift_bad.out create mode 100755 test_regress/t/t_lint_width_shift_bad.pl create mode 100644 test_regress/t/t_lint_width_shift_bad.v diff --git a/Changes b/Changes index 910cba754..8bb3acd35 100644 --- a/Changes +++ b/Changes @@ -41,6 +41,7 @@ Verilator 5.027 devel * Fix tracing with `--main-top-name -` (#5261). [Ethan Sifferman] * Fix randomization when used with inheritance (#5268). [Krzysztof Bieganski, Antmicro Ltd.] * Fix inline constraints creating class random generator (#5280). [Krzysztof Bieganski, Antmicro Ltd.] +* Fix WIDTHEXPAND on left shift of intuitive amount (#5284). [Greg Taylor] * Fix elaborating foreach loops (#5285). [Arkadiusz Kozdra, Antmicro Ltd.] * Fix initializing static array in dynamic arrays and queues (#5287). [Baruch Sterin] * Fix randomizing current object with `rand` class instance member (#5292). [Krzysztof Bieganski, Antmicro Ltd.] diff --git a/src/V3Width.cpp b/src/V3Width.cpp index bb9bb83eb..9e22969ab 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -6545,10 +6545,19 @@ class WidthVisitor final : public VNVisitor { bool warnOn = true; // No warning if "X = 1'b1<lhsp()->isOne() && VN_IS(nodep->backp(), NodeAssign)) warnOn = false; + // We don't currently suppress these, as it's the upper operator (e.g. assign) + // that reports the WIDTHEXPAND. + AstConst* const shiftp = VN_CAST(nodep->rhsp(), Const); + if (shiftp && !shiftp->num().isFourState() && shiftp->width() <= 32) { + const int64_t shiftVal = shiftp->num().toSQuad(); + if (VN_IS(nodep, ShiftL)) { + if (shiftVal > 0 && nodep->width() == nodep->lhsp()->width() + shiftVal) + warnOn = false; + } + } iterateCheck(nodep, "LHS", nodep->lhsp(), CONTEXT_DET, FINAL, subDTypep, EXTEND_EXP, warnOn); if (nodep->rhsp()->width() > 32) { - AstConst* const shiftp = VN_CAST(nodep->rhsp(), Const); if (shiftp && shiftp->num().mostSetBitP1() <= 32) { // If (number)<<96'h1, then make it into (number)<<32'h1 V3Number num(shiftp, 32, 0); diff --git a/test_regress/t/t_lint_width_shift_bad.out b/test_regress/t/t_lint_width_shift_bad.out new file mode 100644 index 000000000..bc5643001 --- /dev/null +++ b/test_regress/t/t_lint_width_shift_bad.out @@ -0,0 +1,23 @@ +%Warning-WIDTHTRUNC: t/t_lint_width_shift_bad.v:19:15: Operator ASSIGNW expects 3 bits on the Assign RHS, but Assign RHS's SHIFTL generates 4 bits. + : ... note: In instance 't' + 19 | assign ol3 = i4 << 1; + | ^ + ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest + ... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message. +%Warning-WIDTHTRUNC: t/t_lint_width_shift_bad.v:23:15: Operator ASSIGNW expects 3 bits on the Assign RHS, but Assign RHS's SHIFTR generates 4 bits. + : ... note: In instance 't' + 23 | assign or3 = i4 >> 1; + | ^ +%Warning-WIDTHEXPAND: t/t_lint_width_shift_bad.v:25:20: Operator SHIFTR expects 5 bits on the LHS, but LHS's VARREF 'i4' generates 4 bits. + : ... note: In instance 't' + 25 | assign or5 = i4 >> 1; + | ^~ +%Warning-WIDTHTRUNC: t/t_lint_width_shift_bad.v:27:15: Operator ASSIGNW expects 3 bits on the Assign RHS, but Assign RHS's SHIFTRS generates 4 bits. + : ... note: In instance 't' + 27 | assign os3 = i4 >>> 1; + | ^ +%Warning-WIDTHEXPAND: t/t_lint_width_shift_bad.v:29:20: Operator SHIFTRS expects 5 bits on the LHS, but LHS's VARREF 'i4' generates 4 bits. + : ... note: In instance 't' + 29 | assign os5 = i4 >>> 1; + | ^~~ +%Error: Exiting due to diff --git a/test_regress/t/t_lint_width_shift_bad.pl b/test_regress/t/t_lint_width_shift_bad.pl new file mode 100755 index 000000000..07964a1b5 --- /dev/null +++ b/test_regress/t/t_lint_width_shift_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 2003-2009 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(vlt => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_width_shift_bad.v b/test_regress/t/t_lint_width_shift_bad.v new file mode 100644 index 000000000..7cb1bfbfb --- /dev/null +++ b/test_regress/t/t_lint_width_shift_bad.v @@ -0,0 +1,31 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2009 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t + (input signed [3:0] i4, + output signed [2:0] ol3, + output signed [3:0] ol4, + output signed [4:0] ol5, + output signed [2:0] or3, + output signed [3:0] or4, + output signed [4:0] or5, + output signed [2:0] os3, + output signed [3:0] os4, + output signed [4:0] os5); + + assign ol3 = i4 << 1; // WIDTHTRUNC + assign ol4 = i4 << 1; + assign ol5 = i4 << 1; // WIDTHEXPAND, but ok due to shift amount 1 + + assign or3 = i4 >> 1; // WIDTHTRUNC, currently warn, but in future ok due to shift amount 1? + assign or4 = i4 >> 1; + assign or5 = i4 >> 1; // WIDTHEXPAND + + assign os3 = i4 >>> 1; // WIDTHTRUNC, currently warn, but in future ok due to shift amount 1? + assign os4 = i4 >>> 1; + assign os5 = i4 >>> 1; // WIDTHEXPAND + +endmodule