diff --git a/README.md b/README.md index 3be1b4c2ef2..51e21657003 100644 --- a/README.md +++ b/README.md @@ -587,8 +587,8 @@ from SystemVerilog: - enums are supported (including inside packages) - but are currently not strongly typed -- packed structs and unions are supported - - arrays of packed structs/unions are currently not supported +- structs and unions are supported + - arrays of structs/unions are currently not supported - structure literals are currently not supported - multidimensional arrays are supported diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc index ead79fd9561..0fa013d064e 100644 --- a/frontends/ast/ast.cc +++ b/frontends/ast/ast.cc @@ -212,6 +212,7 @@ AstNode::AstNode(AstNodeType type, AstNode *child1, AstNode *child2, AstNode *ch is_reg = false; is_logic = false; is_signed = false; + is_unpacked = false; is_string = false; is_enum = false; is_wand = false; @@ -336,6 +337,8 @@ void AstNode::dumpAst(FILE *f, std::string indent) const fprintf(f, " reg"); if (is_signed) fprintf(f, " signed"); + if (is_unpacked) + fprintf(f, " unpacked"); if (is_unsized) fprintf(f, " unsized"); if (basic_prep) @@ -359,6 +362,8 @@ void AstNode::dumpAst(FILE *f, std::string indent) const std::swap(left, right); fprintf(f, "[%d:%d]", left, right); } + if (unpacked_dimensions) + fprintf(f, " unpacked_dimensions=%d", unpacked_dimensions); } if (is_enum) { fprintf(f, " type=enum"); diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index f05b568be22..fc14d6b43a8 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -195,7 +195,7 @@ namespace AST // node content - most of it is unused in most node types std::string str; std::vector bits; - bool is_input, is_output, is_reg, is_logic, is_signed, is_string, is_wand, is_wor, range_valid, range_swapped, was_checked, is_unsized, is_custom_type; + bool is_input, is_output, is_reg, is_logic, is_signed, is_unpacked, is_string, is_wand, is_wor, range_valid, range_swapped, was_checked, is_unsized, is_custom_type; int port_id, range_left, range_right; uint32_t integer; double realvalue; diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 96296aeb884..bb80112e604 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -272,11 +272,6 @@ static int add_dimension(AstNode *node, AstNode *rnode) return width; } -[[noreturn]] static void struct_array_packing_error(AstNode *node) -{ - node->input_error("Unpacked array in packed struct/union member %s\n", node->str.c_str()); -} - static int size_packed_struct(AstNode *snode, int base_offset) { // Struct members will be laid out in the structure contiguously from left to right. @@ -295,64 +290,20 @@ static int size_packed_struct(AstNode *snode, int base_offset) } else { log_assert(node->type == AST_STRUCT_ITEM); - if (node->children.size() > 0 && node->children[0]->type == AST_RANGE) { - // member width e.g. bit [7:0] a - width = range_width(node, node->children[0]); - if (node->children.size() == 2) { - // Unpacked array. Note that this is a Yosys extension; only packed data types - // and integer data types are allowed in packed structs / unions in SystemVerilog. - if (node->children[1]->type == AST_RANGE) { - // Unpacked array, e.g. bit [63:0] a [0:3] - // Pretend it's declared as a packed array, e.g. bit [0:3][63:0] a - auto rnode = node->children[1]; - if (rnode->children.size() == 1) { - // C-style array size, e.g. bit [63:0] a [4] - node->dimensions.push_back({ 0, rnode->range_left, true }); - width *= rnode->range_left; - } else { - width *= add_dimension(node, rnode); - } - add_dimension(node, node->children[0]); - } - else { - // The Yosys extension for unpacked arrays in packed structs / unions - // only supports memories, i.e. e.g. logic [7:0] a [256] - see above. - struct_array_packing_error(node); - } - } else { - // Vector - add_dimension(node, node->children[0]); - } - // range nodes are now redundant - for (AstNode *child : node->children) - delete child; - node->children.clear(); - } - else if (node->children.size() > 0 && node->children[0]->type == AST_MULTIRANGE) { - // Packed array, e.g. bit [3:0][63:0] a - if (node->children.size() != 1) { - // The Yosys extension for unpacked arrays in packed structs / unions - // only supports memories, i.e. e.g. logic [7:0] a [256] - see above. - struct_array_packing_error(node); - } - width = 1; - for (auto rnode : node->children[0]->children) { - width *= add_dimension(node, rnode); - } - // range nodes are now redundant - for (AstNode *child : node->children) - delete child; - node->children.clear(); - } - else if (node->range_left < 0) { - // 1 bit signal: bit, logic or reg - width = 1; - node->dimensions.push_back({ 0, width, false }); - } - else { - // already resolved and compacted - width = node->range_left - node->range_right + 1; + // Pretend it's just a wire in order to resolve the type. + node->type = AST_WIRE; + while (node->simplify(true, 1, -1, false)) { } + node->type = AST_STRUCT_ITEM; + width = 1; + for (int i = 0; i < GetSize(node->dimensions); i++) { + width *= node->dimensions[i].range_width; } + + // range nodes are now redundant + for (AstNode *child : node->children) + delete child; + node->children.clear(); + if (is_union) { node->range_right = base_offset; node->range_left = base_offset + width - 1; @@ -361,8 +312,10 @@ static int size_packed_struct(AstNode *snode, int base_offset) node->range_right = base_offset + offset; node->range_left = base_offset + offset + width - 1; } + node->range_valid = true; } + if (is_union) { // check that all members have the same size if (packed_width == -1) { @@ -1422,6 +1375,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin did_something = true; } } + // determine member offsets and widths size_packed_struct(this, 0); @@ -1443,7 +1397,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin // Pretend it's just a wire in order to resolve the type. type = AST_WIRE; - while (is_custom_type && simplify(const_fold, stage, width_hint, sign_hint)) {}; + while (is_custom_type && simplify(const_fold, stage, width_hint, sign_hint)) { } if (type == AST_WIRE) type = AST_STRUCT_ITEM; @@ -1856,7 +1810,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin // Resolve the typedef from the bottom up, recursing within the current // block of code. Defer further simplification until the complete type is // resolved. - while (template_node->is_custom_type && template_node->simplify(const_fold, stage, width_hint, sign_hint)) {}; + while (template_node->is_custom_type && template_node->simplify(const_fold, stage, width_hint, sign_hint)) { } if (!str.empty() && str[0] == '\\' && (template_node->type == AST_STRUCT || template_node->type == AST_UNION)) { // replace instance with wire representing the packed structure @@ -1883,10 +1837,12 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin // if an enum then add attributes to support simulator tracing newNode->annotateTypedEnums(template_node); - bool add_packed_dimensions = (type == AST_WIRE && GetSize(children) > 1) || (type == AST_MEMORY && GetSize(children) > 2); + bool unpacked_base_type = newNode->is_unpacked || newNode->type == AST_MEMORY; + bool add_unpacked_dimensions = is_unpacked || type == AST_MEMORY; + bool add_packed_dimensions = (!add_unpacked_dimensions && GetSize(children) > 1) || (add_unpacked_dimensions && GetSize(children) > 2); // Cannot add packed dimensions if unpacked dimensions are already specified. - if (add_packed_dimensions && newNode->type == AST_MEMORY) + if (unpacked_base_type && add_packed_dimensions) input_error("Cannot extend unpacked type `%s' with packed dimensions\n", type_name.c_str()); // Add packed dimensions. @@ -1899,7 +1855,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin } // Add unpacked dimensions. - if (type == AST_MEMORY) { + if (add_unpacked_dimensions) { AstNode *unpacked = children.back(); if (GetSize(newNode->children) < 2) newNode->children.push_back(unpacked->clone()); @@ -1927,7 +1883,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin type = AST_WIRE; AstNode *expr = children[0]; children.erase(children.begin()); - while (is_custom_type && simplify(const_fold, stage, width_hint, sign_hint)) {}; + while (is_custom_type && simplify(const_fold, stage, width_hint, sign_hint)) { } type = param_type; children.insert(children.begin(), expr); @@ -2040,22 +1996,23 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin if (!children.empty()) { // Unpacked ranges first, then packed ranges. for (int i = std::min(GetSize(children), 2) - 1; i >= 0; i--) { + int unpacked = i || children[i]->is_unpacked; if (children[i]->type == AST_MULTIRANGE) { int width = 1; for (auto range : children[i]->children) { width *= add_dimension(this, range); - if (i) unpacked_dimensions++; + if (unpacked) unpacked_dimensions++; } delete children[i]; int left = width - 1, right = 0; - if (i) + if (unpacked) std::swap(left, right); children[i] = new AstNode(AST_RANGE, mkconst_int(left, true), mkconst_int(right, true)); fixup_hierarchy_flags(); did_something = true; } else if (children[i]->type == AST_RANGE) { add_dimension(this, children[i]); - if (i) unpacked_dimensions++; + if (unpacked) unpacked_dimensions++; } } } else { @@ -2072,6 +2029,13 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin // Save original number of dimensions for $size() etc. integer = dims_sel; + int tmp_unpacked_dimensions = id2ast->unpacked_dimensions; + + if (id2ast->type != AST_MEMORY) { + // Merge unpacked ranges with packed ranges. + id2ast->unpacked_dimensions = 0; + } + // Split access into unpacked and packed parts. AstNode *unpacked_range = nullptr; AstNode *packed_range = nullptr; @@ -2100,6 +2064,8 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin } } + id2ast->unpacked_dimensions = tmp_unpacked_dimensions; + for (auto &it : children) delete it; children.clear(); @@ -2177,8 +2143,11 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin if (found_sname) { // structure member, rewrite this node to reference the packed struct wire - auto range = make_index_range(item_node); - newNode = new AstNode(AST_IDENTIFIER, range); + // Merge unpacked ranges with packed ranges. + int tmp_unpacked_dimensions = item_node->unpacked_dimensions; + item_node->unpacked_dimensions = 0; + newNode = new AstNode(AST_IDENTIFIER, make_index_range(item_node)); + item_node->unpacked_dimensions = tmp_unpacked_dimensions; newNode->str = sname; // save type and original number of dimensions for $size() etc. newNode->set_attribute(ID::wiretype, item_node->clone()); @@ -2196,6 +2165,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin } } } + // annotate identifiers using scope resolution and create auto-wires as needed if (type == AST_IDENTIFIER) { if (current_scope.count(str) == 0) { @@ -2802,7 +2772,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin if (!children[0]->id2ast->range_valid) goto skip_dynamic_range_lvalue_expansion; - AST::AstNode *member_node = children[0]->get_struct_member(); + AstNode *member_node = children[0]->get_struct_member(); int wire_width = member_node ? member_node->range_left - member_node->range_right + 1 : children[0]->id2ast->range_left - children[0]->id2ast->range_right + 1; diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 15a04eb2885..b0ff2eda84f 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -188,6 +188,9 @@ static void addRange(AstNode *parent, int msb = 31, int lsb = 0, bool isSigned = static AstNode *checkRange(AstNode *type_node, AstNode *range_node) { + if (range_node && type_node->is_unpacked) + frontend_verilog_yyerror("Cannot extend unpacked type `%s' with packed dimensions\n", type_node->str.c_str()); + if (type_node->range_left >= 0 && type_node->range_right >= 0) { // type already restricts the range if (range_node) { @@ -214,26 +217,35 @@ static AstNode *checkRange(AstNode *type_node, AstNode *range_node) return range_node; } -static void rewriteRange(AstNode *rangeNode) +static void rewriteUnpackedRange(AstNode *rangeNode) { if (rangeNode->type == AST_RANGE && rangeNode->children.size() == 1) { // SV array size [n], rewrite as [0:n-1] rangeNode->children.push_back(new AstNode(AST_SUB, rangeNode->children[0], AstNode::mkconst_int(1, true))); rangeNode->children[0] = AstNode::mkconst_int(0, false); + rangeNode->is_unpacked = true; } } -static void rewriteAsMemoryNode(AstNode *node, AstNode *rangeNode) +static void rewriteUnpackedRanges(AstNode *node, AstNode *rangeNode) { - node->type = AST_MEMORY; + node->is_unpacked = true; if (rangeNode->type == AST_MULTIRANGE) { - for (auto *itr : rangeNode->children) - rewriteRange(itr); + rangeNode->is_unpacked = true; + for (auto *itr : rangeNode->children) { + rewriteUnpackedRange(itr); + } } else - rewriteRange(rangeNode); + rewriteUnpackedRange(rangeNode); node->children.push_back(rangeNode); } +static void rewriteAsMemoryNode(AstNode *node, AstNode *rangeNode) +{ + node->type = AST_MEMORY; + rewriteUnpackedRanges(node, rangeNode); +} + static void checkLabelsMatch(const char *element, const std::string *before, const std::string *after) { if (!before && after) @@ -422,7 +434,7 @@ static const AstNode *addAsgnBinopStmt(dict *attr, AstNode * %type wire_type expr basic_expr concat_list rvalue lvalue lvalue_concat_list non_io_wire_type io_wire_type %type opt_label opt_sva_label tok_prim_wrapper hierarchical_id hierarchical_type_id integral_number %type type_name -%type opt_enum_init enum_type struct_type enum_struct_type func_return_type typedef_base_type +%type opt_enum_init enum_type struct_type func_return_type typedef_base_type %type opt_property always_comb_or_latch always_or_always_ff %type opt_signedness_default_signed opt_signedness_default_unsigned %type integer_atom_type integer_vector_type @@ -1847,8 +1859,12 @@ enum_decl: enum_type enum_var_list ';' { delete $1; } ////////////////// struct_decl: - attr struct_type { - append_attr($2, $1); + attr struct_type range_or_multirange { + AstNode *node = $2; + append_attr(node, $1); + AstNode *range = checkRange(node, $3); + if (range) + node->children.push_back(range); } struct_var_list ';' { delete astbuf2; } @@ -1866,8 +1882,8 @@ struct_body: opt_packed '{' struct_member_list '}' ; opt_packed: - TOK_PACKED opt_signed_struct | - %empty { frontend_verilog_yyerror("Only PACKED supported at this time"); }; + TOK_PACKED { astbuf2->is_unpacked = false; } opt_signed_struct + | %empty { astbuf2->is_unpacked = true; } opt_signed_struct: TOK_SIGNED { astbuf2->is_signed = true; } @@ -1887,13 +1903,18 @@ member_name_list: | member_name_list ',' member_name ; -member_name: TOK_ID { +member_name: TOK_ID range_or_multirange { astbuf1->str = $1->substr(1); delete $1; astbuf3 = astbuf1->clone(); + if ($2) { + if (!astbuf2->is_unpacked) + frontend_verilog_yyerror("A packed struct/union cannot contain unpacked members"); + rewriteUnpackedRanges(astbuf3, $2); + } SET_AST_NODE_LOC(astbuf3, @1, @1); astbuf2->children.push_back(astbuf3); - } range { if ($3) astbuf3->children.push_back($3); } + } ; struct_member_type: { astbuf1 = new AstNode(AST_STRUCT_ITEM); } member_type_token @@ -1905,6 +1926,11 @@ member_type_token: if (range) astbuf1->children.push_back(range); } + ; + +member_type: type_atom type_signing + | type_vec type_signing + | hierarchical_type_id { addWiretypeNode($1, astbuf1); } | { delete astbuf1; } struct_union { @@ -1919,21 +1945,19 @@ member_type_token: } ; -member_type: type_atom type_signing - | type_vec type_signing - | hierarchical_type_id { addWiretypeNode($1, astbuf1); } - ; - struct_var_list: struct_var | struct_var_list ',' struct_var ; -struct_var: TOK_ID { auto *var_node = astbuf2->clone(); - var_node->str = *$1; - delete $1; - SET_AST_NODE_LOC(var_node, @1, @1); - ast_stack.back()->children.push_back(var_node); - } +struct_var: TOK_ID range_or_multirange { + auto var_node = astbuf2->clone(); + var_node->str = *$1; + delete $1; + if ($2) + rewriteUnpackedRanges(var_node, $2); + SET_AST_NODE_LOC(var_node, @1, @1); + ast_stack.back()->children.push_back(var_node); + } ; ///////// @@ -2130,10 +2154,11 @@ typedef_decl: rewriteAsMemoryNode(astbuf1, $5); } addTypedefNode($4, astbuf1); } - | TOK_TYPEDEF enum_struct_type type_name ';' { addTypedefNode($3, $2); } + | TOK_TYPEDEF enum_type type_name ';' { addTypedefNode($3, $2); } ; typedef_base_type: + struct_type | hierarchical_type_id { $$ = new AstNode(AST_WIRE); $$->is_logic = true; @@ -2156,11 +2181,6 @@ typedef_base_type: $$->range_right = 0; }; -enum_struct_type: - enum_type - | struct_type - ; - cell_stmt: attr TOK_ID { astbuf1 = new AstNode(AST_CELL); diff --git a/tests/svtypes/struct_array.sv b/tests/svtypes/struct_array.sv index e07c9b2321e..a342c9b9184 100644 --- a/tests/svtypes/struct_array.sv +++ b/tests/svtypes/struct_array.sv @@ -211,11 +211,12 @@ module top; always_comb assert(s3_off==80'hFC00_4200_0012_3400_FFFC); `ifndef VERIFIC - // Note that the tests below for unpacked arrays in structs rely on the - // fact that they are actually packed in Yosys. + // Note that the tests below for unpacked arrays in unpacked structs + // rely on the fact that Yosys always stores structs in packed format. + // The LRM doesn't allow treating unpacked structs as single vectors. - // Same as s2, but using unpacked array syntax - struct packed { + // Same as s2, but using unpacked struct + struct { bit [7:0] a [7:0]; // 8 element unpacked array of bytes bit [15:0] b; // filler for non-zero offset } s4; @@ -235,8 +236,8 @@ module top; always_comb assert(s4==80'hFC00_4200_0012_3400_FFFC); - // Same as s3, but using unpacked array syntax - struct packed { + // Same as s3, but using unpacked struct + struct { bit [7:0] a [0:7]; // 8 element unpacked array of bytes bit [0:15] b; // filler for non-zero offset } s5; @@ -257,7 +258,7 @@ module top; always_comb assert(s5==80'hFC00_4200_0012_3400_FFFC); // Same as s5, but with little endian bit addressing - struct packed { + struct { bit [0:7] a [0:7]; // 8 element unpacked array of bytes bit [0:15] b; // filler for non-zero offset } s5_b; @@ -278,7 +279,7 @@ module top; always_comb assert(s5_b==80'hFC00_4200_0012_3400_FFFC); // Same as s5, but using C-type unpacked array syntax - struct packed { + struct { bit [7:0] a [8]; // 8 element unpacked array of bytes bit [0:15] b; // filler for non-zero offset } s6;