From 5ec72f4cc85e4173c3f4e72d5e08a948a840be1d Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 12 Sep 2022 22:13:25 +0200 Subject: [PATCH 1/2] Add error checking for continuous unpacked array assignments Currently there is no error checking for continuous assignments to unpacked arrays. If the lvalue and rvalue net are not compatible undefined behavior occurs. For some types of incompatibility it will crash during elaboration, for others it will crash during simulation, and for some it will just work, even though the assignment is not allowed by the standard. Implement checking to ensure the two nets are compatible as required by the standard and report an error otherwise. Two arrays are considered to be compatible if their element types are equivalent, they have the same number of ranges and each range has the same number of elements. Signed-off-by: Lars-Peter Clausen --- elab_net.cc | 14 ++++++- elaborate.cc | 107 +++++++++++++++++++++++++++++++++++++++------------ netparray.cc | 12 ++++++ netparray.h | 3 ++ nettypes.cc | 14 +++++++ nettypes.h | 6 +++ 6 files changed, 131 insertions(+), 25 deletions(-) diff --git a/elab_net.cc b/elab_net.cc index 733768c46..ae00cd093 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -1107,8 +1107,20 @@ NetNet*PEIdent::elaborate_unpacked_net(Design*des, NetScope*scope) const perm_string method_name; symbol_search(this, des, scope, path_, sig, par, eve); + if (!sig) { + cerr << get_fileline() << ": error: Net " << path_ + << " is not defined in this context." << endl; + des->errors += 1; + return nullptr; + } - ivl_assert(*this, sig); + const name_component_t&name_tail = path_.back(); + if (name_tail.index.size() != 0) { + cerr << get_fileline() << ": sorry: Array slices are not yet " + << "supported for continuous assignment." << endl; + des->errors += 1; + return nullptr; + } return sig; } diff --git a/elaborate.cc b/elaborate.cc index dfd1fb7d2..052d0d92b 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -260,16 +260,64 @@ void PGAssign::elaborate(Design*des, NetScope*scope) const } +NetNet *elaborate_unpacked_array(Design *des, NetScope *scope, const LineInfo &loc, + const NetNet *lval, PExpr *expr) +{ + PEIdent* ident = dynamic_cast (expr); + if (!ident) { + des->errors++; + if (dynamic_cast (expr)) { + cout << loc.get_fileline() << ": sorry: Continuous assignment" + << " of array concatenation is not yet supported." + << endl; + } else if (dynamic_cast (expr)) { + cout << loc.get_fileline() << ": sorry: Continuous assignment" + << " of assignment pattern is not yet supported." << endl; + } else { + cout << loc.get_fileline() << ": error: Can not assign" + << " non-array expression `" << *expr << "` to array." + << endl; + } + return nullptr; + } + + NetNet *expr_net = ident->elaborate_unpacked_net(des, scope); + if (!expr_net) + return nullptr; + + auto const &lval_dims = lval->unpacked_dims(); + auto const &expr_dims = expr_net->unpacked_dims(); + + if (expr_dims.empty()) { + cerr << loc.get_fileline() << ": error: Can not assign" + << " non-array identifier `" << *expr << "` to array." + << endl; + des->errors++; + return nullptr; + } + + if (!netrange_equivalent(lval_dims, expr_dims)) { + cerr << loc.get_fileline() << ": error: Unpacked dimensions" + << " are not compatible in array assignment." << endl; + des->errors++; + return nullptr; + } + + if (!lval->net_type()->type_equivalent(expr_net->net_type())) { + cerr << loc.get_fileline() << ": error: Element types are not" + << " compatible in array assignment." << endl; + des->errors++; + return nullptr; + } + + return expr_net; +} + void PGAssign::elaborate_unpacked_array_(Design*des, NetScope*scope, NetNet*lval) const { - PEIdent*rval_pident = dynamic_cast (pin(1)); - ivl_assert(*this, rval_pident); - - NetNet*rval_net = rval_pident->elaborate_unpacked_net(des, scope); - - ivl_assert(*this, rval_net->pin_count() == lval->pin_count()); - - assign_unpacked_with_bufz(des, scope, this, lval, rval_net); + NetNet *rval_net = elaborate_unpacked_array(des, scope, *this, lval, pin(1)); + if (rval_net) + assign_unpacked_with_bufz(des, scope, lval, lval, rval_net); } void PGBuiltin::calculate_gate_and_lval_count_(unsigned&gate_count, @@ -1147,6 +1195,29 @@ static void isolate_and_connect(Design*des, NetScope*scope, const PGModule*mod, } } +void elaborate_unpacked_port(Design *des, NetScope *scope, NetNet *port_net, + PExpr *expr, NetNet::PortType port_type, + Module *mod, unsigned int port_idx) +{ + NetNet *expr_net = elaborate_unpacked_array(des, scope, *expr, port_net, + expr); + if (!expr_net) { + perm_string port_name = mod->get_port_name(port_idx); + cerr << expr->get_fileline() << ": : Port " + << port_idx+1 << " (" << port_name << ") of " + << mod->mod_name() << " is connected to " + << *expr << endl; + + return; + } + + ivl_assert(*port_net, expr_net->pin_count() == port_net->pin_count()); + if (port_type == NetNet::POUTPUT) + assign_unpacked_with_bufz(des, scope, port_net, expr_net, port_net); + else + assign_unpacked_with_bufz(des, scope, port_net, port_net, expr_net); +} + /* * Instantiate a module by recursively elaborating it. Set the path of * the recursive elaboration so that signal names get properly @@ -1481,13 +1552,8 @@ void PGModule::elaborate_mod_(Design*des, Module*rmod, NetScope*scope) const // differently. if (prts.size() >= 1 && prts[0]->pin_count()>1) { ivl_assert(*this, prts.size()==1); - - PEIdent*rval_pident = dynamic_cast (pins[idx]); - ivl_assert(*this, rval_pident); - - NetNet*rval_net = rval_pident->elaborate_unpacked_net(des, scope); - ivl_assert(*this, rval_net->pin_count() == prts[0]->pin_count()); - assign_unpacked_with_bufz(des, scope, this, prts[0], rval_net); + elaborate_unpacked_port(des, scope, prts[0], pins[idx], + ptype, rmod, idx); continue; } @@ -1650,15 +1716,8 @@ void PGModule::elaborate_mod_(Design*des, Module*rmod, NetScope*scope) const // "r-value" expression, but since this is an // output port, we assign to it from the internal object. if (prts[0]->pin_count() > 1) { - ivl_assert(*this, prts.size()==1); - - PEIdent*rval_pident = dynamic_cast(pins[idx]); - ivl_assert(*this, rval_pident); - - NetNet*rval_net = rval_pident->elaborate_unpacked_net(des, scope); - ivl_assert(*this, rval_net->pin_count() == prts[0]->pin_count()); - - assign_unpacked_with_bufz(des, scope, this, rval_net, prts[0]); + elaborate_unpacked_port(des, scope, prts[0], pins[idx], + ptype, rmod, idx); continue; } diff --git a/netparray.cc b/netparray.cc index 3e6e81050..df972f7e3 100644 --- a/netparray.cc +++ b/netparray.cc @@ -86,3 +86,15 @@ vector netuarray_t::slice_dimensions() const { return static_dimensions(); } + +bool netuarray_t::test_equivalence(ivl_type_t that) const +{ + const netuarray_t *that_a = dynamic_cast(that); + if (!that_a) + return false; + + if (!netrange_equivalent(static_dimensions(), that_a->static_dimensions())) + return false; + + return element_type()->type_equivalent(that_a->element_type()); +} diff --git a/netparray.h b/netparray.h index 2e51bf80d..cb122e5b4 100644 --- a/netparray.h +++ b/netparray.h @@ -92,6 +92,9 @@ class netuarray_t : public netsarray_t { public: // Virtual methods from the ivl_type_s type... std::vector slice_dimensions() const; + + private: + bool test_equivalence(ivl_type_t that) const; }; inline netuarray_t::netuarray_t(const std::vector&pd, diff --git a/nettypes.cc b/nettypes.cc index ad2fe5860..5efb8ab63 100644 --- a/nettypes.cc +++ b/nettypes.cc @@ -109,6 +109,20 @@ unsigned long netrange_width(const vector&packed) return wid; } +bool netrange_equivalent(const std::vector &a, + const std::vector &b) +{ + if (a.size() != b.size()) + return false; + + for (size_t i = 0; i < a.size(); i++) { + if (!a[i].equivalent(b[i])) + return false; + } + + return true; +} + /* * Given a netrange_t list (which represent packed dimensions) and a * prefix of calculated index values, calculate the canonical offset diff --git a/nettypes.h b/nettypes.h index dc95eef46..0f0d57958 100644 --- a/nettypes.h +++ b/nettypes.h @@ -137,6 +137,10 @@ class netrange_t { return false; } + bool equivalent(const netrange_t &that) const { + return width() == that.width(); + } + private: long msb_; long lsb_; @@ -146,6 +150,8 @@ extern std::ostream&operator << (std::ostream&out, const std::list&r extern std::ostream&operator << (std::ostream&out, const std::vector&rlist); extern unsigned long netrange_width(const std::vector&dims); +extern bool netrange_equivalent(const std::vector &a, + const std::vector &b); /* * There are a few cases where we need to know about the single-level From 4ef5b02bcd647c48f54444ac25419c4b270845d4 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 15 Oct 2022 15:40:09 +0200 Subject: [PATCH 2/2] Add regression tests for continuous array assign compatibility Check various different scenarios for array compatibility in continuous array assign. Both testing cases that should work and cases that should fail. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_array_cassign1.v | 14 ++++++++++++++ ivtest/ivltests/sv_array_cassign2.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign3.v | 15 +++++++++++++++ ivtest/ivltests/sv_array_cassign4.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign5.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail1.v | 15 +++++++++++++++ ivtest/ivltests/sv_array_cassign_fail10.v | 18 ++++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail11.v | 18 ++++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail2.v | 15 +++++++++++++++ ivtest/ivltests/sv_array_cassign_fail3.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail4.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail5.v | 16 ++++++++++++++++ ivtest/ivltests/sv_array_cassign_fail6.v | 14 ++++++++++++++ ivtest/ivltests/sv_array_cassign_fail7.v | 15 +++++++++++++++ ivtest/ivltests/sv_array_cassign_fail8.v | 15 +++++++++++++++ ivtest/ivltests/sv_array_cassign_fail9.v | 15 +++++++++++++++ ivtest/regress-sv.list | 16 ++++++++++++++++ ivtest/regress-vlog95.list | 5 +++++ 18 files changed, 271 insertions(+) create mode 100644 ivtest/ivltests/sv_array_cassign1.v create mode 100644 ivtest/ivltests/sv_array_cassign2.v create mode 100644 ivtest/ivltests/sv_array_cassign3.v create mode 100644 ivtest/ivltests/sv_array_cassign4.v create mode 100644 ivtest/ivltests/sv_array_cassign5.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail1.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail10.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail11.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail2.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail3.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail4.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail5.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail6.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail7.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail8.v create mode 100644 ivtest/ivltests/sv_array_cassign_fail9.v diff --git a/ivtest/ivltests/sv_array_cassign1.v b/ivtest/ivltests/sv_array_cassign1.v new file mode 100644 index 000000000..66c5cec4d --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign1.v @@ -0,0 +1,14 @@ +// Check that continuous assignment of two compatible arrays is supported + +module test; + + wire [1:0] x[1:0]; + wire [1:0] y[1:0]; + + assign x = y; + + initial begin + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign2.v b/ivtest/ivltests/sv_array_cassign2.v new file mode 100644 index 000000000..cbf851e27 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign2.v @@ -0,0 +1,16 @@ +// Check that continuous assignment of two compatible arrays is supported, even +// if the upper and lower bounds of the arrays are not identical, as long as the +// size is the same. + +module test; + + wire [1:0] x[1:0]; + wire [1:0] y[2:1]; + + assign x = y; + + initial begin + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign3.v b/ivtest/ivltests/sv_array_cassign3.v new file mode 100644 index 000000000..f89025ab3 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign3.v @@ -0,0 +1,15 @@ +// Check that continuous assignment of two compatible arrays is supported, even +// if the element types are not identical, but just equivalent. + +module test; + + wire [1:0] x[1:0]; + wire [2:1] y[1:0]; + + assign x = y; + + initial begin + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign4.v b/ivtest/ivltests/sv_array_cassign4.v new file mode 100644 index 000000000..4caf6c31e --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign4.v @@ -0,0 +1,16 @@ +// Check that continuous assignment of two compatible arrays is supported, even +// if the element types are not identical and one is a built-in integer and the +// other a equivalent packed type. + +module test; + + wire signed [31:0] x[1:0]; + wire integer y[1:0]; + + assign x = y; + + initial begin + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign5.v b/ivtest/ivltests/sv_array_cassign5.v new file mode 100644 index 000000000..b4bf1a98b --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign5.v @@ -0,0 +1,16 @@ +// Check that continuous assignment of two compatible arrays is supported, even +// if the element types have different number of dimensions, but have the same +// packed width. + +module test; + + wire [3:0] x[1:0]; + wire [1:0][1:0] y[1:0]; + + assign x = y; + + initial begin + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail1.v b/ivtest/ivltests/sv_array_cassign_fail1.v new file mode 100644 index 000000000..c07dcc896 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail1.v @@ -0,0 +1,15 @@ +// Check that it is an error if the element type is not the same in a +// continuous array assignment. + +module test; + + wire [3:0] x[1:0]; + reg [1:0] y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail10.v b/ivtest/ivltests/sv_array_cassign_fail10.v new file mode 100644 index 000000000..0a72c11c2 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail10.v @@ -0,0 +1,18 @@ +// Check that it is an error trying to continuously assign an unpacked array +// with an enum element type to another unpacked array with an element type that +// is not the same enum type, even if the two element types are the same size. + +module test; + + wire integer x[1:0]; + enum integer { + A + } y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail11.v b/ivtest/ivltests/sv_array_cassign_fail11.v new file mode 100644 index 000000000..12e11384c --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail11.v @@ -0,0 +1,18 @@ +// Check that it is an error to continuously assign an unpacked array with an +// enum element type if the other unpacked array element type is not the same +// enum type, even if the two element types are the same size. + +module test; + + wire enum integer { + A + } x[1:0]; + integer y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail2.v b/ivtest/ivltests/sv_array_cassign_fail2.v new file mode 100644 index 000000000..971ec2bb6 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail2.v @@ -0,0 +1,15 @@ +// Check that it is an error if the array size is not the same in a continuous +// unpacked array assignment. + +module test; + + wire [1:0] x[2:0]; + reg [1:0] y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail3.v b/ivtest/ivltests/sv_array_cassign_fail3.v new file mode 100644 index 000000000..3478b84c8 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail3.v @@ -0,0 +1,16 @@ +// Check that it is an error if the number of unpacked dimensions do not match +// in an continuous array assignment, even if the canonical size of the array is +// the same. + +module test; + + wire [1:0] x[1:0][1:0]; + reg [1:0] y[3:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail4.v b/ivtest/ivltests/sv_array_cassign_fail4.v new file mode 100644 index 000000000..91ad48202 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail4.v @@ -0,0 +1,16 @@ +// Check that it is an error if the element type is not the same in a +// continuous array assignment, even if the difference is just 2-state vs. +// 4-state. + +module test; + + wire [1:0] x[1:0]; + bit [1:0] y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail5.v b/ivtest/ivltests/sv_array_cassign_fail5.v new file mode 100644 index 000000000..4801f9ca7 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail5.v @@ -0,0 +1,16 @@ +// Check that it is an error if the element type is not the same in a +// continuous array assignment, even if one of the types is a packed type and +// the other is a real type. + +module test; + + wire [1:0] x[1:0]; + real [1:0] y[1:0]; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail6.v b/ivtest/ivltests/sv_array_cassign_fail6.v new file mode 100644 index 000000000..fc5063be9 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail6.v @@ -0,0 +1,14 @@ +// Check that it is an error trying to continuously assign a scalar expression +// to a unpacked array. + +module test; + + wire [1:0] x[1:0]; + + assign x = 1'b1 + 1'b1; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail7.v b/ivtest/ivltests/sv_array_cassign_fail7.v new file mode 100644 index 000000000..a534e845e --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail7.v @@ -0,0 +1,15 @@ +// Check that it is an error trying to continuously assign a scalar variable to +// an unpacked array. + +module test; + + wire [1:0] x[1:0]; + reg [1:0] y; + + assign x = y; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail8.v b/ivtest/ivltests/sv_array_cassign_fail8.v new file mode 100644 index 000000000..762ba4074 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail8.v @@ -0,0 +1,15 @@ +// Check that it is an error trying to continuously assign a scalar net to an +// unpacked array. + +module test; + + wire a[1:0]; + wire x; + + assign a = x; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_array_cassign_fail9.v b/ivtest/ivltests/sv_array_cassign_fail9.v new file mode 100644 index 000000000..b685200b1 --- /dev/null +++ b/ivtest/ivltests/sv_array_cassign_fail9.v @@ -0,0 +1,15 @@ +// Check that it is an error trying to continuously assign an element of an +// unpacked array to another unpacked array as a whole. + +module test; + + wire a[1:0]; + wire x[1:0]; + + assign a = x[0]; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index d002e84a3..52f592c44 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -500,6 +500,22 @@ sv_assign_pattern_func normal,-g2005-sv ivltests sv_assign_pattern_op normal,-g2005-sv ivltests sv_assign_pattern_part normal,-g2005-sv ivltests sv_array_assign_pattern2 normal,-g2009 ivltests +sv_array_cassign1 normal,-g2005-sv ivltests +sv_array_cassign2 normal,-g2005-sv ivltests +sv_array_cassign3 normal,-g2005-sv ivltests +sv_array_cassign4 normal,-g2005-sv ivltests +sv_array_cassign5 normal,-g2005-sv ivltests +sv_array_cassign_fail1 CE,-g2005-sv ivltests +sv_array_cassign_fail2 CE,-g2005-sv ivltests +sv_array_cassign_fail3 CE,-g2005-sv ivltests +sv_array_cassign_fail4 CE,-g2005-sv ivltests +sv_array_cassign_fail5 CE,-g2005-sv ivltests +sv_array_cassign_fail6 CE,-g2005-sv ivltests +sv_array_cassign_fail7 CE,-g2005-sv ivltests +sv_array_cassign_fail8 CE,-g2005-sv ivltests +sv_array_cassign_fail9 CE,-g2005-sv ivltests +sv_array_cassign_fail10 CE,-g2005-sv ivltests +sv_array_cassign_fail11 CE,-g2005-sv ivltests sv_array_query normal,-g2005-sv ivltests sv_cast_integer normal,-g2005-sv ivltests sv_cast_integer2 normal,-g2005-sv ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index acd4b9f86..ed42175a3 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -257,6 +257,11 @@ scan-invalid CE ivltests sel_rval_bit_ob CE ivltests sel_rval_part_ob CE ivltests signed_net_display CE,-pallowsigned=1 ivltests +sv_array_cassign1 CE,-g2005-sv ivltests +sv_array_cassign2 CE,-g2005-sv ivltests +sv_array_cassign3 CE,-g2005-sv ivltests +sv_array_cassign4 CE,-g2005-sv ivltests +sv_array_cassign5 CE,-g2005-sv ivltests sv_unpacked_port CE,-g2009 ivltests sv_unpacked_port2 CE,-g2009,-pallowsigned=1 ivltests sv_unpacked_wire CE,-g2009 ivltests