From 3a26bbb59a0d5edfd6eba8338807fc951f0f2318 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 7 Apr 2022 11:35:05 +0200 Subject: [PATCH] Handle context signedness and width expansion for class properties The signedness of an expression can change depending on its context. E.g. for an arithmetic operation with one unsigned operand all operands are treated as unsigned. This is currently not considered when accessing class properties. This can lead to incorrect behavior with regards to sign extension. E.g. the following will print 4294967295 rather than 65535. ``` class C; shortint x = -1; endclass ... C c = new; $display(c.x + 32'h0); ``` Furthermore the return value is not expanded to the width of its context. This can cause vvp to crash with an exception when it expects a vector on the stack to have a certain width. E.g. ``` class C; shortint x = -1; endclass ... C c = new; int x; bit a = 1'b1; x = a ? c.x : 64'h0; ``` Solve both of this by using `pad_to_width()` on the property expression if it is a vectorable type. Since any identifier, not just class properties, need to have this done insert this on the common path and remove the `pad_to_width()` call on individual paths. Signed-off-by: Lars-Peter Clausen --- PExpr.h | 3 +++ elab_expr.cc | 51 +++++++++++++++++++++++---------------------------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/PExpr.h b/PExpr.h index c02630d63..a57a69bb4 100644 --- a/PExpr.h +++ b/PExpr.h @@ -430,6 +430,9 @@ class PEIdent : public PExpr { NetAssign_*) const; private: + NetExpr* elaborate_expr_(Design *des, NetScope *scope, + unsigned expr_wid, unsigned flags) const; + NetExpr*elaborate_expr_param_or_specparam_(Design*des, NetScope*scope, const NetExpr*par, diff --git a/elab_expr.cc b/elab_expr.cc index e980e2188..35e117f11 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -4539,6 +4539,18 @@ NetExpr* PEIdent::elaborate_expr_class_member_(Design*des, NetScope*scope, */ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, unsigned expr_wid, unsigned flags) const +{ + NetExpr *result; + + result = elaborate_expr_(des, scope, expr_wid, flags); + if (!result || !type_is_vectorable(expr_type_)) + return result; + + return pad_to_width(result, expr_wid, signed_flag_, *this); +} + +NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, + unsigned expr_wid, unsigned flags) const { ivl_assert(*this, scope); @@ -4604,13 +4616,9 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, des->errors += 1; } - NetExpr*tmp = elaborate_expr_param_or_specparam_(des, scope, sr.par_val, - sr.scope, sr.par_type, - expr_wid, flags); - - if (!tmp) return 0; - - return pad_to_width(tmp, expr_wid, signed_flag_, *this); + return elaborate_expr_param_or_specparam_(des, scope, sr.par_val, + sr.scope, sr.par_type, + expr_wid, flags); } // If the identifier names a signal (a variable or a net) @@ -4645,11 +4653,9 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, << endl; } - NetExpr*tmp = check_for_struct_members(this, des, use_scope, - sr.net, sr.path_head.back().index, - sr.path_tail); - if (!tmp) return 0; - else return pad_to_width(tmp, expr_wid, signed_flag_, *this); + return check_for_struct_members(this, des, use_scope, sr.net, + sr.path_head.back().index, + sr.path_tail); } // If this is an array object, and there are members in @@ -4674,9 +4680,7 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, arg->set_line(*sr.net); fun->parm(0, arg); - if (!fun) - return 0; - return pad_to_width(fun, expr_wid, signed_flag_, *this); + return fun; } else if (member_comp.name == "find") { cerr << get_fileline() << ": sorry: 'find()' " "array location method is not currently " @@ -4795,9 +4799,7 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, arg->set_line(*sr.net); fun->parm(0, arg); - if (!fun || !type_is_vectorable(expr_type_)) - return fun; - return pad_to_width(fun, expr_wid, signed_flag_, *this); + return fun; } if (member_comp.name == "pop_front") { @@ -4809,9 +4811,7 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, arg->set_line(*sr.net); fun->parm(0, arg); - if (!fun || !type_is_vectorable(expr_type_)) - return fun; - return pad_to_width(fun, expr_wid, signed_flag_, *this); + return fun; } } @@ -4861,13 +4861,10 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, ivl_assert(*this, sr.path_tail.size() == 1); const name_component_t member_comp = sr.path_tail.front(); ivl_assert(*this, member_comp.index.empty()); - NetExpr *tmp = check_for_enum_methods(this, des, use_scope, + return check_for_enum_methods(this, des, use_scope, netenum, sr.path_head, member_comp.name, expr, NULL, 0); - if (!tmp) - return 0; - return pad_to_width(tmp, expr_wid, signed_flag_, *this); } ivl_assert(*this, sr.path_tail.empty()); @@ -4883,7 +4880,7 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, << ", tmp=" << *tmp << endl; } - return pad_to_width(tmp, expr_wid, signed_flag_, *this); + return tmp; } // If the identifier is a named event @@ -5514,8 +5511,6 @@ NetExpr* PEIdent::elaborate_expr_param_(Design*des, << "Elaborate parameter <" << path_ << "> as enumeration constant." << *etmp << endl; tmp = etmp->dup_expr(); - tmp = pad_to_width(tmp, expr_wid, signed_flag_, *this); - } else { perm_string name = peek_tail_name(path_);