From 4625e7e2b65de1650e19ba9f941fc7b36c9564a7 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Mon, 28 Oct 2013 22:07:09 +0000 Subject: [PATCH] Improvements to strict-expr-width mode. Enable error reporting when an unsized number is used in a concatenation operand. Allow greater pruning of expressions containing unsized numbers. --- PExpr.cc | 6 ++++-- PExpr.h | 42 ++++++++++++++++++++++++---------------- elab_expr.cc | 54 +++++++++++++++++++++++++++++----------------------- netmisc.cc | 2 +- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/PExpr.cc b/PExpr.cc index 8365720ef..538f4f37f 100644 --- a/PExpr.cc +++ b/PExpr.cc @@ -83,12 +83,14 @@ const char* PExpr::width_mode_name(width_mode_t mode) switch (mode) { case PExpr::SIZED: return "sized"; + case PExpr::UNSIZED: + return "unsized"; case PExpr::EXPAND: return "expand"; case PExpr::LOSSLESS: return "lossless"; - case PExpr::UNSIZED: - return "unsized"; + case PExpr::UPSIZE: + return "upsize"; default: return "??"; } diff --git a/PExpr.h b/PExpr.h index ea4b20ecc..0c4000b2a 100644 --- a/PExpr.h +++ b/PExpr.h @@ -45,9 +45,10 @@ class PPackage; class PExpr : public LineInfo { public: - enum width_mode_t { SIZED, EXPAND, LOSSLESS, UNSIZED }; + // Mode values used by test_width() (see below for description). + enum width_mode_t { SIZED, UNSIZED, EXPAND, LOSSLESS, UPSIZE }; - // Flag values that can be passed to elaborate_expr. + // Flag values that can be passed to elaborate_expr(). static const unsigned NO_FLAGS = 0x0; static const unsigned NEED_CONST = 0x1; static const unsigned SYS_TASK_ARG = 0x2; @@ -86,29 +87,38 @@ class PExpr : public LineInfo { // test the width of an expression. In SIZED mode the expression // width will be calculated strictly according to the IEEE standard // rules for expression width. - // If the expression contains an unsized literal, mode will be - // changed to LOSSLESS. In LOSSLESS mode the expression width will - // be calculated as the minimum width necessary to avoid arithmetic + // + // If the expression is found to contain an unsized literal number + // and gn_strict_expr_width_flag is set, mode will be changed to + // UNSIZED. In UNSIZED mode the expression width will be calculated + // exactly as in SIZED mode - the change in mode simply flags that + // the expression contains an unsized numbers. + // + // If the expression is found to contain an unsized literal number + // and gn_strict_expr_width_flag is not set, mode will be changed + // to LOSSLESS. In LOSSLESS mode the expression width will be + // calculated as the minimum width necessary to avoid arithmetic // overflow or underflow. - // If the expression both contains an unsized literal and contains + // + // Once in LOSSLESS mode, if the expression is found to contain // an operation that coerces a vector operand to a different type - // (signed <-> unsigned), mode is changed to UNSIZED. UNSIZED mode - // is the same as LOSSLESS, except that the final expression width - // will be forced to be at least integer_width. This is necessary - // to ensure compatibility with the IEEE standard, which requires - // unsized literals to be treated as having the same width as an - // integer. The lossless width calculation is inadequate in this - // case because coercing an operand to a different type means that - // the expression no longer obeys the normal rules of arithmetic. + // (signed <-> unsigned), mode will be changed to UPSIZE. UPSIZE + // mode is the same as LOSSLESS, except that the final expression + // width will be forced to be at least integer_width. This is + // necessary to ensure compatibility with the IEEE standard, which + // requires unsized numbers to be treated as having the same width + // as an integer. The lossless width calculation is inadequate in + // this case because coercing an operand to a different type means + // that the expression no longer obeys the normal rules of arithmetic. // // If mode is initialised to EXPAND instead of SIZED, the expression // width will be calculated as the minimum width necessary to avoid // arithmetic overflow or underflow, even if it contains no unsized - // literals. mode will be changed LOSSLESS or UNSIZED as described + // literals. mode will be changed LOSSLESS or UPSIZE as described // above. This supports a non-standard mode of expression width // calculation. // - // When the final value of mode is UNSIZED, the width returned by + // When the final value of mode is UPSIZE, the width returned by // this method is the calculated lossless width, but the width // returned by a subsequent call to the expr_width method will be // the final expression width. diff --git a/elab_expr.cc b/elab_expr.cc index 1fc1d0918..21c494ae7 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -136,14 +136,14 @@ NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, ivl_type_t lv_net_type, } /* - * If the mode is UNSIZED, make sure the final expression width is at + * If the mode is UPSIZE, make sure the final expression width is at * least integer_width, but return the calculated lossless width to * the caller. */ unsigned PExpr::fix_width_(width_mode_t mode) { unsigned width = expr_width_; - if ((mode == UNSIZED) && type_is_vectorable(expr_type_) + if ((mode == UPSIZE) && type_is_vectorable(expr_type_) && (width < integer_width)) expr_width_ = integer_width; @@ -265,7 +265,7 @@ unsigned PEBinary::test_width(Design*des, NetScope*scope, width_mode_t&mode) unsigned r_width = right_->test_width(des, scope, mode); // If the width mode changed, retest the left operand, as it - // may choose a different width if it is in an unsized context. + // may choose a different width if it is in a lossless context. if ((mode >= LOSSLESS) && (saved_mode < LOSSLESS)) l_width = left_->test_width(des, scope, mode); @@ -293,17 +293,17 @@ unsigned PEBinary::test_width(Design*des, NetScope*scope, width_mode_t&mode) // calculation is unreliable and we need to make sure the // final expression width is at least integer_width. if ((mode == LOSSLESS) && (left_->has_sign() != right_->has_sign())) - mode = UNSIZED; + mode = UPSIZE; switch (op_) { case '+': case '-': - if (mode != SIZED) + if (mode >= EXPAND) expr_width_ += 1; break; case '*': - if (mode != SIZED) + if (mode >= EXPAND) expr_width_ = l_width + r_width; break; @@ -575,8 +575,8 @@ unsigned PEBComp::test_width(Design*des, NetScope*scope, width_mode_t&) unsigned r_width = right_->test_width(des, scope, mode); // If the width mode changed, retest the left operand, as it - // may choose a different width if it is in an unsized context. - if ((mode != SIZED) && (saved_mode == SIZED)) + // may choose a different width if it is in a lossless context. + if ((mode >= LOSSLESS) && (saved_mode < LOSSLESS)) l_width = left_->test_width(des, scope, mode); ivl_variable_type_t l_type = left_->expr_type(); @@ -590,14 +590,14 @@ unsigned PEBComp::test_width(Design*des, NetScope*scope, width_mode_t&) if (type_is_vectorable(r_type) && (l_width > r_width)) r_width_ = l_width; - // If the expression is unsized and smaller than the integer + // If the expression is lossless and smaller than the integer // minimum, then tweak the size up. // NOTE: I really would rather try to figure out what it would // take to get expand the sub-expressions so that they are // exactly the right width to behave just like infinite // width. I suspect that adding 1 more is sufficient in all // cases, but I'm not certain. Ideas? - if (mode != SIZED) { + if (mode >= EXPAND) { if (type_is_vectorable(l_type) && (l_width_ < integer_width)) l_width_ += 1; if (type_is_vectorable(r_type) && (r_width_ < integer_width)) @@ -725,7 +725,7 @@ unsigned PEBLeftWidth::test_width(Design*des, NetScope*scope, width_mode_t&mode) expr_type_ = left_->expr_type(); signed_flag_ = left_->has_sign(); - if ((mode != SIZED) && type_is_vectorable(expr_type_)) { + if ((mode >= EXPAND) && type_is_vectorable(expr_type_)) { // We need to make our best guess at the right operand // value, to minimise the calculated width. This is // particularly important for the power operator... @@ -782,7 +782,7 @@ unsigned PEBLeftWidth::test_width(Design*des, NetScope*scope, width_mode_t&mode) // shift may do the same, as we don't yet know the final // expression type. if ((mode == LOSSLESS) && signed_flag_) - mode = UNSIZED; + mode = UPSIZE; break; case 'p': // ** @@ -1059,7 +1059,7 @@ unsigned PECallFunction::test_width_sfunc_(Design*des, NetScope*scope, min_width_ = expr->min_width(); signed_flag_ = (name[1] == 's'); - if ((arg_mode != SIZED) && type_is_vectorable(expr_type_)) { + if ((arg_mode >= EXPAND) && type_is_vectorable(expr_type_)) { if (mode < LOSSLESS) mode = LOSSLESS; if (expr_width_ < integer_width) @@ -2864,8 +2864,8 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) min_width_ = expr_width_; signed_flag_ = par->has_sign(); - if ((mode < LOSSLESS) && !par->has_width()) - mode = LOSSLESS; + if (!par->has_width() && (mode < LOSSLESS)) + mode = LOSSLESS; return expr_width_; } @@ -2879,8 +2879,12 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) min_width_ = expr_width_; signed_flag_ = true; - if (mode < LOSSLESS) - mode = LOSSLESS; + if (gn_strict_expr_width_flag) { + expr_width_ = integer_width; + mode = UNSIZED; + } else if (mode < LOSSLESS) { + mode = LOSSLESS; + } return expr_width_; } @@ -4838,15 +4842,17 @@ unsigned PENumber::test_width(Design*, NetScope*, width_mode_t&mode) { expr_type_ = IVL_VT_LOGIC; expr_width_ = value_->len(); + min_width_ = expr_width_; signed_flag_ = value_->has_sign(); if (!value_->has_len() && !value_->is_single()) { - if (gn_strict_expr_width_flag) - expr_width_ = integer_width; - else if (mode < LOSSLESS) - mode = LOSSLESS; + if (gn_strict_expr_width_flag) { + expr_width_ = integer_width; + mode = UNSIZED; + } else if (mode < LOSSLESS) { + mode = LOSSLESS; + } } - min_width_ = expr_width_; return expr_width_; } @@ -4941,7 +4947,7 @@ unsigned PETernary::test_width(Design*des, NetScope*scope, width_mode_t&mode) unsigned fal_width = fal_->test_width(des, scope, mode); // If the width mode changed, retest the true clause, as it - // may choose a different width if it is in an unsized context. + // may choose a different width if it is in a lossless context. if ((mode >= LOSSLESS) && (saved_mode < LOSSLESS)) { tru_width = tru_->test_width(des, scope, mode); } @@ -4976,7 +4982,7 @@ unsigned PETernary::test_width(Design*des, NetScope*scope, width_mode_t&mode) // calculation is unreliable and we need to make sure the // final expression width is at least integer_width. if ((mode == LOSSLESS) && (tru_->has_sign() != fal_->has_sign())) - mode = UNSIZED; + mode = UPSIZE; } if (debug_elaborate) diff --git a/netmisc.cc b/netmisc.cc index 61c7344da..351e32f24 100644 --- a/netmisc.cc +++ b/netmisc.cc @@ -859,7 +859,7 @@ NetExpr* elab_sys_task_arg(Design*des, NetScope*scope, perm_string name, // determine the exact width required to hold the result. // But leave literal numbers exactly as the user supplied // them. - if ((mode != PExpr::SIZED) && !dynamic_cast(pe)) + if ((mode >= PExpr::LOSSLESS) && !dynamic_cast(pe)) ce->trim(); }