From c963809709acd4f0dfdfc9fa0f39dc52b644047f Mon Sep 17 00:00:00 2001 From: Jose Tejada Date: Sun, 10 May 2026 14:45:33 +0200 Subject: [PATCH] feat(sv): support interface-typed module ports --- Module.cc | 13 +- Module.h | 16 ++ PGate.h | 9 + elab_expr.cc | 3 + elab_net.cc | 3 + elab_sig.cc | 32 +++ elaborate.cc | 265 +++++++++++------- ..._missing_modport_fail-iverilog-stderr.gold | 2 + ...ort_missing_type_fail-iverilog-stderr.gold | 2 + ...port_input_write_fail-iverilog-stderr.gold | 2 + ...interface_actual_fail-iverilog-stderr.gold | 2 + ..._unlisted_member_fail-iverilog-stderr.gold | 3 + ..._port_wrong_type_fail-iverilog-stderr.gold | 2 + ivtest/ivltests/sv_interface_port_basic.v | 47 ++++ .../sv_interface_port_missing_modport_fail.v | 22 ++ .../sv_interface_port_missing_type_fail.v | 11 + ..._interface_port_modport_input_write_fail.v | 22 ++ ...interface_port_non_interface_actual_fail.v | 22 ++ .../sv_interface_port_unlisted_member_fail.v | 25 ++ .../sv_interface_port_wrong_type_fail.v | 28 ++ ivtest/regress-vvp.list | 7 + ivtest/vvp_tests/sv_interface_port_basic.json | 5 + ...v_interface_port_missing_modport_fail.json | 6 + .../sv_interface_port_missing_type_fail.json | 6 + ...terface_port_modport_input_write_fail.json | 6 + ...erface_port_non_interface_actual_fail.json | 6 + ...v_interface_port_unlisted_member_fail.json | 6 + .../sv_interface_port_wrong_type_fail.json | 6 + net_scope.cc | 33 +++ netlist.h | 22 ++ netmisc.h | 19 ++ parse.y | 48 +++- pform.cc | 27 ++ pform.h | 8 + symbol_search.cc | 72 +++++ vvp/Makefile.in | 4 + 36 files changed, 710 insertions(+), 102 deletions(-) create mode 100644 ivtest/gold/sv_interface_port_missing_modport_fail-iverilog-stderr.gold create mode 100644 ivtest/gold/sv_interface_port_missing_type_fail-iverilog-stderr.gold create mode 100644 ivtest/gold/sv_interface_port_modport_input_write_fail-iverilog-stderr.gold create mode 100644 ivtest/gold/sv_interface_port_non_interface_actual_fail-iverilog-stderr.gold create mode 100644 ivtest/gold/sv_interface_port_unlisted_member_fail-iverilog-stderr.gold create mode 100644 ivtest/gold/sv_interface_port_wrong_type_fail-iverilog-stderr.gold create mode 100644 ivtest/ivltests/sv_interface_port_basic.v create mode 100644 ivtest/ivltests/sv_interface_port_missing_modport_fail.v create mode 100644 ivtest/ivltests/sv_interface_port_missing_type_fail.v create mode 100644 ivtest/ivltests/sv_interface_port_modport_input_write_fail.v create mode 100644 ivtest/ivltests/sv_interface_port_non_interface_actual_fail.v create mode 100644 ivtest/ivltests/sv_interface_port_unlisted_member_fail.v create mode 100644 ivtest/ivltests/sv_interface_port_wrong_type_fail.v create mode 100644 ivtest/vvp_tests/sv_interface_port_basic.json create mode 100644 ivtest/vvp_tests/sv_interface_port_missing_modport_fail.json create mode 100644 ivtest/vvp_tests/sv_interface_port_missing_type_fail.json create mode 100644 ivtest/vvp_tests/sv_interface_port_modport_input_write_fail.json create mode 100644 ivtest/vvp_tests/sv_interface_port_non_interface_actual_fail.json create mode 100644 ivtest/vvp_tests/sv_interface_port_unlisted_member_fail.json create mode 100644 ivtest/vvp_tests/sv_interface_port_wrong_type_fail.json diff --git a/Module.cc b/Module.cc index 6aefe59ab..5babfb973 100644 --- a/Module.cc +++ b/Module.cc @@ -28,6 +28,11 @@ using namespace std; list Module::user_defparms; +Module::port_t::port_t() +: port_kind(P_SIGNAL), default_value(0), lexical_pos(0) +{ +} + /* n is a permallocated string. */ Module::Module(LexicalScope*parent, perm_string n) : PScopeExtra(n, parent) @@ -63,12 +68,18 @@ const vector& Module::get_port(unsigned idx) const ivl_assert(*this, idx < ports.size()); static const vector zero; - if (ports[idx]) + if (ports[idx] && !ports[idx]->is_interface_port()) return ports[idx]->expr; else return zero; } +const Module::port_t* Module::get_port_info(unsigned idx) const +{ + ivl_assert(*this, idx < ports.size()); + return ports[idx]; +} + unsigned Module::find_port(const char*name) const { ivl_assert(*this, name != 0); diff --git a/Module.h b/Module.h index eecfbe781..2713be544 100644 --- a/Module.h +++ b/Module.h @@ -65,9 +65,24 @@ class Module : public PScopeExtra, public PNamedItem { default value. */ public: struct port_t { + enum port_kind_t { P_SIGNAL, P_INTERFACE }; + + port_t(); + + port_kind_t port_kind; perm_string name; std::vector expr; PExpr*default_value; + + /* Interface formal port metadata. For signal ports these + fields are empty/zero. The modport name is optional in the + representation, although the parser initially only accepts + the explicit interface_type.modport form. */ + perm_string interface_type; + perm_string modport_name; + unsigned lexical_pos; + + bool is_interface_port() const { return port_kind == P_INTERFACE; } }; public: @@ -148,6 +163,7 @@ class Module : public PScopeExtra, public PNamedItem { unsigned port_count() const; const std::vector& get_port(unsigned idx) const; + const port_t* get_port_info(unsigned idx) const; unsigned find_port(const char*name) const; // Return port name ("" for undeclared port) diff --git a/PGate.h b/PGate.h index 59c09954c..88e0d8c57 100644 --- a/PGate.h +++ b/PGate.h @@ -241,6 +241,15 @@ class PGModule : public PGate { void elaborate_scope_mod_(Design*des, Module*mod, NetScope*sc) const; void elaborate_scope_mod_instances_(Design*des, Module*mod, NetScope*sc) const; bool elaborate_sig_mod_(Design*des, NetScope*scope, const Module*mod) const; + bool bind_interface_ports_(Design*des, const Module*mod, + NetScope*parent_scope, NetScope*instance_scope, + const std::vector&pins, + const std::vector&pins_fromwc) const; + bool match_module_ports_(Design*des, const Module*mod, + NetScope*scope, + std::vector&pins, + std::vector&pins_fromwc, + std::vector&pins_is_explicitly_not_connected) const; // Not currently used. #if 0 bool elaborate_sig_udp_(Design*des, NetScope*scope, PUdp*udp) const; diff --git a/elab_expr.cc b/elab_expr.cc index 0902f5be1..32751c0b4 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -4747,6 +4747,9 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, // If the identifier names a signal (a variable or a net) // then create a NetESignal node to handle it. if (sr.net != 0) { + if (!check_interface_modport_access(this, des, sr, false)) + return 0; + if (NEED_CONST & flags) { cerr << get_fileline() << ": error: A reference to a net " "or variable (`" << path_ << "') is not allowed in " diff --git a/elab_net.cc b/elab_net.cc index 6ec75d067..dc9dc9782 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -557,6 +557,9 @@ NetNet* PEIdent::elaborate_lnet_common_(Design*des, NetScope*scope, return 0; } + if (!check_interface_modport_access(this, des, sr, true)) + return 0; + if (debug_elaborate) { cerr << get_fileline() << ": " << __func__ << ": " << "Found l-value path_=" << path_ diff --git a/elab_sig.cc b/elab_sig.cc index 5b8ed476b..d0b202ae0 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -44,6 +44,7 @@ # include "netqueue.h" # include "netscalar.h" # include "util.h" +# include "parse_api.h" # include "ivl_assert.h" using namespace std; @@ -298,6 +299,28 @@ bool Module::elaborate_sig(Design*des, NetScope*scope) const if (pp == 0) continue; + if (pp->is_interface_port()) { + map::const_iterator mod = + pform_modules.find(pp->interface_type); + if (mod == pform_modules.end() || !mod->second->is_interface) { + cerr << get_fileline() << ": error: Interface port " + << pp->name << " uses unknown interface type `" + << pp->interface_type << "'." << endl; + des->errors += 1; + continue; + } + + if (pp->modport_name.str() && + mod->second->modports.find(pp->modport_name) == mod->second->modports.end()) { + cerr << get_fileline() << ": error: Interface port " + << pp->name << " uses unknown modport `" + << pp->modport_name << "' of interface `" + << pp->interface_type << "'." << endl; + des->errors += 1; + } + continue; + } + // The port has a name and an array of expressions. The // expression are all identifiers that should reference // wires within the scope. @@ -456,6 +479,12 @@ bool PGModule::elaborate_sig_mod_(Design*des, NetScope*scope, NetScope::scope_vec_t instance = scope->instance_arrays[get_name()]; + vectorpins (rmod->port_count()); + vectorpins_fromwc (rmod->port_count(), false); + vectorpins_is_explicitly_not_connected (rmod->port_count(), false); + flag &= match_module_ports_(des, rmod, scope, pins, pins_fromwc, + pins_is_explicitly_not_connected); + for (unsigned idx = 0 ; idx < instance.size() ; idx += 1) { // I know a priori that the elaborate_scope created the scope // already, so just look it up as a child of the current scope. @@ -471,6 +500,9 @@ bool PGModule::elaborate_sig_mod_(Design*des, NetScope*scope, } ivl_assert(*this, my_scope->parent() == scope); + if (!bind_interface_ports_(des, rmod, scope, my_scope, pins, pins_fromwc)) + flag = false; + if (! rmod->elaborate_sig(des, my_scope)) flag = false; diff --git a/elaborate.cc b/elaborate.cc index 5216ad7b7..8c12940d1 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -1248,6 +1248,164 @@ void elaborate_unpacked_port(Design *des, NetScope *scope, NetNet *port_net, assign_unpacked_with_bufz(des, scope, port_net, port_net, expr_net); } +bool PGModule::match_module_ports_(Design*des, const Module*rmod, + NetScope*scope, + vector&pins, + vector&pins_fromwc, + vector&pins_is_explicitly_not_connected) const +{ + if (pins_) { + unsigned nexp = rmod->port_count(); + + for (unsigned idx = 0 ; idx < npins_ ; idx += 1) { + if (pins_[idx].name[0] == '*') { + for (unsigned j = 0 ; j < nexp ; j += 1) { + if (rmod->ports[j] && !pins[j] && !pins_is_explicitly_not_connected[j]) { + pins_fromwc[j] = true; + pform_name_t path_; + path_.push_back(name_component_t(rmod->ports[j]->name)); + symbol_search_results sr; + symbol_search(this, des, scope, path_, UINT_MAX, &sr); + if (sr.net != 0) { + pins[j] = new PEIdent(rmod->ports[j]->name, UINT_MAX, true); + pins[j]->set_lineno(get_lineno()); + pins[j]->set_file(get_file()); + } + } + } + continue; + } + + unsigned pidx = rmod->find_port(pins_[idx].name); + if (pidx == nexp) { + cerr << get_fileline() << ": error: port ``" << + pins_[idx].name << "'' is not a port of " + << get_name() << "." << endl; + des->errors += 1; + continue; + } + + if (pins_fromwc[pidx]) { + delete pins[pidx]; + pins_fromwc[pidx] = false; + + } else if (pins[pidx]) { + cerr << get_fileline() << ": error: port ``" << + pins_[idx].name << "'' already bound." << + endl; + des->errors += 1; + continue; + } + + pins[pidx] = pins_[idx].parm; + if (!pins[pidx]) + pins_is_explicitly_not_connected[pidx] = true; + } + + } else if (pin_count() == 0) { + for (unsigned idx = 0 ; idx < rmod->port_count() ; idx += 1) + pins[idx] = 0; + + } else { + if (pin_count() > rmod->port_count()) { + cerr << get_fileline() << ": error: Wrong number " + "of ports. Expecting at most " << rmod->port_count() << + ", got " << pin_count() << "." + << endl; + des->errors += 1; + return false; + } + + std::copy(get_pins().begin(), get_pins().end(), pins.begin()); + } + + return true; +} + +bool PGModule::bind_interface_ports_(Design*des, const Module*rmod, + NetScope*parent_scope, + NetScope*instance_scope, + const vector&pins, + const vector&pins_fromwc) const +{ + bool flag = true; + + for (unsigned idx = 0 ; idx < rmod->port_count() ; idx += 1) { + const Module::port_t*port = rmod->get_port_info(idx); + if (!port || !port->is_interface_port()) + continue; + + if (pins_fromwc[idx]) { + cerr << get_fileline() << ": sorry: Wildcard connection " + "to interface port `" << port->name + << "' is not yet supported." << endl; + des->errors += 1; + flag = false; + continue; + } + + if (!pins[idx]) { + cerr << get_fileline() << ": error: Interface port `" + << port->name << "' of module " << rmod->mod_name() + << " is not connected." << endl; + des->errors += 1; + flag = false; + continue; + } + + const PEIdent*actual_ident = dynamic_cast(pins[idx]); + if (!actual_ident || actual_ident->path().package || + actual_ident->path().name.size() != 1 || + !actual_ident->path().name.front().index.empty()) { + cerr << pins[idx]->get_fileline() << ": error: Interface port `" + << port->name << "' must be connected to a simple " + "interface instance name." << endl; + des->errors += 1; + flag = false; + continue; + } + + perm_string actual_name = actual_ident->path().name.front().name; + NetScope*actual_scope = parent_scope->child(hname_t(actual_name)); + if (!actual_scope) + actual_scope = parent_scope->find_interface_port_alias_scope(actual_name); + + if (!actual_scope || !actual_scope->is_interface()) { + cerr << pins[idx]->get_fileline() << ": error: Actual for " + "interface port `" << port->name + << "' is not an interface instance." << endl; + des->errors += 1; + flag = false; + continue; + } + + if (actual_scope->module_name() != port->interface_type) { + cerr << pins[idx]->get_fileline() << ": error: Interface port `" + << port->name << "' expects interface type `" + << port->interface_type << "' but actual `" << actual_name + << "' has type `" << actual_scope->module_name() << "'." << endl; + des->errors += 1; + flag = false; + continue; + } + + map::const_iterator mod = + pform_modules.find(port->interface_type); + const PModport*modport = 0; + if (mod != pform_modules.end()) { + map::const_iterator mp = + mod->second->modports.find(port->modport_name); + if (mp != mod->second->modports.end()) + modport = mp->second; + } + + instance_scope->add_interface_port_alias(port->name, actual_scope, + modport); + } + + return flag; +} + /* * Instantiate a module by recursively elaborating it. Set the path of * the recursive elaboration so that signal names get properly @@ -1274,103 +1432,9 @@ void PGModule::elaborate_mod_(Design*des, Module*rmod, NetScope*scope) const vectorpins_fromwc (rmod->port_count(), false); vectorpins_is_explicitly_not_connected (rmod->port_count(), false); - // If the instance has a pins_ member, then we know we are - // binding by name. Therefore, make up a pins array that - // reflects the positions of the named ports. - if (pins_) { - unsigned nexp = rmod->port_count(); - - // Scan the bindings, matching them with port names. - for (unsigned idx = 0 ; idx < npins_ ; idx += 1) { - - // Handle wildcard named port - if (pins_[idx].name[0] == '*') { - for (unsigned j = 0 ; j < nexp ; j += 1) { - if (rmod->ports[j] && !pins[j] && !pins_is_explicitly_not_connected[j]) { - pins_fromwc[j] = true; - pform_name_t path_; - path_.push_back(name_component_t(rmod->ports[j]->name)); - symbol_search_results sr; - symbol_search(this, des, scope, path_, UINT_MAX, &sr); - if (sr.net != 0) { - pins[j] = new PEIdent(rmod->ports[j]->name, UINT_MAX, true); - pins[j]->set_lineno(get_lineno()); - pins[j]->set_file(get_file()); - } - } - } - continue; - } - - // Given a binding, look at the module port names - // for the position that matches the binding name. - unsigned pidx = rmod->find_port(pins_[idx].name); - - // If the port name doesn't exist, the find_port - // method will return the port count. Detect that - // as an error. - if (pidx == nexp) { - cerr << get_fileline() << ": error: port ``" << - pins_[idx].name << "'' is not a port of " - << get_name() << "." << endl; - des->errors += 1; - continue; - } - - // If I am overriding a wildcard port, delete and - // override it - if (pins_fromwc[pidx]) { - delete pins[pidx]; - pins_fromwc[pidx] = false; - - // If I already explicitly bound something to - // this port, then the pins array will already - // have a pointer value where I want to place this - // expression. - } else if (pins[pidx]) { - cerr << get_fileline() << ": error: port ``" << - pins_[idx].name << "'' already bound." << - endl; - des->errors += 1; - continue; - } - - // OK, do the binding by placing the expression in - // the right place. - pins[pidx] = pins_[idx].parm; - if (!pins[pidx]) - pins_is_explicitly_not_connected[pidx] = true; - } - - - } else if (pin_count() == 0) { - - /* Handle the special case that no ports are - connected. It is possible that this is an empty - connect-by-name list, so we'll allow it and assume - that is the case. */ - - for (unsigned idx = 0 ; idx < rmod->port_count() ; idx += 1) - pins[idx] = 0; - - } else { - - /* Otherwise, this is a positional list of port - connections. Use as many ports as provided. Trailing - missing ports will be left unconnect or use the default - value if one is available */ - - if (pin_count() > rmod->port_count()) { - cerr << get_fileline() << ": error: Wrong number " - "of ports. Expecting at most " << rmod->port_count() << - ", got " << pin_count() << "." - << endl; - des->errors += 1; - return; - } - - std::copy(get_pins().begin(), get_pins().end(), pins.begin()); - } + if (!match_module_ports_(des, rmod, scope, pins, pins_fromwc, + pins_is_explicitly_not_connected)) + return; // Elaborate these instances of the module. The recursive // elaboration causes the module to generate a netlist with @@ -1403,6 +1467,13 @@ void PGModule::elaborate_mod_(Design*des, Module*rmod, NetScope*scope) const bool using_default = false; perm_string port_name = rmod->get_port_name(idx); + const Module::port_t*port_info = rmod->get_port_info(idx); + if (port_info && port_info->is_interface_port()) { + for (unsigned inst = 0 ; inst < instance.size() ; inst += 1) + instance[inst]->add_module_port_info(idx, port_name, + PortType::PIMPLICIT, 0); + continue; + } // If the port is unconnected, substitute the default // value. The parser ensures that a default value only diff --git a/ivtest/gold/sv_interface_port_missing_modport_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_missing_modport_fail-iverilog-stderr.gold new file mode 100644 index 000000000..7b28ae5f5 --- /dev/null +++ b/ivtest/gold/sv_interface_port_missing_modport_fail-iverilog-stderr.gold @@ -0,0 +1,2 @@ +ivltests/sv_interface_port_missing_modport_fail.v:18: error: Interface port bus uses unknown modport `consumer' of interface `bus_if'. +1 error(s) during elaboration. diff --git a/ivtest/gold/sv_interface_port_missing_type_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_missing_type_fail-iverilog-stderr.gold new file mode 100644 index 000000000..601207b55 --- /dev/null +++ b/ivtest/gold/sv_interface_port_missing_type_fail-iverilog-stderr.gold @@ -0,0 +1,2 @@ +ivltests/sv_interface_port_missing_type_fail.v:7: error: Interface port bus uses unknown interface type `missing_if'. +1 error(s) during elaboration. diff --git a/ivtest/gold/sv_interface_port_modport_input_write_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_modport_input_write_fail-iverilog-stderr.gold new file mode 100644 index 000000000..f8f61b1f6 --- /dev/null +++ b/ivtest/gold/sv_interface_port_modport_input_write_fail-iverilog-stderr.gold @@ -0,0 +1,2 @@ +ivltests/sv_interface_port_modport_input_write_fail.v:21: error: Cannot assign to input modport member `value' through interface port `bus'. +1 error(s) during elaboration. diff --git a/ivtest/gold/sv_interface_port_non_interface_actual_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_non_interface_actual_fail-iverilog-stderr.gold new file mode 100644 index 000000000..4c603b47c --- /dev/null +++ b/ivtest/gold/sv_interface_port_non_interface_actual_fail-iverilog-stderr.gold @@ -0,0 +1,2 @@ +ivltests/sv_interface_port_non_interface_actual_fail.v:9: error: Actual for interface port `bus' is not an interface instance. +Elaboration failed diff --git a/ivtest/gold/sv_interface_port_unlisted_member_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_unlisted_member_fail-iverilog-stderr.gold new file mode 100644 index 000000000..4295dc034 --- /dev/null +++ b/ivtest/gold/sv_interface_port_unlisted_member_fail-iverilog-stderr.gold @@ -0,0 +1,3 @@ +ivltests/sv_interface_port_unlisted_member_fail.v:24: error: Interface member `hidden' is not listed in modport `consumer'. +ivltests/sv_interface_port_unlisted_member_fail.v:24: error: Unable to elaborate r-value: bus.hidden +2 error(s) during elaboration. diff --git a/ivtest/gold/sv_interface_port_wrong_type_fail-iverilog-stderr.gold b/ivtest/gold/sv_interface_port_wrong_type_fail-iverilog-stderr.gold new file mode 100644 index 000000000..44e3c706c --- /dev/null +++ b/ivtest/gold/sv_interface_port_wrong_type_fail-iverilog-stderr.gold @@ -0,0 +1,2 @@ +ivltests/sv_interface_port_wrong_type_fail.v:9: error: Interface port `bus' expects interface type `bus_if' but actual `bus' has type `other_if'. +Elaboration failed diff --git a/ivtest/ivltests/sv_interface_port_basic.v b/ivtest/ivltests/sv_interface_port_basic.v new file mode 100644 index 000000000..a9054a523 --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_basic.v @@ -0,0 +1,47 @@ +// This tests a SystemVerilog interface-typed module port with an +// explicit modport and a named actual interface instance connection. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + logic [7:0] lhs; + logic [7:0] rhs; + + bus_if bus(); + + add_if dut(.bus(bus)); + + assign bus.lhs = lhs; + assign bus.rhs = rhs; + + initial begin + lhs = 8'd5; + rhs = 8'd7; + #1; + if (bus.sum !== 9'd12) begin + $display("FAILED"); + $finish; + end + $display("PASSED"); + end +endmodule + +interface bus_if #(parameter WIDTH = 8) (); + logic [WIDTH-1:0] lhs; + logic [WIDTH-1:0] rhs; + logic [WIDTH:0] sum; + + modport consumer( + input lhs, + input rhs, + output sum + ); + +endinterface + +module add_if #(parameter WIDTH = 8) ( + bus_if.consumer bus +); + assign bus.sum = bus.lhs + bus.rhs; +endmodule diff --git a/ivtest/ivltests/sv_interface_port_missing_modport_fail.v b/ivtest/ivltests/sv_interface_port_missing_modport_fail.v new file mode 100644 index 000000000..b1fa58f7b --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_missing_modport_fail.v @@ -0,0 +1,22 @@ +// This tests the diagnostic path for an interface-typed module port +// that names a modport missing from the interface definition. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + bus_if bus(); + bus_user dut(.bus(bus)); +endmodule + +interface bus_if (); + logic value; + + modport producer(output value); +endinterface + +module bus_user( + bus_if.consumer bus +); + initial $display("FAILED"); +endmodule diff --git a/ivtest/ivltests/sv_interface_port_missing_type_fail.v b/ivtest/ivltests/sv_interface_port_missing_type_fail.v new file mode 100644 index 000000000..df1a5a513 --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_missing_type_fail.v @@ -0,0 +1,11 @@ +// This tests the diagnostic path for an interface-typed module port +// whose interface type name is not declared. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module bus_user( + missing_if.consumer bus +); + initial $display("FAILED"); +endmodule diff --git a/ivtest/ivltests/sv_interface_port_modport_input_write_fail.v b/ivtest/ivltests/sv_interface_port_modport_input_write_fail.v new file mode 100644 index 000000000..d08f9016d --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_modport_input_write_fail.v @@ -0,0 +1,22 @@ +// This tests rejection of an assignment through a member declared input +// by the selected interface modport. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + bus_if bus(); + bus_user dut(.bus(bus)); +endmodule + +interface bus_if (); + logic value; + + modport consumer(input value); +endinterface + +module bus_user( + bus_if.consumer bus +); + assign bus.value = 1'b1; +endmodule diff --git a/ivtest/ivltests/sv_interface_port_non_interface_actual_fail.v b/ivtest/ivltests/sv_interface_port_non_interface_actual_fail.v new file mode 100644 index 000000000..018f14543 --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_non_interface_actual_fail.v @@ -0,0 +1,22 @@ +// This tests rejection of a non-interface actual connected to an +// interface-typed module port. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + logic value; + bus_user dut(.bus(value)); +endmodule + +interface bus_if (); + logic value; + + modport consumer(input value); +endinterface + +module bus_user( + bus_if.consumer bus +); + initial $display("FAILED"); +endmodule diff --git a/ivtest/ivltests/sv_interface_port_unlisted_member_fail.v b/ivtest/ivltests/sv_interface_port_unlisted_member_fail.v new file mode 100644 index 000000000..3457e330c --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_unlisted_member_fail.v @@ -0,0 +1,25 @@ +// This tests rejection of access to an interface member that is not +// listed in the selected modport. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + bus_if bus(); + bus_user dut(.bus(bus)); +endmodule + +interface bus_if (); + logic visible; + logic hidden; + + modport consumer(input visible); +endinterface + +module bus_user( + bus_if.consumer bus +); + logic sample; + + assign sample = bus.hidden; +endmodule diff --git a/ivtest/ivltests/sv_interface_port_wrong_type_fail.v b/ivtest/ivltests/sv_interface_port_wrong_type_fail.v new file mode 100644 index 000000000..33dfc40b1 --- /dev/null +++ b/ivtest/ivltests/sv_interface_port_wrong_type_fail.v @@ -0,0 +1,28 @@ +// This tests rejection of an actual interface instance whose type does +// not match the interface type of the formal module port. +// +// This file is placed into the Public Domain, for any use, without +// warranty. + +module test; + other_if bus(); + bus_user dut(.bus(bus)); +endmodule + +interface bus_if (); + logic value; + + modport consumer(input value); +endinterface + +interface other_if (); + logic value; + + modport consumer(input value); +endinterface + +module bus_user( + bus_if.consumer bus +); + initial $display("FAILED"); +endmodule diff --git a/ivtest/regress-vvp.list b/ivtest/regress-vvp.list index e9c9a1633..22f0917b6 100644 --- a/ivtest/regress-vvp.list +++ b/ivtest/regress-vvp.list @@ -269,6 +269,13 @@ sv_default_port_value3 vvp_tests/sv_default_port_value3.json sv_foreach9 vvp_tests/sv_foreach9.json sv_foreach10 vvp_tests/sv_foreach10.json sv_interface vvp_tests/sv_interface.json +sv_interface_port_basic vvp_tests/sv_interface_port_basic.json +sv_interface_port_missing_type_fail vvp_tests/sv_interface_port_missing_type_fail.json +sv_interface_port_missing_modport_fail vvp_tests/sv_interface_port_missing_modport_fail.json +sv_interface_port_non_interface_actual_fail vvp_tests/sv_interface_port_non_interface_actual_fail.json +sv_interface_port_wrong_type_fail vvp_tests/sv_interface_port_wrong_type_fail.json +sv_interface_port_modport_input_write_fail vvp_tests/sv_interface_port_modport_input_write_fail.json +sv_interface_port_unlisted_member_fail vvp_tests/sv_interface_port_unlisted_member_fail.json sv_literals vvp_tests/sv_literals.json sv_mixed_assign1 vvp_tests/sv_mixed_assign1.json sv_mixed_assign2 vvp_tests/sv_mixed_assign2.json diff --git a/ivtest/vvp_tests/sv_interface_port_basic.json b/ivtest/vvp_tests/sv_interface_port_basic.json new file mode 100644 index 000000000..33eef112d --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_basic.json @@ -0,0 +1,5 @@ +{ + "type" : "normal", + "source" : "sv_interface_port_basic.v", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_missing_modport_fail.json b/ivtest/vvp_tests/sv_interface_port_missing_modport_fail.json new file mode 100644 index 000000000..a86412965 --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_missing_modport_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_missing_modport_fail.v", + "gold" : "sv_interface_port_missing_modport_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_missing_type_fail.json b/ivtest/vvp_tests/sv_interface_port_missing_type_fail.json new file mode 100644 index 000000000..f43ed1e1c --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_missing_type_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_missing_type_fail.v", + "gold" : "sv_interface_port_missing_type_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_modport_input_write_fail.json b/ivtest/vvp_tests/sv_interface_port_modport_input_write_fail.json new file mode 100644 index 000000000..10d17db33 --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_modport_input_write_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_modport_input_write_fail.v", + "gold" : "sv_interface_port_modport_input_write_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_non_interface_actual_fail.json b/ivtest/vvp_tests/sv_interface_port_non_interface_actual_fail.json new file mode 100644 index 000000000..786b3e11d --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_non_interface_actual_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_non_interface_actual_fail.v", + "gold" : "sv_interface_port_non_interface_actual_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_unlisted_member_fail.json b/ivtest/vvp_tests/sv_interface_port_unlisted_member_fail.json new file mode 100644 index 000000000..802f08a7e --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_unlisted_member_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_unlisted_member_fail.v", + "gold" : "sv_interface_port_unlisted_member_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/ivtest/vvp_tests/sv_interface_port_wrong_type_fail.json b/ivtest/vvp_tests/sv_interface_port_wrong_type_fail.json new file mode 100644 index 000000000..d0d34c17b --- /dev/null +++ b/ivtest/vvp_tests/sv_interface_port_wrong_type_fail.json @@ -0,0 +1,6 @@ +{ + "type" : "CE", + "source" : "sv_interface_port_wrong_type_fail.v", + "gold" : "sv_interface_port_wrong_type_fail", + "iverilog-args" : [ "-g2012" ] +} diff --git a/net_scope.cc b/net_scope.cc index d90b49b82..6292817a2 100644 --- a/net_scope.cc +++ b/net_scope.cc @@ -812,6 +812,37 @@ const NetScope* NetScope::child(const hname_t&name) const return cur->second; } +void NetScope::add_interface_port_alias(perm_string formal_name, + NetScope*actual_scope, + const PModport*modport) +{ + ivl_assert(*this, actual_scope); + interface_port_aliases_[formal_name] = interface_port_alias_t(actual_scope, modport); +} + +const NetScope::interface_port_alias_t* +NetScope::find_interface_port_alias(perm_string formal_name) const +{ + map::const_iterator cur; + cur = interface_port_aliases_.find(formal_name); + if (cur == interface_port_aliases_.end()) + return 0; + + return &cur->second; +} + +NetScope* NetScope::find_interface_port_alias_scope(perm_string formal_name) const +{ + const interface_port_alias_t*cur = find_interface_port_alias(formal_name); + return cur? cur->actual_scope : 0; +} + +const PModport* NetScope::find_interface_port_modport(perm_string formal_name) const +{ + const interface_port_alias_t*cur = find_interface_port_alias(formal_name); + return cur? cur->modport : 0; +} + /* Helper function to see if the given scope is defined in a class and if * so return the class scope. */ const NetScope* NetScope::get_class_scope() const @@ -867,6 +898,8 @@ bool NetScope::symbol_exists(perm_string sym) return true; if (find_event(sym)) return true; + if (find_interface_port_alias(sym)) + return true; return false; } diff --git a/netlist.h b/netlist.h index d1eb80764..7011c5a9f 100644 --- a/netlist.h +++ b/netlist.h @@ -79,6 +79,7 @@ class NetEvWait; class PClass; class PExpr; class PFunction; +class PModport; class PPackage; class PTaskFunc; class PWire; @@ -1043,6 +1044,26 @@ class NetScope : public Definitions, public Attrib { const NetScope* parent() const { return up_; } const NetScope* child(const hname_t&name) const; + struct interface_port_alias_t { + interface_port_alias_t() : actual_scope(0), modport(0) { } + interface_port_alias_t(NetScope*actual, const PModport*mp) + : actual_scope(actual), modport(mp) { } + + NetScope*actual_scope; + const PModport*modport; + }; + + /* Interface-typed module formals are represented as aliases to + concrete interface instance scopes. These are deliberately kept + out of the real child-scope map; only alias-aware lookup paths + should traverse them. */ + void add_interface_port_alias(perm_string formal_name, + NetScope*actual_scope, + const PModport*modport); + const interface_port_alias_t* find_interface_port_alias(perm_string formal_name) const; + NetScope* find_interface_port_alias_scope(perm_string formal_name) const; + const PModport* find_interface_port_modport(perm_string formal_name) const; + /* A helper function to find the enclosing class scope. */ const NetScope* get_class_scope() const; @@ -1347,6 +1368,7 @@ class NetScope : public Definitions, public Attrib { NetScope*unit_; NetScope*up_; std::map children_; + std::map interface_port_aliases_; unsigned lcounter_; bool need_const_func_, is_const_func_, is_auto_, is_cell_, calls_stask_; diff --git a/netmisc.h b/netmisc.h index be460ad04..ff4aa0e01 100644 --- a/netmisc.h +++ b/netmisc.h @@ -50,6 +50,9 @@ struct symbol_search_results { type = 0; eve = 0; decl_after_use = 0; + interface_alias_scope = 0; + interface_alias_target = 0; + interface_alias_modport = 0; } inline bool is_scope() const { @@ -76,6 +79,10 @@ struct symbol_search_results { return "nothing found"; } + inline bool through_interface_alias() const { + return interface_alias_target != 0; + } + // Scope where symbol was located. This is set in all cases, // assuming the search succeeded. NetScope*scope; @@ -93,6 +100,14 @@ struct symbol_search_results { // one is retained. const LineInfo*decl_after_use; + // If lookup traversed an interface-typed formal port alias, these + // fields describe the alias edge. The resolved object remains in the + // normal scope/net/parameter/event fields. + NetScope*interface_alias_scope; + perm_string interface_alias_name; + NetScope*interface_alias_target; + const PModport*interface_alias_modport; + // Store bread crumbs of the search here. The path_tail is the parts // of the original path that were not found, or are after an object // (and so are probably members or methods). @@ -128,6 +143,10 @@ extern bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, const pform_scoped_name_t &path, unsigned lexical_pos, struct symbol_search_results*res); +extern bool check_interface_modport_access(const LineInfo *li, Design *des, + const symbol_search_results &res, + bool is_write); + /* * This function transforms an expression by either zero or sign extending * the high bits until the expression has the desired width. This may mean diff --git a/parse.y b/parse.y index 10184aeff..99949e7ee 100644 --- a/parse.y +++ b/parse.y @@ -461,6 +461,29 @@ Module::port_t *module_declare_port(const YYLTYPE&loc, char *id, return port; } +Module::port_t *module_declare_interface_port(const YYLTYPE&loc, char *type, + char *modport, char *id, + std::list *attributes) +{ + pform_requires_sv(loc, "Interface port declaration"); + + Module::port_t *port = pform_module_interface_port_reference( + loc, lex_strings.make(type), lex_strings.make(modport), + lex_strings.make(id)); + + delete[] type; + delete[] modport; + delete[] id; + + pform_module_define_interface_port(loc, port, attributes); + + port_declaration_context.port_type = NetNet::NOT_A_PORT; + port_declaration_context.port_net_type = NetNet::NONE; + port_declaration_context.data_type = nullptr; + + return port; +} + %} %union { @@ -4603,10 +4626,20 @@ list_of_port_declarations { std::vector *ports = $1; Module::port_t* port; - port = module_declare_port(@4, $4, port_declaration_context.port_type, - port_declaration_context.port_net_type, - port_declaration_context.data_type, - $5, $6, $3); + if (port_declaration_context.port_type == NetNet::NOT_A_PORT) { + yyerror(@4, "error: Incomplete interface port declaration."); + delete[]$4; + delete $5; + delete $6; + delete $3; + port = 0; + } else { + port = module_declare_port(@4, $4, + port_declaration_context.port_type, + port_declaration_context.port_net_type, + port_declaration_context.data_type, + $5, $6, $3); + } ports->push_back(port); $$ = ports; } @@ -4622,6 +4655,9 @@ port_declaration : attribute_list_opt port_direction net_type_or_var_opt data_type_or_implicit IDENTIFIER dimensions_opt initializer_opt { $$ = module_declare_port(@5, $5, $2, $3, $4, $6, $7, $1); } + | attribute_list_opt IDENTIFIER '.' IDENTIFIER IDENTIFIER + { $$ = module_declare_interface_port(@5, $2, $4, $5, $1); + } | attribute_list_opt net_type_or_var data_type_or_implicit IDENTIFIER dimensions_opt initializer_opt { pform_requires_sv(@4, "Partial ANSI port declaration"); $$ = module_declare_port(@4, $4, port_declaration_context.port_type, @@ -5715,6 +5751,10 @@ port : port_reference { $$ = $1; } + | IDENTIFIER '.' IDENTIFIER IDENTIFIER + { $$ = module_declare_interface_port(@4, $1, $3, $4, 0); + } + /* This syntax attaches an external name to the port reference so that the caller can bind by name to non-trivial port references. The port_t object gets its PWire from the diff --git a/pform.cc b/pform.cc index c7e337b63..affc5c775 100644 --- a/pform.cc +++ b/pform.cc @@ -1386,6 +1386,33 @@ Module::port_t* pform_module_port_reference(const struct vlltype&loc, return ptmp; } +Module::port_t* pform_module_interface_port_reference( + const struct vlltype&loc, + perm_string interface_type, + perm_string modport_name, + perm_string name) +{ + Module::port_t*ptmp = new Module::port_t; + + ptmp->port_kind = Module::port_t::P_INTERFACE; + ptmp->name = name; + ptmp->interface_type = interface_type; + ptmp->modport_name = modport_name; + ptmp->lexical_pos = loc.lexical_pos; + + return ptmp; +} + +void pform_module_define_interface_port(const struct vlltype&loc, + Module::port_t*port, + list*attr) +{ + ivl_assert(loc, port); + ivl_assert(loc, port->is_interface_port()); + + delete attr; +} + void pform_module_set_ports(vector*ports) { assert(! pform_cur_module.empty()); diff --git a/pform.h b/pform.h index f7d5b491b..2f83c5541 100644 --- a/pform.h +++ b/pform.h @@ -167,6 +167,14 @@ extern void pform_module_define_port(const struct vlltype&li, extern Module::port_t* pform_module_port_reference(const struct vlltype&loc, perm_string name); +extern Module::port_t* pform_module_interface_port_reference( + const struct vlltype&loc, + perm_string interface_type, + perm_string modport_name, + perm_string name); +extern void pform_module_define_interface_port(const struct vlltype&loc, + Module::port_t*port, + std::list*attr); extern void pform_endmodule(const char*, bool inside_celldefine, Module::UCDriveType uc_drive_def); diff --git a/symbol_search.cc b/symbol_search.cc index 61a28d4ec..7b83dd664 100644 --- a/symbol_search.cc +++ b/symbol_search.cc @@ -25,6 +25,7 @@ # include "compiler.h" # include "PPackage.h" # include "PWire.h" +# include "PModport.h" # include "ivl_assert.h" using namespace std; @@ -75,6 +76,25 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, if (! flag) return false; + if (res->net && res->path_tail.empty() && !res->path_head.empty()) { + name_component_t prefix_tail = res->path_head.back(); + if (prefix_tail.index.empty() && + res->scope->child_byname(prefix_tail.name)) { + bool eval_flag = false; + hname_t path_item = eval_path_component(des, start_scope, + prefix_tail, eval_flag); + if (eval_flag) { + cerr << li->get_fileline() << ": XXXXX: Errors evaluating scope index" << endl; + } else if (NetScope*chld = res->scope->child(path_item)) { + if (chld->is_interface()) { + res->scope = chld; + res->net = 0; + res->type = 0; + } + } + } + } + // The prefix is found to be something besides a scope. Put the // tail into the path_tail of the result, and return success. The // caller needs to deal with that tail bit. Note that the @@ -292,6 +312,27 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, } } + if (path_tail.index.empty()) { + if (const NetScope::interface_port_alias_t*alias = + scope->find_interface_port_alias(path_tail.name)) { + path.push_back(path_tail); + res->scope = alias->actual_scope; + res->path_head = path; + res->interface_alias_scope = scope; + res->interface_alias_name = path_tail.name; + res->interface_alias_target = alias->actual_scope; + res->interface_alias_modport = alias->modport; + + if (debug_scopes || debug_elaborate) { + cerr << li->get_fileline() << ": symbol_search: " + << "Interface alias " << path_tail.name + << " -> " << scope_path(alias->actual_scope) << endl; + } + + return true; + } + } + // Don't scan up if we are searching within a prefixed scope. if (prefix_scope) break; @@ -387,3 +428,34 @@ bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, return symbol_search(li, des, search_scope, path.name, lexical_pos, res, search_scope, prefix_scope); } + +bool check_interface_modport_access(const LineInfo *li, Design *des, + const symbol_search_results &res, + bool is_write) +{ + if (!res.through_interface_alias() || !res.interface_alias_modport || !res.net) + return true; + + const PModport *modport = res.interface_alias_modport; + perm_string member = res.net->name(); + map::const_iterator cur = + modport->simple_ports.find(member); + + if (cur == modport->simple_ports.end()) { + cerr << li->get_fileline() << ": error: Interface member `" + << member << "' is not listed in modport `" + << modport->name() << "'." << endl; + des->errors += 1; + return false; + } + + if (is_write && cur->second.first == NetNet::PINPUT) { + cerr << li->get_fileline() << ": error: Cannot assign to input " + "modport member `" << member << "' through interface port `" + << res.interface_alias_name << "'." << endl; + des->errors += 1; + return false; + } + + return true; +} diff --git a/vvp/Makefile.in b/vvp/Makefile.in index c924dda85..a9315c8b5 100644 --- a/vvp/Makefile.in +++ b/vvp/Makefile.in @@ -135,6 +135,10 @@ Makefile: $(srcdir)/Makefile.in dep: mkdir dep +# Older dependency files may refer to ivl_dlfcn.h from before the +# shared dlopen wrapper was moved to the top-level source directory. +ivl_dlfcn.h: $(srcdir)/../ivl_dlfcn.h + ifeq (@LIBVVP@,yes) CPPFLAGS+= -fpic