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

5 files changed, 157 insertions, 12 deletions

diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index 069479353..9887d24ea 100644
--- 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 ad5814f1b..b8b9f715e 100644
--- 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 b9965ba99..305f67da8 100644
--- 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
index 000000000..58267b965
--- /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
index 000000000..7856e781c
--- /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