abc9: fix SCC issues (#2694)
authorEddie Hung <eddie@fpgeh.com>
Tue, 30 Mar 2021 05:01:57 +0000 (22:01 -0700)
committerGitHub <noreply@github.com>
Tue, 30 Mar 2021 05:01:57 +0000 (22:01 -0700)
* 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

CODEOWNERS
backends/aiger/xaiger.cc
passes/techmap/abc9.cc
passes/techmap/abc9_ops.cc
techlibs/common/abc9_model.v
techlibs/common/abc9_unmap.v
tests/simple_abc9/abc9.box [deleted file]
tests/simple_abc9/abc9.v
tests/simple_abc9/run-test.sh

index 0419e6e4410a60513b47a1c03fbc7d92f1e2667c..0c92691f4ae7f21e6d64f9c199d82c844a074978 100644 (file)
@@ -16,6 +16,8 @@ backends/cxxrtl/               @whitequark
 passes/cmds/bugpoint.cc        @whitequark
 passes/techmap/flowmap.cc      @whitequark
 passes/opt/opt_lut.cc          @whitequark
+passes/techmap/abc9*.cc        @eddiehung
+backends/aiger/xaiger.cc       @eddiehung
 
 
 ## External Contributors
index 7ed8ff1cfce5b08a3d1b47a93d43cdde7dc67ec3..65ccc748f54c6cb8e47c8be8d51a3117b79bb928 100644 (file)
@@ -156,7 +156,7 @@ struct XAigerWriter
 
                // promote keep wires
                for (auto wire : module->wires())
-                       if (wire->get_bool_attribute(ID::keep) || wire->get_bool_attribute(ID::abc9_keep))
+                       if (wire->get_bool_attribute(ID::keep))
                                sigmap.add(wire);
 
                for (auto wire : module->wires()) {
@@ -177,11 +177,10 @@ struct XAigerWriter
                                undriven_bits.insert(bit);
                                unused_bits.insert(bit);
 
-                               bool keep = wire->get_bool_attribute(ID::abc9_keep);
-                               if (wire->port_input || keep)
+                               if (wire->port_input)
                                        input_bits.insert(bit);
 
-                               keep = keep || wire->get_bool_attribute(ID::keep);
+                               bool keep = wire->get_bool_attribute(ID::keep);
                                if (wire->port_output || keep) {
                                        if (bit != wirebit)
                                                alias_map[wirebit] = bit;
@@ -433,7 +432,7 @@ struct XAigerWriter
                        if (bit == State::Sx)
                                continue;
                        if (aig_map.count(bit))
-                               log_error("Visited AIG node more than once; this could be a combinatorial loop that has not been broken - see Yosys bug 2530\n");
+                               log_error("Visited AIG node more than once; this could be a combinatorial loop that has not been broken\n");
                        aig_map[bit] = 2*aig_m;
                }
 
index 56bb1549578babb116892a2d4b29b3697dcf163f..0fef4a9f26c59e6b69f09d5620ed88bb8be86d2f 100644 (file)
@@ -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)
index 3f3e667de472ae41f163770be047d9d380454bd5..d3bb31cd9111364252e30601a6f8e1d3575c7f74 100644 (file)
@@ -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)
index 4fee60f752a9ab8aa196d7b0ce8c9a17f9eaaac4..570a1ec40daddadadd86e1f39221b34c89d49882 100644 (file)
@@ -6,6 +6,10 @@ module $__ABC9_DELAY (input I, output O);
   endspecify
 endmodule
 
+module $__ABC9_SCC_BREAKER (input [WIDTH-1:0] I, output [WIDTH-1:0] O);
+parameter WIDTH = 0;
+endmodule
+
 (* abc9_flop, abc9_box, lib_whitebox *)
 module $__DFF_N__$abc9_flop (input C, D, Q, output n1);
   assign n1 = D;
index c39648c623af37d74c696dcab3c9f93afd6dcc3f..b1bc4fb6e288dcd891077086a63b29b8c454a7af 100644 (file)
@@ -9,3 +9,8 @@ module $__DFF_x__$abc9_flop (input C, D, (* init = 1'b0 *) input Q, output n1);
     $error("Unrecognised _TECHMAP_CELLTYPE_");
   endgenerate
 endmodule
+
+module $__ABC9_SCC_BREAKER (input [WIDTH-1:0] I, output [WIDTH-1:0] O);
+parameter WIDTH = 0;
+assign O = I;
+endmodule
diff --git a/tests/simple_abc9/abc9.box b/tests/simple_abc9/abc9.box
deleted file mode 100644 (file)
index b3c8843..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-MUXF8 1 0 3 1
-#I0 I1 S
-0 0 0 # O
index 5e969c614566ca8312ff9a802c3bfeb1516305d2..fba089b1f8bbbf009288588f1adaeaeddf821327 100644 (file)
@@ -213,7 +213,7 @@ module arbiter (clk, rst, request, acknowledge, grant, grant_valid, grant_encode
   input rst;
 endmodule
 
-(* abc9_box_id=1, blackbox *)
+(* abc9_box, blackbox *)
 module MUXF8(input I0, I1, S, output O);
 specify
     (I0 => O) = 0;
@@ -300,15 +300,29 @@ endmodule
 module abc9_test036(input A, B, S, output [1:0] O);
   (* keep *)
   MUXF8 m  (
-    .I0(I0),
-    .I1(I1),
+    .I0(A),
+    .I1(B),
     .O(O[0]),
     .S(S)
   );
   MUXF8 m2  (
-    .I0(I0),
-    .I1(I1),
+    .I0(A),
+    .I1(B),
     .O(O[1]),
     .S(S)
   );
 endmodule
+
+(* abc9_box, whitebox *)
+module MUXF7(input I0, I1, S, output O);
+assign O = S ? I1 : I0;
+specify
+    (I0 => O) = 0;
+    (I1 => O) = 0;
+    (S => O) = 0;
+endspecify
+endmodule
+
+module abc9_test037(output o);
+MUXF7 m(.I0(1'b1), .I1(1'b0), .S(o), .O(o));
+endmodule
index b48505e29adc8b4bba4f433ccd3ab606bce4652c..4a5bf01a33c8c1e6ca0c21357361571889208d7c 100755 (executable)
@@ -37,14 +37,18 @@ done
 
 cp ../simple/*.v .
 cp ../simple/*.sv .
+rm specify.v # bug 2675
 DOLLAR='?'
-exec ${MAKE:-make} -f ../tools/autotest.mk $seed *.v *.sv EXTRA_FLAGS="-n 300 -p '\
+exec ${MAKE:-make} -f ../tools/autotest.mk $seed *.v *.sv EXTRA_FLAGS="-f \"verilog -noblackbox -specify\" -n 300 -p '\
+    read_verilog -icells -lib +/abc9_model.v; \
     hierarchy; \
     synth -run coarse; \
     opt -full; \
     techmap; \
-    abc9 -lut 4 -box ../abc9.box; \
+    abc9 -lut 4; \
     clean; \
-    check -assert; \
+    check -assert * abc9_test037 %d; \
     select -assert-none t:${DOLLAR}_NOT_ t:${DOLLAR}_AND_ %%; \
-    setattr -mod -unset blackbox'"
+    setattr -mod -unset blackbox -unset whitebox'"
+
+# NOTE: Skip 'check -assert' on abc9_test037 because it intentionally has a combinatorial loop