Fix signed/unsigned parameter misconversion, bug606.

Changes

@@ -20,6 +20,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 ****  Fix implicit one bit parameter selection, bug603.  [Jeremy Bennett]
 
+****  Fix signed/unsigned parameter misconversion, bug606.  [Jeremy Bennett]
+
 ****  Fix package logic var compile error.
 
 

src/V3Ast.cpp

@@ -1067,8 +1067,7 @@ void AstNode::dtypeChgWidth(int width, int widthMin) {
     dtypeChgWidthSigned(width, widthMin, dtypep()->numeric());
 }
 
-void AstNode::dtypeChgWidthSigned(int width, int widthMin, bool issigned) {
-    AstNumeric numeric = issigned ? AstNumeric::SIGNED : AstNumeric::UNSIGNED;
+void AstNode::dtypeChgWidthSigned(int width, int widthMin, AstNumeric numeric) {
     if (!dtypep()) {
 	// We allow dtypep() to be null, as before/during widthing dtypes are not resolved
 	dtypeSetLogicSized(width, widthMin, numeric);

src/V3Ast.h

@@ -1062,7 +1062,7 @@ public:
     void	dtypeFrom(AstNode* fromp) { if (fromp) { dtypep(fromp->dtypep()); }}
     void	dtypeChgSigned(bool flag=true);
     void	dtypeChgWidth(int width, int widthMin);
-    void	dtypeChgWidthSigned(int width, int widthMin, bool issigned);
+    void	dtypeChgWidthSigned(int width, int widthMin, AstNumeric numeric);
     void	dtypeSetBitSized(int width, int widthMin, AstNumeric numeric) { dtypep(findBitDType(width,widthMin,numeric)); }
     void	dtypeSetLogicSized(int width, int widthMin, AstNumeric numeric) { dtypep(findLogicDType(width,widthMin,numeric)); }
     void	dtypeSetLogicBool()	{ dtypep(findLogicBoolDType()); }

src/V3Param.cpp

@@ -324,13 +324,13 @@ void ParamVisitor::visit(AstCell* nodep, AstNUser*) {
     if (nodep->paramsp()) {
 	UINFO(4,"De-parameterize: "<<nodep<<endl);
 	// Create new module name with _'s between the constants
-	if (debug()>9) nodep->dumpTree(cout,"cell:\t");
+	if (debug()>=10) nodep->dumpTree(cout,"-cell:\t");
 	// Evaluate all module constants
 	V3Const::constifyParamsEdit(nodep);
 
 	// Make sure constification worked
 	// Must be a separate loop, as constant conversion may have changed some pointers.
-	//if (debug()) nodep->dumpTree(cout,"cel2:\t");
+	//if (debug()) nodep->dumpTree(cout,"-cel2:\t");
 	string longname = nodep->modp()->name();
 	bool any_overrides = false;
 	longname += "_";
@@ -442,6 +442,7 @@ void ParamVisitor::visit(AstCell* nodep, AstNUser*) {
 	// Delete the parameters from the cell; they're not relevant any longer.
 	nodep->paramsp()->unlinkFrBackWithNext()->deleteTree();
 	UINFO(8,"     Done with "<<nodep<<endl);
+	//if (debug()>=10) v3Global.rootp()->dumpTreeFile(v3Global.debugFilename("param-out.tree"));
     }
 
     // Now remember to process the child module at the end of the module

src/V3Width.cpp

@@ -836,13 +836,15 @@ private:
 		    if (width==1) {
 			// one bit parameter is same as "parameter [0] foo", not "parameter logic foo"
 			// as you can extract "foo[0]" from a parameter but not a wire
-			nodep->dtypeChgWidthSigned(width, nodep->valuep()->widthMin(),issigned);
+			nodep->dtypeChgWidthSigned(width, nodep->valuep()->widthMin(),
+						   issigned?AstNumeric::SIGNED : AstNumeric::UNSIGNED);
 			nodep->dtypep(nodep->findLogicRangeDType
 				      (VNumRange(0,0,false),
 				       nodep->valuep()->widthMin(),
 				       issigned?AstNumeric::SIGNED : AstNumeric::UNSIGNED));
 		    } else {
-			nodep->dtypeChgWidthSigned(width, nodep->valuep()->widthMin(),issigned);
+			nodep->dtypeChgWidthSigned(width, nodep->valuep()->widthMin(),
+						   issigned?AstNumeric::SIGNED : AstNumeric::UNSIGNED);
 		    }
 		    didchk = true;
 		    nodep->valuep()->iterateAndNext(*this,WidthVP(width,nodep->widthMin(),FINAL).p());
@@ -1901,7 +1903,8 @@ private:
 	int width  = max(vup->c()->width(),    max(nodep->lhsp()->width(),    nodep->rhsp()->width()));
 	int mwidth = max(vup->c()->widthMin(), max(nodep->lhsp()->widthMin(), nodep->rhsp()->widthMin()));
 	bool expSigned = (nodep->lhsp()->isSigned() && nodep->rhsp()->isSigned());
-	nodep->dtypeChgWidthSigned(width,mwidth,expSigned);
+	nodep->dtypeChgWidthSigned(width,mwidth,
+				   expSigned?AstNumeric::SIGNED : AstNumeric::UNSIGNED);
 	if (vup->c()->final()) {
 	    // Final call, so make sure children check their sizes
 	    nodep->lhsp()->iterateAndNext(*this,WidthVP(width,mwidth,FINAL).p());
@@ -2072,7 +2075,8 @@ private:
 	    linker.relink(newp);
 	    nodep=newp;
 	}
-	nodep->dtypeChgWidthSigned(expWidth,expWidth,expSigned);
+	nodep->dtypeChgWidthSigned(expWidth,expWidth,
+				   expSigned?AstNumeric::SIGNED : AstNumeric::UNSIGNED);
 	UINFO(4,"             _new: "<<nodep<<endl);
     }
 

test_regress/t/t_param_module.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 only test
+compile (
+    );
+
+execute (
+    check_finished=>1,
+    );
+
+ok(1);
+1;

test_regress/t/t_param_module.v

@@ -0,0 +1,55 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This test case is used for testing a modeule parameterized with a typed
+// localparam.
+//
+// We find Verilator appears to mis-evaluate the parameter WIDTH as -16 when
+// used in the test module to set the value of MSB. A number of warnings and
+// errors follow, starting with:
+//
+// %Warning-LITENDIAN: t/t_param_module.v:42: Little bit endian vector: MSB
+// < LSB of bit range: -17:0
+//
+// This file ONLY is placed into the Public Domain, for any use, without
+// warranty, 2013 by Jie Xu.
+
+// bug606
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+  localparam logic[4:0] WID = 16;
+  //localparam WID = 16;	// No problem if defined like this
+  wire [15:0] b33;
+
+  test #(WID) i_test_33(.clk (clk),
+			.b   (b33));
+
+endmodule
+
+
+module test (/*AUTOARG*/
+   //Inputs
+   clk,
+   // Outputs
+   b
+   );
+   parameter  WIDTH = 10;
+   localparam MSB   = WIDTH - 1;
+
+   input               clk;
+   output wire [MSB:0] b;
+
+   wire [MSB:0]        a;
+   assign b = {~a[MSB-1:0], clk};
+
+   initial begin
+      if ($bits(WIDTH)!=5) $stop;  // Comes from the parent!
+      if ($bits(MSB)!=32) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule