From: Zachary Snow <zach@zachjs.com>
Date: Tue, 22 Jun 2021 14:39:57 +0000 (-0400)
Subject: sv: fix two struct access bugs
X-Git-Tag: yosys-0.10~103
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=a9c8ca21d583c58a38931389f90bbaae0caec0d6;p=yosys.git

sv: fix two struct access bugs

- preserve signedness of struct members
- fix initial width detection of struct members (e.g., in case expressions)
---

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