From c85eff93f2454bc446a847a87485dc8e1be38821 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Fri, 10 Oct 2008 20:42:07 -0700 Subject: [PATCH 1/9] The test_width methods scan and mark expressions with type and size. Later passes need the intermediate results for width and size so that some special cases, were self-determined arguments occur, can be processed properly during elaboration. This can be especially tricky and interesting for ternary expressions. --- PExpr.cc | 2 + PExpr.h | 36 ++++++---- PGate.h | 2 +- Statement.h | 2 +- elab_expr.cc | 185 +++++++++++++++++++++++++++++++++------------------ elaborate.cc | 28 ++++++-- netmisc.h | 2 +- 7 files changed, 173 insertions(+), 84 deletions(-) diff --git a/PExpr.cc b/PExpr.cc index 70c73a978..87fa6ad39 100644 --- a/PExpr.cc +++ b/PExpr.cc @@ -28,6 +28,8 @@ PExpr::PExpr() { + expr_type_ = IVL_VT_NO_TYPE; + has_sign_ = false; } PExpr::~PExpr() diff --git a/PExpr.h b/PExpr.h index a3da795a3..7237cbc2f 100644 --- a/PExpr.h +++ b/PExpr.h @@ -73,7 +73,13 @@ class PExpr : public LineInfo { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); + + // After the test_width method is complete, these methods + // return valid results. + ivl_variable_type_t expr_type() const { return expr_type_; } + unsigned expr_width() const { return expr_width_; } + bool has_sign() const { return has_sign_; } // During the elaborate_sig phase, we may need to scan // expressions to find implicit net declarations. @@ -134,6 +140,12 @@ class PExpr : public LineInfo { // of expressions. virtual bool is_constant(Module*) const; + protected: + // The derived class test_width methods should fill these in. + ivl_variable_type_t expr_type_; + unsigned expr_width_; + bool has_sign_; + private: // not implemented PExpr(const PExpr&); PExpr& operator= (const PExpr&); @@ -218,7 +230,7 @@ class PEFNumber : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual NetExpr*elaborate_expr(Design*des, NetScope*, int expr_width, bool sys_task_arg) const; virtual NetExpr*elaborate_pexpr(Design*des, NetScope*sc) const; @@ -244,7 +256,7 @@ class PEIdent : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual bool elaborate_sig(Design*des, NetScope*scope) const; @@ -360,7 +372,7 @@ class PENumber : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual NetEConst*elaborate_expr(Design*des, NetScope*, int expr_width, bool) const; @@ -397,7 +409,7 @@ class PEString : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual NetEConst*elaborate_expr(Design*des, NetScope*, int expr_width, bool) const; @@ -421,7 +433,7 @@ class PEUnary : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual bool elaborate_sig(Design*des, NetScope*scope) const; @@ -450,7 +462,7 @@ class PEBinary : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual bool elaborate_sig(Design*des, NetScope*scope) const; @@ -491,7 +503,7 @@ class PEBComp : public PEBinary { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&flag) const; + bool&flag); NetExpr* elaborate_expr(Design*des, NetScope*scope, int expr_width, bool sys_task_arg) const; @@ -506,7 +518,7 @@ class PEBShift : public PEBinary { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&flag) const; + bool&flag); virtual NetExpr*elaborate_expr(Design*des, NetScope*, int expr_width, bool sys_task_arg) const; }; @@ -527,7 +539,7 @@ class PETernary : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); virtual bool elaborate_sig(Design*des, NetScope*scope) const; @@ -571,7 +583,7 @@ class PECallFunction : public PExpr { virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); private: pform_name_t path_; @@ -584,7 +596,7 @@ class PECallFunction : public PExpr { unsigned test_width_sfunc_(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const; + bool&unsized_flag); }; #endif diff --git a/PGate.h b/PGate.h index e9f447451..b52763ee5 100644 --- a/PGate.h +++ b/PGate.h @@ -80,7 +80,7 @@ class PGate : public LineInfo { bool as_net_flag =false) const; unsigned pin_count() const { return pins_? pins_->count() : 0; } - const PExpr*pin(unsigned idx) const { return (*pins_)[idx]; } + PExpr*pin(unsigned idx) const { return (*pins_)[idx]; } strength_t strength0() const; strength_t strength1() const; diff --git a/Statement.h b/Statement.h index d817ff7a2..5662a9ce6 100644 --- a/Statement.h +++ b/Statement.h @@ -99,7 +99,7 @@ class PAssign_ : public Statement { virtual ~PAssign_() =0; const PExpr* lval() const { return lval_; } - const PExpr* rval() const { return rval_; } + PExpr* rval() const { return rval_; } protected: NetAssign_* elaborate_lval(Design*, NetScope*scope) const; diff --git a/elab_expr.cc b/elab_expr.cc index f54d9a47a..d73863d76 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -44,12 +44,25 @@ bool type_is_vectorable(ivl_variable_type_t type) NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, ivl_variable_type_t data_type_lv, int expr_wid_lv, - const PExpr*expr) + PExpr*expr) { - int expr_wid = 0; - bool unsized_flag = false; + bool unsized_flag = type_is_vectorable(data_type_lv)? true : false; ivl_variable_type_t rval_type = IVL_VT_NO_TYPE; + /* Find out what the r-value width is going to be. We + guess it will be the l-value width, but it may turn + out to be something else based on self-determined + widths inside. */ + int expr_wid = expr->test_width(des, scope, expr_wid_lv, expr_wid_lv, rval_type, unsized_flag); + + if (debug_elaborate) { + cerr << expr->get_fileline() << ": debug: r-value tested " + << "type=" << rval_type + << ", width=" << expr_wid + << ", min=" << expr_wid_lv + << ", unsized_flag=" << (unsized_flag?"true":"false") << endl; + } + switch (data_type_lv) { case IVL_VT_REAL: unsized_flag = true; @@ -58,19 +71,6 @@ NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, break; case IVL_VT_BOOL: case IVL_VT_LOGIC: - /* Find out what the r-value width is going to be. We - guess it will be the l-value width, but it may turn - out to be something else based on self-determined - widths inside. */ - expr_wid = expr->test_width(des, scope, expr_wid_lv, expr_wid_lv, rval_type, unsized_flag); - - if (debug_elaborate) { - cerr << expr->get_fileline() << ": debug: r-value tested " - << "width is " << expr_wid - << ", min=" << expr_wid_lv - << ", unsized_flag=" << (unsized_flag?"true":"false") << endl; - } - break; case IVL_VT_VOID: case IVL_VT_NO_TYPE: @@ -90,7 +90,7 @@ NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, */ unsigned PExpr::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, - ivl_variable_type_t&, bool&) const + ivl_variable_type_t&, bool&) { if (debug_elaborate) { cerr << get_fileline() << ": debug: test_width defaults to " @@ -113,7 +113,7 @@ NetExpr* PExpr::elaborate_expr(Design*des, NetScope*, int, bool) const unsigned PEBinary::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { ivl_variable_type_t expr_type_left = IVL_VT_NO_TYPE; ivl_variable_type_t expr_type_right= IVL_VT_NO_TYPE; @@ -126,23 +126,23 @@ unsigned PEBinary::test_width(Design*des, NetScope*scope, if (flag_right && !flag_left) { flag_left = flag_right; - wid_left = left_->test_width(des, scope, min, 0, expr_type_right, flag_right); + wid_left = left_->test_width(des, scope, min, 0, expr_type_left, flag_right); } if (flag_left || flag_right) unsized_flag = true; if (expr_type_left == IVL_VT_REAL || expr_type_right == IVL_VT_REAL) - expr_type = IVL_VT_REAL; + expr_type_ = IVL_VT_REAL; else if (expr_type_left==IVL_VT_LOGIC || expr_type_right==IVL_VT_LOGIC) - expr_type = IVL_VT_LOGIC; + expr_type_ = IVL_VT_LOGIC; else - expr_type = IVL_VT_BOOL; + expr_type_ = IVL_VT_BOOL; switch (op_) { case '+': case '-': - if (unsized_flag && type_is_vectorable(expr_type)) { + if (unsized_flag && type_is_vectorable(expr_type_)) { wid_left += 1; wid_right += 1; } @@ -172,7 +172,13 @@ unsigned PEBinary::test_width(Design*des, NetScope*scope, break; } - return min; + if (type_is_vectorable(expr_type_)) + expr_width_ = min; + else + expr_width_ = 1; + + expr_type = expr_type_; + return expr_width_; } /* @@ -697,7 +703,7 @@ NetExpr* PEBinary::elaborate_expr_base_add_(Design*des, unsigned PEBComp::test_width(Design*, NetScope*,unsigned, unsigned, ivl_variable_type_t&expr_type, - bool&) const + bool&) { expr_type = IVL_VT_LOGIC; return 1; @@ -748,7 +754,7 @@ NetExpr* PEBComp::elaborate_expr(Design*des, NetScope*scope, unsigned PEBShift::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { unsigned wid_left = left_->test_width(des,scope,min, 0, expr_type, unsized_flag); @@ -794,7 +800,7 @@ NetExpr*PEBShift::elaborate_expr(Design*des, NetScope*scope, unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { perm_string name = peek_tail_name(path_); @@ -810,14 +816,33 @@ unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, return wid; } + // Run through the arguments of the system function and make + // sure their widths/types are calculated. They are all self- + // determined. + for (unsigned idx = 0 ; idx < parms_.size() ; idx += 1) { + PExpr*expr = parms_[idx]; + ivl_variable_type_t sub_type = IVL_VT_NO_TYPE; + bool flag = false; + unsigned wid = expr->test_width(des,scope,0,0,sub_type,flag); + if (debug_elaborate) + cerr << get_fileline() << ": debug: test_width" + << " of " << name << " argument " << idx+1 + << " returns type=" << sub_type + << ", wid=" << wid << endl; + } + if (name=="$sizeof" || name=="$bits") { if (debug_elaborate) cerr << get_fileline() << ": debug: test_width" << " of $sizeof/$bits returns test_width" << " of compiler integer." << endl; - expr_type = IVL_VT_BOOL; - return integer_width; + expr_type_ = IVL_VT_BOOL; + expr_width_= integer_width; + has_sign_ = false; + + expr_type = expr_type_; + return expr_width_; } if (name=="$is_signed") { @@ -826,8 +851,10 @@ unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, << " of $is_signed returns test_width" << " of 1." << endl; - expr_type = IVL_VT_BOOL; - return 1; + expr_type_ = IVL_VT_BOOL; + expr_width_ = 1; + expr_type = expr_type_; + return expr_width_; } /* Get the return type of the system function by looking it up @@ -835,22 +862,24 @@ unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, const struct sfunc_return_type*sfunc_info = lookup_sys_func(peek_tail_name(path_)); - expr_type = sfunc_info->type; - unsigned wid = sfunc_info->wid; + expr_type_ = sfunc_info->type; + expr_width_ = sfunc_info->wid; + + expr_type = expr_type_; if (debug_elaborate) cerr << get_fileline() << ": debug: test_width " << "of system function " << name - << " returns wid=" << wid - << ", type=" << expr_type << "." << endl; + << " returns wid=" << expr_width_ + << ", type=" << expr_type_ << "." << endl; - return wid; + return expr_width_; } unsigned PECallFunction::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { if (peek_tail_name(path_)[0] == '$') return test_width_sfunc_(des, scope, min, lval, expr_type, unsized_flag); @@ -1312,10 +1341,14 @@ NetExpr* PEConcat::elaborate_expr(Design*des, NetScope*scope, unsigned PEFNumber::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { - expr_type = IVL_VT_REAL; + expr_type_ = IVL_VT_REAL; + expr_width_ = 1; + has_sign_ = true; unsized_flag = true; + + expr_type = expr_type_; return 1; } @@ -1342,11 +1375,19 @@ bool PEIdent::calculate_parts_(Design*des, NetScope*scope, ivl_assert(*this, index_tail.sel == index_component_t::SEL_PART); ivl_assert(*this, index_tail.msb && index_tail.lsb); + ivl_variable_type_t tmp_type = IVL_VT_NO_TYPE; + bool tmp_flag = false; + int msb_wid = index_tail.msb->test_width(des, scope, 0, 0, tmp_type, tmp_flag); + + tmp_type = IVL_VT_NO_TYPE; + tmp_flag = false; + int lsb_wid = index_tail.lsb->test_width(des, scope, 0, 0, tmp_type, tmp_flag); + /* This handles part selects. In this case, there are two bit select expressions, and both must be constant. Evaluate them and pass the results back to the caller. */ - NetExpr*lsb_ex = elab_and_eval(des, scope, index_tail.lsb, -1); + NetExpr*lsb_ex = elab_and_eval(des, scope, index_tail.lsb, lsb_wid); NetEConst*lsb_c = dynamic_cast(lsb_ex); if (lsb_c == 0) { cerr << index_tail.lsb->get_fileline() << ": error: " @@ -1359,7 +1400,7 @@ bool PEIdent::calculate_parts_(Design*des, NetScope*scope, return false; } - NetExpr*msb_ex = elab_and_eval(des, scope, index_tail.msb, -1); + NetExpr*msb_ex = elab_and_eval(des, scope, index_tail.msb, msb_wid); NetEConst*msb_c = dynamic_cast(msb_ex); if (msb_c == 0) { cerr << index_tail.msb->get_fileline() << ": error: " @@ -1458,7 +1499,7 @@ bool PEIdent::calculate_param_range_(Design*des, NetScope*scope, unsigned PEIdent::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { NetNet* net = 0; const NetExpr*par = 0; @@ -1468,14 +1509,21 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, symbol_search(des, scope, path_, net, par, eve, ex1, ex2); + if (net != 0) + expr_type_ = net->data_type(); + + expr_type = expr_type; + // If there is a part/bit select expression, then process it // here. This constrains the results no matter what kind the // name is. const name_component_t&name_tail = path_.back(); index_component_t::ctype_t use_sel = index_component_t::SEL_NONE; - if (!name_tail.index.empty()) - use_sel = name_tail.index.back().sel; + if (!name_tail.index.empty()) { + const index_component_t&index_tail = name_tail.index.back(); + use_sel = index_tail.sel; + } unsigned use_width = UINT_MAX; switch (use_sel) { @@ -1506,15 +1554,18 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, // The width of a signal expression is the width of the signal. if (net != 0) { - expr_type = net->data_type(); - return max(net->vector_width(), (unsigned long)min); + expr_type_ = net->data_type(); + expr_width_= max(net->vector_width(), (unsigned long)min); + expr_type = expr_type_; + return expr_width_; } // The width of a parameter name is the width of the range for // the parameter name, if a range is declared. Otherwise, the // width is undefined. if (par != 0) { - expr_type = par->expr_type(); + expr_type_ = par->expr_type(); + expr_type = expr_type_; if (ex1) { ivl_assert(*this, ex2); const NetEConst*ex1_const = dynamic_cast (ex1); @@ -1524,19 +1575,22 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, long msb = ex1_const->value().as_long(); long lsb = ex2_const->value().as_long(); if (msb >= lsb) - return msb - lsb + 1; + expr_width_ = msb - lsb + 1; else - return lsb - msb + 1; + expr_width_ = lsb - msb + 1; + return expr_width_; } // This is a parameter. If it is sized (meaning it was // declared with range expresions) then the range // expressions would have been caught above. So if we // got there there we know this is an unsized constant. + expr_width_ = par->expr_width(); unsized_flag = true; - return par->expr_width(); + return expr_width_; } + expr_width_ = min; return min; } @@ -2392,9 +2446,9 @@ NetExpr* PEIdent::elaborate_expr_net(Design*des, NetScope*scope, unsigned PENumber::test_width(Design*, NetScope*, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { - expr_type = IVL_VT_LOGIC; + expr_type_ = IVL_VT_LOGIC; unsigned use_wid = value_->len(); if (min > use_wid) use_wid = min; @@ -2405,6 +2459,8 @@ unsigned PENumber::test_width(Design*, NetScope*, if (lval > 0 && lval < use_wid) use_wid = lval; + expr_type = expr_type_; + expr_width_ = use_wid; return use_wid; } @@ -2435,7 +2491,7 @@ NetEConst* PENumber::elaborate_expr(Design*des, NetScope*, unsigned PEString::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { expr_type = IVL_VT_BOOL; unsigned use_wid = text_? 8*strlen(text_) : 0; @@ -2456,7 +2512,7 @@ NetEConst* PEString::elaborate_expr(Design*des, NetScope*, unsigned PETernary::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&flag) const + bool&flag) { ivl_variable_type_t tru_type = IVL_VT_NO_TYPE; unsigned tru_wid = tru_->test_width(des, scope, min, lval, tru_type,flag); @@ -2477,13 +2533,16 @@ unsigned PETernary::test_width(Design*des, NetScope*scope, } if (tru_type == IVL_VT_REAL || fal_type == IVL_VT_REAL) - expr_type = IVL_VT_REAL; + expr_type_ = IVL_VT_REAL; else if (tru_type == IVL_VT_LOGIC || fal_type == IVL_VT_LOGIC) - expr_type = IVL_VT_LOGIC; + expr_type_ = IVL_VT_LOGIC; else - expr_type = tru_type; - - return max(tru_wid,fal_wid); + expr_type_ = tru_type; + + expr_width_ = max(tru_wid,fal_wid); + + expr_type = expr_type_; + return expr_width_; } bool NetETernary::test_operand_compat(ivl_variable_type_t l, @@ -2520,13 +2579,11 @@ NetExpr*PETernary::elaborate_expr(Design*des, NetScope*scope, int use_wid = expr_wid >= 0? expr_wid : 0; if (expr_wid < 0) { - bool flag = expr_wid == -2; - ivl_variable_type_t expr_type = IVL_VT_NO_TYPE; - use_wid = this->test_width(des, scope, 0, 0, expr_type, flag); + use_wid = expr_width(); if (debug_elaborate) cerr << get_fileline() << ": debug: " << "Self-sized ternary chooses wid="<< use_wid - << ", type=" << expr_type + << ", type=" << expr_type() << endl; ivl_assert(*this, use_wid > 0); } @@ -2611,7 +2668,7 @@ NetExpr*PETernary::elaborate_expr(Design*des, NetScope*scope, unsigned PEUnary::test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, - bool&unsized_flag) const + bool&unsized_flag) { switch (op_) { case '!': diff --git a/elaborate.cc b/elaborate.cc index 59130d660..4b8056411 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -2321,12 +2321,30 @@ NetProc* PCallTask::elaborate_sys(Design*des, NetScope*scope) const for (unsigned idx = 0 ; idx < parm_count ; idx += 1) { PExpr*ex = parm(idx); - eparms[idx] = ex? ex->elaborate_expr(des, scope, -1, true) : 0; + if (ex != 0) { + ivl_variable_type_t use_type; + bool flag = false; + int use_wid = ex->test_width(des,scope,0,0, use_type, flag); + if (debug_elaborate) + cerr << ex->get_fileline() << ": debug: " + << "Argument " << (idx+1) + << " of system task tests its width as " << use_wid + << ", type=" << use_type + << ", unsized_flag=" << flag << endl; - /* Attempt to pre-evaluate the parameters. It may be - possible to at least partially reduce the - expression. */ - if (eparms[idx]) eval_expr(eparms[idx]); + // If the argument expression is unsized, then + // elaborate as self-determined *lossless* instead + // of sized. + if (flag==true) + use_wid = -2; + + eparms[idx] = ex->elaborate_expr(des, scope, use_wid, true); + if (eparms[idx]) + eval_expr(eparms[idx]); + + } else { + eparms[idx] = 0; + } } NetSTask*cur = new NetSTask(peek_tail_name(path_), eparms); diff --git a/netmisc.h b/netmisc.h index 7e31b37fa..97792d8f7 100644 --- a/netmisc.h +++ b/netmisc.h @@ -157,7 +157,7 @@ extern NetExpr* elab_and_eval(Design*des, NetScope*scope, */ extern NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, ivl_variable_type_t data_type_lv, - int expr_wid_lv, const PExpr*expr); + int expr_wid_lv, PExpr*expr); /* * This procedure elaborates an expression and if the elaboration is * successful the original expression is replaced with the new one. From a9497e9c6afa9c029d97ddb16e0e390ac025e137 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Fri, 10 Oct 2008 20:45:11 -0700 Subject: [PATCH 2/9] Less agressive padding of unsized lossless addition. When doing lossless addition to an unsized constant, we make the size be width of an integer, only to be consistent with other tools. In fact, don't go overboard if we don't have to. --- net_expr.cc | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/net_expr.cc b/net_expr.cc index 6b1a9fccd..09d2ce116 100644 --- a/net_expr.cc +++ b/net_expr.cc @@ -56,42 +56,54 @@ NetEBAdd::NetEBAdd(char op, NetExpr*l, NetExpr*r, bool lossless_flag) && (! tmp->has_width()) && (tmp->expr_width() > l->expr_width() || integer_width > l->expr_width()) ) { - unsigned target_width = l->expr_width() + 1; + verinum tmp_v = trim_vnum(tmp->value()); + unsigned target_width = l->expr_width(); + if (target_width < tmp_v.len()) + target_width = tmp_v.len(); + if (lossless_flag) + target_width += 1; if (target_width < integer_width) target_width = integer_width; + r->set_width(target_width); /* Note: This constant value will not gain a defined width from this. Make sure. */ assert(! r->has_width() ); + expr_width(target_width); + } else if ( (tmp = dynamic_cast(l)) && (! tmp->has_width()) && (tmp->expr_width() > r->expr_width() || integer_width > r->expr_width()) ) { - unsigned target_width = r->expr_width() + 1; + verinum tmp_v = trim_vnum(tmp->value()); + unsigned target_width = r->expr_width(); + if (target_width < tmp_v.len()) + target_width = tmp_v.len(); + if (lossless_flag) + target_width += 1; if (target_width < integer_width) target_width = integer_width; + l->set_width(target_width); /* Note: This constant value will not gain a defined width from this. Make sure. */ assert(! l->has_width() ); - } + expr_width(target_width); - unsigned pad_width = lossless_flag? 1 : 0; - cast_signed(l->has_sign() && r->has_sign()); - - /* Now that we have the operand sizes the way we like, or as - good as we are going to get them, set the size of myself. */ - if (r->expr_width() > l->expr_width()) { - - expr_width(r->expr_width() + pad_width); + } else if (r->expr_width() > l->expr_width()) { + unsigned loss_pad = lossless_flag? 1 : 0; + expr_width(r->expr_width() + loss_pad); } else { - expr_width(l->expr_width() + pad_width); + unsigned loss_pad = lossless_flag? 1 : 0; + expr_width(l->expr_width() + loss_pad); } + + cast_signed(l->has_sign() && r->has_sign()); } NetEBAdd::~NetEBAdd() From 3a99a5e800047ef3464daa5e304f9c76b67aca63 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sat, 11 Oct 2008 08:39:06 -0700 Subject: [PATCH 3/9] Suppress operand has_sign if expression is unsigned. Fix processing of arguments of +- and * when only one of the operands is signed. --- PExpr.cc | 1 - PExpr.h | 2 -- elab_expr.cc | 13 +++++++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/PExpr.cc b/PExpr.cc index 87fa6ad39..8e1c9889b 100644 --- a/PExpr.cc +++ b/PExpr.cc @@ -29,7 +29,6 @@ PExpr::PExpr() { expr_type_ = IVL_VT_NO_TYPE; - has_sign_ = false; } PExpr::~PExpr() diff --git a/PExpr.h b/PExpr.h index 7237cbc2f..e0e0f745a 100644 --- a/PExpr.h +++ b/PExpr.h @@ -79,7 +79,6 @@ class PExpr : public LineInfo { // return valid results. ivl_variable_type_t expr_type() const { return expr_type_; } unsigned expr_width() const { return expr_width_; } - bool has_sign() const { return has_sign_; } // During the elaborate_sig phase, we may need to scan // expressions to find implicit net declarations. @@ -144,7 +143,6 @@ class PExpr : public LineInfo { // The derived class test_width methods should fill these in. ivl_variable_type_t expr_type_; unsigned expr_width_; - bool has_sign_; private: // not implemented PExpr(const PExpr&); diff --git a/elab_expr.cc b/elab_expr.cc index d73863d76..ab3785657 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -662,6 +662,12 @@ 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_operand_sign_if_needed_(lp, rp); + + // Multiply will guess a width that is the sum of the // widths of the operand. If that sum is too small, then // pad one of the arguments enough that the sum is the @@ -693,6 +699,11 @@ NetExpr* PEBinary::elaborate_expr_base_add_(Design*des, if (! type_is_vectorable(rp->expr_type())) use_lossless_flag = false; + // 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_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())) tmp->set_width(expr_wid); @@ -839,7 +850,6 @@ unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, expr_type_ = IVL_VT_BOOL; expr_width_= integer_width; - has_sign_ = false; expr_type = expr_type_; return expr_width_; @@ -1345,7 +1355,6 @@ unsigned PEFNumber::test_width(Design*des, NetScope*scope, { expr_type_ = IVL_VT_REAL; expr_width_ = 1; - has_sign_ = true; unsized_flag = true; expr_type = expr_type_; From bd71b7fbb221a362104b46dee7f8c5b7047dca87 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sat, 11 Oct 2008 09:19:11 -0700 Subject: [PATCH 4/9] Real valued multiply are always signed. By definition. --- net_expr.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/net_expr.cc b/net_expr.cc index 09d2ce116..1644b87a4 100644 --- a/net_expr.cc +++ b/net_expr.cc @@ -262,14 +262,10 @@ NetEBMult::NetEBMult(char op, NetExpr*l, NetExpr*r) else expr_width(l->expr_width() + r->expr_width()); - cast_signed(l->has_sign() && r->has_sign()); - - /* If it turns out that this is not a signed expression, then - cast the signedness out of the operands as well. */ - if (! has_sign()) { - l->cast_signed(false); - r->cast_signed(false); - } + if (expr_type() == IVL_VT_REAL) + cast_signed(true); + else + cast_signed(l->has_sign() && r->has_sign()); } NetEBMult::~NetEBMult() From 995fc380e8c75b53f343b784f8fb5d9b30c2df6f Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sat, 11 Oct 2008 09:20:49 -0700 Subject: [PATCH 5/9] Get padding of signed comparison operands. The results of comparisons are unsigned, but the arguments may be signed and the calculation performed may be signed. Handle the padding properly for comparisons so that the math is properly signed. --- elab_expr.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/elab_expr.cc b/elab_expr.cc index ab3785657..09af31374 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -224,6 +224,13 @@ NetExpr* PEBinary::elaborate_expr(Design*des, NetScope*scope, void PEBinary::suppress_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp) { + // If an argument is a non-vector type, then this type + // suppression does not apply. + if (! type_is_vectorable(lp->expr_type())) + return; + if (! type_is_vectorable(rp->expr_type())) + return; + // If either operand is unsigned, then treat the whole // expression as unsigned. This test needs to be done here // instead of in *_expr_base_ because it needs to be done @@ -759,6 +766,14 @@ NetExpr* PEBComp::elaborate_expr(Design*des, NetScope*scope, suppress_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 + // are signed, then this padding will do sign extension. + if (type_is_vectorable(lp->expr_type())) + lp = pad_to_width(lp, use_wid); + if (type_is_vectorable(rp->expr_type())) + rp = pad_to_width(rp, use_wid); + return elaborate_eval_expr_base_(des, lp, rp, use_wid); } From 864a86f7ee86740eee2b69f3d9ab580f7661f21a Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sat, 11 Oct 2008 21:08:14 -0700 Subject: [PATCH 6/9] Multiply nodes are always unsigned. It is up to the elaborator to sign-extend the inputs if the multiply is signed in the Verilog source. The run time always processes the multiply as unsigned. --- ivl_target.h | 5 +++-- vvp/arith.cc | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ivl_target.h b/ivl_target.h index 33b2adb5f..bc5498a52 100644 --- a/ivl_target.h +++ b/ivl_target.h @@ -992,8 +992,9 @@ extern unsigned ivl_lpm_lineno(ivl_lpm_t net); * more than the width of the output, although the possibility of * overflow exists at run time. * - * Multiply may be signed. If so, the output should be sign extended - * to fill in its result. + * The inputs are always treated as unsigned. If the expression is + * supposed to be signed, elaboration will generate the necessary sign + * extension, so the target need not (must not) consider signedness. * * - Power (IVL_LPM_POW) * The power takes two inputs and generates an output. Unlike other diff --git a/vvp/arith.cc b/vvp/arith.cc index 9a2061504..126cfdaa1 100644 --- a/vvp/arith.cc +++ b/vvp/arith.cc @@ -336,13 +336,13 @@ void vvp_arith_mult::recv_vec4(vvp_net_ptr_t ptr, const vvp_vector4_t&bit) } long a; - if (! vector4_to_value(op_a_, a, true, true)) { + if (! vector4_to_value(op_a_, a, false, true)) { vvp_send_vec4(ptr.ptr()->out, x_val_); return; } long b; - if (! vector4_to_value(op_b_, b, true, true)) { + if (! vector4_to_value(op_b_, b, false, true)) { vvp_send_vec4(ptr.ptr()->out, x_val_); return; } From 62f518c205ecbda08c4ee36030d9694b3f3fcac1 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sat, 11 Oct 2008 21:52:41 -0700 Subject: [PATCH 7/9] The signedness of a ternary expression comes from its operands. If either of the operands of a ternary expression are unsigned, then both are treated as unsigned. It works just like a binary expression in that regard. --- PExpr.h | 4 ++-- elab_expr.cc | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/PExpr.h b/PExpr.h index e0e0f745a..652a56d7f 100644 --- a/PExpr.h +++ b/PExpr.h @@ -144,6 +144,8 @@ 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,8 +486,6 @@ class PEBinary : public PExpr { 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; - static void suppress_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp); - }; /* diff --git a/elab_expr.cc b/elab_expr.cc index 09af31374..773d922bf 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -222,7 +222,7 @@ NetExpr* PEBinary::elaborate_expr(Design*des, NetScope*scope, return tmp; } -void PEBinary::suppress_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp) +void PExpr::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. @@ -672,7 +672,7 @@ 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_operand_sign_if_needed_(lp, rp); + suppress_binary_operand_sign_if_needed_(lp, rp); // Multiply will guess a width that is the sum of the @@ -709,7 +709,7 @@ 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_operand_sign_if_needed_(lp, rp); + 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())) @@ -764,7 +764,7 @@ NetExpr* PEBComp::elaborate_expr(Design*des, NetScope*scope, return 0; } - suppress_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 @@ -2679,6 +2679,8 @@ NetExpr*PETernary::elaborate_expr(Design*des, NetScope*scope, return 0; } + 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. */ tru = pad_to_width(tru, use_wid); From 55b8ff4441d76048cbf5075fdff2bb5553403116 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Sun, 12 Oct 2008 21:38:07 -0700 Subject: [PATCH 8/9] Pad the subexpression of unary not. The operand of unary not needs to be padded to the expression width before the operator itself. Otherwise, the high (pad) bits will come out wrong. --- PExpr.h | 3 +++ elab_expr.cc | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/PExpr.h b/PExpr.h index 652a56d7f..c1584d0f6 100644 --- a/PExpr.h +++ b/PExpr.h @@ -444,6 +444,9 @@ class PEUnary : public PExpr { virtual bool is_constant(Module*) const; + private: + NetExpr* elaborate_expr_bits_(NetExpr*operand, int expr_wid) const; + private: char op_; PExpr*expr_; diff --git a/elab_expr.cc b/elab_expr.cc index 773d922bf..4eef2cb81 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -2716,6 +2716,7 @@ unsigned PEUnary::test_width(Design*des, NetScope*scope, // then the tested width. case '-': case '+': + case '~': if (test_wid < min) test_wid = min; break; @@ -2854,10 +2855,38 @@ NetExpr* PEUnary::elaborate_expr(Design*des, NetScope*scope, break; case '~': - tmp = new NetEUBits(op_, ip); - tmp->set_line(*this); + tmp = elaborate_expr_bits_(ip, expr_wid); break; } return tmp; } + +NetExpr* PEUnary::elaborate_expr_bits_(NetExpr*operand, int expr_wid) const +{ + // Handle the special case that the operand is a + // constant. Simply calculate the constant results of the + // expression and return that. + if (NetEConst*ctmp = dynamic_cast (operand)) { + verinum value = ctmp->value(); + if (expr_wid > (int)value.len()) + value = pad_to_width(value, expr_wid); + + // The only operand that I know can get here is the + // unary not (~). + ivl_assert(*this, op_ == '~'); + value = v_not(value); + + ctmp = new NetEConst(value); + ctmp->set_line(*this); + delete operand; + return ctmp; + } + + if (expr_wid > (int)operand->expr_width()) + operand = pad_to_width(operand, expr_wid); + + NetEUBits*tmp = new NetEUBits(op_, operand); + tmp->set_line(*this); + return tmp; +} From 25f6282a541a0cad2173170a74d6ce13768741b0 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Mon, 13 Oct 2008 19:43:02 -0700 Subject: [PATCH 9/9] Generate signed comparisons independent of the signedness of the expression The signedness of comparison expressions is typically unsigned, even if the comparison to be performed is signed. The comparison (and particularly the expr_synth of the comparison) needs to account for this explicitly. --- design_dump.cc | 4 ++- expr_synth.cc | 13 ++++++++- netlist.cc | 72 ++++++++++++++++++-------------------------------- netlist.h | 7 +++-- 4 files changed, 43 insertions(+), 53 deletions(-) diff --git a/design_dump.cc b/design_dump.cc index 5f1dcd91b..3aabb3fdd 100644 --- a/design_dump.cc +++ b/design_dump.cc @@ -343,7 +343,9 @@ void NetCLShift::dump_node(ostream&o, unsigned ind) const void NetCompare::dump_node(ostream&o, unsigned ind) const { - o << setw(ind) << "" << "LPM_COMPARE (NetCompare): " << name() << endl; + o << setw(ind) << "" << "LPM_COMPARE (NetCompare " + << (get_signed()? "signed" : "unsigned") << "): " + << name() << endl; dump_node_pins(o, ind+4); dump_obj_attr(o, ind+4); } diff --git a/expr_synth.cc b/expr_synth.cc index 9fe178fe1..8041a2c42 100644 --- a/expr_synth.cc +++ b/expr_synth.cc @@ -245,7 +245,18 @@ NetNet* NetEBComp::synthesize(Design*des, NetScope*scope) osig->local_flag(true); osig->data_type(IVL_VT_LOGIC); - bool signed_compare = lsig->get_signed() && rsig->get_signed(); + // Test if the comparison is signed. + // + // Note 1: This is not the same as asking if the result is + // signed. In fact, the result will typically be UNsigned. The + // decision to make the comparison signed depends on the + // operand expressions. + // + // Note 2: The operand expressions may be signed even if the + // sig that comes out of synthesis is unsigned. The $signed() + // function markes the expression but doesn't change the + // underlying signals. + bool signed_compare = left_->has_sign() && right_->has_sign(); if (debug_elaborate) { cerr << get_fileline() << ": debug: Comparison (" << op_ << ")" << " is " << (signed_compare? "signed" : "unsigned") diff --git a/netlist.cc b/netlist.cc index 20a9284ff..a3251f29c 100644 --- a/netlist.cc +++ b/netlist.cc @@ -1276,19 +1276,17 @@ const Link& NetCLShift::pin_Distance() const } NetCompare::NetCompare(NetScope*s, perm_string n, unsigned wi) -: NetNode(s, n, 10), width_(wi) +: NetNode(s, n, 8), width_(wi) { signed_flag_ = false; - pin(0).set_dir(Link::INPUT); // Aclr - pin(1).set_dir(Link::INPUT); // Clock - pin(2).set_dir(Link::OUTPUT); // AGB - pin(3).set_dir(Link::OUTPUT); // AGEB - pin(4).set_dir(Link::OUTPUT); // AEB - pin(5).set_dir(Link::OUTPUT); // ANEB - pin(6).set_dir(Link::OUTPUT); // ALB - pin(7).set_dir(Link::OUTPUT); // ALEB - pin(8).set_dir(Link::INPUT); // DataA - pin(9).set_dir(Link::INPUT); // DataB + pin(0).set_dir(Link::OUTPUT); // AGB + pin(1).set_dir(Link::OUTPUT); // AGEB + pin(2).set_dir(Link::OUTPUT); // AEB + pin(3).set_dir(Link::OUTPUT); // ANEB + pin(4).set_dir(Link::OUTPUT); // ALB + pin(5).set_dir(Link::OUTPUT); // ALEB + pin(6).set_dir(Link::INPUT); // DataA + pin(7).set_dir(Link::INPUT); // DataB } NetCompare::~NetCompare() @@ -1311,104 +1309,84 @@ void NetCompare::set_signed(bool flag) } -Link& NetCompare::pin_Aclr() -{ - return pin(0); -} - -const Link& NetCompare::pin_Aclr() const -{ - return pin(0); -} - -Link& NetCompare::pin_Clock() -{ - return pin(1); -} - -const Link& NetCompare::pin_Clock() const -{ - return pin(1); -} - Link& NetCompare::pin_AGB() { - return pin(2); + return pin(0); } const Link& NetCompare::pin_AGB() const { - return pin(2); + return pin(0); } Link& NetCompare::pin_AGEB() { - return pin(3); + return pin(1); } const Link& NetCompare::pin_AGEB() const { - return pin(3); + return pin(1); } Link& NetCompare::pin_AEB() { - return pin(4); + return pin(2); } const Link& NetCompare::pin_AEB() const { - return pin(4); + return pin(2); } Link& NetCompare::pin_ANEB() { - return pin(5); + return pin(3); } const Link& NetCompare::pin_ANEB() const { - return pin(5); + return pin(3); } Link& NetCompare::pin_ALB() { - return pin(6); + return pin(4); } const Link& NetCompare::pin_ALB() const { - return pin(6); + return pin(4); } Link& NetCompare::pin_ALEB() { - return pin(7); + return pin(5); } const Link& NetCompare::pin_ALEB() const { - return pin(7); + return pin(5); } Link& NetCompare::pin_DataA() { - return pin(8); + return pin(6); } const Link& NetCompare::pin_DataA() const { - return pin(8); + return pin(6); } Link& NetCompare::pin_DataB() { - return pin(9); + return pin(7); } const Link& NetCompare::pin_DataB() const { - return pin(9); + return pin(7); } NetDivide::NetDivide(NetScope*sc, perm_string n, unsigned wr, diff --git a/netlist.h b/netlist.h index ed8e16333..5c502a020 100644 --- a/netlist.h +++ b/netlist.h @@ -1040,6 +1040,9 @@ class NetCLShift : public NetNode { * inputs is narrower then the other, it is up to the generator to * make sure all the data pins are properly driven. * + * The signed() property is true if the comparison is to be done to + * signed arguments. The result is always UNsigned. + * * NOTE: This is not the same as the device used to support case * compare. Case comparisons handle Vx and Vz values, whereas this * device need not. @@ -1055,8 +1058,6 @@ class NetCompare : public NetNode { bool get_signed() const; void set_signed(bool); - Link& pin_Aclr(); - Link& pin_Clock(); Link& pin_AGB(); Link& pin_AGEB(); Link& pin_AEB(); @@ -1067,8 +1068,6 @@ class NetCompare : public NetNode { Link& pin_DataA(); Link& pin_DataB(); - const Link& pin_Aclr() const; - const Link& pin_Clock() const; const Link& pin_AGB() const; const Link& pin_AGEB() const; const Link& pin_AEB() const;