Fix procedural concatenation/repetition problems

This patch evaluates the whole concatenation expression and
makes the concatenation padding sign aware. This is needed
when $signed({...}) is passed as an argument.

A repetition is just N copies of the base expression not N
evaluations of the base expression. This is only a problem
when functions have side effects. It's also faster to copy.
The evaluation must also be done when the replication count
is zero (see 1364-2005).

eval_tree.cc

@@ -1161,16 +1161,6 @@ NetEConst* NetEConcat::eval_tree(int prune_to_width)
 
       if (local_errors > 0) return 0;
 
-	// Handle the special case that the repeat expression is
-	// zero. In this case, just return a 0 value with the expected
-	// width.
-      if (repeat_val == 0) {
-	    verinum val (verinum::V0, expr_width());
-	    NetEConst*res = new NetEConst(val);
-	    res->set_width(val.len());
-	    return res;
-      }
-
 	// At this point, the "gap" is the width of a single repeat of
 	// the concatenation. The total width of the result is the gap
 	// times the repeat count.

t-dll-expr.cc

@@ -199,7 +199,7 @@ void dll_target::expr_concat(const NetEConcat*net)
       cur->type_  = IVL_EX_CONCAT;
       cur->value_ = IVL_VT_VECTOR;
       cur->width_ = net->expr_width();
-      cur->signed_ = 0;
+      cur->signed_ = net->has_sign() ? 1 : 0;
 
       cur->u_.concat_.rept  = net->repeat();
       cur->u_.concat_.parms = net->nparms();

tgt-vvp/eval_expr.c

@@ -1619,82 +1619,118 @@ static struct vector_info draw_binary_expr(ivl_expr_t exp,
 static struct vector_info draw_concat_expr(ivl_expr_t exp, unsigned wid,
 					   int stuff_ok_flag)
 {
-      unsigned off, rep;
+      unsigned off, rep, expr_wid, concat_wid, num_sube, idx;
       struct vector_info res;
 
       int alloc_exclusive = (stuff_ok_flag&STUFF_OK_RO) ? 0 : 1;
 
-	/* Allocate a vector to hold the result. */
+	/* Find out how wide the base concatenation expression is. */
-      res.base = allocate_vector(wid);
+      num_sube = ivl_expr_parms(exp);
-      res.wid = wid;
+      expr_wid = 0;
-      if (res.base == 0) {
+      for (idx = 0 ; idx < num_sube; idx += 1) {
-	    fprintf(stderr, "%s:%u: vvp.tgt error: "
+	    expr_wid += ivl_expr_width(ivl_expr_parm(exp, idx));
-		    "Unable to allocate %u thread bits "
-		    "for result of concatenation.\n",
-		    ivl_expr_file(exp), ivl_expr_lineno(exp), wid);
-	    vvp_errors += 1;
       }
 
 	/* Get the repeat count. This must be a constant that has been
 	   evaluated at compile time. The operands will be repeated to
 	   form the result. */
       rep = ivl_expr_repeat(exp);
+
+	/* Allocate a vector to hold the result. */
+      if (rep == 0) {
+	      /* If the replication is zero we need to allocate temporary
+	       * space to build the concatenation. */
+	    res.base = allocate_vector(expr_wid);
+	    res.wid = expr_wid;
+      } else {
+	    res.base = allocate_vector(wid);
+	    res.wid = wid;
+      }
+      if (res.base == 0) {
+	    fprintf(stderr, "%s:%u: vvp.tgt error: "
+		    "Unable to allocate %u thread bits "
+		    "for result of concatenation.\n",
+		    ivl_expr_file(exp), ivl_expr_lineno(exp),
+		    rep ? wid : expr_wid);
+	    vvp_errors += 1;
+      }
+
+	/* If the result is the right size we can just build this in place. */
+      concat_wid = rep*expr_wid;
+      if (concat_wid <= wid) {
 	    off = 0;
-
+	      /* Evaluate the base expression. */
-      while (rep > 0) {
+	    for (idx = num_sube; idx > 0; idx -= 1) {
-
-	      /* Each repeat, evaluate the sub-expressions, from lsb
-		 to msb, and copy each into the result vector. The
-		 expressions are arranged in the concatenation from
-		 MSB to LSB, to go through them backwards.
-
-		 Abort the loop if the result vector gets filled up. */
-
-	    unsigned idx = ivl_expr_parms(exp);
-	    while ((idx > 0) && (off < wid)) {
 		  ivl_expr_t arg = ivl_expr_parm(exp, idx-1);
 		  unsigned awid = ivl_expr_width(arg);
-
-		  unsigned trans;
 		  struct vector_info avec;
 
+		  assert(awid+off <= expr_wid);
+
 		    /* Try to locate the subexpression in the
-		       lookaside map. */
+		     * lookaside map and use it when available. */
 		  avec.base = allocate_vector_exp(arg, awid, alloc_exclusive);
 		  avec.wid = awid;
 
-		  trans = awid;
-		  if ((off + awid) > wid)
-			trans = wid - off;
-
 		  if (avec.base != 0) {
 			assert(awid == avec.wid);
-
+			fprintf(vvp_out, "    %%mov %u, %u, %u; Reuse "
-			fprintf(vvp_out, "    %%mov %u, %u, %u; Reuse calculated expression\n",
+			                 "calculated expression.\n",
-				res.base+off,
+			                 res.base+off, avec.base, awid);
-				avec.base, trans);
 			clr_vector(avec); 
-
 		  } else {
 			struct vector_info dest;
 
 			dest.base = res.base+off;
-			dest.wid  = trans;
+			dest.wid  = awid;
 			draw_eval_expr_dest(arg, dest, 0);
 		  }
 
-
+		  off += awid;
-		  idx -= 1;
-		  off += trans;
-		  assert(off <= wid);
 	    }
+
+	      /* Now repeat the expression as needed. */
+	    if (rep != 0) {
+		  rep -= 1;
+	    } else {
+		    /* Clear the temporary space and return nothing.
+		     * This will be caught in draw_eval_expr_dest()
+		     * and dropped. */
+		  clr_vector(res); 
+		  res.base = 0;
+		  res.wid = 0;
+	    }
+	    while (rep > 0) {
+		  fprintf(vvp_out, "    %%mov %u, %u, %u; Repetition %u\n",
+		                   res.base+expr_wid*rep, res.base, expr_wid,
+		                   rep+1);
 		  rep -= 1;
 	    }
 
-	/* Pad the result with 0, if necessary. */
+	      /* Pad the expression when needed. */
-      if (off < wid) {
+	    if (wid > concat_wid) {
+		    /* We can get a signed concatenation with $signed({...}). */
+		  if (ivl_expr_signed(exp)) {
+			unsigned base = res.base+concat_wid-1;
+			for (idx = 1; idx <= wid-concat_wid; idx += 1) {
+			      fprintf(vvp_out, "    %%mov %u, %u, 1;\n",
+			                       base+idx, base);
+			}
+		  } else {
 			fprintf(vvp_out, "    %%mov %u, 0, %u;\n",
-		    res.base+off, wid-off);
+			                 res.base+concat_wid, wid-concat_wid);
+		  }
+	    }
+      } else {
+	      /* The concatenation is too big for the result so draw it
+	       * at full width and then copy the bits that are needed. */
+	    struct vector_info full_res;
+	    full_res = draw_concat_expr(exp, concat_wid, stuff_ok_flag);
+	    assert(full_res.base);
+
+	    fprintf(vvp_out, "    %%mov %u, %u, %u;\n", res.base,
+	                     full_res.base, wid);
+	    clr_vector(full_res); 
       }
 
 	/* Save the accumulated result in the lookaside map. */
@@ -2843,7 +2879,8 @@ static void draw_eval_expr_dest(ivl_expr_t exp, struct vector_info dest,
       tmp = draw_eval_expr_wid(exp, dest.wid, stuff_ok_flag);
       assert(tmp.wid == dest.wid);
 
-      fprintf(vvp_out, "    %%mov %u, %u, %u;\n",
+	/* If the replication is 0 we can have a zero width, so skip it. */
+      if (dest.wid) fprintf(vvp_out, "    %%mov %u, %u, %u;\n",
                                      dest.base, tmp.base, dest.wid);
 
       if (tmp.base >= 8)