diff --git a/Changes b/Changes index 6760b9f47..d4eb5c207 100644 --- a/Changes +++ b/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. diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index f84254154..62ecb936c 100644 --- a/src/V3Ast.cpp +++ b/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); diff --git a/src/V3Ast.h b/src/V3Ast.h index 80ffb678f..2a2feb00f 100644 --- a/src/V3Ast.h +++ b/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()); } diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 67632dfe0..3b2924b1c 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -324,13 +324,13 @@ void ParamVisitor::visit(AstCell* nodep, AstNUser*) { if (nodep->paramsp()) { UINFO(4,"De-parameterize: "<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 "<=10) v3Global.rootp()->dumpTreeFile(v3Global.debugFilename("param-out.tree")); } // Now remember to process the child module at the end of the module diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 890539afd..a50e4455e 100644 --- a/src/V3Width.cpp +++ b/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: "<1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_param_module.v b/test_regress/t/t_param_module.v new file mode 100644 index 000000000..3ba3d5d99 --- /dev/null +++ b/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