From 7908e15093298c8fdc554e2f89980de40fce6589 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Jun 2023 08:21:19 -0700 Subject: [PATCH 1/2] Support $bits() for arrays and array slices `$bits()` for array types is supposed to return the full size of the array in bits. This currently works for data types that are passed to `$bits()`, but not for array typed identifiers. E.g. ``` typedef int T[1:0]; T x; $display($bits(T)); // -> 64 $display(x); // -> 32 ``` Since the `$bits()` implementation uses the expr_width of an expression include the size of the unpacked dimensions in that for array identifiers and array slices. Strictly speaking an array identifier does not have an expression width, but this would be its expression with if it were for example bitstream cast to a vector. Special care needs to be take to not trying to pad array identifier expressions. Signed-off-by: Lars-Peter Clausen --- elab_expr.cc | 34 ++++++++++++++++++++-------------- netlist.cc | 4 ++-- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/elab_expr.cc b/elab_expr.cc index 1d7d99f1f..16c77baa6 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -4289,10 +4289,7 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) index_component_t::ctype_t use_sel = index_component_t::SEL_NONE; if (!name_tail.index.empty()) { const index_component_t&index_tail = name_tail.index.back(); - // Skip full array word net selects. - if (!sr.net || (name_tail.index.size() > sr.net->unpacked_dimensions())) { - use_sel = index_tail.sel; - } + use_sel = index_tail.sel; } unsigned use_width = UINT_MAX; @@ -4380,13 +4377,12 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) } } - if (use_width != UINT_MAX) { + size_t use_depth = name_tail.index.size(); + if (use_width != UINT_MAX && (!sr.net || use_depth > sr.net->unpacked_dimensions())) { // We have a bit/part select. Account for any remaining dimensions // beyond the indexed dimension. - size_t use_depth = name_tail.index.size(); if (sr.net) { - if (use_depth >= sr.net->unpacked_dimensions()) - use_depth -= sr.net->unpacked_dimensions(); + use_depth -= sr.net->unpacked_dimensions(); use_width *= sr.net->slice_width(use_depth); } @@ -4426,23 +4422,29 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) } } - size_t use_depth = name_tail.index.size(); + // Unindexed indentifier + if (use_width == UINT_MAX) + use_width = 1; // Account for unpacked dimensions by assuming that the // unpacked dimensions are consumed first, so subtract // the unpacked dimensions from the dimension depth // useable for making the slice. if (use_depth >= sr.net->unpacked_dimensions()) { use_depth -= sr.net->unpacked_dimensions(); - } else { - // In this case, we have a slice of an unpacked - // array. This likely handled as an array instead - // of a slice. Hmm... + // In this case, we have an unpacked array or a slice of an + // unpacked array. These expressions strictly speaking do + // not have a width. But we use the value calculated here + // for things $bits(), so return the full number of bits of + // the expression. + const auto &dims = sr.net->unpacked_dims(); + for (size_t idx = use_depth; idx < dims.size(); idx++) + use_width *= dims[idx].width(); use_depth = 0; } expr_type_ = sr.net->data_type(); - expr_width_ = sr.net->slice_width(use_depth); + expr_width_ = sr.net->slice_width(use_depth) * use_width; min_width_ = expr_width_; signed_flag_ = sr.net->get_signed(); if (debug_elaborate) { @@ -4648,6 +4650,10 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, if (!result || !type_is_vectorable(expr_type_)) return result; + auto net_type = result->net_type(); + if (net_type && !net_type->packed()) + return result; + return pad_to_width(result, expr_wid, signed_flag_, *this); } diff --git a/netlist.cc b/netlist.cc index 1fdd07606..f1f825d16 100644 --- a/netlist.cc +++ b/netlist.cc @@ -2432,10 +2432,10 @@ NetESignal::NetESignal(NetNet*n, NetExpr*w) net_->incr_eref(); set_line(*n); - // If it is an array we don't have a type for it yet. But for array - // elements the NetNet returns the element type. if (word_) set_net_type(net_->net_type()); + else + set_net_type(net_->array_type()); } NetESignal::~NetESignal() From 708f7bc6517f18bb20df7bb39e6431582433fc0a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Jun 2023 09:04:34 -0700 Subject: [PATCH 2/2] Add regression test for $bits() on array identifiers Check that for array identifiers $bits() includes the total size of the signal. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/bits4.v | 37 +++++++++++++++++++++++++++++++++++++ ivtest/regress-vvp.list | 1 + ivtest/vvp_tests/bits4.json | 5 +++++ 3 files changed, 43 insertions(+) create mode 100644 ivtest/ivltests/bits4.v create mode 100644 ivtest/vvp_tests/bits4.json diff --git a/ivtest/ivltests/bits4.v b/ivtest/ivltests/bits4.v new file mode 100644 index 000000000..cfd5b1837 --- /dev/null +++ b/ivtest/ivltests/bits4.v @@ -0,0 +1,37 @@ +// Check that passing a array identifiers and array slices to $bits works as expected + +module test; + + bit failed = 1'b0; + + `define check(expr, value) do begin \ + if ($bits(expr) !== value) begin \ + $display("FAILED(%d): $bits(", `"expr`", ") is %0d", `__LINE__, $bits(expr), " expected %0d", value); \ + failed = 1'b1; \ + end \ + end while (0) + + typedef bit T[3:0]; + + T x; + byte y[7:0][2:0]; + + initial begin + integer i; + i = 4; + + `check(x, 4); + `check(y, $bits(byte) * 3 * 8); + `check(y[0], $bits(byte) * 3); + `check(y[1:0], $bits(byte) * 3 * 2); + `check(y[1+:3], $bits(byte) * 3 * 3); + `check(y[4-:4], $bits(byte) * 3 * 4); + `check(y[i-:2], $bits(byte) * 3 * 2); + `check(y[i+:2], $bits(byte) * 3 * 2); + + if (!failed) begin + $display("PASSED"); + end + end + +endmodule diff --git a/ivtest/regress-vvp.list b/ivtest/regress-vvp.list index 6fe3647ff..281262924 100644 --- a/ivtest/regress-vvp.list +++ b/ivtest/regress-vvp.list @@ -3,6 +3,7 @@ # describes the test. array_packed_write_read vvp_tests/array_packed_write_read.json +bits4 vvp_tests/bits4.json bitsel11 vvp_tests/bitsel11.json br_gh13a vvp_tests/br_gh13a.json br_gh13a-vlog95 vvp_tests/br_gh13a-vlog95.json diff --git a/ivtest/vvp_tests/bits4.json b/ivtest/vvp_tests/bits4.json new file mode 100644 index 000000000..24e2cfdb7 --- /dev/null +++ b/ivtest/vvp_tests/bits4.json @@ -0,0 +1,5 @@ +{ + "type" : "normal", + "source" : "bits4.v", + "iverilog-args" : [ "-g2005-sv" ] +}