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.
This commit is contained in:
Martin Whitaker 2008-10-04 15:30:34 +01:00 committed by Stephen Williams
parent fd4018cb33
commit 082e06edb0
11 changed files with 85 additions and 280 deletions

155
PExpr.cc
View File

@ -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<PExpr*>&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<perm_string,Module::param_expr_t>::const_iterator cur;
cur = mod->parameters.find(tmp);
if (cur != mod->parameters.end()) return true;
}
{ map<perm_string,Module::param_expr_t>::const_iterator cur;
cur = mod->localparams.find(tmp);
if (cur != mod->localparams.end()) return true;
}
{ map<perm_string,LineInfo*>::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);
}

23
PExpr.h
View File

@ -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,

View File

@ -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)
{
}

View File

@ -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;

View File

@ -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;

View File

@ -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

View File

@ -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<NetEConst*>(rv)) return rv;
if (dynamic_cast<NetECReal*>(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;
}
/*

View File

@ -53,14 +53,19 @@ attrib_list_t* evaluate_attributes(const map<perm_string,PExpr*>&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<perm_string,PExpr*>&att,
* of complex attributes attached to gates.
*
*/

57
parse.y
View File

@ -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<PExpr*>*tmp = new svector<PExpr*> (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<index_component_t> *tmp = new list<index_component_t>;
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<index_component_t> *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;
}

View File

@ -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);

View File

@ -316,8 +316,6 @@ extern PProcess* pform_make_behavior(PProcess::Type, Statement*,
extern svector<PWire*>* pform_make_udp_input_ports(list<perm_string>*);
extern bool pform_expression_is_constant(const PExpr*);
extern void pform_make_events(list<perm_string>*names,
const char*file, unsigned lineno);
/*