From 9026118a7caec045c45af1fb6ae74ed7fd4a45e9 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 25 Oct 2006 21:41:32 +0000 Subject: [PATCH] Fix (blah) with width violation git-svn-id: file://localhost/svn/verilator/trunk/verilator@828 77ca24e4-aefa-0310-84f0-b9a241c72d87 --- Changes | 4 +++ bin/verilator | 26 ++++++++------ src/V3Width.cpp | 40 +++++++++++++++------ test_regress/t/t_math_mul.pl | 18 ++++++++++ test_regress/t/t_math_mul.v | 67 ++++++++++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 20 deletions(-) create mode 100755 test_regress/t/t_math_mul.pl create mode 100644 test_regress/t/t_math_mul.v diff --git a/Changes b/Changes index 88c36a640..ae6bd77aa 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,10 @@ Revision history for Verilator The contributors that suggested a given feature are shown in []. [by ...] indicates the contributor was also the author of the fix; Thanks! +* Verilator 3.6** + +**** Fix $signed mis-extending when input has a WIDTH violation. [Eugene Weber] + * Verilator 3.622 10/17/2006 Stable **** Fix --skip-identical without --debug, broken in 3.621. [Andy Meier] diff --git a/bin/verilator b/bin/verilator index 49b67c1a0..60a91e7be 100755 --- a/bin/verilator +++ b/bin/verilator @@ -1424,22 +1424,28 @@ simulation which analyzes per-bus: This statement needs to be evaluated multiple times, as a change in "shift_in" requires "x" to be computed 3 times before it becomes stable. -For significantly better performance, split this into 2 separate signals, -and then if necessary generate the original signal: +This is because a change in "x" requires "x" itself to change value, which +causes the warning. - wire [2:1] x_21 = x[1:0]; - wire [0:0] x_0 = shift_in; - wire [2:0] x = {x_21, x_0}; +For significantly better performance, split this into 2 separate signals: -This logic needs to be evaluated only once. These sort of changes may also -speed up your traditional event driven simulator, as it will result in -fewer events per cycle. + wire [2:0] xout = {x[1:0],shift_in}; + +and change all receiving logic to instead receive "xout". Alternatively, +change it to + + wire [2:0] x = {xin[1:0],shift_in}; + +and change all driving logic to instead drive "xin". + +With this change this assignment needs to be evaluated only once. These +sort of changes may also speed up your traditional event driven simulator, +as it will result in fewer events per cycle. The most complicated UNOPTFLAT path we've seen was due to low bits of a bus being generated from an always statement that consumed high bits of the same bus processed by another series of always blocks. The fix is the -same; split it into two separate signals, then create the bus from the two -separate signals. +same; split it into two separate signals generated from each block. The UNOPTFLAT warning may also be due to clock enables, identified from the reported path going through a clock gating cell. To fix these, use the diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 1205603a9..75ca18773 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -144,12 +144,15 @@ private: virtual void visit(AstMul* nodep, AstNUser* vup) { width_Omax_L_Rlhs(nodep,vup); } virtual void visit(AstMulS* nodep, AstNUser* vup) { width_Omax_L_Rlhs(nodep,vup); } - // Widths: out width = lhs width - void width_Olhs_L(AstNode* nodep, AstNUser* vup); + // Widths: out width = lhs width, but upper matters + void width_Olhs_L(AstNodeUniop* nodep, AstNUser* vup); virtual void visit(AstNot* nodep, AstNUser* vup) { width_Olhs_L(nodep,vup); } virtual void visit(AstUnaryMin* nodep, AstNUser* vup) { width_Olhs_L(nodep,vup); } - virtual void visit(AstSigned* nodep, AstNUser* vup) { width_Olhs_L(nodep,vup); } - virtual void visit(AstUnsigned* nodep, AstNUser* vup) { width_Olhs_L(nodep,vup); } + + // Widths: out width = lhs width, upper doesn't matter + void width_Olhs_Lforce(AstNodeUniop* nodep, AstNUser* vup); + virtual void visit(AstSigned* nodep, AstNUser* vup) { width_Olhs_Lforce(nodep,vup); } + virtual void visit(AstUnsigned* nodep, AstNUser* vup) { width_Olhs_Lforce(nodep,vup); } // Widths: Output width from lhs, rhs<33 bits void width_Olhs_L_R32(AstNode* nodep, AstNUser* vup); @@ -924,19 +927,36 @@ void WidthVisitor::width_O1_L_Rlhs(AstNode* nodep, AstNUser* vup) { } } -void WidthVisitor::width_Olhs_L(AstNode* nodep, AstNUser* vup) { +void WidthVisitor::width_Olhs_L(AstNodeUniop* nodep, AstNUser* vup) { // Widths: out width = lhs width // "Interim results shall take the max of operands, including LHS of assignments" if (nodep->op2p()) nodep->v3fatalSrc("For unary ops only!"); if (vup->c()->prelim()) { - nodep->op1p()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); + nodep->lhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); } - int width = max(vup->c()->width(), nodep->op1p()->width()); - int ewidth = max(vup->c()->widthMin(), nodep->op1p()->widthMin()); + int width = max(vup->c()->width(), nodep->lhsp()->width()); + int ewidth = max(vup->c()->widthMin(), nodep->lhsp()->widthMin()); nodep->width(width,ewidth); if (vup->c()->final()) { - nodep->op1p()->iterateAndNext(*this,WidthVP(width,ewidth,FINAL).p()); - widthCheck(nodep,"LHS",nodep->op1p(),width,ewidth); + nodep->lhsp()->iterateAndNext(*this,WidthVP(width,ewidth,FINAL).p()); + widthCheck(nodep,"LHS",nodep->lhsp(),width,ewidth); + } +} + +void WidthVisitor::width_Olhs_Lforce(AstNodeUniop* nodep, AstNUser* vup) { + // Widths: out width = lhs width + // It always comes exactly from LHS; ignores any upper operand + if (nodep->op2p()) nodep->v3fatalSrc("For unary ops only!"); + if (vup->c()->prelim()) { + nodep->lhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); + } + int width = nodep->lhsp()->width(); + int ewidth = nodep->lhsp()->width(); // Not minWidth; force it. + nodep->width(width,ewidth); + if (vup->c()->final()) { + // Final call, so make sure children check their sizes + nodep->lhsp()->iterateAndNext(*this,WidthVP(width,ewidth,FINAL).p()); + widthCheck(nodep,"LHS",nodep->lhsp(),width,ewidth); } } diff --git a/test_regress/t/t_math_mul.pl b/test_regress/t/t_math_mul.pl new file mode 100755 index 000000000..7bfdbe852 --- /dev/null +++ b/test_regress/t/t_math_mul.pl @@ -0,0 +1,18 @@ +#!/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 ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_math_mul.v b/test_regress/t/t_math_mul.v new file mode 100644 index 000000000..14e33416b --- /dev/null +++ b/test_regress/t/t_math_mul.v @@ -0,0 +1,67 @@ +// $Id$ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2006 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + + integer cyc; initial cyc=0; + reg [63:0] crc; + reg [63:0] sum; + + reg [31:0] out1; + sub sub (.in1(crc[15:0]), .in2(crc[31:16]), .out1(out1)); + + always @ (posedge clk) begin + //$write("[%0t] cyc==%0d crc=%x sum=%x out=%x\n",$time, cyc, crc, sum, out1); + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63]^crc[2]^crc[0]}; + sum <= {sum[62:0], sum[63]^sum[2]^sum[0]} ^ {32'h0,out1}; + if (cyc==1) begin + // Setup + crc <= 64'h00000000_00000097; + sum <= 64'h0; + end + else if (cyc==90) begin + if (sum != 64'hc1f743ad62c2c04d) $stop; + end + else if (cyc==91) begin + end + else if (cyc==92) begin + end + else if (cyc==93) begin + end + else if (cyc==94) begin + end + else if (cyc==99) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module sub (/*AUTOARG*/ + // Outputs + out1, + // Inputs + in1, in2 + ); + + input [15:0] in1; + input [15:0] in2; + output reg [31:0] out1; + + always @* begin + // verilator lint_off WIDTH + out1 = $signed(in1) * $signed(in2); + // verilator lint_on WIDTH + end + +endmodule