Merge pull request #1625 from YosysHQ/eddie/abc9_mfs

abc9: re-enable "&mfs" optimisation for synth_{xilinx,ecp5}

6 files changed, 115 insertions, 86 deletions

diff --git a/Makefile b/Makefile
index 374d42f6f..43c4d0890 100644
--- a/Makefile
+++ b/Makefile
@@ -128,7 +128,7 @@ bumpversion:
 # is just a symlink to your actual ABC working directory, as 'make mrproper'
 # will remove the 'abc' directory and you do not want to accidentally
 # delete your work on ABC..
-ABCREV = 144c5be
+ABCREV = 71f2b40
 ABCPULL = 1
 ABCURL ?= https://github.com/berkeley-abc/abc
 ABCMKARGS = CC="$(CXX)" CXX="$(CXX)" ABC_USE_LIBSTDCXX=1
diff --git a/backends/aiger/xaiger.cc b/backends/aiger/xaiger.cc
index 05e9678ee..7ee5058ae 100644
--- a/backends/aiger/xaiger.cc
+++ b/backends/aiger/xaiger.cc
@@ -407,7 +407,7 @@ struct XAigerWriter
 					}
 					if (w->port_output) {
 						RTLIL::SigSpec rhs;
-						auto it = cell->connections_.find(w->name);
+						auto it = cell->connections_.find(port_name);
 						if (it != cell->connections_.end()) {
 							if (GetSize(it->second) < GetSize(w))
 								it->second.append(module->addWire(NEW_ID, GetSize(w)-GetSize(it->second)));
@@ -474,7 +474,8 @@ struct XAigerWriter
 		if (holes_mode) {
 			struct sort_by_port_id {
 				bool operator()(const RTLIL::SigBit& a, const RTLIL::SigBit& b) const {
-					return a.wire->port_id < b.wire->port_id;
+					return a.wire->port_id < b.wire->port_id ||
+					    (a.wire->port_id == b.wire->port_id && a.offset < b.offset);
 				}
 			};
 			input_bits.sort(sort_by_port_id());
@@ -614,7 +615,7 @@ struct XAigerWriter
 			RTLIL::Module *holes_module = module->design->addModule("$__holes__");
 			log_assert(holes_module);
 
-			dict<IdString, Cell*> cell_cache;
+			dict<IdString, std::tuple<Cell*,int,int,int>> cell_cache;
 
 			int port_id = 1;
 			int box_count = 0;
@@ -623,81 +624,94 @@ struct XAigerWriter
 				log_assert(orig_box_module);
 				IdString derived_name = orig_box_module->derive(module->design, cell->parameters);
 				RTLIL::Module* box_module = module->design->module(derived_name);
-				if (box_module->has_processes())
-					Pass::call_on_module(module->design, box_module, "proc");
-
-				auto r = cell_cache.insert(std::make_pair(derived_name, nullptr));
-				Cell *holes_cell = r.first->second;
-				if (r.second && box_module->get_bool_attribute("\\whitebox")) {
-					holes_cell = holes_module->addCell(cell->name, cell->type);
-					holes_cell->parameters = cell->parameters;
-					r.first->second = holes_cell;
-				}
 
-				int box_inputs = 0, box_outputs = 0;
-				for (auto port_name : box_ports.at(cell->type)) {
-					RTLIL::Wire *w = box_module->wire(port_name);
-					log_assert(w);
-					RTLIL::Wire *holes_wire;
-					RTLIL::SigSpec port_sig;
+				auto r = cell_cache.insert(derived_name);
+				auto &v = r.first->second;
+				if (r.second) {
+					if (box_module->has_processes())
+						Pass::call_on_module(module->design, box_module, "proc");
+
+					int box_inputs = 0, box_outputs = 0;
+					if (box_module->get_bool_attribute("\\whitebox")) {
+						auto holes_cell = holes_module->addCell(cell->name, derived_name);
+						for (auto port_name : box_ports.at(cell->type)) {
+							RTLIL::Wire *w = box_module->wire(port_name);
+							log_assert(w);
+							log_assert(!w->port_input || !w->port_output);
+							auto &conn = holes_cell->connections_[port_name];
+							if (w->port_input) {
+								for (int i = 0; i < GetSize(w); i++) {
+									box_inputs++;
+									RTLIL::Wire *holes_wire = holes_module->wire(stringf("\\i%d", box_inputs));
+									if (!holes_wire) {
+										holes_wire = holes_module->addWire(stringf("\\i%d", box_inputs));
+										holes_wire->port_input = true;
+										holes_wire->port_id = port_id++;
+										holes_module->ports.push_back(holes_wire->name);
+									}
+									conn.append(holes_wire);
+								}
+							}
+							else if (w->port_output) {
+								box_outputs += GetSize(w);
+								conn = holes_module->addWire(stringf("%s.%s", derived_name.c_str(), log_id(port_name)), GetSize(w));
+							}
+						}
 
-					if (w->port_input)
-						for (int i = 0; i < GetSize(w); i++) {
+						// For flops only, create an extra 1-bit input that drives a new wire
+						//   called "<cell>.abc9_ff.Q" that is used below
+						if (box_module->get_bool_attribute("\\abc9_flop")) {
 							box_inputs++;
-							holes_wire = holes_module->wire(stringf("\\i%d", box_inputs));
+							Wire *holes_wire = holes_module->wire(stringf("\\i%d", box_inputs));
 							if (!holes_wire) {
 								holes_wire = holes_module->addWire(stringf("\\i%d", box_inputs));
 								holes_wire->port_input = true;
 								holes_wire->port_id = port_id++;
 								holes_module->ports.push_back(holes_wire->name);
 							}
-							if (holes_cell)
-								port_sig.append(holes_wire);
-						}
-					if (w->port_output) {
-						box_outputs += GetSize(w);
-						for (int i = 0; i < GetSize(w); i++) {
-							if (GetSize(w) == 1)
-								holes_wire = holes_module->addWire(stringf("$abc%s.%s", cell->name.c_str(), log_id(w->name)));
-							else
-								holes_wire = holes_module->addWire(stringf("$abc%s.%s[%d]", cell->name.c_str(), log_id(w->name), i));
-							holes_wire->port_output = true;
-							holes_wire->port_id = port_id++;
-							holes_module->ports.push_back(holes_wire->name);
-							if (holes_cell)
-								port_sig.append(holes_wire);
-							else
-								holes_module->connect(holes_wire, State::S0);
+							Wire *Q = holes_module->addWire(stringf("%s.abc9_ff.Q", cell->name.c_str()));
+							holes_module->connect(Q, holes_wire);
 						}
+
+						std::get<0>(v) = holes_cell;
 					}
-					if (!port_sig.empty()) {
-						if (r.second)
-							holes_cell->setPort(w->name, port_sig);
-						else
-							holes_module->connect(holes_cell->getPort(w->name), port_sig);
+					else {
+						for (auto port_name : box_ports.at(cell->type)) {
+							RTLIL::Wire *w = box_module->wire(port_name);
+							log_assert(w);
+							log_assert(!w->port_input || !w->port_output);
+							if (w->port_input)
+								box_inputs += GetSize(w);
+							else if (w->port_output)
+								box_outputs += GetSize(w);
+						}
+						log_assert(std::get<0>(v) == nullptr);
 					}
+
+					std::get<1>(v) = box_inputs;
+					std::get<2>(v) = box_outputs;
+					std::get<3>(v) = box_module->attributes.at("\\abc9_box_id").as_int();
 				}
 
-				// For flops only, create an extra 1-bit input that drives a new wire
-				//   called "<cell>.abc9_ff.Q" that is used below
-				if (box_module->get_bool_attribute("\\abc9_flop")) {
-					log_assert(holes_cell);
-
-					box_inputs++;
-					Wire *holes_wire = holes_module->wire(stringf("\\i%d", box_inputs));
-					if (!holes_wire) {
-						holes_wire = holes_module->addWire(stringf("\\i%d", box_inputs));
-						holes_wire->port_input = true;
-						holes_wire->port_id = port_id++;
-						holes_module->ports.push_back(holes_wire->name);
-					}
-					Wire *w = holes_module->addWire(stringf("%s.abc9_ff.Q", cell->name.c_str()));
-					holes_module->connect(w, holes_wire);
+				auto holes_cell = std::get<0>(v);
+				for (auto port_name : box_ports.at(cell->type)) {
+					RTLIL::Wire *w = box_module->wire(port_name);
+					log_assert(w);
+					if (!w->port_output)
+						continue;
+					Wire *holes_wire = holes_module->addWire(stringf("$abc%s.%s", cell->name.c_str(), log_id(port_name)), GetSize(w));
+					holes_wire->port_output = true;
+					holes_wire->port_id = port_id++;
+					holes_module->ports.push_back(holes_wire->name);
+					if (holes_cell) // whitebox
+						holes_module->connect(holes_wire, holes_cell->getPort(port_name));
+					else // blackbox
+						holes_module->connect(holes_wire, Const(State::S0, GetSize(w)));
 				}
 
-				write_h_buffer(box_inputs);
-				write_h_buffer(box_outputs);
-				write_h_buffer(box_module->attributes.at("\\abc9_box_id").as_int());
+				write_h_buffer(std::get<1>(v));
+				write_h_buffer(std::get<2>(v));
+				write_h_buffer(std::get<3>(v));
 				write_h_buffer(box_count++);
 			}
 
@@ -757,14 +771,14 @@ struct XAigerWriter
 				//   created a new $paramod ...
 				Pass::call_on_module(holes_module->design, holes_module, "flatten -wb; techmap; aigmap");
 
-				dict<SigSig, SigSig> replace;
+				dict<SigSpec, SigSpec> replace;
 				for (auto it = holes_module->cells_.begin(); it != holes_module->cells_.end(); ) {
 					auto cell = it->second;
 					if (cell->type.in("$_DFF_N_", "$_DFF_NN0_", "$_DFF_NN1_", "$_DFF_NP0_", "$_DFF_NP1_",
 								"$_DFF_P_", "$_DFF_PN0_", "$_DFF_PN1", "$_DFF_PP0_", "$_DFF_PP1_")) {
 						SigBit D = cell->getPort("\\D");
 						SigBit Q = cell->getPort("\\Q");
-						// Remove the DFF cell from what needs to be a combinatorial box
+						// Remove the $_DFF_* cell from what needs to be a combinatorial box
 						it = holes_module->cells_.erase(it);
 						Wire *port;
 						if (GetSize(Q.wire) == 1)
@@ -772,10 +786,10 @@ struct XAigerWriter
 						else
 							port = holes_module->wire(stringf("$abc%s[%d]", Q.wire->name.c_str(), Q.offset));
 						log_assert(port);
-						// Prepare to replace "assign <port> = DFF.Q;" with "assign <port> = DFF.D;"
-						//   in order to extract the combinatorial control logic that feeds the box
+						// Prepare to replace "assign <port> = $_DFF_*.Q;" with "assign <port> = $_DFF_*.D;"
+						//   in order to extract just the combinatorial control logic that feeds the box
 						//   (i.e. clock enable, synchronous reset, etc.)
-						replace.insert(std::make_pair(SigSig(port,Q), SigSig(port,D)));
+						replace.insert(std::make_pair(Q,D));
 						// Since `flatten` above would have created wires named "<cell>.Q",
 						//   extract the pre-techmap cell name
 						auto pos = Q.wire->name.str().rfind(".");
@@ -783,7 +797,7 @@ struct XAigerWriter
 						IdString driver = Q.wire->name.substr(0, pos);
 						// And drive the signal that was previously driven by "DFF.Q" (typically
 						//   used to implement clock-enable functionality) with the "<cell>.abc9_ff.Q"
-						//   wire (which itself is driven an input port) we inserted above
+						//   wire (which itself is driven by an input port) we inserted above
 						Wire *currQ = holes_module->wire(stringf("%s.abc9_ff.Q", driver.c_str()));
 						log_assert(currQ);
 						holes_module->connect(Q, currQ);
@@ -794,10 +808,11 @@ struct XAigerWriter
 					++it;
 				}
 
+				SigMap holes_sigmap(holes_module);
 				for (auto &conn : holes_module->connections_) {
-					auto it = replace.find(conn);
+					auto it = replace.find(sigmap(conn.second));
 					if (it != replace.end())
-						conn = it->second;
+						conn.second = it->second;
 				}
 
 				// Move into a new (temporary) design so that "clean" will only
diff --git a/passes/techmap/abc9.cc b/passes/techmap/abc9.cc
index 3fc6ed2c2..471c11a80 100644
--- a/passes/techmap/abc9.cc
+++ b/passes/techmap/abc9.cc
@@ -250,7 +250,7 @@ struct abc9_output_filter
 void abc9_module(RTLIL::Design *design, RTLIL::Module *module, std::string script_file, std::string exe_file,
 		bool cleanup, vector<int> lut_costs, bool dff_mode, std::string delay_target, std::string /*lutin_shared*/, bool fast_mode,
 		bool show_tempdir, std::string box_file, std::string lut_file,
-		std::string wire_delay, bool nomfs
+		std::string wire_delay
 )
 {
 	map_autoidx = autoidx++;
@@ -305,10 +305,6 @@ void abc9_module(RTLIL::Design *design, RTLIL::Module *module, std::string scrip
 	for (size_t pos = abc9_script.find("{W}"); pos != std::string::npos; pos = abc9_script.find("{W}", pos))
 		abc9_script = abc9_script.substr(0, pos) + wire_delay + abc9_script.substr(pos+3);
 
-	if (nomfs)
-		for (size_t pos = abc9_script.find("&mfs"); pos != std::string::npos; pos = abc9_script.find("&mfs", pos))
-			abc9_script = abc9_script.erase(pos, strlen("&mfs"));
-
 	abc9_script += stringf("; &write -n %s/output.aig", tempdir_name.c_str());
 	abc9_script = add_echos_to_abc9_cmd(abc9_script);
 
@@ -833,7 +829,6 @@ struct Abc9Pass : public Pass {
 		std::string delay_target, lutin_shared = "-S 1", wire_delay;
 		bool fast_mode = false, dff_mode = false, cleanup = true;
 		bool show_tempdir = false;
-		bool nomfs = false;
 		vector<int> lut_costs;
 
 #if 0
@@ -865,7 +860,6 @@ struct Abc9Pass : public Pass {
 		if (design->scratchpad.count("abc9.W")) {
 			wire_delay = "-W " + design->scratchpad_get_string("abc9.W");
 		}
-		nomfs = design->scratchpad_get_bool("abc9.nomfs", nomfs);
 
 		size_t argidx;
 		char pwd [PATH_MAX];
@@ -926,10 +920,6 @@ struct Abc9Pass : public Pass {
 				wire_delay = "-W " + args[++argidx];
 				continue;
 			}
-			if (arg == "-nomfs") {
-				nomfs = true;
-				continue;
-			}
 			break;
 		}
 		extra_args(args, argidx, design);
@@ -1043,7 +1033,7 @@ struct Abc9Pass : public Pass {
 			design->selected_active_module = module->name.str();
 			abc9_module(design, module, script_file, exe_file, cleanup, lut_costs, dff_mode,
 					delay_target, lutin_shared, fast_mode, show_tempdir,
-					box_file, lut_file, wire_delay, nomfs);
+					box_file, lut_file, wire_delay);
 			design->selected_active_module.clear();
 		}
 
diff --git a/techlibs/ecp5/synth_ecp5.cc b/techlibs/ecp5/synth_ecp5.cc
index d616391b2..6583f43fd 100644
--- a/techlibs/ecp5/synth_ecp5.cc
+++ b/techlibs/ecp5/synth_ecp5.cc
@@ -323,9 +323,9 @@ struct SynthEcp5Pass : public ScriptPass
 			if (abc9) {
 				run("read_verilog -icells -lib +/ecp5/abc9_model.v");
 				if (nowidelut)
-					run("abc9 -lut +/ecp5/abc9_5g_nowide.lut -box +/ecp5/abc9_5g.box -W 200 -nomfs");
+					run("abc9 -lut +/ecp5/abc9_5g_nowide.lut -box +/ecp5/abc9_5g.box -W 200");
 				else
-					run("abc9 -lut +/ecp5/abc9_5g.lut -box +/ecp5/abc9_5g.box -W 200 -nomfs");
+					run("abc9 -lut +/ecp5/abc9_5g.lut -box +/ecp5/abc9_5g.box -W 200");
 				run("techmap -map +/ecp5/abc9_unmap.v");
 			} else {
 				if (nowidelut)
diff --git a/techlibs/xilinx/synth_xilinx.cc b/techlibs/xilinx/synth_xilinx.cc
index 51d2cbbd2..63d00027a 100644
--- a/techlibs/xilinx/synth_xilinx.cc
+++ b/techlibs/xilinx/synth_xilinx.cc
@@ -556,7 +556,6 @@ struct SynthXilinxPass : public ScriptPass
 				run("read_verilog -icells -lib +/xilinx/abc9_model.v");
 				std::string abc9_opts = " -box +/xilinx/abc9_xc7.box";
 				abc9_opts += stringf(" -W %d", XC7_WIRE_DELAY);
-				abc9_opts += " -nomfs";
 				if (nowidelut)
 					abc9_opts += " -lut +/xilinx/abc9_xc7_nowide.lut";
 				else
diff --git a/tests/arch/ecp5/bug1459.ys b/tests/arch/ecp5/bug1459.ys
new file mode 100644
index 000000000..1142ae0b5
--- /dev/null
+++ b/tests/arch/ecp5/bug1459.ys
@@ -0,0 +1,25 @@
+read_verilog <<EOT
+module register_file(
+    input wire clk,
+    input wire write_enable,
+    input wire [63:0] write_data,
+    input wire [4:0] write_reg,
+    input wire [4:0] read1_reg,
+    output reg [63:0] read1_data,
+    );
+
+    reg [63:0] registers[0:31];
+
+    always @(posedge clk) begin
+      if (write_enable == 1'b1) begin
+        registers[write_reg] <= write_data;
+      end
+    end
+
+    always @(all) begin
+      read1_data <= registers[read1_reg];
+    end
+endmodule
+EOT
+
+synth_ecp5 -abc9