Fix the signed-ness calculations of +- in parameter expressions.

This fixes up the elaboration of binary expressions found in
parameter expressions. Parameter expressions are special because
they elaborate early, before all the other parameters are necessarily
completed.

PExpr.h

@@ -143,8 +143,6 @@ class PExpr : public LineInfo {
       ivl_variable_type_t expr_type_;
       unsigned expr_width_;
 
-      static void suppress_binary_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp);
-
     private: // not implemented
       PExpr(const PExpr&);
       PExpr& operator= (const PExpr&);
@@ -484,15 +482,20 @@ class PEBinary : public PExpr {
       PExpr*left_;
       PExpr*right_;
 
-      NetExpr*elaborate_expr_base_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
-      NetExpr*elaborate_eval_expr_base_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
+      NetExpr*elaborate_expr_base_(Design*, NetExpr*lp, NetExpr*rp,
+				   int use_wid, bool is_pexpr =false) const;
+      NetExpr*elaborate_eval_expr_base_(Design*, NetExpr*lp, NetExpr*rp,
+					int use_wid) const;
 
       NetExpr*elaborate_expr_base_bits_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
-      NetExpr*elaborate_expr_base_div_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
+      NetExpr*elaborate_expr_base_div_(Design*, NetExpr*lp, NetExpr*rp,
+				       int use_wid, bool is_pexpr) const;
       NetExpr*elaborate_expr_base_lshift_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
       NetExpr*elaborate_expr_base_rshift_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
-      NetExpr*elaborate_expr_base_mult_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
-      NetExpr*elaborate_expr_base_add_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const;
+      NetExpr*elaborate_expr_base_mult_(Design*, NetExpr*lp, NetExpr*rp,
+					int use_wid, bool is_pexpr) const;
+      NetExpr*elaborate_expr_base_add_(Design*, NetExpr*lp, NetExpr*rp,
+				       int use_wid, bool is_pexpr) const;
 
 };
 

compiler.h

@@ -85,6 +85,7 @@ extern bool verbose_flag;
 extern bool debug_scopes;
 extern bool debug_eval_tree;
 extern bool debug_elaborate;
+extern bool debug_elab_pexpr;
 extern bool debug_synth2;
 extern bool debug_optimizer;
 extern bool debug_automatic;

elab_expr.cc

@@ -274,7 +274,7 @@ NetExpr* PEBinary::elaborate_expr(Design*des, NetScope*scope,
       return tmp;
 }
 
-void PExpr::suppress_binary_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp)
+void suppress_binary_operand_sign_if_needed(NetExpr*lp, NetExpr*rp)
 {
 	// If an argument is a non-vector type, then this type
 	// suppression does not apply.
@@ -315,7 +315,7 @@ NetExpr* PEBinary::elaborate_eval_expr_base_(Design*des,
  */
 NetExpr* PEBinary::elaborate_expr_base_(Design*des,
 					NetExpr*lp, NetExpr*rp,
-					int expr_wid) const
+					int expr_wid, bool is_pexpr) const
 {
       if (debug_elaborate) {
 	    cerr << get_fileline() << ": debug: elaborate expression "
@@ -345,12 +345,12 @@ NetExpr* PEBinary::elaborate_expr_base_(Design*des,
 	    break;
 
 	  case '*':
-	    tmp = elaborate_expr_base_mult_(des, lp, rp, expr_wid);
+	    tmp = elaborate_expr_base_mult_(des, lp, rp, expr_wid, is_pexpr);
 	    break;
 
 	  case '%':
 	  case '/':
-	    tmp = elaborate_expr_base_div_(des, lp, rp, expr_wid);
+	    tmp = elaborate_expr_base_div_(des, lp, rp, expr_wid, is_pexpr);
 	    break;
 
 	  case 'l':
@@ -374,7 +374,7 @@ NetExpr* PEBinary::elaborate_expr_base_(Design*des,
 
 	  case '+':
 	  case '-':
-	    tmp = elaborate_expr_base_add_(des, lp, rp, expr_wid);
+	    tmp = elaborate_expr_base_add_(des, lp, rp, expr_wid, is_pexpr);
 	    break;
 
 	  case 'E': /* === */
@@ -429,7 +429,7 @@ NetExpr* PEBinary::elaborate_expr_base_bits_(Design*des,
 
 NetExpr* PEBinary::elaborate_expr_base_div_(Design*des,
 					    NetExpr*lp, NetExpr*rp,
-					    int expr_wid) const
+					    int expr_wid, bool is_pexpr) const
 {
 	/* The % operator does not support real arguments in
 	   baseline Verilog. But we allow it in our extended
@@ -670,7 +670,7 @@ NetExpr* PEBinary::elaborate_expr_base_rshift_(Design*des,
 
 NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des,
 					     NetExpr*lp, NetExpr*rp,
-					     int expr_wid) const
+					     int expr_wid, bool is_pexpr) const
 {
 	// First, Make sure that signed arguments are padded to the
 	// width of the output. This is necessary for 2s complement
@@ -711,7 +711,8 @@ NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des,
 	// If this expression is unsigned, then make sure the
 	// arguments are unsigned so that the padding below doesn't
 	// cause any sign extension to happen.
-      suppress_binary_operand_sign_if_needed_(lp, rp);
+      if (! is_pexpr)
+	    suppress_binary_operand_sign_if_needed(lp, rp);
 
 
 	// Multiply will guess a width that is the sum of the
@@ -732,7 +733,7 @@ NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des,
 
 NetExpr* PEBinary::elaborate_expr_base_add_(Design*des,
 					    NetExpr*lp, NetExpr*rp,
-					    int expr_wid) const
+					    int expr_wid, bool is_pexpr) const
 {
       NetExpr*tmp;
       bool use_lossless_flag = expr_wid == -2;
@@ -748,7 +749,8 @@ NetExpr* PEBinary::elaborate_expr_base_add_(Design*des,
 	// If the expression is unsigned, then force the operands to
 	// unsigned so taht the set_width below doesn't cause them to
 	// be sign-extended.
-      suppress_binary_operand_sign_if_needed_(lp, rp);
+      if (! is_pexpr)
+	    suppress_binary_operand_sign_if_needed(lp, rp);
 
       tmp = new NetEBAdd(op_, lp, rp, use_lossless_flag);
       if (expr_wid > 0 && type_is_vectorable(tmp->expr_type()))
@@ -811,7 +813,7 @@ NetExpr* PEBComp::elaborate_expr(Design*des, NetScope*scope,
 	    return 0;
       }
 
-      suppress_binary_operand_sign_if_needed_(lp, rp);
+      suppress_binary_operand_sign_if_needed(lp, rp);
 
 	// The arguments of a compare need to have matching widths, so
 	// pad the width here. This matters because if the arguments
@@ -2970,7 +2972,7 @@ NetExpr*PETernary::elaborate_expr(Design*des, NetScope*scope,
 	    return 0;
       }
 
-      suppress_binary_operand_sign_if_needed_(tru, fal);
+      suppress_binary_operand_sign_if_needed(tru, fal);
 
 	/* Whatever the width we choose for the ternary operator, we
 	need to make sure the operands match. */

elab_pexpr.cc

@@ -53,7 +53,7 @@ NetExpr*PEBinary::elaborate_pexpr (Design*des, NetScope*scope) const
 	    return 0;
       }
 
-      NetExpr*tmp = elaborate_expr_base_(des, lp, rp, -2);
+      NetExpr*tmp = elaborate_expr_base_(des, lp, rp, -2, true);
       return tmp;
 }
 
@@ -67,8 +67,6 @@ NetExpr*PEBComp::elaborate_pexpr(Design*des, NetScope*scope) const
 	    return 0;
       }
 
-      suppress_binary_operand_sign_if_needed_(lp, rp);
-
       NetEBComp*tmp = new NetEBComp(op_, lp, rp);
       tmp->set_line(*this);
       bool flag = tmp->set_width(1);
@@ -452,3 +450,47 @@ NetExpr* PECallFunction::elaborate_pexpr(Design*des, NetScope*scope) const
       des->errors += 1;
       return 0;
 }
+
+void NetExpr::resolve_pexpr_type(void)
+{
+}
+
+void NetEBinary::resolve_pexpr_type(void)
+{
+      if (debug_elab_pexpr) {
+	    cerr << get_fileline() << ": debug: "
+		 << "Resolve_pexpr_type for binary " << human_readable_op(op_)
+		 << "." << endl;
+      }
+
+      left_->resolve_pexpr_type();
+      right_->resolve_pexpr_type();
+
+      switch (op_) {
+	  case '+':
+	  case '-':
+	  case '*':
+	  case '/':
+	    suppress_binary_operand_sign_if_needed(left_, right_);
+	    cast_signed_base_(left_->has_sign() && right_->has_sign());
+	    break;
+	  default:
+	    break;
+      }
+}
+
+void NetEParam::resolve_pexpr_type(void)
+{
+      if (debug_elab_pexpr) {
+	    cerr << get_fileline() << ": debug: "
+		 << "Resolve_pexpr_type for parameter " << reference_->first
+		 << "." << endl;
+      }
+
+      if (reference_->second.signed_flag) {
+	    cast_signed_base_(true);
+
+      } else {
+	    cast_signed_base_( reference_->second.expr->has_sign() );
+      }
+}

eval_tree.cc

@@ -129,7 +129,14 @@ NetExpr* NetEBAdd::eval_tree(int prune_to_width)
       lc = se? dynamic_cast<NetEConst*>(se->right_) : 0;
 
       if (lc != 0 && rc != 0) {
-	    assert(se != 0);
+	    ivl_assert(*this, se != 0);
+
+	    if (debug_eval_tree) {
+		  cerr << get_fileline() << ": debug: "
+		       << "Partially evalutate " << *this
+		       << " using (a+2)-1 --> (a+1) transform." << endl;
+	    }
+
 	    verinum lval = lc->value();
 	    verinum rval = rc->value();
 

main.cc

@@ -122,6 +122,7 @@ bool error_implicit = false;
 bool debug_scopes = false;
 bool debug_eval_tree = false;
 bool debug_elaborate = false;
+bool debug_elab_pexpr = false;
 bool debug_synth2 = false;
 bool debug_optimizer = false;
 bool debug_automatic = false;
@@ -387,6 +388,9 @@ static void read_iconfig_file(const char*ipath)
 		  } else if (strcmp(cp,"elaborate") == 0) {
 			debug_elaborate = true;
 			cerr << "debug: Enable elaborate debug" << endl;
+		  } else if (strcmp(cp,"elab_pexpr") == 0) {
+			debug_elab_pexpr = true;
+			cerr << "debug: Enable elaborate-pexpr debug" << endl;
 		  } else if (strcmp(cp,"synth2") == 0) {
 			debug_synth2 = true;
 			cerr << "debug: Enable synth2 debug" << endl;

net_design.cc

@@ -590,6 +590,8 @@ void NetScope::evaluate_parameters(Design*des)
       for (param_ref_t cur = parameters.begin()
 		 ; cur != parameters.end() ;  cur ++) {
 
+	    cur->second.expr->resolve_pexpr_type();
+
 	    switch ((*cur).second.type) {
 		case IVL_VT_BOOL:
 		case IVL_VT_LOGIC:

net_expr.cc

@@ -494,11 +494,13 @@ NetEParam::NetEParam()
 NetEParam::NetEParam(Design*d, NetScope*s, perm_string n)
     : des_(d), scope_(s), reference_(scope_->find_parameter(n))
 {
+      cast_signed_base_(reference_->second.signed_flag);
 }
 
 NetEParam::NetEParam(Design*d, NetScope*s, ref_t ref)
     : des_(d), scope_(s), reference_(ref)
 {
+      cast_signed_base_(reference_->second.signed_flag);
 }
 
 NetEParam::~NetEParam()

netlist.cc

@@ -2154,6 +2154,7 @@ ivl_variable_type_t NetEConst::expr_type() const
 NetEConstParam::NetEConstParam(NetScope*s, perm_string n, const verinum&v)
 : NetEConst(v), scope_(s), name_(n)
 {
+      cast_signed_base_(v.has_sign());
 }
 
 NetEConstParam::~NetEConstParam()

netlist.h

@@ -817,7 +817,7 @@ class NetScope : public Attrib {
 	/* After everything is all set up, the code generators like
 	   access to these things to make up the parameter lists. */
       struct param_expr_t : public LineInfo {
-	    param_expr_t() : type(IVL_VT_NO_TYPE), range(0) { }
+	    param_expr_t() : type(IVL_VT_NO_TYPE), signed_flag(false), msb(0), lsb(0), range(0), expr(0) { }
 	      // Type information
 	    ivl_variable_type_t type;
 	    bool signed_flag;
@@ -1562,6 +1562,14 @@ class NetExpr  : public LineInfo {
 	// expressions to check validity.
       virtual bool has_width() const;
 
+	// Expressions in parameter declarations may have encountered
+	// arguments that are themselves untyped parameters. These
+	// cannot be fully resolved for type when elaborated (they are
+	// elaborated before all parameter overrides are complete) so
+	// this virtual method needs to be called right before
+	// evaluating the expression. This wraps up the evaluation of
+	// the type.
+      virtual void resolve_pexpr_type();
 
 	// This method evaluates the expression and returns an
 	// equivalent expression that is reduced as far as compile
@@ -3201,6 +3209,7 @@ class NetEBinary  : public NetExpr {
 	// widths.
       virtual bool has_width() const;
 
+      virtual void resolve_pexpr_type();
       virtual NetEBinary* dup_expr() const;
       virtual NexusSet* nex_input(bool rem_out = true);
 
@@ -3499,6 +3508,7 @@ class NetEParam  : public NetExpr {
       ~NetEParam();
 
       virtual NexusSet* nex_input(bool rem_out = true);
+      virtual void resolve_pexpr_type();
       virtual bool set_width(unsigned w, bool last_chance);
       virtual bool has_width() const;
       virtual void expr_scan(struct expr_scan_t*) const;

netmisc.cc

@@ -387,6 +387,9 @@ const char *human_readable_op(const char op)
 	switch (op) {
 	    case '~': type = "~";  break;  // Negation
 
+	    case '+': type = "+";  break;
+	    case '-': type = "-";  break;
+
 	    case '^': type = "^";  break;  // XOR
 	    case 'X': type = "~^"; break;  // XNOR
 	    case '&': type = "&";  break;  // Bitwise AND

netmisc.h

@@ -166,6 +166,13 @@ void probe_expr_width(Design*des, NetScope*scope, PExpr*pe);
 extern NetExpr* elaborate_rval_expr(Design*des, NetScope*scope,
 				    ivl_variable_type_t data_type_lv,
 				    int expr_wid_lv, PExpr*expr);
+
+/*
+ * Used by elaboration to suppress the sign of an operand if the other
+ * is unsigned.
+ */
+extern void suppress_binary_operand_sign_if_needed(NetExpr*lp, NetExpr*rp);
+
 /*
  * This procedure elaborates an expression and if the elaboration is
  * successful the original expression is replaced with the new one.

scripts/devel-stub.conf

@@ -16,6 +16,7 @@ sys_func:vpi/va_math.sft
 warnings:implicit
 debug:eval_tree
 debug:elaborate
+debug:elab_pexpr
 debug:scopes
 debug:synth2
 out:a.out