Merge pull request #2077 from YosysHQ/eddie/abc9_dff_improve

abc9: -dff improvements

backends/aiger/xaiger.cc

@@ -85,7 +85,7 @@ struct XAigerWriter
 	dict<SigBit, SigBit> not_map, alias_map;
 	dict<SigBit, pair<SigBit, SigBit>> and_map;
 	vector<SigBit> ci_bits, co_bits;
-	dict<SigBit, Cell*> ff_bits;
+	vector<Cell*> ff_list;
 	dict<SigBit, float> arrival_times;
 
 	vector<pair<int, int>> aig_gates;
@@ -232,8 +232,7 @@ struct XAigerWriter
 					unused_bits.erase(D);
 					undriven_bits.erase(Q);
 					alias_map[Q] = D;
-					auto r YS_ATTRIBUTE(unused) = ff_bits.insert(std::make_pair(D, cell));
+					ff_list.emplace_back(cell);
-					log_assert(r.second);
 					continue;
 				}
 
@@ -420,8 +419,7 @@ struct XAigerWriter
 			aig_map[bit] = 2*aig_m;
 		}
 
-		for (const auto &i : ff_bits) {
+		for (auto cell : ff_list) {
-			const Cell *cell = i.second;
 			const SigBit &q = sigmap(cell->getPort(ID::Q));
 			aig_m++, aig_i++;
 			log_assert(!aig_map.count(q));
@@ -468,8 +466,8 @@ struct XAigerWriter
 			aig_outputs.push_back(aig);
 		}
 
-		for (auto &i : ff_bits) {
+		for (auto cell : ff_list) {
-			const SigBit &d = i.first;
+			const SigBit &d = sigmap(cell->getPort(ID::D));
 			aig_o++;
 			aig_outputs.push_back(aig_map.at(d));
 		}
@@ -541,16 +539,16 @@ struct XAigerWriter
 		std::stringstream h_buffer;
 		auto write_h_buffer = std::bind(write_buffer, std::ref(h_buffer), std::placeholders::_1);
 		write_h_buffer(1);
-		log_debug("ciNum = %d\n", GetSize(input_bits) + GetSize(ff_bits) + GetSize(ci_bits));
+		log_debug("ciNum = %d\n", GetSize(input_bits) + GetSize(ff_list) + GetSize(ci_bits));
-		write_h_buffer(input_bits.size() + ff_bits.size() + ci_bits.size());
+		write_h_buffer(GetSize(input_bits) + GetSize(ff_list) + GetSize(ci_bits));
-		log_debug("coNum = %d\n", GetSize(output_bits) + GetSize(ff_bits) + GetSize(co_bits));
+		log_debug("coNum = %d\n", GetSize(output_bits) + GetSize(ff_list) + GetSize(co_bits));
-		write_h_buffer(output_bits.size() + GetSize(ff_bits) + GetSize(co_bits));
+		write_h_buffer(GetSize(output_bits) + GetSize(ff_list) + GetSize(co_bits));
-		log_debug("piNum = %d\n", GetSize(input_bits) + GetSize(ff_bits));
+		log_debug("piNum = %d\n", GetSize(input_bits) + GetSize(ff_list));
-		write_h_buffer(input_bits.size() + ff_bits.size());
+		write_h_buffer(GetSize(input_bits) + GetSize(ff_list));
-		log_debug("poNum = %d\n", GetSize(output_bits) + GetSize(ff_bits));
+		log_debug("poNum = %d\n", GetSize(output_bits) + GetSize(ff_list));
-		write_h_buffer(output_bits.size() + ff_bits.size());
+		write_h_buffer(GetSize(output_bits) + GetSize(ff_list));
 		log_debug("boxNum = %d\n", GetSize(box_list));
-		write_h_buffer(box_list.size());
+		write_h_buffer(GetSize(box_list));
 
 		auto write_buffer_float = [](std::stringstream &buffer, float f32) {
 			buffer.write(reinterpret_cast<const char*>(&f32), sizeof(f32));
@@ -564,7 +562,7 @@ struct XAigerWriter
 		//for (auto bit : output_bits)
 		//	write_o_buffer(0);
 
-		if (!box_list.empty() || !ff_bits.empty()) {
+		if (!box_list.empty() || !ff_list.empty()) {
 			dict<IdString, std::tuple<int,int,int>> cell_cache;
 
 			int box_count = 0;
@@ -601,17 +599,17 @@ struct XAigerWriter
 
 			std::stringstream r_buffer;
 			auto write_r_buffer = std::bind(write_buffer, std::ref(r_buffer), std::placeholders::_1);
-			log_debug("flopNum = %d\n", GetSize(ff_bits));
+			log_debug("flopNum = %d\n", GetSize(ff_list));
-			write_r_buffer(ff_bits.size());
+			write_r_buffer(ff_list.size());
 
 			std::stringstream s_buffer;
 			auto write_s_buffer = std::bind(write_buffer, std::ref(s_buffer), std::placeholders::_1);
-			write_s_buffer(ff_bits.size());
+			write_s_buffer(ff_list.size());
 
 			dict<SigSpec, int> clk_to_mergeability;
-			for (const auto &i : ff_bits) {
+			for (const auto cell : ff_list) {
-				const SigBit &d = i.first;
+				const SigBit &d = sigmap(cell->getPort(ID::D));
-				const Cell *cell = i.second;
+				const SigBit &q = sigmap(cell->getPort(ID::Q));
 
 				SigSpec clk_and_pol{sigmap(cell->getPort(ID::C)), cell->type[6] == 'P' ? State::S1 : State::S0};
 				auto r = clk_to_mergeability.insert(std::make_pair(clk_and_pol, clk_to_mergeability.size()+1));
@@ -619,8 +617,7 @@ struct XAigerWriter
 				log_assert(mergeability > 0);
 				write_r_buffer(mergeability);
 
-				SigBit Q = sigmap(cell->getPort(ID::Q));
+				State init = init_map.at(q, State::Sx);
-				State init = init_map.at(Q, State::Sx);
 				log_debug("Cell '%s' (type %s) has (* init *) value '%s'.\n", log_id(cell), log_id(cell->type), log_signal(init));
 				if (init == State::S1)
 					write_s_buffer(1);
@@ -700,8 +697,6 @@ struct XAigerWriter
 
 		for (auto wire : module->wires())
 		{
-			SigSpec sig = sigmap(wire);
-
 			for (int i = 0; i < GetSize(wire); i++)
 			{
 				RTLIL::SigBit b(wire, i);
@@ -714,7 +709,6 @@ struct XAigerWriter
 				if (output_bits.count(b)) {
 					int o = ordered_outputs.at(b);
 					output_lines[o] += stringf("output %d %d %s\n", o - GetSize(co_bits), wire->start_offset+i, log_id(wire));
-					continue;
 				}
 			}
 		}

frontends/aiger/aigerparse.cc

@@ -775,7 +775,6 @@ void AigerReader::post_process()
 		}
 	}
 
-	dict<int, Wire*> mergeability_to_clock;
 	for (uint32_t i = 0; i < flopNum; i++) {
 		RTLIL::Wire *d = outputs[outputs.size() - flopNum + i];
 		log_assert(d);
@@ -895,7 +894,9 @@ void AigerReader::post_process()
 			}
 			else if (type == "box") {
 				RTLIL::Cell* cell = module->cell(stringf("$box%d", variable));
-				if (cell) // ABC could have optimised this box away
+				if (!cell)
+					log_debug("Box %d (%s) no longer exists.\n", variable, log_id(escaped_s));
+				else
 					module->rename(cell, escaped_s);
 			}
 			else
@@ -907,6 +908,8 @@ void AigerReader::post_process()
 		auto name = wp.first;
 		int min = wp.second.first;
 		int max = wp.second.second;
+		if (min == 0 && max == 0)
+			continue;
 
 		RTLIL::Wire *wire = module->wire(name);
 		if (wire)

passes/techmap/abc9_ops.cc

@@ -102,8 +102,6 @@ void check(RTLIL::Design *design, bool dff_mode)
 				auto inst_module = design->module(cell->type);
 				if (!inst_module)
 					continue;
-				if (!inst_module->get_blackbox_attribute())
-					continue;
 				IdString derived_type;
 				Module *derived_module;
 				if (cell->parameters.empty()) {
@@ -111,6 +109,10 @@ void check(RTLIL::Design *design, bool dff_mode)
 					derived_module = inst_module;
 				}
 				else {
+					// Check potential (since its value may depend on a parameter,
+					//   but not its existence)
+					if (!inst_module->has_attribute(ID::abc9_flop))
+						continue;
 					derived_type = inst_module->derive(design, cell->parameters);
 					derived_module = design->module(derived_type);
 					log_assert(derived_module);
@@ -127,20 +129,20 @@ void check(RTLIL::Design *design, bool dff_mode)
 				for (auto derived_cell : derived_module->cells()) {
 					if (derived_cell->type.in(ID($dff), ID($_DFF_N_), ID($_DFF_P_))) {
 						if (found)
-							log_error("Module '%s' with (* abc9_flop *) contains more than one $_DFF_[NP]_ cell.\n", log_id(derived_module));
+							log_error("Whitebox '%s' with (* abc9_flop *) contains more than one $_DFF_[NP]_ cell.\n", log_id(derived_module));
 						found = true;
 
 						SigBit Q = derived_cell->getPort(ID::Q);
 						log_assert(GetSize(Q.wire) == 1);
 
 						if (!Q.wire->port_output)
-							log_error("Module '%s' contains a %s cell where its 'Q' port does not drive a module output!\n", log_id(derived_module), log_id(derived_cell->type));
+							log_error("Whitebox '%s' with (* abc9_flop *) contains a %s cell where its 'Q' port does not drive a module output.\n", log_id(derived_module), log_id(derived_cell->type));
 
 						Const init = Q.wire->attributes.at(ID::init, State::Sx);
 						log_assert(GetSize(init) == 1);
 					}
 					else if (unsupported.count(derived_cell->type))
-						log_error("Module '%s' with (* abc9_flop *) contains a %s cell, which is not supported for sequential synthesis.\n", log_id(derived_module), log_id(derived_cell->type));
+						log_error("Whitebox '%s' with (* abc9_flop *) contains a %s cell, which is not supported for sequential synthesis.\n", log_id(derived_module), log_id(derived_cell->type));
 				}
 			}
 	}
@@ -173,8 +175,6 @@ void prep_hier(RTLIL::Design *design, bool dff_mode)
 			auto inst_module = design->module(cell->type);
 			if (!inst_module)
 				continue;
-			if (!inst_module->get_blackbox_attribute())
-				continue;
 			IdString derived_type;
 			Module *derived_module;
 			if (cell->parameters.empty()) {
@@ -182,6 +182,10 @@ void prep_hier(RTLIL::Design *design, bool dff_mode)
 				derived_module = inst_module;
 			}
 			else {
+				// Check potential for any one of those three
+				//   (since its value may depend on a parameter, but not its existence)
+				if (!inst_module->has_attribute(ID::abc9_flop) && !inst_module->has_attribute(ID::abc9_box) && !inst_module->get_bool_attribute(ID::abc9_bypass))
+					continue;
 				derived_type = inst_module->derive(design, cell->parameters);
 				derived_module = design->module(derived_type);
 			}
@@ -211,7 +215,7 @@ void prep_hier(RTLIL::Design *design, bool dff_mode)
 							// Block sequential synthesis on cells with (* init *) != 1'b0
 							//   because ABC9 doesn't support them
 							if (init != State::S0) {
-								log_warning("Module '%s' contains a %s cell with non-zero initial state -- this is not unsupported for ABC9 sequential synthesis. Treating as a blackbox.\n", log_id(derived_module), log_id(derived_cell->type));
+								log_warning("Whitebox '%s' with (* abc9_flop *) contains a %s cell with non-zero initial state -- this is not supported for ABC9 sequential synthesis. Treating as a blackbox.\n", log_id(derived_module), log_id(derived_cell->type));
 								derived_module->set_bool_attribute(ID::abc9_flop, false);
 							}
 							break;
@@ -232,10 +236,8 @@ void prep_hier(RTLIL::Design *design, bool dff_mode)
 						auto w = unmap_module->addWire(port, derived_module->wire(port));
 						// Do not propagate (* init *) values into the box,
 						//   in fact, remove it from outside too
-						if (w->port_output && w->attributes.erase(ID::init)) {
+						if (w->port_output)
-							auto r = unmap_module->addWire(stringf("\\_TECHMAP_REMOVEINIT_%s_", log_id(port)));
+							w->attributes.erase(ID::init);
-							unmap_module->connect(r, State::S1);
-						}
 					}
 					unmap_module->ports = derived_module->ports;
 					unmap_module->check();
@@ -1149,16 +1151,36 @@ void reintegrate(RTLIL::Module *module, bool dff_mode)
 		}
 	}
 
+	SigMap initmap;
+	if (dff_mode) {
+		// Build a sigmap prioritising bits with (* init *)
+		initmap.set(module);
+		for (auto w : module->wires()) {
+			auto it = w->attributes.find(ID::init);
+			if (it == w->attributes.end())
+				continue;
+			for (auto i = 0; i < GetSize(w); i++)
+				if (it->second[i] == State::S0 || it->second[i] == State::S1)
+					initmap.add(w);
+		}
+	}
+
 	std::vector<Cell*> boxes;
 	for (auto cell : module->cells().to_vector()) {
 		if (cell->has_keep_attr())
 			continue;
 
-		// Short out $_DFF_[NP]_ cells since the flop box already has
+		// Short out (so that existing name can be preserved) and remove
-		//   all the information we need to reconstruct cell
+		//   $_DFF_[NP]_ cells since flop box already has all the information
+		//   we need to reconstruct them
 		if (dff_mode && cell->type.in(ID($_DFF_N_), ID($_DFF_P_)) && !cell->get_bool_attribute(ID::abc9_keep)) {
-			module->connect(cell->getPort(ID::Q), cell->getPort(ID::D));
+			SigBit Q = cell->getPort(ID::Q);
+			module->connect(Q, cell->getPort(ID::D));
 			module->remove(cell);
+			auto Qi = initmap(Q);
+			auto it = Qi.wire->attributes.find(ID::init);
+			if (it != Qi.wire->attributes.end())
+				it->second[Qi.offset] = State::Sx;
 		}
 		else if (cell->type.in(ID($_AND_), ID($_NOT_)))
 			module->remove(cell);
@@ -1301,7 +1323,25 @@ void reintegrate(RTLIL::Module *module, bool dff_mode)
 			mapped_cell->connections_.erase(jt);
 
 			auto abc9_flop = box_module->get_bool_attribute(ID::abc9_flop);
-			if (!abc9_flop) {
+			if (abc9_flop) {
+				// Link this sole flop box output to the output of the existing
+				//   flop box, so that any (public) signal it drives will be
+				//   preserved
+				SigBit old_q;
+				for (const auto &port_name : box_ports.at(existing_cell->type)) {
+					RTLIL::Wire *w = box_module->wire(port_name);
+					log_assert(w);
+					if (!w->port_output)
+						continue;
+					log_assert(old_q == SigBit());
+					log_assert(GetSize(w) == 1);
+					old_q = existing_cell->getPort(port_name);
+				}
+				auto new_q = outputs[0];
+				new_q.wire = module->wires_.at(remap_name(new_q.wire->name));
+				module->connect(old_q,  new_q);
+			}
+			else {
 				for (const auto &i : inputs)
 					bit_users[i].insert(mapped_cell->name);
 				for (const auto &i : outputs)
@@ -1334,11 +1374,12 @@ void reintegrate(RTLIL::Module *module, bool dff_mode)
 						c.wire = module->wires_.at(remap_name(c.wire->name));
 					newsig.append(c);
 				}
-				cell->setPort(port_name, newsig);
 
 				if (w->port_input && !abc9_flop)
 					for (const auto &i : newsig)
 						bit2sinks[i].push_back(cell);
+
+				cell->setPort(port_name, std::move(newsig));
 			}
 		}
 
@@ -1400,7 +1441,7 @@ void reintegrate(RTLIL::Module *module, bool dff_mode)
 	//   treated as being "free"), in particular driving primary
 	//   outputs (real primary outputs, or cells treated as blackboxes)
 	//   or driving box inputs.
-	// Instead of just mapping those $_NOT_ gates into 2-input $lut-s
+	// Instead of just mapping those $_NOT_ gates into 1-input $lut-s
 	//   at an area and delay cost, see if it is possible to push
 	//   this $_NOT_ into the driving LUT, or into all sink LUTs.
 	// When this is not possible, (i.e. this signal drives two primary

techlibs/common/abc9_unmap.v

@@ -1,5 +1,5 @@
 (* techmap_celltype = "$__DFF_N__$abc9_flop $__DFF_P__$abc9_flop" *)
-module $__DFF_x__$abc9_flop (input C, D, Q, output n1);
+module $__DFF_x__$abc9_flop (input C, D, (* init = 1'b0 *) input Q, output n1);
   parameter _TECHMAP_CELLTYPE_ = "";
   generate if (_TECHMAP_CELLTYPE_ == "$__DFF_N__$abc9_flop")
     $_DFF_N_ _TECHMAP_REPLACE_ (.C(C), .D(D), .Q(Q));

tests/arch/xilinx/abc9_dff.ys

@@ -50,10 +50,10 @@ FDCE_1 /*#(.INIT(1))*/ fd7(.C(C), .CE(1'b0), .D(D), .CLR(1'b0), .Q(Q[6]));
 FDPE_1 #(.INIT(1)) fd8(.C(C), .CE(1'b0), .D(D), .PRE(1'b0), .Q(Q[7]));
 endmodule
 EOT
-logger -expect warning "Module '\$paramod\\FDRE\\INIT=1' contains a \$dff cell .*" 1
+logger -expect warning "Whitebox '\$paramod\\FDRE\\INIT=1' with \(\* abc9_flop \*\) contains a \$dff cell with non-zero initial state -- this is not supported for ABC9 sequential synthesis. Treating as a blackbox\." 1
-logger -expect warning "Module '\$paramod\\FDRE_1\\INIT=1' contains a \$dff cell .*" 1
+logger -expect warning "Whitebox '\$paramod\\FDRE_1\\INIT=1' with \(\* abc9_flop \*\) contains a \$dff cell with non-zero initial state -- this is not supported for ABC9 sequential synthesis. Treating as a blackbox\." 1
-logger -expect warning "Module 'FDSE' contains a \$dff cell .*" 1
+logger -expect warning "Whitebox 'FDSE' with \(\* abc9_flop \*\) contains a \$dff cell with non-zero initial state -- this is not supported for ABC9 sequential synthesis. Treating as a blackbox\." 1
-logger -expect warning "Module '\$paramod\\FDSE_1\\INIT=1' contains a \$dff cell .*" 1
+logger -expect warning "Whitebox '\$paramod\\FDSE_1\\INIT=1' with \(\* abc9_flop \*\) contains a \$dff cell with non-zero initial state -- this is not supported for ABC9 sequential synthesis. Treating as a blackbox\." 1
 equiv_opt -assert -multiclock -map +/xilinx/cells_sim.v synth_xilinx -abc9 -dff -noiopad -noclkbuf
 design -load postopt
 select -assert-count 8 t:FD*
@@ -82,4 +82,53 @@ select -assert-count 1 t:FDPE
 select -assert-count 2 t:INV
 select -assert-count 0 t:FD* t:INV %% t:* %D
 
+
+design -reset
+read_verilog <<EOT
+module top(input clk, input d, output q);
+reg r;
+always @(posedge clk) begin
+r <= d;
+end
+assign q = ~r;
+endmodule
+EOT
+proc
+equiv_opt -assert -multiclock -map +/xilinx/cells_sim.v synth_xilinx -abc9 -dff -noiopad -noclkbuf
+design -load postopt
+select -assert-count 1 t:FDRE %co w:r %i
+
+
+design -reset
+read_verilog <<EOT
+module top(input clk, input a, b, output reg q1, output q2);
+reg r;
+always @(posedge clk) begin
+    q1 <= a | b;
+    r <= ~(~a & ~b);
+end
+assign q2 = r;
+endmodule
+EOT
+proc
+equiv_opt -assert -multiclock -map +/xilinx/cells_sim.v synth_xilinx -abc9 -dff -noiopad -noclkbuf
+design -load postopt
+select -assert-count 1 t:FDRE %co %a w:r %i
+
+
+design -reset
+read_verilog <<EOT
+module top(input clk, input a, b, output o);
+reg r1, r2;
+always @(posedge clk) begin
+    r1 <= a | b;
+    r2 <= ~(~a & ~b);
+end
+assign o = r1 | r2;
+endmodule
+EOT
+proc
+equiv_opt -assert -multiclock -map +/xilinx/cells_sim.v synth_xilinx -abc9 -dff -noiopad -noclkbuf
+
+
 logger -expect-no-warnings

tests/various/abc9.ys

@@ -97,4 +97,5 @@ select -assert-count 3 t:$_DFF_N_
 select -assert-none c:ff1 c:ff2 c:ff4 %% c:* %D
 clean
 select -assert-count 2 a:init
-select -assert-none w:w w:z %% a:init %D
+select -assert-count 1 w:w a:init %i
+select -assert-count 1 c:ff4 %co c:ff4 %d %a a:init %i