From 68c67c40ec75b192f4f1be9711afe0df8973e797 Mon Sep 17 00:00:00 2001 From: Kamil Rakoczy Date: Mon, 14 Feb 2022 14:34:20 +0100 Subject: [PATCH] Fix access to whole sub-structs (#3086) * Add support for accessing whole struct * Update tests Signed-off-by: Kamil Rakoczy --- CHANGELOG | 3 +++ frontends/ast/genrtlil.cc | 2 +- frontends/ast/simplify.cc | 22 +++++++++++++---- tests/various/param_struct.ys | 3 +-- tests/various/struct_access.sv | 43 ++++++++++++++++++++++++++++++++++ tests/various/struct_access.ys | 5 ++++ tests/verilog/struct_access.sv | 5 ++-- 7 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 tests/various/struct_access.sv create mode 100644 tests/various/struct_access.ys diff --git a/CHANGELOG b/CHANGELOG index 46ce01699..187aeb635 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,9 @@ Yosys 0.14 .. Yosys 0.14-dev - Fixed elaboration of dynamic range assignments where the vector is reversed or is not zero-indexed + * SystemVerilog + - Added support for accessing whole sub-structures in expressions + Yosys 0.13 .. Yosys 0.14 -------------------------- diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc index 4c25287ad..020b4e5e8 100644 --- a/frontends/ast/genrtlil.cc +++ b/frontends/ast/genrtlil.cc @@ -877,7 +877,7 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint, bool *foun this_width = id_ast->children[0]->range_left - id_ast->children[0]->range_right + 1; if (children.size() > 1) range = children[1]; - } else if (id_ast->type == AST_STRUCT_ITEM) { + } else if (id_ast->type == AST_STRUCT_ITEM || id_ast->type == AST_STRUCT) { AstNode *tmp_range = make_struct_member_range(this, id_ast); this_width = tmp_range->range_left - tmp_range->range_right + 1; delete tmp_range; diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 000877612..565025d3a 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -307,6 +307,10 @@ static int size_packed_struct(AstNode *snode, int base_offset) if (node->type == AST_STRUCT || node->type == AST_UNION) { // embedded struct or union width = size_packed_struct(node, base_offset + offset); + // set range of struct + node->range_right = base_offset + offset; + node->range_left = base_offset + offset + width - 1; + node->range_valid = true; } else { log_assert(node->type == AST_STRUCT_ITEM); @@ -493,14 +497,12 @@ static void add_members_to_scope(AstNode *snode, std::string name) // in case later referenced in assignments log_assert(snode->type==AST_STRUCT || snode->type==AST_UNION); for (auto *node : snode->children) { + auto member_name = name + "." + node->str; + current_scope[member_name] = node; if (node->type != AST_STRUCT_ITEM) { // embedded struct or union add_members_to_scope(node, name + "." + node->str); } - else { - auto member_name = name + "." + node->str; - current_scope[member_name] = node; - } } } @@ -1341,6 +1343,16 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, case AST_PARAMETER: case AST_LOCALPARAM: + // if parameter is implicit type which is the typename of a struct or union, + // save information about struct in wiretype attribute + if (children[0]->type == AST_IDENTIFIER && current_scope.count(children[0]->str) > 0) { + auto item_node = current_scope[children[0]->str]; + if (item_node->type == AST_STRUCT || item_node->type == AST_UNION) { + attributes[ID::wiretype] = item_node->clone(); + size_packed_struct(attributes[ID::wiretype], 0); + add_members_to_scope(attributes[ID::wiretype], str); + } + } while (!children[0]->basic_prep && children[0]->simplify(false, false, false, stage, -1, false, true) == true) did_something = true; children[0]->detectSignWidth(width_hint, sign_hint); @@ -2018,7 +2030,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, if (name_has_dot(str, sname)) { if (current_scope.count(str) > 0) { auto item_node = current_scope[str]; - if (item_node->type == AST_STRUCT_ITEM) { + if (item_node->type == AST_STRUCT_ITEM || item_node->type == AST_STRUCT) { // structure member, rewrite this node to reference the packed struct wire auto range = make_struct_member_range(this, item_node); newNode = new AstNode(AST_IDENTIFIER, range); diff --git a/tests/various/param_struct.ys b/tests/various/param_struct.ys index 6d7a7c6ad..b8de67968 100644 --- a/tests/various/param_struct.ys +++ b/tests/various/param_struct.ys @@ -41,8 +41,7 @@ always_comb begin assert(j == 1'b1); assert(k == 1'b0); assert(l == 3'b111); -// TODO: support access to whole sub-structs and unions -// assert(m == 2'b10); + assert(m == 2'b10); assert(u == 5'b11001); end endmodule diff --git a/tests/various/struct_access.sv b/tests/various/struct_access.sv new file mode 100644 index 000000000..d41a7114f --- /dev/null +++ b/tests/various/struct_access.sv @@ -0,0 +1,43 @@ +module dut(); +typedef struct packed { + logic a; + logic b; +} sub_sub_struct_t; + +typedef struct packed { + sub_sub_struct_t c; +} sub_struct_t; + +typedef struct packed { + sub_struct_t d; + sub_struct_t e; +} struct_t; + +parameter struct_t P = 4'b1100; + +localparam sub_struct_t f = P.d; +localparam sub_struct_t g = P.e; +localparam sub_sub_struct_t h = f.c; +localparam logic i = P.d.c.a; +localparam logic j = P.d.c.b; +localparam x = P.e; +localparam y = x.c; +localparam z = y.a; +localparam q = P.d; +localparam n = q.c.a; + +always_comb begin + assert(P == 4'b1100); + assert(f == 2'b11); + assert(g == 2'b00); + assert(h == 2'b11); + assert(i == 1'b1); + assert(j == 1'b1); + assert(x == 2'b00); + assert(y == 2'b00); + assert(x.c == 2'b00); + assert(y.b == 1'b0); + assert(n == 1'b1); + assert(z == 1'b0); +end +endmodule diff --git a/tests/various/struct_access.ys b/tests/various/struct_access.ys new file mode 100644 index 000000000..2282edd92 --- /dev/null +++ b/tests/various/struct_access.ys @@ -0,0 +1,5 @@ +read_verilog -sv struct_access.sv +hierarchy +proc +opt +sat -verify -seq 1 -prove-asserts -show-all diff --git a/tests/verilog/struct_access.sv b/tests/verilog/struct_access.sv index f13b8dd51..bc91e3f01 100644 --- a/tests/verilog/struct_access.sv +++ b/tests/verilog/struct_access.sv @@ -77,9 +77,8 @@ module top; `CHECK(s.y.a, 1, 0) `CHECK(s.y.b, 1, 1) - // TODO(zachjs): support access to whole sub-structs and unions - // `CHECK(s.x, 2, 0) - // `CHECK(s.y, 2, 1) + `CHECK(s.x, 2, 0) + `CHECK(s.y, 2, 1) assert (fail === 0); end -- 2.30.2