fix(interface): address port array review feedback

This commit is contained in:
Jose Tejada 2026-05-11 07:44:43 +02:00
parent 417ab54445
commit 377881b723
43 changed files with 142 additions and 134 deletions

View File

@ -1255,13 +1255,19 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
vector<bool>&pins_fromwc,
vector<bool>&pins_is_explicitly_not_connected) const
{
// 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;
@ -1269,7 +1275,6 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
if (sr.net != 0 ||
(rmod->ports[j]->is_interface_port() &&
sr.scope != 0 && sr.scope->is_interface())) {
pins_fromwc[j] = true;
pins[j] = new PEIdent(rmod->ports[j]->name, UINT_MAX, true);
pins[j]->set_lineno(get_lineno());
pins[j]->set_file(get_file());
@ -1279,7 +1284,13 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
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 "
@ -1288,10 +1299,16 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
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." <<
@ -1300,16 +1317,26 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
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() <<
@ -1326,7 +1353,7 @@ bool PGModule::match_module_ports_(Design*des, const Module*rmod,
}
struct interface_actual_scope_t {
interface_actual_scope_t() : scope(0), modport(0) { }
interface_actual_scope_t() : scope(nullptr), modport(nullptr) { }
NetScope*scope;
const PModport*modport;
@ -1464,8 +1491,7 @@ static bool resolve_interface_actual_array(const PExpr*actual,
res.display_name = name;
for (NetScope*scope = parent_scope ; scope ; scope = scope->parent()) {
map<perm_string,NetScope::scope_vec_t>::const_iterator arr =
scope->instance_arrays.find(name);
auto arr = scope->instance_arrays.find(name);
if (arr != scope->instance_arrays.end()) {
for (unsigned idx = 0 ; idx < arr->second.size() ; idx += 1) {
NetScope*inst = arr->second[idx];
@ -1475,7 +1501,7 @@ static bool resolve_interface_actual_array(const PExpr*actual,
if (!hname.has_numbers())
return false;
res.elements[hname.peek_number(0)] =
NetScope::interface_port_alias_t(inst, 0);
NetScope::interface_port_alias_t(inst, nullptr);
}
return true;
}
@ -1521,7 +1547,7 @@ bool PGModule::bind_interface_ports_(Design*des, const Module*rmod,
flag = false;
continue;
}
bool formal_is_array = port->interface_unpacked_dimensions != 0;
bool formal_is_array = port->interface_unpacked_dimensions != nullptr;
interface_formal_port_t formal;
resolve_interface_formal_port(pins[idx], des, port, formal, false);
@ -1552,8 +1578,7 @@ bool PGModule::bind_interface_ports_(Design*des, const Module*rmod,
unsigned pos = 0;
bool array_ok = true;
for (map<long,NetScope::interface_port_alias_t>::const_iterator cur =
actual_array.elements.begin()
for (auto cur = actual_array.elements.begin()
; cur != actual_array.elements.end() ; ++cur, ++pos) {
NetScope*actual_scope = cur->second.actual_scope;
if (!actual_scope || !actual_scope->is_interface()) {

View File

@ -1,3 +1,3 @@
ivltests/sv_interface_port_forwarding_restrict_fail.v:23: error: Interface member `hidden' is not listed in modport `consumer'.
ivltests/sv_interface_port_forwarding_restrict_fail.v:23: error: Unable to elaborate r-value: bus.hidden
ivltests/sv_interface_port_forwarding_restrict_fail.v:30: error: Interface member `hidden' is not listed in modport `consumer'.
ivltests/sv_interface_port_forwarding_restrict_fail.v:30: error: Unable to elaborate r-value: bus.hidden
2 error(s) during elaboration.

View File

@ -1,2 +0,0 @@
ivltests/sv_interface_port_missing_type_fail.v:7: error: Interface port bus uses unknown interface type `missing_if'.
1 error(s) during elaboration.

View File

@ -1,2 +1,2 @@
ivltests/sv_interface_port_positional_unconnected_fail.v:7: error: Interface port `bus' of module bus_user is not connected.
ivltests/sv_interface_port_positional_unconnected_fail.v:13: error: Interface port `bus' of module bus_user is not connected.
Elaboration failed

View File

@ -1,2 +0,0 @@
ivltests/sv_interface_port_unmodported_missing_type_fail.v:7: error: Interface port bus uses unknown interface type `missing_if'.
1 error(s) during elaboration.

View File

@ -4,6 +4,14 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic value;
logic mirror;
logic hidden;
modport consumer(input value, output mirror);
endinterface
module test;
bus_if bus();
@ -31,11 +39,3 @@ module child(
);
assign bus.mirror = bus.value;
endmodule
interface bus_if ();
logic value;
logic mirror;
logic hidden;
modport consumer(input value, output mirror);
endinterface

View File

@ -4,6 +4,13 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic value;
logic hidden;
modport consumer(input value);
endinterface
module test;
bus_if bus();
parent dut(.bus(bus));
@ -22,10 +29,3 @@ module child(
assign sample = bus.hidden;
endmodule
interface bus_if ();
logic value;
logic hidden;
modport consumer(input value);
endinterface

View File

@ -1,11 +0,0 @@
// 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

View File

@ -3,6 +3,13 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic value;
logic sample;
modport consumer(input value, output sample);
endinterface
module test;
bus_if bus();
@ -24,10 +31,3 @@ module bus_user(
);
assign bus.sample = bus.value;
endmodule
interface bus_if ();
logic value;
logic sample;
modport consumer(input value, output sample);
endinterface

View File

@ -3,6 +3,12 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic value;
modport consumer(input value);
endinterface
module test;
bus_user dut();
endmodule
@ -11,9 +17,3 @@ module bus_user(
bus_if.consumer bus
);
endmodule
interface bus_if ();
logic value;
modport consumer(input value);
endinterface

View File

@ -4,6 +4,12 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic [7:0] lhs;
logic [7:0] rhs;
logic [8:0] sum;
endinterface
module test;
logic [7:0] lhs;
logic [7:0] rhs;
@ -32,9 +38,3 @@ module add_if(
);
assign bus.sum = bus.lhs + bus.rhs;
endmodule
interface bus_if ();
logic [7:0] lhs;
logic [7:0] rhs;
logic [8:0] sum;
endinterface

View File

@ -1,11 +0,0 @@
// This tests the diagnostic path for an unmodported 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 bus
);
initial $display("FAILED");
endmodule

View File

@ -3,6 +3,13 @@
// This file is placed into the Public Domain, for any use, without
// warranty.
interface bus_if ();
logic value;
logic sample;
modport consumer(input value, output sample);
endinterface
module test;
bus_if bus();
@ -24,10 +31,3 @@ module bus_user(
);
assign bus.sample = bus.value;
endmodule
interface bus_if ();
logic value;
logic sample;
modport consumer(input value, output sample);
endinterface

View File

@ -270,14 +270,12 @@ 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_interface_port_unmodported_basic vvp_tests/sv_interface_port_unmodported_basic.json
sv_interface_port_unmodported_missing_type_fail vvp_tests/sv_interface_port_unmodported_missing_type_fail.json
sv_interface_port_forwarding vvp_tests/sv_interface_port_forwarding.json
sv_interface_port_forwarding_restrict_fail vvp_tests/sv_interface_port_forwarding_restrict_fail.json
sv_interface_port_positional vvp_tests/sv_interface_port_positional.json

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_array_basic.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_array_modport_restrict_fail.v",
"gold" : "sv_interface_port_array_modport_restrict_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_array_size_mismatch_fail.v",
"gold" : "sv_interface_port_array_size_mismatch_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_basic.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_forwarding.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_forwarding_restrict_fail.v",
"gold" : "sv_interface_port_forwarding_restrict_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_indexed_actual.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_indexed_actual_generate.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_missing_modport_fail.v",
"gold" : "sv_interface_port_missing_modport_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,6 +0,0 @@
{
"type" : "CE",
"source" : "sv_interface_port_missing_type_fail.v",
"gold" : "sv_interface_port_missing_type_fail",
"iverilog-args" : [ "-g2012" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_modport_input_write_fail.v",
"gold" : "sv_interface_port_modport_input_write_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_non_interface_actual_fail.v",
"gold" : "sv_interface_port_non_interface_actual_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_plain_ansi_regression.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_positional.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_positional_unconnected_fail.v",
"gold" : "sv_interface_port_positional_unconnected_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_typedef_ansi_regression.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_unlisted_member_fail.v",
"gold" : "sv_interface_port_unlisted_member_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_unmodported_basic.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -1,6 +0,0 @@
{
"type" : "CE",
"source" : "sv_interface_port_unmodported_missing_type_fail.v",
"gold" : "sv_interface_port_unmodported_missing_type_fail",
"iverilog-args" : [ "-g2012" ]
}

View File

@ -1,5 +1,5 @@
{
"type" : "normal",
"source" : "sv_interface_port_wildcard.v",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -2,5 +2,5 @@
"type" : "CE",
"source" : "sv_interface_port_wrong_type_fail.v",
"gold" : "sv_interface_port_wrong_type_fail",
"iverilog-args" : [ "-g2012" ]
"iverilog-args" : [ "-g2005-sv" ]
}

View File

@ -417,6 +417,13 @@ TU [munpf]
}
}
/* If this identifier names a previously declared interface, then
return this as an INTERFACE_IDENTIFIER instead. */
if (rc == IDENTIFIER && gn_system_verilog()) {
if (pform_test_interface_identifier(yylval.text))
rc = INTERFACE_IDENTIFIER;
}
/* If this identifier names a previously declared type, then
return this as a TYPE_IDENTIFIER instead. */
if (rc == IDENTIFIER && gn_system_verilog()) {
@ -442,6 +449,10 @@ TU [munpf]
return PACKAGE_IDENTIFIER;
}
}
if (gn_system_verilog()) {
if (pform_test_interface_identifier(yylval.text))
return INTERFACE_IDENTIFIER;
}
if (gn_system_verilog()) {
if (typedef_t*type = pform_test_type_identifier(yylloc, yylval.text)) {
yylval.type_identifier.text = yylval.text;

View File

@ -845,13 +845,11 @@ const NetScope::interface_port_alias_t*
NetScope::find_interface_port_alias_element(perm_string formal_name,
long index) const
{
map<perm_string,map<long,interface_port_alias_t> >::const_iterator arr;
arr = interface_port_alias_arrays_.find(formal_name);
auto arr = interface_port_alias_arrays_.find(formal_name);
if (arr == interface_port_alias_arrays_.end())
return 0;
map<long,interface_port_alias_t>::const_iterator cur;
cur = arr->second.find(index);
auto cur = arr->second.find(index);
if (cur == arr->second.end())
return 0;
@ -861,8 +859,7 @@ NetScope::find_interface_port_alias_element(perm_string formal_name,
const map<long,NetScope::interface_port_alias_t>*
NetScope::find_interface_port_alias_array(perm_string formal_name) const
{
map<perm_string,map<long,interface_port_alias_t> >::const_iterator cur;
cur = interface_port_alias_arrays_.find(formal_name);
auto cur = interface_port_alias_arrays_.find(formal_name);
if (cur == interface_port_alias_arrays_.end())
return 0;

View File

@ -1045,7 +1045,7 @@ class NetScope : public Definitions, public Attrib {
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() : actual_scope(nullptr), modport(nullptr) { }
interface_port_alias_t(NetScope*actual, const PModport*mp)
: actual_scope(actual), modport(mp) { }

28
parse.y
View File

@ -606,7 +606,7 @@ Module::port_t *module_declare_interface_port(const YYLTYPE&loc, char *type,
enum typedef_t::basic_type typedef_basic_type;
};
%token <text> IDENTIFIER SYSTEM_IDENTIFIER STRING TIME_LITERAL
%token <text> IDENTIFIER INTERFACE_IDENTIFIER SYSTEM_IDENTIFIER STRING TIME_LITERAL
%token <type_identifier> TYPE_IDENTIFIER
%token <package> PACKAGE_IDENTIFIER
%token <discipline> DISCIPLINE_IDENTIFIER
@ -4658,10 +4658,10 @@ 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 dimensions_opt
| attribute_list_opt INTERFACE_IDENTIFIER '.' IDENTIFIER IDENTIFIER dimensions_opt
{ $$ = module_declare_interface_port(@5, $2, $4, $5, $6, $1);
}
| attribute_list_opt IDENTIFIER IDENTIFIER dimensions_opt
| attribute_list_opt INTERFACE_IDENTIFIER IDENTIFIER dimensions_opt
{ $$ = module_declare_interface_port(@3, $2, 0, $3, $4, $1);
}
| attribute_list_opt net_type_or_var data_type_or_implicit IDENTIFIER dimensions_opt initializer_opt
@ -5207,6 +5207,13 @@ module_item
delete[]$2;
}
| attribute_list_opt
INTERFACE_IDENTIFIER parameter_value_opt gate_instance_list ';'
{ perm_string tmp1 = lex_strings.make($2);
pform_make_modgates(@2, tmp1, $3, $4, $1);
delete[]$2;
}
| attribute_list_opt
IDENTIFIER parameter_value_opt error ';'
{ yyerror(@2, "error: Invalid module instantiation");
@ -5214,6 +5221,13 @@ module_item
if ($1) delete $1;
}
| attribute_list_opt
INTERFACE_IDENTIFIER parameter_value_opt error ';'
{ yyerror(@2, "error: Invalid module instantiation");
delete[]$2;
if ($1) delete $1;
}
/* Continuous assignment can have an optional drive strength, then
an optional delay3 that applies to all the assignments in the
cont_assign_list. */
@ -5757,14 +5771,6 @@ port
: port_reference
{ $$ = $1; }
| IDENTIFIER '.' IDENTIFIER IDENTIFIER dimensions_opt
{ $$ = module_declare_interface_port(@4, $1, $3, $4, $5, 0);
}
| IDENTIFIER IDENTIFIER dimensions_opt
{ $$ = module_declare_interface_port(@2, $1, 0, $2, $3, 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

View File

@ -92,6 +92,11 @@ extern void lex_in_package_scope(PPackage*pkg);
extern typedef_t* pform_test_type_identifier(const YYLTYPE&loc, const char*txt);
extern typedef_t* pform_test_type_identifier(PPackage*pkg, const char*txt);
/*
* Test if this identifier is a previously declared interface name.
*/
extern bool pform_test_interface_identifier(const char*txt);
/*
* Test if this identifier is a package name. The pform needs to help
* the lexor here because the parser detects packages and saves them.

View File

@ -926,6 +926,14 @@ typedef_t* pform_test_type_identifier(const struct vlltype&loc, const char*txt)
return 0;
}
bool pform_test_interface_identifier(const char*txt)
{
perm_string name = lex_strings.make(txt);
map<perm_string,Module*>::const_iterator cur = pform_modules.find(name);
return cur != pform_modules.end() && cur->second->is_interface;
}
PECallFunction* pform_make_call_function(const struct vlltype&loc,
const pform_name_t&name,
const list<named_pexpr_t> &parms)

View File

@ -331,7 +331,7 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope,
return true;
}
} else {
} else if (scope->find_interface_port_alias_array(path_tail.name)) {
bool flag = false;
hname_t path_item = eval_path_component(des, start_scope, path_tail, flag);
if (!flag && path_item.has_numbers() == 1) {

View File

@ -135,10 +135,6 @@ 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