From 862b118098d56a77a0418a8ef5e52b03baca94d6 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 24 Dec 2022 07:56:33 -0800 Subject: [PATCH 1/5] PEIdent::elaborate_{expr,lval}(): Use new symbol_search() The PEIdent elaborate_expr() and elaborate_lval() are sort of open-coding the path traversal implemented by the new symbol_search() using the old symbol_search(). Switch them over to use the new symbol search as it is better at handling the corner cases and is also less code. Signed-off-by: Lars-Peter Clausen --- elab_expr.cc | 60 +++++++++++++++++++++------------------------------- elab_lval.cc | 26 +++++------------------ 2 files changed, 29 insertions(+), 57 deletions(-) diff --git a/elab_expr.cc b/elab_expr.cc index 8214daa76..9088387bd 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -4322,12 +4322,6 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, { bool need_const = NEED_CONST & flags; - NetNet* net = 0; - ivl_type_t cls_val = 0; - const NetExpr*par = 0; - ivl_type_t par_type = 0; - NetEvent* eve = 0; - NetScope*use_scope = scope; if (package_) { use_scope = des->find_package(package_->pscope_name()); @@ -4338,50 +4332,44 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, return tmp; } - symbol_search(this, des, use_scope, path_, net, par, eve, par_type, cls_val); + symbol_search_results sr; + symbol_search(this, des, use_scope, path_, &sr); - if (net == 0 && gn_system_verilog() && path_.size() >= 2) { - // NOTE: this is assuming the member_path is only one - // component long, and that the use_path will wind up - // being the path to the variable. This is not - // necessarily true. Should fix this. - pform_name_t use_path = path_; - name_component_t member_comp = use_path.back(); - use_path.pop_back(); + if (!sr.net) { + cerr << get_fileline() << ": error: Unable to bind variable `" + << path_ << "' in `" << scope_path(use_scope) << "'" << endl; + des->errors++; + return nullptr; + } - ivl_assert(*this, net == 0); - symbol_search(this, des, use_scope, use_path, net, par, eve, - par_type, cls_val); + NetNet *net = sr.net; - if (net == 0) { - // Nope, no struct/class with member. + if (!sr.path_tail.empty()) { + if (net->struct_type()) { + return check_for_struct_members(this, des, use_scope, net, + sr.path_head.back().index, + sr.path_tail); + } else if (net->class_type()) { + const name_component_t member_comp = sr.path_tail.front(); - } else if (net->struct_type() != 0) { - pform_name_t member_path; - member_path.push_back( member_comp ); - return check_for_struct_members(this, des, use_scope, - net, use_path.back().index, - member_path); - - } else if (net->class_type()!=0) { if (debug_elaborate) { cerr << get_fileline() << ": PEIdent::elaborate_expr: " - << "Ident " << use_path + << "Ident " << sr.path_head << " look for property " << member_comp << endl; } + if (sr.path_tail.size() > 1) { + cerr << get_fileline() << ": sorry: " + << "Nested member path not yet supported in this context." + << endl; + return nullptr; + } + return elaborate_expr_class_field_(des, scope, net, member_comp, 0, flags); } } - if (net == 0) { - cerr << get_fileline() << ": error: Unable to bind variable `" - << path_ << "' in `" << scope_path(use_scope) << "'" << endl; - des->errors += 1; - return 0; - } - if (debug_elaborate) { cerr << get_fileline() << ": PEIdent::elaborate_expr: " << "Found net " << net->name() << " for expr " << *this << endl; diff --git a/elab_lval.cc b/elab_lval.cc index 36bee8ac5..cdbf52b0e 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -159,9 +159,6 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, bool is_cassign, bool is_force) const { - NetNet* reg = 0; - const NetExpr*par = 0; - NetEvent* eve = 0; if (debug_elaborate) { cerr << get_fileline() << ": PEIdent::elaborate_lval: " @@ -182,25 +179,12 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, ivl_assert(*this, use_scope); } - /* Try to find the base part of the path that names the - variable. The remainer is the member path. For example, if - the path is a.b.c.d, and a.b is the path to a variable, - then a.b becomes the base_path and c.d becomes the - member_path. If we cannot find the variable with any - prefix, then the base_path will be empty after this loop - and reg will remain nil. */ - pform_name_t base_path = path_; - pform_name_t member_path; - while (reg == 0 && !base_path.empty()) { - symbol_search(this, des, use_scope, base_path, reg, par, eve); - // Found it! - if (reg != 0) break; - // Not found. Try to pop another name off the base_path - // and push it to the front of the member_path. - member_path.push_front( base_path.back() ); - base_path.pop_back(); - } + symbol_search_results sr; + symbol_search(this, des, use_scope, path_, &sr); + NetNet *reg = sr.net; + pform_name_t &base_path = sr.path_head; + pform_name_t &member_path = sr.path_tail; /* The l-value must be a variable. If not, then give up and print a useful error message. */ From 7188f7b210a8114be4e344bbfe3d4ff8d3ffdb01 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Sep 2022 15:45:33 +0200 Subject: [PATCH 2/5] Pretty print THIS_TOKEN and SUPER_TOKEN Internally the special THIS_TOKEN("@") and SUPER_TOKEN("#") are used to represent the special `this` and `super` keywords in a component name. When printing an identifier replace the tokens with their keywords. This generates nicer error and debug messages. Signed-off-by: Lars-Peter Clausen --- pform_dump.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pform_dump.cc b/pform_dump.cc index fdf6fcfae..5b72cbeb4 100644 --- a/pform_dump.cc +++ b/pform_dump.cc @@ -87,7 +87,12 @@ ostream& operator<< (ostream&out, const index_component_t&that) ostream& operator<< (ostream&out, const name_component_t&that) { - out << that.name.str(); + if (that.name == THIS_TOKEN) + out << "this"; + else if (that.name == SUPER_TOKEN) + out << "super"; + else + out << that.name.str(); typedef std::list::const_iterator index_it_t; for (index_it_t idx = that.index.begin() From c0adbd0deb1b19ebf118759946bc4534245baa26 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Sep 2022 14:39:58 +0200 Subject: [PATCH 3/5] Add support for handling `super` keyword SystemVerilog allows to use the `super` keyword to access properties and methods of a base class. This is useful if there is for example an identifier with the same name in the current class as in the base class and the code wants to access the base class identifier. To support this a bit of refactoring is required. Currently properties are internally referenced by name, this does not work if there are multiple properties of the same. Instead reference properties always by index. In addition when looking up an identifier that resolves to an object return both the type and the object itself. This is necessary since both `this` and `super` resolve to the same object, but each with a different type. Signed-off-by: Lars-Peter Clausen --- PExpr.h | 4 +-- elab_expr.cc | 82 +++++++++++++++++++++--------------------------- elab_lval.cc | 13 ++++---- elaborate.cc | 2 +- net_assign.cc | 14 +++------ net_expr.cc | 5 ++- netlist.h | 7 +++-- netmisc.h | 4 +-- symbol_search.cc | 26 ++++++++++----- t-dll-proc.cc | 20 +----------- 10 files changed, 76 insertions(+), 101 deletions(-) diff --git a/PExpr.h b/PExpr.h index 14ba1577a..8114eff2b 100644 --- a/PExpr.h +++ b/PExpr.h @@ -417,6 +417,7 @@ class PEIdent : public PExpr { index_component_t::ctype_t, bool need_const_idx) const; NetAssign_*elaborate_lval_net_class_member_(Design*, NetScope*, + const netclass_t *class_type, NetNet*, pform_name_t) const; bool elaborate_lval_net_packed_member_(Design*, NetScope*, @@ -511,8 +512,7 @@ class PEIdent : public PExpr { unsigned flags) const; NetExpr *elaborate_expr_class_field_(Design*des, NetScope*scope, - NetNet*net, - const name_component_t&comp, + const symbol_search_results &sr, unsigned expr_wid, unsigned flags) const; diff --git a/elab_expr.cc b/elab_expr.cc index 9088387bd..c084b8029 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -1480,8 +1480,7 @@ unsigned PECallFunction::test_width_method_(Design*, NetScope*, // and the scope is the scope where the instance lives. The class method // in turn defines it's own scope. Use that to find the return value. if (search_results.net && search_results.net->data_type()==IVL_VT_CLASS) { - NetNet*net = search_results.net; - const netclass_t*class_type = net->class_type(); + const netclass_t *class_type = dynamic_cast(search_results.type); ivl_assert(*this, class_type); NetScope*method = class_type->method_from_name(method_name); @@ -2536,12 +2535,26 @@ static NetExpr* class_static_property_expression(const LineInfo*li, } NetExpr* PEIdent::elaborate_expr_class_field_(Design*des, NetScope*scope, - NetNet*net, - const name_component_t&comp, + const symbol_search_results &sr, unsigned expr_wid, unsigned flags) const { - const netclass_t*class_type = net->class_type(); + + const netclass_t *class_type = dynamic_cast(sr.type); + const name_component_t comp = sr.path_tail.front(); + + if (debug_elaborate) { + cerr << get_fileline() << ": PEIdent::elaborate_expr: " + << "Ident " << sr.path_head + << " look for property " << comp << endl; + } + + if (sr.path_tail.size() > 1) { + cerr << get_fileline() << ": sorry: " + << "Nested member path not yet supported for class properties." + << endl; + return nullptr; + } ivl_type_t par_type; const NetExpr *par_val = class_type->get_parameter(des, comp.name, par_type); @@ -2562,7 +2575,7 @@ NetExpr* PEIdent::elaborate_expr_class_field_(Design*des, NetScope*scope, if (debug_elaborate) { cerr << get_fileline() << ": check_for_class_property: " << "Property " << comp.name - << " of net " << net->name() + << " of net " << sr.net->name() << ", context scope=" << scope_path(scope) << endl; } @@ -2582,7 +2595,7 @@ NetExpr* PEIdent::elaborate_expr_class_field_(Design*des, NetScope*scope, prop_name); } - NetEProperty*tmp = new NetEProperty(net, comp.name); + NetEProperty*tmp = new NetEProperty(sr.net, pidx); tmp->set_line(*this); return tmp; } @@ -2751,6 +2764,7 @@ NetExpr* PECallFunction::elaborate_expr_(Design*des, NetScope*scope, use_search_results.path_tail.push_back(search_results.path_head.back()); use_search_results.path_head.push_back(name_component_t(perm_string::literal(THIS_TOKEN))); use_search_results.net = scope->find_signal(perm_string::literal(THIS_TOKEN)); + use_search_results.type = use_search_results.net->net_type(); ivl_assert(*this, use_search_results.net); return elaborate_expr_method_(des, scope, use_search_results); @@ -3026,12 +3040,12 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope, if (search_results.par_val) cerr << get_fileline() << ": PECallFunction::elaborate_expr_method_: " << "search_results.par_val: " << *search_results.par_val << endl; - if (search_results.par_type) + if (search_results.type) cerr << get_fileline() << ": PECallFunction::elaborate_expr_method_: " - << "search_results.par_type: " << *search_results.par_type << endl; + << "search_results.type: " << *search_results.type << endl; } - if (search_results.par_val && search_results.par_type) { + if (search_results.par_val && search_results.type) { return elaborate_expr_method_par_(des, scope, search_results); } @@ -3177,7 +3191,7 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope, perm_string method_name = search_results.path_tail.back().name; NetNet*net = search_results.net; - const netclass_t*class_type = net->class_type(); + const netclass_t*class_type = dynamic_cast(search_results.type); ivl_assert(*this, class_type); NetScope*method = class_type->method_from_name(method_name); @@ -3285,10 +3299,10 @@ NetExpr* PECallFunction::elaborate_expr_method_par_(Design*des, NetScope*scope, const { ivl_assert(*this, search_results.par_val); - ivl_assert(*this, search_results.par_type); + ivl_assert(*this, search_results.type); const NetExpr*par_val = search_results.par_val; - ivl_type_t par_type = search_results.par_type; + ivl_type_t par_type = search_results.type; perm_string method_name = search_results.path_tail.back().name; // If the parameter is of type string, then look for the standard string @@ -4230,8 +4244,8 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) // Similarly, if this net is an object, the path tail may // be a class property. - if (sr.net->class_type() != 0 && !sr.path_tail.empty()) { - const netclass_t*class_type = sr.net->class_type(); + const netclass_t *class_type = dynamic_cast(sr.type); + if (class_type && !sr.path_tail.empty()) { perm_string pname = peek_tail_name(sr.path_tail); ivl_type_t par_type; const NetExpr *par = class_type->get_parameter(des, pname, par_type); @@ -4349,24 +4363,8 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, return check_for_struct_members(this, des, use_scope, net, sr.path_head.back().index, sr.path_tail); - } else if (net->class_type()) { - const name_component_t member_comp = sr.path_tail.front(); - - if (debug_elaborate) { - cerr << get_fileline() << ": PEIdent::elaborate_expr: " - << "Ident " << sr.path_head - << " look for property " << member_comp << endl; - } - - if (sr.path_tail.size() > 1) { - cerr << get_fileline() << ": sorry: " - << "Nested member path not yet supported in this context." - << endl; - return nullptr; - } - - return elaborate_expr_class_field_(des, scope, net, - member_comp, 0, flags); + } else if (dynamic_cast(sr.type)) { + return elaborate_expr_class_field_(des, scope, sr, 0, flags); } } @@ -4570,7 +4568,7 @@ NetExpr* PEIdent::elaborate_expr_class_member_(Design*des, NetScope*scope, << " canonical index: " << *canon_index << endl; } - NetEProperty*tmp = new NetEProperty(this_net, member_name, canon_index); + NetEProperty*tmp = new NetEProperty(this_net, pidx, canon_index); tmp->set_line(*this); return tmp; } @@ -4667,7 +4665,7 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, } return elaborate_expr_param_or_specparam_(des, scope, sr.par_val, - sr.scope, sr.par_type, + sr.scope, sr.type, expr_wid, flags); } @@ -4882,18 +4880,8 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, return 0; } - if (sr.net->class_type() && !sr.path_tail.empty()) { - if (debug_elaborate) { - cerr << get_fileline() << ": PEIdent::elaborate_expr: " - "Ident " << sr.path_head - << " look for class property " << sr.path_tail - << endl; - } - - ivl_assert(*this, sr.path_tail.size() == 1); - const name_component_t member_comp = sr.path_tail.front(); - return elaborate_expr_class_field_(des, use_scope, - sr.net, member_comp, + if (dynamic_cast(sr.type) && !sr.path_tail.empty()) { + return elaborate_expr_class_field_(des, use_scope, sr, expr_wid, flags); } diff --git a/elab_lval.cc b/elab_lval.cc index cdbf52b0e..3c8a82c69 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -285,8 +285,9 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, // If the variable is a class object, then handle it with the // net_class_member_ method. - if (reg->class_type() && !member_path.empty() && gn_system_verilog()) { - NetAssign_*lv = elaborate_lval_net_class_member_(des, use_scope, reg, member_path); + const netclass_t *class_type = dynamic_cast(sr.type); + if (class_type && !member_path.empty() && gn_system_verilog()) { + NetAssign_*lv = elaborate_lval_net_class_member_(des, use_scope, class_type, reg, member_path); return lv; } @@ -475,7 +476,7 @@ NetAssign_* PEIdent::elaborate_lval_method_class_member_(Design*des, } NetAssign_*this_lval = new NetAssign_(this_net); - this_lval->set_property(member_name); + this_lval->set_property(member_name, pidx); if (canon_index) this_lval->set_word(canon_index); return this_lval; @@ -1078,7 +1079,8 @@ bool PEIdent::elaborate_lval_net_idx_(Design*des, * obj, and member_path=base.x. */ NetAssign_* PEIdent::elaborate_lval_net_class_member_(Design*des, NetScope*scope, - NetNet*sig, pform_name_t member_path) const + const netclass_t *class_type, NetNet*sig, + pform_name_t member_path) const { if (debug_elaborate) { cerr << get_fileline() << ": PEIdent::elaborate_lval_net_class_member_: " @@ -1086,7 +1088,6 @@ NetAssign_* PEIdent::elaborate_lval_net_class_member_(Design*des, NetScope*scope << " of " << sig->name() << "." << endl; } - const netclass_t*class_type = sig->class_type(); ivl_assert(*this, class_type); // Iterate over the member_path. This handles nested class @@ -1149,7 +1150,7 @@ NetAssign_* PEIdent::elaborate_lval_net_class_member_(Design*des, NetScope*scope } lv = lv? new NetAssign_(lv) : new NetAssign_(sig); - lv->set_property(method_name); + lv->set_property(method_name, pidx); // Now get the type of the property. ivl_type_t ptype = class_type->get_prop_type(pidx); diff --git a/elaborate.cc b/elaborate.cc index 25e47d236..41ce06ca0 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -3906,7 +3906,7 @@ NetProc* PCallTask::elaborate_method_(Design*des, NetScope*scope, "$ivl_queue_method$pop_back"); } - if (const netclass_t*class_type = net->class_type()) { + if (const netclass_t*class_type = dynamic_cast(par_type)) { NetScope*task = class_type->method_from_name(method_name); if (task == 0) { // If an implicit this was added it is not an error if we diff --git a/net_assign.cc b/net_assign.cc index d3ecf5215..45379b265 100644 --- a/net_assign.cc +++ b/net_assign.cc @@ -158,10 +158,7 @@ const ivl_type_s* NetAssign_::net_type() const return ntype; if (const netclass_t*class_type = dynamic_cast(ntype)) { - int pidx = class_type->property_idx_from_name(member_); - ivl_assert(*this, pidx >= 0); - ivl_type_t tmp = class_type->get_prop_type(pidx); - return tmp; + return class_type->get_prop_type(member_idx_); } if (const netdarray_t*darray = dynamic_cast (ntype)) { @@ -178,10 +175,7 @@ const ivl_type_s* NetAssign_::net_type() const if (member_.nil()) return sig_->net_type(); - int pidx = class_type->property_idx_from_name(member_); - ivl_assert(*sig_, pidx >= 0); - ivl_type_t tmp = class_type->get_prop_type(pidx); - return tmp; + return class_type->get_prop_type(member_idx_); } if (const netdarray_t*darray = dynamic_cast (sig_->net_type())) { @@ -246,10 +240,10 @@ void NetAssign_::set_part(NetExpr*base, unsigned wid, sel_type_ = sel_type; } -void NetAssign_::set_property(const perm_string&mname) +void NetAssign_::set_property(const perm_string&mname, unsigned idx) { - //ivl_assert(*sig_, sig_->class_type()); member_ = mname; + member_idx_ = idx; } /* diff --git a/net_expr.cc b/net_expr.cc index db163e84a..25f10dcb5 100644 --- a/net_expr.cc +++ b/net_expr.cc @@ -382,13 +382,12 @@ NetENull::~NetENull() { } -NetEProperty::NetEProperty(NetNet*net, perm_string pnam, NetExpr*idx) -: net_(net), index_(idx) +NetEProperty::NetEProperty(NetNet*net, size_t pidx, NetExpr*idx) +: net_(net), pidx_(pidx), index_(idx) { const netclass_t*use_type = dynamic_cast(net->net_type()); assert(use_type); - pidx_ = use_type->property_idx_from_name(pnam); ivl_type_t prop_type = use_type->get_prop_type(pidx_); expr_width(prop_type->packed_width()); cast_signed(prop_type->get_signed()); diff --git a/netlist.h b/netlist.h index 78cb7a8ef..45cb82ee8 100644 --- a/netlist.h +++ b/netlist.h @@ -2838,8 +2838,8 @@ class NetAssign_ { ivl_select_type_t = IVL_SEL_OTHER); // Set the member or property name if the signal type is a // class. - void set_property(const perm_string&name); - inline perm_string get_property(void) const { return member_; } + void set_property(const perm_string&name, unsigned int idx); + inline int get_property_idx(void) const { return member_idx_; } // Determine if the assigned object is signed or unsigned. // This is used when determining the expression type for @@ -2897,6 +2897,7 @@ class NetAssign_ { NetExpr*word_; // member/property if signal is a class. perm_string member_; + int member_idx_ = -1; bool signed_; bool turn_sig_to_wire_on_release_; @@ -4579,7 +4580,7 @@ class NetENull : public NetExpr { */ class NetEProperty : public NetExpr { public: - NetEProperty(NetNet*n, perm_string pname, NetExpr*canon_index =0); + NetEProperty(NetNet*n, size_t pidx_, NetExpr*canon_index =0); ~NetEProperty(); inline const NetNet* get_sig() const { return net_; } diff --git a/netmisc.h b/netmisc.h index 621a95757..c523b4e61 100644 --- a/netmisc.h +++ b/netmisc.h @@ -48,7 +48,7 @@ struct symbol_search_results { net = 0; cls_val = 0; par_val = 0; - par_type = 0; + type = 0; eve = 0; } @@ -80,7 +80,7 @@ struct symbol_search_results { // If this was a parameter, the value expression and the // optional value dimensions. const NetExpr*par_val; - ivl_type_t par_type; + ivl_type_t type; // If this is a named event, ... NetEvent*eve; diff --git a/symbol_search.cc b/symbol_search.cc index 2815777c7..2b6cc707f 100644 --- a/symbol_search.cc +++ b/symbol_search.cc @@ -132,12 +132,6 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, if (scope->genvar_tmp.str() && path_tail.name == scope->genvar_tmp) return false; - if (path_tail.name == "#") { - cerr << li->get_fileline() << ": sorry: " - << "Implicit class handle \"super\" not supported." << endl; - return false; - } - // These items cannot be seen outside the bounding module where // the search starts. But we continue searching up because scope // names can match. For example: @@ -151,10 +145,26 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, // ... top.not_ok; // Matches. // endmodule if (!passed_module_boundary) { + // Special case `super` keyword. Return the `this` object, but + // with the type of the base class. + if (path_tail.name == "#") { + if (NetNet *net = scope->find_signal(perm_string::literal(THIS_TOKEN))) { + const netclass_t *class_type = dynamic_cast(net->net_type()); + path.push_back(path_tail); + res->scope = scope; + res->net = net; + res->type = class_type->get_super(); + res->path_head = path; + return true; + } + return false; + } + if (NetNet*net = scope->find_signal(path_tail.name)) { path.push_back(path_tail); res->scope = scope; res->net = net; + res->type = net->net_type(); res->path_head = path; return true; } @@ -167,7 +177,7 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, return true; } - if (const NetExpr*par = scope->get_parameter(des, path_tail.name, res->par_type)) { + if (const NetExpr*par = scope->get_parameter(des, path_tail.name, res->type)) { path.push_back(path_tail); res->scope = scope; res->par_val = par; @@ -326,7 +336,7 @@ NetScope*symbol_search(const LineInfo*li, Design*des, NetScope*scope, net = recurse.net; cls_val = recurse.cls_val; par = recurse.par_val; - par_type = recurse.par_type; + par_type = recurse.type; eve = recurse.eve; if (! flag) { return 0; diff --git a/t-dll-proc.cc b/t-dll-proc.cc index 27ec54785..ab4ee69d2 100644 --- a/t-dll-proc.cc +++ b/t-dll-proc.cc @@ -179,8 +179,6 @@ bool dll_target::make_single_lval_(const LineInfo*li, struct ivl_lval_s*cur, con cur->width_ = asn->lwidth(); - ivl_type_t nest_type = 0; - if (asn->sig()) { cur->type_ = IVL_LVAL_REG; cur->n.sig = find_signal(des_, asn->sig()); @@ -188,7 +186,6 @@ bool dll_target::make_single_lval_(const LineInfo*li, struct ivl_lval_s*cur, con } else { const NetAssign_*asn_nest = asn->nest(); ivl_assert(*li, asn_nest); - nest_type = asn_nest->net_type(); struct ivl_lval_s*cur_nest = new struct ivl_lval_s; make_single_lval_(li, cur_nest, asn_nest); @@ -209,22 +206,7 @@ bool dll_target::make_single_lval_(const LineInfo*li, struct ivl_lval_s*cur, con expr_ = 0; } - cur->property_idx = -1; - perm_string pname = asn->get_property(); - if (!pname.nil()) { - const netclass_t*use_type; - switch (cur->type_) { - case IVL_LVAL_LVAL: - assert(nest_type); - use_type = dynamic_cast (nest_type); - break; - default: - use_type = dynamic_cast (cur->n.sig->net_type); - break; - } - assert(use_type); - cur->property_idx = use_type->property_idx_from_name(pname); - } + cur->property_idx = asn->get_property_idx(); return flag; } From 15fa24a664462829497856b0e3e252428d66d564 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Fri, 7 Oct 2022 14:21:20 +0200 Subject: [PATCH 4/5] Add support for `this.super` SystemVerilog allows to use either `super` or `this.super` to access the base class. Both have the same behavior. Currently only `super` is supported, also add support for `this.super`. To support it the parser has to be changed slightly and move the trailing `.` after the `this` or `super` keywords into the `implicit_class_handle` parser rule. This is necessary to avoid reduce conflicts in the grammar. As a side effect `super` can no longer be used as a standalone identifier. E.g. `return super;` But that's not legal SystemVerilog anyway, so that is OK. Signed-off-by: Lars-Peter Clausen --- parse.y | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/parse.y b/parse.y index 852e32f29..081851b2f 100644 --- a/parse.y +++ b/parse.y @@ -1554,15 +1554,16 @@ import_export /* IEEE1800-2012: A.2.9 */ ; implicit_class_handle /* IEEE1800-2005: A.8.4 */ - : K_this { $$ = pform_create_this(); } - | K_super { $$ = pform_create_super(); } + : K_this '.' { $$ = pform_create_this(); } + | K_super '.' { $$ = pform_create_super(); } + | K_this '.' K_super '.' { $$ = pform_create_super(); } ; /* `this` or `super` followed by an identifier */ class_hierarchy_identifier - : implicit_class_handle '.' hierarchy_identifier - { $1->splice($1->end(), *$3); - delete $3; + : implicit_class_handle hierarchy_identifier + { $1->splice($1->end(), *$2); + delete $2; $$ = $1; } ; @@ -3793,10 +3794,9 @@ expr_primary pform_requires_sv(@1, "Empty function argument list"); } - | implicit_class_handle - { PEIdent*tmp = new PEIdent(*$1); + | K_this + { PEIdent*tmp = new PEIdent(perm_string::literal(THIS_TOKEN)); FILE_NAME(tmp,@1); - delete $1; $$ = tmp; } @@ -6693,8 +6693,8 @@ statement_item /* This is roughly statement_item in the LRM */ beginning of a constructor, but let the elaborator figure that out. */ - | implicit_class_handle '.' K_new argument_list_parens_opt ';' - { PChainConstructor*tmp = new PChainConstructor(*$4); + | implicit_class_handle K_new argument_list_parens_opt ';' + { PChainConstructor*tmp = new PChainConstructor(*$3); FILE_NAME(tmp, @3); if (peek_head_name(*$1) == THIS_TOKEN) { yyerror(@1, "error: this.new is invalid syntax. Did you mean super.new?"); From 295b65da2cd2cedbe317406d01727140c4a80a6a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 17 Dec 2022 18:03:29 -0800 Subject: [PATCH 5/5] Add regression tests for using `super` to access the base class Check that it is possible to use the `super` keyword to access properties and methods of the base class that exist with the same name in current class. Also check that `this.super` is supported as an alternative to `super` and has the same behavior. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_class_super3.v | 39 +++++++++++++++++++++++++++++++ ivtest/ivltests/sv_class_super4.v | 26 +++++++++++++++++++++ ivtest/ivltests/sv_class_super5.v | 30 ++++++++++++++++++++++++ ivtest/ivltests/sv_class_super6.v | 39 +++++++++++++++++++++++++++++++ ivtest/regress-sv.list | 4 ++++ ivtest/regress-vlog95.list | 4 ++++ 6 files changed, 142 insertions(+) create mode 100644 ivtest/ivltests/sv_class_super3.v create mode 100644 ivtest/ivltests/sv_class_super4.v create mode 100644 ivtest/ivltests/sv_class_super5.v create mode 100644 ivtest/ivltests/sv_class_super6.v diff --git a/ivtest/ivltests/sv_class_super3.v b/ivtest/ivltests/sv_class_super3.v new file mode 100644 index 000000000..861721a85 --- /dev/null +++ b/ivtest/ivltests/sv_class_super3.v @@ -0,0 +1,39 @@ +// Check that `super` keyword can be used to access members of the base class + +class B; + int x, y; + + task set_y; + y = 2000; + endtask + + function bit check_x; + return x === 1000; + endfunction +endclass + +class C extends B; + byte x, y; + task set_x; + super.x = 1000; + endtask + + function bit check_y; + return super.y === 2000; + endfunction +endclass + +module test; + C c; + + initial begin + c = new; + c.set_x; + c.set_y; + if (c.check_x() && c.check_y()) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end +endmodule diff --git a/ivtest/ivltests/sv_class_super4.v b/ivtest/ivltests/sv_class_super4.v new file mode 100644 index 000000000..f0e05a757 --- /dev/null +++ b/ivtest/ivltests/sv_class_super4.v @@ -0,0 +1,26 @@ +// Check that `super` keyword can be used to call tasks of the base class + +class B; + task t; + $display("PASSED"); + endtask +endclass + +class C extends B; + task t; + $display("FAILED"); + endtask + + task check; + super.t; + endtask +endclass + +module test; + C c; + + initial begin + c = new; + c.check; + end +endmodule diff --git a/ivtest/ivltests/sv_class_super5.v b/ivtest/ivltests/sv_class_super5.v new file mode 100644 index 000000000..d11b755c3 --- /dev/null +++ b/ivtest/ivltests/sv_class_super5.v @@ -0,0 +1,30 @@ +// Check that `super` keyword can be used to call functions of the base class + +class B; + function int f; + return 1; + endfunction +endclass + +class C extends B; + function int f; + return 2; + endfunction + + task check; + if (super.f() === 1) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + endtask +endclass + +module test; + C c; + + initial begin + c = new; + c.check; + end +endmodule diff --git a/ivtest/ivltests/sv_class_super6.v b/ivtest/ivltests/sv_class_super6.v new file mode 100644 index 000000000..cfc1a156f --- /dev/null +++ b/ivtest/ivltests/sv_class_super6.v @@ -0,0 +1,39 @@ +// Check that `this.super` is supported + +class B; + int x, y; + + task set_y; + y = 2000; + endtask + + function bit check_x; + return x === 1000; + endfunction +endclass + +class C extends B; + byte x, y; + task set_x; + this.super.x = 1000; + endtask + + function bit check_y; + return this.super.y === 2000; + endfunction +endclass + +module test; + C c; + + initial begin + c = new; + c.set_x; + c.set_y; + if (c.check_x() && c.check_y()) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 3af28d4ff..be291eb87 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -587,6 +587,10 @@ sv_class_static_prop2 normal,-g2009 ivltests sv_class_static_prop3 normal,-g2009 ivltests sv_class_super1 normal,-g2009 ivltests sv_class_super2 normal,-g2009 ivltests +sv_class_super3 normal,-g2009 ivltests +sv_class_super4 normal,-g2009 ivltests +sv_class_super5 normal,-g2009 ivltests +sv_class_super6 normal,-g2009 ivltests sv_class_task1 normal,-g2009 ivltests sv_class_virt_new_fail CE,-g2009 ivltests sv_darray1 normal,-g2009 ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index d042f2b8c..abb56ce33 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -435,6 +435,10 @@ sv_class_static_prop2 CE,-g2009 ivltests sv_class_static_prop3 CE,-g2009 ivltests sv_class_super1 CE,-g2009 ivltests sv_class_super2 CE,-g2009 ivltests +sv_class_super3 CE,-g2009 ivltests +sv_class_super4 CE,-g2009 ivltests +sv_class_super5 CE,-g2009 ivltests +sv_class_super6 CE,-g2009 ivltests sv_class_task1 CE,-g2009 ivltests sv_end_label CE,-g2009 ivltests # Also generate sv_foreach2 CE,-g2009,-pallowsigned=1 ivltests