diff --git a/elab_expr.cc b/elab_expr.cc index 9f1074221..38b401524 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -5843,11 +5843,6 @@ NetExpr* PEIdent::elaborate_expr_net_part_(Design*des, NetScope*scope, if (!flag) return 0; - /* The indices of part selects are signed integers, so allow - negative values. However, the width that they represent is - unsigned. Remember that any order is possible, - i.e., [1:0], [-4:6], etc. */ - unsigned long wid = 1 + labs(msv-lsv); /* But wait... if the part select expressions are not fully defined, then fall back on the tested width. */ if (!parts_defined_flag) { @@ -5878,11 +5873,10 @@ NetExpr* PEIdent::elaborate_expr_net_part_(Design*des, NetScope*scope, // Make this work by finding the indexed slices and // creating a generated slice that spans the whole // range. - long loff, moff; unsigned long lwid, mwid; bool lrc, mrc; - lrc = net->sig()->sb_to_slice(prefix_indices, lsv, loff, lwid); - mrc = net->sig()->sb_to_slice(prefix_indices, msv, moff, mwid); + lrc = net->sig()->sb_to_slice(prefix_indices, lsv, sb_lsb, lwid); + mrc = net->sig()->sb_to_slice(prefix_indices, msv, sb_msb, mwid); if (!mrc || !lrc) { cerr << get_fileline() << ": error: "; cerr << "Part-select [" << msv << ":" << lsv; @@ -5894,15 +5888,7 @@ NetExpr* PEIdent::elaborate_expr_net_part_(Design*des, NetScope*scope, return 0; } ivl_assert(*this, lwid == mwid); - - if (moff > loff) { - sb_lsb = loff; - sb_msb = moff + mwid - 1; - } else { - sb_lsb = moff; - sb_msb = loff + lwid - 1; - } - wid = sb_msb - sb_lsb + 1; + sb_msb += mwid - 1; } else { // This case, the prefix indices are enough to index // down to a single bit/slice. @@ -5951,6 +5937,8 @@ NetExpr* PEIdent::elaborate_expr_net_part_(Design*des, NetScope*scope, } } + unsigned long wid = sb_msb - sb_lsb + 1; + // If the part select covers exactly the entire // vector, then do not bother with it. Return the // signal itself, casting to unsigned if necessary. diff --git a/elab_lval.cc b/elab_lval.cc index 77b7829bb..6b2927602 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -752,15 +752,14 @@ bool PEIdent::elaborate_lval_net_part_(Design*des, const netranges_t&packed = reg->packed_dims(); long loff, moff; - long wid; if (prefix_indices.size()+1 < packed.size()) { // If there are fewer indices then there are packed // dimensions, then this is a range of slices. Calculate // it into a big slice. bool lrc, mrc; - unsigned long tmp_lwid, tmp_mwid; - lrc = reg->sb_to_slice(prefix_indices, lsb, loff, tmp_lwid); - mrc = reg->sb_to_slice(prefix_indices, msb, moff, tmp_mwid); + unsigned long lwid, mwid; + lrc = reg->sb_to_slice(prefix_indices, lsb, loff, lwid); + mrc = reg->sb_to_slice(prefix_indices, msb, moff, mwid); if (!mrc || !lrc) { cerr << get_fileline() << ": error: "; cerr << "Part-select [" << msb << ":" << lsb; @@ -771,33 +770,26 @@ bool PEIdent::elaborate_lval_net_part_(Design*des, des->errors += 1; return 0; } - - if (loff < moff) { - moff = moff + tmp_mwid - 1; - } else { - long ltmp = moff; - moff = loff + tmp_lwid - 1; - loff = ltmp; - } - wid = moff - loff + 1; - + assert(lwid == mwid); + moff += mwid - 1; } else { loff = reg->sb_to_idx(prefix_indices,lsb); moff = reg->sb_to_idx(prefix_indices,msb); - wid = moff - loff + 1; - - if (moff < loff) { - cerr << get_fileline() << ": error: part select " - << reg->name() << "[" << msb<<":"<errors += 1; - return false; - } } + if (moff < loff) { + cerr << get_fileline() << ": error: part select " + << reg->name() << "[" << msb<<":"<errors += 1; + return false; + } + + unsigned long wid = moff - loff + 1; + // Special case: The range winds up selecting the entire // vector. Treat this as no part select at all. - if (loff == 0 && moff == (long)(reg->vector_width()-1)) { + if (loff == 0 && wid == reg->vector_width()) { return true; } diff --git a/elab_net.cc b/elab_net.cc index 9635bf0e8..4d6d8dd86 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -380,11 +380,10 @@ bool PEIdent::eval_part_select_(Design*des, NetScope*scope, NetNet*sig, // Make this work by finding the indexed slices and // creating a generated slice that spans the whole // range. - long loff, moff; unsigned long lwid, mwid; bool lrc, mrc; - lrc = sig->sb_to_slice(prefix_indices, lsb, loff, lwid); - mrc = sig->sb_to_slice(prefix_indices, msb, moff, mwid); + lrc = sig->sb_to_slice(prefix_indices, lsb, lidx, lwid); + mrc = sig->sb_to_slice(prefix_indices, msb, midx, mwid); if (!mrc || !lrc) { cerr << get_fileline() << ": error: "; cerr << "Part-select [" << msb << ":" << lsb; @@ -396,49 +395,39 @@ bool PEIdent::eval_part_select_(Design*des, NetScope*scope, NetNet*sig, return 0; } ivl_assert(*this, lwid == mwid); - - if (moff > loff) { - lidx = loff; - midx = moff + mwid - 1; - } else { - lidx = moff; - midx = loff + lwid - 1; - } + midx += mwid - 1; } else { - long lidx_tmp = sig->sb_to_idx(prefix_indices, lsb); - long midx_tmp = sig->sb_to_idx(prefix_indices, msb); + lidx = sig->sb_to_idx(prefix_indices, lsb); + midx = sig->sb_to_idx(prefix_indices, msb); + } - /* Detect reversed indices of a part select. */ - if (lidx_tmp > midx_tmp) { - cerr << get_fileline() << ": error: Part select " - << sig->name() << "[" << msb << ":" - << lsb << "] indices reversed." << endl; - cerr << get_fileline() << ": : Did you mean " - << sig->name() << "[" << lsb << ":" - << msb << "]?" << endl; - long tmp = midx_tmp; - midx_tmp = lidx_tmp; - lidx_tmp = tmp; - des->errors += 1; - } + /* Detect reversed indices of a part select. */ + if (lidx > midx) { + cerr << get_fileline() << ": error: Part select " + << sig->name() << "[" << msb << ":" + << lsb << "] indices reversed." << endl; + cerr << get_fileline() << ": : Did you mean " + << sig->name() << "[" << lsb << ":" + << msb << "]?" << endl; + des->errors += 1; - /* Warn about a part select that is out of range. */ - if (midx_tmp >= (long)sig->vector_width() || lidx_tmp < 0) { - cerr << get_fileline() << ": warning: Part select " - << sig->name(); - if (sig->unpacked_dimensions() > 0) { - cerr << "[]"; - } - cerr << "[" << msb << ":" << lsb - << "] is out of range." << endl; + std::swap(lidx, midx); } - /* This is completely out side the signal so just skip it. */ - if (lidx_tmp >= (long)sig->vector_width() || midx_tmp < 0) { - return false; - } - midx = midx_tmp; - lidx = lidx_tmp; + /* Warn about a part select that is out of range. */ + if (midx >= (long)sig->vector_width() || lidx < 0) { + cerr << get_fileline() << ": warning: Part select " + << sig->name(); + if (sig->unpacked_dimensions() > 0) { + cerr << "[]"; + } + cerr << "[" << msb << ":" << lsb + << "] is out of range." << endl; + } + + /* This is completely out side the signal so just skip it. */ + if (lidx >= (long)sig->vector_width() || midx < 0) { + return false; } break; } diff --git a/ivtest/ivltests/partsel_reversed_idx1.v b/ivtest/ivltests/partsel_reversed_idx1.v new file mode 100644 index 000000000..16551be95 --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx1.v @@ -0,0 +1,14 @@ +// Check that an inverted part select in a continuous assign is reported as an +// error. + +module test; + + reg [1:0] x; + + assign x[0:1] = 2'b00; // Error: Part select indices swapped + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/partsel_reversed_idx2.v b/ivtest/ivltests/partsel_reversed_idx2.v new file mode 100644 index 000000000..459732af0 --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx2.v @@ -0,0 +1,14 @@ +// Check that an inverted part select on an inner dimension in a continuous +// assign is reported as an error. + +module test; + + reg [1:0][1:0] x; + + assign x[0:1] = 2'b00; // Error: Part select indices swapped + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/partsel_reversed_idx3.v b/ivtest/ivltests/partsel_reversed_idx3.v new file mode 100644 index 000000000..f8d0ea003 --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx3.v @@ -0,0 +1,14 @@ +// Check that an inverted part select in a procedural assign is reported as an +// error. + +module test; + + reg [1:0] x; + + initial begin + x[0:1] = 2'b00; // Error: Part select indices swapped + + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/partsel_reversed_idx4.v b/ivtest/ivltests/partsel_reversed_idx4.v new file mode 100644 index 000000000..c09eaba3f --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx4.v @@ -0,0 +1,14 @@ +// Check that an inverted part select on an inner dimension in a procedural +// assign is reported as an error. + +module test; + + reg [1:0][1:0] x; + + initial begin + x[0:1] = 2'b00; // Error: Part select indices swapped + + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/partsel_reversed_idx5.v b/ivtest/ivltests/partsel_reversed_idx5.v new file mode 100644 index 000000000..b3daa49c9 --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx5.v @@ -0,0 +1,14 @@ +// Check that an inverted part select in an expression is reported as an error. + +module test; + + reg [1:0] x; + reg [1:0] y; + + initial begin + y = x[0:1]; // Error: Part select indices swapped + + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/partsel_reversed_idx6.v b/ivtest/ivltests/partsel_reversed_idx6.v new file mode 100644 index 000000000..55f92a942 --- /dev/null +++ b/ivtest/ivltests/partsel_reversed_idx6.v @@ -0,0 +1,15 @@ +// Check that an inverted part select on an inner dimension in an expression is +// reported as an error. + +module test; + + reg [1:0][1:0] x; + reg [1:0] y; + + initial begin + y = x[0:1]; // Error: Part select indices swapped + + $display("FAILED"); + end + +endmodule diff --git a/ivtest/regress-vvp.list b/ivtest/regress-vvp.list index 0dccca4a7..82ab51268 100644 --- a/ivtest/regress-vvp.list +++ b/ivtest/regress-vvp.list @@ -95,6 +95,12 @@ partsel_invalid_idx3 vvp_tests/partsel_invalid_idx3.json partsel_invalid_idx4 vvp_tests/partsel_invalid_idx4.json partsel_invalid_idx5 vvp_tests/partsel_invalid_idx5.json partsel_invalid_idx6 vvp_tests/partsel_invalid_idx6.json +partsel_reversed_idx1 vvp_tests/partsel_reversed_idx1.json +partsel_reversed_idx2 vvp_tests/partsel_reversed_idx2.json +partsel_reversed_idx3 vvp_tests/partsel_reversed_idx3.json +partsel_reversed_idx4 vvp_tests/partsel_reversed_idx4.json +partsel_reversed_idx5 vvp_tests/partsel_reversed_idx5.json +partsel_reversed_idx6 vvp_tests/partsel_reversed_idx6.json param_test3 vvp_tests/param_test3.json param-width vvp_tests/param-width.json param-width-vlog95 vvp_tests/param-width-vlog95.json diff --git a/ivtest/vvp_tests/partsel_reversed_idx1.json b/ivtest/vvp_tests/partsel_reversed_idx1.json new file mode 100644 index 000000000..e251399c1 --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx1.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx1.v" +} diff --git a/ivtest/vvp_tests/partsel_reversed_idx2.json b/ivtest/vvp_tests/partsel_reversed_idx2.json new file mode 100644 index 000000000..5d45f578a --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx2.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx2.v" +} diff --git a/ivtest/vvp_tests/partsel_reversed_idx3.json b/ivtest/vvp_tests/partsel_reversed_idx3.json new file mode 100644 index 000000000..60522804a --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx3.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx3.v" +} diff --git a/ivtest/vvp_tests/partsel_reversed_idx4.json b/ivtest/vvp_tests/partsel_reversed_idx4.json new file mode 100644 index 000000000..949ce7bc2 --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx4.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx4.v" +} diff --git a/ivtest/vvp_tests/partsel_reversed_idx5.json b/ivtest/vvp_tests/partsel_reversed_idx5.json new file mode 100644 index 000000000..3c619c720 --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx5.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx5.v" +} diff --git a/ivtest/vvp_tests/partsel_reversed_idx6.json b/ivtest/vvp_tests/partsel_reversed_idx6.json new file mode 100644 index 000000000..8bfa2fff7 --- /dev/null +++ b/ivtest/vvp_tests/partsel_reversed_idx6.json @@ -0,0 +1,4 @@ +{ + "type" : "CE", + "source" : "partsel_reversed_idx6.v" +}