From cff53d6d873493c2560a8ed1997a01f71c3571e6 Mon Sep 17 00:00:00 2001 From: Dag Lem Date: Sun, 29 Jan 2023 20:22:00 +0100 Subject: [PATCH] Corrected handling of nested typedefs of struct/union This also corrects shadowing of constants in struct/union types. --- frontends/ast/simplify.cc | 58 ++++++++++++++++++++++++++++-- frontends/verilog/verilog_parser.y | 51 ++------------------------ tests/svtypes/typedef_scopes.sv | 20 +++++++++++ tests/svtypes/typedef_struct.sv | 4 ++- 4 files changed, 81 insertions(+), 52 deletions(-) diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 1efd1b24d..28f316d26 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -1380,6 +1380,60 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, break; case AST_STRUCT_ITEM: + if (is_custom_type) { + log_assert(children.size() == 1); + log_assert(children[0]->type == AST_WIRETYPE); + auto type_name = children[0]->str; + if (!current_scope.count(type_name)) { + log_file_error(filename, location.first_line, "Unknown identifier `%s' used as type name\n", type_name.c_str()); + } + AstNode *resolved_type_node = current_scope.at(type_name); + if (resolved_type_node->type != AST_TYPEDEF) + log_file_error(filename, location.first_line, "`%s' does not name a type\n", type_name.c_str()); + log_assert(resolved_type_node->children.size() == 1); + AstNode *template_node = resolved_type_node->children[0]; + + // Ensure typedef itself is fully simplified + while (template_node->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {}; + + // Remove type reference + delete children[0]; + children.pop_back(); + + switch (template_node->type) { + case AST_WIRE: + type = AST_STRUCT_ITEM; + break; + case AST_STRUCT: + case AST_UNION: + type = template_node->type; + break; + default: + log_file_error(filename, location.first_line, "Invalid type for struct member: %s", type2str(template_node->type).c_str()); + } + + is_reg = template_node->is_reg; + is_logic = template_node->is_logic; + is_signed = template_node->is_signed; + is_string = template_node->is_string; + is_custom_type = template_node->is_custom_type; + + range_valid = template_node->range_valid; + range_swapped = template_node->range_swapped; + range_left = template_node->range_left; + range_right = template_node->range_right; + + attributes[ID::wiretype] = mkconst_str(resolved_type_node->str); + + // Copy clones of children from template + for (auto template_child : template_node->children) { + children.push_back(template_child->clone()); + } + + did_something = true; + + } + log_assert(!is_custom_type); break; case AST_ENUM: @@ -1793,8 +1847,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, // Ensure typedef itself is fully simplified while (template_node->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {}; - if (template_node->type == AST_STRUCT || template_node->type == AST_UNION) { - // replace with wire representing the packed structure + if (!str.empty() && str[0] == '\\' && (template_node->type == AST_STRUCT || template_node->type == AST_UNION)) { + // replace instance with wire representing the packed structure newNode = make_packed_struct(template_node, str, attributes); newNode->attributes[ID::wiretype] = mkconst_str(resolved_type_node->str); // add original input/output attribute to resolved wire diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 4c5bd0b10..98bdbf9e5 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -171,36 +171,6 @@ static bool isInLocalScope(const std::string *name) return (user_types.count(*name) > 0); } -static AstNode *getTypeDefinitionNode(std::string type_name) -{ - // check package types - if (type_name.find("::") != std::string::npos && pkg_user_types.count(type_name) > 0) { - auto typedef_node = pkg_user_types[type_name]; - log_assert(typedef_node->type == AST_TYPEDEF); - return typedef_node->children[0]; - } - - // check current scope then outer scopes for a name - for (auto it = user_type_stack.rbegin(); it != user_type_stack.rend(); ++it) { - if (it->count(type_name) > 0) { - // return the definition nodes from the typedef statement - auto typedef_node = (*it)[type_name]; - log_assert(typedef_node->type == AST_TYPEDEF); - return typedef_node->children[0]; - } - } - - // The lexer recognized the name as a TOK_USER_TYPE, but now we can't find it anymore? - log_error("typedef for user type `%s' not found", type_name.c_str()); -} - -static AstNode *copyTypeDefinition(std::string type_name) -{ - // return a copy of the template from a typedef definition - auto typedef_node = getTypeDefinitionNode(type_name); - return typedef_node->clone(); -} - static AstNode *makeRange(int msb = 31, int lsb = 0, bool isSigned = true) { auto range = new AstNode(AST_RANGE); @@ -1633,10 +1603,7 @@ param_implicit_type: param_signed param_range; param_type: param_integer_type | param_real | param_range_type | param_implicit_type | hierarchical_type_id { - astbuf1->is_custom_type = true; - astbuf1->children.push_back(new AstNode(AST_WIRETYPE)); - astbuf1->children.back()->str = *$1; - delete $1; + addWiretypeNode($1, astbuf1); }; param_decl: @@ -1865,21 +1832,7 @@ struct_member_type: { astbuf1 = new AstNode(AST_STRUCT_ITEM); } member_type_toke member_type_token: member_type | hierarchical_type_id { - // use a clone of the typedef definition nodes - auto template_node = copyTypeDefinition(*$1); - delete $1; - switch (template_node->type) { - case AST_WIRE: - template_node->type = AST_STRUCT_ITEM; - break; - case AST_STRUCT: - case AST_UNION: - break; - default: - frontend_verilog_yyerror("Invalid type for struct member: %s", type2str(template_node->type).c_str()); - } - delete astbuf1; - astbuf1 = template_node; + addWiretypeNode($1, astbuf1); } | { delete astbuf1; diff --git a/tests/svtypes/typedef_scopes.sv b/tests/svtypes/typedef_scopes.sv index 9a898fac8..5ac9a4664 100644 --- a/tests/svtypes/typedef_scopes.sv +++ b/tests/svtypes/typedef_scopes.sv @@ -17,6 +17,12 @@ module top; always @(*) assert(inner_i1 == 4'hA); always @(*) assert(inner_enum1 == 3'h3); + // adapted from tests/verilog/typedef_const_shadow.sv + localparam W = 5; + typedef logic [W-1:0] T; + T x; // width 5 + always @(*) assert($bits(x) == 5); + if (1) begin: genblock // type declarations in child scopes shadow their parents typedef logic [7:0] inner_type; @@ -34,6 +40,20 @@ module top; } mystruct_t; mystruct_t mystruct; always @(*) assert($bits(mystruct) == 4); + + // adapted from tests/verilog/typedef_const_shadow.sv + localparam W = 10; + typedef T U; + typedef logic [W-1:0] V; + struct packed { + logic [W-1:0] x; // width 10 + U y; // width 5 + V z; // width 10 + } shadow; + // This currently only works as long as long as shadow is not typedef'ed + always @(*) assert($bits(shadow.x) == 10); + always @(*) assert($bits(shadow.y) == 5); + always @(*) assert($bits(shadow.z) == 10); end inner_type inner_i2 = 8'h42; diff --git a/tests/svtypes/typedef_struct.sv b/tests/svtypes/typedef_struct.sv index 8df8e32b0..136bb5c1b 100644 --- a/tests/svtypes/typedef_struct.sv +++ b/tests/svtypes/typedef_struct.sv @@ -19,8 +19,10 @@ module top; p::p_t ps; } s_t; + typedef s_t s1_t; + s_t s; - s_t s1; + s1_t s1; p::p_t ps;