From 30ff2aa5d1136201bcebee19ec29f21475a8a334 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 23 Dec 2021 14:17:13 +0100 Subject: [PATCH 1/3] Support full set of constant expressions in attributes There are currently two different systems to evaluate constant expressions in iverilog. The PExpr based system using the eval_const() method and the NetExpr based system using the eval_tree() method. The latter is more complete while the former only implements the bare minimum and also has some minor bugs. The PExpr based system is only used to evaluate expressions within attributes. Switch attribute expression evaluation over to elab_and_eval(). This enables to use the full set of constant expressions for attributes, maybe most importantly constant functions and system math functions. It also allows to remove the PExpr based system since there are no more users. Signed-off-by: Lars-Peter Clausen --- eval_attrib.cc | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/eval_attrib.cc b/eval_attrib.cc index 2c0d7b3c7..527df43ad 100644 --- a/eval_attrib.cc +++ b/eval_attrib.cc @@ -21,6 +21,7 @@ # include "util.h" # include "PExpr.h" # include "netlist.h" +# include "netmisc.h" # include # include @@ -52,23 +53,25 @@ 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 = 0; if (exp) { - tmp = exp->eval_const(des, scope); - if (tmp == 0) { + NetExpr *tmp = elab_and_eval(des, scope, exp, -1, true); + if (!tmp) + continue; + + if (NetEConst *ce = dynamic_cast(tmp)) { + table[idx].val = ce->value(); + } else if (NetECReal *cer = dynamic_cast(tmp)) { + table[idx].val = verinum(cer->value().as_long()); + } else { cerr << exp->get_fileline() << ": error: ``" << *exp << "'' is not a constant expression." << endl; des->errors += 1; - } - } - if (tmp == 0) - tmp = new verinum(1); - - assert(tmp); - - table[idx].val = *tmp; - delete tmp; + } + delete tmp; + } else { + table[idx].val = verinum(1); + } } assert(idx == natt); From 497c98bf9b6ede6cfb664e1a837f9d753b1bbb3d Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 23 Dec 2021 14:16:25 +0100 Subject: [PATCH 2/3] Remove eval_const() There are no more users of the eval_const() system. Lets remove it. Signed-off-by: Lars-Peter Clausen --- Makefile.in | 2 +- PExpr.h | 19 ---- eval.cc | 282 ---------------------------------------------------- 3 files changed, 1 insertion(+), 302 deletions(-) delete mode 100644 eval.cc diff --git a/Makefile.in b/Makefile.in index 62062f65c..3f0c00377 100644 --- a/Makefile.in +++ b/Makefile.in @@ -106,7 +106,7 @@ FF = cprop.o exposenodes.o nodangle.o synth.o synth2.o syn-rules.o O = main.o async.o design_dump.o discipline.o dup_expr.o elaborate.o \ elab_expr.o elaborate_analog.o elab_lval.o elab_net.o \ elab_scope.o elab_sig.o elab_sig_analog.o elab_type.o \ - emit.o eval.o eval_attrib.o \ + emit.o eval_attrib.o \ eval_tree.o expr_synth.o functor.o lexor.o lexor_keyword.o link_const.o \ load_module.o netlist.o netmisc.o nettypes.o net_analog.o net_assign.o \ net_design.o netclass.o netdarray.o \ diff --git a/PExpr.h b/PExpr.h index 93ddae44d..70af29613 100644 --- a/PExpr.h +++ b/PExpr.h @@ -173,11 +173,6 @@ class PExpr : public LineInfo { bool is_cassign, bool is_force) const; - // This attempts to evaluate a constant expression, and return - // a verinum as a result. If the expression cannot be - // evaluated, return 0. - virtual verinum* eval_const(Design*des, NetScope*sc) const; - // This method returns true if the expression represents a // structural net that can have multiple drivers. This is // used to test whether an input port connection can be @@ -235,7 +230,6 @@ class PEConcat : public PExpr { explicit PEConcat(const std::list&p, PExpr*r =0); ~PEConcat(); - virtual verinum* eval_const(Design*des, NetScope*sc) const; virtual void dump(std::ostream&) const; virtual void declare_implicit_nets(LexicalScope*scope, NetNet::Type type); @@ -312,11 +306,6 @@ class PEFNumber : public PExpr { const verireal& value() const; - /* The eval_const method as applied to a floating point number - gets the *integer* value of the number. This accounts for - any rounding that is needed to get the value. */ - virtual verinum* eval_const(Design*des, NetScope*sc) const; - virtual unsigned test_width(Design*des, NetScope*scope, width_mode_t&mode); virtual NetExpr*elaborate_expr(Design*des, NetScope*, @@ -378,8 +367,6 @@ class PEIdent : public PExpr { // expressions can can be unpacked arrays. NetNet* elaborate_unpacked_net(Design*des, NetScope*sc) const; - verinum* eval_const(Design*des, NetScope*sc) const; - virtual bool is_collapsible_net(Design*des, NetScope*scope, NetNet::PortType port_type) const; @@ -635,8 +622,6 @@ class PENumber : public PExpr { bool is_cassign, bool is_force) const; - virtual verinum* eval_const(Design*des, NetScope*sc) const; - virtual bool is_the_same(const PExpr*that) const; private: @@ -667,7 +652,6 @@ class PEString : public PExpr { virtual NetEConst*elaborate_expr(Design*des, NetScope*, unsigned expr_wid, unsigned) const; - verinum* eval_const(Design*, NetScope*) const; private: char*text_; @@ -708,7 +692,6 @@ class PEUnary : public PExpr { virtual NetExpr*elaborate_expr(Design*des, NetScope*, unsigned expr_wid, unsigned flags) const; - virtual verinum* eval_const(Design*des, NetScope*sc) const; public: inline char get_op() const { return op_; } @@ -740,7 +723,6 @@ class PEBinary : public PExpr { virtual NetExpr*elaborate_expr(Design*des, NetScope*, unsigned expr_wid, unsigned flags) const; - virtual verinum* eval_const(Design*des, NetScope*sc) const; protected: char op_; @@ -865,7 +847,6 @@ class PETernary : public PExpr { virtual NetExpr*elaborate_expr(Design*des, NetScope*, unsigned expr_wid, unsigned flags) const; - virtual verinum* eval_const(Design*des, NetScope*sc) const; private: NetExpr* elab_and_eval_alternative_(Design*des, NetScope*scope, diff --git a/eval.cc b/eval.cc deleted file mode 100644 index 3b29d46e5..000000000 --- a/eval.cc +++ /dev/null @@ -1,282 +0,0 @@ -/* - * Copyright (c) 1998-2021 Stephen Williams (steve@icarus.com) - * - * This source code is free software; you can redistribute it - * and/or modify it in source code form under the terms of the GNU - * General Public License as published by the Free Software - * Foundation; either version 2 of the License, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -# include "config.h" - -# include -# include - -# include "PExpr.h" -# include "netlist.h" -# include "netmisc.h" -# include "compiler.h" - -using namespace std; - -verinum* PExpr::eval_const(Design*, NetScope*) const -{ - return 0; -} - -verinum* PEBinary::eval_const(Design*des, NetScope*scope) const -{ - verinum*l = left_->eval_const(des, scope); - if (l == 0) return 0; - verinum*r = right_->eval_const(des, scope); - if (r == 0) { - delete l; - return 0; - } - verinum*res; - - switch (op_) { - case 'p': { - if (l->is_defined() && r->is_defined()) { - res = new verinum(pow(*l, *r)); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '+': { - if (l->is_defined() && r->is_defined()) { - res = new verinum(*l + *r); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '-': { - if (l->is_defined() && r->is_defined()) { - res = new verinum(*l - *r); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '*': { - if (l->is_defined() && r->is_defined()) { - res = new verinum(*l * *r); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '/': { - if (l->is_defined() && r->is_defined()) { - long lv = l->as_long(); - long rv = r->as_long(); - res = new verinum(lv / rv, l->len()); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '%': { - if (l->is_defined() && r->is_defined()) { - long lv = l->as_long(); - long rv = r->as_long(); - res = new verinum(lv % rv, l->len()); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '>': { - if (l->is_defined() && r->is_defined()) { - long lv = l->as_long(); - long rv = r->as_long(); - res = new verinum(lv > rv, l->len()); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case '<': { - if (l->is_defined() && r->is_defined()) { - long lv = l->as_long(); - long rv = r->as_long(); - res = new verinum(lv < rv, l->len()); - } else { - res = new verinum(verinum::Vx, l->len()); - } - break; - } - case 'l': { // left shift (<<) - assert(r->is_defined()); - unsigned long rv = r->as_ulong(); - res = new verinum(verinum::V0, l->len()); - if (rv < res->len()) { - unsigned cnt = res->len() - rv; - for (unsigned idx = 0 ; idx < cnt ; idx += 1) - res->set(idx+rv, l->get(idx)); - } - break; - } - case 'r': { // right shift (>>) - assert(r->is_defined()); - unsigned long rv = r->as_ulong(); - res = new verinum(verinum::V0, l->len()); - if (rv < res->len()) { - unsigned cnt = res->len() - rv; - for (unsigned idx = 0 ; idx < cnt ; idx += 1) - res->set(idx, l->get(idx+rv)); - } - break; - } - - default: - delete l; - delete r; - return 0; - } - - delete l; - delete r; - return res; -} -verinum* PEConcat::eval_const(Design*des, NetScope*scope) const -{ - verinum*accum = parms_[0]->eval_const(des, scope); - if (accum == 0) - return 0; - - for (unsigned idx = 1 ; idx < parms_.size() ; idx += 1) { - - verinum*tmp = parms_[idx]->eval_const(des, scope); - if (tmp == 0) { - delete accum; - return 0; - } - assert(tmp); - - *accum = concat(*accum, *tmp); - delete tmp; - } - - return accum; -} - - -/* - * Evaluate an identifier as a constant expression. This is only - * possible if the identifier is that of a parameter. - */ -verinum* PEIdent::eval_const(Design*des, NetScope*scope) const -{ - assert(scope); - NetNet*net; - NetEvent*eve; - const NetExpr*expr; - - const name_component_t&name_tail = path_.back(); - - // Handle the special case that this ident is a genvar - // variable name. In that case, the genvar meaning preempts - // everything and we just return that value immediately. - if (scope->genvar_tmp - && strcmp(name_tail.name,scope->genvar_tmp) == 0) { - return new verinum(scope->genvar_tmp_val); - } - - symbol_search(this, des, scope, path_, net, expr, eve); - - if (expr == 0) - return 0; - - const NetEConst*eval = dynamic_cast(expr); - if (eval == 0) { - cerr << get_fileline() << ": internal error: Unable to evaluate " - << "constant expression (parameter=" << path_ - << "): " << *expr << endl; - return 0; - } - - assert(eval); - - if (!name_tail.index.empty()) - return 0; - - - return new verinum(eval->value()); -} - -verinum* PEFNumber::eval_const(Design*, NetScope*) const -{ - long val = value_->as_long(); - return new verinum(val); -} - -verinum* PENumber::eval_const(Design*, NetScope*) const -{ - return new verinum(value()); -} - -verinum* PEString::eval_const(Design*, NetScope*) const -{ - return new verinum(string(text_)); -} - -verinum* PETernary::eval_const(Design*des, NetScope*scope) const -{ - verinum*test = expr_->eval_const(des, scope); - if (test == 0) - return 0; - - verinum::V bit = test->get(0); - delete test; - switch (bit) { - case verinum::V0: - return fal_->eval_const(des, scope); - case verinum::V1: - return tru_->eval_const(des, scope); - default: - return 0; - // XXXX It is possible to handle this case if both fal_ - // and tru_ are constant. Someday... - } -} - -verinum* PEUnary::eval_const(Design*des, NetScope*scope) const -{ - verinum*val = expr_->eval_const(des, scope); - if (val == 0) - return 0; - - switch (op_) { - case '+': - return val; - - case '-': { - /* We need to expand the value a bit if we are - taking the 2's complement so that we are - guaranteed to not overflow. */ - verinum tmp ((uint64_t)0, val->len()+1); - for (unsigned idx = 0 ; idx < val->len() ; idx += 1) - tmp.set(idx, val->get(idx)); - - *val = -tmp; - val->has_sign(true); - return val; - } - - } - delete val; - return 0; -} From 481f461e59ab2bbc96b30f02699898409e9e94ff Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 12 Feb 2022 17:33:02 +0100 Subject: [PATCH 3/3] Add regression test for expressions in attributes Check that all types of constant expressions are supported in attributes. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/attrib_expr.v | 81 +++++++++++++++++++++++++++++++++++ ivtest/regress-vlg.list | 1 + 2 files changed, 82 insertions(+) create mode 100644 ivtest/ivltests/attrib_expr.v diff --git a/ivtest/ivltests/attrib_expr.v b/ivtest/ivltests/attrib_expr.v new file mode 100644 index 000000000..8f6c1d636 --- /dev/null +++ b/ivtest/ivltests/attrib_expr.v @@ -0,0 +1,81 @@ +// Check that all types of constant expression are supported for attributes + +module test; + +localparam [7:0] x = 1; + +// Binary operators +(* attr = x + x *) reg attr0; +(* attr = x - x *) reg attr1; +(* attr = x * x *) reg attr2; +(* attr = x / x *) reg attr3; +(* attr = x % x *) reg attr4; +(* attr = x == x *) reg attr5; +(* attr = x != x *) reg attr6; +(* attr = x === x *) reg attr7; +(* attr = x !== x *) reg attr8; +(* attr = x && x *) reg attr9; +(* attr = x || x *) reg attr10; +(* attr = x ** x *) reg attr11; +(* attr = x < x *) reg attr12; +(* attr = x <= x *) reg attr13; +(* attr = x > x *) reg attr14; +(* attr = x >= x *) reg attr15; +(* attr = x & x *) reg attr16; +(* attr = x | x *) reg attr17; +(* attr = x ^ x *) reg attr18; +(* attr = x ^~ x *) reg attr19; +(* attr = x >> x *) reg attr20; +(* attr = x << x *) reg attr21; +(* attr = x >>> x *) reg attr22; +(* attr = x <<< x *) reg attr23; + +// Unary operators +(* attr = +x *) reg attr24; +(* attr = -x *) reg attr25; +(* attr = !x *) reg attr26; +(* attr = ~x *) reg attr27; +(* attr = &x *) reg attr28; +(* attr = ~&x *) reg attr29; +(* attr = |x *) reg attr30; +(* attr = ~|x *) reg attr31; +(* attr = ^x *) reg attr32; +(* attr = ~^x *) reg attr33; + +// Ternary operator +(* attr = x ? x : x *) reg attr34; + +// Concat +(* attr = {x,x} *) reg attr35; +(* attr = {3{x}} *) reg attr36; + +// Part select +(* attr = x[0] *) reg attr37; +(* attr = x[1:0] *) reg attr38; +(* attr = x[0+:1] *) reg attr39; +(* attr = x[1-:1] *) reg attr40; + +// Parenthesis +(* attr = (x) *) reg attr41; + +// Literals +(* attr = 10 *) reg attr42; +(* attr = 32'h20 *) reg attr43; +(* attr = "test" *) reg attr44; + +// System function +(* attr = $clog2(10) *) reg attr45; + +// Function +function fn; + input x; + fn = x*2; +endfunction + +(* attr = fn(10) *) reg attr46; + +initial begin + $display("PASSED"); +end + +endmodule diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index 0b78224c5..3a0a70e24 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -183,6 +183,7 @@ attrib06_operator_suffix normal ivltests attrib07_func_call normal ivltests attrib08_mod_inst normal ivltests attrib09_case normal ivltests +attrib_expr normal ivltests automatic_error1 CE ivltests automatic_error2 CE ivltests automatic_error3 CE ivltests