From 082e06edb0898c6bf3675eddb12f44f16bce1dae Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Sat, 4 Oct 2008 15:30:34 +0100 Subject: [PATCH 1/4] Remove checks for constant expressions from the parser. This patch removes all the checks for constant expressions performed during the parsing phase, as these checks are (mostly) repeated during elaboration. It adds the missing check in the elaboration phase (the RHS of a register initialisation), and improves the error reporting and error recovery in other checks. This patch fixes pr2132552, which was caused by a fault in the parser constant expression checking. --- PExpr.cc | 155 ------------------------------------------------- PExpr.h | 23 -------- Statement.cc | 17 ++++-- Statement.h | 4 +- elab_expr.cc | 18 +++--- elab_sig.cc | 53 +++++++++++------ elaborate.cc | 13 ++++- eval_attrib.cc | 16 +++-- parse.y | 57 +----------------- pform.cc | 7 +-- pform.h | 2 - 11 files changed, 85 insertions(+), 280 deletions(-) diff --git a/PExpr.cc b/PExpr.cc index 70c73a978..e1d2081ea 100644 --- a/PExpr.cc +++ b/PExpr.cc @@ -39,11 +39,6 @@ bool PExpr::is_the_same(const PExpr*that) const return typeid(this) == typeid(that); } -bool PExpr::is_constant(Module*) const -{ - return false; -} - NetNet* PExpr::elaborate_lnet(Design*des, NetScope*) const { cerr << get_fileline() << ": error: expression not valid in assign l-value: " @@ -68,11 +63,6 @@ PEBinary::~PEBinary() { } -bool PEBinary::is_constant(Module*mod) const -{ - return left_->is_constant(mod) && right_->is_constant(mod); -} - PEBComp::PEBComp(char op, PExpr*l, PExpr*r) : PEBinary(op, l, r) { @@ -129,98 +119,11 @@ PECallFunction::~PECallFunction() { } -bool PECallFunction::is_constant(Module*mod) const -{ - /* Only $clog2 and the builtin mathematical functions can - * be a constant system function. */ - perm_string name = peek_tail_name(path_); - if (name[0] == '$' && (generation_flag >= GN_VER2005 || - gn_icarus_misc_flag || gn_verilog_ams_flag)) { - if (name == "$clog2" || - name == "$ln" || - name == "$log10" || - name == "$exp" || - name == "$sqrt" || - name == "$floor" || - name == "$ceil" || - name == "$sin" || - name == "$cos" || - name == "$tan" || - name == "$asin" || - name == "$acos" || - name == "$atan" || - name == "$sinh" || - name == "$cosh" || - name == "$tanh" || - name == "$asinh" || - name == "$acosh" || - name == "$atanh") { - if (parms_.size() != 1 || parms_[0] == 0) { - cerr << get_fileline() << ": error: " << name - << " takes a single argument." << endl; - return false; - } - /* If the argument is constant the function is constant. */ - return parms_[0]->is_constant(mod); - } - - if (name == "$pow" || - name == "$atan2" || - name == "$hypot") { - if (parms_.size() != 2 || parms_[0] == 0 || parms_[1] == 0) { - cerr << get_fileline() << ": error: " << name - << " takes two arguments." << endl; - return false; - /* If the arguments are constant the function is constant. */ - return parms_[0]->is_constant(mod) && - parms_[1]->is_constant(mod); - } - } - - /* These are only available with verilog-ams or icarus-misc. */ - if ((gn_icarus_misc_flag || gn_verilog_ams_flag) && - (name == "$log" || name == "$abs")) { - if (parms_.size() != 1 || parms_[0] == 0) { - cerr << get_fileline() << ": error: " << name - << " takes a single argument." << endl; - return false; - } - /* If the argument is constant the function is constant. */ - return parms_[0]->is_constant(mod); - } - if ((gn_icarus_misc_flag || gn_verilog_ams_flag) && - (name == "$min" || name == "$max")) { - if (parms_.size() != 2 || parms_[0] == 0 || parms_[1] == 0) { - cerr << get_fileline() << ": error: " << name - << " takes two arguments." << endl; - return false; - /* If the arguments are constant the function is constant. */ - return parms_[0]->is_constant(mod) && - parms_[1]->is_constant(mod); - } - } - - return false; /* The other system functions are not constant. */ - } - - /* Checking for constant user functions goes here. */ - return false; -} - PEConcat::PEConcat(const svector&p, PExpr*r) : parms_(p), repeat_(r) { } -bool PEConcat::is_constant(Module *mod) const -{ - bool constant = repeat_? repeat_->is_constant(mod) : true; - for (unsigned i = 0; constant && i < parms_.count(); ++i) { - constant = constant && parms_[i]->is_constant(mod); - } - return constant; -} - PEConcat::~PEConcat() { delete repeat_; @@ -260,11 +163,6 @@ const verireal& PEFNumber::value() const return *value_; } -bool PEFNumber::is_constant(Module*) const -{ - return true; -} - PEIdent::PEIdent(const pform_name_t&that) : path_(that) { @@ -279,37 +177,6 @@ PEIdent::~PEIdent() { } -/* - * An identifier can be in a constant expression if (and only if) it is - * a parameter or genvar. - * - * NOTE: This test does not work if the name is hierarchical! - */ -bool PEIdent::is_constant(Module*mod) const -{ - if (mod == 0) return false; - - /* */ - perm_string tmp = path_.back().name; - - { map::const_iterator cur; - cur = mod->parameters.find(tmp); - if (cur != mod->parameters.end()) return true; - } - - { map::const_iterator cur; - cur = mod->localparams.find(tmp); - if (cur != mod->localparams.end()) return true; - } - - { map::const_iterator cur; - cur = mod->genvars.find(tmp); - if (cur != mod->genvars.end()) return true; - } - - return false; -} - PENumber::PENumber(verinum*vp) : value_(vp) { @@ -335,11 +202,6 @@ bool PENumber::is_the_same(const PExpr*that) const return *value_ == *obj->value_; } -bool PENumber::is_constant(Module*) const -{ - return true; -} - PEString::PEString(char*s) : text_(s) { @@ -355,11 +217,6 @@ string PEString::value() const return text_; } -bool PEString::is_constant(Module*) const -{ - return true; -} - PETernary::PETernary(PExpr*e, PExpr*t, PExpr*f) : expr_(e), tru_(t), fal_(f) { @@ -369,13 +226,6 @@ PETernary::~PETernary() { } -bool PETernary::is_constant(Module*m) const -{ - return expr_->is_constant(m) - && tru_->is_constant(m) - && fal_->is_constant(m); -} - PEUnary::PEUnary(char op, PExpr*ex) : op_(op), expr_(ex) { @@ -384,8 +234,3 @@ PEUnary::PEUnary(char op, PExpr*ex) PEUnary::~PEUnary() { } - -bool PEUnary::is_constant(Module*m) const -{ - return expr_->is_constant(m); -} diff --git a/PExpr.h b/PExpr.h index a3da795a3..7ad81de88 100644 --- a/PExpr.h +++ b/PExpr.h @@ -128,12 +128,6 @@ class PExpr : public LineInfo { // expressions that must be structurally "identical". virtual bool is_the_same(const PExpr*that) const; - // Return true if this expression is a valid constant - // expression. the Module pointer is needed to find parameter - // identifiers and any other module specific interpretations - // of expressions. - virtual bool is_constant(Module*) const; - private: // not implemented PExpr(const PExpr&); PExpr& operator= (const PExpr&); @@ -159,8 +153,6 @@ class PEConcat : public PExpr { virtual NetAssign_* elaborate_lval(Design*des, NetScope*scope, bool is_force) const; - virtual bool is_constant(Module*) const; - private: NetNet* elaborate_lnet_common_(Design*des, NetScope*scope, bool bidirectional_flag) const; @@ -212,9 +204,6 @@ class PEFNumber : public PExpr { any rounding that is needed to get the value. */ virtual verinum* eval_const(Design*des, NetScope*sc) const; - /* A PEFNumber is a constant, so this returns true. */ - virtual bool is_constant(Module*) const; - virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, ivl_variable_type_t&expr_type, @@ -266,7 +255,6 @@ class PEIdent : public PExpr { // only applies to Ident expressions. NetNet* elaborate_port(Design*des, NetScope*sc) const; - virtual bool is_constant(Module*) const; verinum* eval_const(Design*des, NetScope*sc) const; const pform_name_t& path() const { return path_; } @@ -372,7 +360,6 @@ class PENumber : public PExpr { virtual verinum* eval_const(Design*des, NetScope*sc) const; virtual bool is_the_same(const PExpr*that) const; - virtual bool is_constant(Module*) const; private: verinum*const value_; @@ -404,8 +391,6 @@ class PEString : public PExpr { virtual NetEConst*elaborate_pexpr(Design*des, NetScope*sc) const; verinum* eval_const(Design*, NetScope*) const; - virtual bool is_constant(Module*) const; - private: char*text_; }; @@ -430,8 +415,6 @@ class PEUnary : public PExpr { virtual NetExpr*elaborate_pexpr(Design*des, NetScope*sc) const; virtual verinum* eval_const(Design*des, NetScope*sc) const; - virtual bool is_constant(Module*) const; - private: char op_; PExpr*expr_; @@ -443,8 +426,6 @@ class PEBinary : public PExpr { explicit PEBinary(char op, PExpr*l, PExpr*r); ~PEBinary(); - virtual bool is_constant(Module*) const; - virtual void dump(ostream&out) const; virtual unsigned test_width(Design*des, NetScope*scope, @@ -521,8 +502,6 @@ class PETernary : public PExpr { explicit PETernary(PExpr*e, PExpr*t, PExpr*f); ~PETernary(); - virtual bool is_constant(Module*) const; - virtual void dump(ostream&out) const; virtual unsigned test_width(Design*des, NetScope*scope, unsigned min, unsigned lval, @@ -560,8 +539,6 @@ class PECallFunction : public PExpr { ~PECallFunction(); - virtual bool is_constant(Module*) const; - virtual void dump(ostream &) const; virtual NetExpr*elaborate_expr(Design*des, NetScope*scope, diff --git a/Statement.cc b/Statement.cc index a8c9b9627..6cebaf471 100644 --- a/Statement.cc +++ b/Statement.cc @@ -26,20 +26,20 @@ Statement::~Statement() { } -PAssign_::PAssign_(PExpr*lval, PExpr*ex) -: event_(0), count_(0), lval_(lval), rval_(ex) +PAssign_::PAssign_(PExpr*lval, PExpr*ex, bool is_constant) +: event_(0), count_(0), lval_(lval), rval_(ex), is_constant_(is_constant) { delay_ = 0; } PAssign_::PAssign_(PExpr*lval, PExpr*de, PExpr*ex) -: event_(0), count_(0), lval_(lval), rval_(ex) +: event_(0), count_(0), lval_(lval), rval_(ex), is_constant_(false) { delay_ = de; } PAssign_::PAssign_(PExpr*lval, PExpr*cnt, PEventStatement*ev, PExpr*ex) -: event_(ev), count_(cnt), lval_(lval), rval_(ex) +: event_(ev), count_(cnt), lval_(lval), rval_(ex), is_constant_(false) { delay_ = 0; } @@ -51,7 +51,7 @@ PAssign_::~PAssign_() } PAssign::PAssign(PExpr*lval, PExpr*ex) -: PAssign_(lval, ex) +: PAssign_(lval, ex, false) { } @@ -65,12 +65,17 @@ PAssign::PAssign(PExpr*lval, PExpr*cnt, PEventStatement*d, PExpr*ex) { } +PAssign::PAssign(PExpr*lval, PExpr*ex, bool is_constant) +: PAssign_(lval, ex, is_constant) +{ +} + PAssign::~PAssign() { } PAssignNB::PAssignNB(PExpr*lval, PExpr*ex) -: PAssign_(lval, ex) +: PAssign_(lval, ex, false) { } diff --git a/Statement.h b/Statement.h index d817ff7a2..965caff21 100644 --- a/Statement.h +++ b/Statement.h @@ -93,7 +93,7 @@ class Statement : public LineInfo { */ class PAssign_ : public Statement { public: - explicit PAssign_(PExpr*lval, PExpr*ex); + explicit PAssign_(PExpr*lval, PExpr*ex, bool is_constant); explicit PAssign_(PExpr*lval, PExpr*de, PExpr*ex); explicit PAssign_(PExpr*lval, PExpr*cnt, PEventStatement*de, PExpr*ex); virtual ~PAssign_() =0; @@ -113,6 +113,7 @@ class PAssign_ : public Statement { private: PExpr* lval_; PExpr* rval_; + bool is_constant_; }; class PAssign : public PAssign_ { @@ -121,6 +122,7 @@ class PAssign : public PAssign_ { explicit PAssign(PExpr*lval, PExpr*ex); explicit PAssign(PExpr*lval, PExpr*de, PExpr*ex); explicit PAssign(PExpr*lval, PExpr*cnt, PEventStatement*de, PExpr*ex); + explicit PAssign(PExpr*lval, PExpr*ex, bool is_constant); ~PAssign(); virtual void dump(ostream&out, unsigned ind) const; diff --git a/elab_expr.cc b/elab_expr.cc index f54d9a47a..21518e99a 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -1356,7 +1356,10 @@ bool PEIdent::calculate_parts_(Design*des, NetScope*scope, "This lsb expression violates the rule: " << *index_tail.lsb << endl; des->errors += 1; - return false; + /* Attempt to recover from error. */ + lsb = 0; + } else { + lsb = lsb_c->value().as_long(); } NetExpr*msb_ex = elab_and_eval(des, scope, index_tail.msb, -1); @@ -1365,15 +1368,16 @@ bool PEIdent::calculate_parts_(Design*des, NetScope*scope, cerr << index_tail.msb->get_fileline() << ": error: " "Part select expressions must be constant." << endl; - cerr << index_tail.msb->get_fileline() << ": : This msb expression " - "violates the rule: " << *index_tail.msb << endl; + cerr << index_tail.msb->get_fileline() << ": : " + "This msb expression violates the rule: " + << *index_tail.msb << endl; des->errors += 1; - return false; + /* Attempt to recover from error. */ + msb = lsb; + } else { + msb = msb_c->value().as_long(); } - msb = msb_c->value().as_long(); - lsb = lsb_c->value().as_long(); - delete msb_ex; delete lsb_ex; return true; diff --git a/elab_sig.cc b/elab_sig.cc index 92e71b6c7..cf323c783 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -813,16 +813,19 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const if (port_set_ || net_set_) { long pmsb = 0, plsb = 0, nmsb = 0, nlsb = 0; + bool bad_lsb = false, bad_msb = false; /* If they exist get the port definition MSB and LSB */ if (port_set_ && port_msb_ != 0) { NetExpr*texpr = elab_and_eval(des, scope, port_msb_, -1); if (! eval_as_long(pmsb, texpr)) { cerr << port_msb_->get_fileline() << ": error: " - "Unable to evaluate MSB constant expression ``" - << *port_msb_ << "''." << endl; + "Range expressions must be constant." << endl; + cerr << port_msb_->get_fileline() << " : " + "This MSB expression violates the rule: " + << *port_msb_ << endl; des->errors += 1; - return 0; + bad_msb = true; } delete texpr; @@ -831,10 +834,12 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const if (! eval_as_long(plsb, texpr)) { cerr << port_lsb_->get_fileline() << ": error: " - "Unable to evaluate LSB constant expression ``" - << *port_lsb_ << "''." << endl; + "Range expressions must be constant." << endl; + cerr << port_lsb_->get_fileline() << " : " + "This LSB expression violates the rule: " + << *port_lsb_ << endl; des->errors += 1; - return 0; + bad_lsb = true; } delete texpr; @@ -846,15 +851,17 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const if (port_lsb_ == 0) assert(port_msb_ == 0); /* If they exist get the net/etc. definition MSB and LSB */ - if (net_set_ && net_msb_ != 0) { + if (net_set_ && net_msb_ != 0 && !bad_msb && !bad_lsb) { NetExpr*texpr = elab_and_eval(des, scope, net_msb_, -1); if (! eval_as_long(nmsb, texpr)) { cerr << net_msb_->get_fileline() << ": error: " - "Unable to evaluate MSB constant expression ``" - << *net_msb_ << "''." << endl; + "Range expressions must be constant." << endl; + cerr << net_msb_->get_fileline() << " : " + "This MSB expression violates the rule: " + << *net_msb_ << endl; des->errors += 1; - return 0; + bad_msb = true; } delete texpr; @@ -863,10 +870,12 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const if (! eval_as_long(nlsb, texpr)) { cerr << net_lsb_->get_fileline() << ": error: " - "Unable to evaluate LSB constant expression ``" - << *net_lsb_ << "''." << endl; + "Range expressions must be constant." << endl; + cerr << net_lsb_->get_fileline() << " : " + "This LSB expression violates the rule: " + << *net_lsb_ << endl; des->errors += 1; - return 0; + bad_lsb = true; } delete texpr; @@ -919,6 +928,10 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const } } + /* Attempt to recover from errors. */ + if (bad_lsb) nlsb = 0; + if (bad_msb) nmsb = nlsb; + lsb = nlsb; msb = nmsb; if (nmsb > nlsb) @@ -963,16 +976,18 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const delete lexp; if (!const_flag) { - cerr << get_fileline() << ": internal error: The indices " + cerr << get_fileline() << ": error: The indices " << "are not constant for array ``" << name_ << "''." << endl; des->errors += 1; - return 0; - } - + /* Attempt to recover from error, */ + array_s0 = 0; + array_e0 = 0; + } else { + array_s0 = lval.as_long(); + array_e0 = rval.as_long(); + } array_dimensions = 1; - array_s0 = lval.as_long(); - array_e0 = rval.as_long(); } /* If the net type is supply0 or supply1, replace it diff --git a/elaborate.cc b/elaborate.cc index 59130d660..5785eace7 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -1736,7 +1736,18 @@ NetExpr* PAssign_::elaborate_rval_(Design*des, NetScope*scope, NetExpr*rv = elaborate_rval_expr(des, scope, lv_type, lv_width, rval()); - return rv; + if (!is_constant_ || !rv) return rv; + + if (dynamic_cast(rv)) return rv; + if (dynamic_cast(rv)) return rv; + + cerr << get_fileline() << ": error: " + "The RHS expression must be constant." << endl; + cerr << get_fileline() << " : " + "This expression violates the rule: " << *rv << endl; + des->errors += 1; + delete rv; + return 0; } /* diff --git a/eval_attrib.cc b/eval_attrib.cc index dfc488041..ecb717230 100644 --- a/eval_attrib.cc +++ b/eval_attrib.cc @@ -53,14 +53,19 @@ attrib_list_t* evaluate_attributes(const map&att, /* If the attribute value is given in the source, then evaluate it as a constant. If the value is not given, then assume the value is 1. */ - verinum*tmp; - if (exp) + verinum*tmp = 0; + if (exp) { tmp = exp->eval_const(des, scope); - else + if (tmp == 0) { + cerr << exp->get_fileline() << ": error: ``" + << *exp << "'' is not a constant expression." + << endl; + des->errors += 1; + } + } + if (tmp == 0) tmp = new verinum(1); - if (tmp == 0) - cerr << "internal error: no result for " << *exp << endl; assert(tmp); table[idx].val = *tmp; @@ -106,4 +111,3 @@ attrib_list_t* evaluate_attributes(const map&att, * of complex attributes attached to gates. * */ - diff --git a/parse.y b/parse.y index bbd049151..99475d19c 100644 --- a/parse.y +++ b/parse.y @@ -396,12 +396,6 @@ attribute } | IDENTIFIER '=' expression { PExpr*tmp = $3; - if (!pform_expression_is_constant(tmp)) { - yyerror(@3, "error: attribute value " - "expression must be constant."); - delete tmp; - tmp = 0; - } named_pexpr_t*tmp2 = new named_pexpr_t; tmp2->name = lex_strings.make($1); tmp2->parm = tmp; @@ -567,14 +561,7 @@ charge_strength_opt defparam_assign : hierarchy_identifier '=' expression - { PExpr*tmp = $3; - if (!pform_expression_is_constant(tmp)) { - yyerror(@3, "error: parameter value " - "must be constant."); - delete tmp; - tmp = 0; - } - pform_set_defparam(*$1, $3); + { pform_set_defparam(*$1, $3); delete $1; } ; @@ -1813,9 +1800,6 @@ port_declaration delete port_declaration_context.range; port_declaration_context.range = $5; - if (! pform_expression_is_constant($8)) - yyerror(@8, "error: register declaration assignment" - " value must be a constant expression."); pform_make_reginit(@6, name, $8); delete $1; @@ -2777,15 +2761,7 @@ port_reference } | IDENTIFIER '[' expression ':' expression ']' - { if (!pform_expression_is_constant($3)) { - yyerror(@3, "error: msb expression of " - "port part select must be constant."); - } - if (!pform_expression_is_constant($5)) { - yyerror(@5, "error: lsb expression of " - "port part select must be constant."); - } - index_component_t itmp; + { index_component_t itmp; itmp.sel = index_component_t::SEL_PART; itmp.msb = $3; itmp.lsb = $5; @@ -2809,11 +2785,7 @@ port_reference } | IDENTIFIER '[' expression ']' - { if (!pform_expression_is_constant($3)) { - yyerror(@3, "error: port bit select " - "must be constant."); - } - index_component_t itmp; + { index_component_t itmp; itmp.sel = index_component_t::SEL_BIT; itmp.msb = $3; itmp.lsb = 0; @@ -2912,16 +2884,8 @@ port_type range : '[' expression ':' expression ']' { svector*tmp = new svector (2); - if (!pform_expression_is_constant($2)) - yyerror(@2, "error: msb of range must be constant."); - (*tmp)[0] = $2; - - if (!pform_expression_is_constant($4)) - yyerror(@4, "error: lsb of range must be constant."); - (*tmp)[1] = $4; - $$ = tmp; } ; @@ -2938,13 +2902,7 @@ dimensions : '[' expression ':' expression ']' { list *tmp = new list; index_component_t index; - if (!pform_expression_is_constant($2)) - yyerror(@2, "error: left array address must be " - "constant."); index.msb = $2; - if (!pform_expression_is_constant($4)) - yyerror(@4, "error: right array address must be " - "constant."); index.lsb = $4; tmp->push_back(index); $$ = tmp; @@ -2952,13 +2910,7 @@ dimensions | dimensions '[' expression ':' expression ']' { list *tmp = $1; index_component_t index; - if (!pform_expression_is_constant($3)) - yyerror(@3, "error: left array address must be " - "constant."); index.msb = $3; - if (!pform_expression_is_constant($5)) - yyerror(@5, "error: right array address must be " - "constant."); index.lsb = $5; tmp->push_back(index); $$ = tmp; @@ -3001,9 +2953,6 @@ register_variable { perm_string ident_name = lex_strings.make($1); pform_makewire(@1, ident_name, NetNet::REG, NetNet::NOT_A_PORT, IVL_VT_NO_TYPE, 0); - if (! pform_expression_is_constant($3)) - yyerror(@3, "error: register declaration assignment" - " value must be a constant expression."); pform_make_reginit(@1, ident_name, $3); $$ = $1; } diff --git a/pform.cc b/pform.cc index 03c6b266f..56c5db928 100644 --- a/pform.cc +++ b/pform.cc @@ -577,11 +577,6 @@ void pform_endgenerate() } } -bool pform_expression_is_constant(const PExpr*ex) -{ - return ex->is_constant(pform_cur_module); -} - MIN_TYP_MAX min_typ_max_flag = TYP; unsigned min_typ_max_warn = 10; @@ -1307,7 +1302,7 @@ void pform_make_reginit(const struct vlltype&li, PEIdent*lval = new PEIdent(name); FILE_NAME(lval, li); - PAssign*ass = new PAssign(lval, expr); + PAssign*ass = new PAssign(lval, expr, true); FILE_NAME(ass, li); PProcess*top = new PProcess(PProcess::PR_INITIAL, ass); FILE_NAME(top, li); diff --git a/pform.h b/pform.h index d372db52b..5422e74a2 100644 --- a/pform.h +++ b/pform.h @@ -316,8 +316,6 @@ extern PProcess* pform_make_behavior(PProcess::Type, Statement*, extern svector* pform_make_udp_input_ports(list*); -extern bool pform_expression_is_constant(const PExpr*); - extern void pform_make_events(list*names, const char*file, unsigned lineno); /* From 610dedc2b85b21c34841986a03d77da3728aa203 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Sat, 4 Oct 2008 21:19:27 +0100 Subject: [PATCH 2/4] Fix for pr2146824. Currently, a bitwise boolean operator will cause the expression width to be reduced to the size of the result. This can give rise to incorrect results. This patch changes the behaviour to take into account the required precision for calculating the two operands. --- set_width.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/set_width.cc b/set_width.cc index cb736cea0..24a6f8df0 100644 --- a/set_width.cc +++ b/set_width.cc @@ -122,7 +122,10 @@ bool NetEBBits::set_width(unsigned w, bool) /* */ unsigned use_width = w; - + if (left_->expr_width() > use_width) + use_width = left_->expr_width(); + if (right_->expr_width() > use_width) + use_width = right_->expr_width(); /* If the operands end up too small, then pad them to suit. */ From b5ad161f908890357fdb7630d892eeff7130b731 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Mon, 6 Oct 2008 00:44:17 +0100 Subject: [PATCH 3/4] Fix for pr2148500. This patch fixes the expression width calculation for a multiply operation with an unsized operation. The expression width needs to be at least the minimum of the maximum multiply result width and the width of an integer. --- elab_expr.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/elab_expr.cc b/elab_expr.cc index 21518e99a..d7a8dd7e8 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -154,6 +154,20 @@ unsigned PEBinary::test_width(Design*des, NetScope*scope, min = lval; break; + case '*': + if (unsized_flag && type_is_vectorable(expr_type)) { + unsigned use_wid = wid_left + wid_right; + if (use_wid > integer_width) + use_wid = integer_width; + if (use_wid > min) + min = use_wid; + } + if (wid_left > min) + min = wid_left; + if (wid_right > min) + min = wid_right; + break; + case 'l': // << Should be handled by PEBShift case '<': // < Should be handled by PEBComp case '>': // > Should be handled by PEBComp From c010145eaf69c9f72dce296728e7fb9e102c1b4c Mon Sep 17 00:00:00 2001 From: Larry Doolittle Date: Mon, 6 Oct 2008 09:53:39 -0700 Subject: [PATCH 4/4] Shadow reduction part 1 Start cleaning up shadowed variables, flagged by turning on -Wshadow. No intended change in functionality. Patch looks right, and is tested to compile and run on my machine. YMMV. --- netlist.cc | 82 +++++++++++++++++++++++------------------------ netlist.h | 2 +- vvp/array.cc | 8 ++--- vvp/compile.cc | 6 ++-- vvp/ufunc.cc | 4 +-- vvp/vthread.cc | 14 ++++---- vvp/vvp_island.cc | 9 +++--- vvp/vvp_net.cc | 12 +++---- vvp/vvp_net.h | 20 ++++++------ 9 files changed, 79 insertions(+), 78 deletions(-) diff --git a/netlist.cc b/netlist.cc index 20a9284ff..22de41afb 100644 --- a/netlist.cc +++ b/netlist.cc @@ -253,8 +253,8 @@ NetBranch::~NetBranch() { } -NetBus::NetBus(NetScope*s, unsigned pin_count) -: NetObj(s, perm_string::literal(""), pin_count) +NetBus::NetBus(NetScope*s, unsigned pin_count__) +: NetObj(s, perm_string::literal(""), pin_count__) { } @@ -782,9 +782,9 @@ const NetDelaySrc* NetNet::delay_path(unsigned idx) const } NetPartSelect::NetPartSelect(NetNet*sig, unsigned off, unsigned wid, - NetPartSelect::dir_t dir) + NetPartSelect::dir_t dir__) : NetNode(sig->scope(), sig->scope()->local_symbol(), 2), - off_(off), wid_(wid), dir_(dir) + off_(off), wid_(wid), dir_(dir__) { connect(pin(1), sig->pin(0)); set_line(*sig); @@ -880,22 +880,22 @@ const NetScope* NetProcTop::scope() const return scope_; } -NetCastInt::NetCastInt(NetScope*scope, perm_string n, unsigned width) -: NetNode(scope, n, 2), width_(width) +NetCastInt::NetCastInt(NetScope*scope__, perm_string n, unsigned width__) +: NetNode(scope__, n, 2), width_(width__) { pin(0).set_dir(Link::OUTPUT); pin(1).set_dir(Link::INPUT); } -NetCastReal::NetCastReal(NetScope*scope, perm_string n, bool signed_flag) -: NetNode(scope, n, 2), signed_flag_(signed_flag) +NetCastReal::NetCastReal(NetScope*scope__, perm_string n, bool signed_flag__) +: NetNode(scope__, n, 2), signed_flag_(signed_flag__) { pin(0).set_dir(Link::OUTPUT); pin(1).set_dir(Link::INPUT); } -NetConcat::NetConcat(NetScope*scope, perm_string n, unsigned wid, unsigned cnt) -: NetNode(scope, n, cnt+1), width_(wid) +NetConcat::NetConcat(NetScope*scope__, perm_string n, unsigned wid, unsigned cnt) +: NetNode(scope__, n, cnt+1), width_(wid) { pin(0).set_dir(Link::OUTPUT); for (unsigned idx = 1 ; idx < cnt+1 ; idx += 1) { @@ -912,9 +912,9 @@ unsigned NetConcat::width() const return width_; } -NetReplicate::NetReplicate(NetScope*scope, perm_string n, +NetReplicate::NetReplicate(NetScope*scope__, perm_string n, unsigned wid, unsigned rpt) -: NetNode(scope, n, 2), width_(wid), repeat_(rpt) +: NetNode(scope__, n, 2), width_(wid), repeat_(rpt) { pin(0).set_dir(Link::OUTPUT); pin(1).set_dir(Link::INPUT); @@ -948,8 +948,8 @@ unsigned NetReplicate::repeat() const * ... */ -NetFF::NetFF(NetScope*s, perm_string n, unsigned width) -: NetNode(s, n, 8), width_(width) +NetFF::NetFF(NetScope*s, perm_string n, unsigned width__) +: NetNode(s, n, 8), width_(width__) { pin_Clock().set_dir(Link::INPUT); pin_Enable().set_dir(Link::INPUT); @@ -1153,15 +1153,15 @@ const Link& NetAddSub::pin_Result() const return pin(3); } -NetArrayDq::NetArrayDq(NetScope*s, perm_string n, NetNet*mem, unsigned awid) +NetArrayDq::NetArrayDq(NetScope*s, perm_string n, NetNet*mem__, unsigned awid) : NetNode(s, n, 2), - mem_(mem), awidth_(awid) + mem_(mem__), awidth_(awid) { pin(0).set_dir(Link::OUTPUT); // Result pin(1).set_dir(Link::INPUT); // Address // Increment the expression reference count for the target // memory so that it is not deleted underneath me. - mem->incr_eref(); + mem_->incr_eref(); } NetArrayDq::~NetArrayDq() @@ -1210,11 +1210,11 @@ const Link& NetArrayDq::pin_Address() const * 2 -- Distance */ NetCLShift::NetCLShift(NetScope*s, perm_string n, - unsigned width, unsigned width_dist, - bool right_flag, bool signed_flag) + unsigned width__, unsigned width_dist__, + bool right_flag__, bool signed_flag__) : NetNode(s, n, 3), - width_(width), width_dist_(width_dist), - right_flag_(right_flag), signed_flag_(signed_flag) + width_(width__), width_dist_(width_dist__), + right_flag_(right_flag__), signed_flag_(signed_flag__) { pin(0).set_dir(Link::OUTPUT); // Result pin(1).set_dir(Link::INPUT); // Data @@ -1742,8 +1742,8 @@ unsigned NetBUFZ::width() const return width_; } -NetCaseCmp::NetCaseCmp(NetScope*s, perm_string n, unsigned wid, bool eeq) -: NetNode(s, n, 3), width_(wid), eeq_(eeq) +NetCaseCmp::NetCaseCmp(NetScope*s, perm_string n, unsigned wid, bool eeq__) +: NetNode(s, n, 3), width_(wid), eeq_(eeq__) { pin(0).set_dir(Link::OUTPUT); pin(1).set_dir(Link::INPUT); @@ -1974,8 +1974,8 @@ const NetScope* NetUTask::task() const return task_; } -NetAlloc::NetAlloc(NetScope*scope) -: scope_(scope) +NetAlloc::NetAlloc(NetScope*scope__) +: scope_(scope__) { } @@ -1993,8 +1993,8 @@ const NetScope* NetAlloc::scope() const return scope_; } -NetFree::NetFree(NetScope*scope) -: scope_(scope) +NetFree::NetFree(NetScope*scope__) +: scope_(scope__) { } @@ -2050,8 +2050,8 @@ bool NetExpr::has_width() const * be. However, if we don't, our default will be the width of the * largest operand. */ -NetEBBits::NetEBBits(char op, NetExpr*l, NetExpr*r) -: NetEBinary(op, l, r) +NetEBBits::NetEBBits(char op__, NetExpr*l, NetExpr*r) +: NetEBinary(op__, l, r) { if (r->expr_width() > l->expr_width()) expr_width(r->expr_width()); @@ -2070,8 +2070,8 @@ NetEBBits* NetEBBits::dup_expr() const return result; } -NetEBinary::NetEBinary(char op, NetExpr*l, NetExpr*r) -: op_(op), left_(l), right_(r) +NetEBinary::NetEBinary(char op__, NetExpr*l, NetExpr*r) +: op_(op__), left_(l), right_(r) { // Binary expressions of all sorts are signed if both the // arguments are signed. @@ -2095,8 +2095,8 @@ NetEBinary* NetEBinary::dup_expr() const return 0; } -NetEBLogic::NetEBLogic(char op, NetExpr*l, NetExpr*r) -: NetEBinary(op, l, r) +NetEBLogic::NetEBLogic(char op__, NetExpr*l, NetExpr*r) +: NetEBinary(op__, l, r) { expr_width(1); } @@ -2317,8 +2317,8 @@ ivl_variable_type_t NetETernary::expr_type() const return tru; } -NetEUnary::NetEUnary(char op, NetExpr*ex) -: NetExpr(ex->expr_width()), op_(op), expr_(ex) +NetEUnary::NetEUnary(char op__, NetExpr*ex) +: NetExpr(ex->expr_width()), op_(op__), expr_(ex) { switch (op_) { case '!': @@ -2346,8 +2346,8 @@ ivl_variable_type_t NetEUnary::expr_type() const return expr_->expr_type(); } -NetEUBits::NetEUBits(char op, NetExpr*ex) -: NetEUnary(op, ex) +NetEUBits::NetEUBits(char op__, NetExpr*ex) +: NetEUnary(op__, ex) { } @@ -2360,8 +2360,8 @@ ivl_variable_type_t NetEUBits::expr_type() const return expr_->expr_type(); } -NetEUReduce::NetEUReduce(char op, NetExpr*ex) -: NetEUnary(op, ex) +NetEUReduce::NetEUReduce(char op__, NetExpr*ex) +: NetEUnary(op__, ex) { expr_width(1); } @@ -2395,9 +2395,9 @@ unsigned NetLogic::width() const return width_; } -NetUReduce::NetUReduce(NetScope*scope, perm_string n, +NetUReduce::NetUReduce(NetScope*scope__, perm_string n, NetUReduce::TYPE t, unsigned wid) -: NetNode(scope, n, 2), type_(t), width_(wid) +: NetNode(scope__, n, 2), type_(t), width_(wid) { pin(0).set_dir(Link::OUTPUT); pin(1).set_dir(Link::INPUT); diff --git a/netlist.h b/netlist.h index ed8e16333..359b3eadd 100644 --- a/netlist.h +++ b/netlist.h @@ -728,7 +728,7 @@ class NetScope : public Attrib { bool in_func(); /* Is the task or function automatic. */ - void is_auto(bool is_auto) { is_auto_ = is_auto; }; + void is_auto(bool is_auto__) { is_auto_ = is_auto__; }; bool is_auto() const { return is_auto_; }; const NetTaskDef* task_def() const; diff --git a/vvp/array.cc b/vvp/array.cc index 936ca9c12..b480b6004 100644 --- a/vvp/array.cc +++ b/vvp/array.cc @@ -1026,7 +1026,7 @@ void array_word_change(vvp_array_t array, unsigned long addr) class array_port_resolv_list_t : public resolv_list_s { public: - explicit array_port_resolv_list_t(char*label) : resolv_list_s(label) { } + explicit array_port_resolv_list_t(char*lab) : resolv_list_s(lab) { } vvp_net_t*ptr; bool use_addr; @@ -1063,9 +1063,9 @@ void vpip_array_word_change(struct __vpiCallback*cb, vpiHandle obj) unsigned addr = decode_array_word_pointer(word, parent); cb->extra_data = addr; - } else if (struct __vpiArrayVthrA*word = array_vthr_a_from_handle(obj)) { - parent = word->array; - cb->extra_data = word->address; + } else if (struct __vpiArrayVthrA*tword = array_vthr_a_from_handle(obj)) { + parent = tword->array; + cb->extra_data = tword->address; } assert(parent); diff --git a/vvp/compile.cc b/vvp/compile.cc index 672b6e08e..28ce20ce4 100644 --- a/vvp/compile.cc +++ b/vvp/compile.cc @@ -425,7 +425,7 @@ void functor_ref_lookup(vvp_net_t**ref, char*lab) */ struct vpi_handle_resolv_list_s: public resolv_list_s { - explicit vpi_handle_resolv_list_s(char*label) : resolv_list_s(label) { } + explicit vpi_handle_resolv_list_s(char*lab) : resolv_list_s(lab) { } virtual bool resolve(bool mes); vpiHandle *handle; }; @@ -524,7 +524,7 @@ void compile_vpi_lookup(vpiHandle *handle, char*label) */ struct code_label_resolv_list_s: public resolv_list_s { - code_label_resolv_list_s(char*label) : resolv_list_s(label) { } + code_label_resolv_list_s(char*lab) : resolv_list_s(lab) { } struct vvp_code_s *code; virtual bool resolve(bool mes); }; @@ -557,7 +557,7 @@ void code_label_lookup(struct vvp_code_s *code, char *label) } struct code_array_resolv_list_s: public resolv_list_s { - code_array_resolv_list_s(char*label) : resolv_list_s(label) { } + code_array_resolv_list_s(char*lab) : resolv_list_s(lab) { } struct vvp_code_s *code; virtual bool resolve(bool mes); }; diff --git a/vvp/ufunc.cc b/vvp/ufunc.cc index 4570a0275..caa7d78de 100644 --- a/vvp/ufunc.cc +++ b/vvp/ufunc.cc @@ -37,7 +37,7 @@ ufunc_core::ufunc_core(unsigned owid, vvp_net_t*ptr, unsigned nports, vvp_net_t**ports, - vvp_code_t sa, struct __vpiScope*call_scope, + vvp_code_t sa, struct __vpiScope*call_scope__, char*result_label, char*scope_label) : vvp_wide_fun_core(ptr, nports) { @@ -45,7 +45,7 @@ ufunc_core::ufunc_core(unsigned owid, vvp_net_t*ptr, ports_ = ports; code_ = sa; thread_ = 0; - call_scope_ = call_scope; + call_scope_ = call_scope__; functor_ref_lookup(&result_, result_label); diff --git a/vvp/vthread.cc b/vvp/vthread.cc index 3bb7bd353..4a0b89249 100644 --- a/vvp/vthread.cc +++ b/vvp/vthread.cc @@ -3762,8 +3762,8 @@ bool of_POW(vthread_t thr, vvp_code_t cp) /* If we have an X or Z in the arguments return X. */ if (xv2.is_NaN() || yv2.is_NaN()) { - for (unsigned idx = 0 ; idx < wid ; idx += 1) - thr_put_bit(thr, cp->bit_idx[0]+idx, BIT4_X); + for (unsigned jdx = 0 ; jdx < wid ; jdx += 1) + thr_put_bit(thr, cp->bit_idx[0]+jdx, BIT4_X); return true; } @@ -3775,15 +3775,15 @@ bool of_POW(vthread_t thr, vvp_code_t cp) /* If the result is too small zero pad it. */ if (result.size() < wid) { - for (unsigned idx = wid-1; idx >= result.size(); idx -= 1) - thr_put_bit(thr, cp->bit_idx[0]+idx, BIT4_0); + for (unsigned jdx = wid-1; jdx >= result.size(); jdx -= 1) + thr_put_bit(thr, cp->bit_idx[0]+jdx, BIT4_0); wid = result.size(); } /* Copy only what we need of the result. */ - for (unsigned idx = 0; idx < wid; idx += 1) - thr_put_bit(thr, cp->bit_idx[0]+idx, - result.value(idx) ? BIT4_1 : BIT4_0); + for (unsigned jdx = 0; jdx < wid; jdx += 1) + thr_put_bit(thr, cp->bit_idx[0]+jdx, + result.value(jdx) ? BIT4_1 : BIT4_0); return true; } diff --git a/vvp/vvp_island.cc b/vvp/vvp_island.cc index c258309a6..411aadcda 100644 --- a/vvp/vvp_island.cc +++ b/vvp/vvp_island.cc @@ -237,6 +237,7 @@ void vvp_island::add_port(const char*key, vvp_net_t*net) void vvp_island::add_branch(vvp_island_branch*branch, const char*pa, const char*pb) { + vvp_island_branch*cur; assert(ports_); branch->a = ports_->sym_get_value(pa); branch->b = ports_->sym_get_value(pb); @@ -249,10 +250,10 @@ void vvp_island::add_branch(vvp_island_branch*branch, const char*pa, const char* if (bnodes_ == 0) bnodes_ = new symbol_map_s; - if (vvp_island_branch*cur = anodes_->sym_get_value(pa)) { + if ((cur = anodes_->sym_get_value(pa))) { branch->link[0] = cur->link[0]; cur->link[0] = ptra; - } else if (vvp_island_branch*cur = bnodes_->sym_get_value(pa)) { + } else if ((cur = bnodes_->sym_get_value(pa))) { branch->link[0] = cur->link[1]; cur->link[1] = ptra; } else { @@ -260,10 +261,10 @@ void vvp_island::add_branch(vvp_island_branch*branch, const char*pa, const char* anodes_->sym_set_value(pa, branch); } - if (vvp_island_branch*cur = anodes_->sym_get_value(pb)) { + if ((cur = anodes_->sym_get_value(pb))) { branch->link[1] = cur->link[0]; cur->link[0] = ptrb; - } else if (vvp_island_branch*cur = bnodes_->sym_get_value(pb)) { + } else if ((cur = bnodes_->sym_get_value(pb))) { branch->link[1] = cur->link[1]; cur->link[1] = ptrb; } else { diff --git a/vvp/vvp_net.cc b/vvp/vvp_net.cc index bff6765f3..b998e13b7 100644 --- a/vvp/vvp_net.cc +++ b/vvp/vvp_net.cc @@ -382,8 +382,8 @@ void vvp_vector4_t::allocate_words_(unsigned wid, unsigned long inita, unsigned } } -vvp_vector4_t::vvp_vector4_t(unsigned size, double val) -: size_(size) +vvp_vector4_t::vvp_vector4_t(unsigned size__, double val) +: size_(size__) { bool is_neg = false; double fraction; @@ -391,7 +391,7 @@ vvp_vector4_t::vvp_vector4_t(unsigned size, double val) /* We return 'bx for a NaN or +/- infinity. */ if (val != val || (val && (val == 0.5*val))) { - allocate_words_(size, WORD_X_ABITS, WORD_X_BBITS); + allocate_words_(size_, WORD_X_ABITS, WORD_X_BBITS); return; } @@ -400,7 +400,7 @@ vvp_vector4_t::vvp_vector4_t(unsigned size, double val) is_neg = true; val = -val; } - allocate_words_(size, WORD_0_ABITS, WORD_0_BBITS); + allocate_words_(size_, WORD_0_ABITS, WORD_0_BBITS); /* Get the exponent and fractional part of the number. */ fraction = frexp(val, &exponent); @@ -1298,9 +1298,9 @@ bool vector4_to_value(const vvp_vector4_t&vec, double&val, bool signed_flag) return flag; } -vvp_vector4array_t::vvp_vector4array_t(unsigned width, unsigned words, +vvp_vector4array_t::vvp_vector4array_t(unsigned width__, unsigned words__, bool is_automatic) -: width_(width), words_(words), array_(0) +: width_(width__), words_(words__), array_(0) { if (is_automatic) return; diff --git a/vvp/vvp_net.h b/vvp/vvp_net.h index d5d40fbe4..24b888c85 100644 --- a/vvp/vvp_net.h +++ b/vvp/vvp_net.h @@ -289,8 +289,8 @@ inline vvp_vector4_t::vvp_vector4_t(const vvp_vector4_t&that) copy_from_(that); } -inline vvp_vector4_t::vvp_vector4_t(unsigned size, vvp_bit4_t val) -: size_(size) +inline vvp_vector4_t::vvp_vector4_t(unsigned size__, vvp_bit4_t val) +: size_(size__) { /* note: this relies on the bit encoding for the vvp_bit4_t. */ const static unsigned long init_atable[4] = { @@ -304,7 +304,7 @@ inline vvp_vector4_t::vvp_vector4_t(unsigned size, vvp_bit4_t val) WORD_Z_BBITS, WORD_X_BBITS }; - allocate_words_(size, init_atable[val], init_btable[val]); + allocate_words_(size_, init_atable[val], init_btable[val]); } inline vvp_vector4_t::~vvp_vector4_t() @@ -750,8 +750,8 @@ extern vvp_vector8_t part_expand(const vvp_vector8_t&a, unsigned wid, unsigned o /* Print a vector8 value to a stream. */ extern ostream& operator<< (ostream&, const vvp_vector8_t&); -inline vvp_vector8_t::vvp_vector8_t(unsigned size) -: size_(size) +inline vvp_vector8_t::vvp_vector8_t(unsigned size__) +: size_(size__) { if (size_ <= PTR_THRESH) { new (val_) vvp_scalar_t[PTR_THRESH]; @@ -822,12 +822,12 @@ template class vvp_sub_pointer_t { public: vvp_sub_pointer_t() : bits_(0) { } - vvp_sub_pointer_t(T*ptr, unsigned port) + vvp_sub_pointer_t(T*ptr__, unsigned port__) { - bits_ = reinterpret_cast (ptr); - assert( (bits_ & 3) == 0 ); - assert( (port & ~3) == 0 ); - bits_ |= port; + bits_ = reinterpret_cast (ptr__); + assert( (bits_ & 3) == 0 ); + assert( (port__ & ~3) == 0 ); + bits_ |= port__; } ~vvp_sub_pointer_t() { }