Fix (blah) with width violation

git-svn-id: file://localhost/svn/verilator/trunk/verilator@828 77ca24e4-aefa-0310-84f0-b9a241c72d87

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]

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

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);
     }
 }
 

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;

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