From e00e89d8f8000e2ee5cd6ad8308af0c0f23f6b4c Mon Sep 17 00:00:00 2001 From: Andrew Pullin Date: Fri, 23 Jan 2026 14:54:06 -0800 Subject: [PATCH] Fix #1134: Allow struct member access in unpacked array of packed structs When accessing a member of a packed struct within an unpacked array (e.g., `tests[0].a` where `tests` is `test_t tests[0:1]`), the assertion `base_index.size()+1 == net->packed_dimensions()` would fail because unpacked indices were incorrectly included in base_index. The fix: 1. Separate unpacked indices from packed indices before calling check_for_struct_members() 2. Compute the word index for unpacked array access using normalize_variable_unpacked() 3. Pass the word index to check_for_struct_members(), which now creates NetESignal with the proper word selector This allows expressions like `array_of_structs[i].member` to work correctly, selecting the right array element before extracting the packed struct member. Co-Authored-By: Claude Opus 4.5 --- elab_expr.cc | 69 +++++++++++++++++++++++++++++++------ ivtest/ivltests/br_gh1134.v | 27 +++++++++++++++ ivtest/regress-sv.list | 1 + 3 files changed, 87 insertions(+), 10 deletions(-) create mode 100644 ivtest/ivltests/br_gh1134.v diff --git a/elab_expr.cc b/elab_expr.cc index 306f2e319..9538d2994 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -2389,12 +2389,17 @@ bool calculate_part(const LineInfo*li, Design*des, NetScope*scope, * the net is a struct. If that turns out to be the case, and the * struct is packed, then return a NetExpr that selects the member out * of the variable. + * + * The word_index parameter is used when the struct is in an unpacked + * array - it selects which array word to access before the packed + * member selection. */ static NetExpr* check_for_struct_members(const LineInfo*li, Design*des, NetScope*scope, NetNet*net, const list&base_index, - pform_name_t member_path) + pform_name_t member_path, + NetExpr*word_index = 0) { const netstruct_t*struct_type = net->struct_type(); ivl_assert(*li, struct_type); @@ -2707,7 +2712,7 @@ static NetExpr* check_for_struct_members(const LineInfo*li, packed_base = 0; } - NetESignal*sig = new NetESignal(net); + NetESignal*sig = new NetESignal(net, word_index); NetExpr *base = packed_base? packed_base : make_const_val(off); NetESelect*sel = new NetESelect(sig, base, use_width, member_type); @@ -2715,13 +2720,59 @@ static NetExpr* check_for_struct_members(const LineInfo*li, cerr << li->get_fileline() << ": check_for_struct_member: " << "Finally, completed_path=" << completed_path << ", off=" << off << ", use_width=" << use_width - << ", base=" << *base - << endl; + << ", base=" << *base; + if (word_index) + cerr << ", word_index=" << *word_index; + cerr << endl; } return sel; } +/* + * Helper function to elaborate struct member access in an unpacked array. + * This handles separating unpacked indices from packed indices, + * computing the word index, and calling check_for_struct_members. + */ +static NetExpr* elaborate_struct_member_access(const PExpr*expr, + Design*des, NetScope*scope, + NetNet*net, + const pform_name_t&path_head, + const pform_name_t&path_tail) +{ + // Separate unpacked indices from packed indices. + // check_for_struct_members expects only packed indices. + list all_index = path_head.back().index; + list unpacked_index; + for (unsigned idx = 0 ; idx < net->unpacked_dimensions() ; idx += 1) { + unpacked_index.push_back(all_index.front()); + all_index.pop_front(); + } + list& packed_index = all_index; + + // Compute word index for unpacked array access. + NetExpr*word_index = 0; + if (net->unpacked_dimensions() > 0) { + listunpacked_indices_expr; + list unpacked_indices_const; + indices_flags idx_flags; + indices_to_expressions(des, scope, expr, + unpacked_index, net->unpacked_dimensions(), + false, idx_flags, + unpacked_indices_expr, unpacked_indices_const); + + if (idx_flags.variable) { + word_index = normalize_variable_unpacked(net, unpacked_indices_expr); + } else if (!idx_flags.invalid && !idx_flags.undefined) { + word_index = normalize_variable_unpacked(net, unpacked_indices_const); + } + } + + return check_for_struct_members(expr, des, scope, net, + packed_index, + path_tail, word_index); +} + static NetExpr* class_static_property_expression(const LineInfo*li, const netclass_t*class_type, perm_string name) @@ -4555,9 +4606,8 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, if (!sr.path_tail.empty()) { if (net->struct_type()) { - return check_for_struct_members(this, des, scope, net, - sr.path_head.back().index, - sr.path_tail); + return elaborate_struct_member_access(this, des, scope, net, + sr.path_head, sr.path_tail); } else if (dynamic_cast(sr.type)) { return elaborate_expr_class_field_(des, scope, sr, 0, flags); } @@ -4782,9 +4832,8 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, << endl; } - return check_for_struct_members(this, des, scope, sr.net, - sr.path_head.back().index, - sr.path_tail); + return elaborate_struct_member_access(this, des, scope, sr.net, + sr.path_head, sr.path_tail); } // If this is an array object, and there are members in diff --git a/ivtest/ivltests/br_gh1134.v b/ivtest/ivltests/br_gh1134.v new file mode 100644 index 000000000..f5af54a79 --- /dev/null +++ b/ivtest/ivltests/br_gh1134.v @@ -0,0 +1,27 @@ +// Test for GitHub issue #1134 +// Accessing member of packed struct in unpacked array should work +typedef struct packed { + logic a, b; +} test_t; + +module test; + // tests[0] = {a:0, b:1}, tests[1] = {a:1, b:0} + test_t tests [0:1] = '{'{'b0, 'b1}, '{'b1, 'b0}}; + wire w0, w1; + + assign w0 = tests[0].a; // Should be 0 + assign w1 = tests[1].a; // Should be 1 + + initial begin + #1; + if (w0 !== 1'b0) begin + $display("FAILED: tests[0].a = %b, expected 0", w0); + $finish; + end + if (w1 !== 1'b1) begin + $display("FAILED: tests[1].a = %b, expected 1", w1); + $finish; + end + $display("PASSED"); + end +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 76846780a..830218e2d 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -992,5 +992,6 @@ ipsupsel_real_idx CE,-g2012 ivltests gold=ipsupsel_real_idx.gold real_edges CE,-g2012 ivltests gold=real_edges.gold br_gh1112 CE,-g2009 ivltests gold=br_gh1112.gold br_gh670 normal,-g2009 ivltests +br_gh1134 normal,-g2012 ivltests br_gh1267 normal,-g2012 ivltests br_gh1268 normal,-g2012 ivltests