From 2385b32cb34016f3f59a52e3fe1d284412c3f91a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 11 Apr 2022 09:48:29 +0200 Subject: [PATCH 1/4] Correctly handle unpacked array typedefs for ports If the type of a port is an array type it currently always gets evaluated in the scope where the port is declared. But if the type is a typedef it might be declared in a different scope and must be evaluated in that scope. E.g. the following will declare an array port with 10 entries and an element type of a 5 bit vector, while it should declare one with 4 entries and an element type of a 2 bit vector. ``` localparam A = 2; localparam B = 4; typedef [A-1:0] T[B]; module test ( T x ); localparam A = 5; localparam B = 10; endmodule ``` This is in part due to array types being given special handling. This was necessary before because each base type required slightly different handling and so the base type had to be extracted from the array type. This has now been consolidated and all data types are treated the same. The only exception is the vector type which still needs special handling to support separate definition of port direction and type. As a result it is possible to remove the special handling of the array type. This solves the problem of evaluating the type in the wrong scope. Some special handling needs to be retained though to be able to differentiate between array dimensions that are part of a type and array dimensions that are part of port declaration. This is again necessary to correctly support separate definition of port direction and type. E.g. in the example below port `x` and `y` get treated slightly differently, even though the resulting signals will be identical. ``` typedef logic [7:0] T[1:0]; ... input T x; input [7:0] y[1:0]; ``` Signed-off-by: Lars-Peter Clausen --- PWire.cc | 3 +-- PWire.h | 2 -- elab_sig.cc | 14 +++++--------- parse.y | 33 +++++++++++++++++---------------- pform.cc | 40 +++------------------------------------- pform.h | 1 + 6 files changed, 27 insertions(+), 66 deletions(-) diff --git a/PWire.cc b/PWire.cc index b3b198981..d9a9899c6 100644 --- a/PWire.cc +++ b/PWire.cc @@ -33,8 +33,7 @@ PWire::PWire(perm_string n, : name_(n), type_(t), port_type_(pt), data_type_(dt), signed_(false), port_set_(false), net_set_(false), is_scalar_(false), - error_cnt_(0), uarray_type_(0), set_data_type_(0), - discipline_(0) + error_cnt_(0), set_data_type_(0), discipline_(0) { switch (rt) { case SR_PORT: diff --git a/PWire.h b/PWire.h index af718eb9d..e9565a2d5 100644 --- a/PWire.h +++ b/PWire.h @@ -77,7 +77,6 @@ class PWire : public PNamedItem { void set_range(const std::list&ranges, PWSRType type); void set_unpacked_idx(const std::list&ranges); - void set_uarray_type(uarray_type_t*type) { uarray_type_ = type; } void set_data_type(data_type_t*type); @@ -121,7 +120,6 @@ class PWire : public PNamedItem { // If this wire is actually a memory, these indices will give // me the size and address ranges of the memory. std::listunpacked_; - uarray_type_t*uarray_type_; // This is the complex type of the wire. the data_type_ may // modify how this is interpreted. diff --git a/elab_sig.cc b/elab_sig.cc index 288bbb634..a726f22e8 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -1003,13 +1003,9 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const unsigned wid = 1; vectorpacked_dimensions; - NetScope*array_type_scope = scope; - if (uarray_type_ && !uarray_type_->name.nil()) - array_type_scope = array_type_scope->find_typedef_scope(des, uarray_type_); - - NetScope*base_type_scope = array_type_scope; + NetScope *type_scope = scope; if (set_data_type_ && !set_data_type_->name.nil()) - base_type_scope = base_type_scope->find_typedef_scope(des, set_data_type_); + type_scope = type_scope->find_typedef_scope(des, set_data_type_); des->errors += error_cnt_; @@ -1050,7 +1046,7 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const cerr << get_fileline() << ": PWire::elaborate_sig: " << "Evaluate ranges for net " << basename() << endl; } - dimensions_ok &= evaluate_ranges(des, base_type_scope, this, nlist, net_); + dimensions_ok &= evaluate_ranges(des, type_scope, this, nlist, net_); } assert(net_set_ || net_.empty()); @@ -1151,10 +1147,10 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const wtype = NetNet::WIRE; } - ivl_type_t type = elaborate_type(des, array_type_scope, packed_dimensions); + ivl_type_t type = elaborate_type(des, type_scope, packed_dimensions); // Create the type for the unpacked dimensions. If the // unpacked_dimensions are empty this will just return the base type. - type = elaborate_array_type(des, array_type_scope, *this, type, unpacked_); + type = elaborate_array_type(des, type_scope, *this, type, unpacked_); list unpacked_dimensions; // If this is an unpacked array extract the base type and unpacked diff --git a/parse.y b/parse.y index a8719965d..7ece997e1 100644 --- a/parse.y +++ b/parse.y @@ -4350,7 +4350,8 @@ list_of_port_declarations pform_module_define_port(@3, name, port_declaration_context.port_type, port_declaration_context.port_net_type, - port_declaration_context.data_type, 0); + port_declaration_context.data_type, + nullptr, nullptr); delete[]$3; $$ = tmp; } @@ -4369,10 +4370,9 @@ port_declaration : attribute_list_opt K_input net_type_or_var_opt data_type_or_implicit IDENTIFIER dimensions_opt { Module::port_t*ptmp; perm_string name = lex_strings.make($5); - data_type_t*use_type = $4; - if ($6) use_type = new uarray_type_t(use_type, $6); ptmp = pform_module_port_reference(@2, name); - pform_module_define_port(@2, name, NetNet::PINPUT, $3, use_type, $1); + pform_module_define_port(@2, name, NetNet::PINPUT, $3, $4, $6, nullptr, + $1); port_declaration_context.port_type = NetNet::PINPUT; port_declaration_context.port_net_type = $3; port_declaration_context.data_type = $4; @@ -4386,8 +4386,8 @@ port_declaration ptmp = pform_module_port_reference(@2, name); real_type_t*real_type = new real_type_t(real_type_t::REAL); FILE_NAME(real_type, @3); - pform_module_define_port(@2, name, NetNet::PINPUT, - NetNet::WIRE, real_type, $1); + pform_module_define_port(@2, name, NetNet::PINPUT, NetNet::WIRE, + real_type, nullptr, $1); port_declaration_context.port_type = NetNet::PINPUT; port_declaration_context.port_net_type = NetNet::WIRE; port_declaration_context.data_type = real_type; @@ -4401,7 +4401,8 @@ port_declaration data_type_t*use_type = $4; ptmp = pform_module_port_reference(@2, name); ptmp->default_value = $7; - pform_module_define_port(@2, name, NetNet::PINPUT, $3, use_type, $1); + pform_module_define_port(@2, name, NetNet::PINPUT, $3, use_type, + nullptr, $1); port_declaration_context.port_type = NetNet::PINPUT; port_declaration_context.port_net_type = $3; port_declaration_context.data_type = $4; @@ -4412,7 +4413,8 @@ port_declaration { Module::port_t*ptmp; perm_string name = lex_strings.make($5); ptmp = pform_module_port_reference(@2, name); - pform_module_define_port(@2, name, NetNet::PINOUT, $3, $4, $1); + pform_module_define_port(@2, name, NetNet::PINOUT, $3, $4, nullptr, + $1); port_declaration_context.port_type = NetNet::PINOUT; port_declaration_context.port_net_type = $3; port_declaration_context.data_type = $4; @@ -4430,8 +4432,8 @@ port_declaration ptmp = pform_module_port_reference(@2, name); real_type_t*real_type = new real_type_t(real_type_t::REAL); FILE_NAME(real_type, @3); - pform_module_define_port(@2, name, NetNet::PINOUT, - NetNet::WIRE, real_type, $1); + pform_module_define_port(@2, name, NetNet::PINOUT, NetNet::WIRE, + real_type, nullptr, $1); port_declaration_context.port_type = NetNet::PINOUT; port_declaration_context.port_net_type = NetNet::WIRE; port_declaration_context.data_type = real_type; @@ -4441,8 +4443,6 @@ port_declaration | attribute_list_opt K_output net_type_or_var_opt data_type_or_implicit IDENTIFIER dimensions_opt { Module::port_t*ptmp; perm_string name = lex_strings.make($5); - data_type_t*use_dtype = $4; - if ($6) use_dtype = new uarray_type_t(use_dtype, $6); NetNet::Type use_type = $3; if (use_type == NetNet::IMPLICIT) { if (vector_type_t*dtype = dynamic_cast ($4)) { @@ -4460,7 +4460,7 @@ port_declaration } } ptmp = pform_module_port_reference(@2, name); - pform_module_define_port(@2, name, NetNet::POUTPUT, use_type, use_dtype, $1); + pform_module_define_port(@2, name, NetNet::POUTPUT, use_type, $4, $6, $1); port_declaration_context.port_type = NetNet::POUTPUT; port_declaration_context.port_net_type = use_type; port_declaration_context.data_type = $4; @@ -4474,8 +4474,8 @@ port_declaration ptmp = pform_module_port_reference(@2, name); real_type_t*real_type = new real_type_t(real_type_t::REAL); FILE_NAME(real_type, @3); - pform_module_define_port(@2, name, NetNet::POUTPUT, - NetNet::WIRE, real_type, $1); + pform_module_define_port(@2, name, NetNet::POUTPUT, NetNet::WIRE, + real_type, nullptr, $1); port_declaration_context.port_type = NetNet::POUTPUT; port_declaration_context.port_net_type = NetNet::WIRE; port_declaration_context.data_type = real_type; @@ -4490,7 +4490,8 @@ port_declaration use_type = NetNet::IMPLICIT_REG; } ptmp = pform_module_port_reference(@2, name); - pform_module_define_port(@2, name, NetNet::POUTPUT, use_type, $4, $1); + pform_module_define_port(@2, name, NetNet::POUTPUT, use_type, $4, + nullptr, $1); port_declaration_context.port_type = NetNet::POUTPUT; port_declaration_context.port_net_type = use_type; port_declaration_context.data_type = $4; diff --git a/pform.cc b/pform.cc index b92e909f6..01cd48d8d 100644 --- a/pform.cc +++ b/pform.cc @@ -2591,21 +2591,12 @@ void pform_module_define_port(const struct vlltype&li, NetNet::PortType port_kind, NetNet::Type type, data_type_t*vtype, + list*urange, list*attr, bool keep_attr) { pform_check_net_data_type(li, type, vtype); - // Unpacked dimensions - list*urange = 0; - - // If this is an unpacked array, then split out the parts that - // we can send to the PWire object that we create. - if (uarray_type_t*uarr_type = dynamic_cast (vtype)) { - urange = uarr_type->dims.get(); - vtype = uarr_type->base_type; - } - PWire *cur = pform_get_or_make_wire(li, name, type, port_kind, SR_BOTH); vector_type_t*vec_type = dynamic_cast (vtype); @@ -2616,6 +2607,7 @@ void pform_module_define_port(const struct vlltype&li, if (urange) { cur->set_unpacked_idx(*urange); + delete urange; } pform_bind_attributes(cur->attributes, attr, keep_attr); @@ -2632,13 +2624,9 @@ void pform_module_define_port(const struct vlltype&li, ; cur != ports->end() ; ++ cur ) { data_type_t*use_type = vtype; - if (cur->udims) - use_type = new uarray_type_t(vtype, cur->udims); pform_module_define_port(li, cur->name, port_kind, type, use_type, - attr, true); - if (cur->udims) - delete use_type; + cur->udims, attr, true); if (cur->expr) pform_make_var_init(li, cur->name, cur->expr); @@ -2820,12 +2808,6 @@ vector*pform_make_task_ports(const struct vlltype&loc, bool allow_implicit) { vector*ret = NULL; - std::list*unpacked_dims = NULL; - - if (uarray_type_t*uarray = dynamic_cast (vtype)) { - unpacked_dims = uarray->dims.get(); - vtype = uarray->base_type; - } if (vector_type_t*vec_type = dynamic_cast (vtype)) { ret = pform_make_task_ports_vec(loc, pt, vec_type, ports, @@ -2834,14 +2816,6 @@ vector*pform_make_task_ports(const struct vlltype&loc, ret = do_make_task_ports(loc, pt, vtype, ports); } - if (unpacked_dims) { - for (list::iterator cur = ports->begin() - ; cur != ports->end() ; ++ cur ) { - PWire*wire = pform_get_wire_in_scope(cur->name); - wire->set_unpacked_idx(*unpacked_dims); - } - } - delete ports; return ret; } @@ -3213,10 +3187,6 @@ void pform_set_data_type(const struct vlltype&li, data_type_t*data_type, assert(0); } - uarray_type_t*uarray_type = dynamic_cast (data_type); - if (uarray_type) - data_type = uarray_type->base_type; - vector_type_t*vec_type = dynamic_cast (data_type); for (std::vector::iterator it= wires->begin(); @@ -3230,10 +3200,6 @@ void pform_set_data_type(const struct vlltype&li, data_type_t*data_type, bool rc = wire->set_wire_type(net_type); ivl_assert(li, rc); - if (uarray_type) { - wire->set_unpacked_idx(*uarray_type->dims.get()); - wire->set_uarray_type(uarray_type); - } wire->set_data_type(data_type); pform_bind_attributes(wire->attributes, attr, true); diff --git a/pform.h b/pform.h index 09e2392f7..6e7d617d6 100644 --- a/pform.h +++ b/pform.h @@ -154,6 +154,7 @@ extern void pform_module_define_port(const struct vlltype&li, NetNet::PortType, NetNet::Type type, data_type_t*vtype, + std::list*urange, std::list*attr, bool keep_attr =false); extern void pform_module_define_port(const struct vlltype&li, From bcc0730b6b341956b3f462976f6c8c62413ed6f9 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 23 Apr 2022 09:04:45 +0200 Subject: [PATCH 2/4] Add regression test for module port with array typedef Check that for a module port with an array type identifier the type is elaborated in the right scope. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/module_port_typedef_array1.v | 32 ++++++++++++++++++++ ivtest/regress-sv.list | 1 + ivtest/regress-vlog95.list | 1 + 3 files changed, 34 insertions(+) create mode 100644 ivtest/ivltests/module_port_typedef_array1.v diff --git a/ivtest/ivltests/module_port_typedef_array1.v b/ivtest/ivltests/module_port_typedef_array1.v new file mode 100644 index 000000000..f84d27e36 --- /dev/null +++ b/ivtest/ivltests/module_port_typedef_array1.v @@ -0,0 +1,32 @@ +// Check that an array type identifier used for a module port is elaborated in +// the correct scope. + +localparam A = 2; +localparam B = 4; + +typedef logic [A-1:0] T[B]; + +module test ( + input T x +); + + localparam A = 5; + localparam B = 10; + + bit failed = 1'b0; + + `define check(expr, val) \ + if (expr !== val) begin \ + $display("FAILED: %s, expected %0d, got %0d", `"expr`", val, expr); \ + failed = 1'b1; \ + end + + initial begin + `check($size(x, 1), 4); + `check($size(x, 2), 2); + if (!failed) begin + $display("PASSED"); + end + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index acc997669..547a3752c 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -368,6 +368,7 @@ module_nonansi_struct2 normal,-g2005-sv ivltests module_nonansi_struct_fail CE,-g2005-sv ivltests module_output_port_sv_var1 normal,-g2005-sv ivltests module_output_port_sv_var2 normal,-g2005-sv ivltests +module_port_typedef_array1 normal,-g2005-sv ivltests named_begin normal,-g2009 ivltests named_begin_fail CE,-g2009 ivltests named_fork normal,-g2009 ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index 0aeb61f90..6853f3969 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -225,6 +225,7 @@ br_ml20171017 CE ivltests genvar_scopes CE ivltests meminit2 CE ivltests memsynth4 CE,-S ivltests # Synthesized net array +module_port_typedef_array1 CE,-g2005-sv ivltests # Module port array negative_genvar CE ivltests pr1565544 CE ivltests pr1657307 CE ivltests From b19a6a7518e7420b58019428a8da70fa1b2fc251 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 20 Mar 2022 16:25:51 +0100 Subject: [PATCH 3/4] Support nested unpacked array types It is currently possible to declare an unpacked array with multiple dimensions. But trying to declare an unpacked array that has another unpacked array as a base type will result in undefined behavior. E.g. ``` typedef int T1[1:0]; typedef T1 T2[3:0]; T2 x[7:0]; ``` To support this recursively unwrap the data type and add the unpacked dimensions to the signal until the base type no longer is a unpacked array type. Signed-off-by: Lars-Peter Clausen --- elab_sig.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elab_sig.cc b/elab_sig.cc index a726f22e8..f4e26c5bd 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -1155,7 +1155,7 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const list unpacked_dimensions; // If this is an unpacked array extract the base type and unpacked // dimensions as these are separate properties of the NetNet. - if (const netuarray_t *atype = dynamic_cast(type)) { + while (const netuarray_t *atype = dynamic_cast(type)) { unpacked_dimensions.insert(unpacked_dimensions.begin(), atype->static_dimensions().begin(), atype->static_dimensions().end()); From 4c0b06329f0268683dce5e5f35bc14b8639ee514 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 16 Apr 2022 21:15:14 +0200 Subject: [PATCH 4/4] Add regression test for nested unpacked arrays Check that it is possible to declare an unpacked array type with an unpacked array type as the base type. Also check that it is possible to declare an signal with an unpacked array dimension with an unpacked array base type. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_typedef_nested_array.v | 43 +++++++++++++++++++++++ ivtest/regress-sv.list | 1 + 2 files changed, 44 insertions(+) create mode 100644 ivtest/ivltests/sv_typedef_nested_array.v diff --git a/ivtest/ivltests/sv_typedef_nested_array.v b/ivtest/ivltests/sv_typedef_nested_array.v new file mode 100644 index 000000000..8078b5c84 --- /dev/null +++ b/ivtest/ivltests/sv_typedef_nested_array.v @@ -0,0 +1,43 @@ +// Check that nested unpacked array types are supported. + +module test; + + localparam A = 3; + localparam B = 5; + localparam C = 2; + localparam D = 7; + + typedef logic [31:0] T1[A]; + typedef T1 T2[B][C]; + + T2 x[D]; + T2 y; + + bit failed = 1'b0; + + `define check(expr, val) \ + if (expr !== val) begin \ + $display("FAILED: %s, expected %0d, got %0d", `"expr`", val, expr); \ + failed = 1'b1; \ + end + + initial begin + `check($unpacked_dimensions(x), 4) + `check($size(x, 1), A) + `check($size(x, 2), B) + `check($size(x, 3), C) + `check($size(x, 4), D) + + `check($unpacked_dimensions(y), 3) + `check($size(y, 1), A) + `check($size(y, 2), B) + `check($size(y, 3), C) + + `check($bits(T2), $bits(integer) * A * B * C) + + if (!failed) begin + $display("PASSED"); + end + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 547a3752c..d75b11333 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -674,6 +674,7 @@ sv_typedef_darray_base1 normal,-g2009 ivltests sv_typedef_darray_base2 normal,-g2009 ivltests sv_typedef_darray_base3 normal,-g2009 ivltests sv_typedef_darray_base4 normal,-g2009 ivltests +sv_typedef_nested_array normal,-g2009 ivltests sv_typedef_queue_base1 normal,-g2009 ivltests sv_typedef_queue_base2 normal,-g2009 ivltests sv_typedef_queue_base3 normal,-g2009 ivltests