verilog: fix case expression sign and width handling

- The case expression and case item expressions are extended to the
  maximum width among them, and are only interpreted as signed if all of
  them are signed
- Add overall width and sign detection for AST_CASE
- Add sign argument to genWidthRTLIL helper
- Coverage for both const and non-const case statements

diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index 069479353820e7180e4c8b646addc2f361f483e2..9887d24ead16c630c8a8b05123acff78eab8b61d 100644 (file)
--- a/frontends/ast/ast.h
+++ b/frontends/ast/ast.h
@@ -291,7 +291,7 @@ namespace AST
                // for expressions the resulting signal vector is returned
                // all generated cell instances, etc. are written to the RTLIL::Module pointed to by AST_INTERNAL::current_module
                RTLIL::SigSpec genRTLIL(int width_hint = -1, bool sign_hint = false);
-               RTLIL::SigSpec genWidthRTLIL(int width, const dict<RTLIL::SigBit, RTLIL::SigBit> *new_subst_ptr = NULL);
+               RTLIL::SigSpec genWidthRTLIL(int width, bool sgn, const dict<RTLIL::SigBit, RTLIL::SigBit> *new_subst_ptr = NULL);
 
                // compare AST nodes
                bool operator==(const AstNode &other) const;

diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc
index ad5814f1b124ed9811c3f94cca2cb542c136890a..b8b9f715e8f2b7615fe788b41d44c6458ee59dea 100644 (file)
--- a/frontends/ast/genrtlil.cc
+++ b/frontends/ast/genrtlil.cc
@@ -566,7 +566,7 @@ struct AST_INTERNAL::ProcessGenerator
                case AST_ASSIGN_LE:
                        {
                                RTLIL::SigSpec unmapped_lvalue = ast->children[0]->genRTLIL(), lvalue = unmapped_lvalue;
-                               RTLIL::SigSpec rvalue = ast->children[1]->genWidthRTLIL(lvalue.size(), &subst_rvalue_map.stdmap());
+                               RTLIL::SigSpec rvalue = ast->children[1]->genWidthRTLIL(lvalue.size(), true, &subst_rvalue_map.stdmap());
 
                                pool<SigBit> lvalue_sigbits;
                                for (int i = 0; i < GetSize(lvalue); i++) {
@@ -593,9 +593,13 @@ struct AST_INTERNAL::ProcessGenerator
 
                case AST_CASE:
                        {
+                               int width_hint;
+                               bool sign_hint;
+                               ast->detectSignWidth(width_hint, sign_hint);
+
                                RTLIL::SwitchRule *sw = new RTLIL::SwitchRule;
                                set_src_attr(sw, ast);
-                               sw->signal = ast->children[0]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap());
+                               sw->signal = ast->children[0]->genWidthRTLIL(width_hint, sign_hint, &subst_rvalue_map.stdmap());
                                current_case->switches.push_back(sw);
 
                                for (auto &attr : ast->attributes) {
@@ -637,7 +641,7 @@ struct AST_INTERNAL::ProcessGenerator
                                                else if (node->type == AST_BLOCK)
                                                        processAst(node);
                                                else
-                                                       current_case->compare.push_back(node->genWidthRTLIL(sw->signal.size(), &subst_rvalue_map.stdmap()));
+                                                       current_case->compare.push_back(node->genWidthRTLIL(width_hint, sign_hint, &subst_rvalue_map.stdmap()));
                                        }
                                        if (default_case != current_case)
                                                sw->cases.push_back(current_case);
@@ -715,9 +719,9 @@ struct AST_INTERNAL::ProcessGenerator
                                RTLIL::MemWriteAction action;
                                set_src_attr(&action, child);
                                action.memid = memid;
-                               action.address = child->children[0]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap());
-                               action.data = child->children[1]->genWidthRTLIL(current_module->memories[memid]->width, &subst_rvalue_map.stdmap());
-                               action.enable = child->children[2]->genWidthRTLIL(-1, &subst_rvalue_map.stdmap());
+                               action.address = child->children[0]->genWidthRTLIL(-1, true, &subst_rvalue_map.stdmap());
+                               action.data = child->children[1]->genWidthRTLIL(current_module->memories[memid]->width, true, &subst_rvalue_map.stdmap());
+                               action.enable = child->children[2]->genWidthRTLIL(-1, true, &subst_rvalue_map.stdmap());
                                RTLIL::Const orig_priority_mask = child->children[4]->bitsAsConst();
                                RTLIL::Const priority_mask = RTLIL::Const(0, cur_idx);
                                for (int i = 0; i < portid; i++) {
@@ -954,6 +958,32 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint, bool *foun
                width_hint = max(width_hint, this_width);
                break;
 
+       case AST_CASE:
+       {
+               // This detects the _overall_ sign and width to be used for comparing
+               // the case expression with the case item expressions. The case
+               // expression and case item expressions are extended to the maximum
+               // width among them, and are only interpreted as signed if all of them
+               // are signed.
+               width_hint = -1;
+               sign_hint = true;
+               auto visit_case_expr = [&width_hint, &sign_hint] (AstNode *node) {
+                       int sub_width_hint = -1;
+                       bool sub_sign_hint = true;
+                       node->detectSignWidth(sub_width_hint, sub_sign_hint);
+                       width_hint = max(width_hint, sub_width_hint);
+                       sign_hint &= sub_sign_hint;
+               };
+               visit_case_expr(children[0]);
+               for (size_t i = 1; i < children.size(); i++) {
+                       AstNode *child = children[i];
+                       for (AstNode *v : child->children)
+                               if (v->type != AST_DEFAULT && v->type != AST_BLOCK)
+                                       visit_case_expr(v);
+               }
+               break;
+       }
+
        case AST_FCALL:
                if (str == "\\$anyconst" || str == "\\$anyseq" || str == "\\$allconst" || str == "\\$allseq") {
                        if (GetSize(children) == 1) {
@@ -1695,7 +1725,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                        SigSpec addr_sig = children[0]->genRTLIL();
 
                        cell->setPort(ID::ADDR, addr_sig);
-                       cell->setPort(ID::DATA, children[1]->genWidthRTLIL(current_module->memories[str]->width * num_words));
+                       cell->setPort(ID::DATA, children[1]->genWidthRTLIL(current_module->memories[str]->width * num_words, true));
 
                        cell->parameters[ID::MEMID] = RTLIL::Const(str);
                        cell->parameters[ID::ABITS] = RTLIL::Const(GetSize(addr_sig));
@@ -1754,7 +1784,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
        case AST_ASSIGN:
                {
                        RTLIL::SigSpec left = children[0]->genRTLIL();
-                       RTLIL::SigSpec right = children[1]->genWidthRTLIL(left.size());
+                       RTLIL::SigSpec right = children[1]->genWidthRTLIL(left.size(), true);
                        if (left.has_const()) {
                                RTLIL::SigSpec new_left, new_right;
                                for (int i = 0; i < GetSize(left); i++)
@@ -1979,14 +2009,14 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 // this is a wrapper for AstNode::genRTLIL() when a specific signal width is requested and/or
 // signals must be substituted before being used as input values (used by ProcessGenerator)
 // note that this is using some global variables to communicate this special settings to AstNode::genRTLIL().
-RTLIL::SigSpec AstNode::genWidthRTLIL(int width, const dict<RTLIL::SigBit, RTLIL::SigBit> *new_subst_ptr)
+RTLIL::SigSpec AstNode::genWidthRTLIL(int width, bool sgn, const dict<RTLIL::SigBit, RTLIL::SigBit> *new_subst_ptr)
 {
        const dict<RTLIL::SigBit, RTLIL::SigBit> *backup_subst_ptr = genRTLIL_subst_ptr;
 
        if (new_subst_ptr)
                genRTLIL_subst_ptr = new_subst_ptr;
 
-       bool sign_hint = true;
+       bool sign_hint = sgn;
        int width_hint = width;
        detectSignWidthWorker(width_hint, sign_hint);
        RTLIL::SigSpec sig = genRTLIL(width_hint, sign_hint);

diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc
index b9965ba99e2423ac32eb657741ed4790c5d407d1..305f67da82d01e0fa8e13e2207df8344cc274fe9 100644 (file)
--- a/frontends/ast/simplify.cc
+++ b/frontends/ast/simplify.cc
@@ -1185,8 +1185,12 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 
        if (const_fold && type == AST_CASE)
        {
+               int width_hint;
+               bool sign_hint;
+               detectSignWidth(width_hint, sign_hint);
                while (children[0]->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) { }
                if (children[0]->type == AST_CONSTANT && children[0]->bits_only_01()) {
+                       RTLIL::Const case_expr = children[0]->bitsAsConst(width_hint, sign_hint);
                        std::vector<AstNode*> new_children;
                        new_children.push_back(children[0]);
                        for (int i = 1; i < GetSize(children); i++) {
@@ -1199,7 +1203,10 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
                                                continue;
                                        while (v->simplify(const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) { }
                                        if (v->type == AST_CONSTANT && v->bits_only_01()) {
-                                               if (v->bits == children[0]->bits) {
+                                               RTLIL::Const case_item_expr = v->bitsAsConst(width_hint, sign_hint);
+                                               RTLIL::Const match = const_eq(case_expr, case_item_expr, sign_hint, sign_hint, 1);
+                                               log_assert(match.bits.size() == 1);
+                                               if (match.bits.front() == RTLIL::State::S1) {
                                                        while (i+1 < GetSize(children))
                                                                delete children[++i];
                                                        goto keep_const_cond;

diff --git a/tests/simple/case_expr_const.v b/tests/simple/case_expr_const.v
new file mode 100644 (file)
index 0000000..58267b9
--- /dev/null
+++ b/tests/simple/case_expr_const.v
@@ -0,0 +1,49 @@
+// Note: case_expr_{,non_}const.v should be modified in tandem to ensure both
+// the constant and non-constant case evaluation logic is covered
+module top(
+       // expected to output all 1s
+    output reg a, b, c, d, e, f, g, h
+);
+    initial begin
+        case (2'b0)
+            1'b0:    a = 1;
+            default: a = 0;
+        endcase
+        case (2'sb11)
+            2'sb01:  b = 0;
+            1'sb1:   b = 1;
+        endcase
+        case (2'sb11)
+            1'sb0:   c = 0;
+            1'sb1:   c = 1;
+        endcase
+        case (2'sb11)
+            1'b0:    d = 0;
+            1'sb1:   d = 0;
+            default: d = 1;
+        endcase
+        case (2'b11)
+            1'sb0:   e = 0;
+            1'sb1:   e = 0;
+            default: e = 1;
+        endcase
+        case (1'sb1)
+            1'sb0:   f = 0;
+            2'sb11:  f = 1;
+            default: f = 0;
+        endcase
+        case (1'sb1)
+            1'sb0:   g = 0;
+            3'b0:    g = 0;
+            2'sb11:  g = 0;
+            default: g = 1;
+        endcase
+        case (1'sb1)
+            1'sb0:   h = 0;
+            1'b1:    h = 1;
+            3'b0:    h = 0;
+            2'sb11:  h = 0;
+            default: h = 0;
+        endcase
+    end
+endmodule

diff --git a/tests/simple/case_expr_non_const.v b/tests/simple/case_expr_non_const.v
new file mode 100644 (file)
index 0000000..7856e78
--- /dev/null
+++ b/tests/simple/case_expr_non_const.v
@@ -0,0 +1,59 @@
+// Note: case_expr_{,non_}const.v should be modified in tandem to ensure both
+// the constant and non-constant case evaluation logic is covered
+module top(
+       // expected to output all 1s
+    output reg a, b, c, d, e, f, g, h
+);
+    reg x_1b0 = 1'b0;
+    reg x_1b1 = 1'b1;
+    reg signed x_1sb0 = 1'sb0;
+    reg signed x_1sb1 = 1'sb1;
+    reg [1:0] x_2b0 = 2'b0;
+    reg [1:0] x_2b11 = 2'b11;
+    reg signed [1:0] x_2sb01 = 2'sb01;
+    reg signed [1:0] x_2sb11 = 2'sb11;
+    reg [2:0] x_3b0 = 3'b0;
+
+    initial begin
+        case (x_2b0)
+            x_1b0:   a = 1;
+            default: a = 0;
+        endcase
+        case (x_2sb11)
+            x_2sb01: b = 0;
+            x_1sb1:  b = 1;
+        endcase
+        case (x_2sb11)
+            x_1sb0:  c = 0;
+            x_1sb1:  c = 1;
+        endcase
+        case (x_2sb11)
+            x_1b0:   d = 0;
+            x_1sb1:  d = 0;
+            default: d = 1;
+        endcase
+        case (x_2b11)
+            x_1sb0:  e = 0;
+            x_1sb1:  e = 0;
+            default: e = 1;
+        endcase
+        case (x_1sb1)
+            x_1sb0:  f = 0;
+            x_2sb11: f = 1;
+            default: f = 0;
+        endcase
+        case (x_1sb1)
+            x_1sb0:  g = 0;
+            x_3b0:   g = 0;
+            x_2sb11: g = 0;
+            default: g = 1;
+        endcase
+        case (x_1sb1)
+            x_1sb0:  h = 0;
+            x_1b1:   h = 1;
+            x_3b0:   h = 0;
+            x_2sb11: h = 0;
+            default: h = 0;
+        endcase
+    end
+endmodule