Optimization of nowrshmsk

Avoid multiplication of index by stride for two-dimensional packed
arrays and variable slices on the form dst[i*w +: w] = src

frontends/ast/simplify.cc

@@ -2824,27 +2824,12 @@ bool AstNode::simplify(bool const_fold, bool in_lvalue, int stage, int width_hin
 		if (!children[0]->id2ast->range_valid)
 			goto skip_dynamic_range_lvalue_expansion;
 
-		int source_width = children[0]->id2ast->range_left - children[0]->id2ast->range_right + 1;
+		AST::AstNode *member_node = get_struct_member(children[0]);
+		int source_width = member_node ?
+			member_node->range_left - member_node->range_right + 1 :
+			children[0]->id2ast->range_left - children[0]->id2ast->range_right + 1;
 		int source_offset = children[0]->id2ast->range_right;
 		int result_width = 1;
-		int stride = 1;
-		AST::AstNode *member_node = get_struct_member(children[0]);
-		if (member_node) {
-			// Clamp chunk to range of member within struct/union.
-			log_assert(!source_offset && !children[0]->id2ast->range_swapped);
-			source_width = member_node->range_left - member_node->range_right + 1;
-
-			// When the (* nowrshmsk *) attribute is set, a CASE block is generated below
-			// to select the indexed bit slice. When a multirange array is indexed, the
-			// start of each possible slice is separated by the bit stride of the last
-			// index dimension, and we can optimize the CASE block accordingly.
-			// The dimension of the original array expression is saved in the 'integer' field.
-			int dims = children[0]->integer;
-			stride = source_width;
-			for (int dim = 0; dim < dims; dim++) {
-				stride /= get_struct_range_width(member_node, dim);
-			}
-		}
 
 		AstNode *shift_expr = NULL;
 		AstNode *range = children[0]->children[0];
@@ -2875,12 +2860,45 @@ bool AstNode::simplify(bool const_fold, bool in_lvalue, int stage, int width_hin
 		{
 			// big case block
 
+			int stride = 1;
+			int div_stride = 1;
+
+			// Optimization: Remove multiplication of index by stride for two-dimensional packed arrays and
+			// variable slices on the form dst[i*w +: w] = src.
+			if (!source_offset && shift_expr->type == AST_MUL &&
+			    ((shift_expr->children[0]->type == AST_CONSTANT && (int)shift_expr->children[0]->integer == result_width) ||
+			     (shift_expr->children[1]->type == AST_CONSTANT && (int)shift_expr->children[1]->integer == result_width)))
+			{
+				int var_i = shift_expr->children[0]->type == AST_CONSTANT;
+				AstNode *tmp = shift_expr->children[var_i]->clone();
+				stride = result_width;
+				div_stride = stride;
+				delete shift_expr;
+				shift_expr = tmp;
+			}
+			else if (member_node)  // Member in packed struct/union
+			{
+				// Clamp chunk to range of member within struct/union.
+				log_assert(!source_offset && !children[0]->id2ast->range_swapped);
+
+				// When the (* nowrshmsk *) attribute is set, a CASE block is generated below
+				// to select the indexed bit slice. When a multirange array is indexed, the
+				// start of each possible slice is separated by the bit stride of the last
+				// index dimension, and we can optimize the CASE block accordingly.
+				// The dimension of the original array expression is saved in the 'integer' field.
+				int dims = children[0]->integer;
+				stride = source_width;
+				for (int dim = 0; dim < dims; dim++) {
+					stride /= get_struct_range_width(member_node, dim);
+				}
+			}
+
 			did_something = true;
 			newNode = new AstNode(AST_CASE, shift_expr);
 			for (int i = 0; i < source_width; i += stride) {
 				int start_bit = source_offset + 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 *cond = new AstNode(AST_COND, mkconst_int(start_bit/div_stride, true));
 				AstNode *lvalue = children[0]->clone();
 				lvalue->delete_children();
 				if (member_node)
@@ -2893,7 +2911,7 @@ bool AstNode::simplify(bool const_fold, bool in_lvalue, int stage, int width_hin
 		}
 		else
 		{
-			// mask and shift operations, disabled for now
+			// mask and shift operations
 
 			AstNode *wire_mask = new AstNode(AST_WIRE, new AstNode(AST_RANGE, mkconst_int(source_width-1, true), mkconst_int(0, true)));
 			wire_mask->str = stringf("$bitselwrite$mask$%s:%d$%d", RTLIL::encode_filename(filename).c_str(), location.first_line, autoidx++);

frontends/verilog/verilog_parser.y

@@ -884,15 +884,15 @@ non_opt_range:
 	} |
 	'[' expr TOK_POS_INDEXED expr ']' {
 		$$ = new AstNode(AST_RANGE);
-		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)));
+		AstNode *expr = new AstNode(AST_SELFSZ, $2->clone());
+		$$->children.push_back(new AstNode(AST_SUB, new AstNode(AST_ADD, expr, $4), AstNode::mkconst_int(1, true)));
+		$$->children.push_back($2);
 	} |
 	'[' expr TOK_NEG_INDEXED expr ']' {
 		$$ = new AstNode(AST_RANGE);
-		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));
+		AstNode *expr = new AstNode(AST_SELFSZ, $2->clone());
+		$$->children.push_back($2);
+		$$->children.push_back(new AstNode(AST_SUB, new AstNode(AST_ADD, expr, AstNode::mkconst_int(1, true)), $4));
 	} |
 	'[' expr ']' {
 		$$ = new AstNode(AST_RANGE);

tests/svtypes/struct_dynamic_range.ys

@@ -1,5 +1,5 @@
 read_verilog -sv struct_dynamic_range.sv
-select -assert-count 4 t:$mul
+select -assert-count 3 t:$mul
 select -assert-count 2 t:$shift
 select -assert-count 2 t:$shiftx
 prep -top top