From d1ce6d25356f774e76fb46e17834e438b8075a65 Mon Sep 17 00:00:00 2001 From: Stephen Williams Date: Thu, 18 Dec 2008 21:33:31 -0800 Subject: [PATCH] Fix the signed-ness calculations of +- in parameter expressions. This fixes up the elaboration of binary expressions found in parameter expressions. Parameter expressions are special because they elaborate early, before all the other parameters are necessarily completed. --- PExpr.h | 17 +++++++++------ compiler.h | 1 + elab_expr.cc | 26 +++++++++++----------- elab_pexpr.cc | 48 ++++++++++++++++++++++++++++++++++++++--- eval_tree.cc | 9 +++++++- main.cc | 4 ++++ net_design.cc | 2 ++ net_expr.cc | 2 ++ netlist.cc | 1 + netlist.h | 12 ++++++++++- netmisc.cc | 3 +++ netmisc.h | 7 ++++++ scripts/devel-stub.conf | 1 + 13 files changed, 109 insertions(+), 24 deletions(-) diff --git a/PExpr.h b/PExpr.h index 3f85f7932..27d7f259d 100644 --- a/PExpr.h +++ b/PExpr.h @@ -143,8 +143,6 @@ class PExpr : public LineInfo { ivl_variable_type_t expr_type_; unsigned expr_width_; - static void suppress_binary_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp); - private: // not implemented PExpr(const PExpr&); PExpr& operator= (const PExpr&); @@ -484,15 +482,20 @@ class PEBinary : public PExpr { PExpr*left_; PExpr*right_; - NetExpr*elaborate_expr_base_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; - NetExpr*elaborate_eval_expr_base_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; + NetExpr*elaborate_expr_base_(Design*, NetExpr*lp, NetExpr*rp, + int use_wid, bool is_pexpr =false) const; + NetExpr*elaborate_eval_expr_base_(Design*, NetExpr*lp, NetExpr*rp, + int use_wid) const; NetExpr*elaborate_expr_base_bits_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; - NetExpr*elaborate_expr_base_div_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; + NetExpr*elaborate_expr_base_div_(Design*, NetExpr*lp, NetExpr*rp, + int use_wid, bool is_pexpr) const; NetExpr*elaborate_expr_base_lshift_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; NetExpr*elaborate_expr_base_rshift_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; - NetExpr*elaborate_expr_base_mult_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; - NetExpr*elaborate_expr_base_add_(Design*, NetExpr*lp, NetExpr*rp, int use_wid) const; + NetExpr*elaborate_expr_base_mult_(Design*, NetExpr*lp, NetExpr*rp, + int use_wid, bool is_pexpr) const; + NetExpr*elaborate_expr_base_add_(Design*, NetExpr*lp, NetExpr*rp, + int use_wid, bool is_pexpr) const; }; diff --git a/compiler.h b/compiler.h index ba0d371f6..2caed586c 100644 --- a/compiler.h +++ b/compiler.h @@ -85,6 +85,7 @@ extern bool verbose_flag; extern bool debug_scopes; extern bool debug_eval_tree; extern bool debug_elaborate; +extern bool debug_elab_pexpr; extern bool debug_synth2; extern bool debug_optimizer; extern bool debug_automatic; diff --git a/elab_expr.cc b/elab_expr.cc index 39360ab30..1aba390a9 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -274,7 +274,7 @@ NetExpr* PEBinary::elaborate_expr(Design*des, NetScope*scope, return tmp; } -void PExpr::suppress_binary_operand_sign_if_needed_(NetExpr*lp, NetExpr*rp) +void suppress_binary_operand_sign_if_needed(NetExpr*lp, NetExpr*rp) { // If an argument is a non-vector type, then this type // suppression does not apply. @@ -315,7 +315,7 @@ NetExpr* PEBinary::elaborate_eval_expr_base_(Design*des, */ NetExpr* PEBinary::elaborate_expr_base_(Design*des, NetExpr*lp, NetExpr*rp, - int expr_wid) const + int expr_wid, bool is_pexpr) const { if (debug_elaborate) { cerr << get_fileline() << ": debug: elaborate expression " @@ -345,12 +345,12 @@ NetExpr* PEBinary::elaborate_expr_base_(Design*des, break; case '*': - tmp = elaborate_expr_base_mult_(des, lp, rp, expr_wid); + tmp = elaborate_expr_base_mult_(des, lp, rp, expr_wid, is_pexpr); break; case '%': case '/': - tmp = elaborate_expr_base_div_(des, lp, rp, expr_wid); + tmp = elaborate_expr_base_div_(des, lp, rp, expr_wid, is_pexpr); break; case 'l': @@ -374,7 +374,7 @@ NetExpr* PEBinary::elaborate_expr_base_(Design*des, case '+': case '-': - tmp = elaborate_expr_base_add_(des, lp, rp, expr_wid); + tmp = elaborate_expr_base_add_(des, lp, rp, expr_wid, is_pexpr); break; case 'E': /* === */ @@ -429,7 +429,7 @@ NetExpr* PEBinary::elaborate_expr_base_bits_(Design*des, NetExpr* PEBinary::elaborate_expr_base_div_(Design*des, NetExpr*lp, NetExpr*rp, - int expr_wid) const + int expr_wid, bool is_pexpr) const { /* The % operator does not support real arguments in baseline Verilog. But we allow it in our extended @@ -670,7 +670,7 @@ NetExpr* PEBinary::elaborate_expr_base_rshift_(Design*des, NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des, NetExpr*lp, NetExpr*rp, - int expr_wid) const + int expr_wid, bool is_pexpr) const { // First, Make sure that signed arguments are padded to the // width of the output. This is necessary for 2s complement @@ -711,7 +711,8 @@ NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des, // If this expression is unsigned, then make sure the // arguments are unsigned so that the padding below doesn't // cause any sign extension to happen. - suppress_binary_operand_sign_if_needed_(lp, rp); + if (! is_pexpr) + suppress_binary_operand_sign_if_needed(lp, rp); // Multiply will guess a width that is the sum of the @@ -732,7 +733,7 @@ NetExpr* PEBinary::elaborate_expr_base_mult_(Design*des, NetExpr* PEBinary::elaborate_expr_base_add_(Design*des, NetExpr*lp, NetExpr*rp, - int expr_wid) const + int expr_wid, bool is_pexpr) const { NetExpr*tmp; bool use_lossless_flag = expr_wid == -2; @@ -748,7 +749,8 @@ NetExpr* PEBinary::elaborate_expr_base_add_(Design*des, // If the expression is unsigned, then force the operands to // unsigned so taht the set_width below doesn't cause them to // be sign-extended. - suppress_binary_operand_sign_if_needed_(lp, rp); + if (! is_pexpr) + suppress_binary_operand_sign_if_needed(lp, rp); tmp = new NetEBAdd(op_, lp, rp, use_lossless_flag); if (expr_wid > 0 && type_is_vectorable(tmp->expr_type())) @@ -811,7 +813,7 @@ NetExpr* PEBComp::elaborate_expr(Design*des, NetScope*scope, return 0; } - suppress_binary_operand_sign_if_needed_(lp, rp); + suppress_binary_operand_sign_if_needed(lp, rp); // The arguments of a compare need to have matching widths, so // pad the width here. This matters because if the arguments @@ -2970,7 +2972,7 @@ NetExpr*PETernary::elaborate_expr(Design*des, NetScope*scope, return 0; } - suppress_binary_operand_sign_if_needed_(tru, fal); + suppress_binary_operand_sign_if_needed(tru, fal); /* Whatever the width we choose for the ternary operator, we need to make sure the operands match. */ diff --git a/elab_pexpr.cc b/elab_pexpr.cc index e30eaac64..4c0fbe195 100644 --- a/elab_pexpr.cc +++ b/elab_pexpr.cc @@ -53,7 +53,7 @@ NetExpr*PEBinary::elaborate_pexpr (Design*des, NetScope*scope) const return 0; } - NetExpr*tmp = elaborate_expr_base_(des, lp, rp, -2); + NetExpr*tmp = elaborate_expr_base_(des, lp, rp, -2, true); return tmp; } @@ -67,8 +67,6 @@ NetExpr*PEBComp::elaborate_pexpr(Design*des, NetScope*scope) const return 0; } - suppress_binary_operand_sign_if_needed_(lp, rp); - NetEBComp*tmp = new NetEBComp(op_, lp, rp); tmp->set_line(*this); bool flag = tmp->set_width(1); @@ -452,3 +450,47 @@ NetExpr* PECallFunction::elaborate_pexpr(Design*des, NetScope*scope) const des->errors += 1; return 0; } + +void NetExpr::resolve_pexpr_type(void) +{ +} + +void NetEBinary::resolve_pexpr_type(void) +{ + if (debug_elab_pexpr) { + cerr << get_fileline() << ": debug: " + << "Resolve_pexpr_type for binary " << human_readable_op(op_) + << "." << endl; + } + + left_->resolve_pexpr_type(); + right_->resolve_pexpr_type(); + + switch (op_) { + case '+': + case '-': + case '*': + case '/': + suppress_binary_operand_sign_if_needed(left_, right_); + cast_signed_base_(left_->has_sign() && right_->has_sign()); + break; + default: + break; + } +} + +void NetEParam::resolve_pexpr_type(void) +{ + if (debug_elab_pexpr) { + cerr << get_fileline() << ": debug: " + << "Resolve_pexpr_type for parameter " << reference_->first + << "." << endl; + } + + if (reference_->second.signed_flag) { + cast_signed_base_(true); + + } else { + cast_signed_base_( reference_->second.expr->has_sign() ); + } +} diff --git a/eval_tree.cc b/eval_tree.cc index f4f5c93aa..c16bddb08 100644 --- a/eval_tree.cc +++ b/eval_tree.cc @@ -129,7 +129,14 @@ NetExpr* NetEBAdd::eval_tree(int prune_to_width) lc = se? dynamic_cast(se->right_) : 0; if (lc != 0 && rc != 0) { - assert(se != 0); + ivl_assert(*this, se != 0); + + if (debug_eval_tree) { + cerr << get_fileline() << ": debug: " + << "Partially evalutate " << *this + << " using (a+2)-1 --> (a+1) transform." << endl; + } + verinum lval = lc->value(); verinum rval = rc->value(); diff --git a/main.cc b/main.cc index 05e75afa7..0b67231e6 100644 --- a/main.cc +++ b/main.cc @@ -122,6 +122,7 @@ bool error_implicit = false; bool debug_scopes = false; bool debug_eval_tree = false; bool debug_elaborate = false; +bool debug_elab_pexpr = false; bool debug_synth2 = false; bool debug_optimizer = false; bool debug_automatic = false; @@ -387,6 +388,9 @@ static void read_iconfig_file(const char*ipath) } else if (strcmp(cp,"elaborate") == 0) { debug_elaborate = true; cerr << "debug: Enable elaborate debug" << endl; + } else if (strcmp(cp,"elab_pexpr") == 0) { + debug_elab_pexpr = true; + cerr << "debug: Enable elaborate-pexpr debug" << endl; } else if (strcmp(cp,"synth2") == 0) { debug_synth2 = true; cerr << "debug: Enable synth2 debug" << endl; diff --git a/net_design.cc b/net_design.cc index 3d7e2f275..dc7723499 100644 --- a/net_design.cc +++ b/net_design.cc @@ -590,6 +590,8 @@ void NetScope::evaluate_parameters(Design*des) for (param_ref_t cur = parameters.begin() ; cur != parameters.end() ; cur ++) { + cur->second.expr->resolve_pexpr_type(); + switch ((*cur).second.type) { case IVL_VT_BOOL: case IVL_VT_LOGIC: diff --git a/net_expr.cc b/net_expr.cc index e923edbe3..dee99b45d 100644 --- a/net_expr.cc +++ b/net_expr.cc @@ -494,11 +494,13 @@ NetEParam::NetEParam() NetEParam::NetEParam(Design*d, NetScope*s, perm_string n) : des_(d), scope_(s), reference_(scope_->find_parameter(n)) { + cast_signed_base_(reference_->second.signed_flag); } NetEParam::NetEParam(Design*d, NetScope*s, ref_t ref) : des_(d), scope_(s), reference_(ref) { + cast_signed_base_(reference_->second.signed_flag); } NetEParam::~NetEParam() diff --git a/netlist.cc b/netlist.cc index c950e5f38..16b262205 100644 --- a/netlist.cc +++ b/netlist.cc @@ -2154,6 +2154,7 @@ ivl_variable_type_t NetEConst::expr_type() const NetEConstParam::NetEConstParam(NetScope*s, perm_string n, const verinum&v) : NetEConst(v), scope_(s), name_(n) { + cast_signed_base_(v.has_sign()); } NetEConstParam::~NetEConstParam() diff --git a/netlist.h b/netlist.h index 34df32771..38295a788 100644 --- a/netlist.h +++ b/netlist.h @@ -817,7 +817,7 @@ class NetScope : public Attrib { /* After everything is all set up, the code generators like access to these things to make up the parameter lists. */ struct param_expr_t : public LineInfo { - param_expr_t() : type(IVL_VT_NO_TYPE), range(0) { } + param_expr_t() : type(IVL_VT_NO_TYPE), signed_flag(false), msb(0), lsb(0), range(0), expr(0) { } // Type information ivl_variable_type_t type; bool signed_flag; @@ -1562,6 +1562,14 @@ class NetExpr : public LineInfo { // expressions to check validity. virtual bool has_width() const; + // Expressions in parameter declarations may have encountered + // arguments that are themselves untyped parameters. These + // cannot be fully resolved for type when elaborated (they are + // elaborated before all parameter overrides are complete) so + // this virtual method needs to be called right before + // evaluating the expression. This wraps up the evaluation of + // the type. + virtual void resolve_pexpr_type(); // This method evaluates the expression and returns an // equivalent expression that is reduced as far as compile @@ -3201,6 +3209,7 @@ class NetEBinary : public NetExpr { // widths. virtual bool has_width() const; + virtual void resolve_pexpr_type(); virtual NetEBinary* dup_expr() const; virtual NexusSet* nex_input(bool rem_out = true); @@ -3499,6 +3508,7 @@ class NetEParam : public NetExpr { ~NetEParam(); virtual NexusSet* nex_input(bool rem_out = true); + virtual void resolve_pexpr_type(); virtual bool set_width(unsigned w, bool last_chance); virtual bool has_width() const; virtual void expr_scan(struct expr_scan_t*) const; diff --git a/netmisc.cc b/netmisc.cc index 4223a0399..2a6540849 100644 --- a/netmisc.cc +++ b/netmisc.cc @@ -387,6 +387,9 @@ const char *human_readable_op(const char op) switch (op) { case '~': type = "~"; break; // Negation + case '+': type = "+"; break; + case '-': type = "-"; break; + case '^': type = "^"; break; // XOR case 'X': type = "~^"; break; // XNOR case '&': type = "&"; break; // Bitwise AND diff --git a/netmisc.h b/netmisc.h index d5ff896ae..d8e8be9e1 100644 --- a/netmisc.h +++ b/netmisc.h @@ -166,6 +166,13 @@ void probe_expr_width(Design*des, NetScope*scope, PExpr*pe); extern NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, ivl_variable_type_t data_type_lv, int expr_wid_lv, PExpr*expr); + +/* + * Used by elaboration to suppress the sign of an operand if the other + * is unsigned. + */ +extern void suppress_binary_operand_sign_if_needed(NetExpr*lp, NetExpr*rp); + /* * This procedure elaborates an expression and if the elaboration is * successful the original expression is replaced with the new one. diff --git a/scripts/devel-stub.conf b/scripts/devel-stub.conf index e3fecf5da..e15a73645 100644 --- a/scripts/devel-stub.conf +++ b/scripts/devel-stub.conf @@ -16,6 +16,7 @@ sys_func:vpi/va_math.sft warnings:implicit debug:eval_tree debug:elaborate +debug:elab_pexpr debug:scopes debug:synth2 out:a.out