From 04ca6a43075c53b3c4d479a018fb313ed5071e36 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 1 Oct 2017 10:21:27 -0400 Subject: [PATCH] Fix compiler warning when WIDTH warning ignored on large compare. --- Changes | 2 + src/V3Const.cpp | 37 ++++++-- test_regress/t/t_var_overcmp.pl | 18 ++++ test_regress/t/t_var_overcmp.v | 145 ++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 9 deletions(-) create mode 100755 test_regress/t/t_var_overcmp.pl create mode 100644 test_regress/t/t_var_overcmp.v diff --git a/Changes b/Changes index c290e1477..7417a5c01 100644 --- a/Changes +++ b/Changes @@ -13,6 +13,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix Ubuntu 17.10 issues, bug1223 partial. [John Coiner] +**** Fix compiler warning when WIDTH warning ignored on large compare. + * Verilator 3.912 2017-09-23 diff --git a/src/V3Const.cpp b/src/V3Const.cpp index a59573b9e..372232024 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -369,11 +369,11 @@ private: return true; } - bool operandBiExtendConst(AstNodeBiop* nodep) { + bool operandBiExtendConstShrink(AstNodeBiop* nodep) { // Loop unrolling favors standalone compares // EQ(const{width32}, EXTEND(xx{width3})) -> EQ(const{3}, xx{3}) - // Beware that the constant must have zero bits (+ 1 if signed) or compare - // would be incorrect + // The constant must have zero bits (+ 1 if signed) or compare + // would be incorrect. See also operandBiExtendConst AstExtend* extendp = nodep->rhsp()->castExtend(); if (!extendp) return false; AstNode* smallerp = extendp->lhsp(); @@ -395,6 +395,19 @@ private: if (debug()>=9) nodep->dumpTree(cout,"BI(EXTEND)-ou:"); return true; } + bool operandBiExtendConstOver(AstNodeBiop* nodep) { + // EQ(const{width32}, EXTEND(xx{width3})) -> constant + // When the constant has non-zero bits above the extend it's a constant. + // Avoids compiler warning + AstExtend* extendp = nodep->rhsp()->castExtend(); + if (!extendp) return false; + AstNode* smallerp = extendp->lhsp(); + int subsize = smallerp->width(); + AstConst* constp = nodep->lhsp()->castConst(); + if (!constp) return false; + if (constp->num().isBitsZero(constp->width()-1, subsize)) return false; + return true; + } AstNode* afterComment(AstNode* nodep) { // Ignore comments, such as to determine if a AstIf is empty. @@ -2216,12 +2229,18 @@ private: TREEOP ("AstShiftR{operandShiftShift(nodep)}", "replaceShiftShift(nodep)"); TREEOP ("AstWordSel{operandWordOOB(nodep)}", "replaceZero(nodep)"); // Compress out EXTENDs to appease loop unroller - TREEOPV("AstEq {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); - TREEOPV("AstNeq {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); - TREEOPV("AstGt {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); - TREEOPV("AstGte {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); - TREEOPV("AstLt {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); - TREEOPV("AstLte {$rhsp.castExtend,operandBiExtendConst(nodep)}", "DONE"); + TREEOPV("AstEq {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstNeq {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstGt {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstGte {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstLt {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstLte {$rhsp.castExtend,operandBiExtendConstShrink(nodep)}", "DONE"); + TREEOPV("AstEq {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceZero(nodep)"); + TREEOPV("AstNeq {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceNum(nodep,1)"); + TREEOPV("AstGt {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceNum(nodep,1)"); + TREEOPV("AstGte {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceNum(nodep,1)"); + TREEOPV("AstLt {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceZero(nodep)"); + TREEOPV("AstLte {$rhsp.castExtend,operandBiExtendConstOver(nodep)}", "replaceZero(nodep)"); // Identical operands on both sides // AstLogAnd/AstLogOr already converted to AstAnd/AstOr for these rules // AstAdd->ShiftL(#,1) but uncommon diff --git a/test_regress/t/t_var_overcmp.pl b/test_regress/t/t_var_overcmp.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_var_overcmp.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# 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 +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_overcmp.v b/test_regress/t/t_var_overcmp.v new file mode 100644 index 000000000..b04f0baeb --- /dev/null +++ b/test_regress/t/t_var_overcmp.v @@ -0,0 +1,145 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Outputs + dout, + // Inputs + clk, rstn, dval0, dval1, dbgsel_w + ); + + input clk; + input rstn; + input [7:0] dval0; + input [7:0] dval1; + input [7:0] dbgsel_w; + output [7:0] dout; + + wire [7:0] dout = dout0 | dout1; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + wire [7:0] dout0; // From sub0 of sub0.v + wire [7:0] dout1; // From sub1 of sub1.v + // End of automatics + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end + + reg [7:0] dbgsel_msk; + always_comb begin + reg [7:0] mask; + mask = 8'hff; + dbgsel_msk = (dbgsel_w & mask); + end + + reg [7:0] dbgsel; + always @(posedge clk) begin + if ((rstn == 0)) begin + dbgsel <= 0; + end + else begin + dbgsel <= dbgsel_msk; + end + end + + sub0 sub0 (/*AUTOINST*/ + // Outputs + .dout0 (dout0[7:0]), + // Inputs + .rstn (rstn), + .clk (clk), + .dval1 (dval1[7:0]), + .dbgsel (dbgsel[7:0])); + sub1 sub1 (/*AUTOINST*/ + // Outputs + .dout1 (dout1[7:0]), + // Inputs + .rstn (rstn), + .clk (clk), + .dval1 (dval1[7:0]), + .dbgsel (dbgsel[7:0])); + +endmodule + +module sub0 + ( + /*AUTOARG*/ + // Outputs + dout0, + // Inputs + rstn, clk, dval1, dbgsel + ); + + input rstn; + input clk; + input [7:0] dval1; + input [7:0] dbgsel; + output reg [7:0] dout0; + + reg [7:0] dbgsel_d1r; + + always_comb begin + // verilator lint_off WIDTH + if (((dbgsel_d1r >= 34) && (dbgsel_d1r < 65))) begin + // verilator lint_on WIDTH + dout0 = dval1; + end + else begin + dout0 = 0; + end + end + + always @(posedge clk) begin + if ((rstn == 0)) begin + dbgsel_d1r <= 0; + end + else begin + dbgsel_d1r <= dbgsel; + end + end + +endmodule + +module sub1 + ( + /*AUTOARG*/ + // Outputs + dout1, + // Inputs + rstn, clk, dval1, dbgsel + ); + + input rstn; + input clk; + input [7:0] dval1; + input [7:0] dbgsel; + output reg [7:0] dout1; + + reg [7:0] dbgsel_d1r; + + always_comb begin + // verilator lint_off WIDTH + if (((dbgsel_d1r >= 334) && (dbgsel_d1r < 365))) begin + // verilator lint_on WIDTH + dout1 = dval1; + end + else begin + dout1 = 0; + end + end + + always @(posedge clk) begin + if ((rstn == 0)) begin + dbgsel_d1r <= 0; + end + else begin + dbgsel_d1r <= dbgsel; + end + end + +endmodule