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 2bda8fa92899d4cc4f237210328405c931803dbd..60c7de32d8f0362d948d327240dbf03b5b379354 100644 (file)
--- 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 e6f7b30c1f2c81eb8262f536191eaac1bbe0e81b..29d0bddefd0e449d5dd68731fc80e70c942b7d75 100644 (file)
--- 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 695fc429dd26917b53ba59c8902c8d1b3b66e4ed..5845c050126863510795bca301d4763bc2783ee1 100644 (file)
--- 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 (file)
index 0000000..f13b8dd
--- /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 (file)
index 0000000..29d569c
--- /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