Merge c599d6a67e into a2aeef6c96

Fix regression in configuring ABC techmapping

Makefile

@@ -161,7 +161,7 @@ ifeq ($(OS), Haiku)
 CXXFLAGS += -D_DEFAULT_SOURCE
 endif
 
-YOSYS_VER := 0.58+132
+YOSYS_VER := 0.58+138
 YOSYS_MAJOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f1)
 YOSYS_MINOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f2 | cut -d'+' -f1)
 YOSYS_COMMIT := $(shell echo $(YOSYS_VER) | cut -d'+' -f2)
@@ -366,7 +366,7 @@ CXXFLAGS += -I$(PYBIND11_INCLUDE) -DYOSYS_ENABLE_PYTHON
 CXXFLAGS += $(shell $(PYTHON_CONFIG) --includes) -DYOSYS_ENABLE_PYTHON
 
 OBJS += $(PY_WRAPPER_FILE).o
-PY_GEN_SCRIPT = pyosys/generator.py
+PY_GEN_SCRIPT = $(YOSYS_SRC)/pyosys/generator.py
 PY_WRAP_INCLUDES := $(shell $(UV_ENV) $(PYTHON_EXECUTABLE) $(PY_GEN_SCRIPT) --print-includes)
 endif # ENABLE_PYOSYS
 

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

passes/techmap/abc.cc

@@ -117,15 +117,6 @@ struct gate_t
 	std::string bit_str;
 };
 
-bool map_mux4;
-bool map_mux8;
-bool map_mux16;
-
-bool markgroups;
-
-pool<std::string> enabled_gates;
-bool cmos_cost;
-
 struct AbcConfig
 {
 	std::string global_tempdir_name;
@@ -146,6 +137,12 @@ struct AbcConfig
 	bool show_tempdir = false;
 	bool sop_mode = false;
 	bool abc_dress = false;
+	bool map_mux4 = false;
+	bool map_mux8 = false;
+	bool map_mux16 = false;
+	bool markgroups = false;
+	pool<std::string> enabled_gates;
+	bool cmos_cost = false;
 };
 
 struct AbcSigVal {
@@ -1382,7 +1379,7 @@ void emit_global_input_files(const AbcConfig &config)
 			fprintf(f, "%d %d.00 1.00\n", i+1, config.lut_costs.at(i));
 		fclose(f);
 	} else {
-		auto &cell_cost = cmos_cost ? CellCosts::cmos_gate_cost() : CellCosts::default_gate_cost();
+		auto &cell_cost = config.cmos_cost ? CellCosts::cmos_gate_cost() : CellCosts::default_gate_cost();
 
 		std::string buffer = stringf("%s/stdcells.genlib", config.global_tempdir_name.c_str());
 		FILE *f = fopen(buffer.c_str(), "wt");
@@ -1392,39 +1389,39 @@ void emit_global_input_files(const AbcConfig &config)
 		fprintf(f, "GATE ONE     1 Y=CONST1;\n");
 		fprintf(f, "GATE BUF    %d Y=A;                  PIN * NONINV  1 999 1 0 1 0\n", cell_cost.at(ID($_BUF_)));
 		fprintf(f, "GATE NOT    %d Y=!A;                 PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_NOT_)));
-		if (enabled_gates.count("AND"))
+		if (config.enabled_gates.count("AND"))
 			fprintf(f, "GATE AND    %d Y=A*B;                PIN * NONINV  1 999 1 0 1 0\n", cell_cost.at(ID($_AND_)));
-		if (enabled_gates.count("NAND"))
+		if (config.enabled_gates.count("NAND"))
 			fprintf(f, "GATE NAND   %d Y=!(A*B);             PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_NAND_)));
-		if (enabled_gates.count("OR"))
+		if (config.enabled_gates.count("OR"))
 			fprintf(f, "GATE OR     %d Y=A+B;                PIN * NONINV  1 999 1 0 1 0\n", cell_cost.at(ID($_OR_)));
-		if (enabled_gates.count("NOR"))
+		if (config.enabled_gates.count("NOR"))
 			fprintf(f, "GATE NOR    %d Y=!(A+B);             PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_NOR_)));
-		if (enabled_gates.count("XOR"))
+		if (config.enabled_gates.count("XOR"))
 			fprintf(f, "GATE XOR    %d Y=(A*!B)+(!A*B);      PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_XOR_)));
-		if (enabled_gates.count("XNOR"))
+		if (config.enabled_gates.count("XNOR"))
 			fprintf(f, "GATE XNOR   %d Y=(A*B)+(!A*!B);      PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_XNOR_)));
-		if (enabled_gates.count("ANDNOT"))
+		if (config.enabled_gates.count("ANDNOT"))
 			fprintf(f, "GATE ANDNOT %d Y=A*!B;               PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_ANDNOT_)));
-		if (enabled_gates.count("ORNOT"))
+		if (config.enabled_gates.count("ORNOT"))
 			fprintf(f, "GATE ORNOT  %d Y=A+!B;               PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_ORNOT_)));
-		if (enabled_gates.count("AOI3"))
+		if (config.enabled_gates.count("AOI3"))
 			fprintf(f, "GATE AOI3   %d Y=!((A*B)+C);         PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_AOI3_)));
-		if (enabled_gates.count("OAI3"))
+		if (config.enabled_gates.count("OAI3"))
 			fprintf(f, "GATE OAI3   %d Y=!((A+B)*C);         PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_OAI3_)));
-		if (enabled_gates.count("AOI4"))
+		if (config.enabled_gates.count("AOI4"))
 			fprintf(f, "GATE AOI4   %d Y=!((A*B)+(C*D));     PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_AOI4_)));
-		if (enabled_gates.count("OAI4"))
+		if (config.enabled_gates.count("OAI4"))
 			fprintf(f, "GATE OAI4   %d Y=!((A+B)*(C+D));     PIN * INV     1 999 1 0 1 0\n", cell_cost.at(ID($_OAI4_)));
-		if (enabled_gates.count("MUX"))
+		if (config.enabled_gates.count("MUX"))
 			fprintf(f, "GATE MUX    %d Y=(A*B)+(S*B)+(!S*A); PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_MUX_)));
-		if (enabled_gates.count("NMUX"))
+		if (config.enabled_gates.count("NMUX"))
 			fprintf(f, "GATE NMUX   %d Y=!((A*B)+(S*B)+(!S*A)); PIN * UNKNOWN 1 999 1 0 1 0\n", cell_cost.at(ID($_NMUX_)));
-		if (map_mux4)
+		if (config.map_mux4)
 			fprintf(f, "GATE MUX4   %d Y=(!S*!T*A)+(S*!T*B)+(!S*T*C)+(S*T*D); PIN * UNKNOWN 1 999 1 0 1 0\n", 2*cell_cost.at(ID($_MUX_)));
-		if (map_mux8)
+		if (config.map_mux8)
 			fprintf(f, "GATE MUX8   %d Y=(!S*!T*!U*A)+(S*!T*!U*B)+(!S*T*!U*C)+(S*T*!U*D)+(!S*!T*U*E)+(S*!T*U*F)+(!S*T*U*G)+(S*T*U*H); PIN * UNKNOWN 1 999 1 0 1 0\n", 4*cell_cost.at(ID($_MUX_)));
-		if (map_mux16)
+		if (config.map_mux16)
 			fprintf(f, "GATE MUX16  %d Y=(!S*!T*!U*!V*A)+(S*!T*!U*!V*B)+(!S*T*!U*!V*C)+(S*T*!U*!V*D)+(!S*!T*U*!V*E)+(S*!T*U*!V*F)+(!S*T*U*!V*G)+(S*T*U*!V*H)+(!S*!T*!U*V*I)+(S*!T*!U*V*J)+(!S*T*!U*V*K)+(S*T*!U*V*L)+(!S*!T*U*V*M)+(S*!T*U*V*N)+(!S*T*U*V*O)+(S*T*U*V*P); PIN * UNKNOWN 1 999 1 0 1 0\n", 8*cell_cost.at(ID($_MUX_)));
 		fclose(f);
 	}
@@ -1456,6 +1453,7 @@ void AbcModuleState::extract(AbcSigMap &assign_map, RTLIL::Design *design, RTLIL
 	RTLIL::Module *mapped_mod = mapped_design->module(ID(netlist));
 	if (mapped_mod == nullptr)
 		log_error("ABC output file does not contain a module `netlist'.\n");
+	bool markgroups = run_abc.config.markgroups;
 	for (auto w : mapped_mod->wires()) {
 		RTLIL::Wire *orig_wire = nullptr;
 		RTLIL::Wire *wire = module->addWire(remap_name(w->name, &orig_wire));
@@ -1998,9 +1996,9 @@ struct AbcPass : public Pass {
 		lut_arg = design->scratchpad_get_string("abc.lut", lut_arg);
 		luts_arg = design->scratchpad_get_string("abc.luts", luts_arg);
 		config.sop_mode = design->scratchpad_get_bool("abc.sop", false);
-		map_mux4 = design->scratchpad_get_bool("abc.mux4", map_mux4);
-		map_mux8 = design->scratchpad_get_bool("abc.mux8", map_mux8);
-		map_mux16 = design->scratchpad_get_bool("abc.mux16", map_mux16);
+		config.map_mux4 = design->scratchpad_get_bool("abc.mux4", false);
+		config.map_mux8 = design->scratchpad_get_bool("abc.mux8", false);
+		config.map_mux16 = design->scratchpad_get_bool("abc.mux16", false);
 		config.abc_dress = design->scratchpad_get_bool("abc.dress", false);
 		g_arg = design->scratchpad_get_string("abc.g", g_arg);
 
@@ -2014,7 +2012,7 @@ struct AbcPass : public Pass {
 		config.keepff = design->scratchpad_get_bool("abc.keepff", false);
 		config.cleanup = !design->scratchpad_get_bool("abc.nocleanup", false);
 		config.show_tempdir = design->scratchpad_get_bool("abc.showtmp", false);
-		markgroups = design->scratchpad_get_bool("abc.markgroups", markgroups);
+		config.markgroups = design->scratchpad_get_bool("abc.markgroups", false);
 
 		if (config.cleanup)
 			config.global_tempdir_name = get_base_tmpdir() + "/";
@@ -2094,15 +2092,15 @@ struct AbcPass : public Pass {
 				continue;
 			}
 			if (arg == "-mux4") {
-				map_mux4 = true;
+				config.map_mux4 = true;
 				continue;
 			}
 			if (arg == "-mux8") {
-				map_mux8 = true;
+				config.map_mux8 = true;
 				continue;
 			}
 			if (arg == "-mux16") {
-				map_mux16 = true;
+				config.map_mux16 = true;
 				continue;
 			}
 			if (arg == "-dress") {
@@ -2143,7 +2141,7 @@ struct AbcPass : public Pass {
 				continue;
 			}
 			if (arg == "-markgroups") {
-				markgroups = true;
+				config.markgroups = true;
 				continue;
 			}
 			break;
@@ -2236,14 +2234,14 @@ struct AbcPass : public Pass {
 				}
 				if (g == "cmos2") {
 					if (!remove_gates)
-						cmos_cost = true;
+						config.cmos_cost = true;
 					gate_list.push_back("NAND");
 					gate_list.push_back("NOR");
 					goto ok_alias;
 				}
 				if (g == "cmos3") {
 					if (!remove_gates)
-						cmos_cost = true;
+						config.cmos_cost = true;
 					gate_list.push_back("NAND");
 					gate_list.push_back("NOR");
 					gate_list.push_back("AOI3");
@@ -2252,7 +2250,7 @@ struct AbcPass : public Pass {
 				}
 				if (g == "cmos4") {
 					if (!remove_gates)
-						cmos_cost = true;
+						config.cmos_cost = true;
 					gate_list.push_back("NAND");
 					gate_list.push_back("NOR");
 					gate_list.push_back("AOI3");
@@ -2263,7 +2261,7 @@ struct AbcPass : public Pass {
 				}
 				if (g == "cmos") {
 					if (!remove_gates)
-						cmos_cost = true;
+						config.cmos_cost = true;
 					gate_list.push_back("NAND");
 					gate_list.push_back("NOR");
 					gate_list.push_back("AOI3");
@@ -2322,9 +2320,9 @@ struct AbcPass : public Pass {
 			ok_alias:
 				for (auto gate : gate_list) {
 					if (remove_gates)
-						enabled_gates.erase(gate);
+						config.enabled_gates.erase(gate);
 					else
-						enabled_gates.insert(gate);
+						config.enabled_gates.insert(gate);
 				}
 			}
 		}
@@ -2334,21 +2332,21 @@ struct AbcPass : public Pass {
 		if (!config.constr_file.empty() && (config.liberty_files.empty() && config.genlib_files.empty()))
 			log_cmd_error("Got -constr but no -liberty/-genlib!\n");
 
-		if (enabled_gates.empty()) {
-			enabled_gates.insert("AND");
-			enabled_gates.insert("NAND");
-			enabled_gates.insert("OR");
-			enabled_gates.insert("NOR");
-			enabled_gates.insert("XOR");
-			enabled_gates.insert("XNOR");
-			enabled_gates.insert("ANDNOT");
-			enabled_gates.insert("ORNOT");
-			// enabled_gates.insert("AOI3");
-			// enabled_gates.insert("OAI3");
-			// enabled_gates.insert("AOI4");
-			// enabled_gates.insert("OAI4");
-			enabled_gates.insert("MUX");
-			// enabled_gates.insert("NMUX");
+		if (config.enabled_gates.empty()) {
+			config.enabled_gates.insert("AND");
+			config.enabled_gates.insert("NAND");
+			config.enabled_gates.insert("OR");
+			config.enabled_gates.insert("NOR");
+			config.enabled_gates.insert("XOR");
+			config.enabled_gates.insert("XNOR");
+			config.enabled_gates.insert("ANDNOT");
+			config.enabled_gates.insert("ORNOT");
+			// config.enabled_gates.insert("AOI3");
+			// config.enabled_gates.insert("OAI3");
+			// config.enabled_gates.insert("AOI4");
+			// config.enabled_gates.insert("OAI4");
+			config.enabled_gates.insert("MUX");
+			// config.enabled_gates.insert("NMUX");
 		}
 
 		emit_global_input_files(config);

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

tests/techmap/abc_state.ys

@@ -0,0 +1,26 @@
+read_verilog <<EOT
+module simple(I1, I2, O);
+	input wire I1;
+	input wire I2;
+	output wire O;
+
+	assign O = I1 | I2;
+endmodule
+EOT
+abc -g all
+
+design -reset
+read_verilog <<EOT
+module simple(I1, I2, O);
+	input wire I1;
+	input wire I2;
+	output wire O;
+
+	assign O = I1 | I2;
+endmodule
+EOT
+techmap
+abc -g AND
+
+select -assert-count 0 t:$_OR_
+select -assert-count 1 t:$_AND_