From 92718a819c69c64788595e58b038448d99f21b24 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Fri, 9 Oct 2009 22:55:37 -0400 Subject: [PATCH] Fix cell port connection to unsized integer causing false width warning. --- Changes | 2 + src/V3Width.cpp | 48 +++++++++--- test_regress/t/t_inst_notunsized.pl | 19 +++++ test_regress/t/t_inst_notunsized.v | 110 +++++++++++++++++++++++++++ test_regress/t/t_param_concat_bad.pl | 2 - test_regress/t/t_select_bad_tri.pl | 2 +- 6 files changed, 168 insertions(+), 15 deletions(-) create mode 100755 test_regress/t/t_inst_notunsized.pl create mode 100644 test_regress/t/t_inst_notunsized.v diff --git a/Changes b/Changes index 05dbcd8e5..a69f7f0ba 100644 --- a/Changes +++ b/Changes @@ -9,6 +9,8 @@ indicates the contributor was also the author of the fix; Thanks! **** Fix VCD files showing internal flattened hierarchy, broke in 3.714. +**** Fix cell port connection to unsized integer causing false width warning. + * Verilator 3.714 2009/09/18 ** Add --bbox-sys option to blackbox $system calls. diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 374a83f39..876a489da 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -284,8 +284,8 @@ private: } virtual void visit(AstSel* nodep, AstNUser* vup) { if (vup->c()->prelim()) { - nodep->fromp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); - nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); + nodep->fromp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); + nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); nodep->widthp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); V3Const::constifyParam(nodep->widthp()); AstConst* widthConstp = nodep->widthp()->castConst(); @@ -323,7 +323,8 @@ private: fromlsb = varrp->varp()->lsb(); } int selwidth = V3Number::log2b(frommsb+1-1)+1; // Width to address a bit - nodep->fromp()->iterateAndNext(*this,WidthVP(selwidth,selwidth,BOTH).p()); + nodep->fromp()->iterateAndNext(*this,WidthVP(selwidth,selwidth,FINAL).p()); + nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,FINAL).p()); if (widthBad(nodep->lsbp(),selwidth,selwidth) && nodep->lsbp()->width()!=32) { nodep->v3warn(WIDTH,"Bit extraction of var["<msbConst()<<":"<lsbConst() <<" outside "<lsbp(),selwidth,selwidth,true); } } @@ -348,7 +350,7 @@ private: virtual void visit(AstArraySel* nodep, AstNUser* vup) { if (vup->c()->prelim()) { nodep->bitp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); - nodep->fromp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); + nodep->fromp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); AstNode* basefromp = AstArraySel::baseFromp(nodep->fromp()); int dimension = AstArraySel::dimension(nodep->fromp()); AstNodeVarRef* varrp = basefromp->castNodeVarRef(); @@ -369,7 +371,7 @@ private: nodep->width(outwidth,outwidth); // Width out = width of array } int selwidth = V3Number::log2b(frommsb+1-1)+1; // Width to address a bit - nodep->fromp()->iterateAndNext(*this,WidthVP(selwidth,selwidth,BOTH).p()); + nodep->fromp()->iterateAndNext(*this,WidthVP(selwidth,selwidth,FINAL).p()); if (widthBad(nodep->bitp(),selwidth,selwidth) && nodep->bitp()->width()!=32) nodep->v3warn(WIDTH,"Bit extraction of array["<width(nodep->num().width(), nodep->num().minWidth()); } } + // We don't size the constant until we commit the widths, as need parameters + // to remain unsized, and numbers to remain unsized to avoid backp() warnings } virtual void visit(AstRand* nodep, AstNUser* vup) { if (vup->c()->prelim()) { @@ -459,7 +463,10 @@ private: width = mwidth = 0; // But see below later. } if (nodep->initp()) { - nodep->initp()->iterateAndNext(*this,WidthVP(width,0,BOTH).p()); + nodep->initp()->iterateAndNext(*this,WidthVP(width,0,PRELIM).p()); + // Although nodep will get a different width for parameters just below, + // we want the init numbers to retain their width/minwidth until parameters are replaced. + nodep->initp()->iterateAndNext(*this,WidthVP(width,0,FINAL).p()); if (nodep->isParam()) { if (nodep->rangep()) { // Parameters need to preserve widthMin from the value, not get a constant size @@ -481,7 +488,8 @@ private: nodep->rangep(new AstRange(nodep->fileline(),width-1,0)); nodep->rangep()->width(width,width); } - nodep->width(width,mwidth); // No need to check; varrefs are the "checkers" + nodep->width(width,mwidth); + // See above note about initp()->...FINAL if (nodep->initp()) widthCheck(nodep,"Initial value",nodep->initp(),width,mwidth); UINFO(4,"varWidthed "<dumpTree(cout," InitPos: "); @@ -730,7 +738,6 @@ private: <<" generates "<width(awidth,awidth); } else { if (nodep->modVarp()->isTristate()) { if (pinwidth != expwidth) { @@ -749,10 +756,12 @@ private: // We can't make the RANGE/EXTEND until V3Inst phase, as need RHS of assignment awidth = expwidth; } - nodep->width(awidth,awidth); + } + nodep->width(awidth,awidth); + nodep->exprp()->iterateAndNext(*this,WidthVP(awidth,awidth,FINAL).p()); + if (!m_cellRangep) { widthCheckPin(nodep, nodep->exprp(), pinwidth, inputPin); } - nodep->exprp()->iterateAndNext(*this,WidthVP(awidth,awidth,FINAL).p()); } //if (debug()) nodep->dumpTree(cout,"- PinPos: "); } @@ -874,8 +883,8 @@ void WidthVisitor::fixWidthExtend (AstNode* nodep, int expWidth) { // Save later constant propagation work, just right-size it. V3Number num (nodep->fileline(), expWidth); num.opAssign(constp->num()); + num.isSigned(nodep->isSigned()); AstNode* newp = new AstConst(nodep->fileline(), num); - newp->signedFrom(constp); constp->replaceWith(newp); pushDeletep(constp); constp=NULL; nodep=newp; @@ -910,8 +919,8 @@ void WidthVisitor::fixWidthReduce (AstNode* nodep, int expWidth) { if (constp) { V3Number num (nodep->fileline(), expWidth); num.opRedOr(constp->num()); + num.isSigned(constp->isSigned()); AstNode* newp = new AstConst(nodep->fileline(), num); - newp->signedFrom(constp); constp->replaceWith(newp); nodep=newp; } else { @@ -979,6 +988,7 @@ void WidthVisitor::widthCheck (AstNode* nodep, const char* side, void WidthVisitor::widthCheckReduce (AstNode* nodep, const char* side, AstNode* underp, int expWidth, int expWidthMin, bool ignoreWarn) { + // Before calling this, iterate into underp with FINAL state, so numbers get resized appropriately if (expWidthMin==0) expWidthMin = expWidth; if (expWidth!=1) nodep->v3fatalSrc("Only for binary functions"); bool bad = widthBad(underp,expWidth,expWidthMin); @@ -997,6 +1007,7 @@ void WidthVisitor::widthCheckReduce (AstNode* nodep, const char* side, } void WidthVisitor::widthCheckPin (AstNode* nodep, AstNode* underp, int expWidth, bool inputPin) { + // Before calling this, iterate into underp with FINAL state, so numbers get resized appropriately bool bad = widthBad(underp,expWidth,expWidth); if (bad && fixAutoExtend(underp/*ref*/,expWidth)) bad=false; // Changes underp if (bad) { @@ -1163,6 +1174,19 @@ class WidthCommitVisitor : public AstNVisitor { // Copy all width() to widthMin(). V3Const expects this private: // VISITORS + virtual void visit(AstConst* nodep, AstNUser*) { + nodep->width(nodep->width(),nodep->width()); + if ((nodep->width() != nodep->num().width()) || !nodep->num().sized()) { + V3Number num (nodep->fileline(), nodep->width()); + num.opAssign(nodep->num()); + num.isSigned(nodep->isSigned()); + AstNode* newp = new AstConst(nodep->fileline(), num); + nodep->replaceWith(newp); + //if (debug()>4) nodep->dumpTree(cout," fixConstSize_old: "); + //if (debug()>4) newp->dumpTree(cout," _new: "); + pushDeletep(nodep); nodep=NULL; + } + } virtual void visit(AstNode* nodep, AstNUser*) { nodep->width(nodep->width(),nodep->width()); nodep->iterateChildren(*this); diff --git a/test_regress/t/t_inst_notunsized.pl b/test_regress/t/t_inst_notunsized.pl new file mode 100755 index 000000000..0d595fbd7 --- /dev/null +++ b/test_regress/t/t_inst_notunsized.pl @@ -0,0 +1,19 @@ +#!/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 ( + verilator_flags2=>["-Wno-IMPLICIT"], + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_inst_notunsized.v b/test_regress/t/t_inst_notunsized.v new file mode 100644 index 000000000..bf62f8f15 --- /dev/null +++ b/test_regress/t/t_inst_notunsized.v @@ -0,0 +1,110 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2009 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc=0; + reg [63:0] crc; + reg [63:0] sum; + + // Take CRC data and apply to testblock inputs + wire [31:0] in = crc[31:0]; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + wire [71:0] muxed; // From test of Test.v + // End of automatics + + Test test (/*AUTOINST*/ + // Outputs + .muxed (muxed[71:0]), + // Inputs + .clk (clk), + .in (in[31:0])); + + // Aggregate outputs into a single result vector + wire [63:0] result = {muxed[63:0]}; + + wire [5:0] width_check = cyc[5:0] + 1; + + // Test loop + always @ (posedge clk) begin +`ifdef TEST_VERBOSE + $write("[%0t] cyc==%0d crc=%x result=%x\n",$time, cyc, crc, result); +`endif + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63]^crc[2]^crc[0]}; + sum <= result ^ {sum[62:0],sum[63]^sum[2]^sum[0]}; + if (cyc==0) begin + // Setup + crc <= 64'h5aef0c8d_d70a4497; + sum <= 64'h0; + end + else if (cyc<10) begin + sum <= 64'h0; + end + else if (cyc<90) begin + end + else if (cyc==99) begin + $write("[%0t] cyc==%0d crc=%x sum=%x\n",$time, cyc, crc, sum); + if (crc !== 64'hc77bb9b3784ea091) $stop; + // What checksum will we end up with (above print should match) +`define EXPECTED_SUM 64'h20050a66e7b253d1 + if (sum !== `EXPECTED_SUM) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module Test (/*AUTOARG*/ + // Outputs + muxed, + // Inputs + clk, in + ); + input clk; + input [31:0] in; + output [71:0] muxed; + + wire [71:0] a = {in[7:0],~in[31:0],in[31:0]}; + wire [71:0] b = {~in[7:0],in[31:0],~in[31:0]}; + + /*AUTOWIRE*/ + Muxer muxer ( + .sa (0), + .sb (in[0]), + /*AUTOINST*/ + // Outputs + .muxed (muxed[71:0]), + // Inputs + .a (a[71:0]), + .b (b[71:0])); +endmodule + +module Muxer (/*AUTOARG*/ + // Outputs + muxed, + // Inputs + sa, sb, a, b + ); + input sa; + input sb; + + output wire [71:0] muxed; + input [71:0] a; + input [71:0] b; + + // Constification wasn't sizing with inlining and gave + // unsized error on below + // v + assign muxed = (({72{sa}} & a) + | ({72{sb}} & b)); +endmodule diff --git a/test_regress/t/t_param_concat_bad.pl b/test_regress/t/t_param_concat_bad.pl index 7c502b6c1..496a4682f 100755 --- a/test_regress/t/t_param_concat_bad.pl +++ b/test_regress/t/t_param_concat_bad.pl @@ -15,8 +15,6 @@ compile ( '%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.*', ); diff --git a/test_regress/t/t_select_bad_tri.pl b/test_regress/t/t_select_bad_tri.pl index 1dd538e88..1dd2d0389 100755 --- a/test_regress/t/t_select_bad_tri.pl +++ b/test_regress/t/t_select_bad_tri.pl @@ -11,7 +11,7 @@ compile ( v_flags2 => ["--lint-only"], fails=>$Self->{v3}, expect=> -q{%Error: t/t_select_bad_tri.v:\d+: Selection index is constantly unknown or tristated: lsb=7'bxxxxxxx width=\?32\?sh47 +q{%Error: t/t_select_bad_tri.v:\d+: Selection index is constantly unknown or tristated: lsb=7'bxxxxxxx width=32'sh47 %Error: Exiting due to.*}, );