iopadmap: Refactor and fix tristate buffer mapping. (#1527)

The previous code for rerouting wires when inserting tristate buffers
was overcomplicated and didn't handle all cases correctly (in
particular, only cell connections were rewired — internal connections
were not).

passes/techmap/iopadmap.cc

@@ -87,11 +87,11 @@ struct IopadmapPass : public Pass {
 	{
 		log_header(design, "Executing IOPADMAP pass (mapping inputs/outputs to IO-PAD cells).\n");
 
-		std::string inpad_celltype, inpad_portname, inpad_portname2;
+		std::string inpad_celltype, inpad_portname_o, inpad_portname_pad;
-		std::string outpad_celltype, outpad_portname, outpad_portname2;
+		std::string outpad_celltype, outpad_portname_i, outpad_portname_pad;
-		std::string inoutpad_celltype, inoutpad_portname, inoutpad_portname2;
+		std::string inoutpad_celltype, inoutpad_portname_io, inoutpad_portname_pad;
-		std::string toutpad_celltype, toutpad_portname, toutpad_portname2, toutpad_portname3;
+		std::string toutpad_celltype, toutpad_portname_oe, toutpad_portname_i, toutpad_portname_pad;
-		std::string tinoutpad_celltype, tinoutpad_portname, tinoutpad_portname2, tinoutpad_portname3, tinoutpad_portname4;
+		std::string tinoutpad_celltype, tinoutpad_portname_oe, tinoutpad_portname_o, tinoutpad_portname_i, tinoutpad_portname_pad;
 		std::string widthparam, nameparam;
 		pool<pair<IdString, IdString>> ignore;
 		bool flag_bits = false;
@@ -102,35 +102,35 @@ struct IopadmapPass : public Pass {
 			std::string arg = args[argidx];
 			if (arg == "-inpad" && argidx+2 < args.size()) {
 				inpad_celltype = args[++argidx];
-				inpad_portname = args[++argidx];
+				inpad_portname_o = args[++argidx];
-				split_portname_pair(inpad_portname, inpad_portname2);
+				split_portname_pair(inpad_portname_o, inpad_portname_pad);
 				continue;
 			}
 			if (arg == "-outpad" && argidx+2 < args.size()) {
 				outpad_celltype = args[++argidx];
-				outpad_portname = args[++argidx];
+				outpad_portname_i = args[++argidx];
-				split_portname_pair(outpad_portname, outpad_portname2);
+				split_portname_pair(outpad_portname_i, outpad_portname_pad);
 				continue;
 			}
 			if (arg == "-inoutpad" && argidx+2 < args.size()) {
 				inoutpad_celltype = args[++argidx];
-				inoutpad_portname = args[++argidx];
+				inoutpad_portname_io = args[++argidx];
-				split_portname_pair(inoutpad_portname, inoutpad_portname2);
+				split_portname_pair(inoutpad_portname_io, inoutpad_portname_pad);
 				continue;
 			}
 			if (arg == "-toutpad" && argidx+2 < args.size()) {
 				toutpad_celltype = args[++argidx];
-				toutpad_portname = args[++argidx];
+				toutpad_portname_oe = args[++argidx];
-				split_portname_pair(toutpad_portname, toutpad_portname2);
+				split_portname_pair(toutpad_portname_oe, toutpad_portname_i);
-				split_portname_pair(toutpad_portname2, toutpad_portname3);
+				split_portname_pair(toutpad_portname_i, toutpad_portname_pad);
 				continue;
 			}
 			if (arg == "-tinoutpad" && argidx+2 < args.size()) {
 				tinoutpad_celltype = args[++argidx];
-				tinoutpad_portname = args[++argidx];
+				tinoutpad_portname_oe = args[++argidx];
-				split_portname_pair(tinoutpad_portname, tinoutpad_portname2);
+				split_portname_pair(tinoutpad_portname_oe, tinoutpad_portname_o);
-				split_portname_pair(tinoutpad_portname2, tinoutpad_portname3);
+				split_portname_pair(tinoutpad_portname_o, tinoutpad_portname_i);
-				split_portname_pair(tinoutpad_portname3, tinoutpad_portname4);
+				split_portname_pair(tinoutpad_portname_i, tinoutpad_portname_pad);
 				continue;
 			}
 			if (arg == "-ignore" && argidx+2 < args.size()) {
@@ -161,16 +161,16 @@ struct IopadmapPass : public Pass {
 		}
 		extra_args(args, argidx, design);
 
-		if (!inpad_portname2.empty())
+		if (!inpad_portname_pad.empty())
-			ignore.insert(make_pair(RTLIL::escape_id(inpad_celltype), RTLIL::escape_id(inpad_portname2)));
+			ignore.insert(make_pair(RTLIL::escape_id(inpad_celltype), RTLIL::escape_id(inpad_portname_pad)));
-		if (!outpad_portname2.empty())
+		if (!outpad_portname_pad.empty())
-			ignore.insert(make_pair(RTLIL::escape_id(outpad_celltype), RTLIL::escape_id(outpad_portname2)));
+			ignore.insert(make_pair(RTLIL::escape_id(outpad_celltype), RTLIL::escape_id(outpad_portname_pad)));
-		if (!inoutpad_portname2.empty())
+		if (!inoutpad_portname_pad.empty())
-			ignore.insert(make_pair(RTLIL::escape_id(inoutpad_celltype), RTLIL::escape_id(inoutpad_portname2)));
+			ignore.insert(make_pair(RTLIL::escape_id(inoutpad_celltype), RTLIL::escape_id(inoutpad_portname_pad)));
-		if (!toutpad_portname3.empty())
+		if (!toutpad_portname_pad.empty())
-			ignore.insert(make_pair(RTLIL::escape_id(toutpad_celltype), RTLIL::escape_id(toutpad_portname3)));
+			ignore.insert(make_pair(RTLIL::escape_id(toutpad_celltype), RTLIL::escape_id(toutpad_portname_pad)));
-		if (!tinoutpad_portname4.empty())
+		if (!tinoutpad_portname_pad.empty())
-			ignore.insert(make_pair(RTLIL::escape_id(tinoutpad_celltype), RTLIL::escape_id(tinoutpad_portname4)));
+			ignore.insert(make_pair(RTLIL::escape_id(tinoutpad_celltype), RTLIL::escape_id(tinoutpad_portname_pad)));
 
 		for (auto module : design->modules())
 			if (module->get_blackbox_attribute())
@@ -180,34 +180,25 @@ struct IopadmapPass : public Pass {
 
 		for (auto module : design->selected_modules())
 		{
-			dict<IdString, pool<int>> skip_wires;
 			pool<SigBit> skip_wire_bits;
-			SigMap sigmap(module);
+			dict<Wire *, dict<int, pair<Cell *, IdString>>> rewrite_bits;
 
 			for (auto cell : module->cells())
 			for (auto port : cell->connections())
 				if (ignore.count(make_pair(cell->type, port.first)))
-					for (auto bit : sigmap(port.second))
+					for (auto bit : port.second)
 						skip_wire_bits.insert(bit);
 
 			if (!toutpad_celltype.empty() || !tinoutpad_celltype.empty())
 			{
-				dict<SigBit, pair<IdString, pool<IdString>>> tbuf_bits;
+				dict<SigBit, Cell *> tbuf_bits;
-				pool<pair<IdString, IdString>> norewrites;
-				SigMap rewrites;
 
 				for (auto cell : module->cells())
 					if (cell->type == ID($_TBUF_)) {
-						SigBit bit = sigmap(cell->getPort(ID::Y).as_bit());
+						SigBit bit = cell->getPort(ID::Y).as_bit();
-						tbuf_bits[bit].first = cell->name;
+						tbuf_bits[bit] = cell;
 					}
 
-				for (auto cell : module->cells())
-				for (auto port : cell->connections())
-				for (auto bit : sigmap(port.second))
-					if (tbuf_bits.count(bit))
-						tbuf_bits.at(bit).second.insert(cell->name);
-
 				for (auto wire : module->selected_wires())
 				{
 					if (!wire->port_output)
@@ -216,16 +207,11 @@ struct IopadmapPass : public Pass {
 					for (int i = 0; i < GetSize(wire); i++)
 					{
 						SigBit wire_bit(wire, i);
-						SigBit mapped_wire_bit = sigmap(wire_bit);
 
-						if (tbuf_bits.count(mapped_wire_bit) == 0)
+						if (tbuf_bits.count(wire_bit) == 0)
 							continue;
 
-						if (skip_wire_bits.count(mapped_wire_bit))
+						Cell *tbuf_cell = tbuf_bits.at(wire_bit);
-							continue;
-
-						auto &tbuf_cache = tbuf_bits.at(mapped_wire_bit);
-						Cell *tbuf_cell = module->cell(tbuf_cache.first);
 
 						if (tbuf_cell == nullptr)
 							continue;
@@ -238,37 +224,16 @@ struct IopadmapPass : public Pass {
 							log("Mapping port %s.%s[%d] using %s.\n", log_id(module), log_id(wire), i, tinoutpad_celltype.c_str());
 
 							Cell *cell = module->addCell(NEW_ID, RTLIL::escape_id(tinoutpad_celltype));
-							Wire *owire = module->addWire(NEW_ID);
 
-							cell->setPort(RTLIL::escape_id(tinoutpad_portname), en_sig);
+							cell->setPort(RTLIL::escape_id(tinoutpad_portname_oe), en_sig);
-							cell->setPort(RTLIL::escape_id(tinoutpad_portname2), owire);
+							cell->setPort(RTLIL::escape_id(tinoutpad_portname_o), wire_bit);
-							cell->setPort(RTLIL::escape_id(tinoutpad_portname3), data_sig);
+							cell->setPort(RTLIL::escape_id(tinoutpad_portname_i), data_sig);
-							cell->setPort(RTLIL::escape_id(tinoutpad_portname4), wire_bit);
 							cell->attributes[ID::keep] = RTLIL::Const(1);
 
-							for (auto cn : tbuf_cache.second) {
-								auto c = module->cell(cn);
-								if (c == nullptr)
-									continue;
-								for (auto port : c->connections()) {
-									SigSpec sig = port.second;
-									bool newsig = false;
-									for (auto &bit : sig)
-										if (sigmap(bit) == mapped_wire_bit) {
-											bit = owire;
-											newsig = true;
-										}
-									if (newsig)
-										c->setPort(port.first, sig);
-								}
-							}
-
-
 							module->remove(tbuf_cell);
-							skip_wires[wire->name].insert(i);
+							skip_wire_bits.insert(wire_bit);
-
+							if (!tinoutpad_portname_pad.empty())
-							norewrites.insert(make_pair(cell->name, RTLIL::escape_id(tinoutpad_portname4)));
+								rewrite_bits[wire][i] = make_pair(cell, RTLIL::escape_id(tinoutpad_portname_pad));
-							rewrites.add(sigmap(wire_bit), owire);
 							continue;
 						}
 
@@ -278,50 +243,19 @@ struct IopadmapPass : public Pass {
 
 							Cell *cell = module->addCell(NEW_ID, RTLIL::escape_id(toutpad_celltype));
 
-							cell->setPort(RTLIL::escape_id(toutpad_portname), en_sig);
+							cell->setPort(RTLIL::escape_id(toutpad_portname_oe), en_sig);
-							cell->setPort(RTLIL::escape_id(toutpad_portname2), data_sig);
+							cell->setPort(RTLIL::escape_id(toutpad_portname_i), data_sig);
-							cell->setPort(RTLIL::escape_id(toutpad_portname3), wire_bit);
 							cell->attributes[ID::keep] = RTLIL::Const(1);
 
-							for (auto cn : tbuf_cache.second) {
-								auto c = module->cell(cn);
-								if (c == nullptr)
-									continue;
-								for (auto port : c->connections()) {
-									SigSpec sig = port.second;
-									bool newsig = false;
-									for (auto &bit : sig)
-										if (sigmap(bit) == mapped_wire_bit) {
-											bit = data_sig;
-											newsig = true;
-										}
-									if (newsig)
-										c->setPort(port.first, sig);
-								}
-							}
-
 							module->remove(tbuf_cell);
-							skip_wires[wire->name].insert(i);
+							module->connect(wire_bit, data_sig);
+							skip_wire_bits.insert(wire_bit);
+							if (!toutpad_portname_pad.empty())
+								rewrite_bits[wire][i] = make_pair(cell, RTLIL::escape_id(toutpad_portname_pad));
 							continue;
 						}
 					}
 				}
-
-				if (GetSize(norewrites))
-				{
-					for (auto cell : module->cells())
-					for (auto port : cell->connections())
-					{
-						if (norewrites.count(make_pair(cell->name, port.first)))
-							continue;
-
-						SigSpec orig_sig = sigmap(port.second);
-						SigSpec new_sig = rewrites(orig_sig);
-
-						if (orig_sig != new_sig)
-							cell->setPort(port.first, new_sig);
-					}
-				}
 			}
 
 			for (auto wire : module->selected_wires())
@@ -329,17 +263,11 @@ struct IopadmapPass : public Pass {
 				if (!wire->port_id)
 					continue;
 
-				std::string celltype, portname, portname2;
+				std::string celltype, portname_int, portname_pad;
 				pool<int> skip_bit_indices;
 
-				if (skip_wires.count(wire->name)) {
-					if (!flag_bits)
-						continue;
-					skip_bit_indices = skip_wires.at(wire->name);
-				}
-
 				for (int i = 0; i < GetSize(wire); i++)
-					if (skip_wire_bits.count(sigmap(SigBit(wire, i))))
+					if (skip_wire_bits.count(SigBit(wire, i)))
 						skip_bit_indices.insert(i);
 
 				if (GetSize(wire) == GetSize(skip_bit_indices))
@@ -351,8 +279,8 @@ struct IopadmapPass : public Pass {
 						continue;
 					}
 					celltype = inpad_celltype;
-					portname = inpad_portname;
+					portname_int = inpad_portname_o;
-					portname2 = inpad_portname2;
+					portname_pad = inpad_portname_pad;
 				} else
 				if (!wire->port_input && wire->port_output) {
 					if (outpad_celltype.empty()) {
@@ -360,8 +288,8 @@ struct IopadmapPass : public Pass {
 						continue;
 					}
 					celltype = outpad_celltype;
-					portname = outpad_portname;
+					portname_int = outpad_portname_i;
-					portname2 = outpad_portname2;
+					portname_pad = outpad_portname_pad;
 				} else
 				if (wire->port_input && wire->port_output) {
 					if (inoutpad_celltype.empty()) {
@@ -369,8 +297,8 @@ struct IopadmapPass : public Pass {
 						continue;
 					}
 					celltype = inoutpad_celltype;
-					portname = inoutpad_portname;
+					portname_int = inoutpad_portname_io;
-					portname2 = inoutpad_portname2;
+					portname_pad = inoutpad_portname_pad;
 				} else
 					log_abort();
 
@@ -381,29 +309,20 @@ struct IopadmapPass : public Pass {
 
 				log("Mapping port %s.%s using %s.\n", RTLIL::id2cstr(module->name), RTLIL::id2cstr(wire->name), celltype.c_str());
 
-				RTLIL::Wire *new_wire = NULL;
-				if (!portname2.empty()) {
-					new_wire = module->addWire(NEW_ID, wire);
-					module->swap_names(new_wire, wire);
-					wire->attributes.clear();
-				}
-
 				if (flag_bits)
 				{
 					for (int i = 0; i < wire->width; i++)
 					{
-						if (skip_bit_indices.count(i)) {
+						if (skip_bit_indices.count(i))
-							if (wire->port_output)
-								module->connect(SigSpec(new_wire, i), SigSpec(wire, i));
-							else
-								module->connect(SigSpec(wire, i), SigSpec(new_wire, i));
 							continue;
-						}
+
+						SigBit wire_bit(wire, i);
 
 						RTLIL::Cell *cell = module->addCell(NEW_ID, RTLIL::escape_id(celltype));
-						cell->setPort(RTLIL::escape_id(portname), RTLIL::SigSpec(wire, i));
+						cell->setPort(RTLIL::escape_id(portname_int), wire_bit);
-						if (!portname2.empty())
+
-							cell->setPort(RTLIL::escape_id(portname2), RTLIL::SigSpec(new_wire, i));
+						if (!portname_pad.empty())
+							rewrite_bits[wire][i] = make_pair(cell, RTLIL::escape_id(portname_pad));
 						if (!widthparam.empty())
 							cell->parameters[RTLIL::escape_id(widthparam)] = RTLIL::Const(1);
 						if (!nameparam.empty())
@@ -414,9 +333,15 @@ struct IopadmapPass : public Pass {
 				else
 				{
 					RTLIL::Cell *cell = module->addCell(NEW_ID, RTLIL::escape_id(celltype));
-					cell->setPort(RTLIL::escape_id(portname), RTLIL::SigSpec(wire));
+					cell->setPort(RTLIL::escape_id(portname_int), RTLIL::SigSpec(wire));
-					if (!portname2.empty())
+
-						cell->setPort(RTLIL::escape_id(portname2), RTLIL::SigSpec(new_wire));
+					if (!portname_pad.empty()) {
+						RTLIL::Wire *new_wire = NULL;
+						new_wire = module->addWire(NEW_ID, wire);
+						module->swap_names(new_wire, wire);
+						wire->attributes.clear();
+						cell->setPort(RTLIL::escape_id(portname_pad), RTLIL::SigSpec(new_wire));
+					}
 					if (!widthparam.empty())
 						cell->parameters[RTLIL::escape_id(widthparam)] = RTLIL::Const(wire->width);
 					if (!nameparam.empty())
@@ -424,6 +349,32 @@ struct IopadmapPass : public Pass {
 					cell->attributes[ID::keep] = RTLIL::Const(1);
 				}
 
+				if (!rewrite_bits.count(wire)) {
+					wire->port_id = 0;
+					wire->port_input = false;
+					wire->port_output = false;
+				}
+			}
+
+			for (auto &it : rewrite_bits) {
+				RTLIL::Wire *wire = it.first;
+				RTLIL::Wire *new_wire = module->addWire(NEW_ID, wire);
+				module->swap_names(new_wire, wire);
+				wire->attributes.clear();
+				for (int i = 0; i < wire->width; i++)
+				{
+					SigBit wire_bit(wire, i);
+					if (!it.second.count(i)) {
+						if (wire->port_output)
+							module->connect(SigSpec(new_wire, i), SigSpec(wire, i));
+						else
+							module->connect(SigSpec(wire, i), SigSpec(new_wire, i));
+					} else {
+						auto &new_conn = it.second.at(i);
+						new_conn.first->setPort(new_conn.second, RTLIL::SigSpec(new_wire, i));
+					}
+				}
+
 				wire->port_id = 0;
 				wire->port_input = false;
 				wire->port_output = false;

tests/techmap/iopadmap.ys

@@ -0,0 +1,99 @@
+read_verilog << EOT
+module ibuf ((* iopad_external_pin *) input i, output o); endmodule
+module obuf (input i, (* iopad_external_pin *) output o); endmodule
+module obuft (input i, input oe, (* iopad_external_pin *) output o); endmodule
+module iobuf (input i, input oe, output o, (* iopad_external_pin *) inout io); endmodule
+
+module a(input i, output o);
+assign o = i;
+endmodule
+
+module b(input i, output o);
+assign o = i;
+ibuf b (.i(i), .o(o));
+endmodule
+
+module c(input i, output o);
+obuf b (.i(i), .o(o));
+endmodule
+
+module d(input i, oe, output o, o2, o3);
+assign o = oe ? i : 1'bz;
+assign o2 = o;
+assign o3 = ~o;
+endmodule
+
+module e(input i, oe, inout io, output o2, o3);
+assign io = oe ? i : 1'bz;
+assign o2 = io;
+assign o3 = ~io;
+endmodule
+EOT
+
+opt_clean
+tribuf
+simplemap
+iopadmap -bits -inpad ibuf o:i -outpad obuf i:o -toutpad obuft oe:i:o -tinoutpad iobuf oe:o:i:io
+opt_clean
+
+select -assert-count 1 a/t:ibuf
+select -assert-count 1 a/t:obuf
+select -set ib w:i %a %co a/t:ibuf %i
+select -set ob w:o %a %ci a/t:obuf %i
+select -assert-count 1 @ib
+select -assert-count 1 @ob
+select -assert-count 1 @ib %co %co @ob %i
+
+select -assert-count 1 b/t:ibuf
+select -assert-count 1 b/t:obuf
+select -set ib w:i %a %co b/t:ibuf %i
+select -set ob w:o %a %ci b/t:obuf %i
+select -assert-count 1 @ib
+select -assert-count 1 @ob
+select -assert-count 1 @ib %co %co @ob %i
+
+select -assert-count 1 c/t:ibuf
+select -assert-count 1 c/t:obuf
+select -set ib w:i %a %co c/t:ibuf %i
+select -set ob w:o %a %ci c/t:obuf %i
+select -assert-count 1 @ib
+select -assert-count 1 @ob
+select -assert-count 1 @ib %co %co @ob %i
+
+select -assert-count 2 d/t:ibuf
+select -assert-count 2 d/t:obuf
+select -assert-count 1 d/t:obuft
+select -set ib w:i %a %co d/t:ibuf %i
+select -set oeb w:oe %a %co d/t:ibuf %i
+select -set ob w:o %a %ci d/t:obuft %i
+select -set o2b w:o2 %a %ci d/t:obuf %i
+select -set o3b w:o3 %a %ci d/t:obuf %i
+select -assert-count 1 @ib
+select -assert-count 1 @oeb
+select -assert-count 1 @ob
+select -assert-count 1 @o2b
+select -assert-count 1 @o3b
+select -assert-count 1 @ib %co %co @ob %i
+select -assert-count 1 @oeb %co %co @ob %i
+select -assert-count 1 @ib %co %co @o2b %i
+select -assert-count 1 @ib %co %co t:$_NOT_ %i
+select -assert-count 1 @o3b %ci %ci t:$_NOT_ %i
+
+select -assert-count 2 e/t:ibuf
+select -assert-count 2 e/t:obuf
+select -assert-count 1 e/t:iobuf
+select -set ib w:i %a %co e/t:ibuf %i
+select -set oeb w:oe %a %co e/t:ibuf %i
+select -set iob w:io %a %ci e/t:iobuf %i
+select -set o2b w:o2 %a %ci e/t:obuf %i
+select -set o3b w:o3 %a %ci e/t:obuf %i
+select -assert-count 1 @ib
+select -assert-count 1 @oeb
+select -assert-count 1 @iob
+select -assert-count 1 @o2b
+select -assert-count 1 @o3b
+select -assert-count 1 @ib %co %co @iob %i
+select -assert-count 1 @oeb %co %co @iob %i
+select -assert-count 1 @iob %co %co @o2b %i
+select -assert-count 1 @iob %co %co t:$_NOT_ %i
+select -assert-count 1 @o3b %ci %ci t:$_NOT_ %i