From a9c8ca21d583c58a38931389f90bbaae0caec0d6 Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Tue, 22 Jun 2021 10:39:57 -0400 Subject: [PATCH] sv: fix two struct access bugs - preserve signedness of struct members - fix initial width detection of struct members (e.g., in case expressions) --- frontends/ast/ast.h | 3 ++ frontends/ast/genrtlil.cc | 4 ++ frontends/ast/simplify.cc | 4 +- tests/verilog/struct_access.sv | 88 ++++++++++++++++++++++++++++++++++ tests/verilog/struct_access.ys | 4 ++ 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/verilog/struct_access.sv create mode 100644 tests/verilog/struct_access.ys diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index 2bda8fa92..60c7de32d 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -370,6 +370,9 @@ namespace AST // Helper for setting the src attribute. void set_src_attr(RTLIL::AttrObject *obj, const AstNode *ast); + + // struct helper exposed from simplify for genrtlil + AstNode *make_struct_member_range(AstNode *node, AstNode *member_node); } namespace AST_INTERNAL diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc index e6f7b30c1..29d0bddef 100644 --- a/frontends/ast/genrtlil.cc +++ b/frontends/ast/genrtlil.cc @@ -812,6 +812,10 @@ 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) { + AstNode *tmp_range = make_struct_member_range(this, id_ast); + this_width = tmp_range->range_left - tmp_range->range_right + 1; + delete tmp_range; } else log_file_error(filename, location.first_line, "Failed to detect width for identifier %s!\n", str.c_str()); if (range) { diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 695fc429d..5845c0501 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -456,7 +456,7 @@ static AstNode *slice_range(AstNode *rnode, AstNode *snode) } -static AstNode *make_struct_member_range(AstNode *node, AstNode *member_node) +AstNode *AST::make_struct_member_range(AstNode *node, AstNode *member_node) { // Work out the range in the packed array that corresponds to a struct member // taking into account any range operations applicable to the current node @@ -1693,6 +1693,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, newNode = new AstNode(AST_IDENTIFIER, range); newNode->str = sname; newNode->basic_prep = true; + if (item_node->is_signed) + newNode = new AstNode(AST_TO_SIGNED, newNode); goto apply_newNode; } } diff --git a/tests/verilog/struct_access.sv b/tests/verilog/struct_access.sv new file mode 100644 index 000000000..f13b8dd51 --- /dev/null +++ b/tests/verilog/struct_access.sv @@ -0,0 +1,88 @@ +module top; + + typedef struct packed { + logic a; + logic signed b; + byte c; + byte unsigned d; + reg [3:0] e; + reg signed [3:0] f; + struct packed { + logic a; + logic signed b; + } x; + struct packed signed { + logic a; + logic signed b; + } y; + } S; + S s; + + initial begin + // test codegen for LHS + s.a = '1; + s.b = '1; + s.c = '1; + s.d = '1; + s.e = '1; + s.f = '1; + s.x.a = '1; + s.x.b = '1; + s.y.a = '1; + s.y.b = '1; + end + +`define CHECK(expr, width, signedness) \ + case (expr) \ + 1'sb1: \ + case (expr) \ + 2'sb11: if (!(signedness)) fail = 1; \ + default: if (signedness) fail = 1; \ + endcase \ + default: if (signedness) fail = 1; \ + endcase \ + case (expr) \ + 1'b1: if ((width) != 1) fail = 1; \ + 2'b11: if ((width) != 2) fail = 1; \ + 3'b111: if ((width) != 3) fail = 1; \ + 4'b1111: if ((width) != 4) fail = 1; \ + 5'b1111_1: if ((width) != 5) fail = 1; \ + 6'b1111_11: if ((width) != 6) fail = 1; \ + 7'b1111_11: if ((width) != 7) fail = 1; \ + 8'b1111_1111: if ((width) != 8) fail = 1; \ + 9'b1111_1111_1: if ((width) != 9) fail = 1; \ + default: fail = 1; \ + endcase \ + begin \ + reg [9:0] indirect; \ + indirect = (expr); \ + if ((indirect != (expr)) != (signedness)) fail = 1; \ + indirect = $unsigned(expr); \ + if ($countones(indirect) != (width)) fail = 1; \ + end + + initial begin + reg fail; + fail = 0; + + `CHECK(s.a, 1, 0) + `CHECK(s.b, 1, 1) + `CHECK(s.c, 8, 1) + `CHECK(s.d, 8, 0) + `CHECK(s.e, 4, 0) + `CHECK(s.f, 4, 1) + + `CHECK(s.x.a, 1, 0) + `CHECK(s.x.b, 1, 1) + `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) + + assert (fail === 0); + end + + +endmodule diff --git a/tests/verilog/struct_access.ys b/tests/verilog/struct_access.ys new file mode 100644 index 000000000..29d569c01 --- /dev/null +++ b/tests/verilog/struct_access.ys @@ -0,0 +1,4 @@ +read_verilog -formal -sv struct_access.sv +proc +opt -full +sat -verify -seq 1 -prove-asserts -show-all -- 2.30.2