Merge pull request #2005 from YosysHQ/claire/fix1990

Add "nowrshmsk" attribute, fix shift-and-mask bit slice write for signed offset

9 files changed, 142 insertions, 19 deletions

diff --git a/README.md b/README.md
index 1e486c3ac..c17c0c3b1 100644
--- a/README.md
+++ b/README.md
@@ -281,6 +281,9 @@ Verilog Attributes and non-standard features
   temporary variable within an always block. This is mostly used internally
   by Yosys to synthesize Verilog functions and access arrays.
 
+- The ``nowrshmsk`` attribute on a register prohibits the generation of
+  shift-and-mask type circuits for writing to bit slices of that register.
+
 - The ``onehot`` attribute on wires mark them as one-hot state register. This
   is used for example for memory port sharing and set by the fsm_map pass.
 
diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc
index 6a9af3f57..689fa9fb4 100644
--- a/frontends/ast/ast.cc
+++ b/frontends/ast/ast.cc
@@ -94,6 +94,7 @@ std::string AST::type2str(AstNodeType type)
 	X(AST_TO_BITS)
 	X(AST_TO_SIGNED)
 	X(AST_TO_UNSIGNED)
+	X(AST_SELFSZ)
 	X(AST_CONCAT)
 	X(AST_REPLICATE)
 	X(AST_BIT_NOT)
@@ -110,6 +111,8 @@ std::string AST::type2str(AstNodeType type)
 	X(AST_SHIFT_RIGHT)
 	X(AST_SHIFT_SLEFT)
 	X(AST_SHIFT_SRIGHT)
+	X(AST_SHIFTX)
+	X(AST_SHIFT)
 	X(AST_LT)
 	X(AST_LE)
 	X(AST_EQ)
@@ -615,6 +618,7 @@ void AstNode::dumpVlog(FILE *f, std::string indent) const
 	if (0) { case AST_POS:         txt = "+";  }
 	if (0) { case AST_NEG:         txt = "-";  }
 	if (0) { case AST_LOGIC_NOT:   txt = "!";  }
+	if (0) { case AST_SELFSZ:      txt = "@selfsz@";  }
 		fprintf(f, "%s(", txt.c_str());
 		children[0]->dumpVlog(f, "");
 		fprintf(f, ")");
@@ -628,6 +632,8 @@ void AstNode::dumpVlog(FILE *f, std::string indent) const
 	if (0) { case AST_SHIFT_RIGHT:  txt = ">>";  }
 	if (0) { case AST_SHIFT_SLEFT:  txt = "<<<"; }
 	if (0) { case AST_SHIFT_SRIGHT: txt = ">>>"; }
+	if (0) { case AST_SHIFTX:       txt = "@shiftx@"; }
+	if (0) { case AST_SHIFT:        txt = "@shift@"; }
 	if (0) { case AST_LT:           txt = "<";   }
 	if (0) { case AST_LE:           txt = "<=";  }
 	if (0) { case AST_EQ:           txt = "==";  }
diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index 3f6329112..8932108e3 100644
--- a/frontends/ast/ast.h
+++ b/frontends/ast/ast.h
@@ -75,6 +75,7 @@ namespace AST
 		AST_TO_BITS,
 		AST_TO_SIGNED,
 		AST_TO_UNSIGNED,
+		AST_SELFSZ,
 		AST_CONCAT,
 		AST_REPLICATE,
 		AST_BIT_NOT,
@@ -91,6 +92,8 @@ namespace AST
 		AST_SHIFT_RIGHT,
 		AST_SHIFT_SLEFT,
 		AST_SHIFT_SRIGHT,
+		AST_SHIFTX,
+		AST_SHIFT,
 		AST_LT,
 		AST_LE,
 		AST_EQ,
diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc
index 996762715..05bea69c1 100644
--- a/frontends/ast/genrtlil.cc
+++ b/frontends/ast/genrtlil.cc
@@ -809,6 +809,11 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint, bool *foun
 		sign_hint = false;
 		break;
 
+	case AST_SELFSZ:
+		sub_width_hint = 0;
+		children.at(0)->detectSignWidthWorker(sub_width_hint, sign_hint);
+		break;
+
 	case AST_CONCAT:
 		for (auto child : children) {
 			sub_width_hint = 0;
@@ -856,6 +861,8 @@ void AstNode::detectSignWidthWorker(int &width_hint, bool &sign_hint, bool *foun
 	case AST_SHIFT_RIGHT:
 	case AST_SHIFT_SLEFT:
 	case AST_SHIFT_SRIGHT:
+	case AST_SHIFTX:
+	case AST_SHIFT:
 	case AST_POW:
 		children[0]->detectSignWidthWorker(width_hint, sign_hint, found_real);
 		break;
@@ -1205,13 +1212,18 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 					AstNode *fake_ast = new AstNode(AST_NONE, clone(), children[0]->children.size() >= 2 ?
 							children[0]->children[1]->clone() : children[0]->children[0]->clone());
 					fake_ast->children[0]->delete_children();
-					RTLIL::SigSpec shift_val = fake_ast->children[1]->genRTLIL();
+
+					int fake_ast_width = 0;
+					bool fake_ast_sign = true;
+					fake_ast->children[1]->detectSignWidth(fake_ast_width, fake_ast_sign);
+					RTLIL::SigSpec shift_val = fake_ast->children[1]->genRTLIL(fake_ast_width, fake_ast_sign);
+
 					if (id2ast->range_right != 0) {
-						shift_val = current_module->Sub(NEW_ID, shift_val, id2ast->range_right, fake_ast->children[1]->is_signed);
+						shift_val = current_module->Sub(NEW_ID, shift_val, id2ast->range_right, fake_ast_sign);
 						fake_ast->children[1]->is_signed = true;
 					}
 					if (id2ast->range_swapped) {
-						shift_val = current_module->Sub(NEW_ID, RTLIL::SigSpec(source_width - width), shift_val, fake_ast->children[1]->is_signed);
+						shift_val = current_module->Sub(NEW_ID, RTLIL::SigSpec(source_width - width), shift_val, fake_ast_sign);
 						fake_ast->children[1]->is_signed = true;
 					}
 					if (GetSize(shift_val) >= 32)
@@ -1265,7 +1277,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 
 	// just pass thru the signal. the parent will evaluate the is_signed property and interpret the SigSpec accordingly
 	case AST_TO_SIGNED:
-	case AST_TO_UNSIGNED: {
+	case AST_TO_UNSIGNED:
+	case AST_SELFSZ: {
 			RTLIL::SigSpec sig = children[0]->genRTLIL();
 			if (sig.size() < width_hint)
 				sig.extend_u0(width_hint, sign_hint);
@@ -1356,6 +1369,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 	if (0) { case AST_SHIFT_RIGHT:  type_name = ID($shr); }
 	if (0) { case AST_SHIFT_SLEFT:  type_name = ID($sshl); }
 	if (0) { case AST_SHIFT_SRIGHT: type_name = ID($sshr); }
+	if (0) { case AST_SHIFTX:       type_name = ID($shiftx); }
+	if (0) { case AST_SHIFT:        type_name = ID($shift); }
 		{
 			if (width_hint < 0)
 				detectSignWidth(width_hint, sign_hint);
diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc
index 252219094..e6200d663 100644
--- a/frontends/ast/simplify.cc
+++ b/frontends/ast/simplify.cc
@@ -608,6 +608,7 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 	case AST_TO_BITS:
 	case AST_TO_SIGNED:
 	case AST_TO_UNSIGNED:
+	case AST_SELFSZ:
 	case AST_CONCAT:
 	case AST_REPLICATE:
 	case AST_REDUCE_AND:
@@ -1788,7 +1789,18 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 			result_width = abs(int(left_at_zero_ast->integer - right_at_zero_ast->integer)) + 1;
 		}
 
-		if (0)
+		bool use_case_method = false;
+
+		if (children[0]->id2ast->attributes.count(ID::nowrshmsk)) {
+			AstNode *node = children[0]->id2ast->attributes.at(ID::nowrshmsk);
+			while (node->simplify(true, false, false, stage, -1, false, false)) { }
+			if (node->type != AST_CONSTANT)
+				log_file_error(filename, location.first_line, "Non-constant value for `nowrshmsk' attribute on `%s'!\n", children[0]->id2ast->str.c_str());
+			if (node->asAttrConst().as_bool())
+				use_case_method = true;
+		}
+
+		if (use_case_method)
 		{
 			// big case block
 
@@ -1796,10 +1808,10 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 			newNode = new AstNode(AST_CASE, shift_expr);
 			for (int i = 0; i < source_width; i++) {
 				int start_bit = children[0]->id2ast->range_right + i;
+				int end_bit = std::min(start_bit+result_width,source_width) - 1;
 				AstNode *cond = new AstNode(AST_COND, mkconst_int(start_bit, true));
 				AstNode *lvalue = children[0]->clone();
 				lvalue->delete_children();
-				int end_bit = std::min(start_bit+result_width,source_width) - 1;
 				lvalue->children.push_back(new AstNode(AST_RANGE,
 						mkconst_int(end_bit, true), mkconst_int(start_bit, true)));
 				cond->children.push_back(new AstNode(AST_BLOCK, new AstNode(type, lvalue, children[1]->clone())));
@@ -1846,11 +1858,40 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
 
 			AstNode *shamt = shift_expr;
 
-			newNode->children.push_back(new AstNode(AST_ASSIGN_EQ, ref_mask->clone(),
-					new AstNode(AST_SHIFT_LEFT, mkconst_bits(std::vector<RTLIL::State>(result_width, State::S1), false), shamt->clone())));
-			newNode->children.push_back(new AstNode(AST_ASSIGN_EQ, ref_data->clone(),
-					new AstNode(AST_SHIFT_LEFT, new AstNode(AST_BIT_AND, mkconst_bits(std::vector<RTLIL::State>(result_width, State::S1), false), children[1]->clone()), shamt)));
-			newNode->children.push_back(new AstNode(type, lvalue, new AstNode(AST_BIT_OR, new AstNode(AST_BIT_AND, old_data, new AstNode(AST_BIT_NOT, ref_mask)), ref_data)));
+			int shamt_width_hint = 0;
+			bool shamt_sign_hint = true;
+			shamt->detectSignWidth(shamt_width_hint, shamt_sign_hint);
+
+			int start_bit = children[0]->id2ast->range_right;
+			bool use_shift = shamt_sign_hint;
+
+			if (start_bit != 0) {
+				shamt = new AstNode(AST_SUB, shamt, mkconst_int(start_bit, true));
+				use_shift = true;
+			}
+
+			AstNode *t;
+
+			t = mkconst_bits(std::vector<RTLIL::State>(result_width, State::S1), false);
+			if (use_shift)
+				t = new AstNode(AST_SHIFT, t, new AstNode(AST_NEG, shamt->clone()));
+			else
+				t = new AstNode(AST_SHIFT_LEFT, t, shamt->clone());
+			t = new AstNode(AST_ASSIGN_EQ, ref_mask->clone(), t);
+			newNode->children.push_back(t);
+
+			t = new AstNode(AST_BIT_AND, mkconst_bits(std::vector<RTLIL::State>(result_width, State::S1), false), children[1]->clone());
+			if (use_shift)
+				t = new AstNode(AST_SHIFT, t, new AstNode(AST_NEG, shamt));
+			else
+				t = new AstNode(AST_SHIFT_LEFT, t, shamt);
+			t = new AstNode(AST_ASSIGN_EQ, ref_data->clone(), t);
+			newNode->children.push_back(t);
+
+			t = new AstNode(AST_BIT_AND, old_data, new AstNode(AST_BIT_NOT, ref_mask));
+			t = new AstNode(AST_BIT_OR, t, ref_data);
+			t = new AstNode(type, lvalue, t);
+			newNode->children.push_back(t);
 		}
 
 		goto apply_newNode;
@@ -3026,6 +3067,7 @@ replace_fcall_later:;
 				}
 			}
 			break;
+		if (0) { case AST_SELFSZ: const_func = RTLIL::const_pos; }
 		if (0) { case AST_POS: const_func = RTLIL::const_pos; }
 		if (0) { case AST_NEG: const_func = RTLIL::const_neg; }
 			if (children[0]->type == AST_CONSTANT) {
@@ -3034,10 +3076,10 @@ replace_fcall_later:;
 			} else
 			if (children[0]->isConst()) {
 				newNode = new AstNode(AST_REALVALUE);
-				if (type == AST_POS)
-					newNode->realvalue = +children[0]->asReal(sign_hint);
-				else
+				if (type == AST_NEG)
 					newNode->realvalue = -children[0]->asReal(sign_hint);
+				else
+					newNode->realvalue = +children[0]->asReal(sign_hint);
 			}
 			break;
 		case AST_TERNARY:
diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y
index 4a531c09f..db9a130cf 100644
--- a/frontends/verilog/verilog_parser.y
+++ b/frontends/verilog/verilog_parser.y
@@ -645,13 +645,13 @@ non_opt_range:
 	} |
 	'[' expr TOK_POS_INDEXED expr ']' {
 		$$ = new AstNode(AST_RANGE);
-		AstNode *expr = new AstNode(AST_CONCAT, $2);
+		AstNode *expr = new AstNode(AST_SELFSZ, $2);
 		$$->children.push_back(new AstNode(AST_SUB, new AstNode(AST_ADD, expr->clone(), $4), AstNode::mkconst_int(1, true)));
 		$$->children.push_back(new AstNode(AST_ADD, expr, AstNode::mkconst_int(0, true)));
 	} |
 	'[' expr TOK_NEG_INDEXED expr ']' {
 		$$ = new AstNode(AST_RANGE);
-		AstNode *expr = new AstNode(AST_CONCAT, $2);
+		AstNode *expr = new AstNode(AST_SELFSZ, $2);
 		$$->children.push_back(new AstNode(AST_ADD, expr, AstNode::mkconst_int(0, true)));
 		$$->children.push_back(new AstNode(AST_SUB, new AstNode(AST_ADD, expr->clone(), AstNode::mkconst_int(1, true)), $4));
 	} |
diff --git a/kernel/constids.inc b/kernel/constids.inc
index aa75a9c09..6b40a5908 100644
--- a/kernel/constids.inc
+++ b/kernel/constids.inc
@@ -125,6 +125,7 @@ X(nomem2init)
 X(nomem2reg)
 X(nomeminit)
 X(nosync)
+X(nowrshmsk)
 X(O)
 X(OFFSET)
 X(onehot)
diff --git a/passes/tests/test_autotb.cc b/passes/tests/test_autotb.cc
index 42e8a61ea..19f21493d 100644
--- a/passes/tests/test_autotb.cc
+++ b/passes/tests/test_autotb.cc
@@ -81,6 +81,7 @@ static void autotest(std::ostream &f, RTLIL::Design *design, int num_iter, int s
 
 	f << stringf("integer i;\n");
 	f << stringf("integer file;\n\n");
+	f << stringf("reg [1023:0] filename;\n\n");
 
 	f << stringf("reg [31:0] xorshift128_x = 123456789;\n");
 	f << stringf("reg [31:0] xorshift128_y = 362436069;\n");
@@ -305,9 +306,15 @@ static void autotest(std::ostream &f, RTLIL::Design *design, int num_iter, int s
 	}
 
 	f << stringf("initial begin\n");
-	f << stringf("\t// $dumpfile(\"testbench.vcd\");\n");
-	f << stringf("\t// $dumpvars(0, testbench);\n");
-	f << stringf("\tfile = $fopen(`outfile);\n");
+	f << stringf("\tif ($value$plusargs(\"VCD=%%s\", filename)) begin\n");
+	f << stringf("\t\t$dumpfile(filename);\n");
+	f << stringf("\t\t$dumpvars(0, testbench);\n");
+	f << stringf("\tend\n");
+	f << stringf("\tif ($value$plusargs(\"OUT=%%s\", filename)) begin\n");
+	f << stringf("\t\tfile = $fopen(filename);\n");
+	f << stringf("\tend else begin\n");
+	f << stringf("\t\tfile = $fopen(`outfile);\n");
+	f << stringf("\tend\n");
 	for (auto module : design->modules())
 		if (!module->get_bool_attribute(ID::gentb_skip))
 			f << stringf("\t%s;\n", idy(module->name.str(), "test").c_str());
diff --git a/tests/simple/partsel.v b/tests/simple/partsel.v
index 83493fcb0..5e9730d6b 100644
--- a/tests/simple/partsel.v
+++ b/tests/simple/partsel.v
@@ -64,3 +64,49 @@ endmodule
 module partsel_test003(input [2:0] a, b, input [31:0] din, output [3:0] dout);
 assign dout = din[a*b +: 2];
 endmodule
+
+module partsel_test004 (
+	input [31:0] din,
+	input signed [4:0] n,
+	output reg [31:0] dout
+);
+	always @(*) begin
+		dout = 0;
+		dout[n+1 +: 2] = din[n +: 2];
+	end
+endmodule
+
+
+module partsel_test005 (
+	input [31:0] din,
+	input signed [4:0] n,
+	output reg [31:0] dout
+);
+	always @(*) begin
+		dout = 0;
+		dout[n+1] = din[n];
+	end
+endmodule
+
+module partsel_test006 (
+	input [31:-32] din,
+	input signed [4:0] n,
+	output reg [31:-32] dout
+);
+	always @(*) begin
+		dout = 0;
+		dout[n+1 +: 2] = din[n +: 2];
+	end
+endmodule
+
+
+module partsel_test007 (
+	input [31:-32] din,
+	input signed [4:0] n,
+	output reg [31:-32] dout
+);
+	always @(*) begin
+		dout = 0;
+		dout[n+1] = din[n];
+	end
+endmodule