abc9: fix SCC issues (#2694)

* xilinx: add SCC test for DSP48E1

* xilinx: Gate DSP48E1 being a whitebox behind ALLOW_WHITEBOX_DSP48E1

Have a test that checks it works through ABC9 when enabled

* abc9 to break SCCs using $__ABC9_SCC_BREAKER module

* Add test

* abc9_ops: remove refs to (* abc9_keep *) on wires

* abc9_ops: do not bypass cells in an SCC

* Add myself to CODEOWNERS for abc9*

* Fix compile

* abc9_ops: run -prep_hier before scc

* Fix tests

* Remove bug reference pending fix

* abc9: fix for -prep_hier -dff

* xaiger: restore PI handling

* abc9_ops: -prep_xaiger sigmap

* abc9_ops: -mark_scc -> -break_scc

* abc9: eliminate hard-coded abc9.box from tests

Also tidy up

* Address review

2 files changed, 52 insertions, 28 deletions

diff --git a/passes/techmap/abc9.cc b/passes/techmap/abc9.cc
index 56bb15495..0fef4a9f2 100644
--- a/passes/techmap/abc9.cc
+++ b/passes/techmap/abc9.cc
@@ -283,9 +283,14 @@ struct Abc9Pass : public ScriptPass
 
 		if (check_label("map")) {
 			if (help_mode)
-				run("abc9_ops -prep_hier -prep_bypass [-prep_dff -dff]", "(option if -dff)");
+				run("abc9_ops -prep_hier [-dff]", "(option if -dff)");
 			else
-				run(stringf("abc9_ops -prep_hier -prep_bypass %s", dff_mode ? "-prep_dff -dff" : ""));
+				run(stringf("abc9_ops -prep_hier %s", dff_mode ? "-dff" : ""));
+			run("scc -specify -set_attr abc9_scc_id {}");
+			if (help_mode)
+				run("abc9_ops -prep_bypass [-prep_dff]", "(option if -dff)");
+			else
+				run(stringf("abc9_ops -prep_bypass %s", dff_mode ? "-prep_dff" : ""));
 			if (dff_mode) {
 				run("design -copy-to $abc9_map @$abc9_flops", "(only if -dff)");
 				run("select -unset $abc9_flops", "             (only if -dff)");
@@ -330,20 +335,20 @@ struct Abc9Pass : public ScriptPass
 			run("design -stash $abc9_map");
 			run("design -load $abc9");
 			run("design -delete $abc9");
+			// Insert bypass modules (and perform +/abc9_map.v transformations), except for those cells part of a SCC
 			if (help_mode)
 				run("techmap -wb -max_iter 1 -map %$abc9_map -map +/abc9_map.v [-D DFF]", "(option if -dff)");
 			else
-				run(stringf("techmap -wb -max_iter 1 -map %%$abc9_map -map +/abc9_map.v %s", dff_mode ? "-D DFF" : ""));
+				run(stringf("techmap -wb -max_iter 1 -map %%$abc9_map -map +/abc9_map.v %s a:abc9_scc_id %%n", dff_mode ? "-D DFF" : ""));
 			run("design -delete $abc9_map");
 		}
 
 		if (check_label("pre")) {
 			run("read_verilog -icells -lib -specify +/abc9_model.v");
-			run("scc -specify -set_attr abc9_scc_id {}");
 			if (help_mode)
-				run("abc9_ops -mark_scc -prep_delays -prep_xaiger [-dff]", "(option for -dff)");
+				run("abc9_ops -break_scc -prep_delays -prep_xaiger [-dff]", "(option for -dff)");
 			else
-				run("abc9_ops -mark_scc -prep_delays -prep_xaiger" + std::string(dff_mode ? " -dff" : ""));
+				run("abc9_ops -break_scc -prep_delays -prep_xaiger" + std::string(dff_mode ? " -dff" : ""));
 			if (help_mode)
 				run("abc9_ops -prep_lut <maxlut>", "(skip if -lut or -luts)");
 			else if (!lut_mode)
diff --git a/passes/techmap/abc9_ops.cc b/passes/techmap/abc9_ops.cc
index 3f3e667de..d3bb31cd9 100644
--- a/passes/techmap/abc9_ops.cc
+++ b/passes/techmap/abc9_ops.cc
@@ -544,18 +544,31 @@ void prep_dff_unmap(RTLIL::Design *design)
 	}
 }
 
-void mark_scc(RTLIL::Module *module)
+void break_scc(RTLIL::Module *module)
 {
 	// For every unique SCC found, (arbitrarily) find the first
-	//   cell in the component, and replace its output connections
-	//   with a new wire driven by the old connection but with a
-	//   special (* abc9_keep *) attribute set (which is used by
-	//   write_xaiger to break this wire into PI and POs)
+	//   cell in the component, and interrupt all its output connections
+	//   with the $__ABC9_SCC_BREAKER cell
+
+	// Do not break SCCs which have a cell instantiating an abc9_bypass-able
+	// module (but which wouldn't have been bypassed)
+	auto design = module->design;
+	pool<RTLIL::Cell*> scc_cells;
 	pool<RTLIL::Const> ids_seen;
 	for (auto cell : module->cells()) {
 		auto it = cell->attributes.find(ID::abc9_scc_id);
 		if (it == cell->attributes.end())
 			continue;
+		scc_cells.insert(cell);
+		auto inst_module = design->module(cell->type);
+		if (inst_module && inst_module->has_attribute(ID::abc9_bypass))
+			ids_seen.insert(it->second);
+	}
+
+	SigSpec I, O;
+	for (auto cell : scc_cells) {
+		auto it = cell->attributes.find(ID::abc9_scc_id);
+		log_assert(it != cell->attributes.end());
 		auto id = it->second;
 		auto r = ids_seen.insert(id);
 		cell->attributes.erase(it);
@@ -565,12 +578,21 @@ void mark_scc(RTLIL::Module *module)
 			if (c.second.is_fully_const()) continue;
 			if (cell->output(c.first)) {
 				Wire *w = module->addWire(NEW_ID, GetSize(c.second));
-				w->set_bool_attribute(ID::abc9_keep);
-				module->connect(w, c.second);
+				I.append(w);
+				O.append(c.second);
 				c.second = w;
 			}
 		}
 	}
+
+	if (!I.empty())
+	{
+		auto cell = module->addCell(NEW_ID, ID($__ABC9_SCC_BREAKER));
+		log_assert(GetSize(I) == GetSize(O));
+		cell->setParam(ID::WIDTH, GetSize(I));
+		cell->setPort(ID::I, std::move(I));
+		cell->setPort(ID::O, std::move(O));
+	}
 }
 
 void prep_delays(RTLIL::Design *design, bool dff_mode)
@@ -721,10 +743,8 @@ void prep_xaiger(RTLIL::Module *module, bool dff)
 					bit_users[bit].insert(cell->name);
 
 			if (cell->output(conn.first) && !abc9_flop)
-				for (const auto &chunk : conn.second.chunks())
-				    if (!chunk.wire->get_bool_attribute(ID::abc9_keep))
-					    for (auto b : sigmap(SigSpec(chunk)))
-						    bit_drivers[b].insert(cell->name);
+				for (auto bit : sigmap(conn.second))
+					bit_drivers[bit].insert(cell->name);
 		}
 		toposort.node(cell->name);
 	}
@@ -1424,7 +1444,6 @@ void reintegrate(RTLIL::Module *module, bool dff_mode)
 		RTLIL::Wire *mapped_wire = mapped_mod->wire(port);
 		RTLIL::Wire *wire = module->wire(port);
 		log_assert(wire);
-		wire->attributes.erase(ID::abc9_keep);
 
 		RTLIL::Wire *remap_wire = module->wire(remap_name(port));
 		RTLIL::SigSpec signal(wire, remap_wire->start_offset-wire->start_offset, GetSize(remap_wire));
@@ -1587,11 +1606,11 @@ struct Abc9OpsPass : public Pass {
 		log("        insert `$__ABC9_DELAY' blackbox cells into the design to account for\n");
 		log("        certain required times.\n");
 		log("\n");
-		log("    -mark_scc\n");
+		log("    -break_scc\n");
 		log("        for an arbitrarily chosen cell in each unique SCC of each selected module\n");
-		log("        (tagged with an (* abc9_scc_id = <int> *) attribute), temporarily mark all\n");
-		log("        wires driven by this cell's outputs with a (* keep *) attribute in order\n");
-		log("        to break the SCC. this temporary attribute will be removed on -reintegrate.\n");
+		log("        (tagged with an (* abc9_scc_id = <int> *) attribute) interrupt all wires\n");
+		log("        driven by this cell's outputs with a temporary $__ABC9_SCC_BREAKER cell\n");
+		log("        to break the SCC.\n");
 		log("\n");
 		log("    -prep_xaiger\n");
 		log("        prepare the design for XAIGER output. this includes computing the\n");
@@ -1628,7 +1647,7 @@ struct Abc9OpsPass : public Pass {
 
 		bool check_mode = false;
 		bool prep_delays_mode = false;
-		bool mark_scc_mode = false;
+		bool break_scc_mode = false;
 		bool prep_hier_mode = false;
 		bool prep_bypass_mode = false;
 		bool prep_dff_mode = false, prep_dff_submod_mode = false, prep_dff_unmap_mode = false;
@@ -1650,8 +1669,8 @@ struct Abc9OpsPass : public Pass {
 				valid = true;
 				continue;
 			}
-			if (arg == "-mark_scc") {
-				mark_scc_mode = true;
+			if (arg == "-break_scc") {
+				break_scc_mode = true;
 				valid = true;
 				continue;
 			}
@@ -1727,7 +1746,7 @@ struct Abc9OpsPass : public Pass {
 		extra_args(args, argidx, design);
 
 		if (!valid)
-			log_cmd_error("At least one of -check, -mark_scc, -prep_{delays,xaiger,dff[123],lut,box}, -write_{lut,box}, -reintegrate must be specified.\n");
+			log_cmd_error("At least one of -check, -break_scc, -prep_{delays,xaiger,dff[123],lut,box}, -write_{lut,box}, -reintegrate must be specified.\n");
 
 		if (dff_mode && !check_mode && !prep_hier_mode && !prep_delays_mode && !prep_xaiger_mode && !reintegrate_mode)
 			log_cmd_error("'-dff' option is only relevant for -prep_{hier,delay,xaiger} or -reintegrate.\n");
@@ -1764,8 +1783,8 @@ struct Abc9OpsPass : public Pass {
 				write_lut(mod, write_lut_dst);
 			if (!write_box_dst.empty())
 				write_box(mod, write_box_dst);
-			if (mark_scc_mode)
-				mark_scc(mod);
+			if (break_scc_mode)
+				break_scc(mod);
 			if (prep_xaiger_mode)
 				prep_xaiger(mod, dff_mode);
 			if (reintegrate_mode)