Catch partial support of SVI

docs/source/using_yosys/verilog.rst

@@ -9,7 +9,7 @@ Yosys and there are currently no plans to add support
 for them:
 
 - Non-synthesizable language features as defined in
-	IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002
+  IEC 62142(E):2005 / IEEE Std. 1364.1(E):2002
 
 - The ``tri``, ``triand`` and ``trior`` net types
 
@@ -356,21 +356,29 @@ from SystemVerilog:
   files being read into the same design afterwards.
 
 - typedefs are supported (including inside packages)
-	- type casts are currently not supported
+
+  - type casts are currently not supported
 
 - enums are supported (including inside packages)
-	- but are currently not strongly typed
+
+  - but are currently not strongly typed
 
 - packed structs and unions are supported
-	- arrays of packed structs/unions are currently not supported
-	- structure literals are currently not supported
+
+  - arrays of packed structs/unions are currently not supported
+  - structure literals are currently not supported
 
 - multidimensional arrays are supported
-	- array assignment of unpacked arrays is currently not supported
-	- array literals are currently not supported
 
-- SystemVerilog interfaces (SVIs) are supported. Modports for specifying whether
-  ports are inputs or outputs are supported.
+  - array assignment of unpacked arrays is currently not supported
+  - array literals are currently not supported
+
+- SystemVerilog interfaces (SVIs), including modports for specifying whether
+  ports are inputs or outputs, are partially supported.
+
+  - interfaces must be provided as *named* arguments, not positional arguments.
+    i.e. ``foo bar(.intf(intf0), .x(x));`` is supported but ``foo bar(intf0,
+    x);`` is not.
 
 - Assignments within expressions are supported.
 

passes/hierarchy/hierarchy.cc

@@ -156,6 +156,21 @@ std::string basic_cell_type(const std::string celltype, int pos[3] = nullptr) {
 	return basicType;
 }
 
+// Try to read an IdString as a numbered connection name ("$123" or similar),
+// writing the result to dst. If the string isn't of the right format, ignore
+// dst and return false.
+bool read_id_num(RTLIL::IdString str, int *dst)
+{
+	log_assert(dst);
+
+	const char *c_str = str.c_str();
+	if (c_str[0] != '$' || !('0' <= c_str[1] && c_str[1] <= '9'))
+		return false;
+
+	*dst = atoi(c_str + 1);
+	return true;
+}
+
 // A helper struct for expanding a module's interface connections in expand_module
 struct IFExpander
 {
@@ -283,15 +298,42 @@ struct IFExpander
 	                   RTLIL::IdString       conn_name,
 	                   const RTLIL::SigSpec &conn_signals)
 	{
-		// Check if the connection is present as an interface in the sub-module's port list
-		const RTLIL::Wire *wire = submodule.wire(conn_name);
-		if (!wire || !wire->get_bool_attribute(ID::is_interface))
+		// Does the connection look like an interface
+		if (
+			conn_signals.size() != 1 ||
+			conn_signals[0].wire == nullptr ||
+			conn_signals[0].wire->get_bool_attribute(ID::is_interface) == false ||
+			conn_signals[0].wire->name.str().find("$dummywireforinterface") != 0
+		)
 			return;
 
+		// Check if the connection is present as an interface in the sub-module's port list
+		int id;
+		if (read_id_num(conn_name, &id)) {
+			/* Interface expansion is incompatible with positional arguments
+			 * during expansion, the port list gets each interface signal
+			 * inserted after the interface itself which means that the argument
+			 * positions in the parent module no longer match.
+			 *
+			 * Supporting this would require expanding the interfaces in the
+			 * parent module, renumbering the arguments to match, and then
+			 * iterating over the ports list to find the matching interface
+			 * (refactoring on_interface to accept different conn_names on the
+			 * parent and child).
+			 */
+			log_error("Unable to connect `%s' to submodule `%s' with positional interface argument `%s'!\n",
+				module.name,
+				submodule.name,
+				conn_signals[0].wire->name.str().substr(23)
+			);
+		} else {
+			// Lookup connection by name
+			const RTLIL::Wire *wire = submodule.wire(conn_name);
+			if (!wire || !wire->get_bool_attribute(ID::is_interface))
+				return;
+			}
 		// If the connection looks like an interface, handle it.
-		const auto &bits = conn_signals;
-		if (bits.size() == 1 && bits[0].wire->get_bool_attribute(ID::is_interface))
-			on_interface(submodule, conn_name, conn_signals);
+		on_interface(submodule, conn_name, conn_signals);
 	}
 
 	// Iterate over the connections in a cell, tracking any interface
@@ -376,21 +418,6 @@ RTLIL::Module *get_module(RTLIL::Design                  &design,
 	return nullptr;
 }
 
-// Try to read an IdString as a numbered connection name ("$123" or similar),
-// writing the result to dst. If the string isn't of the right format, ignore
-// dst and return false.
-bool read_id_num(RTLIL::IdString str, int *dst)
-{
-	log_assert(dst);
-
-	const char *c_str = str.c_str();
-	if (c_str[0] != '$' || !('0' <= c_str[1] && c_str[1] <= '9'))
-		return false;
-
-	*dst = atoi(c_str + 1);
-	return true;
-}
-
 // Check that the connections on the cell match those that are defined
 // on the type: each named connection should match the name of a port
 // and each positional connection should have an index smaller than

tests/svinterfaces/positional_args.ys

@@ -0,0 +1,33 @@
+read_verilog -sv << EOF
+interface simple_if;
+    logic receiver;
+    logic driver;
+endinterface
+
+module driver_mod(simple_if intf, input in);
+    assign intf.driver = in;
+endmodule
+
+module receiver_mod(simple_if intf);
+    assign intf.receiver = intf.driver;
+endmodule
+
+module top(
+    input logic [1:0] inputs,
+    output logic [1:0] outputs
+);
+    simple_if intf0();
+    simple_if intf1();
+
+    driver_mod d0(intf0, inputs[0]);
+    driver_mod d1(intf1, inputs[1]);
+
+    receiver_mod r0(intf0);
+    receiver_mod r1(intf1);
+
+    assign outputs = {intf0.receiver, intf1.receiver};
+endmodule
+EOF
+
+logger -expect error "Unable to connect.* with positional interface" 1
+hierarchy -top top

tests/svinterfaces/run-test.sh

@@ -5,3 +5,4 @@
 
 ./run_simple.sh load_and_derive
 ./run_simple.sh resolve_types
+./run_simple.sh positional_args